From e1df25c9f4f9e8f0174b4fc55eda7632f8640357 Mon Sep 17 00:00:00 2001 From: gallo Date: Thu, 31 Oct 2024 20:42:09 +0100 Subject: [PATCH 01/57] chore: cleanup --- src/BribeInitiative.sol | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/BribeInitiative.sol b/src/BribeInitiative.sol index c39434fd..584d3e54 100644 --- a/src/BribeInitiative.sol +++ b/src/BribeInitiative.sol @@ -99,13 +99,7 @@ contract BribeInitiative is IInitiative, IBribeInitiative { ); (uint88 totalLQTY, uint32 totalAverageTimestamp) = _decodeLQTYAllocation(totalLQTYAllocation.value); - // WHAT HAPPENS IF WE ENFORCE EPOCH AT END? - // THEN WE LINEARLY TRANSLATE TO EPOCH END? - // EPOCH_START + epoch * EPOCH_DURATION is the time to claim (I think) - // Since epoch 1 starts at Epoch Start, epoch * Duration goes to the end of - // But is this safe vs last second of the epoch? - // I recall having a similar issue already with Velodrome uint32 epochEnd = uint32(governance.EPOCH_START()) + uint32(_epoch) * uint32(governance.EPOCH_DURATION()); /// @audit User Invariant From b18c629214884250dab1b7c5c938b1e94c421e14 Mon Sep 17 00:00:00 2001 From: gallo Date: Thu, 31 Oct 2024 21:02:28 +0100 Subject: [PATCH 02/57] chore: e2E --- test/E2E.t.sol | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/test/E2E.t.sol b/test/E2E.t.sol index 5ce38029..078bb8eb 100644 --- a/test/E2E.t.sol +++ b/test/E2E.t.sol @@ -121,6 +121,24 @@ contract E2ETests is Test { _allocate(address(0x123123), 1e18, 0); } + // forge test --match-test test_noVetoGriefAtEpochOne -vv + function test_noVetoGriefAtEpochOne() public { + /// @audit NOTE: In order for this to work, the constructor must set the start time a week behind + /// This will make the initiatives work on the first epoch + vm.startPrank(user); + // Check that we can vote on the first epoch, right after deployment + _deposit(1000e18); + + console.log("epoch", governance.epoch()); + _allocate(baseInitiative1, 0, 1e18); // Doesn't work due to cool down I think + + vm.expectRevert(); + governance.unregisterInitiative(baseInitiative1); + + vm.warp(block.timestamp + EPOCH_DURATION); + governance.unregisterInitiative(baseInitiative1); + } + // forge test --match-test test_deregisterIsSound -vv function test_deregisterIsSound() public { // Deregistration works as follows: @@ -292,8 +310,8 @@ contract E2ETests is Test { ++skipCount; assertEq( uint256(Governance.InitiativeStatus.CLAIMABLE), _getInitiativeStatus(newInitiative), "UNREGISTERABLE" - ); } - + ); + } function _deposit(uint88 amt) internal { address userProxy = governance.deployUserProxy(); From f693d5f26fe1d5a3ac96fa8bf9d24c214aa3f5c3 Mon Sep 17 00:00:00 2001 From: gallo Date: Thu, 31 Oct 2024 21:04:45 +0100 Subject: [PATCH 03/57] fix: cap amount for bribes --- src/CurveV2GaugeRewards.sol | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/CurveV2GaugeRewards.sol b/src/CurveV2GaugeRewards.sol index dfe6da5c..e46be99e 100644 --- a/src/CurveV2GaugeRewards.sol +++ b/src/CurveV2GaugeRewards.sol @@ -38,6 +38,11 @@ contract CurveV2GaugeRewards is BribeInitiative { remainder = 0; + uint256 available = bold.balanceOf(address(this)); + if(available < total) { + total = available; // Cap due to rounding error causing a bit more bold being given away + } + bold.approve(address(gauge), total); gauge.deposit_reward_token(address(bold), total, duration); From 01457bd3bd6706594dccb6922c23105bb14980f3 Mon Sep 17 00:00:00 2001 From: gallo Date: Thu, 31 Oct 2024 21:10:53 +0100 Subject: [PATCH 04/57] chore: desc --- test/recon/properties/OptimizationProperties.sol | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/test/recon/properties/OptimizationProperties.sol b/test/recon/properties/OptimizationProperties.sol index 925ca819..f7c327a8 100644 --- a/test/recon/properties/OptimizationProperties.sol +++ b/test/recon/properties/OptimizationProperties.sol @@ -9,9 +9,10 @@ import {vm} from "@chimera/Hevm.sol"; import {IUserProxy} from "src/interfaces/IUserProxy.sol"; import {GovernanceProperties} from "./GovernanceProperties.sol"; +// NOTE: These run only if you use `optimization` mode and set the correct prefix +// See echidna.yaml abstract contract OptimizationProperties is GovernanceProperties { - // TODO: Add Optimization for property_sum_of_user_voting_weights function optimize_max_sum_of_user_voting_weights_insolvent() public returns (int256) { VotesSumAndInitiativeSum[] memory results = _getUserVotesSumAndInitiativesVotes(); @@ -42,8 +43,6 @@ abstract contract OptimizationProperties is GovernanceProperties { } - - // TODO: Add Optimization for property_sum_of_lqty_global_user_matches function optimize_property_sum_of_lqty_global_user_matches_insolvency() public returns (int256) { int256 max = 0; @@ -69,8 +68,6 @@ abstract contract OptimizationProperties is GovernanceProperties { return max; } - // TODO: Add Optimization for property_sum_of_initatives_matches_total_votes - function optimize_property_sum_of_initatives_matches_total_votes_insolvency() public returns (int256) { int256 max = 0; From a589a29ecd8c0006031aabfb32d6ebce1045ec90 Mon Sep 17 00:00:00 2001 From: gallo Date: Fri, 1 Nov 2024 09:26:45 +0100 Subject: [PATCH 05/57] feat: upscale ts to u120 --- src/BribeInitiative.sol | 26 ++++++++------ src/Governance.sol | 53 +++++++++++++++++------------ src/interfaces/IBribeInitiative.sol | 8 ++--- src/interfaces/IGovernance.sol | 20 +++++------ src/utils/EncodingDecodingLib.sol | 8 ++--- 5 files changed, 65 insertions(+), 50 deletions(-) diff --git a/src/BribeInitiative.sol b/src/BribeInitiative.sol index 584d3e54..c9e8da37 100644 --- a/src/BribeInitiative.sol +++ b/src/BribeInitiative.sol @@ -45,12 +45,12 @@ contract BribeInitiative is IInitiative, IBribeInitiative { } /// @inheritdoc IBribeInitiative - function totalLQTYAllocatedByEpoch(uint16 _epoch) external view returns (uint88, uint32) { + function totalLQTYAllocatedByEpoch(uint16 _epoch) external view returns (uint88, uint120) { return _loadTotalLQTYAllocation(_epoch); } /// @inheritdoc IBribeInitiative - function lqtyAllocatedByUserAtEpoch(address _user, uint16 _epoch) external view returns (uint88, uint32) { + function lqtyAllocatedByUserAtEpoch(address _user, uint16 _epoch) external view returns (uint88, uint120) { return _loadLQTYAllocation(_user, _epoch); } @@ -70,6 +70,9 @@ contract BribeInitiative is IInitiative, IBribeInitiative { bribeToken.safeTransferFrom(msg.sender, address(this), _bribeTokenAmount); } + uint256 constant WAD = 1e18; + + function _claimBribe( address _user, uint16 _epoch, @@ -98,9 +101,10 @@ contract BribeInitiative is IInitiative, IBribeInitiative { "BribeInitiative: invalid-prev-total-lqty-allocation-epoch" ); - (uint88 totalLQTY, uint32 totalAverageTimestamp) = _decodeLQTYAllocation(totalLQTYAllocation.value); + (uint88 totalLQTY, uint120 totalAverageTimestamp) = _decodeLQTYAllocation(totalLQTYAllocation.value); - uint32 epochEnd = uint32(governance.EPOCH_START()) + uint32(_epoch) * uint32(governance.EPOCH_DURATION()); + // TODO: SCALING!!! + uint120 epochEnd = (uint120(governance.EPOCH_START()) + uint120(_epoch) * uint120(governance.EPOCH_DURATION())) * uint120(WAD); /// @audit User Invariant assert(totalAverageTimestamp <= epochEnd); /// NOTE: Tests break because they are not realistic @@ -108,7 +112,7 @@ contract BribeInitiative is IInitiative, IBribeInitiative { uint240 totalVotes = governance.lqtyToVotes(totalLQTY, epochEnd, totalAverageTimestamp); if (totalVotes != 0) { - (uint88 lqty, uint32 averageTimestamp) = _decodeLQTYAllocation(lqtyAllocation.value); + (uint88 lqty, uint120 averageTimestamp) = _decodeLQTYAllocation(lqtyAllocation.value); /// @audit Governance Invariant assert(averageTimestamp <= epochEnd); /// NOTE: Tests break because they are not realistic @@ -159,7 +163,7 @@ contract BribeInitiative is IInitiative, IBribeInitiative { /// @inheritdoc IInitiative function onUnregisterInitiative(uint16) external virtual override onlyGovernance {} - function _setTotalLQTYAllocationByEpoch(uint16 _epoch, uint88 _lqty, uint32 _averageTimestamp, bool _insert) + function _setTotalLQTYAllocationByEpoch(uint16 _epoch, uint88 _lqty, uint120 _averageTimestamp, bool _insert) private { uint224 value = _encodeLQTYAllocation(_lqty, _averageTimestamp); @@ -175,7 +179,7 @@ contract BribeInitiative is IInitiative, IBribeInitiative { address _user, uint16 _epoch, uint88 _lqty, - uint32 _averageTimestamp, + uint120 _averageTimestamp, bool _insert ) private { uint224 value = _encodeLQTYAllocation(_lqty, _averageTimestamp); @@ -187,20 +191,20 @@ contract BribeInitiative is IInitiative, IBribeInitiative { emit ModifyLQTYAllocation(_user, _epoch, _lqty, _averageTimestamp); } - function _encodeLQTYAllocation(uint88 _lqty, uint32 _averageTimestamp) private pure returns (uint224) { + function _encodeLQTYAllocation(uint88 _lqty, uint120 _averageTimestamp) private pure returns (uint224) { return EncodingDecodingLib.encodeLQTYAllocation(_lqty, _averageTimestamp); } - function _decodeLQTYAllocation(uint224 _value) private pure returns (uint88, uint32) { + function _decodeLQTYAllocation(uint224 _value) private pure returns (uint88, uint120) { return EncodingDecodingLib.decodeLQTYAllocation(_value); } - function _loadTotalLQTYAllocation(uint16 _epoch) private view returns (uint88, uint32) { + function _loadTotalLQTYAllocation(uint16 _epoch) private view returns (uint88, uint120) { require(_epoch <= governance.epoch(), "No future Lookup"); return _decodeLQTYAllocation(totalLQTYAllocationByEpoch.items[_epoch].value); } - function _loadLQTYAllocation(address _user, uint16 _epoch) private view returns (uint88, uint32) { + function _loadLQTYAllocation(address _user, uint16 _epoch) private view returns (uint88, uint120) { require(_epoch <= governance.epoch(), "No future Lookup"); return _decodeLQTYAllocation(lqtyAllocationByUserAtEpoch[_user].items[_epoch].value); } diff --git a/src/Governance.sol b/src/Governance.sol index dbd8f8ef..df7063f7 100644 --- a/src/Governance.sol +++ b/src/Governance.sol @@ -118,39 +118,47 @@ contract Governance is Multicall, UserProxyFactory, ReentrancyGuard, IGovernance } } - function _averageAge(uint32 _currentTimestamp, uint32 _averageTimestamp) internal pure returns (uint32) { + function _averageAge(uint120 _currentTimestamp, uint120 _averageTimestamp) internal pure returns (uint120) { if (_averageTimestamp == 0 || _currentTimestamp < _averageTimestamp) return 0; return _currentTimestamp - _averageTimestamp; } function _calculateAverageTimestamp( - uint32 _prevOuterAverageTimestamp, - uint32 _newInnerAverageTimestamp, + uint120 _prevOuterAverageTimestamp, + uint120 _newInnerAverageTimestamp, uint88 _prevLQTYBalance, uint88 _newLQTYBalance - ) internal view returns (uint32) { + ) internal view returns (uint120) { if (_newLQTYBalance == 0) return 0; - uint32 prevOuterAverageAge = _averageAge(uint32(block.timestamp), _prevOuterAverageTimestamp); - uint32 newInnerAverageAge = _averageAge(uint32(block.timestamp), _newInnerAverageTimestamp); + // NOTE: Truncation + // NOTE: u32 -> u120 + /// @audit Investigate this + uint120 currentTime = uint120(uint32(block.timestamp)) * uint120(WAD); - uint88 newOuterAverageAge; + uint120 prevOuterAverageAge = _averageAge(currentTime, _prevOuterAverageTimestamp); + uint120 newInnerAverageAge = _averageAge(currentTime, _newInnerAverageTimestamp); + + // 120 for timestamps + // 208 for voting power + + uint120 newOuterAverageAge; if (_prevLQTYBalance <= _newLQTYBalance) { uint88 deltaLQTY = _newLQTYBalance - _prevLQTYBalance; - uint240 prevVotes = uint240(_prevLQTYBalance) * uint240(prevOuterAverageAge); - uint240 newVotes = uint240(deltaLQTY) * uint240(newInnerAverageAge); - uint240 votes = prevVotes + newVotes; - newOuterAverageAge = uint32(votes / uint240(_newLQTYBalance)); + uint208 prevVotes = uint208(_prevLQTYBalance) * uint208(prevOuterAverageAge); + uint208 newVotes = uint208(deltaLQTY) * uint208(newInnerAverageAge); + uint208 votes = prevVotes + newVotes; + newOuterAverageAge = uint120(votes / uint208(_newLQTYBalance)); } else { uint88 deltaLQTY = _prevLQTYBalance - _newLQTYBalance; - uint240 prevVotes = uint240(_prevLQTYBalance) * uint240(prevOuterAverageAge); - uint240 newVotes = uint240(deltaLQTY) * uint240(newInnerAverageAge); - uint240 votes = (prevVotes >= newVotes) ? prevVotes - newVotes : 0; - newOuterAverageAge = uint32(votes / uint240(_newLQTYBalance)); + uint208 prevVotes = uint208(_prevLQTYBalance) * uint208(prevOuterAverageAge); + uint208 newVotes = uint208(deltaLQTY) * uint208(newInnerAverageAge); + uint208 votes = (prevVotes >= newVotes) ? prevVotes - newVotes : 0; + newOuterAverageAge = uint120(votes / uint208(_newLQTYBalance)); } - if (newOuterAverageAge > block.timestamp) return 0; - return uint32(block.timestamp - newOuterAverageAge); + if (newOuterAverageAge > currentTime) return 0; + return uint120(currentTime - newOuterAverageAge); } /*////////////////////////////////////////////////////////////// @@ -176,8 +184,11 @@ contract Governance is Multicall, UserProxyFactory, ReentrancyGuard, IGovernance // update the average staked timestamp for LQTY staked by the user + /// @audit TODO: u32 -> u120 + // IMO: Define a shutdown time at which all math is ignored + // if TS > u32 -> Just withdraw and don't check userState.averageStakingTimestamp = _calculateAverageTimestamp( - userState.averageStakingTimestamp, uint32(block.timestamp), lqtyStaked, lqtyStaked + _lqtyAmount + userState.averageStakingTimestamp, uint120(block.timestamp * WAD), lqtyStaked, lqtyStaked + _lqtyAmount ); userStates[msg.sender] = userState; @@ -248,12 +259,12 @@ contract Governance is Multicall, UserProxyFactory, ReentrancyGuard, IGovernance } /// @inheritdoc IGovernance - function lqtyToVotes(uint88 _lqtyAmount, uint256 _currentTimestamp, uint32 _averageTimestamp) + function lqtyToVotes(uint88 _lqtyAmount, uint120 _currentTimestamp, uint120 _averageTimestamp) public pure - returns (uint240) + returns (uint208) { - return uint240(_lqtyAmount) * _averageAge(uint32(_currentTimestamp), _averageTimestamp); + return uint208(_lqtyAmount) * uint208(_averageAge(_currentTimestamp, _averageTimestamp)); } /*////////////////////////////////////////////////////////////// diff --git a/src/interfaces/IBribeInitiative.sol b/src/interfaces/IBribeInitiative.sol index dd58e931..061bdf3a 100644 --- a/src/interfaces/IBribeInitiative.sol +++ b/src/interfaces/IBribeInitiative.sol @@ -7,8 +7,8 @@ import {IGovernance} from "./IGovernance.sol"; interface IBribeInitiative { event DepositBribe(address depositor, uint128 boldAmount, uint128 bribeTokenAmount, uint16 epoch); - event ModifyLQTYAllocation(address user, uint16 epoch, uint88 lqtyAllocated, uint32 averageTimestamp); - event ModifyTotalLQTYAllocation(uint16 epoch, uint88 totalLQTYAllocated, uint32 averageTimestamp); + event ModifyLQTYAllocation(address user, uint16 epoch, uint88 lqtyAllocated, uint120 averageTimestamp); + event ModifyTotalLQTYAllocation(uint16 epoch, uint88 totalLQTYAllocated, uint120 averageTimestamp); event ClaimBribe(address user, uint16 epoch, uint256 boldAmount, uint256 bribeTokenAmount); /// @notice Address of the governance contract @@ -43,7 +43,7 @@ interface IBribeInitiative { function totalLQTYAllocatedByEpoch(uint16 _epoch) external view - returns (uint88 totalLQTYAllocated, uint32 averageTimestamp); + returns (uint88 totalLQTYAllocated, uint120 averageTimestamp); /// @notice LQTY allocated by a user to the initiative at a given epoch /// @param _user Address of the user /// @param _epoch Epoch at which the LQTY was allocated by the user @@ -51,7 +51,7 @@ interface IBribeInitiative { function lqtyAllocatedByUserAtEpoch(address _user, uint16 _epoch) external view - returns (uint88 lqtyAllocated, uint32 averageTimestamp); + returns (uint88 lqtyAllocated, uint120 averageTimestamp); /// @notice Deposit bribe tokens for a given epoch /// @dev The caller has to approve this contract to spend the BOLD and bribe tokens. diff --git a/src/interfaces/IGovernance.sol b/src/interfaces/IGovernance.sol index 834c6d12..d25f3e95 100644 --- a/src/interfaces/IGovernance.sol +++ b/src/interfaces/IGovernance.sol @@ -117,20 +117,20 @@ interface IGovernance { struct UserState { uint88 allocatedLQTY; // LQTY allocated by the user - uint32 averageStakingTimestamp; // Average timestamp at which LQTY was staked by the user + uint120 averageStakingTimestamp; // Average timestamp at which LQTY was staked by the user } struct InitiativeState { uint88 voteLQTY; // LQTY allocated vouching for the initiative uint88 vetoLQTY; // LQTY allocated vetoing the initiative - uint32 averageStakingTimestampVoteLQTY; // Average staking timestamp of the voting LQTY for the initiative - uint32 averageStakingTimestampVetoLQTY; // Average staking timestamp of the vetoing LQTY for the initiative + uint120 averageStakingTimestampVoteLQTY; // Average staking timestamp of the voting LQTY for the initiative + uint120 averageStakingTimestampVetoLQTY; // Average staking timestamp of the vetoing LQTY for the initiative uint16 lastEpochClaim; } struct GlobalState { uint88 countedVoteLQTY; // Total LQTY that is included in vote counting - uint32 countedVoteLQTYAverageTimestamp; // Average timestamp: derived initiativeAllocation.averageTimestamp + uint120 countedVoteLQTYAverageTimestamp; // Average timestamp: derived initiativeAllocation.averageTimestamp } /// TODO: Bold balance? Prob cheaper @@ -138,7 +138,7 @@ interface IGovernance { /// @param _user Address of the user /// @return allocatedLQTY LQTY allocated by the user /// @return averageStakingTimestamp Average timestamp at which LQTY was staked (deposited) by the user - function userStates(address _user) external view returns (uint88 allocatedLQTY, uint32 averageStakingTimestamp); + function userStates(address _user) external view returns (uint88 allocatedLQTY, uint120 averageStakingTimestamp); /// @notice Returns the initiative's state /// @param _initiative Address of the initiative /// @return voteLQTY LQTY allocated vouching for the initiative @@ -152,14 +152,14 @@ interface IGovernance { returns ( uint88 voteLQTY, uint88 vetoLQTY, - uint32 averageStakingTimestampVoteLQTY, - uint32 averageStakingTimestampVetoLQTY, + uint120 averageStakingTimestampVoteLQTY, + uint120 averageStakingTimestampVetoLQTY, uint16 lastEpochClaim ); /// @notice Returns the global state /// @return countedVoteLQTY Total LQTY that is included in vote counting /// @return countedVoteLQTYAverageTimestamp Average timestamp: derived initiativeAllocation.averageTimestamp - function globalState() external view returns (uint88 countedVoteLQTY, uint32 countedVoteLQTYAverageTimestamp); + function globalState() external view returns (uint88 countedVoteLQTY, uint120 countedVoteLQTYAverageTimestamp); /// @notice Returns the amount of voting and vetoing LQTY a user allocated to an initiative /// @param _user Address of the user /// @param _initiative Address of the initiative @@ -215,10 +215,10 @@ interface IGovernance { /// @param _currentTimestamp Current timestamp /// @param _averageTimestamp Average timestamp at which the LQTY was staked /// @return votes Number of votes - function lqtyToVotes(uint88 _lqtyAmount, uint256 _currentTimestamp, uint32 _averageTimestamp) + function lqtyToVotes(uint88 _lqtyAmount, uint120 _currentTimestamp, uint120 _averageTimestamp) external pure - returns (uint240); + returns (uint208); /// @notice Voting threshold is the max. of either: /// - 4% of the total voting LQTY in the previous epoch diff --git a/src/utils/EncodingDecodingLib.sol b/src/utils/EncodingDecodingLib.sol index 5173b166..79026859 100644 --- a/src/utils/EncodingDecodingLib.sol +++ b/src/utils/EncodingDecodingLib.sol @@ -2,12 +2,12 @@ pragma solidity ^0.8.24; library EncodingDecodingLib { - function encodeLQTYAllocation(uint88 _lqty, uint32 _averageTimestamp) internal pure returns (uint224) { - uint224 _value = (uint224(_lqty) << 32) | _averageTimestamp; + function encodeLQTYAllocation(uint88 _lqty, uint120 _averageTimestamp) internal pure returns (uint224) { + uint224 _value = (uint224(_lqty) << 120) | _averageTimestamp; return _value; } - function decodeLQTYAllocation(uint224 _value) internal pure returns (uint88, uint32) { - return (uint88(_value >> 32), uint32(_value)); + function decodeLQTYAllocation(uint224 _value) internal pure returns (uint88, uint120) { + return (uint88(_value >> 120), uint120(_value)); } } From 7e777aedcab12c7ed088726961bd050ff7cfc013 Mon Sep 17 00:00:00 2001 From: gallo Date: Fri, 1 Nov 2024 09:56:27 +0100 Subject: [PATCH 06/57] feat: tests somewhat compile --- test/BribeInitiativeAllocate.t.sol | 226 ++++++++++++++--------------- test/EncodingDecoding.t.sol | 15 +- test/Governance.t.sol | 170 +++++++++++----------- test/GovernanceAttacks.t.sol | 4 +- test/VotingPower.t.sol | 2 +- 5 files changed, 206 insertions(+), 211 deletions(-) diff --git a/test/BribeInitiativeAllocate.t.sol b/test/BribeInitiativeAllocate.t.sol index 860d8cda..54a9d8d4 100644 --- a/test/BribeInitiativeAllocate.t.sol +++ b/test/BribeInitiativeAllocate.t.sol @@ -80,14 +80,14 @@ contract BribeInitiativeAllocateTest is Test { }); bribeInitiative.onAfterAllocateLQTY(governance.epoch(), user2, userState, allocation, initiativeState); } - (uint88 totalLQTYAllocated, uint32 totalAverageTimestamp) = + (uint88 totalLQTYAllocated, uint120 totalAverageTimestamp) = bribeInitiative.totalLQTYAllocatedByEpoch(governance.epoch()); assertEq(totalLQTYAllocated, 1e18); - assertEq(totalAverageTimestamp, uint32(block.timestamp)); - (uint88 userLQTYAllocated, uint32 userAverageTimestamp) = + assertEq(totalAverageTimestamp, uint120(block.timestamp)); + (uint88 userLQTYAllocated, uint120 userAverageTimestamp) = bribeInitiative.lqtyAllocatedByUserAtEpoch(user2, governance.epoch()); assertEq(userLQTYAllocated, 1e18); - assertEq(userAverageTimestamp, uint32(block.timestamp)); + assertEq(userAverageTimestamp, uint120(block.timestamp)); { IGovernance.UserState memory userState2 = @@ -104,11 +104,11 @@ contract BribeInitiativeAllocateTest is Test { bribeInitiative.onAfterAllocateLQTY(governance.epoch(), user, userState2, allocation2, initiativeState2); } - (uint88 totalLQTYAllocated2, uint32 totalAverageTimestamp2) = + (uint88 totalLQTYAllocated2, uint120 totalAverageTimestamp2) = bribeInitiative.totalLQTYAllocatedByEpoch(governance.epoch()); assertEq(totalLQTYAllocated2, 1001e18); assertEq(totalAverageTimestamp2, block.timestamp); - (uint88 userLQTYAllocated2, uint32 userAverageTimestamp2) = + (uint88 userLQTYAllocated2, uint120 userAverageTimestamp2) = bribeInitiative.lqtyAllocatedByUserAtEpoch(user, governance.epoch()); assertEq(userLQTYAllocated2, 1000e18); assertEq(userAverageTimestamp2, block.timestamp); @@ -177,14 +177,14 @@ contract BribeInitiativeAllocateTest is Test { lastEpochClaim: 0 }); bribeInitiative.onAfterAllocateLQTY(governance.epoch(), user2, userState, allocation, initiativeState); - (uint88 totalLQTYAllocated, uint32 totalAverageTimestamp) = + (uint88 totalLQTYAllocated, uint120 totalAverageTimestamp) = bribeInitiative.totalLQTYAllocatedByEpoch(governance.epoch()); assertEq(totalLQTYAllocated, 1e18); - assertEq(totalAverageTimestamp, uint32(block.timestamp)); - (uint88 userLQTYAllocated, uint32 userAverageTimestamp) = + assertEq(totalAverageTimestamp, uint120(block.timestamp)); + (uint88 userLQTYAllocated, uint120 userAverageTimestamp) = bribeInitiative.lqtyAllocatedByUserAtEpoch(user2, governance.epoch()); assertEq(userLQTYAllocated, 1e18); - assertEq(userAverageTimestamp, uint32(block.timestamp)); + assertEq(userAverageTimestamp, uint120(block.timestamp)); } // set user2 allocations like governance would using onAfterAllocateLQTY at epoch 1 @@ -202,14 +202,14 @@ contract BribeInitiativeAllocateTest is Test { }); bribeInitiative.onAfterAllocateLQTY(governance.epoch(), user2, userState, allocation, initiativeState); - (uint88 totalLQTYAllocated, uint32 totalAverageTimestamp) = + (uint88 totalLQTYAllocated, uint120 totalAverageTimestamp) = bribeInitiative.totalLQTYAllocatedByEpoch(governance.epoch()); assertEq(totalLQTYAllocated, 1001e18); - assertEq(totalAverageTimestamp, uint32(block.timestamp)); - (uint88 userLQTYAllocated, uint32 userAverageTimestamp) = + assertEq(totalAverageTimestamp, uint120(block.timestamp)); + (uint88 userLQTYAllocated, uint120 userAverageTimestamp) = bribeInitiative.lqtyAllocatedByUserAtEpoch(user2, governance.epoch()); assertEq(userLQTYAllocated, 1e18); - assertEq(userAverageTimestamp, uint32(block.timestamp)); + assertEq(userAverageTimestamp, uint120(block.timestamp)); } // lusdHolder deposits bribes into the initiative @@ -238,14 +238,14 @@ contract BribeInitiativeAllocateTest is Test { lastEpochClaim: 0 }); bribeInitiative.onAfterAllocateLQTY(governance.epoch(), user, userState, allocation, initiativeState); - (uint88 totalLQTYAllocated, uint32 totalAverageTimestamp) = + (uint88 totalLQTYAllocated, uint120 totalAverageTimestamp) = bribeInitiative.totalLQTYAllocatedByEpoch(governance.epoch()); assertEq(totalLQTYAllocated, 0); - assertEq(totalAverageTimestamp, uint32(block.timestamp)); - (uint88 userLQTYAllocated, uint32 userAverageTimestamp) = + assertEq(totalAverageTimestamp, uint120(block.timestamp)); + (uint88 userLQTYAllocated, uint120 userAverageTimestamp) = bribeInitiative.lqtyAllocatedByUserAtEpoch(user, governance.epoch()); assertEq(userLQTYAllocated, 0); - assertEq(userAverageTimestamp, uint32(block.timestamp)); + assertEq(userAverageTimestamp, uint120(block.timestamp)); } // set allocation in initiative for user2 in epoch 1 @@ -262,14 +262,14 @@ contract BribeInitiativeAllocateTest is Test { lastEpochClaim: 0 }); bribeInitiative.onAfterAllocateLQTY(governance.epoch(), user2, userState, allocation, initiativeState); - (uint88 totalLQTYAllocated, uint32 totalAverageTimestamp) = + (uint88 totalLQTYAllocated, uint120 totalAverageTimestamp) = bribeInitiative.totalLQTYAllocatedByEpoch(governance.epoch()); assertEq(totalLQTYAllocated, 0); - assertEq(totalAverageTimestamp, uint32(block.timestamp)); - (uint88 userLQTYAllocated, uint32 userAverageTimestamp) = + assertEq(totalAverageTimestamp, uint120(block.timestamp)); + (uint88 userLQTYAllocated, uint120 userAverageTimestamp) = bribeInitiative.lqtyAllocatedByUserAtEpoch(user2, governance.epoch()); assertEq(userLQTYAllocated, 0); - assertEq(userAverageTimestamp, uint32(block.timestamp)); + assertEq(userAverageTimestamp, uint120(block.timestamp)); } governance.setEpoch(3); @@ -305,14 +305,14 @@ contract BribeInitiativeAllocateTest is Test { }); bribeInitiative.onAfterAllocateLQTY(governance.epoch(), user2, userState, allocation, initiativeState); - (uint88 totalLQTYAllocated, uint32 totalAverageTimestamp) = + (uint88 totalLQTYAllocated, uint120 totalAverageTimestamp) = bribeInitiative.totalLQTYAllocatedByEpoch(governance.epoch()); assertEq(totalLQTYAllocated, 1e18); - assertEq(totalAverageTimestamp, uint32(block.timestamp)); - (uint88 userLQTYAllocated, uint32 userAverageTimestamp) = + assertEq(totalAverageTimestamp, uint120(block.timestamp)); + (uint88 userLQTYAllocated, uint120 userAverageTimestamp) = bribeInitiative.lqtyAllocatedByUserAtEpoch(user2, governance.epoch()); assertEq(userLQTYAllocated, 1e18); - assertEq(userAverageTimestamp, uint32(block.timestamp)); + assertEq(userAverageTimestamp, uint120(block.timestamp)); IGovernance.UserState memory userStateVeto = IGovernance.UserState({allocatedLQTY: 1000e18, averageStakingTimestamp: uint32(block.timestamp)}); @@ -329,14 +329,14 @@ contract BribeInitiativeAllocateTest is Test { governance.epoch(), user, userStateVeto, allocationVeto, initiativeStateVeto ); - (uint88 totalLQTYAllocatedAfterVeto, uint32 totalAverageTimestampAfterVeto) = + (uint88 totalLQTYAllocatedAfterVeto, uint120 totalAverageTimestampAfterVeto) = bribeInitiative.totalLQTYAllocatedByEpoch(governance.epoch()); assertEq(totalLQTYAllocatedAfterVeto, 1e18); - assertEq(totalAverageTimestampAfterVeto, uint32(block.timestamp)); - (uint88 userLQTYAllocatedAfterVeto, uint32 userAverageTimestampAfterVeto) = + assertEq(totalAverageTimestampAfterVeto, uint120(block.timestamp)); + (uint88 userLQTYAllocatedAfterVeto, uint120 userAverageTimestampAfterVeto) = bribeInitiative.lqtyAllocatedByUserAtEpoch(user, governance.epoch()); assertEq(userLQTYAllocatedAfterVeto, 0); - assertEq(userAverageTimestampAfterVeto, uint32(block.timestamp)); + assertEq(userAverageTimestampAfterVeto, uint120(block.timestamp)); governance.setEpoch(2); vm.warp(block.timestamp + governance.EPOCH_DURATION()); // warp to second epoch ts @@ -356,14 +356,14 @@ contract BribeInitiativeAllocateTest is Test { governance.epoch(), user, userStateNewEpoch, allocationNewEpoch, initiativeStateNewEpoch ); - (uint88 totalLQTYAllocatedNewEpoch, uint32 totalAverageTimestampNewEpoch) = + (uint88 totalLQTYAllocatedNewEpoch, uint120 totalAverageTimestampNewEpoch) = bribeInitiative.totalLQTYAllocatedByEpoch(governance.epoch()); assertEq(totalLQTYAllocatedNewEpoch, 1e18); - assertEq(totalAverageTimestampNewEpoch, uint32(block.timestamp)); - (uint88 userLQTYAllocatedNewEpoch, uint32 userAverageTimestampNewEpoch) = + assertEq(totalAverageTimestampNewEpoch, uint120(block.timestamp)); + (uint88 userLQTYAllocatedNewEpoch, uint120 userAverageTimestampNewEpoch) = bribeInitiative.lqtyAllocatedByUserAtEpoch(user, governance.epoch()); assertEq(userLQTYAllocatedNewEpoch, 0); - assertEq(userAverageTimestampNewEpoch, uint32(block.timestamp)); + assertEq(userAverageTimestampNewEpoch, uint120(block.timestamp)); vm.startPrank(lusdHolder); lqty.approve(address(bribeInitiative), 1000e18); @@ -391,14 +391,14 @@ contract BribeInitiativeAllocateTest is Test { governance.epoch(), user, userStateNewEpoch3, allocationNewEpoch3, initiativeStateNewEpoch3 ); - (uint88 totalLQTYAllocatedNewEpoch3, uint32 totalAverageTimestampNewEpoch3) = + (uint88 totalLQTYAllocatedNewEpoch3, uint120 totalAverageTimestampNewEpoch3) = bribeInitiative.totalLQTYAllocatedByEpoch(governance.epoch()); assertEq(totalLQTYAllocatedNewEpoch3, 2001e18); - assertEq(totalAverageTimestampNewEpoch3, uint32(block.timestamp)); - (uint88 userLQTYAllocatedNewEpoch3, uint32 userAverageTimestampNewEpoch3) = + assertEq(totalAverageTimestampNewEpoch3, uint120(block.timestamp)); + (uint88 userLQTYAllocatedNewEpoch3, uint120 userAverageTimestampNewEpoch3) = bribeInitiative.lqtyAllocatedByUserAtEpoch(user, governance.epoch()); assertEq(userLQTYAllocatedNewEpoch3, 2000e18); - assertEq(userAverageTimestampNewEpoch3, uint32(block.timestamp)); + assertEq(userAverageTimestampNewEpoch3, uint120(block.timestamp)); governance.setEpoch(4); vm.warp(block.timestamp + governance.EPOCH_DURATION()); // warp to fourth epoch ts @@ -431,14 +431,14 @@ contract BribeInitiativeAllocateTest is Test { }); bribeInitiative.onAfterAllocateLQTY(governance.epoch(), user2, userState, allocation, initiativeState); - (uint88 totalLQTYAllocated, uint32 totalAverageTimestamp) = + (uint88 totalLQTYAllocated, uint120 totalAverageTimestamp) = bribeInitiative.totalLQTYAllocatedByEpoch(governance.epoch()); assertEq(totalLQTYAllocated, 1e18); - assertEq(totalAverageTimestamp, uint32(block.timestamp)); - (uint88 userLQTYAllocated, uint32 userAverageTimestamp) = + assertEq(totalAverageTimestamp, uint120(block.timestamp)); + (uint88 userLQTYAllocated, uint120 userAverageTimestamp) = bribeInitiative.lqtyAllocatedByUserAtEpoch(user2, governance.epoch()); assertEq(userLQTYAllocated, 1e18); - assertEq(userAverageTimestamp, uint32(block.timestamp)); + assertEq(userAverageTimestamp, uint120(block.timestamp)); } { @@ -455,14 +455,14 @@ contract BribeInitiativeAllocateTest is Test { }); bribeInitiative.onAfterAllocateLQTY(governance.epoch(), user, userState, allocation, initiativeState); - (uint88 totalLQTYAllocated, uint32 totalAverageTimestamp) = + (uint88 totalLQTYAllocated, uint120 totalAverageTimestamp) = bribeInitiative.totalLQTYAllocatedByEpoch(governance.epoch()); assertEq(totalLQTYAllocated, 1001e18); - assertEq(totalAverageTimestamp, uint32(block.timestamp)); - (uint88 userLQTYAllocated, uint32 userAverageTimestamp) = + assertEq(totalAverageTimestamp, uint120(block.timestamp)); + (uint88 userLQTYAllocated, uint120 userAverageTimestamp) = bribeInitiative.lqtyAllocatedByUserAtEpoch(user, governance.epoch()); assertEq(userLQTYAllocated, 1000e18); - assertEq(userAverageTimestamp, uint32(block.timestamp)); + assertEq(userAverageTimestamp, uint120(block.timestamp)); } governance.setEpoch(2); @@ -481,14 +481,14 @@ contract BribeInitiativeAllocateTest is Test { }); bribeInitiative.onAfterAllocateLQTY(governance.epoch(), user, userState, allocation, initiativeState); - (uint88 totalLQTYAllocated, uint32 totalAverageTimestamp) = + (uint88 totalLQTYAllocated, uint120 totalAverageTimestamp) = bribeInitiative.totalLQTYAllocatedByEpoch(governance.epoch()); assertEq(totalLQTYAllocated, 1e18); - assertEq(totalAverageTimestamp, uint32(block.timestamp)); - (uint88 userLQTYAllocated, uint32 userAverageTimestamp) = + assertEq(totalAverageTimestamp, uint120(block.timestamp)); + (uint88 userLQTYAllocated, uint120 userAverageTimestamp) = bribeInitiative.lqtyAllocatedByUserAtEpoch(user, governance.epoch()); assertEq(userLQTYAllocated, 0); - assertEq(userAverageTimestamp, uint32(block.timestamp)); + assertEq(userAverageTimestamp, uint120(block.timestamp)); } governance.setEpoch(3); @@ -507,14 +507,14 @@ contract BribeInitiativeAllocateTest is Test { }); bribeInitiative.onAfterAllocateLQTY(governance.epoch(), user, userState, allocation, initiativeState); - (uint88 totalLQTYAllocated, uint32 totalAverageTimestamp) = + (uint88 totalLQTYAllocated, uint120 totalAverageTimestamp) = bribeInitiative.totalLQTYAllocatedByEpoch(governance.epoch()); assertEq(totalLQTYAllocated, 1e18); - assertEq(totalAverageTimestamp, uint32(block.timestamp)); - (uint88 userLQTYAllocated, uint32 userAverageTimestamp) = + assertEq(totalAverageTimestamp, uint120(block.timestamp)); + (uint88 userLQTYAllocated, uint120 userAverageTimestamp) = bribeInitiative.lqtyAllocatedByUserAtEpoch(user, governance.epoch()); assertEq(userLQTYAllocated, 0); - assertEq(userAverageTimestamp, uint32(block.timestamp)); + assertEq(userAverageTimestamp, uint120(block.timestamp)); } } @@ -544,14 +544,14 @@ contract BribeInitiativeAllocateTest is Test { }); bribeInitiative.onAfterAllocateLQTY(governance.epoch(), user2, userState, allocation, initiativeState); - (uint88 totalLQTYAllocated, uint32 totalAverageTimestamp) = + (uint88 totalLQTYAllocated, uint120 totalAverageTimestamp) = bribeInitiative.totalLQTYAllocatedByEpoch(governance.epoch()); assertEq(totalLQTYAllocated, 1e18); - assertEq(totalAverageTimestamp, uint32(block.timestamp)); - (uint88 userLQTYAllocated, uint32 userAverageTimestamp) = + assertEq(totalAverageTimestamp, uint120(block.timestamp)); + (uint88 userLQTYAllocated, uint120 userAverageTimestamp) = bribeInitiative.lqtyAllocatedByUserAtEpoch(user2, governance.epoch()); assertEq(userLQTYAllocated, 1e18); - assertEq(userAverageTimestamp, uint32(block.timestamp)); + assertEq(userAverageTimestamp, uint120(block.timestamp)); } { @@ -568,14 +568,14 @@ contract BribeInitiativeAllocateTest is Test { }); bribeInitiative.onAfterAllocateLQTY(governance.epoch(), user, userState, allocation, initiativeState); - (uint88 totalLQTYAllocated, uint32 totalAverageTimestamp) = + (uint88 totalLQTYAllocated, uint120 totalAverageTimestamp) = bribeInitiative.totalLQTYAllocatedByEpoch(governance.epoch()); assertEq(totalLQTYAllocated, 1001e18); - assertEq(totalAverageTimestamp, uint32(block.timestamp)); - (uint88 userLQTYAllocated, uint32 userAverageTimestamp) = + assertEq(totalAverageTimestamp, uint120(block.timestamp)); + (uint88 userLQTYAllocated, uint120 userAverageTimestamp) = bribeInitiative.lqtyAllocatedByUserAtEpoch(user, governance.epoch()); assertEq(userLQTYAllocated, 1000e18); - assertEq(userAverageTimestamp, uint32(block.timestamp)); + assertEq(userAverageTimestamp, uint120(block.timestamp)); } { @@ -592,14 +592,14 @@ contract BribeInitiativeAllocateTest is Test { }); bribeInitiative.onAfterAllocateLQTY(governance.epoch(), user, userState, allocation, initiativeState); - (uint88 totalLQTYAllocated, uint32 totalAverageTimestamp) = + (uint88 totalLQTYAllocated, uint120 totalAverageTimestamp) = bribeInitiative.totalLQTYAllocatedByEpoch(governance.epoch()); assertEq(totalLQTYAllocated, 2001e18); - assertEq(totalAverageTimestamp, uint32(block.timestamp)); - (uint88 userLQTYAllocated, uint32 userAverageTimestamp) = + assertEq(totalAverageTimestamp, uint120(block.timestamp)); + (uint88 userLQTYAllocated, uint120 userAverageTimestamp) = bribeInitiative.lqtyAllocatedByUserAtEpoch(user, governance.epoch()); assertEq(userLQTYAllocated, 2000e18); - assertEq(userAverageTimestamp, uint32(block.timestamp)); + assertEq(userAverageTimestamp, uint120(block.timestamp)); } governance.setEpoch(2); @@ -640,14 +640,14 @@ contract BribeInitiativeAllocateTest is Test { }); bribeInitiative.onAfterAllocateLQTY(governance.epoch(), user2, userState, allocation, initiativeState); - (uint88 totalLQTYAllocated, uint32 totalAverageTimestamp) = + (uint88 totalLQTYAllocated, uint120 totalAverageTimestamp) = bribeInitiative.totalLQTYAllocatedByEpoch(governance.epoch()); assertEq(totalLQTYAllocated, 1e18); - assertEq(totalAverageTimestamp, uint32(block.timestamp)); - (uint88 userLQTYAllocated, uint32 userAverageTimestamp) = + assertEq(totalAverageTimestamp, uint120(block.timestamp)); + (uint88 userLQTYAllocated, uint120 userAverageTimestamp) = bribeInitiative.lqtyAllocatedByUserAtEpoch(user2, governance.epoch()); assertEq(userLQTYAllocated, 1e18); - assertEq(userAverageTimestamp, uint32(block.timestamp)); + assertEq(userAverageTimestamp, uint120(block.timestamp)); } { @@ -664,14 +664,14 @@ contract BribeInitiativeAllocateTest is Test { }); bribeInitiative.onAfterAllocateLQTY(governance.epoch(), user, userState, allocation, initiativeState); - (uint88 totalLQTYAllocated, uint32 totalAverageTimestamp) = + (uint88 totalLQTYAllocated, uint120 totalAverageTimestamp) = bribeInitiative.totalLQTYAllocatedByEpoch(governance.epoch()); assertEq(totalLQTYAllocated, 1001e18); - assertEq(totalAverageTimestamp, uint32(block.timestamp)); - (uint88 userLQTYAllocated, uint32 userAverageTimestamp) = + assertEq(totalAverageTimestamp, uint120(block.timestamp)); + (uint88 userLQTYAllocated, uint120 userAverageTimestamp) = bribeInitiative.lqtyAllocatedByUserAtEpoch(user, governance.epoch()); assertEq(userLQTYAllocated, 1000e18); - assertEq(userAverageTimestamp, uint32(block.timestamp)); + assertEq(userAverageTimestamp, uint120(block.timestamp)); } { @@ -688,14 +688,14 @@ contract BribeInitiativeAllocateTest is Test { }); bribeInitiative.onAfterAllocateLQTY(governance.epoch(), user, userState, allocation, initiativeState); - (uint88 totalLQTYAllocated, uint32 totalAverageTimestamp) = + (uint88 totalLQTYAllocated, uint120 totalAverageTimestamp) = bribeInitiative.totalLQTYAllocatedByEpoch(governance.epoch()); assertEq(totalLQTYAllocated, 1e18); - assertEq(totalAverageTimestamp, uint32(block.timestamp)); - (uint88 userLQTYAllocated, uint32 userAverageTimestamp) = + assertEq(totalAverageTimestamp, uint120(block.timestamp)); + (uint88 userLQTYAllocated, uint120 userAverageTimestamp) = bribeInitiative.lqtyAllocatedByUserAtEpoch(user, governance.epoch()); assertEq(userLQTYAllocated, 0); - assertEq(userAverageTimestamp, uint32(block.timestamp)); + assertEq(userAverageTimestamp, uint120(block.timestamp)); } governance.setEpoch(2); @@ -738,14 +738,14 @@ contract BribeInitiativeAllocateTest is Test { }); bribeInitiative.onAfterAllocateLQTY(governance.epoch(), user2, userState, allocation, initiativeState); - (uint88 totalLQTYAllocated, uint32 totalAverageTimestamp) = + (uint88 totalLQTYAllocated, uint120 totalAverageTimestamp) = bribeInitiative.totalLQTYAllocatedByEpoch(governance.epoch()); assertEq(totalLQTYAllocated, 1e18); - assertEq(totalAverageTimestamp, uint32(block.timestamp)); - (uint88 userLQTYAllocated, uint32 userAverageTimestamp) = + assertEq(totalAverageTimestamp, uint120(block.timestamp)); + (uint88 userLQTYAllocated, uint120 userAverageTimestamp) = bribeInitiative.lqtyAllocatedByUserAtEpoch(user2, governance.epoch()); assertEq(userLQTYAllocated, 1e18); - assertEq(userAverageTimestamp, uint32(block.timestamp)); + assertEq(userAverageTimestamp, uint120(block.timestamp)); } { @@ -762,14 +762,14 @@ contract BribeInitiativeAllocateTest is Test { }); bribeInitiative.onAfterAllocateLQTY(governance.epoch(), user, userState, allocation, initiativeState); - (uint88 totalLQTYAllocated, uint32 totalAverageTimestamp) = + (uint88 totalLQTYAllocated, uint120 totalAverageTimestamp) = bribeInitiative.totalLQTYAllocatedByEpoch(governance.epoch()); assertEq(totalLQTYAllocated, 1001e18); - assertEq(totalAverageTimestamp, uint32(block.timestamp)); - (uint88 userLQTYAllocated, uint32 userAverageTimestamp) = + assertEq(totalAverageTimestamp, uint120(block.timestamp)); + (uint88 userLQTYAllocated, uint120 userAverageTimestamp) = bribeInitiative.lqtyAllocatedByUserAtEpoch(user, governance.epoch()); assertEq(userLQTYAllocated, 1000e18); - assertEq(userAverageTimestamp, uint32(block.timestamp)); + assertEq(userAverageTimestamp, uint120(block.timestamp)); } { @@ -786,14 +786,14 @@ contract BribeInitiativeAllocateTest is Test { }); bribeInitiative.onAfterAllocateLQTY(governance.epoch(), user, userState, allocation, initiativeState); - (uint88 totalLQTYAllocated, uint32 totalAverageTimestamp) = + (uint88 totalLQTYAllocated, uint120 totalAverageTimestamp) = bribeInitiative.totalLQTYAllocatedByEpoch(governance.epoch()); assertEq(totalLQTYAllocated, 1e18); - assertEq(totalAverageTimestamp, uint32(block.timestamp)); - (uint88 userLQTYAllocated, uint32 userAverageTimestamp) = + assertEq(totalAverageTimestamp, uint120(block.timestamp)); + (uint88 userLQTYAllocated, uint120 userAverageTimestamp) = bribeInitiative.lqtyAllocatedByUserAtEpoch(user, governance.epoch()); assertEq(userLQTYAllocated, 0); - assertEq(userAverageTimestamp, uint32(block.timestamp)); + assertEq(userAverageTimestamp, uint120(block.timestamp)); } { @@ -810,14 +810,14 @@ contract BribeInitiativeAllocateTest is Test { }); bribeInitiative.onAfterAllocateLQTY(governance.epoch(), user, userState, allocation, initiativeState); - (uint88 totalLQTYAllocated, uint32 totalAverageTimestamp) = + (uint88 totalLQTYAllocated, uint120 totalAverageTimestamp) = bribeInitiative.totalLQTYAllocatedByEpoch(governance.epoch()); assertEq(totalLQTYAllocated, 2001e18); - assertEq(totalAverageTimestamp, uint32(block.timestamp)); - (uint88 userLQTYAllocated, uint32 userAverageTimestamp) = + assertEq(totalAverageTimestamp, uint120(block.timestamp)); + (uint88 userLQTYAllocated, uint120 userAverageTimestamp) = bribeInitiative.lqtyAllocatedByUserAtEpoch(user, governance.epoch()); assertEq(userLQTYAllocated, 2000e18); - assertEq(userAverageTimestamp, uint32(block.timestamp)); + assertEq(userAverageTimestamp, uint120(block.timestamp)); } governance.setEpoch(2); @@ -851,14 +851,14 @@ contract BribeInitiativeAllocateTest is Test { }); bribeInitiative.onAfterAllocateLQTY(governance.epoch(), user2, userState, allocation, initiativeState); - (uint88 totalLQTYAllocated, uint32 totalAverageTimestamp) = + (uint88 totalLQTYAllocated, uint120 totalAverageTimestamp) = bribeInitiative.totalLQTYAllocatedByEpoch(governance.epoch()); assertEq(totalLQTYAllocated, 1e18); - assertEq(totalAverageTimestamp, uint32(block.timestamp)); - (uint88 userLQTYAllocated, uint32 userAverageTimestamp) = + assertEq(totalAverageTimestamp, uint120(block.timestamp)); + (uint88 userLQTYAllocated, uint120 userAverageTimestamp) = bribeInitiative.lqtyAllocatedByUserAtEpoch(user2, governance.epoch()); assertEq(userLQTYAllocated, 1e18); - assertEq(userAverageTimestamp, uint32(block.timestamp)); + assertEq(userAverageTimestamp, uint120(block.timestamp)); } { @@ -875,59 +875,59 @@ contract BribeInitiativeAllocateTest is Test { }); bribeInitiative.onAfterAllocateLQTY(governance.epoch(), user, userState, allocation, initiativeState); - (uint88 totalLQTYAllocated, uint32 totalAverageTimestamp) = + (uint88 totalLQTYAllocated, uint120 totalAverageTimestamp) = bribeInitiative.totalLQTYAllocatedByEpoch(governance.epoch()); assertEq(totalLQTYAllocated, 1001e18); - assertEq(totalAverageTimestamp, uint32(block.timestamp)); - (uint88 userLQTYAllocated, uint32 userAverageTimestamp) = + assertEq(totalAverageTimestamp, uint120(block.timestamp)); + (uint88 userLQTYAllocated, uint120 userAverageTimestamp) = bribeInitiative.lqtyAllocatedByUserAtEpoch(user, governance.epoch()); assertEq(userLQTYAllocated, 1000e18); - assertEq(userAverageTimestamp, uint32(block.timestamp)); + assertEq(userAverageTimestamp, uint120(block.timestamp)); } { IGovernance.UserState memory userState = - IGovernance.UserState({allocatedLQTY: 1, averageStakingTimestamp: uint32(block.timestamp)}); + IGovernance.UserState({allocatedLQTY: 1, averageStakingTimestamp: uint120(block.timestamp)}); IGovernance.Allocation memory allocation = IGovernance.Allocation({voteLQTY: 0, vetoLQTY: 1, atEpoch: uint16(governance.epoch())}); IGovernance.InitiativeState memory initiativeState = IGovernance.InitiativeState({ voteLQTY: 1e18, vetoLQTY: 0, - averageStakingTimestampVoteLQTY: uint32(block.timestamp), + averageStakingTimestampVoteLQTY: uint120(block.timestamp), averageStakingTimestampVetoLQTY: 0, lastEpochClaim: 0 }); bribeInitiative.onAfterAllocateLQTY(governance.epoch(), user, userState, allocation, initiativeState); - (uint88 totalLQTYAllocated, uint32 totalAverageTimestamp) = + (uint88 totalLQTYAllocated, uint120 totalAverageTimestamp) = bribeInitiative.totalLQTYAllocatedByEpoch(governance.epoch()); assertEq(totalLQTYAllocated, 1e18); - assertEq(totalAverageTimestamp, uint32(block.timestamp)); - (uint88 userLQTYAllocated, uint32 userAverageTimestamp) = + assertEq(totalAverageTimestamp, uint120(block.timestamp)); + (uint88 userLQTYAllocated, uint120 userAverageTimestamp) = bribeInitiative.lqtyAllocatedByUserAtEpoch(user, governance.epoch()); assertEq(userLQTYAllocated, 0); - assertEq(userAverageTimestamp, uint32(block.timestamp)); + assertEq(userAverageTimestamp, uint120(block.timestamp)); } { IGovernance.UserState memory userState = - IGovernance.UserState({allocatedLQTY: 2, averageStakingTimestamp: uint32(block.timestamp)}); + IGovernance.UserState({allocatedLQTY: 2, averageStakingTimestamp: uint120(block.timestamp)}); IGovernance.Allocation memory allocation = IGovernance.Allocation({voteLQTY: 0, vetoLQTY: 2, atEpoch: uint16(governance.epoch())}); IGovernance.InitiativeState memory initiativeState = IGovernance.InitiativeState({ voteLQTY: 1e18, vetoLQTY: 0, - averageStakingTimestampVoteLQTY: uint32(block.timestamp), + averageStakingTimestampVoteLQTY: uint120(block.timestamp), averageStakingTimestampVetoLQTY: 0, lastEpochClaim: 0 }); bribeInitiative.onAfterAllocateLQTY(governance.epoch(), user, userState, allocation, initiativeState); - (uint88 totalLQTYAllocated, uint32 totalAverageTimestamp) = + (uint88 totalLQTYAllocated, uint120 totalAverageTimestamp) = bribeInitiative.totalLQTYAllocatedByEpoch(governance.epoch()); assertEq(totalLQTYAllocated, 1e18); - assertEq(totalAverageTimestamp, uint32(block.timestamp)); - (uint88 userLQTYAllocated, uint32 userAverageTimestamp) = + assertEq(totalAverageTimestamp, uint120(block.timestamp)); + (uint88 userLQTYAllocated, uint120 userAverageTimestamp) = bribeInitiative.lqtyAllocatedByUserAtEpoch(user, governance.epoch()); assertEq(userLQTYAllocated, 0); assertEq(userAverageTimestamp, uint32(block.timestamp)); diff --git a/test/EncodingDecoding.t.sol b/test/EncodingDecoding.t.sol index d7571ff3..249d4ab2 100644 --- a/test/EncodingDecoding.t.sol +++ b/test/EncodingDecoding.t.sol @@ -7,16 +7,16 @@ import {EncodingDecodingLib} from "src/utils/EncodingDecodingLib.sol"; contract EncodingDecodingTest is Test { // value -> encoding -> decoding -> value - function test_encoding_and_decoding_symmetrical(uint88 lqty, uint32 averageTimestamp) public { + function test_encoding_and_decoding_symmetrical(uint88 lqty, uint120 averageTimestamp) public { uint224 encodedValue = EncodingDecodingLib.encodeLQTYAllocation(lqty, averageTimestamp); - (uint88 decodedLqty, uint32 decodedAverageTimestamp) = EncodingDecodingLib.decodeLQTYAllocation(encodedValue); + (uint88 decodedLqty, uint120 decodedAverageTimestamp) = EncodingDecodingLib.decodeLQTYAllocation(encodedValue); assertEq(lqty, decodedLqty); assertEq(averageTimestamp, decodedAverageTimestamp); // Redo uint224 reEncoded = EncodingDecodingLib.encodeLQTYAllocation(decodedLqty, decodedAverageTimestamp); - (uint88 reDecodedLqty, uint32 reDecodedAverageTimestamp) = + (uint88 reDecodedLqty, uint120 reDecodedAverageTimestamp) = EncodingDecodingLib.decodeLQTYAllocation(encodedValue); assertEq(reEncoded, encodedValue); @@ -24,11 +24,6 @@ contract EncodingDecodingTest is Test { assertEq(reDecodedAverageTimestamp, decodedAverageTimestamp); } - /// We expect this test to fail as the encoding is ambigous past u120 - function testFail_encoding_not_equal_reproducer() public { - _receive_undo_compare(18371677541005923091065047412368542483005086202); - } - // receive -> undo -> check -> redo -> compare function test_receive_undo_compare(uint120 encodedValue) public { _receive_undo_compare(encodedValue); @@ -37,10 +32,10 @@ contract EncodingDecodingTest is Test { // receive -> undo -> check -> redo -> compare function _receive_undo_compare(uint224 encodedValue) public { /// These values fail because we could pass a value that is bigger than intended - (uint88 decodedLqty, uint32 decodedAverageTimestamp) = EncodingDecodingLib.decodeLQTYAllocation(encodedValue); + (uint88 decodedLqty, uint120 decodedAverageTimestamp) = EncodingDecodingLib.decodeLQTYAllocation(encodedValue); uint224 encodedValue2 = EncodingDecodingLib.encodeLQTYAllocation(decodedLqty, decodedAverageTimestamp); - (uint88 decodedLqty2, uint32 decodedAverageTimestamp2) = EncodingDecodingLib.decodeLQTYAllocation(encodedValue2); + (uint88 decodedLqty2, uint120 decodedAverageTimestamp2) = EncodingDecodingLib.decodeLQTYAllocation(encodedValue2); assertEq(encodedValue, encodedValue2, "encoded values not equal"); assertEq(decodedLqty, decodedLqty2, "decoded lqty not equal"); diff --git a/test/Governance.t.sol b/test/Governance.t.sol index 001f5f4f..5bbf1fca 100644 --- a/test/Governance.t.sol +++ b/test/Governance.t.sol @@ -28,16 +28,16 @@ contract GovernanceInternal is Governance { address[] memory _initiatives ) Governance(_lqty, _lusd, _stakingV1, _bold, _config, _initiatives) {} - function averageAge(uint32 _currentTimestamp, uint32 _averageTimestamp) external pure returns (uint32) { + function averageAge(uint120 _currentTimestamp, uint120 _averageTimestamp) external pure returns (uint120) { return _averageAge(_currentTimestamp, _averageTimestamp); } function calculateAverageTimestamp( - uint32 _prevOuterAverageTimestamp, - uint32 _newInnerAverageTimestamp, + uint120 _prevOuterAverageTimestamp, + uint120 _newInnerAverageTimestamp, uint88 _prevLQTYBalance, uint88 _newLQTYBalance - ) external view returns (uint32) { + ) external view returns (uint208) { return _calculateAverageTimestamp( _prevOuterAverageTimestamp, _newInnerAverageTimestamp, _prevLQTYBalance, _newLQTYBalance ); @@ -145,8 +145,8 @@ contract GovernanceTest is Test { } // should not revert under any input - function test_averageAge(uint32 _currentTimestamp, uint32 _timestamp) public { - uint32 averageAge = governanceInternal.averageAge(_currentTimestamp, _timestamp); + function test_averageAge(uint120 _currentTimestamp, uint120 _timestamp) public { + uint120 averageAge = governanceInternal.averageAge(_currentTimestamp, _timestamp); if (_timestamp == 0 || _currentTimestamp < _timestamp) { assertEq(averageAge, 0); } else { @@ -196,7 +196,7 @@ contract GovernanceTest is Test { // deploy and deposit 1 LQTY governance.depositLQTY(1e18); assertEq(UserProxy(payable(userProxy)).staked(), 1e18); - (uint88 allocatedLQTY, uint32 averageStakingTimestamp) = governance.userStates(user); + (uint88 allocatedLQTY, uint120 averageStakingTimestamp) = governance.userStates(user); assertEq(allocatedLQTY, 0); // first deposit should have an averageStakingTimestamp if block.timestamp assertEq(averageStakingTimestamp, block.timestamp); @@ -301,7 +301,7 @@ contract GovernanceTest is Test { // deploy and deposit 1 LQTY governance.depositLQTYViaPermit(1e18, permitParams); assertEq(UserProxy(payable(userProxy)).staked(), 1e18); - (uint88 allocatedLQTY, uint32 averageStakingTimestamp) = governance.userStates(wallet.addr); + (uint88 allocatedLQTY, uint120 averageStakingTimestamp) = governance.userStates(wallet.addr); assertEq(allocatedLQTY, 0); assertEq(averageStakingTimestamp, block.timestamp); } @@ -382,7 +382,7 @@ contract GovernanceTest is Test { } // should not revert under any input - function test_lqtyToVotes(uint88 _lqtyAmount, uint256 _currentTimestamp, uint32 _averageTimestamp) public { + function test_lqtyToVotes(uint88 _lqtyAmount, uint120 _currentTimestamp, uint120 _averageTimestamp) public { governance.lqtyToVotes(_lqtyAmount, _currentTimestamp, _averageTimestamp); } @@ -669,12 +669,12 @@ contract GovernanceTest is Test { (uint256 allocatedLQTY,) = governance.userStates(user); assertEq(allocatedLQTY, 1_000e18); - (uint88 voteLQTY1,, uint32 averageStakingTimestampVoteLQTY1,,) = governance.initiativeStates(baseInitiative1); + (uint88 voteLQTY1,, uint120 averageStakingTimestampVoteLQTY1,,) = governance.initiativeStates(baseInitiative1); (uint88 voteLQTY2,,,,) = governance.initiativeStates(baseInitiative2); // Get power at time of vote - uint256 votingPower = governance.lqtyToVotes(voteLQTY1, block.timestamp, averageStakingTimestampVoteLQTY1); + uint256 votingPower = governance.lqtyToVotes(voteLQTY1, uint120(block.timestamp), averageStakingTimestampVoteLQTY1); assertGt(votingPower, 0, "Non zero power"); /// @audit TODO Fully digest and explain the bug @@ -694,7 +694,7 @@ contract GovernanceTest is Test { assertLt(initiativeVoteSnapshot1.votes, threshold, "it didn't get rewards"); uint256 votingPowerWithProjection = governance.lqtyToVotes( - voteLQTY1, governance.epochStart() + governance.EPOCH_DURATION(), averageStakingTimestampVoteLQTY1 + voteLQTY1, uint120(governance.epochStart() + governance.EPOCH_DURATION()), averageStakingTimestampVoteLQTY1 ); assertLt(votingPower, threshold, "Current Power is not enough - Desynch A"); assertLt(votingPowerWithProjection, threshold, "Future Power is also not enough - Desynch B"); @@ -817,7 +817,7 @@ contract GovernanceTest is Test { // Get state here // Get initiative state - (uint88 b4_countedVoteLQTY, uint32 b4_countedVoteLQTYAverageTimestamp) = governance.globalState(); + (uint88 b4_countedVoteLQTY, uint120 b4_countedVoteLQTYAverageTimestamp) = governance.globalState(); @@ -837,7 +837,7 @@ contract GovernanceTest is Test { { // Get state here // TODO Get initiative state - (uint88 after_countedVoteLQTY, uint32 after_countedVoteLQTYAverageTimestamp) = governance.globalState(); + (uint88 after_countedVoteLQTY, uint120 after_countedVoteLQTYAverageTimestamp) = governance.globalState(); assertEq(after_countedVoteLQTY, b4_countedVoteLQTY, "LQTY should not change"); assertEq(b4_countedVoteLQTYAverageTimestamp, after_countedVoteLQTYAverageTimestamp, "Avg TS should not change"); @@ -889,8 +889,8 @@ contract GovernanceTest is Test { // Grab values b4 unregistering and b4 removing user allocation - (uint88 b4_countedVoteLQTY, uint32 b4_countedVoteLQTYAverageTimestamp) = governance.globalState(); - (uint88 b4_allocatedLQTY, uint32 b4_averageStakingTimestamp) = governance.userStates(user); + (uint88 b4_countedVoteLQTY, uint120 b4_countedVoteLQTYAverageTimestamp) = governance.globalState(); + (uint88 b4_allocatedLQTY, uint120 b4_averageStakingTimestamp) = governance.userStates(user); (uint88 b4_voteLQTY,,,,) = governance.initiativeStates(baseInitiative1); // Unregistering @@ -906,14 +906,14 @@ contract GovernanceTest is Test { assertEq(after_countedVoteLQTY, b4_countedVoteLQTY - b4_voteLQTY, "Global Lqty change after unregister"); assertEq(1e18, b4_voteLQTY, "sanity check"); - (uint88 after_allocatedLQTY, uint32 after_averageStakingTimestamp) = governance.userStates(user); + (uint88 after_allocatedLQTY, uint120 after_averageStakingTimestamp) = governance.userStates(user); // We expect no changes here ( uint88 after_voteLQTY, uint88 after_vetoLQTY, - uint32 after_averageStakingTimestampVoteLQTY, - uint32 after_averageStakingTimestampVetoLQTY, + uint120 after_averageStakingTimestampVoteLQTY, + uint120 after_averageStakingTimestampVetoLQTY, uint16 after_lastEpochClaim ) = governance.initiativeStates(baseInitiative1); assertEq(b4_voteLQTY, after_voteLQTY, "Initiative votes are the same"); @@ -937,7 +937,7 @@ contract GovernanceTest is Test { // After user counts LQTY the { - (uint88 after_user_countedVoteLQTY, uint32 after_user_countedVoteLQTYAverageTimestamp) = + (uint88 after_user_countedVoteLQTY, uint120 after_user_countedVoteLQTYAverageTimestamp) = governance.globalState(); // The LQTY was already removed assertEq(after_user_countedVoteLQTY, 0, "Removal 1"); @@ -1025,7 +1025,7 @@ contract GovernanceTest is Test { lqty.approve(address(userProxy), 1e18); governance.depositLQTY(1e18); - (uint88 allocatedLQTY, uint32 averageStakingTimestampUser) = governance.userStates(user); + (uint88 allocatedLQTY, uint120 averageStakingTimestampUser) = governance.userStates(user); assertEq(allocatedLQTY, 0); (uint88 countedVoteLQTY,) = governance.globalState(); assertEq(countedVoteLQTY, 0); @@ -1049,8 +1049,8 @@ contract GovernanceTest is Test { ( uint88 voteLQTY, uint88 vetoLQTY, - uint32 averageStakingTimestampVoteLQTY, - uint32 averageStakingTimestampVetoLQTY, + uint120 averageStakingTimestampVoteLQTY, + uint120 averageStakingTimestampVetoLQTY, ) = governance.initiativeStates(baseInitiative1); // should update the `voteLQTY` and `vetoLQTY` variables assertEq(voteLQTY, 1e18); @@ -1090,8 +1090,8 @@ contract GovernanceTest is Test { lqty.approve(address(user2Proxy), 1e18); governance.depositLQTY(1e18); - (, uint32 averageAge) = governance.userStates(user2); - assertEq(governance.lqtyToVotes(1e18, block.timestamp, averageAge), 0); + (, uint120 averageAge) = governance.userStates(user2); + assertEq(governance.lqtyToVotes(1e18, uint120(block.timestamp), averageAge), 0); deltaLQTYVetos[0] = 1e18; @@ -1149,7 +1149,7 @@ contract GovernanceTest is Test { lqty.approve(address(userProxy), 1e18); governance.depositLQTY(1e18); - (uint88 allocatedLQTY, uint32 averageStakingTimestampUser) = governance.userStates(user); + (uint88 allocatedLQTY, uint120 averageStakingTimestampUser) = governance.userStates(user); assertEq(allocatedLQTY, 0); (uint88 countedVoteLQTY,) = governance.globalState(); assertEq(countedVoteLQTY, 0); @@ -1173,8 +1173,8 @@ contract GovernanceTest is Test { ( uint88 voteLQTY, uint88 vetoLQTY, - uint32 averageStakingTimestampVoteLQTY, - uint32 averageStakingTimestampVetoLQTY, + uint120 averageStakingTimestampVoteLQTY, + uint120 averageStakingTimestampVetoLQTY, ) = governance.initiativeStates(baseInitiative1); // should update the `voteLQTY` and `vetoLQTY` variables assertEq(voteLQTY, 1e18); @@ -1214,8 +1214,8 @@ contract GovernanceTest is Test { lqty.approve(address(user2Proxy), 1e18); governance.depositLQTY(1e18); - (, uint32 averageAge) = governance.userStates(user2); - assertEq(governance.lqtyToVotes(1e18, block.timestamp, averageAge), 0); + (, uint120 averageAge) = governance.userStates(user2); + assertEq(governance.lqtyToVotes(1e18, uint120(block.timestamp), averageAge), 0); deltaLQTYVetos[0] = 1e18; @@ -1291,8 +1291,8 @@ contract GovernanceTest is Test { ( uint88 voteLQTY, uint88 vetoLQTY, - uint32 averageStakingTimestampVoteLQTY, - uint32 averageStakingTimestampVetoLQTY, + uint120 averageStakingTimestampVoteLQTY, + uint120 averageStakingTimestampVetoLQTY, ) = governance.initiativeStates(baseInitiative1); assertEq(voteLQTY, 1e18); assertEq(vetoLQTY, 0); @@ -1708,12 +1708,12 @@ contract GovernanceTest is Test { uint88 lqtyAmount = 1e18; _stakeLQTY(user, lqtyAmount); - (uint88 allocatedLQTY0, uint32 averageStakingTimestamp0) = governance.userStates(user); - uint240 currentUserPower0 = governance.lqtyToVotes(allocatedLQTY0, block.timestamp, averageStakingTimestamp0); + (uint88 allocatedLQTY0, uint120 averageStakingTimestamp0) = governance.userStates(user); + uint240 currentUserPower0 = governance.lqtyToVotes(allocatedLQTY0, uint120(block.timestamp), averageStakingTimestamp0); - (uint88 voteLQTY0,, uint32 averageStakingTimestampVoteLQTY0,,) = governance.initiativeStates(baseInitiative1); + (uint88 voteLQTY0,, uint120 averageStakingTimestampVoteLQTY0,,) = governance.initiativeStates(baseInitiative1); uint240 currentInitiativePower0 = - governance.lqtyToVotes(voteLQTY0, block.timestamp, averageStakingTimestampVoteLQTY0); + governance.lqtyToVotes(voteLQTY0, uint120(block.timestamp), averageStakingTimestampVoteLQTY0); // (uint224 votes, uint16 forEpoch,,) = governance.votesForInitiativeSnapshot(baseInitiative1); // console2.log("votes0: ", votes); @@ -1725,15 +1725,15 @@ contract GovernanceTest is Test { _allocateLQTY(user, lqtyAmount); // check user voting power for the current epoch - (uint88 allocatedLQTY1, uint32 averageStakingTimestamp1) = governance.userStates(user); - uint240 currentUserPower1 = governance.lqtyToVotes(allocatedLQTY1, block.timestamp, averageStakingTimestamp1); + (uint88 allocatedLQTY1, uint120 averageStakingTimestamp1) = governance.userStates(user); + uint240 currentUserPower1 = governance.lqtyToVotes(allocatedLQTY1, uint120(block.timestamp), averageStakingTimestamp1); // user's allocated lqty should immediately increase their voting power assertGt(currentUserPower1, 0, "current user voting power is 0"); // check initiative voting power for the current epoch - (uint88 voteLQTY1,, uint32 averageStakingTimestampVoteLQTY1,,) = governance.initiativeStates(baseInitiative1); + (uint88 voteLQTY1,, uint120 averageStakingTimestampVoteLQTY1,,) = governance.initiativeStates(baseInitiative1); uint240 currentInitiativePower1 = - governance.lqtyToVotes(voteLQTY1, block.timestamp, averageStakingTimestampVoteLQTY1); + governance.lqtyToVotes(voteLQTY1, uint120(block.timestamp), averageStakingTimestampVoteLQTY1); assertGt(currentInitiativePower1, 0, "current initiative voting power is 0"); assertEq(currentUserPower1, currentInitiativePower1, "initiative and user voting power should be equal"); @@ -1746,14 +1746,14 @@ contract GovernanceTest is Test { governance.snapshotVotesForInitiative(baseInitiative1); // user voting power should increase over a given chunk of time - (uint88 allocatedLQTY2, uint32 averageStakingTimestamp2) = governance.userStates(user); - uint240 currentUserPower2 = governance.lqtyToVotes(allocatedLQTY2, block.timestamp, averageStakingTimestamp2); + (uint88 allocatedLQTY2, uint120 averageStakingTimestamp2) = governance.userStates(user); + uint240 currentUserPower2 = governance.lqtyToVotes(allocatedLQTY2, uint120(block.timestamp), averageStakingTimestamp2); assertGt(currentUserPower2, currentUserPower1); // initiative voting power should increase over a given chunk of time - (uint88 voteLQTY2,, uint32 averageStakingTimestampVoteLQTY2,,) = governance.initiativeStates(baseInitiative1); + (uint88 voteLQTY2,, uint120 averageStakingTimestampVoteLQTY2,,) = governance.initiativeStates(baseInitiative1); uint240 currentInitiativePower2 = - governance.lqtyToVotes(voteLQTY2, block.timestamp, averageStakingTimestampVoteLQTY2); + governance.lqtyToVotes(voteLQTY2, uint120(block.timestamp), averageStakingTimestampVoteLQTY2); assertEq( currentUserPower2, currentInitiativePower2, "user power and initiative power should increase by same amount" ); @@ -1768,13 +1768,13 @@ contract GovernanceTest is Test { governance.snapshotVotesForInitiative(baseInitiative1); // user voting power should increase - (uint88 allocatedLQTY3, uint32 averageStakingTimestamp3) = governance.userStates(user); - uint240 currentUserPower3 = governance.lqtyToVotes(allocatedLQTY3, block.timestamp, averageStakingTimestamp3); + (uint88 allocatedLQTY3, uint120 averageStakingTimestamp3) = governance.userStates(user); + uint240 currentUserPower3 = governance.lqtyToVotes(allocatedLQTY3, uint120(block.timestamp), averageStakingTimestamp3); // votes should match the voting power for the initiative and subsequently the user since they're the only one allocated - (uint88 voteLQTY3,, uint32 averageStakingTimestampVoteLQTY3,,) = governance.initiativeStates(baseInitiative1); + (uint88 voteLQTY3,, uint120 averageStakingTimestampVoteLQTY3,,) = governance.initiativeStates(baseInitiative1); uint240 currentInitiativePower3 = - governance.lqtyToVotes(voteLQTY3, block.timestamp, averageStakingTimestampVoteLQTY3); + governance.lqtyToVotes(voteLQTY3, uint120(block.timestamp), averageStakingTimestampVoteLQTY3); // votes should be counted in this epoch (votes, forEpoch,,) = governance.votesForInitiativeSnapshot(baseInitiative1); @@ -1785,12 +1785,12 @@ contract GovernanceTest is Test { vm.warp(block.timestamp + EPOCH_DURATION - 1); governance.snapshotVotesForInitiative(baseInitiative1); - (uint88 allocatedLQTY4, uint32 averageStakingTimestamp4) = governance.userStates(user); - uint240 currentUserPower4 = governance.lqtyToVotes(allocatedLQTY4, block.timestamp, averageStakingTimestamp4); + (uint88 allocatedLQTY4, uint120 averageStakingTimestamp4) = governance.userStates(user); + uint240 currentUserPower4 = governance.lqtyToVotes(allocatedLQTY4, uint120(block.timestamp), averageStakingTimestamp4); - (uint88 voteLQTY4,, uint32 averageStakingTimestampVoteLQTY4,,) = governance.initiativeStates(baseInitiative1); + (uint88 voteLQTY4,, uint120 averageStakingTimestampVoteLQTY4,,) = governance.initiativeStates(baseInitiative1); uint240 currentInitiativePower4 = - governance.lqtyToVotes(voteLQTY4, block.timestamp, averageStakingTimestampVoteLQTY4); + governance.lqtyToVotes(voteLQTY4, uint120(block.timestamp), averageStakingTimestampVoteLQTY4); // checking if snapshotting at the end of an epoch increases the voting power (uint224 votes2,,,) = governance.votesForInitiativeSnapshot(baseInitiative1); @@ -1833,14 +1833,14 @@ contract GovernanceTest is Test { assertEq(2, governance.epoch(), "not in epoch 2"); // check user voting power before allocation at epoch start - (uint88 allocatedLQTY0, uint32 averageStakingTimestamp0) = governance.userStates(user); - uint240 currentUserPower0 = governance.lqtyToVotes(allocatedLQTY0, block.timestamp, averageStakingTimestamp0); + (uint88 allocatedLQTY0, uint120 averageStakingTimestamp0) = governance.userStates(user); + uint240 currentUserPower0 = governance.lqtyToVotes(allocatedLQTY0, uint120(block.timestamp), averageStakingTimestamp0); assertEq(currentUserPower0, 0, "user has voting power > 0"); // check initiative voting power before allocation at epoch start - (uint88 voteLQTY0,, uint32 averageStakingTimestampVoteLQTY0,,) = governance.initiativeStates(baseInitiative1); + (uint88 voteLQTY0,, uint120 averageStakingTimestampVoteLQTY0,,) = governance.initiativeStates(baseInitiative1); uint240 currentInitiativePower0 = - governance.lqtyToVotes(voteLQTY0, block.timestamp, averageStakingTimestampVoteLQTY0); + governance.lqtyToVotes(voteLQTY0, uint120(block.timestamp), averageStakingTimestampVoteLQTY0); assertEq(currentInitiativePower0, 0, "current initiative voting power is > 0"); _allocateLQTY(user, lqtyAmount); @@ -1849,14 +1849,14 @@ contract GovernanceTest is Test { assertEq(2, governance.epoch(), "not in epoch 2"); // check user voting power after allocation at epoch end - (uint88 allocatedLQTY1, uint32 averageStakingTimestamp1) = governance.userStates(user); - uint240 currentUserPower1 = governance.lqtyToVotes(allocatedLQTY1, block.timestamp, averageStakingTimestamp1); + (uint88 allocatedLQTY1, uint120 averageStakingTimestamp1) = governance.userStates(user); + uint240 currentUserPower1 = governance.lqtyToVotes(allocatedLQTY1, uint120(block.timestamp), averageStakingTimestamp1); assertGt(currentUserPower1, 0, "user has no voting power after allocation"); // check initiative voting power after allocation at epoch end - (uint88 voteLQTY1,, uint32 averageStakingTimestampVoteLQTY1,,) = governance.initiativeStates(baseInitiative1); + (uint88 voteLQTY1,, uint120 averageStakingTimestampVoteLQTY1,,) = governance.initiativeStates(baseInitiative1); uint240 currentInitiativePower1 = - governance.lqtyToVotes(voteLQTY1, block.timestamp, averageStakingTimestampVoteLQTY1); + governance.lqtyToVotes(voteLQTY1, uint120(block.timestamp), averageStakingTimestampVoteLQTY1); assertGt(currentInitiativePower1, 0, "initiative has no voting power after allocation"); // check that user and initiative voting power is equivalent at epoch end @@ -1866,14 +1866,14 @@ contract GovernanceTest is Test { assertEq(42, governance.epoch(), "not in epoch 42"); // get user voting power after multiple epochs - (uint88 allocatedLQTY2, uint32 averageStakingTimestamp2) = governance.userStates(user); - uint240 currentUserPower2 = governance.lqtyToVotes(allocatedLQTY2, block.timestamp, averageStakingTimestamp2); + (uint88 allocatedLQTY2, uint120 averageStakingTimestamp2) = governance.userStates(user); + uint240 currentUserPower2 = governance.lqtyToVotes(allocatedLQTY2, uint120(block.timestamp), averageStakingTimestamp2); assertGt(currentUserPower2, currentUserPower1, "user voting power doesn't increase"); // get initiative voting power after multiple epochs - (uint88 voteLQTY2,, uint32 averageStakingTimestampVoteLQTY2,,) = governance.initiativeStates(baseInitiative1); + (uint88 voteLQTY2,, uint120 averageStakingTimestampVoteLQTY2,,) = governance.initiativeStates(baseInitiative1); uint240 currentInitiativePower2 = - governance.lqtyToVotes(voteLQTY2, block.timestamp, averageStakingTimestampVoteLQTY2); + governance.lqtyToVotes(voteLQTY2, uint120(block.timestamp), averageStakingTimestampVoteLQTY2); assertGt(currentInitiativePower2, currentInitiativePower1, "initiative voting power doesn't increase"); // check that initiative and user voting always track each other @@ -1915,9 +1915,9 @@ contract GovernanceTest is Test { vm.warp(block.timestamp + EPOCH_DURATION); // warp to second epoch // get initiative voting power at start of epoch - (uint88 voteLQTY0,, uint32 averageStakingTimestampVoteLQTY0,,) = governance.initiativeStates(baseInitiative1); + (uint88 voteLQTY0,, uint120 averageStakingTimestampVoteLQTY0,,) = governance.initiativeStates(baseInitiative1); uint240 currentInitiativePower0 = - governance.lqtyToVotes(voteLQTY0, block.timestamp, averageStakingTimestampVoteLQTY0); + governance.lqtyToVotes(voteLQTY0, uint120(block.timestamp), averageStakingTimestampVoteLQTY0); assertEq(currentInitiativePower0, 0, "initiative voting power is > 0"); _allocateLQTY(user, lqtyAmount); @@ -1928,9 +1928,9 @@ contract GovernanceTest is Test { governance.snapshotVotesForInitiative(baseInitiative1); // get initiative voting power at time of snapshot - (uint88 voteLQTY1,, uint32 averageStakingTimestampVoteLQTY1,,) = governance.initiativeStates(baseInitiative1); + (uint88 voteLQTY1,, uint120 averageStakingTimestampVoteLQTY1,,) = governance.initiativeStates(baseInitiative1); uint240 currentInitiativePower1 = - governance.lqtyToVotes(voteLQTY1, block.timestamp, averageStakingTimestampVoteLQTY1); + governance.lqtyToVotes(voteLQTY1, uint120(block.timestamp), averageStakingTimestampVoteLQTY1); assertGt(currentInitiativePower1, 0, "initiative voting power is 0"); uint240 deltaInitiativeVotingPower = currentInitiativePower1 - currentInitiativePower0; @@ -1976,11 +1976,11 @@ contract GovernanceTest is Test { // get user voting power at start of epoch from lqtyAllocatedByUserToInitiative (uint88 voteLQTY0,,) = governance.lqtyAllocatedByUserToInitiative(user, baseInitiative1); - (uint88 allocatedLQTY, uint32 averageStakingTimestamp) = governance.userStates(user); + (uint88 allocatedLQTY, uint120 averageStakingTimestamp) = governance.userStates(user); uint240 currentInitiativePowerFrom1 = - governance.lqtyToVotes(voteLQTY0, block.timestamp, averageStakingTimestamp); + governance.lqtyToVotes(voteLQTY0, uint120(block.timestamp), averageStakingTimestamp); uint240 currentInitiativePowerFrom2 = - governance.lqtyToVotes(allocatedLQTY, block.timestamp, averageStakingTimestamp); + governance.lqtyToVotes(allocatedLQTY, uint120(block.timestamp), averageStakingTimestamp); assertEq( currentInitiativePowerFrom1, @@ -2025,7 +2025,7 @@ contract GovernanceTest is Test { _allocateLQTY(user, 1e18); // get user voting power at start of epoch 2 from lqtyAllocatedByUserToInitiative - (, uint32 averageStakingTimestamp1) = governance.userStates(user); + (, uint120 averageStakingTimestamp1) = governance.userStates(user); // =========== epoch 3 (start) ================== // 3. user allocates to baseInitiative2 in epoch 3 @@ -2034,7 +2034,7 @@ contract GovernanceTest is Test { _allocateLQTYToInitiative(user, baseInitiative2, 1e18); // get user voting power at start of epoch 3 from lqtyAllocatedByUserToInitiative - (, uint32 averageStakingTimestamp2) = governance.userStates(user); + (, uint120 averageStakingTimestamp2) = governance.userStates(user); assertEq(averageStakingTimestamp1, averageStakingTimestamp2); } @@ -2075,7 +2075,7 @@ contract GovernanceTest is Test { _allocateLQTY(user, 1e18); // get user voting power at start of epoch 2 from lqtyAllocatedByUserToInitiative - (, uint32 averageStakingTimestamp1) = governance.userStates(user); + (, uint120 averageStakingTimestamp1) = governance.userStates(user); console2.log("averageStakingTimestamp1: ", averageStakingTimestamp1); // =========== epoch 3 (start) ================== @@ -2085,7 +2085,7 @@ contract GovernanceTest is Test { _allocateLQTY(user, 1e18); // get user voting power at start of epoch 3 from lqtyAllocatedByUserToInitiative - (, uint32 averageStakingTimestamp2) = governance.userStates(user); + (, uint120 averageStakingTimestamp2) = governance.userStates(user); assertEq(averageStakingTimestamp1, averageStakingTimestamp2, "average timestamps differ"); } @@ -2127,7 +2127,7 @@ contract GovernanceTest is Test { _allocateLQTY(user, lqtyAmount2); // get user voting power at start of epoch 2 from lqtyAllocatedByUserToInitiative - (, uint32 averageStakingTimestamp1) = governance.userStates(user); + (, uint120 averageStakingTimestamp1) = governance.userStates(user); // =========== epoch 3 (start) ================== // 3. user allocates to baseInitiative1 in epoch 3 @@ -2140,7 +2140,7 @@ contract GovernanceTest is Test { _allocateLQTY(user, lqtyAmount3); // get user voting power at start of epoch 3 from lqtyAllocatedByUserToInitiative - (, uint32 averageStakingTimestamp2) = governance.userStates(user); + (, uint120 averageStakingTimestamp2) = governance.userStates(user); assertEq( averageStakingTimestamp1, averageStakingTimestamp2, "averageStakingTimestamp1 != averageStakingTimestamp2" ); @@ -2178,9 +2178,9 @@ contract GovernanceTest is Test { vm.warp(block.timestamp + EPOCH_DURATION); // warp to second epoch // get initiative voting power at start of epoch - (uint88 voteLQTY0,, uint32 averageStakingTimestampVoteLQTY0,,) = governance.initiativeStates(baseInitiative1); + (uint88 voteLQTY0,, uint120 averageStakingTimestampVoteLQTY0,,) = governance.initiativeStates(baseInitiative1); uint240 currentInitiativePower0 = - governance.lqtyToVotes(voteLQTY0, block.timestamp, averageStakingTimestampVoteLQTY0); + governance.lqtyToVotes(voteLQTY0, uint120(block.timestamp), averageStakingTimestampVoteLQTY0); assertEq(currentInitiativePower0, 0, "initiative voting power is > 0"); _allocateLQTY(user, lqtyAmount); @@ -2194,9 +2194,9 @@ contract GovernanceTest is Test { governance.snapshotVotesForInitiative(baseInitiative1); // get initiative voting power at start of epoch - (uint88 voteLQTY1,, uint32 averageStakingTimestampVoteLQTY1,,) = governance.initiativeStates(baseInitiative1); + (uint88 voteLQTY1,, uint120 averageStakingTimestampVoteLQTY1,,) = governance.initiativeStates(baseInitiative1); uint240 currentInitiativePower1 = - governance.lqtyToVotes(voteLQTY1, block.timestamp, averageStakingTimestampVoteLQTY1); + governance.lqtyToVotes(voteLQTY1, uint120(block.timestamp), averageStakingTimestampVoteLQTY1); // 4a. votes from snapshotting at begging of epoch (uint224 votes,,,) = governance.votesForInitiativeSnapshot(baseInitiative1); @@ -2391,11 +2391,11 @@ contract GovernanceTest is Test { vm.warp(block.timestamp + EPOCH_DURATION); governance.snapshotVotesForInitiative(baseInitiative1); - (, uint32 averageStakingTimestampBefore) = governance.userStates(user); + (, uint120 averageStakingTimestampBefore) = governance.userStates(user); _deAllocateLQTY(user, 0); - (, uint32 averageStakingTimestampAfter) = governance.userStates(user); + (, uint120 averageStakingTimestampAfter) = governance.userStates(user); assertEq(averageStakingTimestampBefore, averageStakingTimestampAfter); } @@ -2446,9 +2446,9 @@ contract GovernanceTest is Test { governance.snapshotVotesForInitiative(baseInitiative1); // voting power for initiative should be the same as votes from snapshot - (uint88 voteLQTY,, uint32 averageStakingTimestampVoteLQTY,,) = governance.initiativeStates(baseInitiative1); + (uint88 voteLQTY,, uint120 averageStakingTimestampVoteLQTY,,) = governance.initiativeStates(baseInitiative1); uint240 currentInitiativePower = - governance.lqtyToVotes(voteLQTY, block.timestamp, averageStakingTimestampVoteLQTY); + governance.lqtyToVotes(voteLQTY, uint120(block.timestamp), averageStakingTimestampVoteLQTY); // 4. votes should not affect accounting for votes (uint224 votes,,,) = governance.votesForInitiativeSnapshot(baseInitiative1); diff --git a/test/GovernanceAttacks.t.sol b/test/GovernanceAttacks.t.sol index eb572667..27b8b9eb 100644 --- a/test/GovernanceAttacks.t.sol +++ b/test/GovernanceAttacks.t.sol @@ -83,10 +83,10 @@ contract GovernanceTest is Test { // deploy and deposit 1 LQTY governance.depositLQTY(1e18); assertEq(UserProxy(payable(userProxy)).staked(), 1e18); - (uint88 allocatedLQTY, uint32 averageStakingTimestamp) = governance.userStates(user); + (uint88 allocatedLQTY, uint120 averageStakingTimestamp) = governance.userStates(user); assertEq(allocatedLQTY, 0); // first deposit should have an averageStakingTimestamp if block.timestamp - assertEq(averageStakingTimestamp, block.timestamp); + assertEq(averageStakingTimestamp, block.timestamp * 1e18); // TODO: Normalize vm.stopPrank(); vm.startPrank(lusdHolder); diff --git a/test/VotingPower.t.sol b/test/VotingPower.t.sol index 8f878d67..521ce78b 100644 --- a/test/VotingPower.t.sol +++ b/test/VotingPower.t.sol @@ -426,7 +426,7 @@ contract VotingPowerTest is Test { // The weights are the avg time * the number function _getAverageTS(address initiative) internal view returns (uint256) { - (,, uint32 averageStakingTimestampVoteLQTY,,) = governance.initiativeStates(initiative); + (,, uint120 averageStakingTimestampVoteLQTY,,) = governance.initiativeStates(initiative); return averageStakingTimestampVoteLQTY; } From d27487f4b9b1142e98fbd5b0ffdaf71f3fd9e531 Mon Sep 17 00:00:00 2001 From: gallo Date: Fri, 1 Nov 2024 09:56:57 +0100 Subject: [PATCH 07/57] feat: invariants compile and have a few TS tests --- test/recon/Properties.sol | 3 +- test/recon/Setup.sol | 4 +++ .../properties/BribeInitiativeProperties.sol | 6 ++-- .../recon/properties/GovernanceProperties.sol | 24 ++++++------- test/recon/properties/SynchProperties.sol | 4 +-- test/recon/properties/TsProperties.sol | 34 +++++++++++++++++++ test/recon/targets/GovernanceTargets.sol | 13 +++++++ 7 files changed, 70 insertions(+), 18 deletions(-) create mode 100644 test/recon/properties/TsProperties.sol diff --git a/test/recon/Properties.sol b/test/recon/Properties.sol index 4633caed..a902a51c 100644 --- a/test/recon/Properties.sol +++ b/test/recon/Properties.sol @@ -8,5 +8,6 @@ import {OptimizationProperties} from "./properties/OptimizationProperties.sol"; import {BribeInitiativeProperties} from "./properties/BribeInitiativeProperties.sol"; import {SynchProperties} from "./properties/SynchProperties.sol"; import {RevertProperties} from "./properties/RevertProperties.sol"; +import {TsProperties} from "./properties/TsProperties.sol"; -abstract contract Properties is OptimizationProperties, BribeInitiativeProperties, SynchProperties, RevertProperties {} +abstract contract Properties is OptimizationProperties, BribeInitiativeProperties, SynchProperties, RevertProperties, TsProperties {} diff --git a/test/recon/Setup.sol b/test/recon/Setup.sol index 40da5d53..a1e8f604 100644 --- a/test/recon/Setup.sol +++ b/test/recon/Setup.sol @@ -43,6 +43,8 @@ abstract contract Setup is BaseSetup { uint32 internal constant EPOCH_DURATION = 604800; uint32 internal constant EPOCH_VOTING_CUTOFF = 518400; + uint120 magnifiedStartTS; + function setup() internal virtual override { vm.warp(block.timestamp + EPOCH_DURATION * 4); // Somehow Medusa goes back after the constructor // Random TS that is realistic @@ -91,6 +93,8 @@ abstract contract Setup is BaseSetup { deployedInitiatives.push(address(initiative1)); governance.registerInitiative(address(initiative1)); + + magnifiedStartTS = uint120(block.timestamp) * uint120(1e18); } function _getDeployedInitiative(uint8 index) internal view returns (address initiative) { diff --git a/test/recon/properties/BribeInitiativeProperties.sol b/test/recon/properties/BribeInitiativeProperties.sol index 1025dab3..28199307 100644 --- a/test/recon/properties/BribeInitiativeProperties.sol +++ b/test/recon/properties/BribeInitiativeProperties.sol @@ -66,7 +66,7 @@ abstract contract BribeInitiativeProperties is BeforeAfter { (uint88 voteLQTY,, uint16 epoch) = governance.lqtyAllocatedByUserToInitiative(user, deployedInitiatives[i]); - try initiative.lqtyAllocatedByUserAtEpoch(user, epoch) returns (uint88 amt, uint32) { + try initiative.lqtyAllocatedByUserAtEpoch(user, epoch) returns (uint88 amt, uint120) { eq(voteLQTY, amt, "Allocation must match"); } catch { t(false, "Allocation doesn't match governance"); @@ -202,10 +202,10 @@ abstract contract BribeInitiativeProperties is BeforeAfter { IBribeInitiative initiative = IBribeInitiative(deployedInitiatives[i]); uint256 sumOfPower; for (uint8 j; j < users.length; j++) { - (uint88 lqtyAllocated, uint32 userTS) = initiative.lqtyAllocatedByUserAtEpoch(users[j], currentEpoch); + (uint88 lqtyAllocated, uint120 userTS) = initiative.lqtyAllocatedByUserAtEpoch(users[j], currentEpoch); sumOfPower += governance.lqtyToVotes(lqtyAllocated, userTS, uint32(block.timestamp)); } - (uint88 totalLQTYAllocated, uint32 totalTS) = initiative.totalLQTYAllocatedByEpoch(currentEpoch); + (uint88 totalLQTYAllocated, uint120 totalTS) = initiative.totalLQTYAllocatedByEpoch(currentEpoch); uint256 totalRecordedPower = governance.lqtyToVotes(totalLQTYAllocated, totalTS, uint32(block.timestamp)); diff --git a/test/recon/properties/GovernanceProperties.sol b/test/recon/properties/GovernanceProperties.sol index 001fefcf..66435dd2 100644 --- a/test/recon/properties/GovernanceProperties.sol +++ b/test/recon/properties/GovernanceProperties.sol @@ -217,16 +217,16 @@ abstract contract GovernanceProperties is BeforeAfter { for (uint256 j; j < users.length; j++) { (uint88 userVoteLQTY,,) = governance.lqtyAllocatedByUserToInitiative(users[j], deployedInitiatives[i]); // TODO: double check that okay to use this average timestamp - (, uint32 averageStakingTimestamp) = governance.userStates(users[j]); + (, uint120 averageStakingTimestamp) = governance.userStates(users[j]); // add the weight calculated for each user's allocation to the accumulator userWeightAccumulatorForInitiative += - governance.lqtyToVotes(userVoteLQTY, block.timestamp, averageStakingTimestamp); + governance.lqtyToVotes(userVoteLQTY, uint120(block.timestamp) * uint120(1e18), averageStakingTimestamp); } - (uint88 initiativeVoteLQTY,, uint32 initiativeAverageStakingTimestampVoteLQTY,,) = + (uint88 initiativeVoteLQTY,, uint120 initiativeAverageStakingTimestampVoteLQTY,,) = governance.initiativeStates(deployedInitiatives[i]); uint240 initiativeWeight = - governance.lqtyToVotes(initiativeVoteLQTY, block.timestamp, initiativeAverageStakingTimestampVoteLQTY); + governance.lqtyToVotes(initiativeVoteLQTY, uint120(block.timestamp) * uint120(1e18), initiativeAverageStakingTimestampVoteLQTY); acc[i].userSum = userWeightAccumulatorForInitiative; acc[i].initiativeWeight = initiativeWeight; @@ -400,13 +400,13 @@ abstract contract GovernanceProperties is BeforeAfter { // GET state and initiative data before allocation ( uint88 totalCountedLQTY, - uint32 user_countedVoteLQTYAverageTimestamp + uint120 user_countedVoteLQTYAverageTimestamp ) = governance.globalState(); ( uint88 voteLQTY, uint88 vetoLQTY, - uint32 averageStakingTimestampVoteLQTY, - uint32 averageStakingTimestampVetoLQTY, + uint120 averageStakingTimestampVoteLQTY, + uint120 averageStakingTimestampVetoLQTY, ) = governance.initiativeStates(targetInitiative); @@ -427,11 +427,11 @@ abstract contract GovernanceProperties is BeforeAfter { // Deposit (Changes total LQTY an hopefully also changes ts) { - (, uint32 averageStakingTimestamp1) = governance.userStates(user); + (, uint120 averageStakingTimestamp1) = governance.userStates(user); lqtyAmount = uint88(lqtyAmount % lqty.balanceOf(user)); governance.depositLQTY(lqtyAmount); - (, uint32 averageStakingTimestamp2) = governance.userStates(user); + (, uint120 averageStakingTimestamp2) = governance.userStates(user); require(averageStakingTimestamp2 > averageStakingTimestamp1, "Must have changed"); } @@ -446,13 +446,13 @@ abstract contract GovernanceProperties is BeforeAfter { { ( uint88 after_totalCountedLQTY, - uint32 after_user_countedVoteLQTYAverageTimestamp + uint120 after_user_countedVoteLQTYAverageTimestamp ) = governance.globalState(); ( uint88 after_voteLQTY, uint88 after_vetoLQTY, - uint32 after_averageStakingTimestampVoteLQTY, - uint32 after_averageStakingTimestampVetoLQTY, + uint120 after_averageStakingTimestampVoteLQTY, + uint120 after_averageStakingTimestampVetoLQTY, ) = governance.initiativeStates(targetInitiative); diff --git a/test/recon/properties/SynchProperties.sol b/test/recon/properties/SynchProperties.sol index 7ff77354..965bc241 100644 --- a/test/recon/properties/SynchProperties.sol +++ b/test/recon/properties/SynchProperties.sol @@ -20,7 +20,7 @@ abstract contract SynchProperties is BeforeAfter { (uint88 votes, , uint16 epoch) = governance.lqtyAllocatedByUserToInitiative(users[j], deployedInitiatives[i]); // Grab epoch from initiative - (uint88 lqtyAllocatedByUserAtEpoch, uint32 ts) = + (uint88 lqtyAllocatedByUserAtEpoch, uint120 ts) = IBribeInitiative(deployedInitiatives[i]).lqtyAllocatedByUserAtEpoch(users[j], epoch); // Check that TS matches (only for votes) @@ -29,7 +29,7 @@ abstract contract SynchProperties is BeforeAfter { if(votes != 0) { // if we're voting and the votes are different from 0 // then we check user TS - (, uint32 averageStakingTimestamp) = governance.userStates(users[j]); + (, uint120 averageStakingTimestamp) = governance.userStates(users[j]); eq(averageStakingTimestamp, ts, "Timestamp must be most recent when it's non zero"); } else { diff --git a/test/recon/properties/TsProperties.sol b/test/recon/properties/TsProperties.sol new file mode 100644 index 00000000..9165d7f1 --- /dev/null +++ b/test/recon/properties/TsProperties.sol @@ -0,0 +1,34 @@ +// SPDX-License-Identifier: GPL-2.0 +pragma solidity ^0.8.0; + +import {BeforeAfter} from "../BeforeAfter.sol"; +import {Governance} from "src/Governance.sol"; +import {IGovernance} from "src/interfaces/IGovernance.sol"; +import {IBribeInitiative} from "src/interfaces/IBribeInitiative.sol"; + +abstract contract TsProperties is BeforeAfter { + // Properties that ensure that a user TS is somewhat sound + + function property_user_ts_is_always_greater_than_start() public { + for(uint256 i; i < users.length; i++) { + (uint88 user_allocatedLQTY, uint120 userTs) = governance.userStates(users[i]); + if(user_allocatedLQTY > 0) { + gte(userTs, magnifiedStartTS, "User ts must always be GTE than start"); + } + } + } + + function property_global_ts_is_always_greater_than_start() public { + ( + uint88 totalCountedLQTY, + uint120 globalTs + ) = governance.globalState(); + + if(totalCountedLQTY > 0) { + gte(globalTs, magnifiedStartTS, "Global ts must always be GTE than start"); + } + } + + + +} \ No newline at end of file diff --git a/test/recon/targets/GovernanceTargets.sol b/test/recon/targets/GovernanceTargets.sol index 2a88b33b..6cdcedb2 100644 --- a/test/recon/targets/GovernanceTargets.sol +++ b/test/recon/targets/GovernanceTargets.sol @@ -106,6 +106,19 @@ abstract contract GovernanceTargets is BaseTargetFunctions, Properties { eq(user_allocatedLQTY, 0, "User has 0 allocated on a reset"); } + function depositTsIsRational(uint88 lqtyAmount) public withChecks { + (uint88 user_allocatedLQTY,) = governance.userStates(user); + + // Deposit on zero + if(user_allocatedLQTY == 0) { + lqtyAmount = uint88(lqtyAmount % lqty.balanceOf(user)); + governance.depositLQTY(lqtyAmount); + + // assert that user TS is now * WAD + (, uint120 ts) = governance.userStates(user); + eq(ts, block.timestamp * 1e18, "User TS is scaled by WAD"); + } + } function depositMustFailOnNonZeroAlloc(uint88 lqtyAmount) public withChecks { (uint88 user_allocatedLQTY,) = governance.userStates(user); From 5d69f9abac143d13e23520af4b0ff04aaf9a3696 Mon Sep 17 00:00:00 2001 From: gallo Date: Fri, 1 Nov 2024 10:03:10 +0100 Subject: [PATCH 08/57] chore: easy tests are fixed --- test/Governance.t.sol | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/test/Governance.t.sol b/test/Governance.t.sol index 5bbf1fca..bab752a2 100644 --- a/test/Governance.t.sol +++ b/test/Governance.t.sol @@ -199,7 +199,7 @@ contract GovernanceTest is Test { (uint88 allocatedLQTY, uint120 averageStakingTimestamp) = governance.userStates(user); assertEq(allocatedLQTY, 0); // first deposit should have an averageStakingTimestamp if block.timestamp - assertEq(averageStakingTimestamp, block.timestamp); + assertEq(averageStakingTimestamp, block.timestamp * 1e18); vm.warp(block.timestamp + timeIncrease); @@ -209,7 +209,7 @@ contract GovernanceTest is Test { (allocatedLQTY, averageStakingTimestamp) = governance.userStates(user); assertEq(allocatedLQTY, 0); // subsequent deposits should have a stake weighted average - assertEq(averageStakingTimestamp, block.timestamp - timeIncrease / 2); + assertEq(averageStakingTimestamp, (block.timestamp - timeIncrease / 2) * 1e18, "Avg ts"); // withdraw 0.5 half of LQTY vm.warp(block.timestamp + timeIncrease); @@ -225,14 +225,14 @@ contract GovernanceTest is Test { assertEq(UserProxy(payable(userProxy)).staked(), 1e18); (allocatedLQTY, averageStakingTimestamp) = governance.userStates(user); assertEq(allocatedLQTY, 0); - assertEq(averageStakingTimestamp, (block.timestamp - timeIncrease) - timeIncrease / 2); + assertEq(averageStakingTimestamp, ((block.timestamp - timeIncrease) - timeIncrease / 2) * 1e18, "avg ts2"); // withdraw remaining LQTY governance.withdrawLQTY(1e18); assertEq(UserProxy(payable(userProxy)).staked(), 0); (allocatedLQTY, averageStakingTimestamp) = governance.userStates(user); assertEq(allocatedLQTY, 0); - assertEq(averageStakingTimestamp, (block.timestamp - timeIncrease) - timeIncrease / 2); + assertEq(averageStakingTimestamp, ((block.timestamp - timeIncrease) - timeIncrease / 2) * 1e18, "avg ts3"); vm.stopPrank(); } @@ -303,7 +303,7 @@ contract GovernanceTest is Test { assertEq(UserProxy(payable(userProxy)).staked(), 1e18); (uint88 allocatedLQTY, uint120 averageStakingTimestamp) = governance.userStates(wallet.addr); assertEq(allocatedLQTY, 0); - assertEq(averageStakingTimestamp, block.timestamp); + assertEq(averageStakingTimestamp, block.timestamp * 1e18); } function test_claimFromStakingV1() public { @@ -1057,7 +1057,7 @@ contract GovernanceTest is Test { assertEq(vetoLQTY, 0); // should update the average staking timestamp for the initiative based on the average staking timestamp of the user's // voting and vetoing LQTY - assertEq(averageStakingTimestampVoteLQTY, block.timestamp - governance.EPOCH_DURATION()); + assertEq(averageStakingTimestampVoteLQTY, (block.timestamp - governance.EPOCH_DURATION()) * 1e18); assertEq(averageStakingTimestampVoteLQTY, averageStakingTimestampUser); assertEq(averageStakingTimestampVetoLQTY, 0); // should remove or add the initiatives voting LQTY from the counter @@ -1110,7 +1110,7 @@ contract GovernanceTest is Test { governance.initiativeStates(baseInitiative1); assertEq(voteLQTY, 2e18); assertEq(vetoLQTY, 0); - assertEq(averageStakingTimestampVoteLQTY, block.timestamp - governance.EPOCH_DURATION()); + assertEq(averageStakingTimestampVoteLQTY, (block.timestamp - governance.EPOCH_DURATION()) * 1e18); assertGt(averageStakingTimestampVoteLQTY, averageStakingTimestampUser); assertEq(averageStakingTimestampVetoLQTY, 0); @@ -1181,7 +1181,7 @@ contract GovernanceTest is Test { assertEq(vetoLQTY, 0); // should update the average staking timestamp for the initiative based on the average staking timestamp of the user's // voting and vetoing LQTY - assertEq(averageStakingTimestampVoteLQTY, block.timestamp - governance.EPOCH_DURATION(), "TS"); + assertEq(averageStakingTimestampVoteLQTY, (block.timestamp - governance.EPOCH_DURATION()) * 1e18, "TS"); assertEq(averageStakingTimestampVoteLQTY, averageStakingTimestampUser); assertEq(averageStakingTimestampVetoLQTY, 0); // should remove or add the initiatives voting LQTY from the counter @@ -1234,7 +1234,7 @@ contract GovernanceTest is Test { governance.initiativeStates(baseInitiative1); assertEq(voteLQTY, 2e18); assertEq(vetoLQTY, 0); - assertEq(averageStakingTimestampVoteLQTY, block.timestamp - governance.EPOCH_DURATION(), "TS 2"); + assertEq(averageStakingTimestampVoteLQTY, (block.timestamp - governance.EPOCH_DURATION()) * 1e18, "TS 2"); assertGt(averageStakingTimestampVoteLQTY, averageStakingTimestampUser); assertEq(averageStakingTimestampVetoLQTY, 0); From 4b75ce1aa5876f51ac968480fcdb1733e8037f9e Mon Sep 17 00:00:00 2001 From: gallo Date: Fri, 1 Nov 2024 10:11:59 +0100 Subject: [PATCH 09/57] chore: fix mock tests --- test/mocks/MockGovernance.sol | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/mocks/MockGovernance.sol b/test/mocks/MockGovernance.sol index 08cc1037..e1f502ef 100644 --- a/test/mocks/MockGovernance.sol +++ b/test/mocks/MockGovernance.sol @@ -19,16 +19,16 @@ contract MockGovernance { return __epoch; } - function _averageAge(uint32 _currentTimestamp, uint32 _averageTimestamp) internal pure returns (uint32) { + function _averageAge(uint120 _currentTimestamp, uint120 _averageTimestamp) internal pure returns (uint120) { if (_averageTimestamp == 0 || _currentTimestamp < _averageTimestamp) return 0; return _currentTimestamp - _averageTimestamp; } - function lqtyToVotes(uint88 _lqtyAmount, uint256 _currentTimestamp, uint32 _averageTimestamp) + function lqtyToVotes(uint88 _lqtyAmount, uint120 _currentTimestamp, uint120 _averageTimestamp) public pure - returns (uint240) + returns (uint208) { - return uint240(_lqtyAmount) * _averageAge(uint32(_currentTimestamp), _averageTimestamp); + return uint208(_lqtyAmount) * uint208(_averageAge(_currentTimestamp, _averageTimestamp)); } } From 7c8a57702e87ae9acc81e3d685425c83c9497f43 Mon Sep 17 00:00:00 2001 From: gallo Date: Fri, 1 Nov 2024 10:53:56 +0100 Subject: [PATCH 10/57] fix: epochStart is magnified with a hacky solution --- src/BribeInitiative.sol | 4 ++-- src/Governance.sol | 6 +++--- test/Governance.t.sol | 4 ++-- test/recon/properties/TsProperties.sol | 3 +++ 4 files changed, 10 insertions(+), 7 deletions(-) diff --git a/src/BribeInitiative.sol b/src/BribeInitiative.sol index c9e8da37..2ee9e909 100644 --- a/src/BribeInitiative.sol +++ b/src/BribeInitiative.sol @@ -107,7 +107,7 @@ contract BribeInitiative is IInitiative, IBribeInitiative { uint120 epochEnd = (uint120(governance.EPOCH_START()) + uint120(_epoch) * uint120(governance.EPOCH_DURATION())) * uint120(WAD); /// @audit User Invariant - assert(totalAverageTimestamp <= epochEnd); /// NOTE: Tests break because they are not realistic + assert(totalAverageTimestamp <= epochEnd); uint240 totalVotes = governance.lqtyToVotes(totalLQTY, epochEnd, totalAverageTimestamp); @@ -115,7 +115,7 @@ contract BribeInitiative is IInitiative, IBribeInitiative { (uint88 lqty, uint120 averageTimestamp) = _decodeLQTYAllocation(lqtyAllocation.value); /// @audit Governance Invariant - assert(averageTimestamp <= epochEnd); /// NOTE: Tests break because they are not realistic + assert(averageTimestamp <= epochEnd); uint240 votes = governance.lqtyToVotes(lqty, epochEnd, averageTimestamp); boldAmount = uint256(bribe.boldAmount) * uint256(votes) / uint256(totalVotes); diff --git a/src/Governance.sol b/src/Governance.sol index df7063f7..649548ae 100644 --- a/src/Governance.sol +++ b/src/Governance.sol @@ -328,7 +328,7 @@ contract Governance is Multicall, UserProxyFactory, ReentrancyGuard, IGovernance if (snapshot.forEpoch < currentEpoch - 1) { shouldUpdate = true; - snapshot.votes = lqtyToVotes(state.countedVoteLQTY, epochStart(), state.countedVoteLQTYAverageTimestamp); + snapshot.votes = lqtyToVotes(state.countedVoteLQTY, uint120(epochStart()) * uint120(1e18), state.countedVoteLQTYAverageTimestamp); snapshot.forEpoch = currentEpoch - 1; } } @@ -367,7 +367,7 @@ contract Governance is Multicall, UserProxyFactory, ReentrancyGuard, IGovernance if (initiativeSnapshot.forEpoch < currentEpoch - 1) { shouldUpdate = true; - uint32 start = epochStart(); + uint120 start = uint120(epochStart()) * uint120(1e18); uint240 votes = lqtyToVotes(initiativeState.voteLQTY, start, initiativeState.averageStakingTimestampVoteLQTY); uint240 vetos = @@ -502,7 +502,7 @@ contract Governance is Multicall, UserProxyFactory, ReentrancyGuard, IGovernance // an initiative can be registered if the registrant has more voting power (LQTY * age) // than the registration threshold derived from the previous epoch's total global votes require( - lqtyToVotes(uint88(stakingV1.stakes(userProxyAddress)), epochStart(), userState.averageStakingTimestamp) + lqtyToVotes(uint88(stakingV1.stakes(userProxyAddress)), uint120(epochStart()) * uint120(1e18), userState.averageStakingTimestamp) >= snapshot.votes * REGISTRATION_THRESHOLD_FACTOR / WAD, "Governance: insufficient-lqty" ); diff --git a/test/Governance.t.sol b/test/Governance.t.sol index bab752a2..97adce5a 100644 --- a/test/Governance.t.sol +++ b/test/Governance.t.sol @@ -1917,7 +1917,7 @@ contract GovernanceTest is Test { // get initiative voting power at start of epoch (uint88 voteLQTY0,, uint120 averageStakingTimestampVoteLQTY0,,) = governance.initiativeStates(baseInitiative1); uint240 currentInitiativePower0 = - governance.lqtyToVotes(voteLQTY0, uint120(block.timestamp), averageStakingTimestampVoteLQTY0); + governance.lqtyToVotes(voteLQTY0, uint120(block.timestamp) * uint120(1e18), averageStakingTimestampVoteLQTY0); assertEq(currentInitiativePower0, 0, "initiative voting power is > 0"); _allocateLQTY(user, lqtyAmount); @@ -1930,7 +1930,7 @@ contract GovernanceTest is Test { // get initiative voting power at time of snapshot (uint88 voteLQTY1,, uint120 averageStakingTimestampVoteLQTY1,,) = governance.initiativeStates(baseInitiative1); uint240 currentInitiativePower1 = - governance.lqtyToVotes(voteLQTY1, uint120(block.timestamp), averageStakingTimestampVoteLQTY1); + governance.lqtyToVotes(voteLQTY1, uint120(block.timestamp) * uint120(1e18), averageStakingTimestampVoteLQTY1); assertGt(currentInitiativePower1, 0, "initiative voting power is 0"); uint240 deltaInitiativeVotingPower = currentInitiativePower1 - currentInitiativePower0; diff --git a/test/recon/properties/TsProperties.sol b/test/recon/properties/TsProperties.sol index 9165d7f1..7523e7ad 100644 --- a/test/recon/properties/TsProperties.sol +++ b/test/recon/properties/TsProperties.sol @@ -28,6 +28,9 @@ abstract contract TsProperties is BeforeAfter { gte(globalTs, magnifiedStartTS, "Global ts must always be GTE than start"); } } + + + // TODO: Waiting 1 second should give 1 an extra second * WAD power From 105bc32fda22ed1165ca0e44743db1f993a1b91a Mon Sep 17 00:00:00 2001 From: gallo Date: Fri, 1 Nov 2024 10:55:44 +0100 Subject: [PATCH 11/57] fix: test ts are magnified --- test/Governance.t.sol | 48 +++++++++++++++++++++---------------------- 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/test/Governance.t.sol b/test/Governance.t.sol index 97adce5a..86a2db3e 100644 --- a/test/Governance.t.sol +++ b/test/Governance.t.sol @@ -674,7 +674,7 @@ contract GovernanceTest is Test { (uint88 voteLQTY2,,,,) = governance.initiativeStates(baseInitiative2); // Get power at time of vote - uint256 votingPower = governance.lqtyToVotes(voteLQTY1, uint120(block.timestamp), averageStakingTimestampVoteLQTY1); + uint256 votingPower = governance.lqtyToVotes(voteLQTY1, uint120(block.timestamp) * uint120(1e18), averageStakingTimestampVoteLQTY1); assertGt(votingPower, 0, "Non zero power"); /// @audit TODO Fully digest and explain the bug @@ -1091,7 +1091,7 @@ contract GovernanceTest is Test { governance.depositLQTY(1e18); (, uint120 averageAge) = governance.userStates(user2); - assertEq(governance.lqtyToVotes(1e18, uint120(block.timestamp), averageAge), 0); + assertEq(governance.lqtyToVotes(1e18, uint120(block.timestamp) * uint120(1e18), averageAge), 0); deltaLQTYVetos[0] = 1e18; @@ -1215,7 +1215,7 @@ contract GovernanceTest is Test { governance.depositLQTY(1e18); (, uint120 averageAge) = governance.userStates(user2); - assertEq(governance.lqtyToVotes(1e18, uint120(block.timestamp), averageAge), 0); + assertEq(governance.lqtyToVotes(1e18, uint120(block.timestamp) * uint120(1e18), averageAge), 0); deltaLQTYVetos[0] = 1e18; @@ -1709,11 +1709,11 @@ contract GovernanceTest is Test { _stakeLQTY(user, lqtyAmount); (uint88 allocatedLQTY0, uint120 averageStakingTimestamp0) = governance.userStates(user); - uint240 currentUserPower0 = governance.lqtyToVotes(allocatedLQTY0, uint120(block.timestamp), averageStakingTimestamp0); + uint240 currentUserPower0 = governance.lqtyToVotes(allocatedLQTY0, uint120(block.timestamp) * uint120(1e18), averageStakingTimestamp0); (uint88 voteLQTY0,, uint120 averageStakingTimestampVoteLQTY0,,) = governance.initiativeStates(baseInitiative1); uint240 currentInitiativePower0 = - governance.lqtyToVotes(voteLQTY0, uint120(block.timestamp), averageStakingTimestampVoteLQTY0); + governance.lqtyToVotes(voteLQTY0, uint120(block.timestamp) * uint120(1e18), averageStakingTimestampVoteLQTY0); // (uint224 votes, uint16 forEpoch,,) = governance.votesForInitiativeSnapshot(baseInitiative1); // console2.log("votes0: ", votes); @@ -1726,14 +1726,14 @@ contract GovernanceTest is Test { // check user voting power for the current epoch (uint88 allocatedLQTY1, uint120 averageStakingTimestamp1) = governance.userStates(user); - uint240 currentUserPower1 = governance.lqtyToVotes(allocatedLQTY1, uint120(block.timestamp), averageStakingTimestamp1); + uint240 currentUserPower1 = governance.lqtyToVotes(allocatedLQTY1, uint120(block.timestamp) * uint120(1e18), averageStakingTimestamp1); // user's allocated lqty should immediately increase their voting power assertGt(currentUserPower1, 0, "current user voting power is 0"); // check initiative voting power for the current epoch (uint88 voteLQTY1,, uint120 averageStakingTimestampVoteLQTY1,,) = governance.initiativeStates(baseInitiative1); uint240 currentInitiativePower1 = - governance.lqtyToVotes(voteLQTY1, uint120(block.timestamp), averageStakingTimestampVoteLQTY1); + governance.lqtyToVotes(voteLQTY1, uint120(block.timestamp) * uint120(1e18), averageStakingTimestampVoteLQTY1); assertGt(currentInitiativePower1, 0, "current initiative voting power is 0"); assertEq(currentUserPower1, currentInitiativePower1, "initiative and user voting power should be equal"); @@ -1747,13 +1747,13 @@ contract GovernanceTest is Test { // user voting power should increase over a given chunk of time (uint88 allocatedLQTY2, uint120 averageStakingTimestamp2) = governance.userStates(user); - uint240 currentUserPower2 = governance.lqtyToVotes(allocatedLQTY2, uint120(block.timestamp), averageStakingTimestamp2); + uint240 currentUserPower2 = governance.lqtyToVotes(allocatedLQTY2, uint120(block.timestamp) * uint120(1e18), averageStakingTimestamp2); assertGt(currentUserPower2, currentUserPower1); // initiative voting power should increase over a given chunk of time (uint88 voteLQTY2,, uint120 averageStakingTimestampVoteLQTY2,,) = governance.initiativeStates(baseInitiative1); uint240 currentInitiativePower2 = - governance.lqtyToVotes(voteLQTY2, uint120(block.timestamp), averageStakingTimestampVoteLQTY2); + governance.lqtyToVotes(voteLQTY2, uint120(block.timestamp) * uint120(1e18), averageStakingTimestampVoteLQTY2); assertEq( currentUserPower2, currentInitiativePower2, "user power and initiative power should increase by same amount" ); @@ -1769,12 +1769,12 @@ contract GovernanceTest is Test { // user voting power should increase (uint88 allocatedLQTY3, uint120 averageStakingTimestamp3) = governance.userStates(user); - uint240 currentUserPower3 = governance.lqtyToVotes(allocatedLQTY3, uint120(block.timestamp), averageStakingTimestamp3); + uint240 currentUserPower3 = governance.lqtyToVotes(allocatedLQTY3, uint120(block.timestamp) * uint120(1e18), averageStakingTimestamp3); // votes should match the voting power for the initiative and subsequently the user since they're the only one allocated (uint88 voteLQTY3,, uint120 averageStakingTimestampVoteLQTY3,,) = governance.initiativeStates(baseInitiative1); uint240 currentInitiativePower3 = - governance.lqtyToVotes(voteLQTY3, uint120(block.timestamp), averageStakingTimestampVoteLQTY3); + governance.lqtyToVotes(voteLQTY3, uint120(block.timestamp) * uint120(1e18), averageStakingTimestampVoteLQTY3); // votes should be counted in this epoch (votes, forEpoch,,) = governance.votesForInitiativeSnapshot(baseInitiative1); @@ -1786,11 +1786,11 @@ contract GovernanceTest is Test { governance.snapshotVotesForInitiative(baseInitiative1); (uint88 allocatedLQTY4, uint120 averageStakingTimestamp4) = governance.userStates(user); - uint240 currentUserPower4 = governance.lqtyToVotes(allocatedLQTY4, uint120(block.timestamp), averageStakingTimestamp4); + uint240 currentUserPower4 = governance.lqtyToVotes(allocatedLQTY4, uint120(block.timestamp) * uint120(1e18), averageStakingTimestamp4); (uint88 voteLQTY4,, uint120 averageStakingTimestampVoteLQTY4,,) = governance.initiativeStates(baseInitiative1); uint240 currentInitiativePower4 = - governance.lqtyToVotes(voteLQTY4, uint120(block.timestamp), averageStakingTimestampVoteLQTY4); + governance.lqtyToVotes(voteLQTY4, uint120(block.timestamp) * uint120(1e18), averageStakingTimestampVoteLQTY4); // checking if snapshotting at the end of an epoch increases the voting power (uint224 votes2,,,) = governance.votesForInitiativeSnapshot(baseInitiative1); @@ -1834,13 +1834,13 @@ contract GovernanceTest is Test { // check user voting power before allocation at epoch start (uint88 allocatedLQTY0, uint120 averageStakingTimestamp0) = governance.userStates(user); - uint240 currentUserPower0 = governance.lqtyToVotes(allocatedLQTY0, uint120(block.timestamp), averageStakingTimestamp0); + uint240 currentUserPower0 = governance.lqtyToVotes(allocatedLQTY0, uint120(block.timestamp) * uint120(1e18), averageStakingTimestamp0); assertEq(currentUserPower0, 0, "user has voting power > 0"); // check initiative voting power before allocation at epoch start (uint88 voteLQTY0,, uint120 averageStakingTimestampVoteLQTY0,,) = governance.initiativeStates(baseInitiative1); uint240 currentInitiativePower0 = - governance.lqtyToVotes(voteLQTY0, uint120(block.timestamp), averageStakingTimestampVoteLQTY0); + governance.lqtyToVotes(voteLQTY0, uint120(block.timestamp) * uint120(1e18), averageStakingTimestampVoteLQTY0); assertEq(currentInitiativePower0, 0, "current initiative voting power is > 0"); _allocateLQTY(user, lqtyAmount); @@ -1850,13 +1850,13 @@ contract GovernanceTest is Test { // check user voting power after allocation at epoch end (uint88 allocatedLQTY1, uint120 averageStakingTimestamp1) = governance.userStates(user); - uint240 currentUserPower1 = governance.lqtyToVotes(allocatedLQTY1, uint120(block.timestamp), averageStakingTimestamp1); + uint240 currentUserPower1 = governance.lqtyToVotes(allocatedLQTY1, uint120(block.timestamp) * uint120(1e18), averageStakingTimestamp1); assertGt(currentUserPower1, 0, "user has no voting power after allocation"); // check initiative voting power after allocation at epoch end (uint88 voteLQTY1,, uint120 averageStakingTimestampVoteLQTY1,,) = governance.initiativeStates(baseInitiative1); uint240 currentInitiativePower1 = - governance.lqtyToVotes(voteLQTY1, uint120(block.timestamp), averageStakingTimestampVoteLQTY1); + governance.lqtyToVotes(voteLQTY1, uint120(block.timestamp) * uint120(1e18), averageStakingTimestampVoteLQTY1); assertGt(currentInitiativePower1, 0, "initiative has no voting power after allocation"); // check that user and initiative voting power is equivalent at epoch end @@ -1867,13 +1867,13 @@ contract GovernanceTest is Test { // get user voting power after multiple epochs (uint88 allocatedLQTY2, uint120 averageStakingTimestamp2) = governance.userStates(user); - uint240 currentUserPower2 = governance.lqtyToVotes(allocatedLQTY2, uint120(block.timestamp), averageStakingTimestamp2); + uint240 currentUserPower2 = governance.lqtyToVotes(allocatedLQTY2, uint120(block.timestamp) * uint120(1e18), averageStakingTimestamp2); assertGt(currentUserPower2, currentUserPower1, "user voting power doesn't increase"); // get initiative voting power after multiple epochs (uint88 voteLQTY2,, uint120 averageStakingTimestampVoteLQTY2,,) = governance.initiativeStates(baseInitiative1); uint240 currentInitiativePower2 = - governance.lqtyToVotes(voteLQTY2, uint120(block.timestamp), averageStakingTimestampVoteLQTY2); + governance.lqtyToVotes(voteLQTY2, uint120(block.timestamp) * uint120(1e18), averageStakingTimestampVoteLQTY2); assertGt(currentInitiativePower2, currentInitiativePower1, "initiative voting power doesn't increase"); // check that initiative and user voting always track each other @@ -1978,9 +1978,9 @@ contract GovernanceTest is Test { (uint88 voteLQTY0,,) = governance.lqtyAllocatedByUserToInitiative(user, baseInitiative1); (uint88 allocatedLQTY, uint120 averageStakingTimestamp) = governance.userStates(user); uint240 currentInitiativePowerFrom1 = - governance.lqtyToVotes(voteLQTY0, uint120(block.timestamp), averageStakingTimestamp); + governance.lqtyToVotes(voteLQTY0, uint120(block.timestamp) * uint120(1e18), averageStakingTimestamp); uint240 currentInitiativePowerFrom2 = - governance.lqtyToVotes(allocatedLQTY, uint120(block.timestamp), averageStakingTimestamp); + governance.lqtyToVotes(allocatedLQTY, uint120(block.timestamp) * uint120(1e18), averageStakingTimestamp); assertEq( currentInitiativePowerFrom1, @@ -2180,7 +2180,7 @@ contract GovernanceTest is Test { // get initiative voting power at start of epoch (uint88 voteLQTY0,, uint120 averageStakingTimestampVoteLQTY0,,) = governance.initiativeStates(baseInitiative1); uint240 currentInitiativePower0 = - governance.lqtyToVotes(voteLQTY0, uint120(block.timestamp), averageStakingTimestampVoteLQTY0); + governance.lqtyToVotes(voteLQTY0, uint120(block.timestamp) * uint120(1e18), averageStakingTimestampVoteLQTY0); assertEq(currentInitiativePower0, 0, "initiative voting power is > 0"); _allocateLQTY(user, lqtyAmount); @@ -2196,7 +2196,7 @@ contract GovernanceTest is Test { // get initiative voting power at start of epoch (uint88 voteLQTY1,, uint120 averageStakingTimestampVoteLQTY1,,) = governance.initiativeStates(baseInitiative1); uint240 currentInitiativePower1 = - governance.lqtyToVotes(voteLQTY1, uint120(block.timestamp), averageStakingTimestampVoteLQTY1); + governance.lqtyToVotes(voteLQTY1, uint120(block.timestamp) * uint120(1e18), averageStakingTimestampVoteLQTY1); // 4a. votes from snapshotting at begging of epoch (uint224 votes,,,) = governance.votesForInitiativeSnapshot(baseInitiative1); @@ -2448,7 +2448,7 @@ contract GovernanceTest is Test { // voting power for initiative should be the same as votes from snapshot (uint88 voteLQTY,, uint120 averageStakingTimestampVoteLQTY,,) = governance.initiativeStates(baseInitiative1); uint240 currentInitiativePower = - governance.lqtyToVotes(voteLQTY, uint120(block.timestamp), averageStakingTimestampVoteLQTY); + governance.lqtyToVotes(voteLQTY, uint120(block.timestamp) * uint120(1e18), averageStakingTimestampVoteLQTY); // 4. votes should not affect accounting for votes (uint224 votes,,,) = governance.votesForInitiativeSnapshot(baseInitiative1); From 075bf4be13020adfc3ad637f7cdb4d7576402c3f Mon Sep 17 00:00:00 2001 From: gallo Date: Fri, 1 Nov 2024 11:05:49 +0100 Subject: [PATCH 12/57] chore: fix overflow --- test/VotingPower.t.sol | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/test/VotingPower.t.sol b/test/VotingPower.t.sol index 521ce78b..97d23e0a 100644 --- a/test/VotingPower.t.sol +++ b/test/VotingPower.t.sol @@ -316,27 +316,35 @@ contract VotingPowerTest is Test { console.log("griefed_avg", griefed_avg); console.log("block.timestamp", block.timestamp); + console.log("0?"); + + uint256 currentMagnifiedTs = uint120(block.timestamp) * uint120(1e18); + vm.startPrank(user2); _allocate(address(baseInitiative1), 15, 0); _allocate(address(baseInitiative1), 0, 0); uint256 ts = _getAverageTS(baseInitiative1); - uint256 delta = block.timestamp - ts; + uint256 delta = currentMagnifiedTs - ts; console.log("griefed_avg", ts); console.log("delta", delta); - console.log("block.timestamp", block.timestamp); + console.log("currentMagnifiedTs", currentMagnifiedTs); + console.log("0?"); uint256 i; while (i++ < 122) { + console.log("i", i); _allocate(address(baseInitiative1), 15, 0); _allocate(address(baseInitiative1), 0, 0); } + console.log("1?"); + ts = _getAverageTS(baseInitiative1); - delta = block.timestamp - ts; + delta = currentMagnifiedTs - ts; console.log("griefed_avg", ts); console.log("delta", delta); - console.log("block.timestamp", block.timestamp); + console.log("currentMagnifiedTs", currentMagnifiedTs); // One more time _allocate(address(baseInitiative1), 15, 0); From dc845b38465b48f71f6929e1824f865fd589bb13 Mon Sep 17 00:00:00 2001 From: gallo Date: Fri, 1 Nov 2024 11:06:22 +0100 Subject: [PATCH 13/57] fix: abi decode --- test/Governance.t.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/Governance.t.sol b/test/Governance.t.sol index 86a2db3e..d26f3f3c 100644 --- a/test/Governance.t.sol +++ b/test/Governance.t.sol @@ -1544,7 +1544,7 @@ contract GovernanceTest is Test { data[6] = abi.encodeWithSignature("withdrawLQTY(uint88)", lqtyAmount); bytes[] memory response = governance.multicall(data); - (uint88 allocatedLQTY,) = abi.decode(response[3], (uint88, uint32)); + (uint88 allocatedLQTY,) = abi.decode(response[3], (uint88, uint120)); assertEq(allocatedLQTY, lqtyAmount); (IGovernance.VoteSnapshot memory votes, IGovernance.InitiativeVoteSnapshot memory votesForInitiative) = abi.decode(response[4], (IGovernance.VoteSnapshot, IGovernance.InitiativeVoteSnapshot)); From 28fe9d91d7a961104de7d482391e85ca3209da18 Mon Sep 17 00:00:00 2001 From: gallo Date: Fri, 1 Nov 2024 11:24:16 +0100 Subject: [PATCH 14/57] fix: BI-07 --- test/recon/properties/BribeInitiativeProperties.sol | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/test/recon/properties/BribeInitiativeProperties.sol b/test/recon/properties/BribeInitiativeProperties.sol index 28199307..bdc1e79c 100644 --- a/test/recon/properties/BribeInitiativeProperties.sol +++ b/test/recon/properties/BribeInitiativeProperties.sol @@ -181,7 +181,9 @@ abstract contract BribeInitiativeProperties is BeforeAfter { IBribeInitiative initiative = IBribeInitiative(deployedInitiatives[i]); uint88 sumLqtyAllocated; for (uint8 j; j < users.length; j++) { - (uint88 lqtyAllocated,) = initiative.lqtyAllocatedByUserAtEpoch(users[j], currentEpoch); + // NOTE: We need to grab user latest + uint16 userEpoch = initiative.getMostRecentUserEpoch(users[j]); + (uint88 lqtyAllocated,) = initiative.lqtyAllocatedByUserAtEpoch(users[j], userEpoch); sumLqtyAllocated += lqtyAllocated; } (uint88 totalLQTYAllocated,) = initiative.totalLQTYAllocatedByEpoch(currentEpoch); @@ -193,6 +195,7 @@ abstract contract BribeInitiativeProperties is BeforeAfter { } } + function property_sum_of_votes_in_bribes_match() public { uint16 currentEpoch = governance.epoch(); From 56d8e8656c4bf9024fa871e13cb2947a32bd7f76 Mon Sep 17 00:00:00 2001 From: gallo Date: Fri, 1 Nov 2024 11:27:03 +0100 Subject: [PATCH 15/57] WARNING: Add tollerance --- test/recon/CryticToFoundry.sol | 6 +++--- .../recon/properties/GovernanceProperties.sol | 21 ++++++++++++++++++- 2 files changed, 23 insertions(+), 4 deletions(-) diff --git a/test/recon/CryticToFoundry.sol b/test/recon/CryticToFoundry.sol index 97b76ed4..cb0ed348 100644 --- a/test/recon/CryticToFoundry.sol +++ b/test/recon/CryticToFoundry.sol @@ -74,7 +74,7 @@ contract CryticToFoundry is Test, TargetFunctions, FoundryAsserts { governance_allocateLQTY_clamped_single_initiative(0,1,0); - property_sum_of_user_voting_weights(); /// Of by 2 + property_sum_of_user_voting_weights_bounded(); /// Of by 2 } @@ -197,7 +197,7 @@ contract CryticToFoundry is Test, TargetFunctions, FoundryAsserts { // Both of these are fine // Meaning all LQTY allocation is fine here // Same for user voting weights - property_sum_of_user_voting_weights(); + property_sum_of_user_voting_weights_bounded(); property_sum_of_lqty_global_user_matches(); /// === BROKEN === /// @@ -295,7 +295,7 @@ uint256 loggerCount; governance_allocateLQTY_clamped_single_initiative(0,1,0); - property_sum_of_user_voting_weights(); + property_sum_of_user_voting_weights_bounded(); } } diff --git a/test/recon/properties/GovernanceProperties.sol b/test/recon/properties/GovernanceProperties.sol index 66435dd2..99afb900 100644 --- a/test/recon/properties/GovernanceProperties.sol +++ b/test/recon/properties/GovernanceProperties.sol @@ -9,6 +9,9 @@ import {vm} from "@chimera/Hevm.sol"; import {IUserProxy} from "src/interfaces/IUserProxy.sol"; abstract contract GovernanceProperties is BeforeAfter { + + uint256 constant TOLLERANCE = 1e6; + /// A Initiative cannot change in status /// Except for being unregistered /// Or claiming rewards @@ -190,7 +193,7 @@ abstract contract GovernanceProperties is BeforeAfter { // sum of voting power for users that allocated to an initiative == the voting power of the initiative /// TODO ?? - function property_sum_of_user_voting_weights() public { + function property_sum_of_user_voting_weights_strict() public { // loop through all users // - calculate user voting weight for the given timestamp // - sum user voting weights for the given epoch @@ -205,6 +208,22 @@ abstract contract GovernanceProperties is BeforeAfter { ); } } + + function property_sum_of_user_voting_weights_bounded() public { + // loop through all users + // - calculate user voting weight for the given timestamp + // - sum user voting weights for the given epoch + // - compare with the voting weight of the initiative for the epoch for the same timestamp + VotesSumAndInitiativeSum[] memory votesSumAndInitiativeValues = _getUserVotesSumAndInitiativesVotes(); + + for(uint256 i; i < votesSumAndInitiativeValues.length; i++) { + t( + votesSumAndInitiativeValues[i].userSum >= votesSumAndInitiativeValues[i].initiativeWeight - TOLLERANCE && + votesSumAndInitiativeValues[i].userSum <= votesSumAndInitiativeValues[i].initiativeWeight + TOLLERANCE, + "initiative voting weights and user's allocated weight match within tollerance" + ); + } + } struct VotesSumAndInitiativeSum { uint256 userSum; uint256 initiativeWeight; From d82a0c30a8f5b630536fef2a5ab0bbaf622f23ec Mon Sep 17 00:00:00 2001 From: gallo Date: Fri, 1 Nov 2024 11:28:10 +0100 Subject: [PATCH 16/57] feat: laxer property --- test/recon/properties/GovernanceProperties.sol | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/test/recon/properties/GovernanceProperties.sol b/test/recon/properties/GovernanceProperties.sol index 99afb900..c7554f28 100644 --- a/test/recon/properties/GovernanceProperties.sol +++ b/test/recon/properties/GovernanceProperties.sol @@ -217,9 +217,12 @@ abstract contract GovernanceProperties is BeforeAfter { VotesSumAndInitiativeSum[] memory votesSumAndInitiativeValues = _getUserVotesSumAndInitiativesVotes(); for(uint256 i; i < votesSumAndInitiativeValues.length; i++) { - t( + t( + votesSumAndInitiativeValues[i].userSum == votesSumAndInitiativeValues[i].initiativeWeight || + ( votesSumAndInitiativeValues[i].userSum >= votesSumAndInitiativeValues[i].initiativeWeight - TOLLERANCE && - votesSumAndInitiativeValues[i].userSum <= votesSumAndInitiativeValues[i].initiativeWeight + TOLLERANCE, + votesSumAndInitiativeValues[i].userSum <= votesSumAndInitiativeValues[i].initiativeWeight + TOLLERANCE + ), "initiative voting weights and user's allocated weight match within tollerance" ); } From 6482f511a883056a63d10dba2c6a6218cf217d49 Mon Sep 17 00:00:00 2001 From: gallo Date: Fri, 1 Nov 2024 11:36:19 +0100 Subject: [PATCH 17/57] feat: debug and fix properties --- test/recon/CryticToFoundry.sol | 263 +-------------- test/recon/targets/GovernanceTargets.sol | 4 +- .../trophies/SecondTrophiesToFoundry.sol | 301 ++++++++++++++++++ 3 files changed, 314 insertions(+), 254 deletions(-) create mode 100644 test/recon/trophies/SecondTrophiesToFoundry.sol diff --git a/test/recon/CryticToFoundry.sol b/test/recon/CryticToFoundry.sol index cb0ed348..3823223b 100644 --- a/test/recon/CryticToFoundry.sol +++ b/test/recon/CryticToFoundry.sol @@ -15,43 +15,9 @@ contract CryticToFoundry is Test, TargetFunctions, FoundryAsserts { setup(); } - // forge test --match-test test_property_sum_of_initatives_matches_total_votes_2 -vv - function test_property_sum_of_initatives_matches_total_votes_2() public { - - governance_depositLQTY_2(2); - - vm.warp(block.timestamp + 434544); - - vm.roll(block.number + 1); - - vm.roll(block.number + 1); - vm.warp(block.timestamp + 171499); - governance_allocateLQTY_clamped_single_initiative_2nd_user(0,1,0); - - helper_deployInitiative(); - - governance_depositLQTY(2); - - vm.warp(block.timestamp + 322216); - - vm.roll(block.number + 1); - - governance_registerInitiative(1); - - vm.roll(block.number + 1); - vm.warp(block.timestamp + 449572); - governance_allocateLQTY_clamped_single_initiative(1,75095343,0); - - vm.roll(block.number + 1); - vm.warp(block.timestamp + 436994); - property_sum_of_initatives_matches_total_votes(); - // Of by 1 - // I think this should be off by a bit more than 1 - // But ultimately always less - - } - - // forge test --match-test test_property_sum_of_user_voting_weights_0 -vv + // forge test --match-test test_property_sum_of_user_voting_weights_0 -vv + // NOTE: property_sum_of_user_voting_weights_strict will false + // NOTE: Whereas property_sum_of_user_voting_weights_bounded will not function test_property_sum_of_user_voting_weights_0() public { vm.warp(block.timestamp + 365090); @@ -74,228 +40,21 @@ contract CryticToFoundry is Test, TargetFunctions, FoundryAsserts { governance_allocateLQTY_clamped_single_initiative(0,1,0); - property_sum_of_user_voting_weights_bounded(); /// Of by 2 - - } - - // forge test --match-test test_property_sum_of_lqty_global_user_matches_3 -vv - function test_property_sum_of_lqty_global_user_matches_3() public { - - vm.roll(block.number + 2); - vm.warp(block.timestamp + 45381); - governance_depositLQTY_2(161673733563); - - vm.roll(block.number + 92); - vm.warp(block.timestamp + 156075); - property_BI03(); - - vm.roll(block.number + 305); - vm.warp(block.timestamp + 124202); - property_BI04(); - - vm.roll(block.number + 2); - vm.warp(block.timestamp + 296079); - governance_allocateLQTY_clamped_single_initiative_2nd_user(0,1,0); - - vm.roll(block.number + 4); - vm.warp(block.timestamp + 179667); - helper_deployInitiative(); - - governance_depositLQTY(2718660550802480907); - - vm.roll(block.number + 6); - vm.warp(block.timestamp + 383590); - property_BI07(); - - vm.warp(block.timestamp + 246073); - - vm.roll(block.number + 79); - - vm.roll(block.number + 4); - vm.warp(block.timestamp + 322216); - governance_depositLQTY(1); - - vm.warp(block.timestamp + 472018); - - vm.roll(block.number + 215); - - governance_registerInitiative(1); - - vm.roll(block.number + 1); - vm.warp(block.timestamp + 419805); - governance_allocateLQTY_clamped_single_initiative(1,3700338125821584341973,0); - - vm.warp(block.timestamp + 379004); - - vm.roll(block.number + 112); - - governance_unregisterInitiative(0); - - property_sum_of_lqty_global_user_matches(); - - } - - // forge test --match-test test_governance_claimForInitiativeDoesntRevert_5 -vv - function test_governance_claimForInitiativeDoesntRevert_5() public { - - governance_depositLQTY_2(96505858); - _loginitiative_and_state(); // 0 - - - vm.roll(block.number + 3); - vm.warp(block.timestamp + 191303); - property_BI03(); - _loginitiative_and_state(); // 1 - - vm.warp(block.timestamp + 100782); - - vm.roll(block.number + 1); - - vm.roll(block.number + 1); - vm.warp(block.timestamp + 344203); - governance_allocateLQTY_clamped_single_initiative_2nd_user(0,1,0); - _loginitiative_and_state(); // 2 - - vm.warp(block.timestamp + 348184); - - vm.roll(block.number + 177); - - helper_deployInitiative(); - _loginitiative_and_state(); // 3 - - helper_accrueBold(1000135831883853852074); - _loginitiative_and_state(); // 4 - - governance_depositLQTY(2293362807359); - _loginitiative_and_state(); // 5 - - vm.roll(block.number + 2); - vm.warp(block.timestamp + 151689); - property_BI04(); - _loginitiative_and_state(); // 6 - - governance_registerInitiative(1); - _loginitiative_and_state(); // 7 - property_sum_of_initatives_matches_total_votes(); - - vm.roll(block.number + 3); - vm.warp(block.timestamp + 449572); - governance_allocateLQTY_clamped_single_initiative(1,330671315851182842292,0); - _loginitiative_and_state(); // 8 - property_sum_of_initatives_matches_total_votes(); - - governance_resetAllocations(); // NOTE: This leaves 1 vote from user2, and removes the votes from user1 - _loginitiative_and_state(); // In lack of reset, we have 2 wei error | With reset the math is off by 7x - property_sum_of_initatives_matches_total_votes(); - console.log("time 0", block.timestamp); - - vm.warp(block.timestamp + 231771); - vm.roll(block.number + 5); - _loginitiative_and_state(); - console.log("time 0", block.timestamp); - - // Both of these are fine - // Meaning all LQTY allocation is fine here - // Same for user voting weights - property_sum_of_user_voting_weights_bounded(); - property_sum_of_lqty_global_user_matches(); - - /// === BROKEN === /// - // property_sum_of_initatives_matches_total_votes(); // THIS IS THE BROKEN PROPERTY - (IGovernance.VoteSnapshot memory snapshot,,) = governance.getTotalVotesAndState(); - - uint256 initiativeVotesSum; - for (uint256 i; i < deployedInitiatives.length; i++) { - (IGovernance.InitiativeVoteSnapshot memory initiativeSnapshot,,) = - governance.getInitiativeSnapshotAndState(deployedInitiatives[i]); - (Governance.InitiativeStatus status,,) = governance.getInitiativeState(deployedInitiatives[i]); - - // if (status != Governance.InitiativeStatus.DISABLED) { - // FIX: Only count total if initiative is not disabled - initiativeVotesSum += initiativeSnapshot.votes; - // } - } - console.log("snapshot.votes", snapshot.votes); - console.log("initiativeVotesSum", initiativeVotesSum); - console.log("bold.balance", lusd.balanceOf(address(governance))); - governance_claimForInitiativeDoesntRevert(0); // Because of the quickfix this will not revert anymore - } - -uint256 loggerCount; - function _loginitiative_and_state() internal { - (IGovernance.VoteSnapshot memory snapshot, IGovernance.GlobalState memory state,) = governance.getTotalVotesAndState(); - console.log(""); - console.log("loggerCount", loggerCount++); - console.log("snapshot.votes", snapshot.votes); - - console.log("state.countedVoteLQTY", state.countedVoteLQTY); - console.log("state.countedVoteLQTYAverageTimestamp", state.countedVoteLQTYAverageTimestamp); - - for (uint256 i; i < deployedInitiatives.length; i++) { - (IGovernance.InitiativeVoteSnapshot memory initiativeSnapshot, IGovernance.InitiativeState memory initiativeState,) = - governance.getInitiativeSnapshotAndState(deployedInitiatives[i]); - - console.log("initiativeState.voteLQTY", initiativeState.voteLQTY); - console.log("initiativeState.averageStakingTimestampVoteLQTY", initiativeState.averageStakingTimestampVoteLQTY); - - assertEq(snapshot.forEpoch, initiativeSnapshot.forEpoch, "No desynch"); - console.log("initiativeSnapshot.votes", initiativeSnapshot.votes); - } - } - - // forge test --match-test test_property_BI07_4 -vv - function test_property_BI07_4() public { - - vm.warp(block.timestamp + 562841); - - vm.roll(block.number + 1); - - governance_depositLQTY_2(2); - - vm.warp(block.timestamp + 243877); - - vm.roll(block.number + 1); - - governance_allocateLQTY_clamped_single_initiative_2nd_user(0,1,0); - - vm.warp(block.timestamp + 403427); - - vm.roll(block.number + 1); - - // SHIFTS the week - // Doesn't check latest alloc for each user - // Property is broken due to wrong spec - // For each user you need to grab the latest via the Governance.allocatedByUser - property_resetting_never_reverts(); - - property_BI07(); + property_sum_of_user_voting_weights_bounded(); } - // forge test --match-test test_property_sum_of_user_voting_weights_0 -vv - function test_property_sum_of_user_voting_weights_1() public { - vm.warp(block.timestamp + 365090); +// // forge test --match-test test_property_shouldNeverRevertgetInitiativeState_arbitrary,,)_3 -vv +// function test_property_shouldNeverRevertgetInitiativeState_arbitrary_3() public { - vm.roll(block.number + 1); - governance_depositLQTY_2(3); +// vm.warp(block.timestamp + 606190); - vm.warp(block.timestamp + 164968); +// vm.roll(block.number + 1); - vm.roll(block.number + 1); +// // TODO: I think the snapshout is not sound, so this is ok to revert, didn't spend enough time +// // property_shouldNeverRevertgetInitiativeState_arbitrary(0x3f85D0b6119B38b7E6B119F7550290fec4BE0e3c,(784230180117921576403247836788904270876780620371067576558428, 0); - governance_depositLQTY(2); - - vm.warp(block.timestamp + 74949); - - vm.roll(block.number + 1); - - governance_allocateLQTY_clamped_single_initiative_2nd_user(0,2,0); - - governance_allocateLQTY_clamped_single_initiative(0,1,0); - - property_sum_of_user_voting_weights_bounded(); - - } +// } } diff --git a/test/recon/targets/GovernanceTargets.sol b/test/recon/targets/GovernanceTargets.sol index 6cdcedb2..f2cf7c2a 100644 --- a/test/recon/targets/GovernanceTargets.sol +++ b/test/recon/targets/GovernanceTargets.sol @@ -107,10 +107,10 @@ abstract contract GovernanceTargets is BaseTargetFunctions, Properties { } function depositTsIsRational(uint88 lqtyAmount) public withChecks { - (uint88 user_allocatedLQTY,) = governance.userStates(user); + uint88 stakedAmount = IUserProxy(governance.deriveUserProxyAddress(user)).staked(); // clamp using the user's staked balance // Deposit on zero - if(user_allocatedLQTY == 0) { + if(stakedAmount == 0) { lqtyAmount = uint88(lqtyAmount % lqty.balanceOf(user)); governance.depositLQTY(lqtyAmount); diff --git a/test/recon/trophies/SecondTrophiesToFoundry.sol b/test/recon/trophies/SecondTrophiesToFoundry.sol new file mode 100644 index 00000000..d6c3fa2e --- /dev/null +++ b/test/recon/trophies/SecondTrophiesToFoundry.sol @@ -0,0 +1,301 @@ +// SPDX-License-Identifier: GPL-2.0 +pragma solidity ^0.8.0; + +import {Test} from "forge-std/Test.sol"; +import {TargetFunctions} from "../TargetFunctions.sol"; +import {FoundryAsserts} from "@chimera/FoundryAsserts.sol"; +import {IBribeInitiative} from "src/interfaces/IBribeInitiative.sol"; +import {IGovernance} from "src/interfaces/IGovernance.sol"; +import {Governance} from "src/Governance.sol"; + +import {console} from "forge-std/console.sol"; + +contract SecondTrophiesToFoundry is Test, TargetFunctions, FoundryAsserts { + function setUp() public { + setup(); + } + + // forge test --match-test test_property_sum_of_initatives_matches_total_votes_2 -vv + function test_property_sum_of_initatives_matches_total_votes_2() public { + + governance_depositLQTY_2(2); + + vm.warp(block.timestamp + 434544); + + vm.roll(block.number + 1); + + vm.roll(block.number + 1); + vm.warp(block.timestamp + 171499); + governance_allocateLQTY_clamped_single_initiative_2nd_user(0,1,0); + + helper_deployInitiative(); + + governance_depositLQTY(2); + + vm.warp(block.timestamp + 322216); + + vm.roll(block.number + 1); + + governance_registerInitiative(1); + + vm.roll(block.number + 1); + vm.warp(block.timestamp + 449572); + governance_allocateLQTY_clamped_single_initiative(1,75095343,0); + + vm.roll(block.number + 1); + vm.warp(block.timestamp + 436994); + property_sum_of_initatives_matches_total_votes(); + // Of by 1 + // I think this should be off by a bit more than 1 + // But ultimately always less + + } + + // forge test --match-test test_property_sum_of_user_voting_weights_0 -vv + function test_property_sum_of_user_voting_weights_0() public { + + vm.warp(block.timestamp + 365090); + + vm.roll(block.number + 1); + + governance_depositLQTY_2(3); + + vm.warp(block.timestamp + 164968); + + vm.roll(block.number + 1); + + governance_depositLQTY(2); + + vm.warp(block.timestamp + 74949); + + vm.roll(block.number + 1); + + governance_allocateLQTY_clamped_single_initiative_2nd_user(0,2,0); + + governance_allocateLQTY_clamped_single_initiative(0,1,0); + + property_sum_of_user_voting_weights_bounded(); /// Of by 2 + + } + + // forge test --match-test test_property_sum_of_lqty_global_user_matches_3 -vv + function test_property_sum_of_lqty_global_user_matches_3() public { + + vm.roll(block.number + 2); + vm.warp(block.timestamp + 45381); + governance_depositLQTY_2(161673733563); + + vm.roll(block.number + 92); + vm.warp(block.timestamp + 156075); + property_BI03(); + + vm.roll(block.number + 305); + vm.warp(block.timestamp + 124202); + property_BI04(); + + vm.roll(block.number + 2); + vm.warp(block.timestamp + 296079); + governance_allocateLQTY_clamped_single_initiative_2nd_user(0,1,0); + + vm.roll(block.number + 4); + vm.warp(block.timestamp + 179667); + helper_deployInitiative(); + + governance_depositLQTY(2718660550802480907); + + vm.roll(block.number + 6); + vm.warp(block.timestamp + 383590); + property_BI07(); + + vm.warp(block.timestamp + 246073); + + vm.roll(block.number + 79); + + vm.roll(block.number + 4); + vm.warp(block.timestamp + 322216); + governance_depositLQTY(1); + + vm.warp(block.timestamp + 472018); + + vm.roll(block.number + 215); + + governance_registerInitiative(1); + + vm.roll(block.number + 1); + vm.warp(block.timestamp + 419805); + governance_allocateLQTY_clamped_single_initiative(1,3700338125821584341973,0); + + vm.warp(block.timestamp + 379004); + + vm.roll(block.number + 112); + + governance_unregisterInitiative(0); + + property_sum_of_lqty_global_user_matches(); + + } + + // forge test --match-test test_governance_claimForInitiativeDoesntRevert_5 -vv + function test_governance_claimForInitiativeDoesntRevert_5() public { + + governance_depositLQTY_2(96505858); + _loginitiative_and_state(); // 0 + + + vm.roll(block.number + 3); + vm.warp(block.timestamp + 191303); + property_BI03(); + _loginitiative_and_state(); // 1 + + vm.warp(block.timestamp + 100782); + + vm.roll(block.number + 1); + + vm.roll(block.number + 1); + vm.warp(block.timestamp + 344203); + governance_allocateLQTY_clamped_single_initiative_2nd_user(0,1,0); + _loginitiative_and_state(); // 2 + + vm.warp(block.timestamp + 348184); + + vm.roll(block.number + 177); + + helper_deployInitiative(); + _loginitiative_and_state(); // 3 + + helper_accrueBold(1000135831883853852074); + _loginitiative_and_state(); // 4 + + governance_depositLQTY(2293362807359); + _loginitiative_and_state(); // 5 + + vm.roll(block.number + 2); + vm.warp(block.timestamp + 151689); + property_BI04(); + _loginitiative_and_state(); // 6 + + governance_registerInitiative(1); + _loginitiative_and_state(); // 7 + property_sum_of_initatives_matches_total_votes(); + + vm.roll(block.number + 3); + vm.warp(block.timestamp + 449572); + governance_allocateLQTY_clamped_single_initiative(1,330671315851182842292,0); + _loginitiative_and_state(); // 8 + property_sum_of_initatives_matches_total_votes(); + + governance_resetAllocations(); // NOTE: This leaves 1 vote from user2, and removes the votes from user1 + _loginitiative_and_state(); // In lack of reset, we have 2 wei error | With reset the math is off by 7x + property_sum_of_initatives_matches_total_votes(); + console.log("time 0", block.timestamp); + + vm.warp(block.timestamp + 231771); + vm.roll(block.number + 5); + _loginitiative_and_state(); + console.log("time 0", block.timestamp); + + // Both of these are fine + // Meaning all LQTY allocation is fine here + // Same for user voting weights + property_sum_of_user_voting_weights_bounded(); + property_sum_of_lqty_global_user_matches(); + + /// === BROKEN === /// + // property_sum_of_initatives_matches_total_votes(); // THIS IS THE BROKEN PROPERTY + (IGovernance.VoteSnapshot memory snapshot,,) = governance.getTotalVotesAndState(); + + uint256 initiativeVotesSum; + for (uint256 i; i < deployedInitiatives.length; i++) { + (IGovernance.InitiativeVoteSnapshot memory initiativeSnapshot,,) = + governance.getInitiativeSnapshotAndState(deployedInitiatives[i]); + (Governance.InitiativeStatus status,,) = governance.getInitiativeState(deployedInitiatives[i]); + + // if (status != Governance.InitiativeStatus.DISABLED) { + // FIX: Only count total if initiative is not disabled + initiativeVotesSum += initiativeSnapshot.votes; + // } + } + console.log("snapshot.votes", snapshot.votes); + console.log("initiativeVotesSum", initiativeVotesSum); + console.log("bold.balance", lusd.balanceOf(address(governance))); + governance_claimForInitiativeDoesntRevert(0); // Because of the quickfix this will not revert anymore + } + +uint256 loggerCount; + function _loginitiative_and_state() internal { + (IGovernance.VoteSnapshot memory snapshot, IGovernance.GlobalState memory state,) = governance.getTotalVotesAndState(); + console.log(""); + console.log("loggerCount", loggerCount++); + console.log("snapshot.votes", snapshot.votes); + + console.log("state.countedVoteLQTY", state.countedVoteLQTY); + console.log("state.countedVoteLQTYAverageTimestamp", state.countedVoteLQTYAverageTimestamp); + + for (uint256 i; i < deployedInitiatives.length; i++) { + (IGovernance.InitiativeVoteSnapshot memory initiativeSnapshot, IGovernance.InitiativeState memory initiativeState,) = + governance.getInitiativeSnapshotAndState(deployedInitiatives[i]); + + console.log("initiativeState.voteLQTY", initiativeState.voteLQTY); + console.log("initiativeState.averageStakingTimestampVoteLQTY", initiativeState.averageStakingTimestampVoteLQTY); + + assertEq(snapshot.forEpoch, initiativeSnapshot.forEpoch, "No desynch"); + console.log("initiativeSnapshot.votes", initiativeSnapshot.votes); + } + } + + // forge test --match-test test_property_BI07_4 -vv + function test_property_BI07_4() public { + + vm.warp(block.timestamp + 562841); + + vm.roll(block.number + 1); + + governance_depositLQTY_2(2); + + vm.warp(block.timestamp + 243877); + + vm.roll(block.number + 1); + + governance_allocateLQTY_clamped_single_initiative_2nd_user(0,1,0); + + vm.warp(block.timestamp + 403427); + + vm.roll(block.number + 1); + + // SHIFTS the week + // Doesn't check latest alloc for each user + // Property is broken due to wrong spec + // For each user you need to grab the latest via the Governance.allocatedByUser + property_resetting_never_reverts(); + + property_BI07(); + + } + + // forge test --match-test test_property_sum_of_user_voting_weights_0 -vv + function test_property_sum_of_user_voting_weights_1() public { + + vm.warp(block.timestamp + 365090); + + vm.roll(block.number + 1); + + governance_depositLQTY_2(3); + + vm.warp(block.timestamp + 164968); + + vm.roll(block.number + 1); + + governance_depositLQTY(2); + + vm.warp(block.timestamp + 74949); + + vm.roll(block.number + 1); + + governance_allocateLQTY_clamped_single_initiative_2nd_user(0,2,0); + + governance_allocateLQTY_clamped_single_initiative(0,1,0); + + property_sum_of_user_voting_weights_bounded(); + + } +} From 4db439efd6ecd18f5ed88e69f3e71e0fa6871a3e Mon Sep 17 00:00:00 2001 From: gallo Date: Fri, 1 Nov 2024 12:44:03 +0100 Subject: [PATCH 18/57] fix: BI07 --- test/recon/CryticToFoundry.sol | 39 ++++--------------- .../properties/BribeInitiativeProperties.sol | 5 ++- 2 files changed, 11 insertions(+), 33 deletions(-) diff --git a/test/recon/CryticToFoundry.sol b/test/recon/CryticToFoundry.sol index 3823223b..0ae22d25 100644 --- a/test/recon/CryticToFoundry.sol +++ b/test/recon/CryticToFoundry.sol @@ -14,47 +14,24 @@ contract CryticToFoundry is Test, TargetFunctions, FoundryAsserts { function setUp() public { setup(); } - - // forge test --match-test test_property_sum_of_user_voting_weights_0 -vv - // NOTE: property_sum_of_user_voting_weights_strict will false - // NOTE: Whereas property_sum_of_user_voting_weights_bounded will not - function test_property_sum_of_user_voting_weights_0() public { - - vm.warp(block.timestamp + 365090); +// forge test --match-test test_property_BI07_4 -vv + function test_property_BI07_4() public { vm.roll(block.number + 1); + vm.warp(block.timestamp + 50976); + governance_depositLQTY_2(2); - governance_depositLQTY_2(3); - - vm.warp(block.timestamp + 164968); + vm.warp(block.timestamp + 554137); vm.roll(block.number + 1); - governance_depositLQTY(2); + governance_allocateLQTY_clamped_single_initiative_2nd_user(0,1,0); - vm.warp(block.timestamp + 74949); + vm.warp(block.timestamp + 608747); vm.roll(block.number + 1); - governance_allocateLQTY_clamped_single_initiative_2nd_user(0,2,0); - - governance_allocateLQTY_clamped_single_initiative(0,1,0); - - property_sum_of_user_voting_weights_bounded(); + property_BI07(); } - - -// // forge test --match-test test_property_shouldNeverRevertgetInitiativeState_arbitrary,,)_3 -vv -// function test_property_shouldNeverRevertgetInitiativeState_arbitrary_3() public { - - -// vm.warp(block.timestamp + 606190); - -// vm.roll(block.number + 1); - -// // TODO: I think the snapshout is not sound, so this is ok to revert, didn't spend enough time -// // property_shouldNeverRevertgetInitiativeState_arbitrary(0x3f85D0b6119B38b7E6B119F7550290fec4BE0e3c,(784230180117921576403247836788904270876780620371067576558428, 0); - -// } } diff --git a/test/recon/properties/BribeInitiativeProperties.sol b/test/recon/properties/BribeInitiativeProperties.sol index bdc1e79c..d77757b3 100644 --- a/test/recon/properties/BribeInitiativeProperties.sol +++ b/test/recon/properties/BribeInitiativeProperties.sol @@ -173,12 +173,12 @@ abstract contract BribeInitiativeProperties is BeforeAfter { } function property_BI07() public { - uint16 currentEpoch = governance.epoch(); - // sum user allocations for an epoch // check that this matches the total allocation for the epoch for (uint8 i; i < deployedInitiatives.length; i++) { IBribeInitiative initiative = IBribeInitiative(deployedInitiatives[i]); + uint16 currentEpoch = initiative.getMostRecentTotalEpoch(); + uint88 sumLqtyAllocated; for (uint8 j; j < users.length; j++) { // NOTE: We need to grab user latest @@ -186,6 +186,7 @@ abstract contract BribeInitiativeProperties is BeforeAfter { (uint88 lqtyAllocated,) = initiative.lqtyAllocatedByUserAtEpoch(users[j], userEpoch); sumLqtyAllocated += lqtyAllocated; } + (uint88 totalLQTYAllocated,) = initiative.totalLQTYAllocatedByEpoch(currentEpoch); eq( sumLqtyAllocated, From 723e9e12b2b0db96b42e7a5ffb40dba389e6e666 Mon Sep 17 00:00:00 2001 From: gallo Date: Fri, 1 Nov 2024 13:04:35 +0100 Subject: [PATCH 19/57] feat: truncation but maybe not so bad --- src/Governance.sol | 28 ++- test/recon/CryticToFoundry.sol | 159 +++++++++++++++++- .../recon/properties/GovernanceProperties.sol | 15 +- .../trophies/SecondTrophiesToFoundry.sol | 14 +- 4 files changed, 194 insertions(+), 22 deletions(-) diff --git a/src/Governance.sol b/src/Governance.sol index 649548ae..3b72e6fc 100644 --- a/src/Governance.sol +++ b/src/Governance.sol @@ -18,6 +18,7 @@ import {Multicall} from "./utils/Multicall.sol"; import {WAD, PermitParams} from "./utils/Types.sol"; import {safeCallWithMinGas} from "./utils/SafeCallMinGas.sol"; + /// @title Governance: Modular Initiative based Governance contract Governance is Multicall, UserProxyFactory, ReentrancyGuard, IGovernance { using SafeERC20 for IERC20; @@ -465,11 +466,24 @@ contract Governance is Multicall, UserProxyFactory, ReentrancyGuard, IGovernance // == Rewards Conditions (votes can be zero, logic is the same) == // // By definition if _votesForInitiativeSnapshot.votes > 0 then _votesSnapshot.votes > 0 + + uint256 upscaledInitiativeVotes = uint256(_votesForInitiativeSnapshot.votes); + uint256 upscaledInitiativeVetos = uint256(_votesForInitiativeSnapshot.vetos); + uint256 upscaledTotalVotes = uint256(_votesSnapshot.votes); + if ( - _votesForInitiativeSnapshot.votes > votingTheshold - && !(_votesForInitiativeSnapshot.vetos >= _votesForInitiativeSnapshot.votes) + upscaledInitiativeVotes > votingTheshold + && !(upscaledInitiativeVetos >= upscaledInitiativeVotes) ) { - uint256 claim = _votesForInitiativeSnapshot.votes * boldAccrued / _votesSnapshot.votes; + /// @audit TODO: We need even more precision, damn + /// NOTE: Maybe we truncate this on purpose to increae likelihood that the + // truncation is in favour of system, making insolvency less likely + // TODO: Technically we can use the voting threshold here to make this work + /// With sufficient precision + /// Alternatively, we need to use fullMath on 512 + // NOTE: This MAY help in causing truncation that prevents an edge case + // That causes the redistribution of an excessive amount of rewards + uint256 claim = upscaledInitiativeVotes * 1e10 / upscaledTotalVotes * boldAccrued / 1e10 ; return (InitiativeStatus.CLAIMABLE, lastEpochClaim, claim); } @@ -477,8 +491,8 @@ contract Governance is Multicall, UserProxyFactory, ReentrancyGuard, IGovernance // e.g. if `UNREGISTRATION_AFTER_EPOCHS` is 4, the 4th epoch flip that would result in SKIP, will result in the initiative being `UNREGISTERABLE` if ( (_initiativeState.lastEpochClaim + UNREGISTRATION_AFTER_EPOCHS < epoch() - 1) - || _votesForInitiativeSnapshot.vetos > _votesForInitiativeSnapshot.votes - && _votesForInitiativeSnapshot.vetos > votingTheshold * UNREGISTRATION_THRESHOLD_FACTOR / WAD + || upscaledInitiativeVetos > upscaledInitiativeVotes + && upscaledInitiativeVetos > votingTheshold * UNREGISTRATION_THRESHOLD_FACTOR / WAD ) { return (InitiativeStatus.UNREGISTERABLE, lastEpochClaim, 0); } @@ -501,9 +515,11 @@ contract Governance is Multicall, UserProxyFactory, ReentrancyGuard, IGovernance // an initiative can be registered if the registrant has more voting power (LQTY * age) // than the registration threshold derived from the previous epoch's total global votes + + uint256 upscaledSnapshotVotes = snapshot.votes; require( lqtyToVotes(uint88(stakingV1.stakes(userProxyAddress)), uint120(epochStart()) * uint120(1e18), userState.averageStakingTimestamp) - >= snapshot.votes * REGISTRATION_THRESHOLD_FACTOR / WAD, + >= upscaledSnapshotVotes * REGISTRATION_THRESHOLD_FACTOR / WAD, "Governance: insufficient-lqty" ); diff --git a/test/recon/CryticToFoundry.sol b/test/recon/CryticToFoundry.sol index 0ae22d25..0ce6477c 100644 --- a/test/recon/CryticToFoundry.sol +++ b/test/recon/CryticToFoundry.sol @@ -13,25 +13,170 @@ import {console} from "forge-std/console.sol"; contract CryticToFoundry is Test, TargetFunctions, FoundryAsserts { function setUp() public { setup(); + } -// forge test --match-test test_property_BI07_4 -vv - function test_property_BI07_4() public { + +// forge test --match-test test_property_shouldNeverRevertgetInitiativeState_1 -vv +// TODO: Fixx with full math + function test_property_shouldNeverRevertgetInitiativeState_1() public { + + helper_accrueBold(18728106356049635796899970); + + governance_claimForInitiativeFuzzTest(10); + + vm.roll(block.number + 37678); + vm.warp(block.timestamp + 562841); + property_sum_of_user_initiative_allocations(); + + vm.roll(block.number + 27633); + vm.warp(block.timestamp + 92508); + property_BI04(); + + check_claim_soundness(); + + governance_depositLQTY(16); + + vm.roll(block.number + 16246); + vm.warp(block.timestamp + 128); + governance_claimForInitiativeDoesntRevert(1); + + vm.warp(block.timestamp + 289814); + + vm.roll(block.number + 12073); + + vm.roll(block.number + 39586); + vm.warp(block.timestamp + 84); + governance_depositLQTY_2(27314363929170282673717281); + + property_viewCalculateVotingThreshold(); + + vm.roll(block.number + 2362); + vm.warp(block.timestamp + 126765); + helper_deployInitiative(); + + vm.roll(block.number + 9675); + vm.warp(block.timestamp + 313709); + governance_claimForInitiativeDoesntRevert(13); + + vm.roll(block.number + 51072); + vm.warp(block.timestamp + 322377); + property_BI04(); + + vm.warp(block.timestamp + 907990); + + vm.roll(block.number + 104736); + + governance_depositLQTY(142249495256913202572780803); + + vm.roll(block.number + 33171); + vm.warp(block.timestamp + 69345); + property_BI03(); + + vm.warp(block.timestamp + 89650); + + vm.roll(block.number + 105024); + + governance_registerInitiative(7); + + vm.roll(block.number + 32547); + vm.warp(block.timestamp + 411452); + property_sum_of_votes_in_bribes_match(); + + vm.roll(block.number + 222); + vm.warp(block.timestamp + 18041); + initiative_claimBribes(7741,24,96,231); + + vm.roll(block.number + 213); + vm.warp(block.timestamp + 93910); + property_BI07(); + + property_viewCalculateVotingThreshold(); + + property_sum_of_lqty_global_user_matches(); + + initiative_claimBribes(8279,2983,19203,63); + + governance_allocateLQTY_clamped_single_initiative_2nd_user(177,999999,0); + + check_skip_consistecy(49); + + property_BI08(); + + property_shouldGetTotalVotesAndState(); + + property_GV01(); + + vm.warp(block.timestamp + 266736); + + vm.roll(block.number + 5014); + + vm.roll(block.number + 12823); + vm.warp(block.timestamp + 582973); + check_unregisterable_consistecy(0); + + helper_accrueBold(165945283488494063896927504); + + vm.roll(block.number + 2169); + vm.warp(block.timestamp + 321375); + check_skip_consistecy(6); + + governance_resetAllocations(); + + governance_allocateLQTY_clamped_single_initiative(151,79228162514264337593543950333,0); + + property_shouldNeverRevertgetInitiativeState(74); + + check_skip_consistecy(60); + + vm.roll(block.number + 4440); + vm.warp(block.timestamp + 277592); + property_allocations_are_never_dangerously_high(); + + vm.warp(block.timestamp + 991261); + + vm.roll(block.number + 56784); + + vm.roll(block.number + 16815); + vm.warp(block.timestamp + 321508); + property_shouldNeverRevertgetInitiativeState(9); // TODO: VERY BAD + + } + + // forge test --match-test test_property_sum_of_initatives_matches_total_votes_2 -vv + function test_property_sum_of_initatives_matches_total_votes_2() public { - vm.roll(block.number + 1); - vm.warp(block.timestamp + 50976); governance_depositLQTY_2(2); - vm.warp(block.timestamp + 554137); + vm.warp(block.timestamp + 284887); vm.roll(block.number + 1); + vm.roll(block.number + 1); + vm.warp(block.timestamp + 344203); governance_allocateLQTY_clamped_single_initiative_2nd_user(0,1,0); - vm.warp(block.timestamp + 608747); + helper_deployInitiative(); + + governance_depositLQTY(3); + + vm.warp(block.timestamp + 151205); vm.roll(block.number + 1); - property_BI07(); + governance_registerInitiative(1); + + vm.roll(block.number + 1); + vm.warp(block.timestamp + 449161); + governance_allocateLQTY_clamped_single_initiative(1,1587890,0); + + vm.warp(block.timestamp + 448394); + + vm.roll(block.number + 1); + + vm.roll(block.number + 1); + vm.warp(block.timestamp + 152076); + property_sum_of_initatives_matches_total_votes_bounded(); + property_sum_of_initatives_matches_total_votes_strict(); } } diff --git a/test/recon/properties/GovernanceProperties.sol b/test/recon/properties/GovernanceProperties.sol index c7554f28..01d2f1b5 100644 --- a/test/recon/properties/GovernanceProperties.sol +++ b/test/recon/properties/GovernanceProperties.sol @@ -267,12 +267,23 @@ abstract contract GovernanceProperties is BeforeAfter { } } - function property_sum_of_initatives_matches_total_votes() public { + function property_sum_of_initatives_matches_total_votes_strict() public { // Sum up all initiatives // Compare to total votes (uint256 initiativeVotesSum, uint256 snapshotVotes) = _getInitiativesSnapshotsAndGlobalState(); - eq(initiativeVotesSum, snapshotVotes, "Sum of votes matches"); + eq(initiativeVotesSum, snapshotVotes, "Sum of votes matches Strict"); + } + function property_sum_of_initatives_matches_total_votes_bounded() public { + // Sum up all initiatives + // Compare to total votes + (uint256 initiativeVotesSum, uint256 snapshotVotes) = _getInitiativesSnapshotsAndGlobalState(); + t( + initiativeVotesSum == snapshotVotes || ( + initiativeVotesSum >= snapshotVotes - TOLLERANCE && + initiativeVotesSum <= snapshotVotes + TOLLERANCE + ), + "Sum of votes matches within tollerance"); } function _getInitiativesSnapshotsAndGlobalState() internal returns (uint256, uint256) { diff --git a/test/recon/trophies/SecondTrophiesToFoundry.sol b/test/recon/trophies/SecondTrophiesToFoundry.sol index d6c3fa2e..dd6f3397 100644 --- a/test/recon/trophies/SecondTrophiesToFoundry.sol +++ b/test/recon/trophies/SecondTrophiesToFoundry.sol @@ -15,8 +15,8 @@ contract SecondTrophiesToFoundry is Test, TargetFunctions, FoundryAsserts { setup(); } - // forge test --match-test test_property_sum_of_initatives_matches_total_votes_2 -vv - function test_property_sum_of_initatives_matches_total_votes_2() public { + // forge test --match-test test_property_sum_of_initatives_matches_total_votes_strict_2 -vv + function test_property_sum_of_initatives_matches_total_votes_strict_2() public { governance_depositLQTY_2(2); @@ -44,7 +44,7 @@ contract SecondTrophiesToFoundry is Test, TargetFunctions, FoundryAsserts { vm.roll(block.number + 1); vm.warp(block.timestamp + 436994); - property_sum_of_initatives_matches_total_votes(); + property_sum_of_initatives_matches_total_votes_strict(); // Of by 1 // I think this should be off by a bit more than 1 // But ultimately always less @@ -176,17 +176,17 @@ contract SecondTrophiesToFoundry is Test, TargetFunctions, FoundryAsserts { governance_registerInitiative(1); _loginitiative_and_state(); // 7 - property_sum_of_initatives_matches_total_votes(); + property_sum_of_initatives_matches_total_votes_strict(); vm.roll(block.number + 3); vm.warp(block.timestamp + 449572); governance_allocateLQTY_clamped_single_initiative(1,330671315851182842292,0); _loginitiative_and_state(); // 8 - property_sum_of_initatives_matches_total_votes(); + property_sum_of_initatives_matches_total_votes_strict(); governance_resetAllocations(); // NOTE: This leaves 1 vote from user2, and removes the votes from user1 _loginitiative_and_state(); // In lack of reset, we have 2 wei error | With reset the math is off by 7x - property_sum_of_initatives_matches_total_votes(); + property_sum_of_initatives_matches_total_votes_strict(); console.log("time 0", block.timestamp); vm.warp(block.timestamp + 231771); @@ -201,7 +201,7 @@ contract SecondTrophiesToFoundry is Test, TargetFunctions, FoundryAsserts { property_sum_of_lqty_global_user_matches(); /// === BROKEN === /// - // property_sum_of_initatives_matches_total_votes(); // THIS IS THE BROKEN PROPERTY + // property_sum_of_initatives_matches_total_votes_strict(); // THIS IS THE BROKEN PROPERTY (IGovernance.VoteSnapshot memory snapshot,,) = governance.getTotalVotesAndState(); uint256 initiativeVotesSum; From 90af7dbfed271698a00b5e35b1f4ea6e132ba716 Mon Sep 17 00:00:00 2001 From: gallo Date: Fri, 1 Nov 2024 13:24:19 +0100 Subject: [PATCH 20/57] fix: we need a higher confidence range --- test/recon/properties/GovernanceProperties.sol | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/recon/properties/GovernanceProperties.sol b/test/recon/properties/GovernanceProperties.sol index 01d2f1b5..3a4440fa 100644 --- a/test/recon/properties/GovernanceProperties.sol +++ b/test/recon/properties/GovernanceProperties.sol @@ -10,7 +10,8 @@ import {IUserProxy} from "src/interfaces/IUserProxy.sol"; abstract contract GovernanceProperties is BeforeAfter { - uint256 constant TOLLERANCE = 1e6; + uint256 constant TOLLERANCE = 1e19; // NOTE: 1e18 is 1 second due to upscaling + /// So we accept at most 10 seconds of errors /// A Initiative cannot change in status /// Except for being unregistered @@ -217,6 +218,7 @@ abstract contract GovernanceProperties is BeforeAfter { VotesSumAndInitiativeSum[] memory votesSumAndInitiativeValues = _getUserVotesSumAndInitiativesVotes(); for(uint256 i; i < votesSumAndInitiativeValues.length; i++) { + eq(votesSumAndInitiativeValues[i].userSum, votesSumAndInitiativeValues[i].initiativeWeight, "Matching"); t( votesSumAndInitiativeValues[i].userSum == votesSumAndInitiativeValues[i].initiativeWeight || ( From d5596965f9b911474a3b9d060d070753acc13dfd Mon Sep 17 00:00:00 2001 From: gallo Date: Fri, 1 Nov 2024 13:36:17 +0100 Subject: [PATCH 21/57] feat: raise the precision to 1e26 --- src/Governance.sol | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/Governance.sol b/src/Governance.sol index 3b72e6fc..276e7653 100644 --- a/src/Governance.sol +++ b/src/Governance.sol @@ -119,6 +119,8 @@ contract Governance is Multicall, UserProxyFactory, ReentrancyGuard, IGovernance } } + uint120 TIMESTAMP_PRECISION = 1e26; // 1e18 * 100_000 + function _averageAge(uint120 _currentTimestamp, uint120 _averageTimestamp) internal pure returns (uint120) { if (_averageTimestamp == 0 || _currentTimestamp < _averageTimestamp) return 0; return _currentTimestamp - _averageTimestamp; @@ -135,13 +137,13 @@ contract Governance is Multicall, UserProxyFactory, ReentrancyGuard, IGovernance // NOTE: Truncation // NOTE: u32 -> u120 /// @audit Investigate this - uint120 currentTime = uint120(uint32(block.timestamp)) * uint120(WAD); + uint120 currentTime = uint120(uint32(block.timestamp)) * uint120(TIMESTAMP_PRECISION); uint120 prevOuterAverageAge = _averageAge(currentTime, _prevOuterAverageTimestamp); uint120 newInnerAverageAge = _averageAge(currentTime, _newInnerAverageTimestamp); - // 120 for timestamps - // 208 for voting power + // 120 for timestamps = 2^32 * 1e18 | 2^32 * 1e26 + // 208 for voting power = 2^120 * 2^88 uint120 newOuterAverageAge; if (_prevLQTYBalance <= _newLQTYBalance) { @@ -189,7 +191,7 @@ contract Governance is Multicall, UserProxyFactory, ReentrancyGuard, IGovernance // IMO: Define a shutdown time at which all math is ignored // if TS > u32 -> Just withdraw and don't check userState.averageStakingTimestamp = _calculateAverageTimestamp( - userState.averageStakingTimestamp, uint120(block.timestamp * WAD), lqtyStaked, lqtyStaked + _lqtyAmount + userState.averageStakingTimestamp, uint120(block.timestamp) * uint120(TIMESTAMP_PRECISION), lqtyStaked, lqtyStaked + _lqtyAmount ); userStates[msg.sender] = userState; @@ -329,7 +331,7 @@ contract Governance is Multicall, UserProxyFactory, ReentrancyGuard, IGovernance if (snapshot.forEpoch < currentEpoch - 1) { shouldUpdate = true; - snapshot.votes = lqtyToVotes(state.countedVoteLQTY, uint120(epochStart()) * uint120(1e18), state.countedVoteLQTYAverageTimestamp); + snapshot.votes = lqtyToVotes(state.countedVoteLQTY, uint120(epochStart()) * uint120(TIMESTAMP_PRECISION), state.countedVoteLQTYAverageTimestamp); snapshot.forEpoch = currentEpoch - 1; } } @@ -368,7 +370,7 @@ contract Governance is Multicall, UserProxyFactory, ReentrancyGuard, IGovernance if (initiativeSnapshot.forEpoch < currentEpoch - 1) { shouldUpdate = true; - uint120 start = uint120(epochStart()) * uint120(1e18); + uint120 start = uint120(epochStart()) * uint120(TIMESTAMP_PRECISION); uint240 votes = lqtyToVotes(initiativeState.voteLQTY, start, initiativeState.averageStakingTimestampVoteLQTY); uint240 vetos = @@ -518,7 +520,7 @@ contract Governance is Multicall, UserProxyFactory, ReentrancyGuard, IGovernance uint256 upscaledSnapshotVotes = snapshot.votes; require( - lqtyToVotes(uint88(stakingV1.stakes(userProxyAddress)), uint120(epochStart()) * uint120(1e18), userState.averageStakingTimestamp) + lqtyToVotes(uint88(stakingV1.stakes(userProxyAddress)), uint120(epochStart()) * uint120(TIMESTAMP_PRECISION), userState.averageStakingTimestamp) >= upscaledSnapshotVotes * REGISTRATION_THRESHOLD_FACTOR / WAD, "Governance: insufficient-lqty" ); From 81c048e6a6ff84e11e7d5471d4d7264f35ab91c5 Mon Sep 17 00:00:00 2001 From: gallo Date: Fri, 1 Nov 2024 13:39:29 +0100 Subject: [PATCH 22/57] fix: upscale the precision in bribes --- src/BribeInitiative.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/BribeInitiative.sol b/src/BribeInitiative.sol index 2ee9e909..fc688948 100644 --- a/src/BribeInitiative.sol +++ b/src/BribeInitiative.sol @@ -70,7 +70,7 @@ contract BribeInitiative is IInitiative, IBribeInitiative { bribeToken.safeTransferFrom(msg.sender, address(this), _bribeTokenAmount); } - uint256 constant WAD = 1e18; + uint256 constant TIMESTAMP_PRECISION = 1e26; function _claimBribe( @@ -104,7 +104,7 @@ contract BribeInitiative is IInitiative, IBribeInitiative { (uint88 totalLQTY, uint120 totalAverageTimestamp) = _decodeLQTYAllocation(totalLQTYAllocation.value); // TODO: SCALING!!! - uint120 epochEnd = (uint120(governance.EPOCH_START()) + uint120(_epoch) * uint120(governance.EPOCH_DURATION())) * uint120(WAD); + uint120 epochEnd = (uint120(governance.EPOCH_START()) + uint120(_epoch) * uint120(governance.EPOCH_DURATION())) * uint120(TIMESTAMP_PRECISION); /// @audit User Invariant assert(totalAverageTimestamp <= epochEnd); From 50933d06ec146355dbeca993c49761d51d703527 Mon Sep 17 00:00:00 2001 From: gallo Date: Fri, 1 Nov 2024 13:39:31 +0100 Subject: [PATCH 23/57] chore: fix tests --- test/Governance.t.sol | 70 ++++++++++++------------ test/GovernanceAttacks.t.sol | 2 +- test/VotingPower.t.sol | 2 +- test/recon/targets/GovernanceTargets.sol | 2 +- 4 files changed, 38 insertions(+), 38 deletions(-) diff --git a/test/Governance.t.sol b/test/Governance.t.sol index d26f3f3c..3cd84a74 100644 --- a/test/Governance.t.sol +++ b/test/Governance.t.sol @@ -199,7 +199,7 @@ contract GovernanceTest is Test { (uint88 allocatedLQTY, uint120 averageStakingTimestamp) = governance.userStates(user); assertEq(allocatedLQTY, 0); // first deposit should have an averageStakingTimestamp if block.timestamp - assertEq(averageStakingTimestamp, block.timestamp * 1e18); + assertEq(averageStakingTimestamp, block.timestamp * 1e26); vm.warp(block.timestamp + timeIncrease); @@ -209,7 +209,7 @@ contract GovernanceTest is Test { (allocatedLQTY, averageStakingTimestamp) = governance.userStates(user); assertEq(allocatedLQTY, 0); // subsequent deposits should have a stake weighted average - assertEq(averageStakingTimestamp, (block.timestamp - timeIncrease / 2) * 1e18, "Avg ts"); + assertEq(averageStakingTimestamp, (block.timestamp - timeIncrease / 2) * 1e26, "Avg ts"); // withdraw 0.5 half of LQTY vm.warp(block.timestamp + timeIncrease); @@ -225,14 +225,14 @@ contract GovernanceTest is Test { assertEq(UserProxy(payable(userProxy)).staked(), 1e18); (allocatedLQTY, averageStakingTimestamp) = governance.userStates(user); assertEq(allocatedLQTY, 0); - assertEq(averageStakingTimestamp, ((block.timestamp - timeIncrease) - timeIncrease / 2) * 1e18, "avg ts2"); + assertEq(averageStakingTimestamp, ((block.timestamp - timeIncrease) - timeIncrease / 2) * 1e26, "avg ts2"); // withdraw remaining LQTY governance.withdrawLQTY(1e18); assertEq(UserProxy(payable(userProxy)).staked(), 0); (allocatedLQTY, averageStakingTimestamp) = governance.userStates(user); assertEq(allocatedLQTY, 0); - assertEq(averageStakingTimestamp, ((block.timestamp - timeIncrease) - timeIncrease / 2) * 1e18, "avg ts3"); + assertEq(averageStakingTimestamp, ((block.timestamp - timeIncrease) - timeIncrease / 2) * 1e26, "avg ts3"); vm.stopPrank(); } @@ -303,7 +303,7 @@ contract GovernanceTest is Test { assertEq(UserProxy(payable(userProxy)).staked(), 1e18); (uint88 allocatedLQTY, uint120 averageStakingTimestamp) = governance.userStates(wallet.addr); assertEq(allocatedLQTY, 0); - assertEq(averageStakingTimestamp, block.timestamp * 1e18); + assertEq(averageStakingTimestamp, block.timestamp * 1e26); } function test_claimFromStakingV1() public { @@ -674,7 +674,7 @@ contract GovernanceTest is Test { (uint88 voteLQTY2,,,,) = governance.initiativeStates(baseInitiative2); // Get power at time of vote - uint256 votingPower = governance.lqtyToVotes(voteLQTY1, uint120(block.timestamp) * uint120(1e18), averageStakingTimestampVoteLQTY1); + uint256 votingPower = governance.lqtyToVotes(voteLQTY1, uint120(block.timestamp) * uint120(1e26), averageStakingTimestampVoteLQTY1); assertGt(votingPower, 0, "Non zero power"); /// @audit TODO Fully digest and explain the bug @@ -1057,7 +1057,7 @@ contract GovernanceTest is Test { assertEq(vetoLQTY, 0); // should update the average staking timestamp for the initiative based on the average staking timestamp of the user's // voting and vetoing LQTY - assertEq(averageStakingTimestampVoteLQTY, (block.timestamp - governance.EPOCH_DURATION()) * 1e18); + assertEq(averageStakingTimestampVoteLQTY, (block.timestamp - governance.EPOCH_DURATION()) * 1e26); assertEq(averageStakingTimestampVoteLQTY, averageStakingTimestampUser); assertEq(averageStakingTimestampVetoLQTY, 0); // should remove or add the initiatives voting LQTY from the counter @@ -1091,7 +1091,7 @@ contract GovernanceTest is Test { governance.depositLQTY(1e18); (, uint120 averageAge) = governance.userStates(user2); - assertEq(governance.lqtyToVotes(1e18, uint120(block.timestamp) * uint120(1e18), averageAge), 0); + assertEq(governance.lqtyToVotes(1e18, uint120(block.timestamp) * uint120(1e26), averageAge), 0); deltaLQTYVetos[0] = 1e18; @@ -1110,7 +1110,7 @@ contract GovernanceTest is Test { governance.initiativeStates(baseInitiative1); assertEq(voteLQTY, 2e18); assertEq(vetoLQTY, 0); - assertEq(averageStakingTimestampVoteLQTY, (block.timestamp - governance.EPOCH_DURATION()) * 1e18); + assertEq(averageStakingTimestampVoteLQTY, (block.timestamp - governance.EPOCH_DURATION()) * 1e26); assertGt(averageStakingTimestampVoteLQTY, averageStakingTimestampUser); assertEq(averageStakingTimestampVetoLQTY, 0); @@ -1181,7 +1181,7 @@ contract GovernanceTest is Test { assertEq(vetoLQTY, 0); // should update the average staking timestamp for the initiative based on the average staking timestamp of the user's // voting and vetoing LQTY - assertEq(averageStakingTimestampVoteLQTY, (block.timestamp - governance.EPOCH_DURATION()) * 1e18, "TS"); + assertEq(averageStakingTimestampVoteLQTY, (block.timestamp - governance.EPOCH_DURATION()) * 1e26, "TS"); assertEq(averageStakingTimestampVoteLQTY, averageStakingTimestampUser); assertEq(averageStakingTimestampVetoLQTY, 0); // should remove or add the initiatives voting LQTY from the counter @@ -1215,7 +1215,7 @@ contract GovernanceTest is Test { governance.depositLQTY(1e18); (, uint120 averageAge) = governance.userStates(user2); - assertEq(governance.lqtyToVotes(1e18, uint120(block.timestamp) * uint120(1e18), averageAge), 0); + assertEq(governance.lqtyToVotes(1e18, uint120(block.timestamp) * uint120(1e26), averageAge), 0); deltaLQTYVetos[0] = 1e18; @@ -1234,7 +1234,7 @@ contract GovernanceTest is Test { governance.initiativeStates(baseInitiative1); assertEq(voteLQTY, 2e18); assertEq(vetoLQTY, 0); - assertEq(averageStakingTimestampVoteLQTY, (block.timestamp - governance.EPOCH_DURATION()) * 1e18, "TS 2"); + assertEq(averageStakingTimestampVoteLQTY, (block.timestamp - governance.EPOCH_DURATION()) * 1e26, "TS 2"); assertGt(averageStakingTimestampVoteLQTY, averageStakingTimestampUser); assertEq(averageStakingTimestampVetoLQTY, 0); @@ -1709,11 +1709,11 @@ contract GovernanceTest is Test { _stakeLQTY(user, lqtyAmount); (uint88 allocatedLQTY0, uint120 averageStakingTimestamp0) = governance.userStates(user); - uint240 currentUserPower0 = governance.lqtyToVotes(allocatedLQTY0, uint120(block.timestamp) * uint120(1e18), averageStakingTimestamp0); + uint240 currentUserPower0 = governance.lqtyToVotes(allocatedLQTY0, uint120(block.timestamp) * uint120(1e26), averageStakingTimestamp0); (uint88 voteLQTY0,, uint120 averageStakingTimestampVoteLQTY0,,) = governance.initiativeStates(baseInitiative1); uint240 currentInitiativePower0 = - governance.lqtyToVotes(voteLQTY0, uint120(block.timestamp) * uint120(1e18), averageStakingTimestampVoteLQTY0); + governance.lqtyToVotes(voteLQTY0, uint120(block.timestamp) * uint120(1e26), averageStakingTimestampVoteLQTY0); // (uint224 votes, uint16 forEpoch,,) = governance.votesForInitiativeSnapshot(baseInitiative1); // console2.log("votes0: ", votes); @@ -1726,14 +1726,14 @@ contract GovernanceTest is Test { // check user voting power for the current epoch (uint88 allocatedLQTY1, uint120 averageStakingTimestamp1) = governance.userStates(user); - uint240 currentUserPower1 = governance.lqtyToVotes(allocatedLQTY1, uint120(block.timestamp) * uint120(1e18), averageStakingTimestamp1); + uint240 currentUserPower1 = governance.lqtyToVotes(allocatedLQTY1, uint120(block.timestamp) * uint120(1e26), averageStakingTimestamp1); // user's allocated lqty should immediately increase their voting power assertGt(currentUserPower1, 0, "current user voting power is 0"); // check initiative voting power for the current epoch (uint88 voteLQTY1,, uint120 averageStakingTimestampVoteLQTY1,,) = governance.initiativeStates(baseInitiative1); uint240 currentInitiativePower1 = - governance.lqtyToVotes(voteLQTY1, uint120(block.timestamp) * uint120(1e18), averageStakingTimestampVoteLQTY1); + governance.lqtyToVotes(voteLQTY1, uint120(block.timestamp) * uint120(1e26), averageStakingTimestampVoteLQTY1); assertGt(currentInitiativePower1, 0, "current initiative voting power is 0"); assertEq(currentUserPower1, currentInitiativePower1, "initiative and user voting power should be equal"); @@ -1747,13 +1747,13 @@ contract GovernanceTest is Test { // user voting power should increase over a given chunk of time (uint88 allocatedLQTY2, uint120 averageStakingTimestamp2) = governance.userStates(user); - uint240 currentUserPower2 = governance.lqtyToVotes(allocatedLQTY2, uint120(block.timestamp) * uint120(1e18), averageStakingTimestamp2); + uint240 currentUserPower2 = governance.lqtyToVotes(allocatedLQTY2, uint120(block.timestamp) * uint120(1e26), averageStakingTimestamp2); assertGt(currentUserPower2, currentUserPower1); // initiative voting power should increase over a given chunk of time (uint88 voteLQTY2,, uint120 averageStakingTimestampVoteLQTY2,,) = governance.initiativeStates(baseInitiative1); uint240 currentInitiativePower2 = - governance.lqtyToVotes(voteLQTY2, uint120(block.timestamp) * uint120(1e18), averageStakingTimestampVoteLQTY2); + governance.lqtyToVotes(voteLQTY2, uint120(block.timestamp) * uint120(1e26), averageStakingTimestampVoteLQTY2); assertEq( currentUserPower2, currentInitiativePower2, "user power and initiative power should increase by same amount" ); @@ -1769,12 +1769,12 @@ contract GovernanceTest is Test { // user voting power should increase (uint88 allocatedLQTY3, uint120 averageStakingTimestamp3) = governance.userStates(user); - uint240 currentUserPower3 = governance.lqtyToVotes(allocatedLQTY3, uint120(block.timestamp) * uint120(1e18), averageStakingTimestamp3); + uint240 currentUserPower3 = governance.lqtyToVotes(allocatedLQTY3, uint120(block.timestamp) * uint120(1e26), averageStakingTimestamp3); // votes should match the voting power for the initiative and subsequently the user since they're the only one allocated (uint88 voteLQTY3,, uint120 averageStakingTimestampVoteLQTY3,,) = governance.initiativeStates(baseInitiative1); uint240 currentInitiativePower3 = - governance.lqtyToVotes(voteLQTY3, uint120(block.timestamp) * uint120(1e18), averageStakingTimestampVoteLQTY3); + governance.lqtyToVotes(voteLQTY3, uint120(block.timestamp) * uint120(1e26), averageStakingTimestampVoteLQTY3); // votes should be counted in this epoch (votes, forEpoch,,) = governance.votesForInitiativeSnapshot(baseInitiative1); @@ -1786,11 +1786,11 @@ contract GovernanceTest is Test { governance.snapshotVotesForInitiative(baseInitiative1); (uint88 allocatedLQTY4, uint120 averageStakingTimestamp4) = governance.userStates(user); - uint240 currentUserPower4 = governance.lqtyToVotes(allocatedLQTY4, uint120(block.timestamp) * uint120(1e18), averageStakingTimestamp4); + uint240 currentUserPower4 = governance.lqtyToVotes(allocatedLQTY4, uint120(block.timestamp) * uint120(1e26), averageStakingTimestamp4); (uint88 voteLQTY4,, uint120 averageStakingTimestampVoteLQTY4,,) = governance.initiativeStates(baseInitiative1); uint240 currentInitiativePower4 = - governance.lqtyToVotes(voteLQTY4, uint120(block.timestamp) * uint120(1e18), averageStakingTimestampVoteLQTY4); + governance.lqtyToVotes(voteLQTY4, uint120(block.timestamp) * uint120(1e26), averageStakingTimestampVoteLQTY4); // checking if snapshotting at the end of an epoch increases the voting power (uint224 votes2,,,) = governance.votesForInitiativeSnapshot(baseInitiative1); @@ -1834,13 +1834,13 @@ contract GovernanceTest is Test { // check user voting power before allocation at epoch start (uint88 allocatedLQTY0, uint120 averageStakingTimestamp0) = governance.userStates(user); - uint240 currentUserPower0 = governance.lqtyToVotes(allocatedLQTY0, uint120(block.timestamp) * uint120(1e18), averageStakingTimestamp0); + uint240 currentUserPower0 = governance.lqtyToVotes(allocatedLQTY0, uint120(block.timestamp) * uint120(1e26), averageStakingTimestamp0); assertEq(currentUserPower0, 0, "user has voting power > 0"); // check initiative voting power before allocation at epoch start (uint88 voteLQTY0,, uint120 averageStakingTimestampVoteLQTY0,,) = governance.initiativeStates(baseInitiative1); uint240 currentInitiativePower0 = - governance.lqtyToVotes(voteLQTY0, uint120(block.timestamp) * uint120(1e18), averageStakingTimestampVoteLQTY0); + governance.lqtyToVotes(voteLQTY0, uint120(block.timestamp) * uint120(1e26), averageStakingTimestampVoteLQTY0); assertEq(currentInitiativePower0, 0, "current initiative voting power is > 0"); _allocateLQTY(user, lqtyAmount); @@ -1850,13 +1850,13 @@ contract GovernanceTest is Test { // check user voting power after allocation at epoch end (uint88 allocatedLQTY1, uint120 averageStakingTimestamp1) = governance.userStates(user); - uint240 currentUserPower1 = governance.lqtyToVotes(allocatedLQTY1, uint120(block.timestamp) * uint120(1e18), averageStakingTimestamp1); + uint240 currentUserPower1 = governance.lqtyToVotes(allocatedLQTY1, uint120(block.timestamp) * uint120(1e26), averageStakingTimestamp1); assertGt(currentUserPower1, 0, "user has no voting power after allocation"); // check initiative voting power after allocation at epoch end (uint88 voteLQTY1,, uint120 averageStakingTimestampVoteLQTY1,,) = governance.initiativeStates(baseInitiative1); uint240 currentInitiativePower1 = - governance.lqtyToVotes(voteLQTY1, uint120(block.timestamp) * uint120(1e18), averageStakingTimestampVoteLQTY1); + governance.lqtyToVotes(voteLQTY1, uint120(block.timestamp) * uint120(1e26), averageStakingTimestampVoteLQTY1); assertGt(currentInitiativePower1, 0, "initiative has no voting power after allocation"); // check that user and initiative voting power is equivalent at epoch end @@ -1867,13 +1867,13 @@ contract GovernanceTest is Test { // get user voting power after multiple epochs (uint88 allocatedLQTY2, uint120 averageStakingTimestamp2) = governance.userStates(user); - uint240 currentUserPower2 = governance.lqtyToVotes(allocatedLQTY2, uint120(block.timestamp) * uint120(1e18), averageStakingTimestamp2); + uint240 currentUserPower2 = governance.lqtyToVotes(allocatedLQTY2, uint120(block.timestamp) * uint120(1e26), averageStakingTimestamp2); assertGt(currentUserPower2, currentUserPower1, "user voting power doesn't increase"); // get initiative voting power after multiple epochs (uint88 voteLQTY2,, uint120 averageStakingTimestampVoteLQTY2,,) = governance.initiativeStates(baseInitiative1); uint240 currentInitiativePower2 = - governance.lqtyToVotes(voteLQTY2, uint120(block.timestamp) * uint120(1e18), averageStakingTimestampVoteLQTY2); + governance.lqtyToVotes(voteLQTY2, uint120(block.timestamp) * uint120(1e26), averageStakingTimestampVoteLQTY2); assertGt(currentInitiativePower2, currentInitiativePower1, "initiative voting power doesn't increase"); // check that initiative and user voting always track each other @@ -1917,7 +1917,7 @@ contract GovernanceTest is Test { // get initiative voting power at start of epoch (uint88 voteLQTY0,, uint120 averageStakingTimestampVoteLQTY0,,) = governance.initiativeStates(baseInitiative1); uint240 currentInitiativePower0 = - governance.lqtyToVotes(voteLQTY0, uint120(block.timestamp) * uint120(1e18), averageStakingTimestampVoteLQTY0); + governance.lqtyToVotes(voteLQTY0, uint120(block.timestamp) * uint120(1e26), averageStakingTimestampVoteLQTY0); assertEq(currentInitiativePower0, 0, "initiative voting power is > 0"); _allocateLQTY(user, lqtyAmount); @@ -1930,7 +1930,7 @@ contract GovernanceTest is Test { // get initiative voting power at time of snapshot (uint88 voteLQTY1,, uint120 averageStakingTimestampVoteLQTY1,,) = governance.initiativeStates(baseInitiative1); uint240 currentInitiativePower1 = - governance.lqtyToVotes(voteLQTY1, uint120(block.timestamp) * uint120(1e18), averageStakingTimestampVoteLQTY1); + governance.lqtyToVotes(voteLQTY1, uint120(block.timestamp) * uint120(1e26), averageStakingTimestampVoteLQTY1); assertGt(currentInitiativePower1, 0, "initiative voting power is 0"); uint240 deltaInitiativeVotingPower = currentInitiativePower1 - currentInitiativePower0; @@ -1978,9 +1978,9 @@ contract GovernanceTest is Test { (uint88 voteLQTY0,,) = governance.lqtyAllocatedByUserToInitiative(user, baseInitiative1); (uint88 allocatedLQTY, uint120 averageStakingTimestamp) = governance.userStates(user); uint240 currentInitiativePowerFrom1 = - governance.lqtyToVotes(voteLQTY0, uint120(block.timestamp) * uint120(1e18), averageStakingTimestamp); + governance.lqtyToVotes(voteLQTY0, uint120(block.timestamp) * uint120(1e26), averageStakingTimestamp); uint240 currentInitiativePowerFrom2 = - governance.lqtyToVotes(allocatedLQTY, uint120(block.timestamp) * uint120(1e18), averageStakingTimestamp); + governance.lqtyToVotes(allocatedLQTY, uint120(block.timestamp) * uint120(1e26), averageStakingTimestamp); assertEq( currentInitiativePowerFrom1, @@ -2180,7 +2180,7 @@ contract GovernanceTest is Test { // get initiative voting power at start of epoch (uint88 voteLQTY0,, uint120 averageStakingTimestampVoteLQTY0,,) = governance.initiativeStates(baseInitiative1); uint240 currentInitiativePower0 = - governance.lqtyToVotes(voteLQTY0, uint120(block.timestamp) * uint120(1e18), averageStakingTimestampVoteLQTY0); + governance.lqtyToVotes(voteLQTY0, uint120(block.timestamp) * uint120(1e26), averageStakingTimestampVoteLQTY0); assertEq(currentInitiativePower0, 0, "initiative voting power is > 0"); _allocateLQTY(user, lqtyAmount); @@ -2196,7 +2196,7 @@ contract GovernanceTest is Test { // get initiative voting power at start of epoch (uint88 voteLQTY1,, uint120 averageStakingTimestampVoteLQTY1,,) = governance.initiativeStates(baseInitiative1); uint240 currentInitiativePower1 = - governance.lqtyToVotes(voteLQTY1, uint120(block.timestamp) * uint120(1e18), averageStakingTimestampVoteLQTY1); + governance.lqtyToVotes(voteLQTY1, uint120(block.timestamp) * uint120(1e26), averageStakingTimestampVoteLQTY1); // 4a. votes from snapshotting at begging of epoch (uint224 votes,,,) = governance.votesForInitiativeSnapshot(baseInitiative1); @@ -2448,7 +2448,7 @@ contract GovernanceTest is Test { // voting power for initiative should be the same as votes from snapshot (uint88 voteLQTY,, uint120 averageStakingTimestampVoteLQTY,,) = governance.initiativeStates(baseInitiative1); uint240 currentInitiativePower = - governance.lqtyToVotes(voteLQTY, uint120(block.timestamp) * uint120(1e18), averageStakingTimestampVoteLQTY); + governance.lqtyToVotes(voteLQTY, uint120(block.timestamp) * uint120(1e26), averageStakingTimestampVoteLQTY); // 4. votes should not affect accounting for votes (uint224 votes,,,) = governance.votesForInitiativeSnapshot(baseInitiative1); diff --git a/test/GovernanceAttacks.t.sol b/test/GovernanceAttacks.t.sol index 27b8b9eb..2b54807e 100644 --- a/test/GovernanceAttacks.t.sol +++ b/test/GovernanceAttacks.t.sol @@ -86,7 +86,7 @@ contract GovernanceTest is Test { (uint88 allocatedLQTY, uint120 averageStakingTimestamp) = governance.userStates(user); assertEq(allocatedLQTY, 0); // first deposit should have an averageStakingTimestamp if block.timestamp - assertEq(averageStakingTimestamp, block.timestamp * 1e18); // TODO: Normalize + assertEq(averageStakingTimestamp, block.timestamp * 1e26); // TODO: Normalize vm.stopPrank(); vm.startPrank(lusdHolder); diff --git a/test/VotingPower.t.sol b/test/VotingPower.t.sol index 97d23e0a..8af007e3 100644 --- a/test/VotingPower.t.sol +++ b/test/VotingPower.t.sol @@ -318,7 +318,7 @@ contract VotingPowerTest is Test { console.log("0?"); - uint256 currentMagnifiedTs = uint120(block.timestamp) * uint120(1e18); + uint256 currentMagnifiedTs = uint120(block.timestamp) * uint120(1e26); vm.startPrank(user2); _allocate(address(baseInitiative1), 15, 0); diff --git a/test/recon/targets/GovernanceTargets.sol b/test/recon/targets/GovernanceTargets.sol index f2cf7c2a..62cde00c 100644 --- a/test/recon/targets/GovernanceTargets.sol +++ b/test/recon/targets/GovernanceTargets.sol @@ -116,7 +116,7 @@ abstract contract GovernanceTargets is BaseTargetFunctions, Properties { // assert that user TS is now * WAD (, uint120 ts) = governance.userStates(user); - eq(ts, block.timestamp * 1e18, "User TS is scaled by WAD"); + eq(ts, block.timestamp * 1e26, "User TS is scaled by WAD"); } } function depositMustFailOnNonZeroAlloc(uint88 lqtyAmount) public withChecks { From bd2cc0af52329593d6f0e6e862b6929b8916f271 Mon Sep 17 00:00:00 2001 From: gallo Date: Fri, 1 Nov 2024 16:06:12 +0100 Subject: [PATCH 24/57] feat: trying out overflow mode --- echidna.yaml | 2 +- test/recon/CryticToFoundry.sol | 60 ++++++++++++++++++++++++ test/recon/targets/GovernanceTargets.sol | 5 +- 3 files changed, 63 insertions(+), 4 deletions(-) diff --git a/echidna.yaml b/echidna.yaml index 64d15907..3dfb97ea 100644 --- a/echidna.yaml +++ b/echidna.yaml @@ -1,4 +1,4 @@ -testMode: "optimization" +testMode: "overflow" prefix: "optimize_" coverage: true corpusDir: "echidna" diff --git a/test/recon/CryticToFoundry.sol b/test/recon/CryticToFoundry.sol index 0ce6477c..3500d2d5 100644 --- a/test/recon/CryticToFoundry.sol +++ b/test/recon/CryticToFoundry.sol @@ -179,4 +179,64 @@ contract CryticToFoundry is Test, TargetFunctions, FoundryAsserts { property_sum_of_initatives_matches_total_votes_strict(); } + // forge test --match-test test_governance_allocateLQTY_clamped_single_initiative_0 -vv + function test_governance_allocateLQTY_clamped_single_initiative_0() public { + + vm.warp(block.timestamp + 944858); + + vm.roll(block.number + 5374); + + vm.roll(block.number + 1); + vm.warp(block.timestamp + 1803); + property_sum_of_user_voting_weights_bounded(); + + vm.roll(block.number + 335); + vm.warp(block.timestamp + 359031); + property_BI08(); + + vm.warp(block.timestamp + 586916); + + vm.roll(block.number + 16871); + + vm.roll(block.number + 3); + vm.warp(block.timestamp + 427175); + property_sum_of_lqty_initiative_user_matches(); + + vm.roll(block.number + 1); + vm.warp(block.timestamp + 132521); + property_BI11(); + + vm.warp(block.timestamp + 19680); + + vm.roll(block.number + 3); + + vm.roll(block.number + 8278); + vm.warp(block.timestamp + 322253); + property_shouldNeverRevertgetInitiativeSnapshotAndState(0); + + vm.warp(block.timestamp + 230528); + + vm.roll(block.number + 3414); + + governance_unregisterInitiative(0); + + vm.warp(block.timestamp + 383213); + + vm.roll(block.number + 1); + + helper_deployInitiative(); + + depositTsIsRational(3); + + governance_registerInitiative(1); + + vm.warp(block.timestamp + 221024); + + vm.roll(block.number + 2535); + + governance_allocateLQTY_clamped_single_initiative(1,1164962138833407039120303983,1500); + + governance_allocateLQTY_clamped_single_initiative(0,21,32455529079152273943377283375); + + } } diff --git a/test/recon/targets/GovernanceTargets.sol b/test/recon/targets/GovernanceTargets.sol index 62cde00c..19298be8 100644 --- a/test/recon/targets/GovernanceTargets.sol +++ b/test/recon/targets/GovernanceTargets.sol @@ -55,9 +55,8 @@ abstract contract GovernanceTargets is BaseTargetFunctions, Properties { (uint88 after_global_allocatedLQTY,) = governance.globalState(); if(status == Governance.InitiativeStatus.DISABLED) { - // State allocation must never change - // Whereas for the user it could | TODO - eq(after_global_allocatedLQTY, b4_global_allocatedLQTY, "Same alloc"); + // NOTE: It could be 0 + lte(after_global_allocatedLQTY, b4_global_allocatedLQTY, "Alloc can only be strictly decreasing"); } } From 0572a616bd44d69435c534ed57d260b0b644bcbe Mon Sep 17 00:00:00 2001 From: gallo Date: Fri, 1 Nov 2024 16:39:22 +0100 Subject: [PATCH 25/57] chore: cleanup --- test/recon/CryticToFoundry.sol | 224 --------------------------------- 1 file changed, 224 deletions(-) diff --git a/test/recon/CryticToFoundry.sol b/test/recon/CryticToFoundry.sol index 3500d2d5..ab7d8590 100644 --- a/test/recon/CryticToFoundry.sol +++ b/test/recon/CryticToFoundry.sol @@ -15,228 +15,4 @@ contract CryticToFoundry is Test, TargetFunctions, FoundryAsserts { setup(); } - -// forge test --match-test test_property_shouldNeverRevertgetInitiativeState_1 -vv -// TODO: Fixx with full math - function test_property_shouldNeverRevertgetInitiativeState_1() public { - - helper_accrueBold(18728106356049635796899970); - - governance_claimForInitiativeFuzzTest(10); - - vm.roll(block.number + 37678); - vm.warp(block.timestamp + 562841); - property_sum_of_user_initiative_allocations(); - - vm.roll(block.number + 27633); - vm.warp(block.timestamp + 92508); - property_BI04(); - - check_claim_soundness(); - - governance_depositLQTY(16); - - vm.roll(block.number + 16246); - vm.warp(block.timestamp + 128); - governance_claimForInitiativeDoesntRevert(1); - - vm.warp(block.timestamp + 289814); - - vm.roll(block.number + 12073); - - vm.roll(block.number + 39586); - vm.warp(block.timestamp + 84); - governance_depositLQTY_2(27314363929170282673717281); - - property_viewCalculateVotingThreshold(); - - vm.roll(block.number + 2362); - vm.warp(block.timestamp + 126765); - helper_deployInitiative(); - - vm.roll(block.number + 9675); - vm.warp(block.timestamp + 313709); - governance_claimForInitiativeDoesntRevert(13); - - vm.roll(block.number + 51072); - vm.warp(block.timestamp + 322377); - property_BI04(); - - vm.warp(block.timestamp + 907990); - - vm.roll(block.number + 104736); - - governance_depositLQTY(142249495256913202572780803); - - vm.roll(block.number + 33171); - vm.warp(block.timestamp + 69345); - property_BI03(); - - vm.warp(block.timestamp + 89650); - - vm.roll(block.number + 105024); - - governance_registerInitiative(7); - - vm.roll(block.number + 32547); - vm.warp(block.timestamp + 411452); - property_sum_of_votes_in_bribes_match(); - - vm.roll(block.number + 222); - vm.warp(block.timestamp + 18041); - initiative_claimBribes(7741,24,96,231); - - vm.roll(block.number + 213); - vm.warp(block.timestamp + 93910); - property_BI07(); - - property_viewCalculateVotingThreshold(); - - property_sum_of_lqty_global_user_matches(); - - initiative_claimBribes(8279,2983,19203,63); - - governance_allocateLQTY_clamped_single_initiative_2nd_user(177,999999,0); - - check_skip_consistecy(49); - - property_BI08(); - - property_shouldGetTotalVotesAndState(); - - property_GV01(); - - vm.warp(block.timestamp + 266736); - - vm.roll(block.number + 5014); - - vm.roll(block.number + 12823); - vm.warp(block.timestamp + 582973); - check_unregisterable_consistecy(0); - - helper_accrueBold(165945283488494063896927504); - - vm.roll(block.number + 2169); - vm.warp(block.timestamp + 321375); - check_skip_consistecy(6); - - governance_resetAllocations(); - - governance_allocateLQTY_clamped_single_initiative(151,79228162514264337593543950333,0); - - property_shouldNeverRevertgetInitiativeState(74); - - check_skip_consistecy(60); - - vm.roll(block.number + 4440); - vm.warp(block.timestamp + 277592); - property_allocations_are_never_dangerously_high(); - - vm.warp(block.timestamp + 991261); - - vm.roll(block.number + 56784); - - vm.roll(block.number + 16815); - vm.warp(block.timestamp + 321508); - property_shouldNeverRevertgetInitiativeState(9); // TODO: VERY BAD - - } - - // forge test --match-test test_property_sum_of_initatives_matches_total_votes_2 -vv - function test_property_sum_of_initatives_matches_total_votes_2() public { - - governance_depositLQTY_2(2); - - vm.warp(block.timestamp + 284887); - - vm.roll(block.number + 1); - - vm.roll(block.number + 1); - vm.warp(block.timestamp + 344203); - governance_allocateLQTY_clamped_single_initiative_2nd_user(0,1,0); - - helper_deployInitiative(); - - governance_depositLQTY(3); - - vm.warp(block.timestamp + 151205); - - vm.roll(block.number + 1); - - governance_registerInitiative(1); - - vm.roll(block.number + 1); - vm.warp(block.timestamp + 449161); - governance_allocateLQTY_clamped_single_initiative(1,1587890,0); - - vm.warp(block.timestamp + 448394); - - vm.roll(block.number + 1); - - vm.roll(block.number + 1); - vm.warp(block.timestamp + 152076); - property_sum_of_initatives_matches_total_votes_bounded(); - property_sum_of_initatives_matches_total_votes_strict(); - - } - // forge test --match-test test_governance_allocateLQTY_clamped_single_initiative_0 -vv - function test_governance_allocateLQTY_clamped_single_initiative_0() public { - - vm.warp(block.timestamp + 944858); - - vm.roll(block.number + 5374); - - vm.roll(block.number + 1); - vm.warp(block.timestamp + 1803); - property_sum_of_user_voting_weights_bounded(); - - vm.roll(block.number + 335); - vm.warp(block.timestamp + 359031); - property_BI08(); - - vm.warp(block.timestamp + 586916); - - vm.roll(block.number + 16871); - - vm.roll(block.number + 3); - vm.warp(block.timestamp + 427175); - property_sum_of_lqty_initiative_user_matches(); - - vm.roll(block.number + 1); - vm.warp(block.timestamp + 132521); - property_BI11(); - - vm.warp(block.timestamp + 19680); - - vm.roll(block.number + 3); - - vm.roll(block.number + 8278); - vm.warp(block.timestamp + 322253); - property_shouldNeverRevertgetInitiativeSnapshotAndState(0); - - vm.warp(block.timestamp + 230528); - - vm.roll(block.number + 3414); - - governance_unregisterInitiative(0); - - vm.warp(block.timestamp + 383213); - - vm.roll(block.number + 1); - - helper_deployInitiative(); - - depositTsIsRational(3); - - governance_registerInitiative(1); - - vm.warp(block.timestamp + 221024); - - vm.roll(block.number + 2535); - - governance_allocateLQTY_clamped_single_initiative(1,1164962138833407039120303983,1500); - - governance_allocateLQTY_clamped_single_initiative(0,21,32455529079152273943377283375); - - } } From 9259e267eb087f61ec96ce8a0c7ff0c571bd34a6 Mon Sep 17 00:00:00 2001 From: gallo Date: Fri, 1 Nov 2024 16:39:27 +0100 Subject: [PATCH 26/57] feat: document overflow risks --- src/BribeInitiative.sol | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/src/BribeInitiative.sol b/src/BribeInitiative.sol index fc688948..ed735997 100644 --- a/src/BribeInitiative.sol +++ b/src/BribeInitiative.sol @@ -103,21 +103,20 @@ contract BribeInitiative is IInitiative, IBribeInitiative { (uint88 totalLQTY, uint120 totalAverageTimestamp) = _decodeLQTYAllocation(totalLQTYAllocation.value); - // TODO: SCALING!!! - uint120 epochEnd = (uint120(governance.EPOCH_START()) + uint120(_epoch) * uint120(governance.EPOCH_DURATION())) * uint120(TIMESTAMP_PRECISION); + // NOTE: SCALING!!! | The timestamp will work until type(uint32).max | After which the math will eventually overflow + uint120 scaledEpochEnd = (uint120(governance.EPOCH_START()) + uint120(_epoch) * uint120(governance.EPOCH_DURATION())) * uint120(TIMESTAMP_PRECISION); /// @audit User Invariant - assert(totalAverageTimestamp <= epochEnd); - + assert(totalAverageTimestamp <= scaledEpochEnd); - uint240 totalVotes = governance.lqtyToVotes(totalLQTY, epochEnd, totalAverageTimestamp); + uint240 totalVotes = governance.lqtyToVotes(totalLQTY, scaledEpochEnd, totalAverageTimestamp); if (totalVotes != 0) { (uint88 lqty, uint120 averageTimestamp) = _decodeLQTYAllocation(lqtyAllocation.value); /// @audit Governance Invariant - assert(averageTimestamp <= epochEnd); + assert(averageTimestamp <= scaledEpochEnd); - uint240 votes = governance.lqtyToVotes(lqty, epochEnd, averageTimestamp); + uint240 votes = governance.lqtyToVotes(lqty, scaledEpochEnd, averageTimestamp); boldAmount = uint256(bribe.boldAmount) * uint256(votes) / uint256(totalVotes); bribeTokenAmount = uint256(bribe.bribeTokenAmount) * uint256(votes) / uint256(totalVotes); } @@ -141,6 +140,9 @@ contract BribeInitiative is IInitiative, IBribeInitiative { bribeTokenAmount += bribeTokenAmount_; } + // NOTE: Due to rounding errors in the `averageTimestamp` bribes may slightly overpay compared to what they have allocated + // We cap to the available amount for this reason + // The error should be below 10 LQTY per annum, in the worst case if (boldAmount != 0) { uint256 max = bold.balanceOf(address(this)); if (boldAmount > max) { @@ -148,6 +150,7 @@ contract BribeInitiative is IInitiative, IBribeInitiative { } bold.safeTransfer(msg.sender, boldAmount); } + if (bribeTokenAmount != 0) { uint256 max = bribeToken.balanceOf(address(this)); if (bribeTokenAmount > max) { From ceffe663ffc50607feaba94dea8e539610319dfb Mon Sep 17 00:00:00 2001 From: gallo Date: Fri, 1 Nov 2024 16:50:53 +0100 Subject: [PATCH 27/57] fix: overflow in timestamp math --- src/Governance.sol | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/Governance.sol b/src/Governance.sol index 276e7653..277f7a90 100644 --- a/src/Governance.sol +++ b/src/Governance.sol @@ -77,6 +77,9 @@ contract Governance is Multicall, UserProxyFactory, ReentrancyGuard, IGovernance uint16 constant UNREGISTERED_INITIATIVE = type(uint16).max; + // 100 Million LQTY will be necessary to make the rounding error cause 1 second of loss per operation + uint120 constant TIMESTAMP_PRECISION = 1e26; + constructor( address _lqty, address _lusd, @@ -119,8 +122,6 @@ contract Governance is Multicall, UserProxyFactory, ReentrancyGuard, IGovernance } } - uint120 TIMESTAMP_PRECISION = 1e26; // 1e18 * 100_000 - function _averageAge(uint120 _currentTimestamp, uint120 _averageTimestamp) internal pure returns (uint120) { if (_averageTimestamp == 0 || _currentTimestamp < _averageTimestamp) return 0; return _currentTimestamp - _averageTimestamp; @@ -136,7 +137,8 @@ contract Governance is Multicall, UserProxyFactory, ReentrancyGuard, IGovernance // NOTE: Truncation // NOTE: u32 -> u120 - /// @audit Investigate this + // While we upscale the Timestamp, the system will stop working at type(uint32).max + // Because the rest of the type is used for precision uint120 currentTime = uint120(uint32(block.timestamp)) * uint120(TIMESTAMP_PRECISION); uint120 prevOuterAverageAge = _averageAge(currentTime, _prevOuterAverageTimestamp); @@ -144,20 +146,19 @@ contract Governance is Multicall, UserProxyFactory, ReentrancyGuard, IGovernance // 120 for timestamps = 2^32 * 1e18 | 2^32 * 1e26 // 208 for voting power = 2^120 * 2^88 - - uint120 newOuterAverageAge; + uint256 newOuterAverageAge; if (_prevLQTYBalance <= _newLQTYBalance) { uint88 deltaLQTY = _newLQTYBalance - _prevLQTYBalance; uint208 prevVotes = uint208(_prevLQTYBalance) * uint208(prevOuterAverageAge); uint208 newVotes = uint208(deltaLQTY) * uint208(newInnerAverageAge); uint208 votes = prevVotes + newVotes; - newOuterAverageAge = uint120(votes / uint208(_newLQTYBalance)); + newOuterAverageAge = votes / _newLQTYBalance; } else { uint88 deltaLQTY = _prevLQTYBalance - _newLQTYBalance; uint208 prevVotes = uint208(_prevLQTYBalance) * uint208(prevOuterAverageAge); uint208 newVotes = uint208(deltaLQTY) * uint208(newInnerAverageAge); uint208 votes = (prevVotes >= newVotes) ? prevVotes - newVotes : 0; - newOuterAverageAge = uint120(votes / uint208(_newLQTYBalance)); + newOuterAverageAge = votes / _newLQTYBalance; } if (newOuterAverageAge > currentTime) return 0; From 426de19d8cd6906be56a2aba8f616b63e4f7141e Mon Sep 17 00:00:00 2001 From: gallo Date: Fri, 1 Nov 2024 16:53:02 +0100 Subject: [PATCH 28/57] chore: type consistency --- src/Governance.sol | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Governance.sol b/src/Governance.sol index 277f7a90..17b0b83d 100644 --- a/src/Governance.sol +++ b/src/Governance.sol @@ -146,7 +146,8 @@ contract Governance is Multicall, UserProxyFactory, ReentrancyGuard, IGovernance // 120 for timestamps = 2^32 * 1e18 | 2^32 * 1e26 // 208 for voting power = 2^120 * 2^88 - uint256 newOuterAverageAge; + // NOTE: 208 / X can go past u120! + uint208 newOuterAverageAge; if (_prevLQTYBalance <= _newLQTYBalance) { uint88 deltaLQTY = _newLQTYBalance - _prevLQTYBalance; uint208 prevVotes = uint208(_prevLQTYBalance) * uint208(prevOuterAverageAge); From 3ecb7a236f0c7329b2d6a9d36a97519b8c0a257c Mon Sep 17 00:00:00 2001 From: gallo Date: Fri, 1 Nov 2024 16:55:11 +0100 Subject: [PATCH 29/57] fix: ensure user TS never decreases --- test/recon/targets/GovernanceTargets.sol | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/test/recon/targets/GovernanceTargets.sol b/test/recon/targets/GovernanceTargets.sol index 19298be8..5d57c5b4 100644 --- a/test/recon/targets/GovernanceTargets.sol +++ b/test/recon/targets/GovernanceTargets.sol @@ -116,6 +116,15 @@ abstract contract GovernanceTargets is BaseTargetFunctions, Properties { // assert that user TS is now * WAD (, uint120 ts) = governance.userStates(user); eq(ts, block.timestamp * 1e26, "User TS is scaled by WAD"); + } else { + // Make sure the TS can never bo before itself + (, uint120 ts_b4) = governance.userStates(user); + lqtyAmount = uint88(lqtyAmount % lqty.balanceOf(user)); + governance.depositLQTY(lqtyAmount); + + (, uint120 ts_after) = governance.userStates(user); + + gte(ts_after, ts_b4, "User TS must always increase"); } } function depositMustFailOnNonZeroAlloc(uint88 lqtyAmount) public withChecks { From 26d44143829175dd04f403056f3a91c094913efe Mon Sep 17 00:00:00 2001 From: gallo Date: Fri, 1 Nov 2024 16:56:12 +0100 Subject: [PATCH 30/57] chore: comments on overflow risk --- src/Governance.sol | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Governance.sol b/src/Governance.sol index 17b0b83d..8433d31c 100644 --- a/src/Governance.sol +++ b/src/Governance.sol @@ -147,6 +147,7 @@ contract Governance is Multicall, UserProxyFactory, ReentrancyGuard, IGovernance // 120 for timestamps = 2^32 * 1e18 | 2^32 * 1e26 // 208 for voting power = 2^120 * 2^88 // NOTE: 208 / X can go past u120! + // Therefore we keep `newOuterAverageAge` as u208 uint208 newOuterAverageAge; if (_prevLQTYBalance <= _newLQTYBalance) { uint88 deltaLQTY = _newLQTYBalance - _prevLQTYBalance; From 55b94aa69d097bb6e02d07c83018f4c2d42bbb09 Mon Sep 17 00:00:00 2001 From: gallo Date: Fri, 1 Nov 2024 16:56:23 +0100 Subject: [PATCH 31/57] gas: order of operations --- src/Governance.sol | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Governance.sol b/src/Governance.sol index 8433d31c..63e0540d 100644 --- a/src/Governance.sol +++ b/src/Governance.sol @@ -180,14 +180,14 @@ contract Governance is Multicall, UserProxyFactory, ReentrancyGuard, IGovernance deployUserProxy(); } - UserProxy userProxy = UserProxy(payable(userProxyAddress)); - - uint88 lqtyStaked = uint88(stakingV1.stakes(userProxyAddress)); - UserState memory userState = userStates[msg.sender]; // Assert that we have resetted here require(userState.allocatedLQTY == 0, "Governance: must-be-zero-allocation"); + UserProxy userProxy = UserProxy(payable(userProxyAddress)); + + uint88 lqtyStaked = uint88(stakingV1.stakes(userProxyAddress)); + // update the average staked timestamp for LQTY staked by the user /// @audit TODO: u32 -> u120 From 9548318fbc9a25d97c8ad0b1efec6dde4ec6827b Mon Sep 17 00:00:00 2001 From: gallo Date: Fri, 1 Nov 2024 16:57:00 +0100 Subject: [PATCH 32/57] chore: rename fn --- src/Governance.sol | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Governance.sol b/src/Governance.sol index 63e0540d..e506c2f3 100644 --- a/src/Governance.sol +++ b/src/Governance.sol @@ -171,7 +171,7 @@ contract Governance is Multicall, UserProxyFactory, ReentrancyGuard, IGovernance STAKING //////////////////////////////////////////////////////////////*/ - function _updateUserStakes(uint88 _lqtyAmount) private returns (UserProxy) { + function _updateUserTimestamp(uint88 _lqtyAmount) private returns (UserProxy) { require(_lqtyAmount > 0, "Governance: zero-lqty-amount"); address userProxyAddress = deriveUserProxyAddress(msg.sender); @@ -205,13 +205,13 @@ contract Governance is Multicall, UserProxyFactory, ReentrancyGuard, IGovernance /// @inheritdoc IGovernance function depositLQTY(uint88 _lqtyAmount) external nonReentrant { - UserProxy userProxy = _updateUserStakes(_lqtyAmount); + UserProxy userProxy = _updateUserTimestamp(_lqtyAmount); userProxy.stake(_lqtyAmount, msg.sender); } /// @inheritdoc IGovernance function depositLQTYViaPermit(uint88 _lqtyAmount, PermitParams calldata _permitParams) external nonReentrant { - UserProxy userProxy = _updateUserStakes(_lqtyAmount); + UserProxy userProxy = _updateUserTimestamp(_lqtyAmount); userProxy.stakeViaPermit(_lqtyAmount, msg.sender, _permitParams); } From 0246b329df26e11792a929df14c43341cbe5b01f Mon Sep 17 00:00:00 2001 From: gallo Date: Fri, 1 Nov 2024 16:58:53 +0100 Subject: [PATCH 33/57] chore: order of operations for gas --- src/Governance.sol | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/src/Governance.sol b/src/Governance.sol index e506c2f3..e56af8f9 100644 --- a/src/Governance.sol +++ b/src/Governance.sol @@ -174,25 +174,23 @@ contract Governance is Multicall, UserProxyFactory, ReentrancyGuard, IGovernance function _updateUserTimestamp(uint88 _lqtyAmount) private returns (UserProxy) { require(_lqtyAmount > 0, "Governance: zero-lqty-amount"); + // Assert that we have resetted here + UserState memory userState = userStates[msg.sender]; + require(userState.allocatedLQTY == 0, "Governance: must-be-zero-allocation"); + address userProxyAddress = deriveUserProxyAddress(msg.sender); if (userProxyAddress.code.length == 0) { deployUserProxy(); } - UserState memory userState = userStates[msg.sender]; - // Assert that we have resetted here - require(userState.allocatedLQTY == 0, "Governance: must-be-zero-allocation"); - UserProxy userProxy = UserProxy(payable(userProxyAddress)); uint88 lqtyStaked = uint88(stakingV1.stakes(userProxyAddress)); // update the average staked timestamp for LQTY staked by the user - /// @audit TODO: u32 -> u120 - // IMO: Define a shutdown time at which all math is ignored - // if TS > u32 -> Just withdraw and don't check + // NOTE: Upscale user TS by `TIMESTAMP_PRECISION` userState.averageStakingTimestamp = _calculateAverageTimestamp( userState.averageStakingTimestamp, uint120(block.timestamp) * uint120(TIMESTAMP_PRECISION), lqtyStaked, lqtyStaked + _lqtyAmount ); @@ -217,16 +215,15 @@ contract Governance is Multicall, UserProxyFactory, ReentrancyGuard, IGovernance /// @inheritdoc IGovernance function withdrawLQTY(uint88 _lqtyAmount) external nonReentrant { + // check that user has reset before changing lqty balance + UserState storage userState = userStates[msg.sender]; + require(userState.allocatedLQTY == 0, "Governance: must-allocate-zero"); + UserProxy userProxy = UserProxy(payable(deriveUserProxyAddress(msg.sender))); require(address(userProxy).code.length != 0, "Governance: user-proxy-not-deployed"); uint88 lqtyStaked = uint88(stakingV1.stakes(address(userProxy))); - UserState storage userState = userStates[msg.sender]; - - // check if user has enough unallocated lqty - require(userState.allocatedLQTY == 0, "Governance: must-allocate-zero"); - (uint256 accruedLUSD, uint256 accruedETH) = userProxy.unstake(_lqtyAmount, msg.sender); emit WithdrawLQTY(msg.sender, _lqtyAmount, accruedLUSD, accruedETH); From 320592206a1651932f80e3bbcb9c9d3cb8a76dfe Mon Sep 17 00:00:00 2001 From: gallo Date: Fri, 1 Nov 2024 17:03:04 +0100 Subject: [PATCH 34/57] chore: consistent types --- src/Governance.sol | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/Governance.sol b/src/Governance.sol index e56af8f9..be1f5edd 100644 --- a/src/Governance.sol +++ b/src/Governance.sol @@ -371,12 +371,13 @@ contract Governance is Multicall, UserProxyFactory, ReentrancyGuard, IGovernance shouldUpdate = true; uint120 start = uint120(epochStart()) * uint120(TIMESTAMP_PRECISION); - uint240 votes = + uint208 votes = lqtyToVotes(initiativeState.voteLQTY, start, initiativeState.averageStakingTimestampVoteLQTY); - uint240 vetos = + uint208 vetos = lqtyToVotes(initiativeState.vetoLQTY, start, initiativeState.averageStakingTimestampVetoLQTY); - initiativeSnapshot.votes = uint224(votes); - initiativeSnapshot.vetos = uint224(vetos); + // NOTE: Upscaling to u224 is safe + initiativeSnapshot.votes = votes; + initiativeSnapshot.vetos = vetos; initiativeSnapshot.forEpoch = currentEpoch - 1; } @@ -477,7 +478,7 @@ contract Governance is Multicall, UserProxyFactory, ReentrancyGuard, IGovernance upscaledInitiativeVotes > votingTheshold && !(upscaledInitiativeVetos >= upscaledInitiativeVotes) ) { - /// @audit TODO: We need even more precision, damn + /// @audit TODO: We need even more precision /// NOTE: Maybe we truncate this on purpose to increae likelihood that the // truncation is in favour of system, making insolvency less likely // TODO: Technically we can use the voting threshold here to make this work @@ -485,6 +486,7 @@ contract Governance is Multicall, UserProxyFactory, ReentrancyGuard, IGovernance /// Alternatively, we need to use fullMath on 512 // NOTE: This MAY help in causing truncation that prevents an edge case // That causes the redistribution of an excessive amount of rewards + // TODO: I think we can do a test to prove the precision required here uint256 claim = upscaledInitiativeVotes * 1e10 / upscaledTotalVotes * boldAccrued / 1e10 ; return (InitiativeStatus.CLAIMABLE, lastEpochClaim, claim); } @@ -518,7 +520,7 @@ contract Governance is Multicall, UserProxyFactory, ReentrancyGuard, IGovernance // an initiative can be registered if the registrant has more voting power (LQTY * age) // than the registration threshold derived from the previous epoch's total global votes - uint256 upscaledSnapshotVotes = snapshot.votes; + uint256 upscaledSnapshotVotes = uint256(snapshot.votes); require( lqtyToVotes(uint88(stakingV1.stakes(userProxyAddress)), uint120(epochStart()) * uint120(TIMESTAMP_PRECISION), userState.averageStakingTimestamp) >= upscaledSnapshotVotes * REGISTRATION_THRESHOLD_FACTOR / WAD, From 84fcb2253f3640c48b85cc17a1d31fafac1d2375 Mon Sep 17 00:00:00 2001 From: gallo Date: Fri, 1 Nov 2024 17:25:48 +0100 Subject: [PATCH 35/57] chore: added precision invariant on `getInitiativeState` --- src/Governance.sol | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/Governance.sol b/src/Governance.sol index be1f5edd..e71a8b12 100644 --- a/src/Governance.sol +++ b/src/Governance.sol @@ -478,6 +478,10 @@ contract Governance is Multicall, UserProxyFactory, ReentrancyGuard, IGovernance upscaledInitiativeVotes > votingTheshold && !(upscaledInitiativeVetos >= upscaledInitiativeVotes) ) { + assert(upscaledInitiativeVotes * WAD / VOTING_THRESHOLD_FACTOR > upscaledTotalVotes); + + uint256 CUSTOM_PRECISION = WAD / VOTING_THRESHOLD_FACTOR; + /// @audit TODO: We need even more precision /// NOTE: Maybe we truncate this on purpose to increae likelihood that the // truncation is in favour of system, making insolvency less likely @@ -487,7 +491,9 @@ contract Governance is Multicall, UserProxyFactory, ReentrancyGuard, IGovernance // NOTE: This MAY help in causing truncation that prevents an edge case // That causes the redistribution of an excessive amount of rewards // TODO: I think we can do a test to prove the precision required here - uint256 claim = upscaledInitiativeVotes * 1e10 / upscaledTotalVotes * boldAccrued / 1e10 ; + // TODO: Because of voting theshold being 3% + // I believe that you should be able to upscaled this by just 100 + uint256 claim = upscaledInitiativeVotes * CUSTOM_PRECISION / upscaledTotalVotes * boldAccrued / CUSTOM_PRECISION; return (InitiativeStatus.CLAIMABLE, lastEpochClaim, claim); } From f4b141974e041d56b20b93a11ac5f7df81bfa8ed Mon Sep 17 00:00:00 2001 From: gallo Date: Fri, 1 Nov 2024 17:34:24 +0100 Subject: [PATCH 36/57] chore: custom precision change --- src/Governance.sol | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/src/Governance.sol b/src/Governance.sol index e71a8b12..b89ea9b5 100644 --- a/src/Governance.sol +++ b/src/Governance.sol @@ -480,19 +480,12 @@ contract Governance is Multicall, UserProxyFactory, ReentrancyGuard, IGovernance ) { assert(upscaledInitiativeVotes * WAD / VOTING_THRESHOLD_FACTOR > upscaledTotalVotes); - uint256 CUSTOM_PRECISION = WAD / VOTING_THRESHOLD_FACTOR; - - /// @audit TODO: We need even more precision - /// NOTE: Maybe we truncate this on purpose to increae likelihood that the - // truncation is in favour of system, making insolvency less likely - // TODO: Technically we can use the voting threshold here to make this work - /// With sufficient precision - /// Alternatively, we need to use fullMath on 512 - // NOTE: This MAY help in causing truncation that prevents an edge case - // That causes the redistribution of an excessive amount of rewards - // TODO: I think we can do a test to prove the precision required here - // TODO: Because of voting theshold being 3% - // I believe that you should be able to upscaled this by just 100 + // 34 times when using 0.03e18 -> 33.3 + 1-> 33 + 1 = 34 + uint256 CUSTOM_PRECISION = WAD / VOTING_THRESHOLD_FACTOR + 1; + + /// @audit Because of the updated timestamp, we can run into overflows if we multiply by `boldAccrued` + /// We use `CUSTOM_PRECISION` for this reason, a smaller multiplicative value + /// The change SHOULD be safe because we already check for `threshold` before getting into these lines uint256 claim = upscaledInitiativeVotes * CUSTOM_PRECISION / upscaledTotalVotes * boldAccrued / CUSTOM_PRECISION; return (InitiativeStatus.CLAIMABLE, lastEpochClaim, claim); } From 04d1b161b80ca0fbc9f7a9a4c3aea4bd8d5e14ed Mon Sep 17 00:00:00 2001 From: Entreprenerd Date: Fri, 1 Nov 2024 19:36:39 +0100 Subject: [PATCH 37/57] chore: remove property that doesn't really work --- test/recon/properties/RevertProperties.sol | 37 +++++++++++----------- 1 file changed, 18 insertions(+), 19 deletions(-) diff --git a/test/recon/properties/RevertProperties.sol b/test/recon/properties/RevertProperties.sol index f4619129..624429ed 100644 --- a/test/recon/properties/RevertProperties.sol +++ b/test/recon/properties/RevertProperties.sol @@ -87,23 +87,22 @@ abstract contract RevertProperties is BeforeAfter { } } - // TODO: Consider adding `getInitiativeState` with random params - // Technically not real, but IMO we should make sure it's safe for all params - - function property_shouldNeverRevertgetInitiativeState_arbitrary( - address _initiative, - IGovernance.VoteSnapshot memory _votesSnapshot, - IGovernance.InitiativeVoteSnapshot memory _votesForInitiativeSnapshot, - IGovernance.InitiativeState memory _initiativeState - ) public { - // NOTE: Maybe this can revert due to specific max values - try governance.getInitiativeState( - _initiative, - _votesSnapshot, - _votesForInitiativeSnapshot, - _initiativeState - ) {} catch { - t(false, "should never revert"); - } - } + /// TODO: Consider creating this with somewhat realistic value + /// Arbitrary values can too easily overflow + // function property_shouldNeverRevertgetInitiativeState_arbitrary( + // address _initiative, + // IGovernance.VoteSnapshot memory _votesSnapshot, + // IGovernance.InitiativeVoteSnapshot memory _votesForInitiativeSnapshot, + // IGovernance.InitiativeState memory _initiativeState + // ) public { + // // NOTE: Maybe this can revert due to specific max values + // try governance.getInitiativeState( + // _initiative, + // _votesSnapshot, + // _votesForInitiativeSnapshot, + // _initiativeState + // ) {} catch { + // t(false, "should never revert"); + // } + // } } \ No newline at end of file From 1ea9df59c00b5019ebaad928be78a198553ab234 Mon Sep 17 00:00:00 2001 From: Entreprenerd Date: Fri, 1 Nov 2024 19:36:51 +0100 Subject: [PATCH 38/57] feat: add a few revert tests that can help detect overflows --- foundry.toml | 4 +- src/Governance.sol | 4 +- test/recon/CryticToFoundry.sol | 239 +++++++++++++++++++++++ test/recon/targets/GovernanceTargets.sol | 23 ++- 4 files changed, 265 insertions(+), 5 deletions(-) diff --git a/foundry.toml b/foundry.toml index 5de5c284..c4557777 100644 --- a/foundry.toml +++ b/foundry.toml @@ -1,8 +1,8 @@ [profile.default] # Compilation -solc_version = "0.8.24" +solc_version = "0.8.25" evm_version = "cancun" -optimizer = true +optimizer = false optimizer_runs = 100000 # Testing diff --git a/src/Governance.sol b/src/Governance.sol index b89ea9b5..81645375 100644 --- a/src/Governance.sol +++ b/src/Governance.sol @@ -478,7 +478,9 @@ contract Governance is Multicall, UserProxyFactory, ReentrancyGuard, IGovernance upscaledInitiativeVotes > votingTheshold && !(upscaledInitiativeVetos >= upscaledInitiativeVotes) ) { - assert(upscaledInitiativeVotes * WAD / VOTING_THRESHOLD_FACTOR > upscaledTotalVotes); + /// @audit 2^208 means we only have 2^48 left + /// Therefore we need to scale the value down by 4 orders of magnitude to make it fit + assert(upscaledInitiativeVotes * 1e14 / (VOTING_THRESHOLD_FACTOR / 1e4) > upscaledTotalVotes); // 34 times when using 0.03e18 -> 33.3 + 1-> 33 + 1 = 34 uint256 CUSTOM_PRECISION = WAD / VOTING_THRESHOLD_FACTOR + 1; diff --git a/test/recon/CryticToFoundry.sol b/test/recon/CryticToFoundry.sol index ab7d8590..59012fff 100644 --- a/test/recon/CryticToFoundry.sol +++ b/test/recon/CryticToFoundry.sol @@ -15,4 +15,243 @@ contract CryticToFoundry is Test, TargetFunctions, FoundryAsserts { setup(); } + // forge test --match-test test_governance_allocateLQTY_clamped_single_initiative_0 -vv + function test_governance_allocateLQTY_clamped_single_initiative_0() public { + + depositTsIsRational(2); + + governance_allocateLQTY_clamped_single_initiative(0,1,0); + + } + + // forge test --match-test test_property_shouldNeverRevertgetInitiativeState_0 -vv + function test_property_shouldNeverRevertgetInitiativeState_0() public { + + property_BI05(); + + property_BI05(); + + vm.roll(block.number + 4976); + vm.warp(block.timestamp + 276463); + helper_deployInitiative(); + + property_shouldNeverRevertgetInitiativeState_arbitrary(0x00000000000000000000000000000001fffffffE); + + property_allocations_are_never_dangerously_high(); + + vm.roll(block.number + 41799); + vm.warp(block.timestamp + 492951); + property_shouldGetTotalVotesAndState(); + + vm.roll(block.number + 5984); + vm.warp(block.timestamp + 33); + property_shouldNeverRevertepoch(); + + vm.roll(block.number + 27160); + vm.warp(block.timestamp + 511328); + governance_snapshotVotesForInitiative(0x00000000000000000000000000000002fFffFffD); + + helper_accrueBold(178153731388271698868196367); + + vm.warp(block.timestamp + 555654); + + vm.roll(block.number + 56598); + + vm.roll(block.number + 896); + vm.warp(block.timestamp + 322143); + depositTsIsRational(170179971686533688480210610); + + vm.roll(block.number + 60461); + vm.warp(block.timestamp + 66543); + property_sum_of_votes_in_bribes_match(); + + check_warmup_unregisterable_consistency(201); + + vm.roll(block.number + 16926); + vm.warp(block.timestamp + 466); + governance_resetAllocations(); + + vm.roll(block.number + 159); + vm.warp(block.timestamp + 220265); + governance_resetAllocations(); + + vm.roll(block.number + 5018); + vm.warp(block.timestamp + 135921); + property_viewCalculateVotingThreshold(); + + vm.roll(block.number + 4945); + vm.warp(block.timestamp + 290780); + property_shouldNeverRevertgetTotalVotesAndState(); + + vm.roll(block.number + 39); + vm.warp(block.timestamp + 191304); + helper_accrueBold(1532892064); + + vm.warp(block.timestamp + 543588); + + vm.roll(block.number + 75614); + + vm.roll(block.number + 4996); + vm.warp(block.timestamp + 254414); + governance_depositLQTY_2(102); + + vm.roll(block.number + 4864); + vm.warp(block.timestamp + 409296); + property_BI06(); + + governance_resetAllocations(); + + vm.roll(block.number + 16086); + vm.warp(block.timestamp + 244384); + governance_snapshotVotesForInitiative(0x00000000000000000000000000000002fFffFffD); + + vm.roll(block.number + 7323); + vm.warp(block.timestamp + 209911); + property_BI01(); + + property_sum_of_lqty_global_user_matches(); + + vm.roll(block.number + 30784); + vm.warp(block.timestamp + 178399); + governance_resetAllocations(); + + vm.roll(block.number + 8345); + vm.warp(block.timestamp + 322355); + property_sum_of_user_initiative_allocations(); + + governance_claimForInitiativeFuzzTest(252); + + helper_deployInitiative(); + + vm.roll(block.number + 16572); + vm.warp(block.timestamp + 109857); + governance_claimForInitiativeDoesntRevert(109); + + vm.roll(block.number + 40001); + vm.warp(block.timestamp + 486890); + property_shouldNeverRevertsecondsWithinEpoch(); + + vm.warp(block.timestamp + 262802); + + vm.roll(block.number + 30011); + + vm.roll(block.number + 124); + vm.warp(block.timestamp + 246181); + property_initiative_ts_matches_user_when_non_zero(); + + vm.roll(block.number + 4501); + vm.warp(block.timestamp + 322247); + governance_claimForInitiativeDoesntRevert(11); + + property_sum_of_lqty_initiative_user_matches(); + + vm.warp(block.timestamp + 185598); + + vm.roll(block.number + 20768); + + vm.roll(block.number + 35461); + vm.warp(block.timestamp + 322365); + property_viewCalculateVotingThreshold(); + + vm.roll(block.number + 48869); + vm.warp(block.timestamp + 153540); + helper_deployInitiative(); + + vm.roll(block.number + 22189); + vm.warp(block.timestamp + 110019); + check_skip_consistecy(67); + + vm.roll(block.number + 51482); + vm.warp(block.timestamp + 67312); + property_sum_of_user_voting_weights_bounded(); + + vm.roll(block.number + 891); + vm.warp(block.timestamp + 226151); + property_shouldNeverRevertgetTotalVotesAndState(); + + property_sum_of_user_voting_weights_bounded(); + + vm.roll(block.number + 26151); + vm.warp(block.timestamp + 321509); + property_shouldNeverRevertsecondsWithinEpoch(); + + vm.roll(block.number + 11); + vm.warp(block.timestamp + 273130); + property_BI03(); + + vm.roll(block.number + 56758); + vm.warp(block.timestamp + 517973); + governance_claimForInitiative(10); + + vm.warp(block.timestamp + 50); + + vm.roll(block.number + 2445); + + vm.roll(block.number + 5014); + vm.warp(block.timestamp + 406789); + governance_claimForInitiativeDoesntRevert(199); + + vm.roll(block.number + 50113); + vm.warp(block.timestamp + 541202); + property_sum_of_user_voting_weights_bounded(); + + vm.roll(block.number + 23859); + vm.warp(block.timestamp + 322287); + governance_registerInitiative(69); + + vm.roll(block.number + 22702); + vm.warp(block.timestamp + 221144); + helper_deployInitiative(); + + vm.roll(block.number + 7566); + vm.warp(block.timestamp + 521319); + property_GV_09(); + + governance_depositLQTY(65457397064557007353296555); + + vm.roll(block.number + 9753); + vm.warp(block.timestamp + 321508); + governance_withdrawLQTY_shouldRevertWhenClamped(96161347592613298005890126); + + vm.roll(block.number + 30630); + vm.warp(block.timestamp + 490165); + governance_allocateLQTY_clamped_single_initiative(6,26053304446932650778388682093,0); + + vm.roll(block.number + 40539); + vm.warp(block.timestamp + 449570); + property_sum_of_lqty_global_user_matches(); + + vm.roll(block.number + 59983); + vm.warp(block.timestamp + 562424); + property_shouldNeverRevertepochStart(22); + + vm.warp(block.timestamp + 337670); + + vm.roll(block.number + 47904); + + vm.roll(block.number + 234); + vm.warp(block.timestamp + 361208); + property_user_ts_is_always_greater_than_start(); + + vm.warp(block.timestamp + 1224371); + + vm.roll(block.number + 68410); + + vm.roll(block.number + 14624); + vm.warp(block.timestamp + 32769); + property_global_ts_is_always_greater_than_start(); + + vm.warp(block.timestamp + 604796); + + vm.roll(block.number + 177); + + property_BI08(); + + property_shouldNeverRevertSnapshotAndState(161); + + vm.roll(block.number + 24224); + vm.warp(block.timestamp + 16802); + property_shouldNeverRevertgetInitiativeState(16); + + } } diff --git a/test/recon/targets/GovernanceTargets.sol b/test/recon/targets/GovernanceTargets.sol index 5d57c5b4..52a62f7a 100644 --- a/test/recon/targets/GovernanceTargets.sol +++ b/test/recon/targets/GovernanceTargets.sol @@ -40,7 +40,11 @@ abstract contract GovernanceTargets is BaseTargetFunctions, Properties { (Governance.InitiativeStatus status, ,) = governance.getInitiativeState(initiatives[0]); - governance.allocateLQTY(deployedInitiatives, initiatives, deltaLQTYVotesArray, deltaLQTYVetosArray); + try governance.allocateLQTY(deployedInitiatives, initiatives, deltaLQTYVotesArray, deltaLQTYVetosArray) { + + } catch { + // t(false, "Clamped allocated should not revert"); // TODO: Consider adding overflow check here + } // The test here should be: // If initiative was DISABLED @@ -154,7 +158,7 @@ abstract contract GovernanceTargets is BaseTargetFunctions, Properties { // For every initiative, make ghost values and ensure they match // For all operations, you also need to add the VESTED AMT? - function governance_allocateLQTY(int88[] calldata _deltaLQTYVotes, int88[] calldata _deltaLQTYVetos) + function governance_allocateLQTY(int88[] memory _deltaLQTYVotes, int88[] memory _deltaLQTYVetos) public withChecks { @@ -262,4 +266,19 @@ abstract contract GovernanceTargets is BaseTargetFunctions, Properties { function governance_withdrawLQTY(uint88 _lqtyAmount) public withChecks { governance.withdrawLQTY(_lqtyAmount); } + + function governance_withdrawLQTY_shouldRevertWhenClamped(uint88 _lqtyAmount) public withChecks { + uint88 stakedAmount = IUserProxy(governance.deriveUserProxyAddress(user)).staked(); // clamp using the user's staked balance + + // Ensure we have 0 votes + try governance.resetAllocations(deployedInitiatives, true) {} catch { + t(false, "Should not revert cause OOG is unlikely"); + } + + _lqtyAmount %= stakedAmount + 1; + try governance.withdrawLQTY(_lqtyAmount) { + } catch { + t(false, "Clamped withdraw should not revert"); + } + } } From 7c96bc7a507e7b034e5befb1a22a33f25fc48eea Mon Sep 17 00:00:00 2001 From: Entreprenerd Date: Fri, 1 Nov 2024 19:37:10 +0100 Subject: [PATCH 39/57] chore: undo de-optimizations --- foundry.toml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/foundry.toml b/foundry.toml index c4557777..5de5c284 100644 --- a/foundry.toml +++ b/foundry.toml @@ -1,8 +1,8 @@ [profile.default] # Compilation -solc_version = "0.8.25" +solc_version = "0.8.24" evm_version = "cancun" -optimizer = false +optimizer = true optimizer_runs = 100000 # Testing From f210e449a782ed065a69afa4ec6b9dbcf27223b6 Mon Sep 17 00:00:00 2001 From: Entreprenerd Date: Fri, 1 Nov 2024 19:47:13 +0100 Subject: [PATCH 40/57] chore: recommendation --- src/Governance.sol | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Governance.sol b/src/Governance.sol index 81645375..ce22502b 100644 --- a/src/Governance.sol +++ b/src/Governance.sol @@ -488,6 +488,7 @@ contract Governance is Multicall, UserProxyFactory, ReentrancyGuard, IGovernance /// @audit Because of the updated timestamp, we can run into overflows if we multiply by `boldAccrued` /// We use `CUSTOM_PRECISION` for this reason, a smaller multiplicative value /// The change SHOULD be safe because we already check for `threshold` before getting into these lines + /// As an alternative, this line could be replaced by https://github.com/Uniswap/v3-core/blob/main/contracts/libraries/FullMath.sol uint256 claim = upscaledInitiativeVotes * CUSTOM_PRECISION / upscaledTotalVotes * boldAccrued / CUSTOM_PRECISION; return (InitiativeStatus.CLAIMABLE, lastEpochClaim, claim); } From e553fea7975de95fab5dd0e9e96036d21a5f5aa0 Mon Sep 17 00:00:00 2001 From: Entreprenerd Date: Fri, 1 Nov 2024 19:56:27 +0100 Subject: [PATCH 41/57] chore: cleanup --- test/recon/CryticToFoundry.sol | 239 --------------------------------- 1 file changed, 239 deletions(-) diff --git a/test/recon/CryticToFoundry.sol b/test/recon/CryticToFoundry.sol index 59012fff..ab7d8590 100644 --- a/test/recon/CryticToFoundry.sol +++ b/test/recon/CryticToFoundry.sol @@ -15,243 +15,4 @@ contract CryticToFoundry is Test, TargetFunctions, FoundryAsserts { setup(); } - // forge test --match-test test_governance_allocateLQTY_clamped_single_initiative_0 -vv - function test_governance_allocateLQTY_clamped_single_initiative_0() public { - - depositTsIsRational(2); - - governance_allocateLQTY_clamped_single_initiative(0,1,0); - - } - - // forge test --match-test test_property_shouldNeverRevertgetInitiativeState_0 -vv - function test_property_shouldNeverRevertgetInitiativeState_0() public { - - property_BI05(); - - property_BI05(); - - vm.roll(block.number + 4976); - vm.warp(block.timestamp + 276463); - helper_deployInitiative(); - - property_shouldNeverRevertgetInitiativeState_arbitrary(0x00000000000000000000000000000001fffffffE); - - property_allocations_are_never_dangerously_high(); - - vm.roll(block.number + 41799); - vm.warp(block.timestamp + 492951); - property_shouldGetTotalVotesAndState(); - - vm.roll(block.number + 5984); - vm.warp(block.timestamp + 33); - property_shouldNeverRevertepoch(); - - vm.roll(block.number + 27160); - vm.warp(block.timestamp + 511328); - governance_snapshotVotesForInitiative(0x00000000000000000000000000000002fFffFffD); - - helper_accrueBold(178153731388271698868196367); - - vm.warp(block.timestamp + 555654); - - vm.roll(block.number + 56598); - - vm.roll(block.number + 896); - vm.warp(block.timestamp + 322143); - depositTsIsRational(170179971686533688480210610); - - vm.roll(block.number + 60461); - vm.warp(block.timestamp + 66543); - property_sum_of_votes_in_bribes_match(); - - check_warmup_unregisterable_consistency(201); - - vm.roll(block.number + 16926); - vm.warp(block.timestamp + 466); - governance_resetAllocations(); - - vm.roll(block.number + 159); - vm.warp(block.timestamp + 220265); - governance_resetAllocations(); - - vm.roll(block.number + 5018); - vm.warp(block.timestamp + 135921); - property_viewCalculateVotingThreshold(); - - vm.roll(block.number + 4945); - vm.warp(block.timestamp + 290780); - property_shouldNeverRevertgetTotalVotesAndState(); - - vm.roll(block.number + 39); - vm.warp(block.timestamp + 191304); - helper_accrueBold(1532892064); - - vm.warp(block.timestamp + 543588); - - vm.roll(block.number + 75614); - - vm.roll(block.number + 4996); - vm.warp(block.timestamp + 254414); - governance_depositLQTY_2(102); - - vm.roll(block.number + 4864); - vm.warp(block.timestamp + 409296); - property_BI06(); - - governance_resetAllocations(); - - vm.roll(block.number + 16086); - vm.warp(block.timestamp + 244384); - governance_snapshotVotesForInitiative(0x00000000000000000000000000000002fFffFffD); - - vm.roll(block.number + 7323); - vm.warp(block.timestamp + 209911); - property_BI01(); - - property_sum_of_lqty_global_user_matches(); - - vm.roll(block.number + 30784); - vm.warp(block.timestamp + 178399); - governance_resetAllocations(); - - vm.roll(block.number + 8345); - vm.warp(block.timestamp + 322355); - property_sum_of_user_initiative_allocations(); - - governance_claimForInitiativeFuzzTest(252); - - helper_deployInitiative(); - - vm.roll(block.number + 16572); - vm.warp(block.timestamp + 109857); - governance_claimForInitiativeDoesntRevert(109); - - vm.roll(block.number + 40001); - vm.warp(block.timestamp + 486890); - property_shouldNeverRevertsecondsWithinEpoch(); - - vm.warp(block.timestamp + 262802); - - vm.roll(block.number + 30011); - - vm.roll(block.number + 124); - vm.warp(block.timestamp + 246181); - property_initiative_ts_matches_user_when_non_zero(); - - vm.roll(block.number + 4501); - vm.warp(block.timestamp + 322247); - governance_claimForInitiativeDoesntRevert(11); - - property_sum_of_lqty_initiative_user_matches(); - - vm.warp(block.timestamp + 185598); - - vm.roll(block.number + 20768); - - vm.roll(block.number + 35461); - vm.warp(block.timestamp + 322365); - property_viewCalculateVotingThreshold(); - - vm.roll(block.number + 48869); - vm.warp(block.timestamp + 153540); - helper_deployInitiative(); - - vm.roll(block.number + 22189); - vm.warp(block.timestamp + 110019); - check_skip_consistecy(67); - - vm.roll(block.number + 51482); - vm.warp(block.timestamp + 67312); - property_sum_of_user_voting_weights_bounded(); - - vm.roll(block.number + 891); - vm.warp(block.timestamp + 226151); - property_shouldNeverRevertgetTotalVotesAndState(); - - property_sum_of_user_voting_weights_bounded(); - - vm.roll(block.number + 26151); - vm.warp(block.timestamp + 321509); - property_shouldNeverRevertsecondsWithinEpoch(); - - vm.roll(block.number + 11); - vm.warp(block.timestamp + 273130); - property_BI03(); - - vm.roll(block.number + 56758); - vm.warp(block.timestamp + 517973); - governance_claimForInitiative(10); - - vm.warp(block.timestamp + 50); - - vm.roll(block.number + 2445); - - vm.roll(block.number + 5014); - vm.warp(block.timestamp + 406789); - governance_claimForInitiativeDoesntRevert(199); - - vm.roll(block.number + 50113); - vm.warp(block.timestamp + 541202); - property_sum_of_user_voting_weights_bounded(); - - vm.roll(block.number + 23859); - vm.warp(block.timestamp + 322287); - governance_registerInitiative(69); - - vm.roll(block.number + 22702); - vm.warp(block.timestamp + 221144); - helper_deployInitiative(); - - vm.roll(block.number + 7566); - vm.warp(block.timestamp + 521319); - property_GV_09(); - - governance_depositLQTY(65457397064557007353296555); - - vm.roll(block.number + 9753); - vm.warp(block.timestamp + 321508); - governance_withdrawLQTY_shouldRevertWhenClamped(96161347592613298005890126); - - vm.roll(block.number + 30630); - vm.warp(block.timestamp + 490165); - governance_allocateLQTY_clamped_single_initiative(6,26053304446932650778388682093,0); - - vm.roll(block.number + 40539); - vm.warp(block.timestamp + 449570); - property_sum_of_lqty_global_user_matches(); - - vm.roll(block.number + 59983); - vm.warp(block.timestamp + 562424); - property_shouldNeverRevertepochStart(22); - - vm.warp(block.timestamp + 337670); - - vm.roll(block.number + 47904); - - vm.roll(block.number + 234); - vm.warp(block.timestamp + 361208); - property_user_ts_is_always_greater_than_start(); - - vm.warp(block.timestamp + 1224371); - - vm.roll(block.number + 68410); - - vm.roll(block.number + 14624); - vm.warp(block.timestamp + 32769); - property_global_ts_is_always_greater_than_start(); - - vm.warp(block.timestamp + 604796); - - vm.roll(block.number + 177); - - property_BI08(); - - property_shouldNeverRevertSnapshotAndState(161); - - vm.roll(block.number + 24224); - vm.warp(block.timestamp + 16802); - property_shouldNeverRevertgetInitiativeState(16); - - } } From d76200765eb4802f903028ccd11024fba5cbc19d Mon Sep 17 00:00:00 2001 From: Entreprenerd Date: Sat, 2 Nov 2024 10:21:36 +0100 Subject: [PATCH 42/57] fix: approve for bribes --- test/recon/targets/BribeInitiativeTargets.sol | 21 ++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/test/recon/targets/BribeInitiativeTargets.sol b/test/recon/targets/BribeInitiativeTargets.sol index b9020006..2c99de6e 100644 --- a/test/recon/targets/BribeInitiativeTargets.sol +++ b/test/recon/targets/BribeInitiativeTargets.sol @@ -25,6 +25,9 @@ abstract contract BribeInitiativeTargets is Test, BaseTargetFunctions, Propertie boldAmount = uint128(boldAmount % lusd.balanceOf(user)); bribeTokenAmount = uint128(bribeTokenAmount % lqty.balanceOf(user)); + lusd.approve(address(initiative), boldAmount); + lqty.approve(address(initiative), bribeTokenAmount); + initiative.depositBribe(boldAmount, bribeTokenAmount, epoch); // tracking to check that bribe accounting is always correct @@ -33,6 +36,20 @@ abstract contract BribeInitiativeTargets is Test, BaseTargetFunctions, Propertie ghostBribeByEpoch[address(initiative)][currentEpoch].bribeTokenAmount += bribeTokenAmount; } + function canary_bribeWasThere(uint8 initiativeIndex) public { + uint16 epoch = governance.epoch(); + IBribeInitiative initiative = IBribeInitiative(_getDeployedInitiative(initiativeIndex)); + + + (uint128 boldAmount, uint128 bribeTokenAmount) = initiative.bribeByEpoch(epoch); + t(boldAmount == 0 && bribeTokenAmount == 0, "A bribe was found"); + } + + bool hasClaimedBribes; + function canary_has_claimed() public { + t(!hasClaimedBribes, "has claimed"); + } + function initiative_claimBribes( uint16 epoch, uint16 prevAllocationEpoch, @@ -55,7 +72,9 @@ abstract contract BribeInitiativeTargets is Test, BaseTargetFunctions, Propertie bool alreadyClaimed = initiative.claimedBribeAtEpoch(user, epoch); - try initiative.claimBribes(claimData) {} + try initiative.claimBribes(claimData) { + hasClaimedBribes = true; + } catch { // check if user had a claimable allocation (uint88 lqtyAllocated,) = initiative.lqtyAllocatedByUserAtEpoch(user, prevAllocationEpoch); From 1219a02d565d4a6183bc88090ac2866fd3264eef Mon Sep 17 00:00:00 2001 From: Entreprenerd Date: Sat, 2 Nov 2024 10:54:05 +0100 Subject: [PATCH 43/57] fix: clamped claim --- test/recon/targets/BribeInitiativeTargets.sol | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/test/recon/targets/BribeInitiativeTargets.sol b/test/recon/targets/BribeInitiativeTargets.sol index 2c99de6e..a5427a0a 100644 --- a/test/recon/targets/BribeInitiativeTargets.sol +++ b/test/recon/targets/BribeInitiativeTargets.sol @@ -50,6 +50,19 @@ abstract contract BribeInitiativeTargets is Test, BaseTargetFunctions, Propertie t(!hasClaimedBribes, "has claimed"); } + function clamped_claimBribes(uint8 initiativeIndex) public { + IBribeInitiative initiative = IBribeInitiative(_getDeployedInitiative(initiativeIndex)); + + uint16 userEpoch = initiative.getMostRecentUserEpoch(user); + uint16 stateEpoch = initiative.getMostRecentTotalEpoch(); + initiative_claimBribes( + governance.epoch() - 1, + userEpoch, + stateEpoch, + initiativeIndex + ); + } + function initiative_claimBribes( uint16 epoch, uint16 prevAllocationEpoch, From 77424915cce2513823216514429105ccc7761173 Mon Sep 17 00:00:00 2001 From: Entreprenerd Date: Sat, 2 Nov 2024 11:36:34 +0100 Subject: [PATCH 44/57] chore: cleanup and simplification --- test/recon/Setup.sol | 4 --- .../properties/BribeInitiativeProperties.sol | 27 +++++-------------- test/recon/targets/BribeInitiativeTargets.sol | 12 ++++++--- test/recon/targets/GovernanceTargets.sol | 6 ++--- 4 files changed, 17 insertions(+), 32 deletions(-) diff --git a/test/recon/Setup.sol b/test/recon/Setup.sol index a1e8f604..2cd45582 100644 --- a/test/recon/Setup.sol +++ b/test/recon/Setup.sol @@ -28,10 +28,6 @@ abstract contract Setup is BaseSetup { bool internal claimedTwice; bool internal unableToClaim; - - // initiative => epoch => bribe - mapping(address => mapping(uint16 => IBribeInitiative.Bribe)) internal ghostBribeByEpoch; - uint128 internal constant REGISTRATION_FEE = 1e18; uint128 internal constant REGISTRATION_THRESHOLD_FACTOR = 0.01e18; uint128 internal constant UNREGISTRATION_THRESHOLD_FACTOR = 4e18; diff --git a/test/recon/properties/BribeInitiativeProperties.sol b/test/recon/properties/BribeInitiativeProperties.sol index d77757b3..00e4b02f 100644 --- a/test/recon/properties/BribeInitiativeProperties.sol +++ b/test/recon/properties/BribeInitiativeProperties.sol @@ -102,6 +102,12 @@ abstract contract BribeInitiativeProperties is BeforeAfter { return totalLQTYAllocatedAtEpoch; } + // TODO: Looks pretty wrong and inaccurate + // Loop over the initiative + // Have all users claim all + // See what the result is + // See the dust + // Dust cap check function property_BI05() public { // users can't claim for current epoch so checking for previous uint16 checkEpoch = governance.epoch() - 1; @@ -151,27 +157,6 @@ abstract contract BribeInitiativeProperties is BeforeAfter { } } - function property_BI06() public { - // using ghost tracking for successful bribe deposits - uint16 currentEpoch = governance.epoch(); - - for (uint8 i; i < deployedInitiatives.length; i++) { - address initiative = deployedInitiatives[i]; - IBribeInitiative.Bribe memory bribe = ghostBribeByEpoch[initiative][currentEpoch]; - (uint128 boldAmount, uint128 bribeTokenAmount) = IBribeInitiative(initiative).bribeByEpoch(currentEpoch); - eq( - bribe.boldAmount, - boldAmount, - "BI-06: Accounting for bold amount in bribe for an epoch is always correct" - ); - eq( - bribe.bribeTokenAmount, - bribeTokenAmount, - "BI-06: Accounting for bold amount in bribe for an epoch is always correct" - ); - } - } - function property_BI07() public { // sum user allocations for an epoch // check that this matches the total allocation for the epoch diff --git a/test/recon/targets/BribeInitiativeTargets.sol b/test/recon/targets/BribeInitiativeTargets.sol index a5427a0a..1183e3fc 100644 --- a/test/recon/targets/BribeInitiativeTargets.sol +++ b/test/recon/targets/BribeInitiativeTargets.sol @@ -28,12 +28,16 @@ abstract contract BribeInitiativeTargets is Test, BaseTargetFunctions, Propertie lusd.approve(address(initiative), boldAmount); lqty.approve(address(initiative), bribeTokenAmount); + (uint128 boldAmountB4, uint128 bribeTokenAmountB4) = IBribeInitiative(initiative).bribeByEpoch(epoch); + initiative.depositBribe(boldAmount, bribeTokenAmount, epoch); - // tracking to check that bribe accounting is always correct - uint16 currentEpoch = governance.epoch(); - ghostBribeByEpoch[address(initiative)][currentEpoch].boldAmount += boldAmount; - ghostBribeByEpoch[address(initiative)][currentEpoch].bribeTokenAmount += bribeTokenAmount; + (uint128 boldAmountAfter, uint128 bribeTokenAmountAfter) = IBribeInitiative(initiative).bribeByEpoch(epoch); + + eq(boldAmountB4 + boldAmount, boldAmountAfter, "Bold amount tracking is sound"); + eq(bribeTokenAmountB4 + bribeTokenAmount, bribeTokenAmountAfter, "Bribe amount tracking is sound"); + + } function canary_bribeWasThere(uint8 initiativeIndex) public { diff --git a/test/recon/targets/GovernanceTargets.sol b/test/recon/targets/GovernanceTargets.sol index 52a62f7a..572e07a6 100644 --- a/test/recon/targets/GovernanceTargets.sol +++ b/test/recon/targets/GovernanceTargets.sol @@ -28,9 +28,9 @@ abstract contract GovernanceTargets is BaseTargetFunctions, Properties { address[] memory initiatives = new address[](1); initiatives[0] = _getDeployedInitiative(initiativesIndex); int88[] memory deltaLQTYVotesArray = new int88[](1); - deltaLQTYVotesArray[0] = int88(uint88(deltaLQTYVotes % stakedAmount)); + deltaLQTYVotesArray[0] = int88(uint88(deltaLQTYVotes % (stakedAmount + 1))); int88[] memory deltaLQTYVetosArray = new int88[](1); - deltaLQTYVetosArray[0] = int88(uint88(deltaLQTYVetos % stakedAmount)); + deltaLQTYVetosArray[0] = int88(uint88(deltaLQTYVetos % (stakedAmount + 1))); // User B4 // (uint88 b4_user_allocatedLQTY,) = governance.userStates(user); // TODO @@ -41,7 +41,7 @@ abstract contract GovernanceTargets is BaseTargetFunctions, Properties { try governance.allocateLQTY(deployedInitiatives, initiatives, deltaLQTYVotesArray, deltaLQTYVetosArray) { - + t(deltaLQTYVotesArray[0] == 0 || deltaLQTYVetosArray[0] == 0, "One alloc must be zero"); } catch { // t(false, "Clamped allocated should not revert"); // TODO: Consider adding overflow check here } From 4ded0c77c91bcee6a331d79724c82cf171f3564d Mon Sep 17 00:00:00 2001 From: Entreprenerd Date: Sat, 2 Nov 2024 11:36:42 +0100 Subject: [PATCH 45/57] feat: repro for claim bribe --- test/recon/CryticToFoundry.sol | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/test/recon/CryticToFoundry.sol b/test/recon/CryticToFoundry.sol index ab7d8590..8d9fd644 100644 --- a/test/recon/CryticToFoundry.sol +++ b/test/recon/CryticToFoundry.sol @@ -15,4 +15,29 @@ contract CryticToFoundry is Test, TargetFunctions, FoundryAsserts { setup(); } + +// forge test --match-test test_property_BI05_3 -vv + function test_property_BI05_3() public { + + initiative_depositBribe(107078662,31,2,0); + + vm.warp(block.timestamp + 607467); + + vm.roll(block.number + 1); + + property_BI05(); + + } +// forge test --match-test test_can_claim -vv + function test_can_claim() public { + vm.warp(block.timestamp + governance.EPOCH_DURATION()); + depositTsIsRational(1); + initiative_depositBribe(1,0,3,0); + governance_allocateLQTY_clamped_single_initiative(0, 1, 0); + vm.warp(block.timestamp + governance.EPOCH_DURATION()); // 4 + assertEq(governance.epoch(), 4, "4th epoch"); + initiative_claimBribes(3, 3, 3, 0); + canary_has_claimed(); + } + } From d60587edcf239e811d0c2a4b795770cfac6138f5 Mon Sep 17 00:00:00 2001 From: Entreprenerd Date: Sat, 2 Nov 2024 11:36:51 +0100 Subject: [PATCH 46/57] feat: extra config for running locally --- foundry.toml | 4 ++-- test/recon/CryticTester.sol | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/foundry.toml b/foundry.toml index 5de5c284..c4557777 100644 --- a/foundry.toml +++ b/foundry.toml @@ -1,8 +1,8 @@ [profile.default] # Compilation -solc_version = "0.8.24" +solc_version = "0.8.25" evm_version = "cancun" -optimizer = true +optimizer = false optimizer_runs = 100000 # Testing diff --git a/test/recon/CryticTester.sol b/test/recon/CryticTester.sol index 332659e0..baf1cbda 100644 --- a/test/recon/CryticTester.sol +++ b/test/recon/CryticTester.sol @@ -5,6 +5,7 @@ import {TargetFunctions} from "./TargetFunctions.sol"; import {CryticAsserts} from "@chimera/CryticAsserts.sol"; // echidna . --contract CryticTester --config echidna.yaml +// echidna . --contract CryticTester --config echidna.yaml --format text --test-limit 1000000 --test-mode assertion // medusa fuzz contract CryticTester is TargetFunctions, CryticAsserts { constructor() payable { From 28d6425342f672b111e62f1a9f40098a1d35230e Mon Sep 17 00:00:00 2001 From: Entreprenerd Date: Sat, 2 Nov 2024 11:37:02 +0100 Subject: [PATCH 47/57] chore: undo config changes --- foundry.toml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/foundry.toml b/foundry.toml index c4557777..5de5c284 100644 --- a/foundry.toml +++ b/foundry.toml @@ -1,8 +1,8 @@ [profile.default] # Compilation -solc_version = "0.8.25" +solc_version = "0.8.24" evm_version = "cancun" -optimizer = false +optimizer = true optimizer_runs = 100000 # Testing From d904b260f09cc7c709b694094d1f249430c02654 Mon Sep 17 00:00:00 2001 From: Entreprenerd Date: Sat, 2 Nov 2024 13:05:39 +0100 Subject: [PATCH 48/57] feat: add solvency tests for Governance --- test/recon/CryticToFoundry.sol | 24 +++++++++++ .../recon/properties/GovernanceProperties.sol | 40 +++++++++++++++++++ test/recon/targets/BribeInitiativeTargets.sol | 23 +++++++---- 3 files changed, 79 insertions(+), 8 deletions(-) diff --git a/test/recon/CryticToFoundry.sol b/test/recon/CryticToFoundry.sol index 8d9fd644..7a58a856 100644 --- a/test/recon/CryticToFoundry.sol +++ b/test/recon/CryticToFoundry.sol @@ -15,6 +15,30 @@ contract CryticToFoundry is Test, TargetFunctions, FoundryAsserts { setup(); } + // forge test --match-test test_property_BI11_0 -vv + function test_property_BI11_0() public { + + vm.warp(block.timestamp + 487191); + + vm.roll(block.number + 1); + + vm.roll(block.number + 1); + vm.warp(block.timestamp + 282660); + governance_depositLQTY(1); + + governance_allocateLQTY_clamped_single_initiative(0,1,0); + + initiative_depositBribe(0,1,3,0); + + vm.warp(block.timestamp + 484046); + + vm.roll(block.number + 5445); + + initiative_claimBribes(443,0,0,0); + + property_BI11(); // invalid-prev-lqty-allocation-epoch + + } // forge test --match-test test_property_BI05_3 -vv function test_property_BI05_3() public { diff --git a/test/recon/properties/GovernanceProperties.sol b/test/recon/properties/GovernanceProperties.sol index 3a4440fa..6e2dd77f 100644 --- a/test/recon/properties/GovernanceProperties.sol +++ b/test/recon/properties/GovernanceProperties.sol @@ -385,6 +385,46 @@ abstract contract GovernanceProperties is BeforeAfter { } } + // TODO: Optimization property to show max loss + // TODO: Same identical optimization property for Bribes claiming + /// Should prob change the math to view it in bribes for easier debug + function check_claimable_solvency() public { + // Accrue all initiatives + // Get bold amount + // Sum up the initiatives claimable vs the bold + + // Check if initiative is claimable + // If it is assert the check + uint256 claimableSum; + for (uint256 i; i < deployedInitiatives.length; i++) { + // NOTE: Non view so it accrues state + (Governance.InitiativeStatus status,, uint256 claimableAmount) = governance.getInitiativeState(deployedInitiatives[i]); + + claimableSum += claimableAmount; + } + + // Grab accrued + uint256 boldAccrued = governance.boldAccrued(); + + lte(claimableSum, boldAccrued, "Total Claims are always LT all bold"); + } + + function check_realized_claiming_solvency() public { + uint256 claimableSum; + for (uint256 i; i < deployedInitiatives.length; i++) { + uint256 claimed = governance.claimForInitiative(deployedInitiatives[i]); + + claimableSum += claimed; + } + + // Grab accrued + uint256 boldAccrued = governance.boldAccrued(); + + lte(claimableSum, boldAccrued, "Total Claims are always LT all bold"); + } + + // TODO: Optimization of this to determine max damage, and max insolvency + function _getUserAllocation(address theUser, address initiative) internal view diff --git a/test/recon/targets/BribeInitiativeTargets.sol b/test/recon/targets/BribeInitiativeTargets.sol index 1183e3fc..3c4d8fcb 100644 --- a/test/recon/targets/BribeInitiativeTargets.sol +++ b/test/recon/targets/BribeInitiativeTargets.sol @@ -91,11 +91,22 @@ abstract contract BribeInitiativeTargets is Test, BaseTargetFunctions, Propertie try initiative.claimBribes(claimData) { hasClaimedBribes = true; + + // Claiming at the same epoch is an issue + if (alreadyClaimed) { + // toggle canary that breaks the BI-02 property + claimedTwice = true; + } } catch { - // check if user had a claimable allocation - (uint88 lqtyAllocated,) = initiative.lqtyAllocatedByUserAtEpoch(user, prevAllocationEpoch); - bool claimedBribe = initiative.claimedBribeAtEpoch(user, prevAllocationEpoch); + // NOTE: This is not a full check, but a sufficient check for some cases + /// Specifically we may have to look at the user last epoch + /// And see if we need to port over that balance from then + (uint88 lqtyAllocated,) = initiative.lqtyAllocatedByUserAtEpoch(user, epoch); + bool claimedBribe = initiative.claimedBribeAtEpoch(user, epoch); + if(initiative.getMostRecentTotalEpoch() != prevTotalAllocationEpoch) { + return; // We are in a edge case + } // Check if there are bribes (uint128 boldAmount, uint128 bribeTokenAmount) = initiative.bribeByEpoch(epoch); @@ -110,10 +121,6 @@ abstract contract BribeInitiativeTargets is Test, BaseTargetFunctions, Propertie } } - // check if the bribe was already claimed at the given epoch - if (alreadyClaimed) { - // toggle canary that breaks the BI-02 property - claimedTwice = true; - } + } } From 96de7097af0cf8dede4cb4bbb0804bfecce1a4f3 Mon Sep 17 00:00:00 2001 From: Entreprenerd Date: Sat, 2 Nov 2024 13:08:10 +0100 Subject: [PATCH 49/57] feat: insolvency optimization tests --- .../properties/OptimizationProperties.sol | 40 +++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/test/recon/properties/OptimizationProperties.sol b/test/recon/properties/OptimizationProperties.sol index f7c327a8..c9e062d9 100644 --- a/test/recon/properties/OptimizationProperties.sol +++ b/test/recon/properties/OptimizationProperties.sol @@ -27,6 +27,7 @@ abstract contract OptimizationProperties is GovernanceProperties { return max; } + function optimize_max_sum_of_user_voting_weights_underpaying() public returns (int256) { VotesSumAndInitiativeSum[] memory results = _getUserVotesSumAndInitiativesVotes(); @@ -41,6 +42,45 @@ abstract contract OptimizationProperties is GovernanceProperties { return max; } + + function optimize_max_claim_insolvent() public returns (int256) { + uint256 claimableSum; + for (uint256 i; i < deployedInitiatives.length; i++) { + // NOTE: Non view so it accrues state + (Governance.InitiativeStatus status,, uint256 claimableAmount) = governance.getInitiativeState(deployedInitiatives[i]); + + claimableSum += claimableAmount; + } + + // Grab accrued + uint256 boldAccrued = governance.boldAccrued(); + + int256 max; + if(claimableSum > boldAccrued) { + max = int256(claimableSum) - int256(boldAccrued); + } + + return max; + } + function optimize_max_claim_underpay() public returns (int256) { + uint256 claimableSum; + for (uint256 i; i < deployedInitiatives.length; i++) { + // NOTE: Non view so it accrues state + (Governance.InitiativeStatus status,, uint256 claimableAmount) = governance.getInitiativeState(deployedInitiatives[i]); + + claimableSum += claimableAmount; + } + + // Grab accrued + uint256 boldAccrued = governance.boldAccrued(); + + int256 max; + if(boldAccrued > claimableSum) { + max = int256(boldAccrued) - int256(claimableSum); + } + + return max; + } function optimize_property_sum_of_lqty_global_user_matches_insolvency() public returns (int256) { From 1f0b3109fe3b61f4a07ae3de0c811bd91f85f302 Mon Sep 17 00:00:00 2001 From: Entreprenerd Date: Sat, 2 Nov 2024 13:43:21 +0100 Subject: [PATCH 50/57] chore: improved coverage and removed canaries --- test/recon/CryticToFoundry.sol | 60 +++++++++---------- .../recon/properties/GovernanceProperties.sol | 2 + test/recon/targets/BribeInitiativeTargets.sol | 22 +++---- 3 files changed, 41 insertions(+), 43 deletions(-) diff --git a/test/recon/CryticToFoundry.sol b/test/recon/CryticToFoundry.sol index 7a58a856..de022150 100644 --- a/test/recon/CryticToFoundry.sol +++ b/test/recon/CryticToFoundry.sol @@ -15,53 +15,49 @@ contract CryticToFoundry is Test, TargetFunctions, FoundryAsserts { setup(); } - // forge test --match-test test_property_BI11_0 -vv - function test_property_BI11_0() public { - vm.warp(block.timestamp + 487191); - - vm.roll(block.number + 1); + // forge test --match-test test_property_BI05_3 -vv + function test_property_BI05_3() public { - vm.roll(block.number + 1); - vm.warp(block.timestamp + 282660); - governance_depositLQTY(1); + initiative_depositBribe(107078662,31,2,0); - governance_allocateLQTY_clamped_single_initiative(0,1,0); + property_BI05(); - initiative_depositBribe(0,1,3,0); + } + // forge test --match-test test_property_sum_of_initatives_matches_total_votes_bounded_0 -vv + function test_property_sum_of_initatives_matches_total_votes_bounded_0() public { - vm.warp(block.timestamp + 484046); + vm.warp(block.timestamp + 576345); - vm.roll(block.number + 5445); + vm.roll(block.number + 1); - initiative_claimBribes(443,0,0,0); + governance_depositLQTY(2); - property_BI11(); // invalid-prev-lqty-allocation-epoch + vm.roll(block.number + 1); + vm.warp(block.timestamp + 41489); + governance_allocateLQTY_clamped_single_initiative(0,1,0); - } + governance_depositLQTY_2(3); -// forge test --match-test test_property_BI05_3 -vv - function test_property_BI05_3() public { + vm.warp(block.timestamp + 455649); - initiative_depositBribe(107078662,31,2,0); + vm.roll(block.number + 1); - vm.warp(block.timestamp + 607467); + governance_allocateLQTY_clamped_single_initiative_2nd_user(0,0,2); vm.roll(block.number + 1); + vm.warp(block.timestamp + 136514); + governance_unregisterInitiative(0); +/** + // TODO: This property is broken, because if a snapshot was taken before the initiative was unregistered + /// Then the votes would still be part of the total state + */ + + + (uint256 initiativeVotesSum, uint256 snapshotVotes) = _getInitiativesSnapshotsAndGlobalState(); + console.log("initiativeVotesSum", initiativeVotesSum); + console.log("snapshotVotes", snapshotVotes); - property_BI05(); - - } -// forge test --match-test test_can_claim -vv - function test_can_claim() public { - vm.warp(block.timestamp + governance.EPOCH_DURATION()); - depositTsIsRational(1); - initiative_depositBribe(1,0,3,0); - governance_allocateLQTY_clamped_single_initiative(0, 1, 0); - vm.warp(block.timestamp + governance.EPOCH_DURATION()); // 4 - assertEq(governance.epoch(), 4, "4th epoch"); - initiative_claimBribes(3, 3, 3, 0); - canary_has_claimed(); } } diff --git a/test/recon/properties/GovernanceProperties.sol b/test/recon/properties/GovernanceProperties.sol index 6e2dd77f..133d6427 100644 --- a/test/recon/properties/GovernanceProperties.sol +++ b/test/recon/properties/GovernanceProperties.sol @@ -297,6 +297,8 @@ abstract contract GovernanceProperties is BeforeAfter { governance.getInitiativeSnapshotAndState(deployedInitiatives[i]); (Governance.InitiativeStatus status,,) = governance.getInitiativeState(deployedInitiatives[i]); + // TODO: This property is broken, because if a snapshot was taken before the initiative was unregistered + /// Then the votes would still be part of the total state if (status != Governance.InitiativeStatus.DISABLED) { // FIX: Only count total if initiative is not disabled initiativeVotesSum += initiativeSnapshot.votes; diff --git a/test/recon/targets/BribeInitiativeTargets.sol b/test/recon/targets/BribeInitiativeTargets.sol index 3c4d8fcb..3718317a 100644 --- a/test/recon/targets/BribeInitiativeTargets.sol +++ b/test/recon/targets/BribeInitiativeTargets.sol @@ -40,19 +40,20 @@ abstract contract BribeInitiativeTargets is Test, BaseTargetFunctions, Propertie } - function canary_bribeWasThere(uint8 initiativeIndex) public { - uint16 epoch = governance.epoch(); - IBribeInitiative initiative = IBribeInitiative(_getDeployedInitiative(initiativeIndex)); + // Canaries are no longer necessary + // function canary_bribeWasThere(uint8 initiativeIndex) public { + // uint16 epoch = governance.epoch(); + // IBribeInitiative initiative = IBribeInitiative(_getDeployedInitiative(initiativeIndex)); - (uint128 boldAmount, uint128 bribeTokenAmount) = initiative.bribeByEpoch(epoch); - t(boldAmount == 0 && bribeTokenAmount == 0, "A bribe was found"); - } + // (uint128 boldAmount, uint128 bribeTokenAmount) = initiative.bribeByEpoch(epoch); + // t(boldAmount == 0 && bribeTokenAmount == 0, "A bribe was found"); + // } - bool hasClaimedBribes; - function canary_has_claimed() public { - t(!hasClaimedBribes, "has claimed"); - } + // bool hasClaimedBribes; + // function canary_has_claimed() public { + // t(!hasClaimedBribes, "has claimed"); + // } function clamped_claimBribes(uint8 initiativeIndex) public { IBribeInitiative initiative = IBribeInitiative(_getDeployedInitiative(initiativeIndex)); @@ -90,7 +91,6 @@ abstract contract BribeInitiativeTargets is Test, BaseTargetFunctions, Propertie bool alreadyClaimed = initiative.claimedBribeAtEpoch(user, epoch); try initiative.claimBribes(claimData) { - hasClaimedBribes = true; // Claiming at the same epoch is an issue if (alreadyClaimed) { From 2bfb2b9dea31f7890057b54a72e9113915f80be8 Mon Sep 17 00:00:00 2001 From: Entreprenerd Date: Sun, 3 Nov 2024 11:55:54 +0100 Subject: [PATCH 51/57] chore: make precision public --- src/Governance.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Governance.sol b/src/Governance.sol index ce22502b..ff2919cb 100644 --- a/src/Governance.sol +++ b/src/Governance.sol @@ -78,7 +78,7 @@ contract Governance is Multicall, UserProxyFactory, ReentrancyGuard, IGovernance uint16 constant UNREGISTERED_INITIATIVE = type(uint16).max; // 100 Million LQTY will be necessary to make the rounding error cause 1 second of loss per operation - uint120 constant TIMESTAMP_PRECISION = 1e26; + uint120 constant public TIMESTAMP_PRECISION = 1e26; constructor( address _lqty, From eb42ca13676ff617c371b81a617d47e3cbc7b888 Mon Sep 17 00:00:00 2001 From: Entreprenerd Date: Sun, 3 Nov 2024 11:56:02 +0100 Subject: [PATCH 52/57] chore: local echidna command --- test/recon/CryticTester.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/recon/CryticTester.sol b/test/recon/CryticTester.sol index baf1cbda..95d29eb1 100644 --- a/test/recon/CryticTester.sol +++ b/test/recon/CryticTester.sol @@ -5,7 +5,7 @@ import {TargetFunctions} from "./TargetFunctions.sol"; import {CryticAsserts} from "@chimera/CryticAsserts.sol"; // echidna . --contract CryticTester --config echidna.yaml -// echidna . --contract CryticTester --config echidna.yaml --format text --test-limit 1000000 --test-mode assertion +// echidna . --contract CryticTester --config echidna.yaml --format text --test-limit 1000000 --test-mode assertion --workers 10 // medusa fuzz contract CryticTester is TargetFunctions, CryticAsserts { constructor() payable { From 4d5526fadde8afcf100e4813d85f2ed492658369 Mon Sep 17 00:00:00 2001 From: Entreprenerd Date: Sun, 3 Nov 2024 11:56:13 +0100 Subject: [PATCH 53/57] cleanup: remove debugged properties --- test/recon/CryticToFoundry.sol | 45 ---------------------------------- 1 file changed, 45 deletions(-) diff --git a/test/recon/CryticToFoundry.sol b/test/recon/CryticToFoundry.sol index de022150..ab7d8590 100644 --- a/test/recon/CryticToFoundry.sol +++ b/test/recon/CryticToFoundry.sol @@ -15,49 +15,4 @@ contract CryticToFoundry is Test, TargetFunctions, FoundryAsserts { setup(); } - - // forge test --match-test test_property_BI05_3 -vv - function test_property_BI05_3() public { - - initiative_depositBribe(107078662,31,2,0); - - property_BI05(); - - } - // forge test --match-test test_property_sum_of_initatives_matches_total_votes_bounded_0 -vv - function test_property_sum_of_initatives_matches_total_votes_bounded_0() public { - - vm.warp(block.timestamp + 576345); - - vm.roll(block.number + 1); - - governance_depositLQTY(2); - - vm.roll(block.number + 1); - vm.warp(block.timestamp + 41489); - governance_allocateLQTY_clamped_single_initiative(0,1,0); - - governance_depositLQTY_2(3); - - vm.warp(block.timestamp + 455649); - - vm.roll(block.number + 1); - - governance_allocateLQTY_clamped_single_initiative_2nd_user(0,0,2); - - vm.roll(block.number + 1); - vm.warp(block.timestamp + 136514); - governance_unregisterInitiative(0); -/** - // TODO: This property is broken, because if a snapshot was taken before the initiative was unregistered - /// Then the votes would still be part of the total state - */ - - - (uint256 initiativeVotesSum, uint256 snapshotVotes) = _getInitiativesSnapshotsAndGlobalState(); - console.log("initiativeVotesSum", initiativeVotesSum); - console.log("snapshotVotes", snapshotVotes); - - } - } From b1b5b2b620f6614b58a149ffd7859fee287ce810 Mon Sep 17 00:00:00 2001 From: Entreprenerd Date: Sun, 3 Nov 2024 11:56:21 +0100 Subject: [PATCH 54/57] fix: remove BI11 --- test/recon/properties/BribeInitiativeProperties.sol | 5 ----- 1 file changed, 5 deletions(-) diff --git a/test/recon/properties/BribeInitiativeProperties.sol b/test/recon/properties/BribeInitiativeProperties.sol index 00e4b02f..8fde6553 100644 --- a/test/recon/properties/BribeInitiativeProperties.sol +++ b/test/recon/properties/BribeInitiativeProperties.sol @@ -260,9 +260,4 @@ abstract contract BribeInitiativeProperties is BeforeAfter { } } - // BI-11: User can always claim a bribe amount for which they are entitled - function property_BI11() public { - // unableToClaim gets set in the call to claimBribes and checks if user had a claimable allocation that wasn't yet claimed and tried to claim it unsuccessfully - t(!unableToClaim, "BI-11: User can always claim a bribe amount for which they are entitled "); - } } From 36ae4c6e474ac90b18aad830e29ee184aa58b605 Mon Sep 17 00:00:00 2001 From: Entreprenerd Date: Sun, 3 Nov 2024 11:56:38 +0100 Subject: [PATCH 55/57] fix: Total and Initiative math can only be compared on state, not snapshot --- .../recon/properties/GovernanceProperties.sol | 62 +++++++++++++------ 1 file changed, 44 insertions(+), 18 deletions(-) diff --git a/test/recon/properties/GovernanceProperties.sol b/test/recon/properties/GovernanceProperties.sol index 133d6427..652f9c49 100644 --- a/test/recon/properties/GovernanceProperties.sol +++ b/test/recon/properties/GovernanceProperties.sol @@ -272,40 +272,66 @@ abstract contract GovernanceProperties is BeforeAfter { function property_sum_of_initatives_matches_total_votes_strict() public { // Sum up all initiatives // Compare to total votes - (uint256 initiativeVotesSum, uint256 snapshotVotes) = _getInitiativesSnapshotsAndGlobalState(); + (uint256 allocatedLQTYSum, uint256 totalCountedLQTY, uint256 votedPowerSum, uint256 govPower) = _getInitiativeStateAndGlobalState(); - eq(initiativeVotesSum, snapshotVotes, "Sum of votes matches Strict"); + eq(allocatedLQTYSum, totalCountedLQTY, "LQTY Sum of Initiative State matches Global State at all times"); + eq(votedPowerSum, govPower, "Voting Power Sum of Initiative State matches Global State at all times"); } function property_sum_of_initatives_matches_total_votes_bounded() public { // Sum up all initiatives // Compare to total votes - (uint256 initiativeVotesSum, uint256 snapshotVotes) = _getInitiativesSnapshotsAndGlobalState(); + (uint256 allocatedLQTYSum, uint256 totalCountedLQTY, uint256 votedPowerSum, uint256 govPower) = _getInitiativeStateAndGlobalState(); + + t( + allocatedLQTYSum == totalCountedLQTY || ( + allocatedLQTYSum >= totalCountedLQTY - TOLLERANCE && + allocatedLQTYSum <= totalCountedLQTY + TOLLERANCE + ), + "Sum of Initiative LQTY And State matches within absolute tollerance"); + t( - initiativeVotesSum == snapshotVotes || ( - initiativeVotesSum >= snapshotVotes - TOLLERANCE && - initiativeVotesSum <= snapshotVotes + TOLLERANCE + votedPowerSum == govPower || ( + votedPowerSum >= govPower - TOLLERANCE && + votedPowerSum <= govPower + TOLLERANCE ), - "Sum of votes matches within tollerance"); + "Sum of Initiative LQTY And State matches within absolute tollerance"); } - function _getInitiativesSnapshotsAndGlobalState() internal returns (uint256, uint256) { - (IGovernance.VoteSnapshot memory snapshot,,) = governance.getTotalVotesAndState(); + function _getInitiativeStateAndGlobalState() internal returns (uint256, uint256, uint256, uint256) { + ( + uint88 totalCountedLQTY, + uint120 global_countedVoteLQTYAverageTimestamp + ) = governance.globalState(); + + // Can sum via projection I guess - uint256 initiativeVotesSum; + // Global Acc + // Initiative Acc + uint256 allocatedLQTYSum; + uint256 votedPowerSum; for (uint256 i; i < deployedInitiatives.length; i++) { - (IGovernance.InitiativeVoteSnapshot memory initiativeSnapshot,,) = - governance.getInitiativeSnapshotAndState(deployedInitiatives[i]); - (Governance.InitiativeStatus status,,) = governance.getInitiativeState(deployedInitiatives[i]); + ( + uint88 voteLQTY, + uint88 vetoLQTY, + uint120 averageStakingTimestampVoteLQTY, + uint120 averageStakingTimestampVetoLQTY, + + ) = governance.initiativeStates(deployedInitiatives[i]); - // TODO: This property is broken, because if a snapshot was taken before the initiative was unregistered - /// Then the votes would still be part of the total state + // Conditional, only if not DISABLED + (Governance.InitiativeStatus status,,) = governance.getInitiativeState(deployedInitiatives[i]); + // Conditionally add based on state if (status != Governance.InitiativeStatus.DISABLED) { - // FIX: Only count total if initiative is not disabled - initiativeVotesSum += initiativeSnapshot.votes; + allocatedLQTYSum += voteLQTY; + // Sum via projection + votedPowerSum += governance.lqtyToVotes(voteLQTY, uint120(block.timestamp) * uint120(governance.TIMESTAMP_PRECISION()), averageStakingTimestampVoteLQTY); } + } - return (initiativeVotesSum, snapshot.votes); + uint256 govPower = governance.lqtyToVotes(totalCountedLQTY, uint120(block.timestamp) * uint120(governance.TIMESTAMP_PRECISION()), global_countedVoteLQTYAverageTimestamp); + + return (allocatedLQTYSum, totalCountedLQTY, votedPowerSum, govPower); } /// NOTE: This property can break in some specific combinations of: From f483b067b433ae2fc16ffcfc7e197f089ce6b77f Mon Sep 17 00:00:00 2001 From: Entreprenerd Date: Sun, 3 Nov 2024 11:56:49 +0100 Subject: [PATCH 56/57] fix: optimization properties to use state values --- test/recon/properties/OptimizationProperties.sol | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/test/recon/properties/OptimizationProperties.sol b/test/recon/properties/OptimizationProperties.sol index c9e062d9..a2d92b65 100644 --- a/test/recon/properties/OptimizationProperties.sol +++ b/test/recon/properties/OptimizationProperties.sol @@ -112,10 +112,11 @@ abstract contract OptimizationProperties is GovernanceProperties { int256 max = 0; - (uint256 sumVotes, uint256 totalVotes) = _getInitiativesSnapshotsAndGlobalState(); + (, , uint256 votedPowerSum, uint256 govPower) = _getInitiativeStateAndGlobalState(); - if(sumVotes > totalVotes) { - max = int256(sumVotes) - int256(totalVotes); + + if(votedPowerSum > govPower) { + max = int256(votedPowerSum) - int256(govPower); } return max; @@ -124,13 +125,14 @@ abstract contract OptimizationProperties is GovernanceProperties { int256 max = 0; - (uint256 sumVotes, uint256 totalVotes) = _getInitiativesSnapshotsAndGlobalState(); + (, , uint256 votedPowerSum, uint256 govPower) = _getInitiativeStateAndGlobalState(); + - if(totalVotes > sumVotes) { - max = int256(totalVotes) - int256(sumVotes); + if(govPower > votedPowerSum) { + max = int256(govPower) - int256(votedPowerSum); } - return max; + return max; // 177155848800000000000000000000000000 (2^117) } From 29471b270b365b655d4ddc74226322376e2ffe60 Mon Sep 17 00:00:00 2001 From: Entreprenerd Date: Sun, 3 Nov 2024 11:56:58 +0100 Subject: [PATCH 57/57] chore: comment on possible more checks --- test/recon/targets/BribeInitiativeTargets.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/recon/targets/BribeInitiativeTargets.sol b/test/recon/targets/BribeInitiativeTargets.sol index 3718317a..2af66ee8 100644 --- a/test/recon/targets/BribeInitiativeTargets.sol +++ b/test/recon/targets/BribeInitiativeTargets.sol @@ -117,7 +117,7 @@ abstract contract BribeInitiativeTargets is Test, BaseTargetFunctions, Propertie if (lqtyAllocated > 0 && !claimedBribe && bribeWasThere) { // user wasn't able to claim a bribe they were entitled to - unableToClaim = true; + unableToClaim = true; /// @audit Consider adding this as a test once claiming is simplified } }