From 86d47413bae00fb960542ac20e522d07c031c7d8 Mon Sep 17 00:00:00 2001 From: madlabman <10616301+madlabman@users.noreply.github.com> Date: Tue, 19 Mar 2024 18:43:22 +0100 Subject: [PATCH] feat: hook to notify CSModule about bond changes --- src/CSAccounting.sol | 41 ++++--- src/CSModule.sol | 111 ++++++++++-------- src/interfaces/ICSModule.sol | 3 + test/CSAccounting.t.sol | 7 ++ test/CSModule.t.sol | 16 +-- .../mocks/CommunityStakingModuleMock.sol | 4 + 6 files changed, 105 insertions(+), 77 deletions(-) diff --git a/src/CSAccounting.sol b/src/CSAccounting.sol index e9d7c03d..847deac5 100644 --- a/src/CSAccounting.sol +++ b/src/CSAccounting.sol @@ -241,7 +241,7 @@ contract CSAccounting is uint256 activeKeys = _getActiveKeys(nodeOperatorId); /// 10 wei added to account for possible stETH rounding errors /// https://github.com/lidofinance/lido-dao/issues/442#issuecomment-1182264205 - /// Should be suficient fot ~ 40 years + /// Should be sufficient for ~ 40 years uint256 currentBond = CSBondCore._ethByShares( _bondShares[nodeOperatorId] ) + 10 wei; @@ -332,7 +332,7 @@ contract CSAccounting is /// @dev if `from` is not the same as `msg.sender`, then `msg.sender` should be CSM /// @param from address to stake ETH and deposit stETH from /// @param nodeOperatorId id of the node operator to stake ETH and deposit stETH for - /// @return stETH shares amount + /// @return shares stETH shares amount function depositETH( address from, uint256 nodeOperatorId @@ -341,10 +341,11 @@ contract CSAccounting is payable whenResumed onlyExistingNodeOperator(nodeOperatorId) - returns (uint256) + returns (uint256 shares) { from = _validateDepositSender(from); - return CSBondCore._depositETH(from, nodeOperatorId); + shares = CSBondCore._depositETH(from, nodeOperatorId); + CSM.onBondChanged(nodeOperatorId); } /// @notice Deposit user's stETH to the bond for the given Node Operator @@ -352,7 +353,7 @@ contract CSAccounting is /// @param from address to deposit stETH from /// @param nodeOperatorId id of the node operator to deposit stETH for /// @param stETHAmount amount of stETH to deposit - /// @return stETH shares amount + /// @return shares stETH shares amount function depositStETH( address from, uint256 nodeOperatorId, @@ -361,11 +362,12 @@ contract CSAccounting is external whenResumed onlyExistingNodeOperator(nodeOperatorId) - returns (uint256) + returns (uint256 shares) { // TODO: can it be two functions rather than one with `from` param and condition? from = _validateDepositSender(from); - return CSBondCore._depositStETH(from, nodeOperatorId, stETHAmount); + shares = CSBondCore._depositStETH(from, nodeOperatorId, stETHAmount); + CSM.onBondChanged(nodeOperatorId); } /// @notice Deposit user's stETH to the bond for the given Node Operator using the proper permit for the contract @@ -374,7 +376,7 @@ contract CSAccounting is /// @param nodeOperatorId id of the node operator to deposit stETH for /// @param stETHAmount amount of stETH to deposit /// @param permit stETH permit for the contract - /// @return stETH shares amount + /// @return shares stETH shares amount function depositStETHWithPermit( address from, uint256 nodeOperatorId, @@ -384,7 +386,7 @@ contract CSAccounting is external whenResumed onlyExistingNodeOperator(nodeOperatorId) - returns (uint256) + returns (uint256 shares) { // TODO: can it be two functions rather than one with `from` param and condition? from = _validateDepositSender(from); @@ -401,7 +403,8 @@ contract CSAccounting is permit.s ); } - return CSBondCore._depositStETH(from, nodeOperatorId, stETHAmount); + shares = CSBondCore._depositStETH(from, nodeOperatorId, stETHAmount); + CSM.onBondChanged(nodeOperatorId); } /// @notice Unwrap user's wstETH and make deposit in stETH to the bond for the given Node Operator @@ -409,7 +412,7 @@ contract CSAccounting is /// @param from address to unwrap wstETH from /// @param nodeOperatorId id of the node operator to deposit stETH for /// @param wstETHAmount amount of wstETH to deposit - /// @return stETH shares amount + /// @return shares stETH shares amount function depositWstETH( address from, uint256 nodeOperatorId, @@ -418,11 +421,12 @@ contract CSAccounting is external whenResumed onlyExistingNodeOperator(nodeOperatorId) - returns (uint256) + returns (uint256 shares) { // TODO: can it be two functions rather than one with `from` param and condition? from = _validateDepositSender(from); - return CSBondCore._depositWstETH(from, nodeOperatorId, wstETHAmount); + shares = CSBondCore._depositWstETH(from, nodeOperatorId, wstETHAmount); + CSM.onBondChanged(nodeOperatorId); } /// @notice Unwrap user's wstETH and make deposit in stETH to the bond for the given Node Operator using the proper permit for the contract @@ -431,7 +435,7 @@ contract CSAccounting is /// @param nodeOperatorId id of the node operator to deposit stETH for /// @param wstETHAmount amount of wstETH to deposit /// @param permit wstETH permit for the contract - /// @return stETH shares amount + /// @return shares stETH shares amount function depositWstETHWithPermit( address from, uint256 nodeOperatorId, @@ -441,7 +445,7 @@ contract CSAccounting is external whenResumed onlyExistingNodeOperator(nodeOperatorId) - returns (uint256) + returns (uint256 shares) { // TODO: can it be two functions rather than one with `from` param and condition? from = _validateDepositSender(from); @@ -458,7 +462,8 @@ contract CSAccounting is permit.s ); } - return CSBondCore._depositWstETH(from, nodeOperatorId, wstETHAmount); + shares = CSBondCore._depositWstETH(from, nodeOperatorId, wstETHAmount); + CSM.onBondChanged(nodeOperatorId); } /// @dev only CSM can pass `from` != `msg.sender` @@ -594,7 +599,7 @@ contract CSAccounting is /// @dev reverts if amount isn't between MIN_STETH_WITHDRAWAL_AMOUNT and MAX_STETH_WITHDRAWAL_AMOUNT /// @param rewardsProof merkle proof of the rewards. /// @param nodeOperatorId id of the node operator to request rewards for. - /// @param cumulativeFeeShares cummulative fee shares for the node operator. + /// @param cumulativeFeeShares cumulative fee shares for the node operator. /// @param ethAmount amount of ETH to request. function requestRewardsETH( bytes32[] memory rewardsProof, @@ -655,6 +660,7 @@ contract CSAccounting is { CSBondLock._reduceAmount(nodeOperatorId, amount); emit BondLockReleased(nodeOperatorId, amount); + CSM.onBondChanged(nodeOperatorId); } /// @notice Compensates locked bond ETH for the given node operator. @@ -665,6 +671,7 @@ contract CSAccounting is payable(LIDO_LOCATOR.elRewardsVault()).transfer(msg.value); CSBondLock._reduceAmount(nodeOperatorId, msg.value); emit BondLockCompensated(nodeOperatorId, msg.value); + CSM.onBondChanged(nodeOperatorId); } /// @notice Settles locked bond ETH for the given node operator. diff --git a/src/CSModule.sol b/src/CSModule.sol index 505948a0..387e6073 100644 --- a/src/CSModule.sol +++ b/src/CSModule.sol @@ -301,21 +301,22 @@ contract CSModule is ICSModule, CSModuleBase, AccessControl, PausableUntil { ) external payable whenResumed { // TODO: sanity checks - uint256 id = _createNodeOperator(); - _processEarlyAdoption(id, eaProof); + uint256 nodeOperatorId = _createNodeOperator(); + _processEarlyAdoption(nodeOperatorId, eaProof); if ( msg.value != accounting.getBondAmountByKeysCount( keysCount, - accounting.getBondCurve(id) + accounting.getBondCurve(nodeOperatorId) ) ) { revert InvalidAmount(); } - accounting.depositETH{ value: msg.value }(msg.sender, id); - _addSigningKeys(id, keysCount, publicKeys, signatures); + _addSigningKeys(nodeOperatorId, keysCount, publicKeys, signatures); + accounting.depositETH{ value: msg.value }(msg.sender, nodeOperatorId); + _normalizeQueue(nodeOperatorId); } /// @notice Adds a new node operator with stETH bond @@ -330,19 +331,19 @@ contract CSModule is ICSModule, CSModuleBase, AccessControl, PausableUntil { ) external whenResumed { // TODO: sanity checks - uint256 id = _createNodeOperator(); - _processEarlyAdoption(id, eaProof); + uint256 nodeOperatorId = _createNodeOperator(); + _processEarlyAdoption(nodeOperatorId, eaProof); + _addSigningKeys(nodeOperatorId, keysCount, publicKeys, signatures); accounting.depositStETH( msg.sender, - id, + nodeOperatorId, accounting.getBondAmountByKeysCount( keysCount, - accounting.getBondCurve(id) + accounting.getBondCurve(nodeOperatorId) ) ); - - _addSigningKeys(id, keysCount, publicKeys, signatures); + _normalizeQueue(nodeOperatorId); } /// @notice Adds a new node operator with permit to use stETH as bond @@ -359,20 +360,20 @@ contract CSModule is ICSModule, CSModuleBase, AccessControl, PausableUntil { ) external whenResumed { // TODO: sanity checks - uint256 id = _createNodeOperator(); - _processEarlyAdoption(id, eaProof); + uint256 nodeOperatorId = _createNodeOperator(); + _processEarlyAdoption(nodeOperatorId, eaProof); + _addSigningKeys(nodeOperatorId, keysCount, publicKeys, signatures); accounting.depositStETHWithPermit( msg.sender, - id, + nodeOperatorId, accounting.getBondAmountByKeysCount( keysCount, - accounting.getBondCurve(id) + accounting.getBondCurve(nodeOperatorId) ), permit ); - - _addSigningKeys(id, keysCount, publicKeys, signatures); + _normalizeQueue(nodeOperatorId); } /// @notice Adds a new node operator with wstETH bond @@ -387,19 +388,19 @@ contract CSModule is ICSModule, CSModuleBase, AccessControl, PausableUntil { ) external whenResumed { // TODO: sanity checks - uint256 id = _createNodeOperator(); - _processEarlyAdoption(id, eaProof); + uint256 nodeOperatorId = _createNodeOperator(); + _processEarlyAdoption(nodeOperatorId, eaProof); + _addSigningKeys(nodeOperatorId, keysCount, publicKeys, signatures); accounting.depositWstETH( msg.sender, - id, + nodeOperatorId, accounting.getBondAmountByKeysCountWstETH( keysCount, - accounting.getBondCurve(id) + accounting.getBondCurve(nodeOperatorId) ) ); - - _addSigningKeys(id, keysCount, publicKeys, signatures); + _normalizeQueue(nodeOperatorId); } /// @notice Adds a new node operator with permit to use wstETH as bond @@ -416,20 +417,20 @@ contract CSModule is ICSModule, CSModuleBase, AccessControl, PausableUntil { ) external whenResumed { // TODO: sanity checks - uint256 id = _createNodeOperator(); - _processEarlyAdoption(id, eaProof); + uint256 nodeOperatorId = _createNodeOperator(); + _processEarlyAdoption(nodeOperatorId, eaProof); + _addSigningKeys(nodeOperatorId, keysCount, publicKeys, signatures); accounting.depositWstETHWithPermit( msg.sender, - id, + nodeOperatorId, accounting.getBondAmountByKeysCountWstETH( keysCount, - accounting.getBondCurve(id) + accounting.getBondCurve(nodeOperatorId) ), permit ); - - _addSigningKeys(id, keysCount, publicKeys, signatures); + _normalizeQueue(nodeOperatorId); } /// @notice Adds a new keys to the node operator with ETH bond @@ -452,9 +453,9 @@ contract CSModule is ICSModule, CSModuleBase, AccessControl, PausableUntil { revert InvalidAmount(); } - accounting.depositETH{ value: msg.value }(msg.sender, nodeOperatorId); - _addSigningKeys(nodeOperatorId, keysCount, publicKeys, signatures); + accounting.depositETH{ value: msg.value }(msg.sender, nodeOperatorId); + _normalizeQueue(nodeOperatorId); } /// @notice Adds a new keys to the node operator with stETH bond @@ -470,13 +471,13 @@ contract CSModule is ICSModule, CSModuleBase, AccessControl, PausableUntil { ) external whenResumed { // TODO: sanity checks - accounting.depositStETH( - msg.sender, + uint256 amount = accounting.getRequiredBondForNextKeys( nodeOperatorId, - accounting.getRequiredBondForNextKeys(nodeOperatorId, keysCount) + keysCount ); - _addSigningKeys(nodeOperatorId, keysCount, publicKeys, signatures); + accounting.depositStETH(msg.sender, nodeOperatorId, amount); + _normalizeQueue(nodeOperatorId); } /// @notice Adds a new keys to the node operator with permit to use stETH as bond @@ -494,14 +495,18 @@ contract CSModule is ICSModule, CSModuleBase, AccessControl, PausableUntil { ) external whenResumed { // TODO: sanity checks + uint256 amount = accounting.getRequiredBondForNextKeys( + nodeOperatorId, + keysCount + ); + _addSigningKeys(nodeOperatorId, keysCount, publicKeys, signatures); accounting.depositStETHWithPermit( msg.sender, nodeOperatorId, - accounting.getRequiredBondForNextKeys(nodeOperatorId, keysCount), + amount, permit ); - - _addSigningKeys(nodeOperatorId, keysCount, publicKeys, signatures); + _normalizeQueue(nodeOperatorId); } /// @notice Adds a new keys to the node operator with wstETH bond @@ -517,16 +522,13 @@ contract CSModule is ICSModule, CSModuleBase, AccessControl, PausableUntil { ) external whenResumed { // TODO: sanity checks - accounting.depositWstETH( - msg.sender, + uint256 amount = accounting.getRequiredBondForNextKeysWstETH( nodeOperatorId, - accounting.getRequiredBondForNextKeysWstETH( - nodeOperatorId, - keysCount - ) + keysCount ); - _addSigningKeys(nodeOperatorId, keysCount, publicKeys, signatures); + accounting.depositWstETH(msg.sender, nodeOperatorId, amount); + _normalizeQueue(nodeOperatorId); } /// @notice Adds a new keys to the node operator with permit to use wstETH as bond @@ -544,17 +546,23 @@ contract CSModule is ICSModule, CSModuleBase, AccessControl, PausableUntil { ) external whenResumed { // TODO: sanity checks + uint256 amount = accounting.getRequiredBondForNextKeysWstETH( + nodeOperatorId, + keysCount + ); + _addSigningKeys(nodeOperatorId, keysCount, publicKeys, signatures); accounting.depositWstETHWithPermit( msg.sender, nodeOperatorId, - accounting.getRequiredBondForNextKeysWstETH( - nodeOperatorId, - keysCount - ), + amount, permit ); + _normalizeQueue(nodeOperatorId); + } - _addSigningKeys(nodeOperatorId, keysCount, publicKeys, signatures); + /// @notice Notify the module about the opetor's bond change. The hook call is optional. + function onBondChanged(uint256 nodeOperatorId) external { + _updateDepositableValidatorsCount(nodeOperatorId); } /// @notice Proposes a new manager address for the node operator @@ -807,6 +815,7 @@ contract CSModule is ICSModule, CSModuleBase, AccessControl, PausableUntil { } if (no.depositableValidatorsCount != newCount) { + // Updating the global counter. _depositableValidatorsCount = _depositableValidatorsCount - no.depositableValidatorsCount + @@ -1225,8 +1234,6 @@ contract CSModule is ICSModule, CSModuleBase, AccessControl, PausableUntil { no.totalAddedKeys += keysCount; emit TotalSigningKeysCountChanged(nodeOperatorId, no.totalAddedKeys); - _updateDepositableValidatorsCount(nodeOperatorId); - _normalizeQueue(nodeOperatorId); _incrementModuleNonce(); } diff --git a/src/interfaces/ICSModule.sol b/src/interfaces/ICSModule.sol index 104e6dd7..6afd9364 100644 --- a/src/interfaces/ICSModule.sol +++ b/src/interfaces/ICSModule.sol @@ -52,4 +52,7 @@ interface ICSModule is IStakingModule { uint256 keyIndex, uint256 amount ) external; + + /// @notice Notify the module about the operator's bond change. The hook call is optional. + function onBondChanged(uint256 nodeOperatorId) external; } diff --git a/test/CSAccounting.t.sol b/test/CSAccounting.t.sol index 1837c77e..0bddabe4 100644 --- a/test/CSAccounting.t.sol +++ b/test/CSAccounting.t.sol @@ -107,6 +107,13 @@ contract CSAccountingBaseTest is accounting.grantRole(accounting.SET_DEFAULT_BOND_CURVE_ROLE(), admin); accounting.grantRole(accounting.SET_BOND_CURVE_ROLE(), admin); vm.stopPrank(); + + // HACK: To not to change Stub to a mock so far. + vm.mockCall( + address(stakingModule), + abi.encodeWithSelector(ICSModule.onBondChanged.selector), + "" + ); } function mock_getNodeOperatorsCount(uint256 returnValue) internal { diff --git a/test/CSModule.t.sol b/test/CSModule.t.sol index a132c78d..84659f26 100644 --- a/test/CSModule.t.sol +++ b/test/CSModule.t.sol @@ -537,10 +537,10 @@ contract CSMAddNodeOperator is CSMCommon, PermitTokenBase { { vm.expectEmit(true, true, false, true, address(csm)); emit NodeOperatorAdded(0, nodeOperator); - vm.expectEmit(true, true, true, true, address(wstETH)); - emit Approval(nodeOperator, address(accounting), wstETHAmount); vm.expectEmit(true, true, false, true, address(csm)); emit TotalSigningKeysCountChanged(0, 1); + vm.expectEmit(true, true, true, true, address(wstETH)); + emit Approval(nodeOperator, address(accounting), wstETHAmount); } csm.addNodeOperatorWstETHWithPermit( @@ -588,10 +588,10 @@ contract CSMAddNodeOperator is CSMCommon, PermitTokenBase { uint256 wstETHAmount = wstETH.wrap(toWrap); uint256 nonce = csm.getNonce(); { - vm.expectEmit(true, true, true, true, address(wstETH)); - emit Approval(nodeOperator, address(accounting), wstETHAmount); vm.expectEmit(true, true, false, true, address(csm)); emit TotalSigningKeysCountChanged(0, 2); + vm.expectEmit(true, true, true, true, address(wstETH)); + emit Approval(nodeOperator, address(accounting), wstETHAmount); } csm.addValidatorKeysWstETHWithPermit( noId, @@ -648,10 +648,10 @@ contract CSMAddNodeOperator is CSMCommon, PermitTokenBase { { vm.expectEmit(true, true, false, true, address(csm)); emit NodeOperatorAdded(0, nodeOperator); - vm.expectEmit(true, true, true, true, address(stETH)); - emit Approval(nodeOperator, address(accounting), BOND_SIZE); vm.expectEmit(true, true, false, true, address(csm)); emit TotalSigningKeysCountChanged(0, 1); + vm.expectEmit(true, true, true, true, address(stETH)); + emit Approval(nodeOperator, address(accounting), BOND_SIZE); } vm.prank(nodeOperator); @@ -701,10 +701,10 @@ contract CSMAddNodeOperator is CSMCommon, PermitTokenBase { uint256 nonce = csm.getNonce(); { - vm.expectEmit(true, true, true, true, address(stETH)); - emit Approval(nodeOperator, address(accounting), required); vm.expectEmit(true, true, false, true, address(csm)); emit TotalSigningKeysCountChanged(0, 2); + vm.expectEmit(true, true, true, true, address(stETH)); + emit Approval(nodeOperator, address(accounting), required); } vm.prank(nodeOperator); csm.addValidatorKeysStETHWithPermit( diff --git a/test/helpers/mocks/CommunityStakingModuleMock.sol b/test/helpers/mocks/CommunityStakingModuleMock.sol index 8116426f..414b7b08 100644 --- a/test/helpers/mocks/CommunityStakingModuleMock.sol +++ b/test/helpers/mocks/CommunityStakingModuleMock.sol @@ -79,4 +79,8 @@ contract CommunityStakingModuleMock { function getNodeOperatorsCount() external view returns (uint256) { return totalNodeOperatorsCount; } + + function onBondChanged(uint256 nodeOperatorId) external { + // do nothing + } }