diff --git a/src/BribeInitiative.sol b/src/BribeInitiative.sol index 7f30015..01cece3 100644 --- a/src/BribeInitiative.sol +++ b/src/BribeInitiative.sol @@ -235,8 +235,6 @@ contract BribeInitiative is IInitiative, IBribeInitiative { IGovernance.Allocation calldata _allocation, IGovernance.InitiativeState calldata _initiativeState ) external virtual onlyGovernance { - if (_currentEpoch == 0) return; - uint16 mostRecentUserEpoch = lqtyAllocationByUserAtEpoch[_user].getHead(); uint16 mostRecentTotalEpoch = totalLQTYAllocationByEpoch.getHead(); diff --git a/src/Governance.sol b/src/Governance.sol index 00adefb..dd155e1 100644 --- a/src/Governance.sol +++ b/src/Governance.sol @@ -108,6 +108,7 @@ contract Governance is Multicall, UserProxyFactory, ReentrancyGuard, Ownable, IG MIN_CLAIM = _config.minClaim; MIN_ACCRUAL = _config.minAccrual; + require(_config.epochStart <= block.timestamp, "Gov: cannot-start-in-future"); EPOCH_START = _config.epochStart; require(_config.epochDuration > 0, "Gov: epoch-duration-zero"); EPOCH_DURATION = _config.epochDuration; @@ -132,48 +133,23 @@ contract Governance is Multicall, UserProxyFactory, ReentrancyGuard, Ownable, IG } function _averageAge(uint120 _currentTimestamp, uint120 _averageTimestamp) internal pure returns (uint120) { - if (_averageTimestamp == 0 || _currentTimestamp < _averageTimestamp) return 0; + // Due to rounding error, _averageTimestamp can sometimes be higher than _currentTimestamp + if (_currentTimestamp < _averageTimestamp) return 0; return _currentTimestamp - _averageTimestamp; } function _calculateAverageTimestamp( - uint120 _prevOuterAverageTimestamp, - uint120 _newInnerAverageTimestamp, - uint88 _prevLQTYBalance, - uint88 _newLQTYBalance - ) internal view returns (uint120) { + uint256 _prevOuterAverageTimestamp, + uint256 _newInnerAverageTimestamp, + uint256 _prevLQTYBalance, + uint256 _newLQTYBalance + ) internal pure returns (uint120) { if (_newLQTYBalance == 0) return 0; - // NOTE: Truncation - // NOTE: u32 -> u120 - // 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); - uint120 newInnerAverageAge = _averageAge(currentTime, _newInnerAverageTimestamp); - - // 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; - uint208 prevVotes = uint208(_prevLQTYBalance) * uint208(prevOuterAverageAge); - uint208 newVotes = uint208(deltaLQTY) * uint208(newInnerAverageAge); - uint208 votes = prevVotes + newVotes; - 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 = votes / _newLQTYBalance; - } - - if (newOuterAverageAge > currentTime) return 0; - return uint120(currentTime - newOuterAverageAge); + return uint120( + _newInnerAverageTimestamp + _prevOuterAverageTimestamp * _prevLQTYBalance / _newLQTYBalance + - _newInnerAverageTimestamp * _prevLQTYBalance / _newLQTYBalance + ); } /*////////////////////////////////////////////////////////////// @@ -271,22 +247,16 @@ contract Governance is Multicall, UserProxyFactory, ReentrancyGuard, Ownable, IG /// @inheritdoc IGovernance function epoch() public view returns (uint16) { - if (block.timestamp < EPOCH_START) { - return 0; - } return uint16(((block.timestamp - EPOCH_START) / EPOCH_DURATION) + 1); } /// @inheritdoc IGovernance function epochStart() public view returns (uint32) { - uint16 currentEpoch = epoch(); - if (currentEpoch == 0) return 0; - return uint32(EPOCH_START + (currentEpoch - 1) * EPOCH_DURATION); + return uint32(EPOCH_START + (epoch() - 1) * EPOCH_DURATION); } /// @inheritdoc IGovernance function secondsWithinEpoch() public view returns (uint32) { - if (block.timestamp < EPOCH_START) return 0; return uint32((block.timestamp - EPOCH_START) % EPOCH_DURATION); } @@ -729,7 +699,7 @@ contract Governance is Multicall, UserProxyFactory, ReentrancyGuard, Ownable, IG assert(deltaLQTYVotes != 0 || deltaLQTYVetos != 0); /// === Check FSM === /// - // Can vote positively in SKIP, CLAIMABLE, CLAIMED and UNREGISTERABLE states + // Can vote positively in SKIP, CLAIMABLE and CLAIMED states // Force to remove votes if disabled // Can remove votes and vetos in every stage (InitiativeVoteSnapshot memory votesForInitiativeSnapshot_, InitiativeState memory initiativeState) = diff --git a/test/Governance.t.sol b/test/Governance.t.sol index 41a43f2..d779d8f 100644 --- a/test/Governance.t.sol +++ b/test/Governance.t.sol @@ -24,32 +24,6 @@ import {MockStakingV1} from "./mocks/MockStakingV1.sol"; import {MockStakingV1Deployer} from "./mocks/MockStakingV1Deployer.sol"; import "./constants.sol"; -contract GovernanceInternal is Governance { - constructor( - address _lqty, - address _lusd, - address _stakingV1, - address _bold, - Configuration memory _config, - address[] memory _initiatives - ) Governance(_lqty, _lusd, _stakingV1, _bold, _config, msg.sender, _initiatives) {} - - function averageAge(uint120 _currentTimestamp, uint120 _averageTimestamp) external pure returns (uint120) { - return _averageAge(_currentTimestamp, _averageTimestamp); - } - - function calculateAverageTimestamp( - uint120 _prevOuterAverageTimestamp, - uint120 _newInnerAverageTimestamp, - uint88 _prevLQTYBalance, - uint88 _newLQTYBalance - ) external view returns (uint208) { - return _calculateAverageTimestamp( - _prevOuterAverageTimestamp, _newInnerAverageTimestamp, _prevLQTYBalance, _newLQTYBalance - ); - } -} - abstract contract GovernanceTest is Test { ILQTY internal lqty; ILUSD internal lusd; @@ -70,7 +44,6 @@ abstract contract GovernanceTest is Test { uint32 private constant EPOCH_VOTING_CUTOFF = 518400; Governance private governance; - GovernanceInternal private governanceInternal; address[] private initialInitiatives; address private baseInitiative2; @@ -108,36 +81,6 @@ abstract contract GovernanceTest is Test { initialInitiatives.push(baseInitiative1); initialInitiatives.push(baseInitiative2); governance.registerInitialInitiatives(initialInitiatives); - - governanceInternal = new GovernanceInternal( - address(lqty), address(lusd), address(stakingV1), address(lusd), config, initialInitiatives - ); - } - - // should not revert under any input - function test_averageAge(uint120 _currentTimestamp, uint120 _timestamp) public { - uint120 averageAge = governanceInternal.averageAge(_currentTimestamp, _timestamp); - if (_timestamp == 0 || _currentTimestamp < _timestamp) { - assertEq(averageAge, 0); - } else { - assertEq(averageAge, _currentTimestamp - _timestamp); - } - } - - // should not revert under any input - function test_calculateAverageTimestamp( - uint32 _prevOuterAverageTimestamp, - uint32 _newInnerAverageTimestamp, - uint88 _prevLQTYBalance, - uint88 _newLQTYBalance - ) public { - uint32 highestTimestamp = (_prevOuterAverageTimestamp > _newInnerAverageTimestamp) - ? _prevOuterAverageTimestamp - : _newInnerAverageTimestamp; - if (highestTimestamp > block.timestamp) vm.warp(highestTimestamp); - governanceInternal.calculateAverageTimestamp( - _prevOuterAverageTimestamp, _newInnerAverageTimestamp, _prevLQTYBalance, _newLQTYBalance - ); } // forge test --match-test test_depositLQTY_withdrawLQTY -vv @@ -315,7 +258,7 @@ abstract contract GovernanceTest is Test { // should not revert under any block.timestamp >= EPOCH_START function test_epoch_fuzz(uint32 _timestamp) public { - vm.warp(_timestamp); + vm.warp(governance.EPOCH_START() + _timestamp); governance.epoch(); } @@ -328,7 +271,7 @@ abstract contract GovernanceTest is Test { // should not revert under any block.timestamp >= EPOCH_START function test_epochStart_fuzz(uint32 _timestamp) public { - vm.warp(_timestamp); + vm.warp(governance.EPOCH_START() + _timestamp); governance.epochStart(); } @@ -347,7 +290,7 @@ abstract contract GovernanceTest is Test { // should not revert under any block.timestamp function test_secondsWithinEpoch_fuzz(uint32 _timestamp) public { - vm.warp(_timestamp); + vm.warp(governance.EPOCH_START() + _timestamp); governance.secondsWithinEpoch(); } diff --git a/test/recon/CryticToFoundry.sol b/test/recon/CryticToFoundry.sol index 6f8d178..3c3b2bd 100644 --- a/test/recon/CryticToFoundry.sol +++ b/test/recon/CryticToFoundry.sol @@ -43,7 +43,6 @@ contract CryticToFoundry is Test, TargetFunctions, FoundryAsserts { console.log("votedPowerSum", votedPowerSum); console.log("govPower", govPower); - // XXX letting broken property pass for now, so we have green CI status - assertFalse(optimize_property_sum_of_initatives_matches_total_votes_insolvency()); + assertTrue(optimize_property_sum_of_initatives_matches_total_votes_insolvency()); } }