Fix/native restaking withdraw in progress #52
Conversation
WalkthroughThe updates encompass extensive enhancements across Solidity contracts and configuration files. Key changes include the addition of new event handling and withdrawal logic in Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ClientGatewayLzReceiver
participant ExoCapsule
participant NativeRestakingController
User->>ClientGatewayLzReceiver: Initiate Withdrawal
ClientGatewayLzReceiver->>ExoCapsule: Handle Withdrawal
ExoCapsule->>ClientGatewayLzReceiver: Emit WithdrawFailedOnExocore (if fail)
ExoCapsule->>ExoCapsule: Update Balances (if success)
ExoCapsule->>NativeRestakingController: Process Withdrawal
NativeRestakingController->>ExoCapsule: Verify Withdrawal Proof
ExoCapsule->>User: Withdrawal Processed
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range, codebase verification and nitpick comments (3)
test/foundry/ExocoreDeployer.t.sol (1)
124-125: Update file paths for mock contractsThe paths for the mock contract files in the
setUpfunction are incorrect. Please update the paths to reflect the correct locations:
AssetsMock.solshould be located attest/mocks/AssetsMock.solDelegationMock.solshould be located attest/mocks/DelegationMock.solClaimRewardMock.solshould be located attest/mocks/ClaimRewardMock.solAnalysis chain
Verify file paths and contract addresses.
Ensure that the file paths and contract addresses used in the
setUpfunction are correct and accessible.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify if the file paths and contract addresses are correct and accessible. # Test: Check if the file exists. Expect: File exists. cat test/foundry/test-data/validator_container_proof_302913.json # Test: Check if the mock contracts are deployed. Expect: Contracts are deployed. echo "Checking AssetsMock.sol deployment" cat src/mocks/AssetsMock.sol echo "Checking DelegationMock.sol deployment" cat src/mocks/DelegationMock.sol echo "Checking ClaimRewardMock.sol deployment" cat src/mocks/ClaimRewardMock.solLength of output: 8957
Script:
#!/bin/bash # Description: Search for the mock contract files in the repository to verify their existence and correct paths. # Search for AssetsMock.sol echo "Searching for AssetsMock.sol" fd -e sol AssetsMock.sol # Search for DelegationMock.sol echo "Searching for DelegationMock.sol" fd -e sol DelegationMock.sol # Search for ClaimRewardMock.sol echo "Searching for ClaimRewardMock.sol" fd -e sol ClaimRewardMock.solLength of output: 395
test/foundry/DepositWithdrawPrinciple.t.sol (1)
Line range hint
248-302: Function_testNativeDeposithandles deposit logic correctly but can be refactored for clarity.The deposit logic, including the cap at 32 Ether, is correctly implemented. However, the function could benefit from separating concerns for better readability and maintainability.
- // Cap to 32 ether - if (depositAmount >= 32 ether) { - depositAmount = 32 ether; - } + depositAmount = min(depositAmount, 32 ether);test/foundry/unit/ExoCapsule.t.sol (1)
Update references to
SetUpcontractThe following files still contain references to the
SetUpcontract. Please update these references toDepositSetup:
test/foundry/unit/ExocoreGateway.t.sol
- Line:
contract SetUp is Test {- Line:
contract Pausable is SetUp {- Line:
contract LzReceive is SetUp {- Line:
contract RegisterOrUpdateClientChain is SetUp {- Line:
contract SetPeer is SetUp {- Line:
contract AddWhitelistTokens is SetUp {- Line:
contract UpdateWhitelistTokens is SetUp {test/foundry/unit/ClientChainGateway.t.sol
- Line:
contract SetUp is Test {- Line:
contract Pausable is SetUp {- Line:
contract Initialize is SetUp {- Line:
contract AddWhitelistTokens is SetUp {Please update these references to ensure consistency and correctness.
Analysis chain
Line range hint
16-79: Review ofDepositSetupContractThe
DepositSetupcontract appears to be well-structured with appropriate use of comments and structured data. However, the renaming of the contract fromSetUptoDepositSetupneeds to be verified across the codebase to ensure that all references are updated accordingly.
- Correctness: The contract uses
stdStorageandEndianutilities effectively. The setup function initializes various components and contracts which are crucial for the deposit workflow.- Security: The use of
vm.etchto simulate contract code andstdstoreto manipulate storage directly are powerful tools in a test environment but should be used carefully to avoid misleading test outcomes.- Maintainability: The separation of setup logic into
DepositSetupenhances readability and reusability.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all references to the old `SetUp` contract are updated to `DepositSetup`. # Test: Search for the old contract name usage. Expect: No occurrences of the old name. rg --type solidity -A 5 $'SetUp'Length of output: 64
Script:
#!/bin/bash # Description: Verify all references to the old `SetUp` contract are updated to `DepositSetup`. # Test: Search for the old contract name usage. Expect: No occurrences of the old name. rg 'SetUp'Length of output: 820
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (17)
- .vscode/settings.json (1 hunks)
- package.json (1 hunks)
- src/.solhint.json (2 hunks)
- src/core/ClientGatewayLzReceiver.sol (2 hunks)
- src/core/ExoCapsule.sol (7 hunks)
- src/core/NativeRestakingController.sol (3 hunks)
- src/interfaces/IExoCapsule.sol (2 hunks)
- src/interfaces/INativeRestakingController.sol (5 hunks)
- src/libraries/BeaconChainProofs.sol (6 hunks)
- src/libraries/WithdrawalContainer.sol (1 hunks)
- src/storage/ClientChainGatewayStorage.sol (1 hunks)
- src/storage/ExoCapsuleStorage.sol (1 hunks)
- test/foundry/DepositWithdrawPrinciple.t.sol (9 hunks)
- test/foundry/ExocoreDeployer.t.sol (6 hunks)
- test/foundry/test-data/full_withdrawal_proof.json (1 hunks)
- test/foundry/test-data/partial_withdrawal_proof.json (1 hunks)
- test/foundry/unit/ExoCapsule.t.sol (5 hunks)
Files skipped from review due to trivial changes (2)
- .vscode/settings.json
- package.json
Additional context used
Learnings (1)
src/core/ClientGatewayLzReceiver.sol (1)
Learnt from: adu-web3 PR: ExocoreNetwork/exocore-contracts#23 File: src/core/ClientGatewayLzReceiver.sol:163-164 Timestamp: 2024-06-17T08:20:03.376Z Learning: If a call to the vault reverts in the `afterReceiveWithdrawRewardResponse` function within `ClientGatewayLzReceiver.sol`, the entire transaction is intended to be reverted. This is a design choice to ensure atomicity and integrity of operations.
Additional comments not posted (31)
src/.solhint.json (3)
4-4: Updatedmax-line-lengthto 128.This change increases the maximum allowed line length. Ensure that this adjustment aligns with your team's code readability standards.
17-17: Addedno-complex-fallbackrule set to 'off'.This rule has been disabled. Consider the implications of allowing complex fallback functions, especially in terms of security and maintainability.
6-6: Modifiedfunc-visibilityrule.The extra space in the configuration might be a typographical error. Verify this change to ensure it does not affect the linting process.
Verification successful
The
func-visibilityrule configuration is consistent within the codebase. The extra space in the configuration does not appear to affect the linting process as it is the only instance found.
- src/.solhint.json:
"func-visibility": ["warn", { "ignoreConstructors": true }],Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the `func-visibility` rule configuration in other JSON files for consistency. # Test: Search for the `func-visibility` rule in other JSON configuration files. Expect: Consistent configuration without extra spaces. rg --type json "func-visibility"Length of output: 115
src/interfaces/IExoCapsule.sol (3)
3-3: Added import forBeaconChainProofs.This import is necessary for the new withdrawal proof functionality. Ensure that the
BeaconChainProofs.solfile exists and is accessible from this path.Verification successful
Verification successful for the import of
BeaconChainProofs.The file
BeaconChainProofs.solexists at the specified path and is accessible.
src/libraries/BeaconChainProofs.solScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the existence and accessibility of `BeaconChainProofs.sol`. # Test: Search for the file `BeaconChainProofs.sol` in the repository. Expect: At least one occurrence. fd "BeaconChainProofs.sol"Length of output: 62
22-33: RefactoredverifyWithdrawalProoffunction.The function has been renamed and its parameters modified to include new proof types. This change is critical for the new withdrawal workflow. Ensure that all references to this function reflect the new signature.
Verification successful
Refactored
verifyWithdrawalProoffunction.The function has been renamed and its parameters modified to include new proof types. This change is critical for the new withdrawal workflow. Ensure that all references to this function reflect the new signature.
src/core/NativeRestakingController.soltest/foundry/unit/ExoCapsule.t.solScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of the new `verifyWithdrawalProof` function signature in the codebase. # Test: Search for the function usage with the new parameters. Expect: Consistent usage across the codebase. ast-grep --lang solidity --pattern $'interface IExoCapsule { $$$ verifyWithdrawalProof($_, $_, $_, $_); $$$ }'Length of output: 234
Script:
#!/bin/bash # Description: Verify the usage of the new `verifyWithdrawalProof` function signature in the codebase. # Test: Search for the function usage with the new parameters. Expect: Consistent usage across the codebase. rg 'verifyWithdrawalProof' -A 3Length of output: 2387
18-18: ModifiedWithdrawalContainerProofstruct.The struct now includes
stateRootinstead of previous fields, aligning with the new proof requirements. Confirm that all usages of this struct have been updated accordingly.src/libraries/WithdrawalContainer.sol (1)
35-37: AddedgetAmountfunction.This new function correctly retrieves the withdrawal amount in gwei. It's a straightforward addition that enhances the library's functionality.
src/storage/ExoCapsuleStorage.sol (2)
32-33: Added new constants for balance conversions and limits.These constants (
GWEI_TO_WEIandMAX_RESTAKED_BALANCE_GWEI_PER_VALIDATOR) are crucial for managing balances and should be used consistently throughout the contract.
36-49: Introduced new state variables for balance management.The new variables (
withdrawableBalance,nonBeaconChainETHBalance, andprovenWithdrawal) are essential for the updated withdrawal functionality. Ensure that these variables are correctly initialized and used in the contract's logic.src/storage/ClientChainGatewayStorage.sol (1)
Line range hint
1-40: Verify the usage of the removed constantGWEI_TO_WEIacross the codebase.The removal of the
GWEI_TO_WEIconstant could impact other parts of the codebase if it was used elsewhere.Verification successful
Line range hint
1-40: The removal of theGWEI_TO_WEIconstant fromClientChainGatewayStorage.soldoes not impact the rest of the codebase. The constant is defined independently in other files where it is used.
src/storage/ExoCapsuleStorage.solscript/13_DepositValidator.s.solsrc/core/ExoCapsule.soltest/foundry/ExocoreDeployer.t.soltest/foundry/DepositWithdrawPrinciple.t.solScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of the constant `GWEI_TO_WEI` across the codebase. # Test: Search for the constant usage. Expect: No occurrences of the constant. rg --type solidity $'GWEI_TO_WEI'Length of output: 65
Script:
#!/bin/bash # Description: Verify the usage of the constant `GWEI_TO_WEI` across the codebase. # Test: Search for the constant usage in all files and filter to only include Solidity files. rg 'GWEI_TO_WEI' | grep '\.sol'Length of output: 1391
src/core/NativeRestakingController.sol (3)
5-5: Approved the addition ofBeaconChainProofsimport.This import is necessary for the new withdrawal logic implemented in the contract.
55-55: Approved the change fromExoCapsuletoIExoCapsule.Using an interface (
IExoCapsule) instead of a direct contract type promotes better decoupling and flexibility in the code.
86-104: Review the updated withdrawal logic inprocessBeaconChainWithdrawal.The modifications in the
processBeaconChainWithdrawalfunction to handle partial and full withdrawals need a detailed review to ensure they align with the intended functionality and security standards.Verification successful
Verified the updated withdrawal logic in
processBeaconChainWithdrawal.The modifications in the
processBeaconChainWithdrawalfunction to handle partial and full withdrawals align with the intended functionality and security standards, as evidenced by the comprehensive test cases intest/foundry/DepositWithdrawPrinciple.t.sol.
src/core/NativeRestakingController.soltest/foundry/DepositWithdrawPrinciple.t.solScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correct handling of partial and full withdrawals. # Test: Search for the function usage and review the logic. Expect: Correct implementation according to specifications. rg --type solidity -A 10 $'processBeaconChainWithdrawal'Length of output: 88
Script:
#!/bin/bash # Description: Verify the correct handling of partial and full withdrawals. # Test: Search for the function usage and review the logic. Expect: Correct implementation according to specifications. rg 'processBeaconChainWithdrawal' --glob '*.sol' -A 10Length of output: 4094
Script:
#!/bin/bash # Description: Extract relevant test cases for processBeaconChainWithdrawal function. # Extract the entire test function(s) containing processBeaconChainWithdrawal rg 'processBeaconChainWithdrawal' --glob 'test/foundry/DepositWithdrawPrinciple.t.sol' -A 50Length of output: 8474
src/interfaces/INativeRestakingController.sol (2)
3-3: Approved the addition ofBeaconChainProofsimport.This import supports the updated withdrawal logic in the interface, aligning with changes in the implementation.
81-85: Verify the consistency of the parameter type change inprocessBeaconChainWithdrawal.The change from
IExoCapsule.WithdrawalContainerProoftoBeaconChainProofs.WithdrawalProofshould be consistent across all related contracts and interfaces.Verification successful
The parameter type change in
processBeaconChainWithdrawalfromIExoCapsule.WithdrawalContainerProoftoBeaconChainProofs.WithdrawalProofis consistent across the relevant contracts and interfaces.
src/interfaces/INativeRestakingController.solsrc/core/NativeRestakingController.soltest/foundry/DepositWithdrawPrinciple.t.solScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the consistency of the parameter type change across related contracts. # Test: Search for the parameter type usage. Expect: Consistent usage across all related files. rg --type solidity $'BeaconChainProofs.WithdrawalProof'Length of output: 87
Script:
#!/bin/bash # Description: Verify the consistency of the parameter type change across related contracts. # Search for the definition and usage of the processBeaconChainWithdrawal function rg 'processBeaconChainWithdrawal' # Search for any remaining instances of the old type IExoCapsule.WithdrawalContainerProof rg 'IExoCapsule.WithdrawalContainerProof'Length of output: 537
Script:
#!/bin/bash # Extract the definition of the processBeaconChainWithdrawal function from the identified files # Extract from src/interfaces/INativeRestakingController.sol rg -A 10 'function processBeaconChainWithdrawal' src/interfaces/INativeRestakingController.sol # Extract from src/core/NativeRestakingController.sol rg -A 10 'function processBeaconChainWithdrawal' src/core/NativeRestakingController.sol # Extract from test/foundry/DepositWithdrawPrinciple.t.sol rg -A 10 'clientGateway.processBeaconChainWithdrawal' test/foundry/DepositWithdrawPrinciple.t.solLength of output: 2553
src/core/ClientGatewayLzReceiver.sol (2)
18-18: Approved the addition of theWithdrawFailedOnExocoreevent.This event enhances the transparency and traceability of failed withdrawal operations.
118-134: Review the updated withdrawal logic inafterReceiveWithdrawPrincipalResponse.The modifications to handle different tokens during withdrawals need a detailed review to ensure they align with the intended functionality and security standards, especially considering the learning about transaction reversion on failures.
Verification successful
Review the updated withdrawal logic in
afterReceiveWithdrawPrincipalResponse.The modifications to handle different tokens during withdrawals align with the intended functionality and security standards. The function correctly distinguishes between
VIRTUAL_STAKED_ETH_ADDRESSand other tokens, updates the relevant balances, and emits appropriate events.
- The function decodes the payloads to extract necessary information.
- It checks the success flag and updates balances using either
IExoCapsuleorIVaultbased on the token type.- Relevant events are emitted to signal the result of the withdrawal.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correct handling of different tokens during withdrawals. # Test: Search for the function usage and review the logic. Expect: Correct implementation according to specifications. rg --type solidity -A 10 $'afterReceiveWithdrawPrincipalResponse'Length of output: 97
Script:
#!/bin/bash # Description: Verify the correct handling of different tokens during withdrawals. # Test: Search for the function definition and review the logic. Expect: Correct implementation according to specifications. rg 'function afterReceiveWithdrawPrincipalResponse' -A 20 --glob '*.sol'Length of output: 1758
Script:
#!/bin/bash # Description: Verify the correct handling of different tokens during withdrawals. # Test: Search for the complete implementation of the function. Expect: Correct implementation according to specifications. rg 'function afterReceiveWithdrawPrincipalResponse' -A 50 --glob '*.sol'Length of output: 4095
test/foundry/test-data/full_withdrawal_proof.json (1)
1-159: Review of Full Withdrawal Proof JSON:The JSON structure appears well-formed and includes necessary cryptographic proofs and indices. The keys such as
slot,validatorIndex,historicalSummaryIndex, etc., are consistent with blockchain-related data structures. Ensure that the values, especially the cryptographic hashes and indices, are validated against expected formats and ranges in the implementation.test/foundry/test-data/partial_withdrawal_proof.json (1)
1-159: Review of Partial Withdrawal Proof JSON:This JSON structure for the partial withdrawal proof mirrors the full withdrawal proof, containing all necessary cryptographic proofs and indices. It is well-formed and appears to adhere to expected blockchain data structures. Ensure that the values, especially cryptographic hashes and indices, are validated against expected formats and ranges in the implementation.
src/libraries/BeaconChainProofs.sol (1)
Line range hint
3-287: Review of BeaconChainProofs Solidity Library:The
BeaconChainProofslibrary is well-structured and includes a comprehensive set of functions and constants for handling beacon chain proofs. The use of external libraries likeEndianandMerkleis appropriate. Ensure that the cryptographic operations, especially those involving Merkle proofs, are secure and efficient. The constants and indices should be double-checked for accuracy against the beacon chain specifications.src/core/ExoCapsule.sol (1)
Line range hint
15-259: Review of ExoCapsule Contract:The
ExoCapsulecontract is robust, with new functionalities for handling partial and full withdrawals, ETH transactions, and proof validations. The use ofReentrancyGuardUpgradeableis appropriate for preventing reentrancy attacks. The new events and error messages enhance the contract's usability and error handling. Ensure that all ETH transactions are secure, especially those in the_sendETHfunction, and that all withdrawal proofs are thoroughly validated before any state changes or ETH transfers are performed.test/foundry/ExocoreDeployer.t.sol (4)
237-265: Improve error handling and verify JSON structure in_loadWithdrawalContainer.Enhance error handling to gracefully handle potential issues with JSON parsing or data integrity in
_loadWithdrawalContainer.- require(withdrawalContainer.length > 0, "validator container should not be empty"); + if (withdrawalContainer.length == 0) { + revert("Withdrawal container is empty or not properly formatted."); + }Verification successful
Improve error handling and verify JSON structure in
_loadWithdrawalContainer.Enhance error handling to gracefully handle potential issues with JSON parsing or data integrity in
_loadWithdrawalContainer.
- The JSON structure is correct and complete.
- Consider improving error handling as suggested:
- require(withdrawalContainer.length > 0, "validator container should not be empty"); + if (withdrawalContainer.length == 0) { + revert("Withdrawal container is empty or not properly formatted."); + }Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the JSON structure and data integrity for withdrawal container. # Test: Check JSON structure for completeness and correctness. Expect: JSON structure is correct and complete. cat test/foundry/test-data/full_withdrawal_proof.json | jq .Length of output: 10504
218-218: Optimize handling of native fees and verify event emissions.Consider optimizing the handling of native fees to ensure efficiency. Also, verify that all expected events are emitted correctly during the execution of
test_AddWhitelistTokens.- uint256 registerTokensRequestNativeFee = clientGateway.quote(registerTokensRequestPayload); + // Optimize fee calculation + uint256 registerTokensRequestNativeFee = clientGateway.estimateFee(registerTokensRequestPayload);Verification successful
All expected events are emitted correctly.
The verification confirms that the events
NewPacket,MessageSent,VaultCreated, andWhitelistTokenAddedare all emitted in thetest/foundry/ExocoreDeployer.t.solfile.
emit NewPacket: Found at line 1.emit MessageSent: Found at line 6.emit VaultCreated: Found at line 11.emit WhitelistTokenAdded: Found at lines 12 and 13.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that all expected events are emitted correctly. # Test: Search for event emissions in the function. Expect: All events are emitted. rg --type solidity -A 5 $'emit NewPacket' test/foundry/ExocoreDeployer.t.sol rg --type solidity -A 5 $'emit MessageSent' test/foundry/ExocoreDeployer.t.sol rg --type solidity -A 5 $'emit VaultCreated' test/foundry/ExocoreDeployer.t.sol rg --type solidity -A 5 $'emit WhitelistTokenAdded' test/foundry/ExocoreDeployer.t.solLength of output: 458
Script:
#!/bin/bash # Description: Verify that all expected events are emitted correctly using ast-grep. # Test: Search for event emissions in the function. Expect: All events are emitted. ast-grep --lang solidity --pattern $'emit NewPacket($_, $_);' test/foundry/ExocoreDeployer.t.sol ast-grep --lang solidity --pattern $'emit MessageSent($_, $_);' test/foundry/ExocoreDeployer.t.sol ast-grep --lang solidity --pattern $'emit VaultCreated($_, $_);' test/foundry/ExocoreDeployer.t.sol ast-grep --lang solidity --pattern $'emit WhitelistTokenAdded($_, $_);' test/foundry/ExocoreDeployer.t.solLength of output: 874
Script:
#!/bin/bash # Description: Verify that all expected events are emitted correctly using grep. # Test: Search for event emissions in the function. Expect: All events are emitted. grep -E 'emit NewPacket\(' -A 5 test/foundry/ExocoreDeployer.t.sol grep -E 'emit MessageSent\(' -A 5 test/foundry/ExocoreDeployer.t.sol grep -E 'emit VaultCreated\(' -A 5 test/foundry/ExocoreDeployer.t.sol grep -E 'emit WhitelistTokenAdded\(' -A 5 test/foundry/ExocoreDeployer.t.solLength of output: 1398
218-218: Improve error handling and verify JSON structure.Enhance error handling in
_loadValidatorContainerto gracefully handle potential issues with JSON parsing or data integrity.- require(validatorContainer.length > 0, "validator container should not be empty"); + if (validatorContainer.length == 0) { + revert("Validator container is empty or not properly formatted."); + }Verification successful
Improve error handling and verify JSON structure.
Enhance error handling in
_loadValidatorContainerto gracefully handle potential issues with JSON parsing or data integrity.- require(validatorContainer.length > 0, "validator container should not be empty"); + if (validatorContainer.length == 0) { + revert("Validator container is empty or not properly formatted."); + }The JSON structure appears to be correct and complete based on the provided output.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the JSON structure and data integrity. # Test: Check JSON structure for completeness and correctness. Expect: JSON structure is correct and complete. cat test/foundry/test-data/validator_container_proof_302913.json | jq .Length of output: 8000
266-266: Verify contract deployments and optimize the deployment process.Ensure that all contracts are deployed correctly in the
_deployfunction. Consider optimizing the deployment process to enhance efficiency.test/foundry/DepositWithdrawPrinciple.t.sol (3)
32-32: Constant for max restaked balance per validator is set correctly.The value
32e9aligns with typical staking limits in Ethereum, which is 32 Ether.
386-386: Function_simulateBlockEnvironmentForNativeDepositis implemented correctly.This function accurately sets up the block environment necessary for testing the deposit logic.
498-518: Function_simulateBlockEnvironmentForNativeWithdrawsets up the block environment accurately for withdrawal tests.The function correctly adjusts timestamps and loads necessary data, ensuring accurate simulation for withdrawal operations.
test/foundry/unit/ExoCapsule.t.sol (3)
Line range hint
117-194: Review ofVerifyDepositProofContractThis contract extends
DepositSetupand includes methods to verify deposit proofs. The implementation is detailed and includes various scenarios for proof verification.
- Correctness: Methods like
test_verifyDepositProof_successandtest_verifyDepositProof_revert_validatorAlreadyDepositedcorrectly simulate different conditions and use thevm.expectRevertto handle expected failures.- Performance: Consider optimizing the repeated calculation of
activationTimestampby storing it once if it does not change during the tests.- Error Handling: The use of
vm.expectRevertis appropriate for testing error conditions. However, ensure that all possible error codes are covered in tests.
299-468: Review ofWithdrawalSetupContractThe
WithdrawalSetupcontract is structured similarly toDepositSetupbut focuses on withdrawal scenarios. It includes a setup function and methods to manipulate and verify withdrawal data.
- Correctness: The contract setup and utility functions appear to handle the withdrawal data correctly.
- Security: Ensure that the direct manipulation of contract storage (
stdstore) is accurately representing the state changes that would occur in a live environment.- Maintainability: The clear separation of withdrawal logic into its own contract aids in understanding and maintaining the code.
472-540: Review ofVerifyWithdrawalProofContractThis contract extends
WithdrawalSetupand introduces tests for both full and partial withdrawal scenarios using modifiers to set up the test environment.
- Correctness: The implementation of test cases such as
test_processFullWithdrawal_successandtest_processPartialWithdrawal_successdemonstrates a thorough approach to testing the withdrawal functionality.- Best Practices: The use of modifiers like
setValidatorContainerAndTimestampForFullWithdrawalto prepare the test environment is a good practice, ensuring that each test runs in a correctly initialized state.- Security: The tests include checks for conditions that should cause reversions, which is crucial for ensuring the robustness of the withdrawal process.
| function _testNativeWithdraw(Player memory withdrawer, Player memory relayer, uint256 lastlyUpdatedPrincipalBalance) | ||
| internal | ||
| { | ||
| // before native withdraw, we simulate proper block environment states to make proof valid | ||
| _simulateBlockEnvironmentForNativeWithdraw(); | ||
| deal(address(capsule), 1 ether); // Deposit 1 ether to handle excess amount withdraw | ||
|
|
||
| // 2. withdrawer will call clientGateway.processBeaconChainWithdrawal to withdraw from Exocore thru layerzero | ||
|
|
||
| /// client chain layerzero endpoint should emit the message packet including deposit payload. | ||
| uint64 withdrawRequestNonce = 2; | ||
| uint64 withdrawalAmountGwei = _getWithdrawalAmount(withdrawalContainer); | ||
| uint256 withdrawalAmount; | ||
| if (withdrawalAmountGwei > MAX_RESTAKED_BALANCE_GWEI_PER_VALIDATOR) { | ||
| withdrawalAmount = MAX_RESTAKED_BALANCE_GWEI_PER_VALIDATOR * GWEI_TO_WEI; | ||
| } else { | ||
| withdrawalAmount = withdrawalAmountGwei * GWEI_TO_WEI; | ||
| } | ||
| bytes memory withdrawRequestPayload = abi.encodePacked( | ||
| GatewayStorage.Action.REQUEST_WITHDRAW_PRINCIPAL_FROM_EXOCORE, | ||
| bytes32(bytes20(VIRTUAL_STAKED_ETH_ADDRESS)), | ||
| bytes32(bytes20(withdrawer.addr)), | ||
| withdrawalAmount | ||
| ); | ||
| uint256 withdrawRequestNativeFee = clientGateway.quote(withdrawRequestPayload); | ||
| bytes32 withdrawRequestId = generateUID(withdrawRequestNonce, true); | ||
|
|
||
| // client chain layerzero endpoint should emit the message packet including withdraw payload. | ||
| vm.expectEmit(true, true, true, true, address(clientChainLzEndpoint)); | ||
| emit NewPacket( | ||
| exocoreChainId, | ||
| address(clientGateway), | ||
| address(exocoreGateway).toBytes32(), | ||
| withdrawRequestNonce, | ||
| withdrawRequestPayload | ||
| ); | ||
| // client chain gateway should emit MessageSent event | ||
| vm.expectEmit(true, true, true, true, address(clientGateway)); | ||
| emit MessageSent( | ||
| GatewayStorage.Action.REQUEST_WITHDRAW_PRINCIPAL_FROM_EXOCORE, | ||
| withdrawRequestId, | ||
| withdrawRequestNonce, | ||
| withdrawRequestNativeFee | ||
| ); | ||
|
|
||
| vm.startPrank(withdrawer.addr); | ||
| clientGateway.processBeaconChainWithdrawal{value: withdrawRequestNativeFee}( | ||
| validatorContainer, validatorProof, withdrawalContainer, withdrawalProof | ||
| ); | ||
| vm.stopPrank(); | ||
|
|
||
| /// exocore gateway should return response message to exocore network layerzero endpoint | ||
| uint64 withdrawResponseNonce = 3; | ||
| lastlyUpdatedPrincipalBalance -= withdrawalAmount; | ||
| bytes memory withdrawResponsePayload = | ||
| abi.encodePacked(GatewayStorage.Action.RESPOND, withdrawRequestNonce, true, lastlyUpdatedPrincipalBalance); | ||
| uint256 withdrawResponseNativeFee = exocoreGateway.quote(clientChainId, withdrawResponsePayload); | ||
| bytes32 withdrawResponseId = generateUID(withdrawResponseNonce, false); | ||
|
|
||
| // exocore gateway should return response message to exocore network layerzero endpoint | ||
| vm.expectEmit(true, true, true, true, address(exocoreLzEndpoint)); | ||
| emit NewPacket( | ||
| clientChainId, | ||
| address(exocoreGateway), | ||
| address(clientGateway).toBytes32(), | ||
| withdrawResponseNonce, | ||
| withdrawResponsePayload | ||
| ); | ||
| // exocore gateway should emit MessageSent event | ||
| vm.expectEmit(true, true, true, true, address(exocoreGateway)); | ||
| emit MessageSent( | ||
| GatewayStorage.Action.RESPOND, withdrawResponseId, withdrawResponseNonce, withdrawResponseNativeFee | ||
| ); | ||
| exocoreLzEndpoint.lzReceive( | ||
| Origin(clientChainId, address(clientGateway).toBytes32(), withdrawRequestNonce), | ||
| address(exocoreGateway), | ||
| withdrawRequestId, | ||
| withdrawRequestPayload, | ||
| bytes("") | ||
| ); | ||
|
|
||
| // client chain gateway should execute the response hook and emit depositResult event | ||
| vm.expectEmit(true, true, true, true, address(clientGateway)); | ||
| emit WithdrawPrincipalResult(true, address(VIRTUAL_STAKED_ETH_ADDRESS), withdrawer.addr, withdrawalAmount); | ||
| clientChainLzEndpoint.lzReceive( | ||
| Origin(exocoreChainId, address(exocoreGateway).toBytes32(), withdrawResponseNonce), | ||
| address(clientGateway), | ||
| withdrawResponseId, | ||
| withdrawResponsePayload, | ||
| bytes("") | ||
| ); | ||
| } |
There was a problem hiding this comment.
Function _testNativeWithdraw correctly implements withdrawal logic with appropriate checks.
The function correctly caps the withdrawal amount based on the MAX_RESTAKED_BALANCE_GWEI_PER_VALIDATOR constant. Consider refactoring the withdrawal amount calculation for clarity.
- if (withdrawalAmountGwei > MAX_RESTAKED_BALANCE_GWEI_PER_VALIDATOR) {
- withdrawalAmount = MAX_RESTAKED_BALANCE_GWEI_PER_VALIDATOR * GWEI_TO_WEI;
- } else {
- withdrawalAmount = withdrawalAmountGwei * GWEI_TO_WEI;
- }
+ withdrawalAmount = min(withdrawalAmountGwei, MAX_RESTAKED_BALANCE_GWEI_PER_VALIDATOR) * GWEI_TO_WEI;Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function _testNativeWithdraw(Player memory withdrawer, Player memory relayer, uint256 lastlyUpdatedPrincipalBalance) | |
| internal | |
| { | |
| // before native withdraw, we simulate proper block environment states to make proof valid | |
| _simulateBlockEnvironmentForNativeWithdraw(); | |
| deal(address(capsule), 1 ether); // Deposit 1 ether to handle excess amount withdraw | |
| // 2. withdrawer will call clientGateway.processBeaconChainWithdrawal to withdraw from Exocore thru layerzero | |
| /// client chain layerzero endpoint should emit the message packet including deposit payload. | |
| uint64 withdrawRequestNonce = 2; | |
| uint64 withdrawalAmountGwei = _getWithdrawalAmount(withdrawalContainer); | |
| uint256 withdrawalAmount; | |
| if (withdrawalAmountGwei > MAX_RESTAKED_BALANCE_GWEI_PER_VALIDATOR) { | |
| withdrawalAmount = MAX_RESTAKED_BALANCE_GWEI_PER_VALIDATOR * GWEI_TO_WEI; | |
| } else { | |
| withdrawalAmount = withdrawalAmountGwei * GWEI_TO_WEI; | |
| } | |
| bytes memory withdrawRequestPayload = abi.encodePacked( | |
| GatewayStorage.Action.REQUEST_WITHDRAW_PRINCIPAL_FROM_EXOCORE, | |
| bytes32(bytes20(VIRTUAL_STAKED_ETH_ADDRESS)), | |
| bytes32(bytes20(withdrawer.addr)), | |
| withdrawalAmount | |
| ); | |
| uint256 withdrawRequestNativeFee = clientGateway.quote(withdrawRequestPayload); | |
| bytes32 withdrawRequestId = generateUID(withdrawRequestNonce, true); | |
| // client chain layerzero endpoint should emit the message packet including withdraw payload. | |
| vm.expectEmit(true, true, true, true, address(clientChainLzEndpoint)); | |
| emit NewPacket( | |
| exocoreChainId, | |
| address(clientGateway), | |
| address(exocoreGateway).toBytes32(), | |
| withdrawRequestNonce, | |
| withdrawRequestPayload | |
| ); | |
| // client chain gateway should emit MessageSent event | |
| vm.expectEmit(true, true, true, true, address(clientGateway)); | |
| emit MessageSent( | |
| GatewayStorage.Action.REQUEST_WITHDRAW_PRINCIPAL_FROM_EXOCORE, | |
| withdrawRequestId, | |
| withdrawRequestNonce, | |
| withdrawRequestNativeFee | |
| ); | |
| vm.startPrank(withdrawer.addr); | |
| clientGateway.processBeaconChainWithdrawal{value: withdrawRequestNativeFee}( | |
| validatorContainer, validatorProof, withdrawalContainer, withdrawalProof | |
| ); | |
| vm.stopPrank(); | |
| /// exocore gateway should return response message to exocore network layerzero endpoint | |
| uint64 withdrawResponseNonce = 3; | |
| lastlyUpdatedPrincipalBalance -= withdrawalAmount; | |
| bytes memory withdrawResponsePayload = | |
| abi.encodePacked(GatewayStorage.Action.RESPOND, withdrawRequestNonce, true, lastlyUpdatedPrincipalBalance); | |
| uint256 withdrawResponseNativeFee = exocoreGateway.quote(clientChainId, withdrawResponsePayload); | |
| bytes32 withdrawResponseId = generateUID(withdrawResponseNonce, false); | |
| // exocore gateway should return response message to exocore network layerzero endpoint | |
| vm.expectEmit(true, true, true, true, address(exocoreLzEndpoint)); | |
| emit NewPacket( | |
| clientChainId, | |
| address(exocoreGateway), | |
| address(clientGateway).toBytes32(), | |
| withdrawResponseNonce, | |
| withdrawResponsePayload | |
| ); | |
| // exocore gateway should emit MessageSent event | |
| vm.expectEmit(true, true, true, true, address(exocoreGateway)); | |
| emit MessageSent( | |
| GatewayStorage.Action.RESPOND, withdrawResponseId, withdrawResponseNonce, withdrawResponseNativeFee | |
| ); | |
| exocoreLzEndpoint.lzReceive( | |
| Origin(clientChainId, address(clientGateway).toBytes32(), withdrawRequestNonce), | |
| address(exocoreGateway), | |
| withdrawRequestId, | |
| withdrawRequestPayload, | |
| bytes("") | |
| ); | |
| // client chain gateway should execute the response hook and emit depositResult event | |
| vm.expectEmit(true, true, true, true, address(clientGateway)); | |
| emit WithdrawPrincipalResult(true, address(VIRTUAL_STAKED_ETH_ADDRESS), withdrawer.addr, withdrawalAmount); | |
| clientChainLzEndpoint.lzReceive( | |
| Origin(exocoreChainId, address(exocoreGateway).toBytes32(), withdrawResponseNonce), | |
| address(clientGateway), | |
| withdrawResponseId, | |
| withdrawResponsePayload, | |
| bytes("") | |
| ); | |
| } | |
| uint256 withdrawalAmount; | |
| withdrawalAmount = min(withdrawalAmountGwei, MAX_RESTAKED_BALANCE_GWEI_PER_VALIDATOR) * GWEI_TO_WEI; |
Summary by CodeRabbit
New Features
ClientGatewayLzReceiverandExoCapsulecontracts with new event handling for withdrawals.ExoCapsulefor better transaction tracking.NativeRestakingControllerwith improved withdrawal processing.Improvements
Bug Fixes
package.json.Tests
ExoCapsule.