From 5acc61e541581926949179d9c5d665bdfa5021ec Mon Sep 17 00:00:00 2001 From: Gus Date: Fri, 9 Feb 2024 10:22:28 -0600 Subject: [PATCH] extend audit fixes --- contracts/LRTDepositPool.sol | 43 +++--------------- contracts/utils/LRTConstants.sol | 2 +- .../LRTNativeEthStakingIntegrationTest.t.sol | 44 +++++++++++++++++-- 3 files changed, 48 insertions(+), 41 deletions(-) diff --git a/contracts/LRTDepositPool.sol b/contracts/LRTDepositPool.sol index f3a787e..48e242e 100644 --- a/contracts/LRTDepositPool.sol +++ b/contracts/LRTDepositPool.sol @@ -250,43 +250,14 @@ contract LRTDepositPool is ILRTDepositPool, LRTConfigRoleChecker, PausableUpgrad /// @dev only callable by LRT admin /// @param nodeDelegatorAddress NodeDelegator contract address function removeNodeDelegatorContractFromQueue(address nodeDelegatorAddress) public onlyLRTAdmin { - // 1. remove assets from node delegator contract to LRTDepositPool - address[] memory supportedAssets = lrtConfig.getSupportedAssetList(); - - uint256 supportedAssetsLength = supportedAssets.length; - - for (uint256 i; i < supportedAssetsLength;) { - // check the case for ETH - if (supportedAssets[i] == LRTConstants.ETH_TOKEN) { - uint256 ethBalance = nodeDelegatorAddress.balance; - if (ethBalance > 0) { - INodeDelegator(nodeDelegatorAddress).sendETHFromDepositPoolToNDC{ value: ethBalance }(); - } - } else { - // LST case - uint256 assetBalance = IERC20(supportedAssets[i]).balanceOf(nodeDelegatorAddress); - - if (assetBalance > 0) { - INodeDelegator(nodeDelegatorAddress).transferBackToLRTDepositPool(supportedAssets[i], assetBalance); - } - } - - unchecked { - ++i; - } - } - - // 2. revert if node delegator contract has asset in eigenlayer asset strategies + // 1. revert if node delegator contract has asset in eigenlayer asset strategies - // check if NDC has native ETH balance in eigen layer - if ( - INodeDelegator(nodeDelegatorAddress).stakedButUnverifiedNativeETH() > 0 - || INodeDelegator(nodeDelegatorAddress).getETHEigenPodBalance() > 0 - ) { + // 1.1 check if NDC has native ETH balance in eigen layer + if (INodeDelegator(nodeDelegatorAddress).getETHEigenPodBalance() > 0) { revert NodeDelegatorHasETHInEigenlayer(); } - // check if NDC has LST balance in eigen layer + // 1.2 check if NDC has LST balance in eigen layer (address[] memory assets, uint256[] memory assetBalances) = INodeDelegator(nodeDelegatorAddress).getAssetBalances(); @@ -305,7 +276,7 @@ contract LRTDepositPool is ILRTDepositPool, LRTConfigRoleChecker, PausableUpgrad } } - // 3. remove node delegator contract from queue + // 2. remove node delegator contract from queue uint256 length = nodeDelegatorQueue.length; uint256 ndcIndex; @@ -324,9 +295,9 @@ contract LRTDepositPool is ILRTDepositPool, LRTConfigRoleChecker, PausableUpgrad } } - // remove from isNodeDelegator mapping + // 2.1 remove from isNodeDelegator mapping isNodeDelegator[nodeDelegatorAddress] = 0; - // remove from nodeDelegatorQueue + // 2.2 remove from nodeDelegatorQueue nodeDelegatorQueue[ndcIndex] = nodeDelegatorQueue[length - 1]; nodeDelegatorQueue.pop(); diff --git a/contracts/utils/LRTConstants.sol b/contracts/utils/LRTConstants.sol index 91175cc..fed18cd 100644 --- a/contracts/utils/LRTConstants.sol +++ b/contracts/utils/LRTConstants.sol @@ -29,7 +29,7 @@ library LRTConstants { bytes32 public constant EIGEN_POD_MANAGER = keccak256("EIGEN_POD_MANAGER"); // native ETH as ERC20 for ease of implementation - address public constant ETH_TOKEN = address(0xeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee); + address public constant ETH_TOKEN = address(0xEeeeeEeeeEeEeeEeEeEeeEEEeeeeEeeeeeeeEEeE); // Operator Role bytes32 public constant OPERATOR_ROLE = keccak256("OPERATOR_ROLE"); diff --git a/test/integration/LRTNativeEthStakingIntegrationTest.t.sol b/test/integration/LRTNativeEthStakingIntegrationTest.t.sol index 5c6c53c..a289104 100644 --- a/test/integration/LRTNativeEthStakingIntegrationTest.t.sol +++ b/test/integration/LRTNativeEthStakingIntegrationTest.t.sol @@ -3,6 +3,7 @@ pragma solidity 0.8.21; import "forge-std/Test.sol"; +import { ProxyFactory } from "script/foundry-scripts/utils/ProxyFactory.sol"; import { LRTConfig, ILRTConfig, LRTConstants } from "contracts/LRTConfig.sol"; import { RSETH } from "contracts/RSETH.sol"; import { LRTOracle } from "contracts/LRTOracle.sol"; @@ -21,6 +22,7 @@ contract LRTNativeEthStakingIntegrationTest is Test { address public manager; address public operator; + ProxyFactory proxyFactory; ProxyAdmin proxyAdmin; LRTDepositPool public lrtDepositPool; LRTConfig public lrtConfig; @@ -41,10 +43,6 @@ contract LRTNativeEthStakingIntegrationTest is Test { newImplementation = address(new LRTDepositPool()); proxyAdmin.upgrade(ITransparentUpgradeableProxy(proxyAddress), newImplementation); - // remove faulty ndcs - lrtDepositPool.removeNodeDelegatorContractFromQueue(0xAb96EB807c9dFE59E9d52f7F428A6D35f12728c6); - lrtDepositPool.removeNodeDelegatorContractFromQueue(0x2107EA068FD85E125Be422AFC86d2E57A6d085d8); - // upgrade all ndcs address[] memory ndcs = lrtDepositPool.getNodeDelegatorQueue(); newImplementation = address(new NodeDelegator()); @@ -73,6 +71,7 @@ contract LRTNativeEthStakingIntegrationTest is Test { manager = 0xCbcdd778AA25476F203814214dD3E9b9c46829A1; operator = makeAddr("operator"); + proxyFactory = ProxyFactory(0x673a669425457bCabeb247f56552A0Fd8141cee2); proxyAdmin = ProxyAdmin(0xb61e0E39b6d4030C36A176f576aaBE44BF59Dc78); lrtDepositPool = LRTDepositPool(payable(0x036676389e48133B63a802f8635AD39E752D375D)); lrtConfig = LRTConfig(0x947Cb49334e6571ccBFEF1f1f1178d8469D65ec7); @@ -197,4 +196,41 @@ contract LRTNativeEthStakingIntegrationTest is Test { "eth not transferred back to deposit pool" ); } + + function test_removeNDCs() external { + // ------ STEP1: add NDCs --------- + + NodeDelegator nodeDelegatorImplementation = new NodeDelegator(); + bytes32 salt1 = keccak256(abi.encodePacked("test-ndc1")); + NodeDelegator testNodeDelegatorProxy1 = NodeDelegator( + payable(proxyFactory.create(address(nodeDelegatorImplementation), address(proxyAdmin), salt1)) + ); + + bytes32 salt2 = keccak256(abi.encodePacked("test-ndc2")); + NodeDelegator testNodeDelegatorProxy2 = NodeDelegator( + payable(proxyFactory.create(address(nodeDelegatorImplementation), address(proxyAdmin), salt2)) + ); + + testNodeDelegatorProxy1.initialize(address(lrtConfig)); + testNodeDelegatorProxy2.initialize(address(lrtConfig)); + + address[] memory testNDCArray = new address[](2); + testNDCArray[0] = address(testNodeDelegatorProxy1); + testNDCArray[1] = address(testNodeDelegatorProxy2); + + // add ndcs to queue + vm.prank(admin); + lrtDepositPool.addNodeDelegatorContractToQueue(testNDCArray); + + assertTrue(lrtDepositPool.isNodeDelegator(address(testNodeDelegatorProxy1)) != 0); + assertTrue(lrtDepositPool.isNodeDelegator(address(testNodeDelegatorProxy2)) != 0); + + // ------ STEP2: remove NDCs --------- + + vm.prank(admin); + lrtDepositPool.removeManyNodeDelegatorContractsFromQueue(testNDCArray); + + assertTrue(lrtDepositPool.isNodeDelegator(address(testNodeDelegatorProxy1)) == 0); + assertTrue(lrtDepositPool.isNodeDelegator(address(testNodeDelegatorProxy2)) == 0); + } }