Skip to content

Commit 4826551

Browse files
skhomutiskhomuti
authored
tests: improve coverage for ejector, strikes, fee distributor (#634)
**Description** - Extend Ejector unit coverage (recoverer role negatives, voluntary eject offset/non-sequential indices, paused/withdrawn ranges) with per-key withdrawal tracking in the mock. - Expand ValidatorStrikes proof tests using shared helpers for value-per-key forwarding, threshold/invalid-proof cases, and tree-not-set handling. - Update FeeDistributor tests with new asserts and add invalid proof coverage. **Checklist** - [x] Appropriate PR labels applied - [ ] Test coverage maintained (`just coverage`) - [x] Tests are added/updated - [ ] Documentation maintained
1 parent 595a5b0 commit 4826551

File tree

4 files changed

+436
-41
lines changed

4 files changed

+436
-41
lines changed

‎test/helpers/mocks/CSMMock.sol‎

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,8 @@ contract CSMMock is Utilities, Fixtures {
1919
NodeOperator internal mockNodeOperator;
2020
uint256 internal nodeOperatorsCount;
2121
uint256 internal nodeOperatorTotalDepositedKeys;
22-
bool internal isValidatorWithdrawnMock;
22+
mapping(uint256 => mapping(uint256 => bool))
23+
internal isValidatorWithdrawnByKey;
2324
IAccounting public immutable ACCOUNTING;
2425
IParametersRegistry public immutable PARAMETERS_REGISTRY;
2526
LidoLocatorMock public immutable LIDO_LOCATOR;
@@ -82,15 +83,19 @@ contract CSMMock is Utilities, Fixtures {
8283
: managementProperties.rewardAddress;
8384
}
8485

85-
function mock_setIsValidatorWithdrawn(bool value) external {
86-
isValidatorWithdrawnMock = value;
86+
function mock_setIsValidatorWithdrawn(
87+
uint256 nodeOperatorId,
88+
uint256 keyIndex,
89+
bool value
90+
) external {
91+
isValidatorWithdrawnByKey[nodeOperatorId][keyIndex] = value;
8792
}
8893

8994
function isValidatorWithdrawn(
90-
uint256,
91-
uint256
95+
uint256 nodeOperatorId,
96+
uint256 keyIndex
9297
) external view returns (bool) {
93-
return isValidatorWithdrawnMock;
98+
return isValidatorWithdrawnByKey[nodeOperatorId][keyIndex];
9499
}
95100

96101
function mock_setNodeOperatorsCount(uint256 count) external {

‎test/unit/Ejector.t.sol‎

Lines changed: 151 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,26 @@ contract EjectorTestMisc is EjectorTestBase {
129129
vm.prank(address(1337));
130130
ejector.recoverEther();
131131
}
132+
133+
function test_recovererRole_revertWhen_noRole() public {
134+
bytes32 role = ejector.RECOVERER_ROLE();
135+
136+
expectRoleRevert(stranger, role);
137+
vm.prank(stranger);
138+
ejector.recoverEther();
139+
140+
expectRoleRevert(stranger, role);
141+
vm.prank(stranger);
142+
ejector.recoverERC20(address(1), 1);
143+
144+
expectRoleRevert(stranger, role);
145+
vm.prank(stranger);
146+
ejector.recoverERC721(address(1), 1);
147+
148+
expectRoleRevert(stranger, role);
149+
vm.prank(stranger);
150+
ejector.recoverERC1155(address(1), 1);
151+
}
132152
}
133153

134154
contract EjectorTestVoluntaryEject is EjectorTestBase {
@@ -214,6 +234,52 @@ contract EjectorTestVoluntaryEject is EjectorTestBase {
214234
ejector.voluntaryEject(NO_ID, keyIndex, keysCount, refundRecipient);
215235
}
216236

237+
function test_voluntaryEject_withOffset() public {
238+
uint256 startFrom = 1;
239+
uint256 keysCount = 2;
240+
bytes memory pubkeys = csm.getSigningKeys(0, startFrom, keysCount);
241+
242+
csm.mock_setNodeOperatorsCount(1);
243+
csm.mock_setNodeOperatorTotalDepositedKeys(3);
244+
csm.mock_setNodeOperatorManagementProperties(
245+
NodeOperatorManagementProperties(
246+
address(this),
247+
address(this),
248+
false
249+
)
250+
);
251+
252+
ValidatorData[] memory expectedExitsData = new ValidatorData[](
253+
keysCount
254+
);
255+
bytes[] memory emittedPubkeys = new bytes[](keysCount);
256+
for (uint256 i; i < keysCount; ++i) {
257+
bytes memory pubkey = slice(pubkeys, 48 * i, 48);
258+
expectedExitsData[i] = ValidatorData(0, NO_ID, pubkey);
259+
emittedPubkeys[i] = pubkey;
260+
}
261+
uint256 exitType = ejector.VOLUNTARY_EXIT_TYPE_ID();
262+
263+
vm.expectCall(
264+
address(twg),
265+
abi.encodeWithSelector(
266+
ITriggerableWithdrawalsGateway.triggerFullWithdrawals.selector,
267+
expectedExitsData,
268+
refundRecipient,
269+
exitType
270+
)
271+
);
272+
for (uint256 i; i < keysCount; ++i) {
273+
vm.expectEmit(address(ejector));
274+
emit IEjector.VoluntaryEjectionRequested({
275+
nodeOperatorId: NO_ID,
276+
pubkey: emittedPubkeys[i],
277+
refundRecipient: refundRecipient
278+
});
279+
}
280+
ejector.voluntaryEject(NO_ID, startFrom, keysCount, refundRecipient);
281+
}
282+
217283
function test_voluntaryEject_refund() public {
218284
uint256 keyIndex = 0;
219285
address nodeOperator = nextAddress("nodeOperator");
@@ -365,12 +431,24 @@ contract EjectorTestVoluntaryEject is EjectorTestBase {
365431
ejector.voluntaryEject(NO_ID, keyIndex, 1, address(0));
366432
}
367433

434+
function test_voluntaryEject_revertWhen_paused() public {
435+
uint256 keyIndex = 0;
436+
437+
vm.startPrank(admin);
438+
ejector.grantRole(ejector.PAUSE_ROLE(), admin);
439+
ejector.pauseFor(100);
440+
vm.stopPrank();
441+
442+
vm.expectRevert(PausableUntil.ResumedExpected.selector);
443+
ejector.voluntaryEject(NO_ID, keyIndex, 1, refundRecipient);
444+
}
445+
368446
function test_voluntaryEject_revertWhen_alreadyWithdrawn() public {
369447
uint256 keyIndex = 0;
370448

371449
csm.mock_setNodeOperatorTotalDepositedKeys(1);
372450
csm.mock_setNodeOperatorsCount(1);
373-
csm.mock_setIsValidatorWithdrawn(true);
451+
csm.mock_setIsValidatorWithdrawn(NO_ID, keyIndex, true);
374452
csm.mock_setNodeOperatorManagementProperties(
375453
NodeOperatorManagementProperties(
376454
address(this),
@@ -382,6 +460,25 @@ contract EjectorTestVoluntaryEject is EjectorTestBase {
382460
vm.expectRevert(IEjector.AlreadyWithdrawn.selector);
383461
ejector.voluntaryEject(NO_ID, keyIndex, 1, address(0));
384462
}
463+
464+
function test_voluntaryEject_revertWhen_withdrawnKeyInsideRange() public {
465+
uint256 keyIndex = 0;
466+
uint256 keysCount = 3;
467+
468+
csm.mock_setNodeOperatorTotalDepositedKeys(keysCount);
469+
csm.mock_setNodeOperatorsCount(1);
470+
csm.mock_setIsValidatorWithdrawn(NO_ID, keyIndex + 2, true);
471+
csm.mock_setNodeOperatorManagementProperties(
472+
NodeOperatorManagementProperties(
473+
address(this),
474+
address(this),
475+
false
476+
)
477+
);
478+
479+
vm.expectRevert(IEjector.AlreadyWithdrawn.selector);
480+
ejector.voluntaryEject(NO_ID, keyIndex, keysCount, refundRecipient);
481+
}
385482
}
386483

387484
contract EjectorTestVoluntaryEjectByArray is EjectorTestBase {
@@ -472,6 +569,57 @@ contract EjectorTestVoluntaryEjectByArray is EjectorTestBase {
472569
ejector.voluntaryEjectByArray(NO_ID, indices, refundRecipient);
473570
}
474571

572+
function test_voluntaryEjectByArray_nonSequentialIndices() public {
573+
uint256 keysCount = 5;
574+
bytes memory pubkeys = csm.getSigningKeys(0, 0, keysCount);
575+
576+
csm.mock_setNodeOperatorsCount(1);
577+
csm.mock_setNodeOperatorTotalDepositedKeys(keysCount);
578+
csm.mock_setNodeOperatorManagementProperties(
579+
NodeOperatorManagementProperties(
580+
address(this),
581+
address(this),
582+
false
583+
)
584+
);
585+
586+
uint256[] memory indices = new uint256[](3);
587+
indices[0] = 0;
588+
indices[1] = 2;
589+
indices[2] = 4;
590+
591+
ValidatorData[] memory expectedExitsData = new ValidatorData[](
592+
indices.length
593+
);
594+
bytes[] memory emittedPubkeys = new bytes[](indices.length);
595+
for (uint256 i; i < indices.length; ++i) {
596+
bytes memory pubkey = slice(pubkeys, 48 * indices[i], 48);
597+
expectedExitsData[i] = ValidatorData(0, NO_ID, pubkey);
598+
emittedPubkeys[i] = pubkey;
599+
}
600+
uint256 exitType = ejector.VOLUNTARY_EXIT_TYPE_ID();
601+
602+
vm.expectCall(
603+
address(twg),
604+
abi.encodeWithSelector(
605+
ITriggerableWithdrawalsGateway.triggerFullWithdrawals.selector,
606+
expectedExitsData,
607+
refundRecipient,
608+
exitType
609+
)
610+
);
611+
for (uint256 i; i < indices.length; ++i) {
612+
vm.expectEmit(address(ejector));
613+
emit IEjector.VoluntaryEjectionRequested({
614+
nodeOperatorId: NO_ID,
615+
pubkey: emittedPubkeys[i],
616+
refundRecipient: refundRecipient
617+
});
618+
}
619+
620+
ejector.voluntaryEjectByArray(NO_ID, indices, refundRecipient);
621+
}
622+
475623
function test_voluntaryEjectByArray_refund() public {
476624
uint256 keyIndex = 0;
477625
address nodeOperator = nextAddress("nodeOperator");
@@ -664,7 +812,7 @@ contract EjectorTestVoluntaryEjectByArray is EjectorTestBase {
664812

665813
csm.mock_setNodeOperatorTotalDepositedKeys(1);
666814
csm.mock_setNodeOperatorsCount(1);
667-
csm.mock_setIsValidatorWithdrawn(true);
815+
csm.mock_setIsValidatorWithdrawn(NO_ID, keyIndex, true);
668816
csm.mock_setNodeOperatorManagementProperties(
669817
NodeOperatorManagementProperties(
670818
address(this),
@@ -747,7 +895,7 @@ contract EjectorTestEjectBadPerformer is EjectorTestBase {
747895
uint256 keyIndex = 0;
748896

749897
csm.mock_setNodeOperatorTotalDepositedKeys(1);
750-
csm.mock_setIsValidatorWithdrawn(true);
898+
csm.mock_setIsValidatorWithdrawn(NO_ID, keyIndex, true);
751899

752900
vm.prank(address(strikes));
753901
vm.expectRevert(IEjector.AlreadyWithdrawn.selector);

‎test/unit/FeeDistributor.t.sol‎

Lines changed: 81 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,14 @@ contract FeeDistributorInitTest is FeeDistributorTestBase {
138138
feeDistributor.initialize(address(this), address(0));
139139
}
140140

141+
function test_initialize_RevertWhen_calledTwice() public {
142+
_enableInitializers(address(feeDistributor));
143+
feeDistributor.initialize(address(this), rebateRecipient);
144+
145+
vm.expectRevert(Initializable.InvalidInitialization.selector);
146+
feeDistributor.initialize(address(this), rebateRecipient);
147+
}
148+
141149
function test_finalizeUpgradeV2() public {
142150
_enableInitializers(address(feeDistributor));
143151

@@ -155,6 +163,14 @@ contract FeeDistributorInitTest is FeeDistributorTestBase {
155163
vm.expectRevert(IFeeDistributor.ZeroRebateRecipientAddress.selector);
156164
feeDistributor.finalizeUpgradeV2(address(0));
157165
}
166+
167+
function test_finalizeUpgradeV2_RevertWhen_calledTwice() public {
168+
_enableInitializers(address(feeDistributor));
169+
feeDistributor.finalizeUpgradeV2(rebateRecipient);
170+
171+
vm.expectRevert(Initializable.InvalidInitialization.selector);
172+
feeDistributor.finalizeUpgradeV2(rebateRecipient);
173+
}
158174
}
159175

160176
contract FeeDistributorTest is FeeDistributorTestBase {
@@ -211,13 +227,17 @@ contract FeeDistributorTest is FeeDistributorTestBase {
211227
emit IFeeDistributor.OperatorFeeDistributed(nodeOperatorId, shares);
212228

213229
vm.prank(address(accounting));
214-
feeDistributor.distributeFees({
230+
uint256 returned = feeDistributor.distributeFees({
215231
proof: proof,
216232
nodeOperatorId: nodeOperatorId,
217233
cumulativeFeeShares: shares
218234
});
219235

236+
assertEq(returned, shares);
220237
assertEq(stETH.sharesOf(address(accounting)), shares);
238+
assertEq(feeDistributor.distributedShares(nodeOperatorId), shares);
239+
assertEq(feeDistributor.totalClaimableShares(), 0);
240+
assertEq(feeDistributor.pendingSharesToDistribute(), 0);
221241
}
222242

223243
function test_getFeesToDistribute_notDistributedYet()
@@ -309,6 +329,35 @@ contract FeeDistributorTest is FeeDistributorTestBase {
309329
});
310330
}
311331

332+
function test_getFeesToDistribute_RevertWhen_InvalidProofContents() public {
333+
uint256 nodeOperatorId = 42;
334+
uint256 shares = 100;
335+
uint256 refSlot = 154;
336+
tree.pushLeaf(abi.encode(nodeOperatorId, shares));
337+
tree.pushLeaf(abi.encode(type(uint64).max, 0));
338+
bytes32[] memory proof = tree.getProof(0);
339+
bytes32 root = tree.root();
340+
341+
stETH.mintShares(address(feeDistributor), shares);
342+
vm.prank(oracle);
343+
feeDistributor.processOracleReport(
344+
root,
345+
someCIDv0(),
346+
someCIDv0(),
347+
shares,
348+
0,
349+
refSlot
350+
);
351+
352+
// tamper the proof leaf by changing cumulativeFeeShares
353+
vm.expectRevert(IFeeDistributor.InvalidProof.selector);
354+
feeDistributor.getFeesToDistribute({
355+
proof: proof,
356+
nodeOperatorId: nodeOperatorId,
357+
cumulativeFeeShares: shares + 1
358+
});
359+
}
360+
312361
function test_getHistoricalDistributionData() public {
313362
uint256 shares = 100;
314363
uint256 rebate = 10;
@@ -357,6 +406,9 @@ contract FeeDistributorTest is FeeDistributorTestBase {
357406
refSlot
358407
);
359408

409+
uint256 initialHistoryLength = feeDistributor
410+
.distributionDataHistoryCount();
411+
360412
shares = 120;
361413
rebate = 10;
362414
refSlot = 155;
@@ -376,6 +428,7 @@ contract FeeDistributorTest is FeeDistributorTestBase {
376428
);
377429

378430
uint256 historyLength = feeDistributor.distributionDataHistoryCount();
431+
assertEq(historyLength, initialHistoryLength + 1);
379432

380433
IFeeDistributor.DistributionData memory data = feeDistributor
381434
.getHistoricalDistributionData(historyLength - 1);
@@ -386,6 +439,10 @@ contract FeeDistributorTest is FeeDistributorTestBase {
386439
assertEq(data.logCid, logCid);
387440
assertEq(data.distributed, shares);
388441
assertEq(data.rebate, rebate);
442+
443+
// previous record remains intact
444+
data = feeDistributor.getHistoricalDistributionData(historyLength - 2);
445+
assertEq(data.refSlot, 154);
389446
}
390447

391448
function test_hashLeaf() public assertInvariants {
@@ -670,6 +727,17 @@ contract FeeDistributorTest is FeeDistributorTestBase {
670727
Vm.Log[] memory entries = vm.getRecordedLogs();
671728
assertEq(entries.length, 2);
672729

730+
IFeeDistributor.DistributionData memory last = feeDistributor
731+
.getHistoricalDistributionData(
732+
feeDistributor.distributionDataHistoryCount() - 1
733+
);
734+
assertEq(last.treeRoot, treeRootOld);
735+
assertEq(last.treeCid, treeCidOld);
736+
assertEq(last.logCid, logCid);
737+
assertEq(last.distributed, shares);
738+
assertEq(last.rebate, rebate);
739+
assertEq(last.refSlot, refSlot);
740+
673741
assertEq(feeDistributor.treeRoot(), treeRootOld);
674742
assertEq(feeDistributor.treeCid(), treeCidOld);
675743
assertEq(feeDistributor.logCid(), logCid);
@@ -1063,4 +1131,16 @@ contract FeeDistributorAssetRecovererTest is FeeDistributorTestBase {
10631131
vm.expectRevert(IAssetRecovererLib.NotAllowedToRecover.selector);
10641132
feeDistributor.recoverERC20(address(stETH), 1000);
10651133
}
1134+
1135+
function test_recoverERC721_RevertWhen_Unauthorized() public {
1136+
expectRoleRevert(stranger, feeDistributor.RECOVERER_ROLE());
1137+
vm.prank(stranger);
1138+
feeDistributor.recoverERC721(address(1), 1);
1139+
}
1140+
1141+
function test_recoverERC1155_RevertWhen_Unauthorized() public {
1142+
expectRoleRevert(stranger, feeDistributor.RECOVERER_ROLE());
1143+
vm.prank(stranger);
1144+
feeDistributor.recoverERC1155(address(1), 1);
1145+
}
10661146
}

0 commit comments

Comments
 (0)