Skip to content

Commit

Permalink
removed target limit setting in case of stuck keys in favor of additi…
Browse files Browse the repository at this point in the history 
…onal check in vetKeys
  • Loading branch information
@skhomuti
skhomuti committed Nov 29, 2023
1 parent 7ed3732 commit d17247f
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 170 deletions.
58 changes: 17 additions & 41 deletions src/CSModule.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,6 @@ struct NodeOperator {
bool active;
uint256 targetLimit;
bool isTargetLimitActive;
uint256 targetLimitForUnstuck;
bool isTargetLimitActiveForUnstuck;
uint256 stuckPenaltyEndTimestamp;
uint256 totalExitedKeys;
uint256 totalAddedKeys;
Expand Down Expand Up @@ -114,8 +112,9 @@ contract CSModuleBase {
error SameAddress();
error AlreadyProposed();
error InvalidVetKeysPointer();
error TargetLimitExceeded();
error StuckKeysPresent();
error InvalidTargetLimit();
error IncreasingTargetLimitWhenStuckKeys();
error StuckKeysHigherThanTotalDeposited();

error QueueLookupNoLimit();
Expand Down Expand Up @@ -623,7 +622,7 @@ contract CSModule is IStakingModule, CSModuleBase {
}

// @notice update stuck validators count by StakingRouter
// Presence of stuck validators leads to setting target limit to total deposited keys
// Presence of stuck validators leads to stop vetting for the node operator
// to prevent further deposits and clean batches from the deposit queue.
// @param nodeOperatorIds - bytes packed array of node operator ids
// @param stuckValidatorsCounts - bytes packed array of stuck validators counts
Expand Down Expand Up @@ -654,30 +653,6 @@ contract CSModule is IStakingModule, CSModuleBase {
if (stuckValidatorsCount == no.stuckValidatorsCount) continue;

no.stuckValidatorsCount = stuckValidatorsCount;

if (stuckValidatorsCount > 0) {
// @dev the oracle can decrease target limit but can't increase it
// after unstuck, target limit will be set to previous value
no.isTargetLimitActiveForUnstuck = no.isTargetLimitActive;
no.targetLimitForUnstuck = no.targetLimit;
if (
!no.isTargetLimitActive ||
no.targetLimit > no.totalDepositedKeys
) {
_setTargetLimit(
nodeOperatorId,
true,
no.totalDepositedKeys
);
}
} else {
_setTargetLimit(
nodeOperatorId,
no.isTargetLimitActiveForUnstuck,
no.targetLimitForUnstuck
);
}

emit StuckSigningKeysCountChanged(
nodeOperatorId,
stuckValidatorsCount
Expand Down Expand Up @@ -710,20 +685,13 @@ contract CSModule is IStakingModule, CSModuleBase {
uint256 targetLimit
) external onlyExistingNodeOperator(nodeOperatorId) onlyStakingRouter {
// TODO sanity checks?
if (_nodeOperators[nodeOperatorId].stuckValidatorsCount > 0) {
_nodeOperators[nodeOperatorId]
.isTargetLimitActiveForUnstuck = isTargetLimitActive;
_nodeOperators[nodeOperatorId].targetLimitForUnstuck = targetLimit;
}
_setTargetLimit(nodeOperatorId, isTargetLimitActive, targetLimit);
_incrementModuleNonce();
}

// @notice update target limits with event emission
// target limit decreasing (or appearing) must unvet node operator's keys from the queue
// @dev it's not expected (yet) that target limit can be disabled with some value.
// in case of stuck keys, the previous limit stored in the targetLimitForUnstuck field
// hence, don't have to store non-zero value in the targetLimit field
function _setTargetLimit(
uint256 nodeOperatorId,
bool isTargetLimitActive,
Expand All @@ -734,8 +702,6 @@ contract CSModule is IStakingModule, CSModuleBase {
revert InvalidTargetLimit();

NodeOperator storage no = _nodeOperators[nodeOperatorId];
if (no.stuckValidatorsCount > 0 && targetLimit > no.totalDepositedKeys)
revert IncreasingTargetLimitWhenStuckKeys();

if (
no.isTargetLimitActive == isTargetLimitActive &&
Expand Down Expand Up @@ -777,10 +743,7 @@ contract CSModule is IStakingModule, CSModuleBase {
if (vetKeysPointer <= no.totalVettedKeys)
revert InvalidVetKeysPointer();
if (vetKeysPointer > no.totalAddedKeys) revert InvalidVetKeysPointer();
if (
no.isTargetLimitActive &&
vetKeysPointer > (no.totalExitedKeys + no.targetLimit)
) revert InvalidVetKeysPointer();
_validateVetKeys(nodeOperatorId, vetKeysPointer);

uint64 count = SafeCast.toUint64(vetKeysPointer - no.totalVettedKeys);
uint64 start = SafeCast.toUint64(no.totalVettedKeys);
Expand All @@ -801,6 +764,19 @@ contract CSModule is IStakingModule, CSModuleBase {
_incrementModuleNonce();
}

function _validateVetKeys(
uint256 nodeOperatorId,
uint64 vetKeysPointer
) internal view {
NodeOperator storage no = _nodeOperators[nodeOperatorId];

if (
no.isTargetLimitActive &&
vetKeysPointer > (no.totalExitedKeys + no.targetLimit)
) revert TargetLimitExceeded();
if (no.stuckValidatorsCount > 0) revert StuckKeysPresent();
}

function unvetKeys(
uint256 nodeOperatorId
)
Expand Down
143 changes: 14 additions & 129 deletions test/CSModule.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -807,7 +807,20 @@ contract CsmVetKeys is CSMCommon {
csm.vetKeys(noId, 1);
csm.updateTargetValidatorsLimits(noId, true, 1);

vm.expectRevert(InvalidVetKeysPointer.selector);
vm.expectRevert(TargetLimitExceeded.selector);
csm.vetKeys(noId, 2);
}

function test_vetKeys_RevertWhenStuckKeysPresent() public {
uint256 noId = createNodeOperator(2);
csm.vetKeys(noId, 1);
csm.obtainDepositData(1, "");
csm.updateStuckValidatorsCount(
bytes.concat(bytes8(0x0000000000000000)),
bytes.concat(bytes16(0x00000000000000000000000000000001))
);

vm.expectRevert(StuckKeysPresent.selector);
csm.vetKeys(noId, 2);
}
}
Expand Down Expand Up @@ -1123,20 +1136,6 @@ contract CsmUpdateTargetValidatorsLimits is CSMCommon {
vm.expectRevert(InvalidTargetLimit.selector);
csm.updateTargetValidatorsLimits(noId, false, 10);
}

function test_updateTargetValidatorsLimits_RevertWhenTargetLimitIsGreaterThanDepositedWhenStuck()
public
{
uint256 noId = createNodeOperator(2);
csm.vetKeys(noId, 2);
csm.obtainDepositData(2, "");
csm.updateStuckValidatorsCount(
bytes.concat(bytes8(0x0000000000000000)),
bytes.concat(bytes16(0x00000000000000000000000000000002))
);
vm.expectRevert(IncreasingTargetLimitWhenStuckKeys.selector);
csm.updateTargetValidatorsLimits(noId, true, 4);
}
}

contract CsmUpdateStuckValidatorsCount is CSMCommon {
Expand All @@ -1145,8 +1144,6 @@ contract CsmUpdateStuckValidatorsCount is CSMCommon {
csm.vetKeys(noId, 3);
csm.obtainDepositData(1, "");

vm.expectEmit(true, true, true, true, address(csm));
emit TargetValidatorsCountChanged(noId, true, 1);
vm.expectEmit(true, true, false, true, address(csm));
emit StuckSigningKeysCountChanged(noId, 1);
csm.updateStuckValidatorsCount(
Expand All @@ -1160,12 +1157,6 @@ contract CsmUpdateStuckValidatorsCount is CSMCommon {
1,
"stuckValidatorsCount not increased"
);
assertEq(
summary.targetValidatorsCount,
summary.totalDepositedValidators,
"targetValidatorsCount should be limited to totalDepositedValidators"
);
assertTrue(summary.isTargetLimitActive, "isTargetLimitActive is false");
}

function test_updateStuckValidatorsCount_Unstuck() public {
Expand All @@ -1178,8 +1169,6 @@ contract CsmUpdateStuckValidatorsCount is CSMCommon {
bytes.concat(bytes16(0x00000000000000000000000000000001))
);

vm.expectEmit(true, true, true, true, address(csm));
emit TargetValidatorsCountChanged(noId, false, 0);
vm.expectEmit(true, true, false, true, address(csm));
emit StuckSigningKeysCountChanged(noId, 0);
csm.updateStuckValidatorsCount(
Expand All @@ -1192,94 +1181,6 @@ contract CsmUpdateStuckValidatorsCount is CSMCommon {
0,
"stuckValidatorsCount should be zero"
);
assertEq(
summary.targetValidatorsCount,
0,
"targetValidatorsCount should be zero"
);
assertFalse(
summary.isTargetLimitActive,
"isTargetLimitActive should be false"
);
}

function test_updateStuckValidatorsCount_DecreaseLimit() public {
uint256 noId = createNodeOperator();
csm.vetKeys(noId, 1);
csm.obtainDepositData(1, "");
csm.updateTargetValidatorsLimits(noId, true, 2);

vm.expectEmit(true, true, true, true, address(csm));
emit TargetValidatorsCountChanged(noId, true, 1);
csm.updateStuckValidatorsCount(
bytes.concat(bytes8(0x0000000000000000)),
bytes.concat(bytes16(0x00000000000000000000000000000001))
);
}

function test_updateStuckValidatorsCount_ReturnToPrevLimit() public {
uint256 noId = createNodeOperator();
csm.vetKeys(noId, 1);
csm.obtainDepositData(1, "");
csm.updateTargetValidatorsLimits(noId, true, 2);

csm.updateStuckValidatorsCount(
bytes.concat(bytes8(0x0000000000000000)),
bytes.concat(bytes16(0x00000000000000000000000000000001))
);

vm.expectEmit(true, true, true, true, address(csm));
emit TargetValidatorsCountChanged(noId, true, 2);
csm.updateStuckValidatorsCount(
bytes.concat(bytes8(0x0000000000000000)),
bytes.concat(bytes16(0x00000000000000000000000000000000))
);
NodeOperatorSummary memory summary = getNodeOperatorSummary(noId);
assertEq(
summary.stuckValidatorsCount,
0,
"stuckValidatorsCount should be zero"
);
assertEq(
summary.targetValidatorsCount,
2,
"targetValidatorsCount should return to prev value"
);
assertTrue(
summary.isTargetLimitActive,
"isTargetLimitActive should remain true"
);
}

function test_updateStuckValidatorsCount_ReturnToPrevZeroLimit() public {
uint256 noId = createNodeOperator();
csm.vetKeys(noId, 1);
csm.obtainDepositData(1, "");
csm.updateTargetValidatorsLimits(noId, true, 0);

csm.updateStuckValidatorsCount(
bytes.concat(bytes8(0x0000000000000000)),
bytes.concat(bytes16(0x00000000000000000000000000000001))
);

vm.recordLogs();
csm.updateStuckValidatorsCount(
bytes.concat(bytes8(0x0000000000000000)),
bytes.concat(bytes16(0x00000000000000000000000000000000))
);
Vm.Log[] memory logs = vm.getRecordedLogs();
assertEq(logs.length, 1);

NodeOperatorSummary memory summary = getNodeOperatorSummary(noId);
assertEq(
summary.targetValidatorsCount,
0,
"targetValidatorsCount should not change"
);
assertTrue(
summary.isTargetLimitActive,
"isTargetLimitActive should remain true"
);
}

function test_updateStuckValidatorsCount_RevertWhenNoNodeOperator() public {
Expand Down Expand Up @@ -1330,20 +1231,4 @@ contract CsmUpdateStuckValidatorsCount is CSMCommon {
Vm.Log[] memory logs = vm.getRecordedLogs();
assertEq(logs.length, 0);
}

function test_updateStuckValidatorsCount_NoEventWhenLimitSame() public {
uint256 noId = createNodeOperator();
csm.vetKeys(noId, 1);
csm.obtainDepositData(1, "");
csm.updateTargetValidatorsLimits(noId, true, 1);

vm.recordLogs();
csm.updateStuckValidatorsCount(
bytes.concat(bytes8(0x0000000000000000)),
bytes.concat(bytes16(0x00000000000000000000000000000001))
);
Vm.Log[] memory logs = vm.getRecordedLogs();
// only StuckSigningKeysCountChanged
assertEq(logs.length, 1);
}
}

0 comments on commit d17247f

Please sign in to comment.