From d39451d082864d97e15751c14f9ed30fdac7f626 Mon Sep 17 00:00:00 2001 From: madlabman <10616301+madlabman@users.noreply.github.com> Date: Wed, 22 Nov 2023 17:24:12 +0100 Subject: [PATCH] fix: queue cleanup --- src/CSAccounting.sol | 8 +- src/CSModule.sol | 52 +++++--- src/lib/Batch.sol | 16 ++- test/Batch.t.sol | 28 ++-- test/CSModule.t.sol | 256 +++++++++++++++++++++++++++++-------- test/helpers/Utilities.sol | 10 +- 6 files changed, 280 insertions(+), 90 deletions(-) diff --git a/src/CSAccounting.sol b/src/CSAccounting.sol index d123d35c..8f27a3e8 100644 --- a/src/CSAccounting.sol +++ b/src/CSAccounting.sol @@ -88,13 +88,13 @@ contract CSAccounting is CSAccountingBase, AccessControlEnumerable { } bytes32 public constant INSTANT_PENALIZE_BOND_ROLE = - keccak256("INSTANT_PENALIZE_BOND_ROLE"); + keccak256("INSTANT_PENALIZE_BOND_ROLE"); // 0x9909cf24c2d3bafa8c229558d86a1b726ba57c3ef6350848dcf434a4181b56c7 bytes32 public constant EL_REWARDS_STEALING_PENALTY_INIT_ROLE = - keccak256("EL_REWARDS_STEALING_PENALTY_INIT_ROLE"); + keccak256("EL_REWARDS_STEALING_PENALTY_INIT_ROLE"); // 0xcc2e7ce7be452f766dd24d55d87a3d42901c31ffa5b600cd1dff475abec91c1f bytes32 public constant EL_REWARDS_STEALING_PENALTY_SETTLE_ROLE = - keccak256("EL_REWARDS_STEALING_PENALTY_SETTLE_ROLE"); + keccak256("EL_REWARDS_STEALING_PENALTY_SETTLE_ROLE"); // 0xdf6226649a1ca132f86d419e46892001284368a8f7445b5eb0d3fadf91329fe6 bytes32 public constant SET_BOND_MULTIPLIER_ROLE = - keccak256("SET_BOND_MULTIPLIER_ROLE"); + keccak256("SET_BOND_MULTIPLIER_ROLE"); // 0x62131145aee19b18b85aa8ead52ba87f0efb6e61e249155edc68a2c24e8f79b5 // todo: should be reconsidered uint256 public constant MIN_BLOCKED_BOND_RETENTION_PERIOD = 4 weeks; diff --git a/src/CSModule.sol b/src/CSModule.sol index b34e9fd9..8a19e3e1 100644 --- a/src/CSModule.sol +++ b/src/CSModule.sol @@ -33,6 +33,7 @@ struct NodeOperator { uint256 stuckValidatorsCount; uint256 refundedValidatorsCount; bool isTargetLimitActive; + uint256 queueNonce; } struct NodeOperatorInfo { @@ -107,9 +108,9 @@ contract CSModuleBase { contract CSModule is IStakingModule, CSModuleBase { using QueueLib for QueueLib.Queue; - // @dev max number of node operators is limited by uint128 due to Batch serialization in 32 bytes + // @dev max number of node operators is limited by uint64 due to Batch serialization in 32 bytes // it seems to be enough - uint128 public constant MAX_NODE_OPERATORS_COUNT = type(uint128).max; + uint128 public constant MAX_NODE_OPERATORS_COUNT = type(uint64).max; bytes32 public constant SIGNING_KEYS_POSITION = keccak256("lido.CommunityStakingModule.signingKeysPosition"); @@ -645,14 +646,13 @@ contract CSModule is IStakingModule, CSModuleBase { if (vetKeysPointer > no.totalAddedKeys) revert InvalidVetKeysPointer(); uint64 count = SafeCast.toUint64(vetKeysPointer - no.totalVettedKeys); - uint64 start = SafeCast.toUint64( - no.totalVettedKeys == 0 ? 0 : no.totalVettedKeys - ); + uint64 start = SafeCast.toUint64(no.totalVettedKeys); bytes32 pointer = Batch.serialize({ - nodeOperatorId: SafeCast.toUint128(nodeOperatorId), + nodeOperatorId: SafeCast.toUint64(nodeOperatorId), start: start, - count: count + count: count, + nonce: SafeCast.toUint64(no.queueNonce) }); no.totalVettedKeys = vetKeysPointer; @@ -682,6 +682,7 @@ contract CSModule is IStakingModule, CSModuleBase { function _unvetKeys(uint256 nodeOperatorId) internal { NodeOperator storage no = _nodeOperators[nodeOperatorId]; no.totalVettedKeys = no.totalDepositedKeys; + no.queueNonce++; emit VettedSigningKeysCountChanged(nodeOperatorId, no.totalVettedKeys); _incrementNonce(); } @@ -754,6 +755,7 @@ contract CSModule is IStakingModule, CSModuleBase { _totalDepositedValidators += keysCount; NodeOperator storage no = _nodeOperators[nodeOperatorId]; no.totalDepositedKeys += keysCount; + // redundant check, enforced by _assertIsValidBatch require( no.totalDepositedKeys <= no.totalVettedKeys, "too many keys" @@ -789,11 +791,12 @@ contract CSModule is IStakingModule, CSModuleBase { { uint256 start; uint256 count; + uint256 nonce; - (nodeOperatorId, start, count) = Batch.deserialize(batch); + (nodeOperatorId, start, count, nonce) = Batch.deserialize(batch); NodeOperator storage no = _nodeOperators[nodeOperatorId]; - _assertIsValidBatch(no, start, count); + _assertIsValidBatch(no, start, count, nonce); startIndex = Math.max(start, no.totalDepositedKeys); depositableKeysCount = start + count - startIndex; @@ -802,9 +805,11 @@ contract CSModule is IStakingModule, CSModuleBase { function _assertIsValidBatch( NodeOperator storage no, uint256 start, - uint256 count + uint256 count, + uint256 nonce ) internal view { require(count != 0, "Empty batch given"); + require(nonce == no.queueNonce, "Invalid batch nonce"); require( _unvettedKeysInBatch(no, start, count) == false, "Batch contains unvetted keys" @@ -836,11 +841,18 @@ contract CSModule is IStakingModule, CSModuleBase { break; } - (uint256 nodeOperatorId, uint256 start, uint256 count) = Batch - .deserialize(item); + ( + uint256 nodeOperatorId, + uint256 start, + uint256 count, + uint256 nonce + ) = Batch.deserialize(item); NodeOperator storage no = _nodeOperators[nodeOperatorId]; - if (_unvettedKeysInBatch(no, start, count)) { + if ( + _unvettedKeysInBatch(no, start, count) || nonce != no.queueNonce + ) { queue.remove(pointer, item); + continue; } pointer = item; @@ -871,7 +883,7 @@ contract CSModule is IStakingModule, CSModuleBase { } /// @dev returns the next pointer to start check from - function isQueueHasUnvettedKeys( + function isQueueDirty( uint256 maxItems, bytes32 pointer ) external view returns (bool, bytes32) { @@ -887,10 +899,16 @@ contract CSModule is IStakingModule, CSModuleBase { break; } - (uint256 nodeOperatorId, uint256 start, uint256 count) = Batch - .deserialize(item); + ( + uint256 nodeOperatorId, + uint256 start, + uint256 count, + uint256 nonce + ) = Batch.deserialize(item); NodeOperator storage no = _nodeOperators[nodeOperatorId]; - if (_unvettedKeysInBatch(no, start, count)) { + if ( + _unvettedKeysInBatch(no, start, count) || nonce != no.queueNonce + ) { return (true, pointer); } diff --git a/src/lib/Batch.sol b/src/lib/Batch.sol index 664f88f7..cc7c3712 100644 --- a/src/lib/Batch.sol +++ b/src/lib/Batch.sol @@ -6,21 +6,23 @@ pragma solidity 0.8.21; library Batch { /// @notice Serialize node operator id, batch start and count of keys into a single bytes32 value function serialize( - uint128 nodeOperatorId, + uint64 nodeOperatorId, uint64 start, - uint64 count + uint64 count, + uint64 nonce ) internal pure returns (bytes32 s) { - return bytes32(abi.encodePacked(nodeOperatorId, start, count)); + return bytes32(abi.encodePacked(nodeOperatorId, start, count, nonce)); } /// @notice Deserialize node operator id, batch start and count of keys from a single bytes32 value function deserialize( bytes32 b - ) internal pure returns (uint128 nodeOperatorId, uint64 start, uint64 count) { + ) internal pure returns (uint64 nodeOperatorId, uint64 start, uint64 count, uint64 nonce) { assembly { - nodeOperatorId := shr(128, b) - start := shr(64, b) - count := b + nodeOperatorId := shr(192, b) + start := shr(128, b) + count := shr(64, b) + nonce := b } } diff --git a/test/Batch.t.sol b/test/Batch.t.sol index a5a197ad..cc93af46 100644 --- a/test/Batch.t.sol +++ b/test/Batch.t.sol @@ -11,44 +11,52 @@ contract BatchTest is Test { bytes32 b = Batch.serialize({ nodeOperatorId: 999, start: 3, - count: 42 + count: 42, + nonce: 7 }); assertEq( b, - // noIndex | start | count | - 0x000000000000000000000000000003e70000000000000003000000000000002a + // noIndex | start | count | nonce + 0x00000000000003e70000000000000003000000000000002a0000000000000007 ); } function test_deserialize() public { - (uint128 nodeOperatorId, uint64 start, uint64 count) = Batch - .deserialize( + ( + uint256 nodeOperatorId, + uint256 start, + uint256 count, + uint256 nonce + ) = Batch.deserialize( 0x0000000000000000000000000000000000000000000000000000000000000000 ); assertEq(nodeOperatorId, 0, "nodeOperatorId != 0"); assertEq(start, 0, "start != 0"); assertEq(count, 0, "count != 0"); + assertEq(nonce, 0, "nonce != 0"); - (nodeOperatorId, start, count) = Batch.deserialize( - 0x000000000000000000000000000003e70000000000000003000000000000002a + (nodeOperatorId, start, count, nonce) = Batch.deserialize( + 0x00000000000003e70000000000000003000000000000002a0000000000000007 ); assertEq(nodeOperatorId, 999, "nodeOperatorId != 999"); assertEq(start, 3, "start != 3"); assertEq(count, 42, "count != 42"); + assertEq(nonce, 7, "nonce != 7"); - (nodeOperatorId, start, count) = Batch.deserialize( + (nodeOperatorId, start, count, nonce) = Batch.deserialize( 0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff ); assertEq( nodeOperatorId, - type(uint128).max, - "nodeOperatorId != uint128.max" + type(uint64).max, + "nodeOperatorId != uint64.max" ); assertEq(start, type(uint64).max, "start != uint64.max"); assertEq(count, type(uint64).max, "count != uint64.max"); + assertEq(nonce, type(uint64).max, "nonce != uint64.max"); } } diff --git a/test/CSModule.t.sol b/test/CSModule.t.sol index 17a94578..8c088613 100644 --- a/test/CSModule.t.sol +++ b/test/CSModule.t.sol @@ -13,7 +13,18 @@ import "./helpers/mocks/LidoMock.sol"; import "./helpers/mocks/WstETHMock.sol"; import "./helpers/Utilities.sol"; +import { Strings } from "@openzeppelin/contracts/utils/Strings.sol"; + contract CSMCommon is Test, Fixtures, Utilities, CSModuleBase { + using Strings for uint256; + + struct BatchInfo { + uint256 nodeOperatorId; + uint256 start; + uint256 count; + uint256 nonce; + } + LidoLocatorMock public locator; WstETHMock public wstETH; LidoMock public stETH; @@ -22,16 +33,16 @@ contract CSMCommon is Test, Fixtures, Utilities, CSModuleBase { CSAccounting public accounting; CommunityStakingFeeDistributorMock public communityStakingFeeDistributor; + address internal admin; address internal stranger; - address internal alice; address internal nodeOperator; function setUp() public { - alice = address(1); - nodeOperator = address(2); - stranger = address(3); - address[] memory penalizeRoleMembers = new address[](1); - penalizeRoleMembers[0] = alice; + vm.label(address(this), "TEST"); + + nodeOperator = nextAddress("NODE_OPERATOR"); + stranger = nextAddress("STRANGER"); + admin = nextAddress("ADMIN"); (locator, wstETH, stETH, burner) = initLido(); @@ -46,7 +57,7 @@ contract CSMCommon is Test, Fixtures, Utilities, CSModuleBase { csm = new CSModule("community-staking-module", address(locator)); accounting = new CSAccounting( 2 ether, - alice, + admin, address(locator), address(wstETH), address(csm), @@ -54,6 +65,13 @@ contract CSMCommon is Test, Fixtures, Utilities, CSModuleBase { 1 days ); csm.setAccounting(address(accounting)); + + vm.startPrank(admin); + accounting.grantRole( + accounting.INSTANT_PENALIZE_BOND_ROLE(), + address(csm) + ); // NOTE: required because of `unvetKeys` + vm.stopPrank(); } function createNodeOperator() internal returns (uint256) { @@ -80,6 +98,66 @@ contract CSMCommon is Test, Fixtures, Utilities, CSModuleBase { ); return csm.getNodeOperatorsCount() - 1; } + + function _assertQueueState(BatchInfo[] memory exp) internal { + // (bytes32 pointer,) = csm.queue(); // it works, but how? + bytes32 pointer = bytes32(0); + + for (uint256 i = 0; i < exp.length; i++) { + BatchInfo memory b = exp[i]; + + assertFalse( + _isLastElementInQueue(pointer), + string.concat("unexpected end of queue at index ", i.toString()) + ); + + pointer = _nextPointer(pointer); + ( + uint256 nodeOperatorId, + uint256 start, + uint256 count, + uint256 nonce + ) = Batch.deserialize(pointer); + + assertEq( + nodeOperatorId, + b.nodeOperatorId, + string.concat( + "unexpected `nodeOperatorId` at index ", + i.toString() + ) + ); + assertEq( + start, + b.start, + string.concat("unexpected `start` at index ", i.toString()) + ); + assertEq( + count, + b.count, + string.concat("unexpected `count` at index ", i.toString()) + ); + assertEq( + nonce, + b.nonce, + string.concat("unexpected `nonce` at index ", i.toString()) + ); + } + + assertTrue(_isLastElementInQueue(pointer), "unexpected tail of queue"); + } + + function _isLastElementInQueue( + bytes32 pointer + ) internal view returns (bool) { + bytes32 next = _nextPointer(pointer); + return next == pointer; + } + + function _nextPointer(bytes32 pointer) internal view returns (bytes32) { + (, bytes32 next, ) = csm.depositQueue(1, pointer); + return next; + } } contract CsmInitialization is CSMCommon { @@ -369,9 +447,9 @@ contract CsmProposeNodeOperatorManagerAddressChange is CSMCommon { assertEq(no.rewardAddress, nodeOperator); vm.expectEmit(true, true, false, true, address(csm)); - emit NodeOperatorManagerAddressChangeProposed(noId, alice); + emit NodeOperatorManagerAddressChangeProposed(noId, stranger); vm.prank(nodeOperator); - csm.proposeNodeOperatorManagerAddressChange(noId, alice); + csm.proposeNodeOperatorManagerAddressChange(noId, stranger); assertEq(no.managerAddress, nodeOperator); assertEq(no.rewardAddress, nodeOperator); } @@ -380,7 +458,7 @@ contract CsmProposeNodeOperatorManagerAddressChange is CSMCommon { public { vm.expectRevert(NodeOperatorDoesNotExist.selector); - csm.proposeNodeOperatorManagerAddressChange(0, alice); + csm.proposeNodeOperatorManagerAddressChange(0, stranger); } function test_proposeNodeOperatorManagerAddressChange_RevertWhenNotManager() @@ -388,7 +466,7 @@ contract CsmProposeNodeOperatorManagerAddressChange is CSMCommon { { uint256 noId = createNodeOperator(); vm.expectRevert(SenderIsNotManagerAddress.selector); - csm.proposeNodeOperatorManagerAddressChange(noId, alice); + csm.proposeNodeOperatorManagerAddressChange(noId, stranger); } function test_proposeNodeOperatorManagerAddressChange_RevertWhenAlreadyProposed() @@ -396,11 +474,11 @@ contract CsmProposeNodeOperatorManagerAddressChange is CSMCommon { { uint256 noId = createNodeOperator(); vm.prank(nodeOperator); - csm.proposeNodeOperatorManagerAddressChange(noId, alice); + csm.proposeNodeOperatorManagerAddressChange(noId, stranger); vm.expectRevert(AlreadyProposed.selector); vm.prank(nodeOperator); - csm.proposeNodeOperatorManagerAddressChange(noId, alice); + csm.proposeNodeOperatorManagerAddressChange(noId, stranger); } function test_proposeNodeOperatorManagerAddressChange_RevertWhenSameAddressProposed() @@ -421,15 +499,15 @@ contract CsmConfirmNodeOperatorManagerAddressChange is CSMCommon { assertEq(no.rewardAddress, nodeOperator); vm.prank(nodeOperator); - csm.proposeNodeOperatorManagerAddressChange(noId, alice); + csm.proposeNodeOperatorManagerAddressChange(noId, stranger); vm.expectEmit(true, true, true, true, address(csm)); - emit NodeOperatorManagerAddressChanged(noId, nodeOperator, alice); - vm.prank(alice); + emit NodeOperatorManagerAddressChanged(noId, nodeOperator, stranger); + vm.prank(stranger); csm.confirmNodeOperatorManagerAddressChange(noId); no = csm.getNodeOperator(noId); - assertEq(no.managerAddress, alice); + assertEq(no.managerAddress, stranger); assertEq(no.rewardAddress, nodeOperator); } @@ -445,7 +523,7 @@ contract CsmConfirmNodeOperatorManagerAddressChange is CSMCommon { { uint256 noId = createNodeOperator(); vm.expectRevert(SenderIsNotProposedAddress.selector); - vm.prank(alice); + vm.prank(stranger); csm.confirmNodeOperatorManagerAddressChange(noId); } @@ -457,7 +535,7 @@ contract CsmConfirmNodeOperatorManagerAddressChange is CSMCommon { csm.proposeNodeOperatorManagerAddressChange(noId, stranger); vm.expectRevert(SenderIsNotProposedAddress.selector); - vm.prank(alice); + vm.prank(nextAddress()); csm.confirmNodeOperatorManagerAddressChange(noId); } } @@ -470,9 +548,9 @@ contract CsmProposeNodeOperatorRewardAddressChange is CSMCommon { assertEq(no.rewardAddress, nodeOperator); vm.expectEmit(true, true, false, true, address(csm)); - emit NodeOperatorRewardAddressChangeProposed(noId, alice); + emit NodeOperatorRewardAddressChangeProposed(noId, stranger); vm.prank(nodeOperator); - csm.proposeNodeOperatorRewardAddressChange(noId, alice); + csm.proposeNodeOperatorRewardAddressChange(noId, stranger); assertEq(no.managerAddress, nodeOperator); assertEq(no.rewardAddress, nodeOperator); } @@ -481,7 +559,7 @@ contract CsmProposeNodeOperatorRewardAddressChange is CSMCommon { public { vm.expectRevert(NodeOperatorDoesNotExist.selector); - csm.proposeNodeOperatorRewardAddressChange(0, alice); + csm.proposeNodeOperatorRewardAddressChange(0, stranger); } function test_proposeNodeOperatorRewardAddressChange_RevertWhenNotRewardAddress() @@ -489,7 +567,7 @@ contract CsmProposeNodeOperatorRewardAddressChange is CSMCommon { { uint256 noId = createNodeOperator(); vm.expectRevert(SenderIsNotRewardAddress.selector); - csm.proposeNodeOperatorRewardAddressChange(noId, alice); + csm.proposeNodeOperatorRewardAddressChange(noId, stranger); } function test_proposeNodeOperatorRewardAddressChange_RevertWhenAlreadyProposed() @@ -497,11 +575,11 @@ contract CsmProposeNodeOperatorRewardAddressChange is CSMCommon { { uint256 noId = createNodeOperator(); vm.prank(nodeOperator); - csm.proposeNodeOperatorRewardAddressChange(noId, alice); + csm.proposeNodeOperatorRewardAddressChange(noId, stranger); vm.expectRevert(AlreadyProposed.selector); vm.prank(nodeOperator); - csm.proposeNodeOperatorRewardAddressChange(noId, alice); + csm.proposeNodeOperatorRewardAddressChange(noId, stranger); } function test_proposeNodeOperatorRewardAddressChange_RevertWhenSameAddressProposed() @@ -522,16 +600,16 @@ contract CsmConfirmNodeOperatorRewardAddressChange is CSMCommon { assertEq(no.rewardAddress, nodeOperator); vm.prank(nodeOperator); - csm.proposeNodeOperatorRewardAddressChange(noId, alice); + csm.proposeNodeOperatorRewardAddressChange(noId, stranger); vm.expectEmit(true, true, true, true, address(csm)); - emit NodeOperatorRewardAddressChanged(noId, nodeOperator, alice); - vm.prank(alice); + emit NodeOperatorRewardAddressChanged(noId, nodeOperator, stranger); + vm.prank(stranger); csm.confirmNodeOperatorRewardAddressChange(noId); no = csm.getNodeOperator(noId); assertEq(no.managerAddress, nodeOperator); - assertEq(no.rewardAddress, alice); + assertEq(no.rewardAddress, stranger); } function test_confirmNodeOperatorRewardAddressChange_RevertWhenNoNodeOperator() @@ -546,7 +624,7 @@ contract CsmConfirmNodeOperatorRewardAddressChange is CSMCommon { { uint256 noId = createNodeOperator(); vm.expectRevert(SenderIsNotProposedAddress.selector); - vm.prank(alice); + vm.prank(stranger); csm.confirmNodeOperatorRewardAddressChange(noId); } @@ -558,7 +636,7 @@ contract CsmConfirmNodeOperatorRewardAddressChange is CSMCommon { csm.proposeNodeOperatorRewardAddressChange(noId, stranger); vm.expectRevert(SenderIsNotProposedAddress.selector); - vm.prank(alice); + vm.prank(nextAddress()); csm.confirmNodeOperatorRewardAddressChange(noId); } } @@ -568,18 +646,18 @@ contract CsmResetNodeOperatorManagerAddress is CSMCommon { uint256 noId = createNodeOperator(); vm.prank(nodeOperator); - csm.proposeNodeOperatorRewardAddressChange(noId, alice); - vm.prank(alice); + csm.proposeNodeOperatorRewardAddressChange(noId, stranger); + vm.prank(stranger); csm.confirmNodeOperatorRewardAddressChange(noId); vm.expectEmit(true, true, true, true, address(csm)); - emit NodeOperatorManagerAddressChanged(noId, nodeOperator, alice); - vm.prank(alice); + emit NodeOperatorManagerAddressChanged(noId, nodeOperator, stranger); + vm.prank(stranger); csm.resetNodeOperatorManagerAddress(noId); NodeOperatorInfo memory no = csm.getNodeOperator(noId); - assertEq(no.managerAddress, alice); - assertEq(no.rewardAddress, alice); + assertEq(no.managerAddress, stranger); + assertEq(no.rewardAddress, stranger); } function test_resetNodeOperatorManagerAddress_RevertWhenNoNodeOperator() @@ -618,13 +696,15 @@ contract CsmVetKeys is CSMCommon { NodeOperatorInfo memory no = csm.getNodeOperator(noId); assertEq(no.totalVettedValidators, 1); - (bytes32[] memory items, , ) = csm.depositQueue(1, bytes32(0)); - (uint128 batchNoId, uint64 start, uint64 count) = Batch.deserialize( - items[0] - ); - assertEq(batchNoId, uint128(noId)); - assertEq(start, 0); - assertEq(count, 1); + + BatchInfo[] memory exp = new BatchInfo[](1); + exp[0] = BatchInfo({ + nodeOperatorId: noId, + start: 0, + count: 1, + nonce: 0 + }); + _assertQueueState(exp); } function test_vetKeys_totalVettedKeysIsNotZero() public { @@ -639,13 +719,21 @@ contract CsmVetKeys is CSMCommon { NodeOperatorInfo memory no = csm.getNodeOperator(noId); assertEq(no.totalVettedValidators, 2); - (bytes32[] memory items, , ) = csm.depositQueue(2, bytes32(0)); - (uint128 batchNoId, uint64 start, uint64 count) = Batch.deserialize( - items[1] - ); - assertEq(batchNoId, uint128(noId)); - assertEq(start, 1); - assertEq(count, 1); + + BatchInfo[] memory exp = new BatchInfo[](2); + exp[0] = BatchInfo({ + nodeOperatorId: noId, + start: 0, + count: 1, + nonce: 0 + }); + exp[1] = BatchInfo({ + nodeOperatorId: noId, + start: 1, + count: 1, + nonce: 0 + }); + _assertQueueState(exp); } function test_vetKeys_RevertWhenNoNodeOperator() public { @@ -667,3 +755,69 @@ contract CsmVetKeys is CSMCommon { csm.vetKeys(noId, 2); } } + +contract CsmQueueOps is CSMCommon { + function test_queueIsCleanByDefault() public { + createNodeOperator({ keysCount: 2 }); + csm.vetKeys(0, 1); + + (bool isDirty /* next */, ) = csm.isQueueDirty(1, bytes32(0)); + assertFalse(isDirty, "queue should be clean"); + } + + function test_queueIsDirty_WhenUnvettedKeys() public { + createNodeOperator({ keysCount: 2 }); + csm.vetKeys(0, 1); + csm.unvetKeys(0); + + (bool isDirty /* next */, ) = csm.isQueueDirty(1, bytes32(0)); + assertTrue(isDirty, "queue should be dirty"); + } + + function test_queueIsClean_AfterCleanup() public { + createNodeOperator({ keysCount: 2 }); + csm.vetKeys(0, 1); + csm.unvetKeys(0); + csm.cleanDepositQueue(1, bytes32(0)); + + (bool isDirty /* next */, ) = csm.isQueueDirty(1, bytes32(0)); + assertFalse(isDirty, "queue should be clean"); + } + + function test_queueIsDirty_WhenDanglingBatch() public { + createNodeOperator({ keysCount: 2 }); + + csm.vetKeys(0, 1); + csm.vetKeys(0, 2); + csm.unvetKeys(0); + csm.vetKeys(0, 2); + + // let's check the state of the queue + BatchInfo[] memory exp = new BatchInfo[](3); + exp[0] = BatchInfo({ nodeOperatorId: 0, start: 0, count: 1, nonce: 0 }); + exp[1] = BatchInfo({ nodeOperatorId: 0, start: 1, count: 1, nonce: 0 }); + exp[2] = BatchInfo({ nodeOperatorId: 0, start: 0, count: 2, nonce: 1 }); + _assertQueueState(exp); + + (bool isDirty /* next */, ) = csm.isQueueDirty(3, bytes32(0)); + assertTrue(isDirty, "queue should be dirty"); + } + + function test_queueIsClean_WhenDanglingBatchCleanedUp() public { + createNodeOperator({ keysCount: 2 }); + + csm.vetKeys(0, 1); + csm.vetKeys(0, 2); + csm.unvetKeys(0); + csm.vetKeys(0, 2); + + csm.cleanDepositQueue(3, bytes32(0)); + // let's check the state of the queue + BatchInfo[] memory exp = new BatchInfo[](1); + exp[0] = BatchInfo({ nodeOperatorId: 0, start: 0, count: 2, nonce: 1 }); + _assertQueueState(exp); + + (bool isDirty /* next */, ) = csm.isQueueDirty(3, bytes32(0)); + assertFalse(isDirty, "queue should be clean"); + } +} diff --git a/test/helpers/Utilities.sol b/test/helpers/Utilities.sol index cbd01853..73695604 100644 --- a/test/helpers/Utilities.sol +++ b/test/helpers/Utilities.sol @@ -2,8 +2,10 @@ // SPDX-License-Identifier: GPL-3.0 pragma solidity 0.8.21; +import { CommonBase } from "forge-std/Base.sol"; + /// @author madlabman -contract Utilities { +contract Utilities is CommonBase { bytes32 internal seed = keccak256("seed sEed seEd"); function nextAddress() internal returns (address) { @@ -14,6 +16,12 @@ contract Utilities { return a; } + function nextAddress(string memory label) internal returns (address) { + address a = nextAddress(); + vm.label(a, label); + return a; + } + function keysSignatures( uint256 keysCount ) public pure returns (bytes memory, bytes memory) {