From f766b65fa3deaaca6d69a3b4b85b0e486cd3c64e Mon Sep 17 00:00:00 2001 From: adu Date: Fri, 23 Aug 2024 09:58:59 +0800 Subject: [PATCH] fix: fix wrongly used withdrawal index for proven withdrawal (#79) * fix: fix wrong withdrawal index * test: fix test * fix: remove unused import * fix: forge fmt --- script/13_DepositValidator.s.sol | 3 ++- src/core/ExoCapsule.sol | 26 +++++++++---------- src/core/NativeRestakingController.sol | 4 +-- src/interfaces/IExoCapsule.sol | 25 ++++-------------- src/interfaces/INativeRestakingController.sol | 5 ++-- src/libraries/BeaconChainProofs.sol | 9 +++++++ test/foundry/ExocoreDeployer.t.sol | 2 +- test/foundry/unit/ExoCapsule.t.sol | 11 +++++--- test/mocks/NonShortCircuitEndpointV2Mock.sol | 4 ++- 9 files changed, 45 insertions(+), 44 deletions(-) diff --git a/script/13_DepositValidator.s.sol b/script/13_DepositValidator.s.sol index 89b0c148..1e3af495 100644 --- a/script/13_DepositValidator.s.sol +++ b/script/13_DepositValidator.s.sol @@ -13,6 +13,7 @@ import "@layerzerolabs/lz-evm-protocol-v2/contracts/libs/GUID.sol"; import {ERC20PresetFixedSupply} from "@openzeppelin/contracts/token/ERC20/presets/ERC20PresetFixedSupply.sol"; import "forge-std/Script.sol"; +import "src/libraries/BeaconChainProofs.sol"; import "src/libraries/Endian.sol"; import {BaseScript} from "./BaseScript.sol"; @@ -24,7 +25,7 @@ contract DepositScript is BaseScript { using Endian for bytes32; bytes32[] validatorContainer; - IExoCapsule.ValidatorContainerProof validatorProof; + BeaconChainProofs.ValidatorContainerProof validatorProof; uint256 internal constant GENESIS_BLOCK_TIMESTAMP = 1_695_902_400; uint256 internal constant SECONDS_PER_SLOT = 12; diff --git a/src/core/ExoCapsule.sol b/src/core/ExoCapsule.sol index 062370e8..d2f61d47 100644 --- a/src/core/ExoCapsule.sol +++ b/src/core/ExoCapsule.sol @@ -161,11 +161,10 @@ contract ExoCapsule is ReentrancyGuardUpgradeable, ExoCapsuleStorage, IExoCapsul } /// @inheritdoc IExoCapsule - function verifyDepositProof(bytes32[] calldata validatorContainer, ValidatorContainerProof calldata proof) - external - onlyGateway - returns (uint256 depositAmount) - { + function verifyDepositProof( + bytes32[] calldata validatorContainer, + BeaconChainProofs.ValidatorContainerProof calldata proof + ) external onlyGateway returns (uint256 depositAmount) { bytes32 validatorPubkey = validatorContainer.getPubkey(); bytes32 withdrawalCredentials = validatorContainer.getWithdrawalCredentials(); Validator storage validator = _capsuleValidators[validatorPubkey]; @@ -203,7 +202,7 @@ contract ExoCapsule is ReentrancyGuardUpgradeable, ExoCapsuleStorage, IExoCapsul /// @inheritdoc IExoCapsule function verifyWithdrawalProof( bytes32[] calldata validatorContainer, - ValidatorContainerProof calldata validatorProof, + BeaconChainProofs.ValidatorContainerProof calldata validatorProof, bytes32[] calldata withdrawalContainer, BeaconChainProofs.WithdrawalProof calldata withdrawalProof ) external onlyGateway returns (bool partialWithdrawal, uint256 withdrawalAmount) { @@ -211,6 +210,7 @@ contract ExoCapsule is ReentrancyGuardUpgradeable, ExoCapsuleStorage, IExoCapsul Validator storage validator = _capsuleValidators[validatorPubkey]; uint64 withdrawalEpoch = withdrawalProof.slotRoot.getWithdrawalEpoch(); partialWithdrawal = withdrawalEpoch < validatorContainer.getWithdrawableEpoch(); + uint256 withdrawalId = uint256(withdrawalContainer.getWithdrawalIndex()); if (!validatorContainer.verifyValidatorContainerBasic()) { revert InvalidValidatorContainer(validatorPubkey); @@ -219,11 +219,11 @@ contract ExoCapsule is ReentrancyGuardUpgradeable, ExoCapsuleStorage, IExoCapsul revert UnregisteredOrWithdrawnValidatorContainer(validatorPubkey); } - if (provenWithdrawal[validatorPubkey][withdrawalProof.withdrawalIndex]) { - revert WithdrawalAlreadyProven(validatorPubkey, withdrawalProof.withdrawalIndex); + if (provenWithdrawal[validatorPubkey][withdrawalId]) { + revert WithdrawalAlreadyProven(validatorPubkey, withdrawalId); } - provenWithdrawal[validatorPubkey][withdrawalProof.withdrawalIndex] = true; + provenWithdrawal[validatorPubkey][withdrawalId] = true; // Validate if validator and withdrawal proof state roots are the same if (validatorProof.stateRoot != withdrawalProof.stateRoot) { @@ -359,10 +359,10 @@ contract ExoCapsule is ReentrancyGuardUpgradeable, ExoCapsuleStorage, IExoCapsul /// @dev Verifies a validator container. /// @param validatorContainer The validator container to verify. /// @param proof The proof of the validator container. - function _verifyValidatorContainer(bytes32[] calldata validatorContainer, ValidatorContainerProof calldata proof) - internal - view - { + function _verifyValidatorContainer( + bytes32[] calldata validatorContainer, + BeaconChainProofs.ValidatorContainerProof calldata proof + ) internal view { bytes32 beaconBlockRoot = getBeaconBlockRoot(proof.beaconBlockTimestamp); bytes32 validatorContainerRoot = validatorContainer.merkleizeValidatorContainer(); bool valid = validatorContainerRoot.isValidValidatorContainerRoot( diff --git a/src/core/NativeRestakingController.sol b/src/core/NativeRestakingController.sol index 875ce34b..abb0c204 100644 --- a/src/core/NativeRestakingController.sol +++ b/src/core/NativeRestakingController.sol @@ -94,7 +94,7 @@ abstract contract NativeRestakingController is /// @param proof The proof of the validator container. function depositBeaconChainValidator( bytes32[] calldata validatorContainer, - IExoCapsule.ValidatorContainerProof calldata proof + BeaconChainProofs.ValidatorContainerProof calldata proof ) external payable whenNotPaused nonReentrant nativeRestakingEnabled { IExoCapsule capsule = _getCapsule(msg.sender); uint256 depositValue = capsule.verifyDepositProof(validatorContainer, proof); @@ -112,7 +112,7 @@ abstract contract NativeRestakingController is /// @param withdrawalProof The proof of the withdrawal. function processBeaconChainWithdrawal( bytes32[] calldata validatorContainer, - IExoCapsule.ValidatorContainerProof calldata validatorProof, + BeaconChainProofs.ValidatorContainerProof calldata validatorProof, bytes32[] calldata withdrawalContainer, BeaconChainProofs.WithdrawalProof calldata withdrawalProof ) external payable whenNotPaused nonReentrant nativeRestakingEnabled { diff --git a/src/interfaces/IExoCapsule.sol b/src/interfaces/IExoCapsule.sol index 7fd4d44a..f285edd6 100644 --- a/src/interfaces/IExoCapsule.sol +++ b/src/interfaces/IExoCapsule.sol @@ -9,22 +9,6 @@ import {BeaconChainProofs} from "../libraries/BeaconChainProofs.sol"; /// operations. It is a contract used for native restaking. interface IExoCapsule { - /// @notice This struct contains the information needed for validator container validity verification - struct ValidatorContainerProof { - uint256 beaconBlockTimestamp; - bytes32 stateRoot; - bytes32[] stateRootProof; - bytes32[] validatorContainerRootProof; - uint256 validatorIndex; - } - - /// @notice This struct contains the information needed for withdrawal container proof verification - struct WithdrawalContainerProof { - uint256 beaconBlockTimestamp; - bytes32 stateRoot; - bytes32[] withdrawalContainerRootProof; - } - /// @notice Initializes the ExoCapsule contract with the given parameters. /// @param gateway The address of the ClientChainGateway contract. /// @param capsuleOwner The address of the ExoCapsule owner. @@ -38,9 +22,10 @@ interface IExoCapsule { /// @dev The container must not have been previously registered, must not be stale, /// must be activated at a previous epoch, must have the correct withdrawal credentials, /// and must have a valid container root. - function verifyDepositProof(bytes32[] calldata validatorContainer, ValidatorContainerProof calldata proof) - external - returns (uint256); + function verifyDepositProof( + bytes32[] calldata validatorContainer, + BeaconChainProofs.ValidatorContainerProof calldata proof + ) external returns (uint256); /// @notice Verifies the withdrawal proof and returns the partial withdrawal status and the withdrawal amount. /// @param validatorContainer The validator container. @@ -55,7 +40,7 @@ interface IExoCapsule { /// withdrawable epoch of the validator. function verifyWithdrawalProof( bytes32[] calldata validatorContainer, - ValidatorContainerProof calldata validatorProof, + BeaconChainProofs.ValidatorContainerProof calldata validatorProof, bytes32[] calldata withdrawalContainer, BeaconChainProofs.WithdrawalProof calldata withdrawalProof ) external returns (bool partialWithdrawal, uint256 withdrawalAmount); diff --git a/src/interfaces/INativeRestakingController.sol b/src/interfaces/INativeRestakingController.sol index 69a4ffe4..818f1191 100644 --- a/src/interfaces/INativeRestakingController.sol +++ b/src/interfaces/INativeRestakingController.sol @@ -3,7 +3,6 @@ pragma solidity ^0.8.19; import {BeaconChainProofs} from "../libraries/BeaconChainProofs.sol"; import {IBaseRestakingController} from "./IBaseRestakingController.sol"; -import {IExoCapsule} from "./IExoCapsule.sol"; /// @title INativeRestakingController /// @author ExocoreNetwork @@ -35,7 +34,7 @@ interface INativeRestakingController is IBaseRestakingController { /// @param proof The proof needed to verify the validator container. function depositBeaconChainValidator( bytes32[] calldata validatorContainer, - IExoCapsule.ValidatorContainerProof calldata proof + BeaconChainProofs.ValidatorContainerProof calldata proof ) external payable; /// @notice Processes a partial withdrawal from the beacon chain to an ExoCapsule contract. @@ -52,7 +51,7 @@ interface INativeRestakingController is IBaseRestakingController { /// block root. function processBeaconChainWithdrawal( bytes32[] calldata validatorContainer, - IExoCapsule.ValidatorContainerProof calldata validatorProof, + BeaconChainProofs.ValidatorContainerProof calldata validatorProof, bytes32[] calldata withdrawalContainer, BeaconChainProofs.WithdrawalProof calldata withdrawalProof ) external payable; diff --git a/src/libraries/BeaconChainProofs.sol b/src/libraries/BeaconChainProofs.sol index 18e112b5..7506fcf4 100644 --- a/src/libraries/BeaconChainProofs.sol +++ b/src/libraries/BeaconChainProofs.sol @@ -72,6 +72,15 @@ library BeaconChainProofs { // slither-disable-next-line unused-state uint64 internal constant SECONDS_PER_EPOCH = SLOTS_PER_EPOCH * SECONDS_PER_SLOT; + /// @notice This struct contains the information needed for validator container validity verification + struct ValidatorContainerProof { + uint256 beaconBlockTimestamp; + uint256 validatorIndex; + bytes32 stateRoot; + bytes32[] stateRootProof; + bytes32[] validatorContainerRootProof; + } + /// @notice This struct contains the merkle proofs and leaves needed to verify a partial/full withdrawal struct WithdrawalProof { bytes32[] withdrawalContainerRootProof; diff --git a/test/foundry/ExocoreDeployer.t.sol b/test/foundry/ExocoreDeployer.t.sol index 8bd9e442..1b8bcb68 100644 --- a/test/foundry/ExocoreDeployer.t.sol +++ b/test/foundry/ExocoreDeployer.t.sol @@ -81,7 +81,7 @@ contract ExocoreDeployer is Test { bytes32[] validatorContainer; bytes32 beaconBlockRoot; // latest beacon block root - IExoCapsule.ValidatorContainerProof validatorProof; + BeaconChainProofs.ValidatorContainerProof validatorProof; bytes32[] withdrawalContainer; BeaconChainProofs.WithdrawalProof withdrawalProof; diff --git a/test/foundry/unit/ExoCapsule.t.sol b/test/foundry/unit/ExoCapsule.t.sol index 102538ac..17cd41ca 100644 --- a/test/foundry/unit/ExoCapsule.t.sol +++ b/test/foundry/unit/ExoCapsule.t.sol @@ -28,7 +28,7 @@ contract DepositSetup is Test { * uint256 validatorContainerRootIndex; * } */ - IExoCapsule.ValidatorContainerProof validatorProof; + BeaconChainProofs.ValidatorContainerProof validatorProof; bytes32 beaconBlockRoot; ExoCapsule capsule; @@ -312,7 +312,7 @@ contract WithdrawalSetup is Test { * uint256 validatorIndex; * } */ - IExoCapsule.ValidatorContainerProof validatorProof; + BeaconChainProofs.ValidatorContainerProof validatorProof; bytes32[] withdrawalContainer; BeaconChainProofs.WithdrawalProof withdrawalProof; @@ -466,11 +466,16 @@ contract WithdrawalSetup is Test { return vc[6].fromLittleEndianUint64(); } + function _getWithdrawalIndex(bytes32[] storage wc) internal view returns (uint64) { + return wc[0].fromLittleEndianUint64(); + } + } contract VerifyWithdrawalProof is WithdrawalSetup { using BeaconChainProofs for bytes32; + using WithdrawalContainer for bytes32[]; using stdStorage for StdStorage; function test_NonBeaconChainETHWithdraw() public { @@ -510,7 +515,7 @@ contract VerifyWithdrawalProof is WithdrawalSetup { abi.encodeWithSelector( ExoCapsule.WithdrawalAlreadyProven.selector, _getPubkey(validatorContainer), - withdrawalProof.withdrawalIndex + uint256(_getWithdrawalIndex(withdrawalContainer)) ) ); capsule.verifyWithdrawalProof(validatorContainer, validatorProof, withdrawalContainer, withdrawalProof); diff --git a/test/mocks/NonShortCircuitEndpointV2Mock.sol b/test/mocks/NonShortCircuitEndpointV2Mock.sol index 8744ec7f..b4c2b661 100644 --- a/test/mocks/NonShortCircuitEndpointV2Mock.sol +++ b/test/mocks/NonShortCircuitEndpointV2Mock.sol @@ -718,7 +718,9 @@ contract NonShortCircuitEndpointV2Mock is ILayerZeroEndpointV2, MessagingContext } else if (optionType == ExecutorOptions.OPTION_TYPE_ORDERED_EXECUTION) { // ordered = true; } - else revert IExecutorFeeLib.Executor_UnsupportedOptionType(optionType); + else { + revert IExecutorFeeLib.Executor_UnsupportedOptionType(optionType); + } } if (cursor != _options.length) {