From 6bd86c1e7093847a6cbec4e88e03ee88697498f0 Mon Sep 17 00:00:00 2001 From: Benjamin Date: Thu, 9 Nov 2023 08:19:29 +0100 Subject: [PATCH] Refactor setup access script --- .gas-snapshot | 73 +++++------ docs/PufferPool.md | 5 +- script/1_DeployGuardians.s.sol | 1 + script/DeployPuffer.s.sol | 2 +- script/SetupAccess.s.sol | 158 ++++++++++++++++++++---- src/NoRestakingStrategy.sol | 2 +- src/PufferPool.sol | 13 +- src/PufferProtocol.sol | 12 +- src/WithdrawalPool.sol | 13 +- src/interface/IPufferPool.sol | 5 - src/interface/IWithdrawalPool.sol | 2 +- test/handlers/PufferProtocolHandler.sol | 5 +- test/mocks/PufferPoolMock.sol | 2 - test/unit/PufferPool.t.sol | 12 +- test/unit/PufferProtocol.t.sol | 15 ++- 15 files changed, 211 insertions(+), 109 deletions(-) diff --git a/.gas-snapshot b/.gas-snapshot index 17d571d9..4b0c7e4a 100644 --- a/.gas-snapshot +++ b/.gas-snapshot @@ -7,62 +7,63 @@ EnclaveVerifierTest:testRaveEvidence3() (gas: 905231) EnclaveVerifierTest:testRemoveLeafX509() (gas: 200770) EnclaveVerifierTest:testSetup() (gas: 115154) EnclaveVerifierTest:testVerifyingStaleEvidence() (gas: 743633) -GuardianModuleTest:testRave() (gas: 45771890) +GuardianModuleTest:testRave() (gas: 47320445) GuardianModuleTest:testRoateGuardianKeyWithInvalidRaveReverts() (gas: 855872) GuardianModuleTest:testRoateGuardianToInvalidPubKeyReverts() (gas: 22860) GuardianModuleTest:testRotateGuardianKeyFromNonGuardianReverts() (gas: 20303) -NoRestakingStartegyTest:testClaimingMultipleProofs() (gas: 188849) -NoRestakingStartegyTest:testCollectRewards() (gas: 183439) +NoRestakingStartegyTest:testClaimingMultipleProofs() (gas: 194011) +NoRestakingStartegyTest:testCollectRewards() (gas: 198074) NoRestakingStartegyTest:testDonation() (gas: 11852) -NoRestakingStartegyTest:testDoubleClaimInSameTransaction() (gas: 134263) +NoRestakingStartegyTest:testDoubleClaimInSameTransaction() (gas: 139421) NoRestakingStartegyTest:testPostRewardsRoot(bytes32,uint256) (runs: 256, μ: 72388, ~: 72388) NoRestakingStartegyTest:testPostRewardsRootReverts(address,bytes32,uint256) (runs: 256, μ: 25698, ~: 25698) NoRestakingStartegyTest:testPostingRewardsForSameBlockReverts() (gas: 76898) -NoRestakingStartegyTest:testRewardsClaimingForAnotherUser(address) (runs: 256, μ: 163248, ~: 163248) +NoRestakingStartegyTest:testRewardsClaimingForAnotherUser(address) (runs: 256, μ: 168409, ~: 168409) NoRestakingStartegyTest:testSetup() (gas: 16031) PufferPoolIntegrationTest:testMulticallStrategyDepositOnGoerli() (gas: 302650) -PufferPoolTest:testBurn(address) (runs: 256, μ: 70123, ~: 70108) -PufferPoolTest:testDeposit(address,uint256) (runs: 256, μ: 101852, ~: 101965) -PufferPoolTest:testDepositAndRedeemRoundingError(address,uint256) (runs: 256, μ: 146005, ~: 146070) -PufferPoolTest:testDepositAndRedeemRoundingErrorForDifferentExchangeRate(address,uint256) (runs: 256, μ: 241309, ~: 241411) -PufferPoolTest:testDepositForOneWei() (gas: 77601) -PufferPoolTest:testMultipleDeposits() (gas: 123056) -PufferPoolTest:testRatioChange() (gas: 188273) +PufferPoolTest:testBurn(address) (runs: 256, μ: 79443, ~: 79429) +PufferPoolTest:testDeposit(address,uint256) (runs: 256, μ: 113487, ~: 113594) +PufferPoolTest:testDepositAndRedeemRoundingError(address,uint256) (runs: 256, μ: 162657, ~: 162721) +PufferPoolTest:testDepositAndRedeemRoundingErrorForDifferentExchangeRate(address,uint256) (runs: 256, μ: 259615, ~: 259725) +PufferPoolTest:testDepositForOneWei() (gas: 89274) +PufferPoolTest:testMultipleDeposits() (gas: 138118) +PufferPoolTest:testRatioChange() (gas: 200300) PufferPoolTest:testSetup() (gas: 32689) PufferPoolTest:testStorageS() (gas: 19492) -PufferProtocolInvariants:invariant_callSummary() (runs: 500, calls: 10000, reverts: 921) -PufferProtocolInvariants:invariant_guardiansCanNeverChange() (runs: 500, calls: 10000, reverts: 947) -PufferProtocolInvariants:invariant_pufEthToETHRate() (runs: 500, calls: 10000, reverts: 947) -PufferProtocolInvariants:invariant_pufferPoolETHCanOnlyGoUp() (runs: 500, calls: 10000, reverts: 947) +PufferProtocolInvariants:invariant_callSummary() (runs: 500, calls: 10000, reverts: 920) +PufferProtocolInvariants:invariant_guardiansCanNeverChange() (runs: 500, calls: 10000, reverts: 931) +PufferProtocolInvariants:invariant_pufEthToETHRate() (runs: 500, calls: 10000, reverts: 931) +PufferProtocolInvariants:invariant_pufferPoolETHCanOnlyGoUp() (runs: 500, calls: 10000, reverts: 931) PufferProtocolTest:testChangeStrategy() (gas: 32124) -PufferProtocolTest:testChangeStrategyToCustom() (gas: 51214) -PufferProtocolTest:testClaimBackBond() (gas: 1367007) +PufferProtocolTest:testChangeStrategyToCustom() (gas: 51236) +PufferProtocolTest:testClaimBackBond() (gas: 1388884) PufferProtocolTest:testCreateExistingStrategyShouldFail() (gas: 32922) PufferProtocolTest:testCreatePufferStrategy() (gas: 284661) PufferProtocolTest:testEmptyQueue() (gas: 32893) -PufferProtocolTest:testFuzzRegisterManyValidators(uint8) (runs: 256, μ: 27742706, ~: 11944202) +PufferProtocolTest:testFuzzRegisterManyValidators(uint8) (runs: 256, μ: 29927623, ~: 19135338) PufferProtocolTest:testGetPayload() (gas: 170740) -PufferProtocolTest:testProofOfReserve() (gas: 151341) -PufferProtocolTest:testProvisionNode() (gas: 4048449) -PufferProtocolTest:testRegisterInvalidPrivKeyShares() (gas: 73087) -PufferProtocolTest:testRegisterInvalidPubKeyShares() (gas: 76595) -PufferProtocolTest:testRegisterMoreValidatorsThanTheLimit() (gas: 813206) -PufferProtocolTest:testRegisterMultipleValidatorKeysAndDequeue(bytes32,bytes32) (runs: 256, μ: 2144361, ~: 2144361) -PufferProtocolTest:testRegisterToInvalidStrategy() (gas: 59839) -PufferProtocolTest:testRegisterWithInvalidAmountPaid() (gas: 81372) -PufferProtocolTest:testRegisterWithInvalidBLSPubKey() (gas: 32603) -PufferProtocolTest:testRegisterWithoutRAVE() (gas: 341935) +PufferProtocolTest:testPause() (gas: 135006) +PufferProtocolTest:testProofOfReserve() (gas: 151363) +PufferProtocolTest:testProvisionNode() (gas: 4115035) +PufferProtocolTest:testRegisterInvalidPrivKeyShares() (gas: 84873) +PufferProtocolTest:testRegisterInvalidPubKeyShares() (gas: 88360) +PufferProtocolTest:testRegisterMoreValidatorsThanTheLimit() (gas: 836582) +PufferProtocolTest:testRegisterMultipleValidatorKeysAndDequeue(bytes32,bytes32) (runs: 256, μ: 2199243, ~: 2199243) +PufferProtocolTest:testRegisterToInvalidStrategy() (gas: 71603) +PufferProtocolTest:testRegisterWithInvalidAmountPaid() (gas: 93159) +PufferProtocolTest:testRegisterWithInvalidBLSPubKey() (gas: 44367) +PufferProtocolTest:testRegisterWithoutRAVE() (gas: 362535) PufferProtocolTest:testSetGuardiansFeeRateOverTheLimit() (gas: 28307) PufferProtocolTest:testSetProtocolFeeRate() (gas: 33494) PufferProtocolTest:testSetProtocolFeeRateOverTheLimit() (gas: 28319) PufferProtocolTest:testSetSmoothingCommitment() (gas: 86626) PufferProtocolTest:testSetSmoothingCommitment(bytes32) (runs: 256, μ: 86576, ~: 86553) PufferProtocolTest:testSetWithdrawalPoolRateOverTheLimit() (gas: 28373) -PufferProtocolTest:testSetup() (gas: 19658) -PufferProtocolTest:testSkipProvisioning() (gas: 1017569) -PufferProtocolTest:testStopRegistration() (gas: 1113830) -PufferProtocolTest:testStrategyDOS() (gas: 3870211) -PufferProtocolTest:testUpgrade() (gas: 4745160) +PufferProtocolTest:testSetup() (gas: 19591) +PufferProtocolTest:testSkipProvisioning() (gas: 1040452) +PufferProtocolTest:testStopRegistration() (gas: 1140780) +PufferProtocolTest:testStrategyDOS() (gas: 3958921) +PufferProtocolTest:testUpgrade() (gas: 4716483) PufferStrategyTest:testBeaconUpgrade() (gas: 367461) -WithdrawalPoolTest:testWithdrawETH() (gas: 160389) -WithdrawalPoolTest:testWithdrawETHWithSignature() (gas: 172560) \ No newline at end of file +WithdrawalPoolTest:testWithdrawETH() (gas: 181225) +WithdrawalPoolTest:testWithdrawETHWithSignature() (gas: 193507) \ No newline at end of file diff --git a/docs/PufferPool.md b/docs/PufferPool.md index dffa5100..0b176dae 100644 --- a/docs/PufferPool.md +++ b/docs/PufferPool.md @@ -4,11 +4,10 @@ PufferPool.sol is a **non upgradeable** ERC20Permit token. -It takes the ETH deposits from the Puffers and mints `pufETH` ERC20 token in return. That can be achieved in 2 ways: -- Triggering a payable `receive()` function on this smart contract +It takes the ETH deposits from the Puffers and mints `pufETH` ERC20 token in return by: - Calling `depositETH()` and sending the ETH along with it -The rewards / donations are going through the `depositETHWithoutMinting()` function. This function will not mint any `pufETH` in return. +The rewards / donations are going through the `receive()` function. This function will not mint any `pufETH` in return. Depositing ETH through this function will eventually change the exchange rate between ETH and pufETH, making it so that for 1 pufETH you will be able to get more ETH in return. The withdrawals and exchanging of `pufETH` to ETH is happening through our [WithdrawalPool](./WithdrawalPool.md) smart contract or any third party exchange. The [Guardians](./Guardians.md) are responsible for reporting the values used for calculation of the exchange rate [PufferPoolStorage](../src//struct/PufferPoolStorage.sol). Those values are stored in the storage of our main [PufferProtocol smart contract](../src/PufferProtocolStorage.sol) diff --git a/script/1_DeployGuardians.s.sol b/script/1_DeployGuardians.s.sol index 476a6d6d..939309a5 100644 --- a/script/1_DeployGuardians.s.sol +++ b/script/1_DeployGuardians.s.sol @@ -40,6 +40,7 @@ contract DeployGuardians is BaseScript { vm.serializeAddress(obj, "safeProxyFactory", address(safeProxy)); vm.serializeAddress(obj, "safeImplementation", address(safeImplementation)); vm.serializeAddress(obj, "enclaveVerifier", address(verifier)); + vm.serializeAddress(obj, "pauser", 0x98BDB87fCF3697F4b356C36Cd621ffF94Ee3Aa19); string memory finalJson = vm.serializeString(obj, "", ""); diff --git a/script/DeployPuffer.s.sol b/script/DeployPuffer.s.sol index 79d2e781..9ce6f771 100644 --- a/script/DeployPuffer.s.sol +++ b/script/DeployPuffer.s.sol @@ -66,7 +66,7 @@ contract DeployPuffer is BaseScript { // Deploy pool PufferPool pool = new PufferPool(pufferProtocol, address(accessManager)); - WithdrawalPool withdrawalPool = new WithdrawalPool(pool); + WithdrawalPool withdrawalPool = new WithdrawalPool(pool, address(accessManager)); // Read guardians module variable address payable guardiansModule = payable(stdJson.readAddress(guardiansDeployment, ".guardianModule")); diff --git a/script/SetupAccess.s.sol b/script/SetupAccess.s.sol index b8220d09..1707da4a 100644 --- a/script/SetupAccess.s.sol +++ b/script/SetupAccess.s.sol @@ -7,6 +7,7 @@ import { stdJson } from "forge-std/StdJson.sol"; import { AccessManager } from "openzeppelin/access/manager/AccessManager.sol"; import { PufferProtocol } from "puffer/PufferProtocol.sol"; import { PufferPool } from "puffer/PufferPool.sol"; +import { IWithdrawalPool } from "puffer/interface/IWithdrawalPool.sol"; import { GuardianModule } from "puffer/GuardianModule.sol"; import { IPufferStrategy } from "puffer/interface/IPufferStrategy.sol"; import { UpgradeableBeacon } from "openzeppelin/proxy/beacon/UpgradeableBeacon.sol"; @@ -15,6 +16,7 @@ import { EnclaveVerifier } from "puffer/EnclaveVerifier.sol"; uint64 constant ROLE_ID_PUFFER_PROTOCOL = 1; uint64 constant ROLE_ID_DAO = 77; uint64 constant ROLE_ID_GUARDIANS = 88; +uint64 constant ROLE_ID_PAUSER = 999; contract SetupAccess is BaseScript { string internal pufferDeployment = vm.readFile(string.concat("./output/puffer.json")); @@ -28,61 +30,145 @@ contract SetupAccess is BaseScript { address internal guardianModule = stdJson.readAddress(guardiansDeployment, ".guardianModule"); address internal pufferProtocol = stdJson.readAddress(pufferDeployment, ".PufferProtocol"); address internal noRestakingStrategy = stdJson.readAddress(pufferDeployment, ".noRestakingStrategy"); + address internal withdrawalPool = stdJson.readAddress(pufferDeployment, ".withdrawalPool"); address internal pufferPool = stdJson.readAddress(pufferDeployment, ".pufferPool"); address internal enclaveVerifier = stdJson.readAddress(guardiansDeployment, ".enclaveVerifier"); + address internal pauser = stdJson.readAddress(guardiansDeployment, ".pauser"); function run(address DAO) external broadcast { - _grantRoles(DAO); - _setupPufferProtocolRoles(); - _setupGuardianModuleRoles(); - _setupPufferPoolRoles(); - _setupNoRestakingStrategyRoles(); - _setupUpgradeableBeacon(); - _setupEnclaveVerifierRoles(); + // We do one multicall to setup everything + bytes[] memory rolesCalldatas = _grantRoles(DAO); + bytes[] memory pufferProtocolRoles = _setupPufferProtocolRoles(); + bytes[] memory pufferPoolRoles = _setupPufferPoolRoles(); + bytes[] memory noRestakingStrategyRoles = _setupNoRestakingStrategyRoles(); + + bytes[] memory calldatas = new bytes[](16); + calldatas[0] = _setupGuardianModuleRoles(); + calldatas[1] = _setupEnclaveVerifierRoles(); + calldatas[2] = _setupWithdrawalPoolRoles(); + calldatas[3] = _setupUpgradeableBeacon(); + calldatas[4] = rolesCalldatas[0]; + calldatas[5] = rolesCalldatas[1]; + calldatas[6] = rolesCalldatas[2]; + calldatas[7] = rolesCalldatas[3]; + + calldatas[8] = pufferProtocolRoles[0]; + calldatas[9] = pufferProtocolRoles[1]; + calldatas[10] = pufferProtocolRoles[2]; + + calldatas[11] = pufferPoolRoles[0]; + calldatas[12] = pufferPoolRoles[1]; + + calldatas[13] = noRestakingStrategyRoles[0]; + calldatas[14] = noRestakingStrategyRoles[1]; + calldatas[15] = noRestakingStrategyRoles[2]; + + // calldatas[16] = _setupPauser(); + + accessManager.multicall(calldatas); } - function _setupGuardianModuleRoles() internal { + function _setupPauser() internal view returns (bytes memory) { + bytes4[] memory selectors = new bytes4[](1); + selectors[0] = AccessManager.setTargetClosed.selector; + + return abi.encodeWithSelector( + AccessManager.setTargetFunctionRole.selector, address(accessManager), selectors, ROLE_ID_PAUSER + ); + } + + function _setupWithdrawalPoolRoles() internal view returns (bytes memory) { + bytes4[] memory selectors = new bytes4[](2); + selectors[0] = bytes4(hex"4782f779"); // IWithdrawalPool.withdrawETH.selector; + selectors[1] = bytes4(hex"945fca09"); // IWithdrawalPool.withdrawETH Permit version + + return abi.encodeWithSelector( + AccessManager.setTargetFunctionRole.selector, withdrawalPool, selectors, accessManager.PUBLIC_ROLE() + ); + } + + function _setupGuardianModuleRoles() internal view returns (bytes memory) { bytes4[] memory selectors = new bytes4[](1); selectors[0] = GuardianModule.setGuardianEnclaveMeasurements.selector; - accessManager.setTargetFunctionRole(guardianModule, selectors, ROLE_ID_DAO); + return + abi.encodeWithSelector(AccessManager.setTargetFunctionRole.selector, guardianModule, selectors, ROLE_ID_DAO); } - function _setupPufferPoolRoles() internal { + function _setupPufferPoolRoles() internal returns (bytes[] memory) { + bytes[] memory calldatas = new bytes[](2); + bytes4[] memory selectors = new bytes4[](1); selectors[0] = PufferPool.transferETH.selector; - accessManager.setTargetFunctionRole(pufferPool, selectors, ROLE_ID_PUFFER_PROTOCOL); + calldatas[0] = abi.encodeWithSelector( + AccessManager.setTargetFunctionRole.selector, pufferPool, selectors, ROLE_ID_PUFFER_PROTOCOL + ); + + bytes4[] memory publicSelectors = new bytes4[](1); + publicSelectors[0] = PufferPool.depositETH.selector; + + calldatas[1] = abi.encodeWithSelector( + AccessManager.setTargetFunctionRole.selector, pufferPool, publicSelectors, accessManager.PUBLIC_ROLE() + ); + + return calldatas; } - function _setupUpgradeableBeacon() internal { + function _setupUpgradeableBeacon() internal view returns (bytes memory) { bytes4[] memory selectors = new bytes4[](1); selectors[0] = UpgradeableBeacon.upgradeTo.selector; - accessManager.setTargetFunctionRole( - PufferProtocol(pufferProtocol).PUFFER_STRATEGY_BEACON(), selectors, ROLE_ID_DAO + return abi.encodeWithSelector( + AccessManager.setTargetFunctionRole.selector, + PufferProtocol(pufferProtocol).PUFFER_STRATEGY_BEACON(), + selectors, + ROLE_ID_DAO ); } - function _setupNoRestakingStrategyRoles() internal { + function _setupNoRestakingStrategyRoles() internal view returns (bytes[] memory) { + bytes[] memory calldatas = new bytes[](3); + bytes4[] memory selectors = new bytes4[](1); selectors[0] = IPufferStrategy.callStake.selector; - accessManager.setTargetFunctionRole(noRestakingStrategy, selectors, ROLE_ID_PUFFER_PROTOCOL); + calldatas[0] = abi.encodeWithSelector( + AccessManager.setTargetFunctionRole.selector, noRestakingStrategy, selectors, ROLE_ID_PUFFER_PROTOCOL + ); bytes4[] memory selectorsForGuardians = new bytes4[](1); selectorsForGuardians[0] = bytes4(hex"abfaad62"); // signature for `function postRewardsRoot(bytes32 root, uint256 blockNumber)` - accessManager.setTargetFunctionRole(noRestakingStrategy, selectorsForGuardians, ROLE_ID_GUARDIANS); + + calldatas[1] = abi.encodeWithSelector( + AccessManager.setTargetFunctionRole.selector, noRestakingStrategy, selectorsForGuardians, ROLE_ID_GUARDIANS + ); + + bytes4[] memory publicSelectors = new bytes4[](1); + publicSelectors[0] = bytes4(hex"6f06f422"); // collectRewards + + calldatas[2] = abi.encodeWithSelector( + AccessManager.setTargetFunctionRole.selector, + noRestakingStrategy, + publicSelectors, + accessManager.PUBLIC_ROLE() + ); + + return calldatas; } - function _setupEnclaveVerifierRoles() internal { + function _setupEnclaveVerifierRoles() internal view returns (bytes memory) { bytes4[] memory selectors = new bytes4[](1); selectors[0] = EnclaveVerifier.removeLeafX509.selector; - accessManager.setTargetFunctionRole(enclaveVerifier, selectors, ROLE_ID_DAO); + return abi.encodeWithSelector( + AccessManager.setTargetFunctionRole.selector, enclaveVerifier, selectors, ROLE_ID_DAO + ); } - function _setupPufferProtocolRoles() internal { + function _setupPufferProtocolRoles() internal view returns (bytes[] memory) { + bytes[] memory calldatas = new bytes[](3); + bytes4[] memory selectors = new bytes4[](9); selectors[0] = PufferProtocol.setProtocolFeeRate.selector; selectors[1] = PufferProtocol.setSmoothingCommitments.selector; @@ -94,7 +180,9 @@ contract SetupAccess is BaseScript { selectors[7] = PufferProtocol.setGuardiansFeeRate.selector; selectors[8] = PufferProtocol.setWithdrawalPoolRate.selector; - accessManager.setTargetFunctionRole(address(pufferProtocol), selectors, ROLE_ID_DAO); + calldatas[0] = abi.encodeWithSelector( + AccessManager.setTargetFunctionRole.selector, address(pufferProtocol), selectors, ROLE_ID_DAO + ); bytes4[] memory guardianSelectors = new bytes4[](4); guardianSelectors[0] = PufferProtocol.skipProvisioning.selector; @@ -102,14 +190,32 @@ contract SetupAccess is BaseScript { guardianSelectors[2] = PufferProtocol.proofOfReserve.selector; guardianSelectors[3] = PufferProtocol.postFullWithdrawalsRoot.selector; - accessManager.setTargetFunctionRole(address(pufferProtocol), guardianSelectors, ROLE_ID_GUARDIANS); + calldatas[1] = abi.encodeWithSelector( + AccessManager.setTargetFunctionRole.selector, address(pufferProtocol), guardianSelectors, ROLE_ID_GUARDIANS + ); + + bytes4[] memory publicSelectors = new bytes4[](1); + publicSelectors[0] = PufferProtocol.registerValidatorKey.selector; + + calldatas[2] = abi.encodeWithSelector( + AccessManager.setTargetFunctionRole.selector, + address(pufferProtocol), + publicSelectors, + accessManager.PUBLIC_ROLE() + ); + + return calldatas; } - function _grantRoles(address DAO) internal { - accessManager.grantRole(ROLE_ID_DAO, DAO, 0); + function _grantRoles(address DAO) internal view returns (bytes[] memory) { + bytes[] memory calldatas = new bytes[](4); - accessManager.grantRole(ROLE_ID_GUARDIANS, guardians, 0); + calldatas[0] = abi.encodeWithSelector(AccessManager.grantRole.selector, ROLE_ID_DAO, DAO, 0); + calldatas[1] = abi.encodeWithSelector(AccessManager.grantRole.selector, ROLE_ID_GUARDIANS, guardians, 0); + calldatas[2] = + abi.encodeWithSelector(AccessManager.grantRole.selector, ROLE_ID_PUFFER_PROTOCOL, pufferProtocol, 0); + calldatas[3] = abi.encodeWithSelector(AccessManager.grantRole.selector, ROLE_ID_PAUSER, pauser, 0); - accessManager.grantRole(ROLE_ID_PUFFER_PROTOCOL, pufferProtocol, 0); + return calldatas; } } diff --git a/src/NoRestakingStrategy.sol b/src/NoRestakingStrategy.sol index 9fe5b9b1..39f37332 100644 --- a/src/NoRestakingStrategy.sol +++ b/src/NoRestakingStrategy.sol @@ -120,7 +120,7 @@ contract NoRestakingStrategy is IPufferStrategy, AccessManaged, TokenRescuer { uint256[] calldata blockNumbers, uint256[] calldata amounts, bytes32[][] calldata merkleProofs - ) external { + ) external restricted { // Anybody can submit a valid proof and the ETH will be sent to the node uint256 ethToSend = 0; diff --git a/src/PufferPool.sol b/src/PufferPool.sol index bb81f8a0..ef773277 100644 --- a/src/PufferPool.sol +++ b/src/PufferPool.sol @@ -27,17 +27,12 @@ contract PufferPool is IPufferPool, TokenRescuer, ERC20Permit, AccessManaged { AccessManaged(initialAuthority) { } - /** - * @notice no calldata automatically triggers the depositETH for `msg.sender` - */ - receive() external payable { - depositETH(); - } + receive() external payable { } /** * @inheritdoc IPufferPool */ - function depositETH() public payable returns (uint256) { + function depositETH() public payable restricted returns (uint256) { uint256 pufETHAmount = _calculateETHToPufETHAmount(msg.value); emit Deposited(msg.sender, msg.value, pufETHAmount); @@ -47,10 +42,6 @@ contract PufferPool is IPufferPool, TokenRescuer, ERC20Permit, AccessManaged { return pufETHAmount; } - function depositETHWithoutMinting() public payable { - // Deposit rewards through this function - } - /** * @inheritdoc IPufferPool */ diff --git a/src/PufferProtocol.sol b/src/PufferProtocol.sol index b6c419fa..f71cb249 100644 --- a/src/PufferProtocol.sol +++ b/src/PufferProtocol.sol @@ -112,6 +112,7 @@ contract PufferProtocol is IPufferProtocol, AccessManagedUpgradeable, UUPSUpgrad function registerValidatorKey(ValidatorKeyData calldata data, bytes32 strategyName, uint256 numberOfMonths) external payable + restricted { ProtocolStorage storage $ = _getPufferProtocolStorage(); @@ -225,11 +226,6 @@ contract PufferProtocol is IPufferProtocol, AccessManagedUpgradeable, UUPSUpgrad _transferFunds($, 0); } - function collectRewards() external { - // ProtocolStorage storage $ = _getPufferProtocolStorage(); - //@todo - } - /** * @inheritdoc IPufferProtocol */ @@ -366,9 +362,7 @@ contract PufferProtocol is IPufferProtocol, AccessManagedUpgradeable, UUPSUpgrad revert Failed(); } // slither-disable-next-line calls-loop - (success,) = IPufferStrategy(strategies[i]).call( - address($.pool), pufferPoolAmount, abi.encodeWithSelector(IPufferPool.depositETHWithoutMinting.selector) - ); + (success,) = IPufferStrategy(strategies[i]).call(address($.pool), pufferPoolAmount, ""); if (!success) { revert Failed(); } @@ -649,7 +643,7 @@ contract PufferProtocol is IPufferProtocol, AccessManagedUpgradeable, UUPSUpgrad uint256 guardiansAmount = _sendETH(address(GUARDIANS), amount, $.guardiansFeeRate); uint256 poolAmount = amount - (treasuryAmount + withdrawalPoolAmount + guardiansAmount); - $.pool.depositETHWithoutMinting{ value: poolAmount }(); + address($.pool).safeTransferETH(poolAmount); } function _setGuardiansFeeRate(uint256 newRate) internal { diff --git a/src/WithdrawalPool.sol b/src/WithdrawalPool.sol index 7b3b9582..c2535c33 100644 --- a/src/WithdrawalPool.sol +++ b/src/WithdrawalPool.sol @@ -1,8 +1,9 @@ // SPDX-License-Identifier: GPL-3.0 pragma solidity >=0.8.0 <0.9.0; -import { PufferPool } from "puffer/PufferPool.sol"; import { IWithdrawalPool } from "puffer/interface/IWithdrawalPool.sol"; +import { AccessManaged } from "openzeppelin/access/manager/AccessManaged.sol"; +import { PufferPool } from "puffer/PufferPool.sol"; import { SafeTransferLib } from "solady/utils/SafeTransferLib.sol"; import { FixedPointMathLib } from "solady/utils/FixedPointMathLib.sol"; @@ -12,7 +13,7 @@ import { FixedPointMathLib } from "solady/utils/FixedPointMathLib.sol"; * @author Puffer finance * @custom:security-contact security@puffer.fi */ -contract WithdrawalPool is IWithdrawalPool { +contract WithdrawalPool is IWithdrawalPool, AccessManaged { using SafeTransferLib for address; /** @@ -29,7 +30,7 @@ contract WithdrawalPool is IWithdrawalPool { // uint256 internal constant _withdrawalFee = FixedPointMathLib.WAD; // 1% uint256 internal constant _withdrawalFee = 0; - constructor(PufferPool pufferPool) payable { + constructor(PufferPool pufferPool, address initialAuthority) payable AccessManaged(initialAuthority) { POOL = pufferPool; } @@ -38,14 +39,14 @@ contract WithdrawalPool is IWithdrawalPool { /** * @inheritdoc IWithdrawalPool */ - function withdrawETH(address to, uint256 pufETHAmount) external returns (uint256) { + function withdrawETH(address to, uint256 pufETHAmount) external restricted returns (uint256) { return _withdrawETH(msg.sender, to, pufETHAmount); } /** * @inheritdoc IWithdrawalPool */ - function withdrawETH(address to, Permit calldata permit) external { + function withdrawETH(address to, Permit calldata permit) external restricted returns (uint256) { // @audit-issue if the attacker gets PERMIT calldata, he can steal money from the permit.owner // @audit-issue it is important that signature is not stored anywhere // @audit-issue frontend hack could cause harm here @@ -61,7 +62,7 @@ contract WithdrawalPool is IWithdrawalPool { r: permit.r }) { } catch { } - _withdrawETH(permit.owner, to, permit.amount); + return _withdrawETH(permit.owner, to, permit.amount); } function _withdrawETH(address from, address to, uint256 pufETHAmount) internal returns (uint256) { diff --git a/src/interface/IPufferPool.sol b/src/interface/IPufferPool.sol index de7ec031..18d42448 100644 --- a/src/interface/IPufferPool.sol +++ b/src/interface/IPufferPool.sol @@ -38,11 +38,6 @@ interface IPufferPool is IERC20 { */ function depositETH() external payable returns (uint256); - /** - * @notice Deposits ETH without minting any pufETH - */ - function depositETHWithoutMinting() external payable; - /** * * @notice Burns `pufETHAmount` from the transaction sender diff --git a/src/interface/IWithdrawalPool.sol b/src/interface/IWithdrawalPool.sol index c01702fa..619f9ee6 100644 --- a/src/interface/IWithdrawalPool.sol +++ b/src/interface/IWithdrawalPool.sol @@ -28,5 +28,5 @@ interface IWithdrawalPool { * Permit allows a gasless approval. Owner signs a message giving transfer approval to this contract. * @param permit is the struct required by IERC20Permit-permit */ - function withdrawETH(address to, Permit calldata permit) external; + function withdrawETH(address to, Permit calldata permit) external returns (uint256); } diff --git a/test/handlers/PufferProtocolHandler.sol b/test/handlers/PufferProtocolHandler.sol index 1f19e57d..c0586a18 100644 --- a/test/handlers/PufferProtocolHandler.sol +++ b/test/handlers/PufferProtocolHandler.sol @@ -16,10 +16,13 @@ import { ValidatorKeyData } from "puffer/struct/ValidatorKeyData.sol"; import { Validator } from "puffer/struct/Validator.sol"; import { Status } from "puffer/struct/Status.sol"; import { TestHelper } from "../helpers/TestHelper.sol"; +import { SafeTransferLib } from "solady/utils/SafeTransferLib.sol"; contract PufferProtocolHandler is Test { using EnumerableMap for EnumerableMap.AddressToUintMap; using EnumerableSet for EnumerableSet.AddressSet; + using SafeTransferLib for address; + using SafeTransferLib for address payable; TestHelper testhelper; @@ -144,7 +147,7 @@ contract PufferProtocolHandler is Test { vm.deal(address(this), stakingRewardsAmount); vm.startPrank(address(this)); - pool.depositETHWithoutMinting{ value: stakingRewardsAmount }(); + address(pool).safeTransferETH(stakingRewardsAmount); vm.stopPrank(); ghost_eth_rewards_amount += stakingRewardsAmount; diff --git a/test/mocks/PufferPoolMock.sol b/test/mocks/PufferPoolMock.sol index a25126cf..56ec881a 100644 --- a/test/mocks/PufferPoolMock.sol +++ b/test/mocks/PufferPoolMock.sol @@ -24,8 +24,6 @@ contract PufferPoolMock is IPufferPool, ERC20PermitUpgradeable { function getWithdrawalPool() external view returns (address) { } - function depositETHWithoutMinting() external payable { } - function depositETH() external payable returns (uint256) { } function burn(uint256 pufETHAmount) external { } diff --git a/test/unit/PufferPool.t.sol b/test/unit/PufferPool.t.sol index 28175053..132538fd 100644 --- a/test/unit/PufferPool.t.sol +++ b/test/unit/PufferPool.t.sol @@ -9,9 +9,12 @@ import { FixedPointMathLib } from "solady/utils/FixedPointMathLib.sol"; import { TestHelper } from "../helpers/TestHelper.sol"; import { PufferProtocol } from "puffer/PufferProtocol.sol"; import { PufferPoolStorage } from "puffer/struct/PufferPoolStorage.sol"; +import { SafeTransferLib } from "solady/utils/SafeTransferLib.sol"; contract PufferPoolTest is TestHelper { using ECDSA for bytes32; + using SafeTransferLib for address; + using SafeTransferLib for address payable; event DepositRateChanged(uint256 oldValue, uint256 newValue); @@ -121,13 +124,10 @@ contract PufferPoolTest is TestHelper { vm.deal(alice, 100 ether); vm.startPrank(bob); - (bool success,) = address(pool).call{ value: 10 ether }(""); - - assertTrue(success, "failed"); - assertEq(pool.balanceOf(bob), 10 ether, "bob balance"); + pool.depositETH{ value: 6 ether }(); + assertEq(pool.balanceOf(bob), 6 ether, "bob balance"); vm.startPrank(alice); - uint256 minted = pool.depositETH{ value: 10 ether }(); assertEq(minted, 10 ether, "amounts dont match"); @@ -159,7 +159,7 @@ contract PufferPoolTest is TestHelper { assertEq(minted, 1 ether, "minted amount"); // Simulate rewards of 1 ETH - pool.depositETHWithoutMinting{ value: 1 ether }(); + address(pool).safeTransferETH(1 ether); // Fast forward 50400 blocks ~ 7 days vm.roll(50401); diff --git a/test/unit/PufferProtocol.t.sol b/test/unit/PufferProtocol.t.sol index 3b269b5b..2abb2b37 100644 --- a/test/unit/PufferProtocol.t.sol +++ b/test/unit/PufferProtocol.t.sol @@ -683,6 +683,8 @@ contract PufferProtocolTest is TestHelper { merkleProof: aliceProof }); + assertEq(pool.balanceOf(alice), 1 ether, "alice received back the bond in pufETH"); + bytes32[] memory bobProof = new bytes32[](1); bobProof[0] = hex"6df1a3c785f77eb353a2a4c0d38629c4d4088032e8ec0695b9bbbee2bd9d4506"; @@ -699,7 +701,6 @@ contract PufferProtocolTest is TestHelper { }); assertEq(pool.balanceOf(bob), 0, "bob has zero pufETH after"); - assertEq(pool.balanceOf(alice), 1 ether, "alice received back the bond in pufETH"); } // Test smart contract upgradeability (UUPS) @@ -715,6 +716,18 @@ contract PufferProtocolTest is TestHelper { assertEq(result, 1337); } + function testPause() public { + pool.depositETH{ value: 1 ether }(); + + vm.startPrank(_broadcaster); // Admin + // Pause + accessManager.setTargetClosed(address(pool), true); + vm.stopPrank(); + + vm.expectRevert(); + pool.depositETH{ value: 1 ether }(); + } + function _getGuardianSignatures(bytes memory pubKey) internal view returns (bytes[] memory) { (bytes32 strategyName, uint256 pendingIdx) = pufferProtocol.getNextValidatorToProvision(); Validator memory validator = pufferProtocol.getValidatorInfo(strategyName, pendingIdx);