From c0134ae9acf1f2375b3a53568b1305790237b60f Mon Sep 17 00:00:00 2001 From: Andreas Penzkofer <36140574+apenzk@users.noreply.github.com> Date: Tue, 18 Feb 2025 15:40:32 +0100 Subject: [PATCH] [postconfirmation] epoch names (#1052) --- .../mcr/contracts/src/settlement/MCR.sol | 67 +++++++++---------- .../contracts/src/settlement/MCRStorage.sol | 2 +- .../contracts/src/staking/MovementStaking.sol | 55 ++++++++------- .../src/staking/MovementStakingStorage.sol | 2 +- .../staking/interfaces/IMovementStaking.sol | 12 ++-- .../mcr/contracts/test/settlement/MCR.sol | 18 ++--- .../test/staking/MovementStaking.t.sol | 32 ++++----- .../mcr/contracts/test/token/MOVEToken.t.sol | 21 ++++-- .../contracts/test/token/MOVETokenV2.t.sol | 15 ++++- 9 files changed, 126 insertions(+), 98 deletions(-) diff --git a/protocol-units/settlement/mcr/contracts/src/settlement/MCR.sol b/protocol-units/settlement/mcr/contracts/src/settlement/MCR.sol index 8b0711ac2..6b41353e8 100644 --- a/protocol-units/settlement/mcr/contracts/src/settlement/MCR.sol +++ b/protocol-units/settlement/mcr/contracts/src/settlement/MCR.sol @@ -67,18 +67,18 @@ contract MCR is Initializable, BaseSettlement, MCRStorage, IMCR { } // gets the would be epoch for the current L1Block time - function getEpochByL1BlockTime() public view returns (uint256) { + function getPresentEpoch() public view returns (uint256) { return stakingContract.getEpochByL1BlockTime(address(this)); } - // gets the current epoch up to which superBlocks have been accepted - function getCurrentEpoch() public view returns (uint256) { - return stakingContract.getCurrentEpoch(address(this)); + // gets the epoch up to which superBlocks have been accepted + function getAcceptingEpoch() public view returns (uint256) { + return stakingContract.getAcceptingEpoch(address(this)); } // gets the next epoch - function getNextEpoch() public view returns (uint256) { - return stakingContract.getNextEpoch(address(this)); + function getNextAcceptingEpoch() public view returns (uint256) { + return stakingContract.getNextAcceptingEpoch(address(this)); } // gets the stake for a given attester at a given epoch @@ -113,17 +113,17 @@ contract MCR is Initializable, BaseSettlement, MCRStorage, IMCR { } // gets the stake for a given attester at the current epoch - function getCurrentEpochStake( + function getAcceptingEpochStake( address custodian, address attester ) public view returns (uint256) { - return getStakeAtEpoch(getCurrentEpoch(), custodian, attester); + return getStakeAtEpoch(getAcceptingEpoch(), custodian, attester); } - function computeAllCurrentEpochStake( + function computeAllStakeFromAcceptingEpoch( address attester ) public view returns (uint256) { - return computeAllStakeAtEpoch(getCurrentEpoch(), attester); + return computeAllStakeAtEpoch(getAcceptingEpoch(), attester); } // gets the total stake for a given epoch @@ -161,19 +161,19 @@ contract MCR is Initializable, BaseSettlement, MCRStorage, IMCR { return totalStake; } - // gets the total stake for the current epoch - function getTotalStakeForCurrentEpoch( + // gets the total stake for the current epoch for a given custodian + function getTotalStakeForAcceptingEpoch( address custodian ) public view returns (uint256) { - return getTotalStakeForEpoch(getCurrentEpoch(), custodian); + return getTotalStakeForEpoch(getAcceptingEpoch(), custodian); } - function computeAllTotalStakeForCurrentEpoch() + function computeAllTotalStakeForAcceptingEpoch() public view returns (uint256) { - return computeAllTotalStakeForEpoch(getCurrentEpoch()); + return computeAllTotalStakeForEpoch(getAcceptingEpoch()); } function getValidatorCommitmentAtSuperBlockHeight( @@ -189,7 +189,8 @@ contract MCR is Initializable, BaseSettlement, MCRStorage, IMCR { hasRole(COMMITMENT_ADMIN, msg.sender), "SET_LAST_ACCEPTED_COMMITMENT_AT_HEIGHT_IS_COMMITMENT_ADMIN_ONLY" ); - versionedAcceptedSuperBlocks[acceptedSuperBlocksVersion][superBlockCommitment.height] = superBlockCommitment; + versionedAcceptedSuperBlocks[acceptedSuperBlocksVersion][superBlockCommitment.height] = superBlockCommitment; + setlastAcceptedSuperBlockHeight(superBlockCommitment.height); } // Sets the last accepted superBlock height. @@ -204,15 +205,11 @@ contract MCR is Initializable, BaseSettlement, MCRStorage, IMCR { // Forces the latest attestation by setting the superBlock height // Note: this only safe when we are running with a single validator as it does not zero out follow-on commitments. function forceLatestCommitment(SuperBlockCommitment memory superBlockCommitment) public { - /*require( - hasRole(DEFAULT_ADMIN_ROLE, msg.sender), + require( + hasRole(COMMITMENT_ADMIN, msg.sender), "FORCE_LATEST_COMMITMENT_IS_COMMITMENT_ADMIN_ONLY" - );*/ - - // increment the acceptedSuperBlocksVersion (effectively removing all other accepted superBlocks) - acceptedSuperBlocksVersion += 1; - versionedAcceptedSuperBlocks[acceptedSuperBlocksVersion][superBlockCommitment.height] = superBlockCommitment; - lastAcceptedSuperBlockHeight = superBlockCommitment.height; + ); + setAcceptedCommitmentAtBlockHeight(superBlockCommitment); } function getAcceptedCommitmentAtSuperBlockHeight(uint256 height) public view returns (SuperBlockCommitment memory) { @@ -244,26 +241,26 @@ contract MCR is Initializable, BaseSettlement, MCRStorage, IMCR { ) revert AttesterAlreadyCommitted(); // assign the superBlock height to the current epoch if it hasn't been assigned yet - if (superBlockHeightEpochAssignments[superBlockCommitment.height] == 0) { + if (superBlockHeightAssignedEpoch[superBlockCommitment.height] == 0) { // note: this is an intended race condition, but it is benign because of the tolerance - superBlockHeightEpochAssignments[ + superBlockHeightAssignedEpoch[ superBlockCommitment.height - ] = getEpochByL1BlockTime(); + ] = getPresentEpoch(); } // register the attester's commitment commitments[superBlockCommitment.height][attester] = superBlockCommitment; // increment the commitment count by stake - uint256 allCurrentEpochStake = computeAllCurrentEpochStake(attester); + uint256 allStakeFromAcceptingEpoch = computeAllStakeFromAcceptingEpoch(attester); commitmentStakes[superBlockCommitment.height][ superBlockCommitment.commitment - ] += allCurrentEpochStake; + ] += allStakeFromAcceptingEpoch; emit SuperBlockCommitmentSubmitted( superBlockCommitment.blockId, superBlockCommitment.commitment, - allCurrentEpochStake + allStakeFromAcceptingEpoch ); // keep ticking through to find accepted superBlocks @@ -279,12 +276,12 @@ contract MCR is Initializable, BaseSettlement, MCRStorage, IMCR { */ function tickOnSuperBlockHeight(uint256 superBlockHeight) internal returns (bool) { // get the epoch assigned to the superBlock height - uint256 superBlockEpoch = superBlockHeightEpochAssignments[superBlockHeight]; + uint256 superBlockEpoch = superBlockHeightAssignedEpoch[superBlockHeight]; // if the current epoch is far behind, that's okay that just means there weren't superBlocks submitted // so long as we ensure that we go through the superBlocks in order and that the superBlock to epoch assignment is non-decreasing, we're good // so, we'll just keep rolling over the epoch until we catch up - while (getCurrentEpoch() < superBlockEpoch) { + while (getAcceptingEpoch() < superBlockEpoch) { rollOverEpoch(); } @@ -360,10 +357,10 @@ contract MCR is Initializable, BaseSettlement, MCRStorage, IMCR { function _acceptSuperBlockCommitment( SuperBlockCommitment memory superBlockCommitment ) internal { - uint256 currentEpoch = getCurrentEpoch(); + uint256 currentAcceptingEpoch = getAcceptingEpoch(); // get the epoch for the superBlock commitment // SuperBlock commitment is not in the current epoch, it cannot be accepted. This indicates a bug in the protocol. - if (superBlockHeightEpochAssignments[superBlockCommitment.height] != currentEpoch) + if (superBlockHeightAssignedEpoch[superBlockCommitment.height] != currentAcceptingEpoch) revert UnacceptableSuperBlockCommitment(); // set accepted superBlock commitment @@ -383,7 +380,7 @@ contract MCR is Initializable, BaseSettlement, MCRStorage, IMCR { ); // if the timestamp epoch is greater than the current epoch, roll over the epoch - if (getEpochByL1BlockTime() > currentEpoch) { + if (getPresentEpoch() > currentAcceptingEpoch) { rollOverEpoch(); } } diff --git a/protocol-units/settlement/mcr/contracts/src/settlement/MCRStorage.sol b/protocol-units/settlement/mcr/contracts/src/settlement/MCRStorage.sol index 6769404b1..4425be947 100644 --- a/protocol-units/settlement/mcr/contracts/src/settlement/MCRStorage.sol +++ b/protocol-units/settlement/mcr/contracts/src/settlement/MCRStorage.sol @@ -29,7 +29,7 @@ contract MCRStorage { } // map each superBlock height to an epoch - mapping(uint256 superBlockHeight => uint256 epoch) public superBlockHeightEpochAssignments; + mapping(uint256 superBlockHeight => uint256 epoch) public superBlockHeightAssignedEpoch; // track each commitment from each attester for each superBlock height mapping(uint256 superBlockHeight => mapping(address attester => SuperBlockCommitment)) public commitments; diff --git a/protocol-units/settlement/mcr/contracts/src/staking/MovementStaking.sol b/protocol-units/settlement/mcr/contracts/src/staking/MovementStaking.sol index 9429669c7..3a7cb10cd 100644 --- a/protocol-units/settlement/mcr/contracts/src/staking/MovementStaking.sol +++ b/protocol-units/settlement/mcr/contracts/src/staking/MovementStaking.sol @@ -65,7 +65,7 @@ contract MovementStaking is if (domainGenesisAccepted[domain]) revert GenesisAlreadyAccepted(); domainGenesisAccepted[domain] = true; // roll over from 0 (genesis) to current epoch by L1Block time - currentEpochByDomain[domain] = getEpochByL1BlockTime(domain); + currentAcceptingEpochByDomain[domain] = getEpochByL1BlockTime(domain); for (uint256 i = 0; i < attestersByDomain[domain].length(); i++) { address attester = attestersByDomain[domain].at(i); @@ -84,7 +84,7 @@ contract MovementStaking is // roll over the genesis stake to the current epoch _addStake( domain, - getCurrentEpoch(domain), + getAcceptingEpoch(domain), custodian, attester, attesterStake @@ -151,20 +151,25 @@ contract MovementStaking is } // gets the current epoch up to which superBlocks have been accepted - function getCurrentEpoch(address domain) public view returns (uint256) { - return currentEpochByDomain[domain]; + function getAcceptingEpoch(address domain) public view returns (uint256) { + return currentAcceptingEpochByDomain[domain]; } - // gets the next epoch - function getNextEpoch(address domain) public view returns (uint256) { - return getCurrentEpoch(domain) == 0 ? 0 : getCurrentEpoch(domain) + 1; + /// @notice Gets the next accepting epoch number + /// @dev Special handling for genesis state (epoch 0): + /// @dev If getAcceptingEpoch(domain) == 0, returns 0 to stay in genesis until ceremony completes + function getNextAcceptingEpoch(address domain) public view returns (uint256) { + return getAcceptingEpoch(domain) == 0 ? 0 : getAcceptingEpoch(domain) + 1; } - function getNextEpochByL1BlockTime( + /// @notice Gets the next present epoch number + /// @dev Special handling for genesis state (epoch 0): + /// @dev If getAcceptingEpoch(domain) == 0, returns 0 to stay in genesis until ceremony completes + function getNextPresentEpochWithException( address domain ) public view returns (uint256) { return - getCurrentEpoch(domain) == 0 ? 0 : getEpochByL1BlockTime(domain) + 1; + getAcceptingEpoch(domain) == 0 ? 0 : getEpochByL1BlockTime(domain) + 1; } // gets the stake for a given attester at a given epoch @@ -178,7 +183,7 @@ contract MovementStaking is } // gets the stake for a given attester at the current epoch - function getCurrentEpochStake( + function getAcceptingEpochStake( address domain, address custodian, address attester @@ -186,7 +191,7 @@ contract MovementStaking is return getStakeAtEpoch( domain, - getCurrentEpoch(domain), + getAcceptingEpoch(domain), custodian, attester ); @@ -203,7 +208,7 @@ contract MovementStaking is } // gets the unstake for a given attester at the current epoch - function getCurrentEpochUnstake( + function getAcceptingEpochUnstake( address domain, address custodian, address attester @@ -211,7 +216,7 @@ contract MovementStaking is return getUnstakeAtEpoch( domain, - getCurrentEpoch(domain), + getAcceptingEpoch(domain), custodian, attester ); @@ -227,12 +232,12 @@ contract MovementStaking is } // gets the total stake for the current epoch - function getTotalStakeForCurrentEpoch( + function getTotalStakeForAcceptingEpoch( address domain, address custodian ) public view returns (uint256) { return - getTotalStakeForEpoch(domain, getCurrentEpoch(domain), custodian); + getTotalStakeForEpoch(domain, getAcceptingEpoch(domain), custodian); } // stakes for the next epoch @@ -263,7 +268,7 @@ contract MovementStaking is // set the attester to stake for the next epoch _addStake( domain, - getNextEpochByL1BlockTime(domain), + getNextPresentEpochWithException(domain), address(custodian), msg.sender, amount @@ -272,7 +277,7 @@ contract MovementStaking is // Let the world know that the attester has staked emit AttesterStaked( domain, - getNextEpoch(domain), + getNextAcceptingEpoch(domain), address(custodian), msg.sender, amount @@ -290,7 +295,7 @@ contract MovementStaking is // note: by tracking in the next epoch we need to make sure when we roll over an epoch we check the amount rolled over from stake by the unstake in the next epoch _addUnstake( domain, - getNextEpochByL1BlockTime(domain), + getNextPresentEpochWithException(domain), custodian, msg.sender, amount @@ -298,7 +303,7 @@ contract MovementStaking is emit AttesterUnstaked( domain, - getNextEpoch(domain), + getNextAcceptingEpoch(domain), custodian, msg.sender, amount @@ -312,7 +317,7 @@ contract MovementStaking is address custodian, address attester ) internal { - // the amount of stake rolled over is stake[currentEpoch] - unstake[nextEpoch] + // the amount of stake rolled over is stake[currentAcceptingEpoch] - unstake[nextEpoch] uint256 stakeAmount = getStakeAtEpoch( domain, epochNumber, @@ -361,13 +366,13 @@ contract MovementStaking is } // increment the current epoch - currentEpochByDomain[domain] = epochNumber + 1; + currentAcceptingEpochByDomain[domain] = epochNumber + 1; emit EpochRolledOver(domain, epochNumber); } function rollOverEpoch() external { - _rollOverEpoch(msg.sender, getCurrentEpoch(msg.sender)); + _rollOverEpoch(msg.sender, getAcceptingEpoch(msg.sender)); } /** @@ -462,7 +467,7 @@ contract MovementStaking is uint256 refundAmount = Math.min( getStakeAtEpoch( msg.sender, - getCurrentEpoch(attesters[i]), + getAcceptingEpoch(attesters[i]), custodians[i], attesters[i] ), @@ -478,7 +483,7 @@ contract MovementStaking is // slash both stake and unstake so that the weight of the attester is reduced and they can't withdraw the unstake at the next epoch _slashStake( msg.sender, - getCurrentEpoch(msg.sender), + getAcceptingEpoch(msg.sender), custodians[i], attesters[i], amounts[i] @@ -486,7 +491,7 @@ contract MovementStaking is _slashUnstake( msg.sender, - getCurrentEpoch(msg.sender), + getAcceptingEpoch(msg.sender), custodians[i], attesters[i] ); diff --git a/protocol-units/settlement/mcr/contracts/src/staking/MovementStakingStorage.sol b/protocol-units/settlement/mcr/contracts/src/staking/MovementStakingStorage.sol index 6f96ff3a0..8e9e9bb0d 100644 --- a/protocol-units/settlement/mcr/contracts/src/staking/MovementStakingStorage.sol +++ b/protocol-units/settlement/mcr/contracts/src/staking/MovementStakingStorage.sol @@ -14,7 +14,7 @@ contract MovementStakingStorage { IERC20 public token; mapping(address domain => uint256 epochDuration) public epochDurationByDomain; - mapping(address domain => uint256 currentEpoch) public currentEpochByDomain; + mapping(address domain => uint256 currentAcceptingEpoch) public currentAcceptingEpochByDomain; mapping(address domain => EnumerableSet.AddressSet attester) internal attestersByDomain; mapping(address domain => EnumerableSet.AddressSet custodian) internal custodiansByDomain; diff --git a/protocol-units/settlement/mcr/contracts/src/staking/interfaces/IMovementStaking.sol b/protocol-units/settlement/mcr/contracts/src/staking/interfaces/IMovementStaking.sol index 7fa4848e2..aa00c3888 100644 --- a/protocol-units/settlement/mcr/contracts/src/staking/interfaces/IMovementStaking.sol +++ b/protocol-units/settlement/mcr/contracts/src/staking/interfaces/IMovementStaking.sol @@ -10,16 +10,16 @@ interface IMovementStaking { ) external; function acceptGenesisCeremony() external; function getEpochByL1BlockTime(address) external view returns (uint256); - function getCurrentEpoch(address) external view returns (uint256); - function getNextEpoch(address) external view returns (uint256); - function getNextEpochByL1BlockTime(address) external view returns (uint256); + function getAcceptingEpoch(address) external view returns (uint256); + function getNextAcceptingEpoch(address) external view returns (uint256); + function getNextPresentEpochWithException(address) external view returns (uint256); function getStakeAtEpoch( address domain, uint256 epoch, address custodian, address attester ) external view returns (uint256); - function getCurrentEpochStake( + function getAcceptingEpochStake( address domain, address custodian, address attester @@ -30,7 +30,7 @@ interface IMovementStaking { address custodian, address attester ) external view returns (uint256); - function getCurrentEpochUnstake( + function getAcceptingEpochUnstake( address domain, address custodian, address attester @@ -40,7 +40,7 @@ interface IMovementStaking { uint256 epoch, address custodian ) external view returns (uint256); - function getTotalStakeForCurrentEpoch( + function getTotalStakeForAcceptingEpoch( address domain, address custodian ) external view returns (uint256); diff --git a/protocol-units/settlement/mcr/contracts/test/settlement/MCR.sol b/protocol-units/settlement/mcr/contracts/test/settlement/MCR.sol index d80a61543..cb8dc87f4 100644 --- a/protocol-units/settlement/mcr/contracts/test/settlement/MCR.sol +++ b/protocol-units/settlement/mcr/contracts/test/settlement/MCR.sol @@ -236,10 +236,10 @@ contract MCRTest is Test, IMCR { mcr.submitSuperBlockCommitment(bc2); // check that roll over happened - assertEq(mcr.getCurrentEpoch(), mcr.getEpochByL1BlockTime()); - assertEq(mcr.getCurrentEpochStake(address(moveToken), alice), 34); - assertEq(mcr.getCurrentEpochStake(address(moveToken), bob), 33); - assertEq(mcr.getCurrentEpochStake(address(moveToken), carol), 33); + assertEq(mcr.getAcceptingEpoch(), mcr.getPresentEpoch()); + assertEq(mcr.getAcceptingEpochStake(address(moveToken), alice), 34); + assertEq(mcr.getAcceptingEpochStake(address(moveToken), bob), 33); + assertEq(mcr.getAcceptingEpochStake(address(moveToken), carol), 33); MCRStorage.SuperBlockCommitment memory retrievedCommitment = mcr.getAcceptedCommitmentAtSuperBlockHeight(1); assert(retrievedCommitment.commitment == bc1.commitment); assert(retrievedCommitment.blockId == bc1.blockId); @@ -374,10 +374,9 @@ contract MCRTest is Test, IMCR { vm.pauseGasMetering(); uint256 blockTime = 300; - vm.warp(blockTime); - // default signer should be able to force commitment + // default signer (test contract) should be able to force commitment since it has COMMITMENT_ADMIN role MCRStorage.SuperBlockCommitment memory forcedCommitment = MCRStorage.SuperBlockCommitment({ height: 1, commitment: keccak256(abi.encodePacked(uint256(3), uint256(2), uint256(1))), @@ -394,15 +393,18 @@ contract MCRTest is Test, IMCR { // create an unauthorized signer address payable alice = payable(vm.addr(1)); - // try to force a different commitment + // try to force a different commitment with unauthorized user MCRStorage.SuperBlockCommitment memory badForcedCommitment = MCRStorage.SuperBlockCommitment({ height: 1, commitment: keccak256(abi.encodePacked(uint256(1), uint256(2), uint256(3))), blockId: keccak256(abi.encodePacked(uint256(1), uint256(2), uint256(3))) }); + + // Alice should not have COMMITMENT_ADMIN role + assertEq(mcr.hasRole(mcr.COMMITMENT_ADMIN(), alice), false); + vm.prank(alice); vm.expectRevert("FORCE_LATEST_COMMITMENT_IS_COMMITMENT_ADMIN_ONLY"); mcr.forceLatestCommitment(badForcedCommitment); - } } \ No newline at end of file diff --git a/protocol-units/settlement/mcr/contracts/test/staking/MovementStaking.t.sol b/protocol-units/settlement/mcr/contracts/test/staking/MovementStaking.t.sol index 2cf39e9da..b37d505ea 100644 --- a/protocol-units/settlement/mcr/contracts/test/staking/MovementStaking.t.sol +++ b/protocol-units/settlement/mcr/contracts/test/staking/MovementStaking.t.sol @@ -45,7 +45,7 @@ contract MovementStakingTest is Test { vm.prank(domain); staking.registerDomain(1 seconds, custodians); - assertEq(staking.getCurrentEpoch(domain), 0); + assertEq(staking.getAcceptingEpoch(domain), 0); } function testWhitelist() public { @@ -105,8 +105,8 @@ contract MovementStakingTest is Test { staking.stake(domain, moveToken, 100); vm.prank(domain); staking.acceptGenesisCeremony(); - assertNotEq(staking.currentEpochByDomain(domain), 0); - assertEq(staking.getCurrentEpochStake(domain, address(moveToken), staker), 100); + assertNotEq(staking.currentAcceptingEpochByDomain(domain), 0); + assertEq(staking.getAcceptingEpochStake(domain, address(moveToken), staker), 100); vm.expectRevert(IMovementStaking.GenesisAlreadyAccepted.selector); vm.prank(domain); @@ -138,12 +138,12 @@ contract MovementStakingTest is Test { // rollover epoch for (uint256 i = 0; i < 10; i++) { vm.warp((i + 1) * 1 seconds); - uint256 epochBefore = staking.getCurrentEpoch(domain); + uint256 epochBefore = staking.getAcceptingEpoch(domain); vm.prank(domain); staking.rollOverEpoch(); - uint256 epochAfter = staking.getCurrentEpoch(domain); + uint256 epochAfter = staking.getAcceptingEpoch(domain); assertEq(epochAfter, epochBefore + 1); - assertEq(staking.getCurrentEpochStake(domain, address(moveToken), staker), 100); + assertEq(staking.getAcceptingEpochStake(domain, address(moveToken), staker), 100); } } @@ -170,18 +170,18 @@ contract MovementStakingTest is Test { for (uint256 i = 0; i < 10; i++) { vm.warp((i + 1) * 1 seconds); - uint256 epochBefore = staking.getCurrentEpoch(domain); + uint256 epochBefore = staking.getAcceptingEpoch(domain); // unstake vm.prank(staker); staking.unstake(domain, address(moveToken), 10); - assertEq(staking.getCurrentEpochStake(domain, address(moveToken), staker), 100 - (i * 10)); + assertEq(staking.getAcceptingEpochStake(domain, address(moveToken), staker), 100 - (i * 10)); assertEq(moveToken.balanceOf(staker), i * 10); // roll over vm.prank(domain); staking.rollOverEpoch(); - uint256 epochAfter = staking.getCurrentEpoch(domain); + uint256 epochAfter = staking.getAcceptingEpoch(domain); assertEq(epochAfter, epochBefore + 1); } } @@ -209,7 +209,7 @@ contract MovementStakingTest is Test { for (uint256 i = 0; i < 10; i++) { vm.warp((i + 1) * 1 seconds); - uint256 epochBefore = staking.getCurrentEpoch(domain); + uint256 epochBefore = staking.getAcceptingEpoch(domain); // unstake vm.prank(staker); @@ -222,13 +222,13 @@ contract MovementStakingTest is Test { staking.stake(domain, moveToken, 5); // check stake - assertEq(staking.getCurrentEpochStake(domain, address(moveToken), staker), (100 - (i * 10)) + (i * 5)); + assertEq(staking.getAcceptingEpochStake(domain, address(moveToken), staker), (100 - (i * 10)) + (i * 5)); assertEq(moveToken.balanceOf(staker), (50 - (i + 1) * 5) + (i * 10)); // roll over vm.prank(domain); staking.rollOverEpoch(); - uint256 epochAfter = staking.getCurrentEpoch(domain); + uint256 epochAfter = staking.getAcceptingEpoch(domain); assertEq(epochAfter, epochBefore + 1); } } @@ -256,7 +256,7 @@ contract MovementStakingTest is Test { for (uint256 i = 0; i < 5; i++) { vm.warp((i + 1) * 1 seconds); - uint256 epochBefore = staking.getCurrentEpoch(domain); + uint256 epochBefore = staking.getAcceptingEpoch(domain); // unstake vm.prank(staker); @@ -270,7 +270,7 @@ contract MovementStakingTest is Test { // check stake assertEq( - staking.getCurrentEpochStake(domain, address(moveToken), staker), (100 - (i * 10)) + (i * 5) - (i * 1) + staking.getAcceptingEpochStake(domain, address(moveToken), staker), (100 - (i * 10)) + (i * 5) - (i * 1) ); assertEq(moveToken.balanceOf(staker), (50 - (i + 1) * 5) + (i * 10)); @@ -288,14 +288,14 @@ contract MovementStakingTest is Test { // slash immediately takes effect assertEq( - staking.getCurrentEpochStake(domain, address(moveToken), staker), + staking.getAcceptingEpochStake(domain, address(moveToken), staker), (100 - (i * 10)) + (i * 5) - ((i + 1) * 1) ); // roll over vm.prank(domain); staking.rollOverEpoch(); - uint256 epochAfter = staking.getCurrentEpoch(domain); + uint256 epochAfter = staking.getAcceptingEpoch(domain); assertEq(epochAfter, epochBefore + 1); } } diff --git a/protocol-units/settlement/mcr/contracts/test/token/MOVEToken.t.sol b/protocol-units/settlement/mcr/contracts/test/token/MOVEToken.t.sol index 674c4f5a4..6d2c0d512 100644 --- a/protocol-units/settlement/mcr/contracts/test/token/MOVEToken.t.sol +++ b/protocol-units/settlement/mcr/contracts/test/token/MOVEToken.t.sol @@ -2,6 +2,7 @@ pragma solidity ^0.8.19; import "forge-std/Test.sol"; +import "forge-std/console2.sol"; import {MOVEToken} from "../../src/token/MOVEToken.sol"; import {MOVETokenDev} from "../../src/token/MOVETokenDev.sol"; import {ProxyAdmin} from "@openzeppelin/contracts/proxy/transparent/ProxyAdmin.sol"; @@ -29,6 +30,7 @@ contract MOVETokenTest is Test { string public moveSignature = "initialize(address,address)"; address public multisig = address(0x00db70A9e12537495C359581b7b3Bc3a69379A00); address public anchorage = address(0xabc); + bytes32 public constant DEFAULT_ADMIN_ROLE = 0x00; function setUp() public { moveTokenImplementation = new MOVEToken(); @@ -79,15 +81,24 @@ contract MOVETokenTest is Test { assertEq(token.balanceOf(anchorage), 10000000000 * 10 ** 8); } + /// @notice Fuzzing test to verify admin role permissions + /// @param other Any address to test against function testAdminRoleFuzz(address other) public { - assertEq(token.hasRole(0x00, other), false); - assertEq(token.hasRole(0x00, multisig), true); - assertEq(token.hasRole(0x00, anchorage), false); + // Verify multisig has admin role (primary admin) + assertEq(token.hasRole(DEFAULT_ADMIN_ROLE, multisig), true); + + // Verify other addresses only have admin if they are the multisig + assertEq(token.hasRole(DEFAULT_ADMIN_ROLE, other), other == multisig); + + // Verify custody address (anchorage) does not have admin role + assertEq(token.hasRole(DEFAULT_ADMIN_ROLE, anchorage), false); + // Test role granting permissions (skip multisig since it should succeed) + vm.assume(other != multisig); vm.expectRevert( - abi.encodeWithSelector(IAccessControl.AccessControlUnauthorizedAccount.selector, address(this), 0x00) + abi.encodeWithSelector(IAccessControl.AccessControlUnauthorizedAccount.selector, address(this), DEFAULT_ADMIN_ROLE) ); - token.grantRole(0x00, other); + token.grantRole(DEFAULT_ADMIN_ROLE, other); } function testUpgradeFromTimelock() public { diff --git a/protocol-units/settlement/mcr/contracts/test/token/MOVETokenV2.t.sol b/protocol-units/settlement/mcr/contracts/test/token/MOVETokenV2.t.sol index 82a690818..e41e13bdb 100644 --- a/protocol-units/settlement/mcr/contracts/test/token/MOVETokenV2.t.sol +++ b/protocol-units/settlement/mcr/contracts/test/token/MOVETokenV2.t.sol @@ -6,6 +6,8 @@ import "../../src/token/MOVETokenDev.sol"; import {ProxyAdmin} from "@openzeppelin/contracts/proxy/transparent/ProxyAdmin.sol"; import {TransparentUpgradeableProxy} from "@openzeppelin/contracts/proxy/transparent/TransparentUpgradeableProxy.sol"; import {IAccessControl} from "@openzeppelin/contracts/access/IAccessControl.sol"; +import {console} from "forge-std/console.sol"; +import {AccessControl} from "@openzeppelin/contracts/access/AccessControl.sol"; contract MOVETokenDevTest is Test { MOVETokenDev public token; @@ -13,6 +15,7 @@ contract MOVETokenDevTest is Test { string public moveSignature = "initialize(address)"; address public multisig = 0x00db70A9e12537495C359581b7b3Bc3a69379A00; bytes32 public MINTER_ROLE; + bytes32 public constant DEFAULT_ADMIN_ROLE = 0x00; function setUp() public { MOVETokenDev moveTokenImplementation = new MOVETokenDev(); @@ -126,12 +129,22 @@ contract MOVETokenDevTest is Test { vm.stopPrank(); } + // Tests that non-admin accounts cannot grant roles by checking for the expected revert function testCannotGrantRoleFuzz(address messenger, address receiver) public { + // impersonate the messenger address for all subsequent calls vm.startPrank(messenger); + + // Only test unauthorized accounts (non-multisig addresses) if (messenger != multisig) { + // Expect the call to revert with AccessControlUnauthorizedAccount error + // - messenger: the account trying to grant the role + // - DEFAULT_ADMIN_ROLE (0x00): the role needed to grant any role vm.expectRevert( - abi.encodeWithSelector(IAccessControl.AccessControlUnauthorizedAccount.selector, messenger, 0x00) + abi.encodeWithSelector(IAccessControl.AccessControlUnauthorizedAccount.selector, messenger, DEFAULT_ADMIN_ROLE) ); + + // Attempt to grant MINTER_ROLE to receiver address + // This should fail since messenger doesn't have DEFAULT_ADMIN_ROLE token.grantRole(MINTER_ROLE, receiver); } vm.stopPrank();