Skip to content

Commit

Permalink
fix: queue cleanup
Browse files Browse the repository at this point in the history
  • Loading branch information
@madlabman
madlabman committed Nov 22, 2023
1 parent 0fe17c3 commit 20ab749
Show file tree
Hide file tree
Showing 6 changed files with 280 additions and 90 deletions.
8 changes: 4 additions & 4 deletions src/CSAccounting.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
52 changes: 35 additions & 17 deletions src/CSModule.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ struct NodeOperator {
uint256 stuckValidatorsCount;
uint256 refundedValidatorsCount;
bool isTargetLimitActive;
uint256 queueNonce;
}

struct NodeOperatorInfo {
Expand Down Expand Up @@ -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");

Expand Down Expand Up @@ -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: no.queueNonce
});

no.totalVettedKeys = vetKeysPointer;
Expand Down Expand Up @@ -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();
}
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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;
Expand All @@ -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"
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand All @@ -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);
}

Expand Down
16 changes: 9 additions & 7 deletions src/lib/Batch.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}

Expand Down
28 changes: 18 additions & 10 deletions test/Batch.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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");
}
}
Loading

0 comments on commit 20ab749

Please sign in to comment.