feat: enable Ethereum native restaking for Bootstrap #121
Conversation
WalkthroughThis pull request introduces significant changes to the staking and deposit functionality within the Ethereum ecosystem. Key modifications include renaming the Changes
Possibly related PRs
Suggested reviewers
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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
Linting failed. Please check the logs. |
|
The tests have failed. Please check the logs. |
|
Storage layout compatibility check failed. This could be due to a mismatch between Bootstrap and ClientChainGateway, or incompatibility with deployed contracts on Sepolia. Please check the logs for details. |
1 similar comment
|
Storage layout compatibility check failed. This could be due to a mismatch between Bootstrap and ClientChainGateway, or incompatibility with deployed contracts on Sepolia. Please check the logs for details. |
adu-web3
force-pushed
the
nst-for-bootstrap
branch
from
255edd0 to
4bb7cea
Compare
adu-web3
requested review from
bwhour,
MaxMustermann2,
magj2006,
cloud8little,
cary007,
trestinlsd,
mikebraver,
TimmyExogenous and
leonz789
|
Storage layout compatibility check failed. This could be due to a mismatch between Bootstrap and ClientChainGateway, or incompatibility with deployed contracts on Sepolia. Please check the logs for details. |
1 similar comment
|
Storage layout compatibility check failed. This could be due to a mismatch between Bootstrap and ClientChainGateway, or incompatibility with deployed contracts on Sepolia. Please check the logs for details. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 17
🧹 Outside diff range and nitpick comments (18)
src/interfaces/INativeRestakingController.sol (2)
59-63: Consider enhancing documentation and security specifications.
A few suggestions for improvement:
- Consider adding an
@noticecomment to explain the purpose more clearly. - Move the parameter constraint from
@devto@param amountToWithdraw. - Consider adding
@returndocumentation. - Consider specifying access control requirements in the interface documentation.
Apply this documentation improvement:
- /// @notice Withdraws the nonBeaconChainETHBalance from the ExoCapsule contract.
- /// @dev @param amountToWithdraw can not be greater than the available nonBeaconChainETHBalance.
- /// @param recipient The payable destination address to which the ETH are sent.
- /// @param amountToWithdraw The amount to withdraw.
+ /// @notice Withdraws non-beacon chain ETH balance from the ExoCapsule contract
+ /// @dev Only callable by authorized roles (implementation should enforce access control)
+ /// @param recipient The payable destination address to receive the ETH
+ /// @param amountToWithdraw The amount of ETH to withdraw (must not exceed available non-beacon chain balance)
+ /// @return success Boolean indicating whether the withdrawal was successfulLine range hint 1-64: Interface design aligns well with native restaking requirements.
The interface provides a complete set of operations needed for Ethereum native restaking:
- Staking via
stake() - ExoCapsule creation via
createExoCapsule() - Deposit verification via
verifyAndDepositNativeStake() - Withdrawal processing via
processBeaconChainWithdrawal() - Non-beacon chain ETH withdrawal via
withdrawNonBeaconChainETHFromCapsule()
This design effectively supports the workflow where stakers can deposit their beacon chain ETH into Exocore through their capsule contracts.
script/13_DepositValidator.s.sol (1)
Line range hint 83-87: Improve error handling in deployment script.
The current error handling is insufficient for a deployment script:
- The catch block silently continues with "fire anyway"
- No error details are logged
- No deployment status is tracked
Consider implementing more robust error handling:
- try clientGateway.verifyAndDepositNativeStake{value: nativeFee}(validatorContainer, validatorProof) {
- console.log("finish");
- } catch {
- console.log("fire anyway");
- }
+ try clientGateway.verifyAndDepositNativeStake{value: nativeFee}(validatorContainer, validatorProof) {
+ console.log("Successfully deposited native stake");
+ return true;
+ } catch Error(string memory reason) {
+ console.log("Deposit failed with reason:", reason);
+ return false;
+ } catch Panic(uint errorCode) {
+ console.log("Deposit failed with panic code:", errorCode);
+ return false;
+ } catch (bytes memory lowLevelData) {
+ console.log("Deposit failed with low level error:", vm.toString(lowLevelData));
+ return false;
+ }src/core/NativeRestakingController.sol (4)
Line range hint 61-89: Fix potential reentrancy vulnerability in createExoCapsule.
The function writes to state (ownerToCapsule) before making an external call to capsule.initialize(), which violates the checks-effects-interactions pattern. Additionally, there's no check if a contract already exists at the computed Create2 address.
Suggested fix:
function createExoCapsule() public whenNotPaused nativeRestakingEnabled returns (address) {
if (address(ownerToCapsule[msg.sender]) != address(0)) {
revert Errors.NativeRestakingControllerCapsuleAlreadyCreated();
}
+ address computedAddress = Create2.computeAddress(
+ bytes32(uint256(uint160(msg.sender))),
+ keccak256(abi.encodePacked(
+ BEACON_PROXY_BYTECODE.getBytecode(),
+ abi.encode(address(EXO_CAPSULE_BEACON), "")
+ ))
+ );
+ if (computedAddress.code.length > 0) {
+ revert Errors.NativeRestakingControllerCapsuleAlreadyExists();
+ }
IExoCapsule capsule = IExoCapsule(
Create2.deploy(
0,
bytes32(uint256(uint160(msg.sender))),
abi.encodePacked(BEACON_PROXY_BYTECODE.getBytecode(), abi.encode(address(EXO_CAPSULE_BEACON), ""))
)
);
+ capsule.initialize(address(this), payable(msg.sender), BEACON_ORACLE_ADDRESS);
ownerToCapsule[msg.sender] = capsule;
- capsule.initialize(address(this), payable(msg.sender), BEACON_ORACLE_ADDRESS);
emit CapsuleCreated(msg.sender, address(capsule));
return address(capsule);
}Line range hint 105-134: Add event emission for partial withdrawals and amount validation.
The function handles partial withdrawals silently without emitting events, making it difficult to track these operations off-chain. Additionally, there's no minimum withdrawal amount validation.
Consider these improvements:
function processBeaconChainWithdrawal(
bytes32[] calldata validatorContainer,
BeaconChainProofs.ValidatorContainerProof calldata validatorProof,
bytes32[] calldata withdrawalContainer,
BeaconChainProofs.WithdrawalProof calldata withdrawalProof
) external payable whenNotPaused nonReentrant nativeRestakingEnabled {
IExoCapsule capsule = _getCapsule(msg.sender);
(bool partialWithdrawal, uint256 withdrawalAmount) =
capsule.verifyWithdrawalProof(validatorContainer, validatorProof, withdrawalContainer, withdrawalProof);
+ if (withdrawalAmount == 0) {
+ revert Errors.NativeRestakingControllerInvalidWithdrawalAmount();
+ }
if (!partialWithdrawal) {
// request full withdraw
bytes32 validatorPubkey = validatorContainer.getPubkey();
bytes memory actionArgs = abi.encodePacked(validatorPubkey, bytes32(bytes20(msg.sender)), withdrawalAmount);
bytes memory encodedRequest = abi.encode(VIRTUAL_NST_ADDRESS, msg.sender, withdrawalAmount);
_processRequest(Action.REQUEST_WITHDRAW_NST, actionArgs, encodedRequest);
+ } else {
+ emit PartialWithdrawalProcessed(msg.sender, withdrawalAmount);
}
}Line range hint 135-150: Add recipient and amount validation for withdrawals.
The function should validate the recipient address and withdrawal amount before proceeding with the withdrawal.
Consider adding these validations:
function withdrawNonBeaconChainETHFromCapsule(address payable recipient, uint256 amountToWithdraw)
external
whenNotPaused
nonReentrant
nativeRestakingEnabled
{
+ if (recipient == address(0)) {
+ revert Errors.NativeRestakingControllerInvalidRecipient();
+ }
+ if (amountToWithdraw == 0) {
+ revert Errors.NativeRestakingControllerInvalidWithdrawalAmount();
+ }
IExoCapsule capsule = _getCapsule(msg.sender);
capsule.withdrawNonBeaconChainETHBalance(recipient, amountToWithdraw);
}Line range hint 1-150: Verify security measures for critical operations.
This contract handles significant ETH amounts and critical validator operations. Please ensure:
- Comprehensive test coverage for all edge cases
- Emergency pause mechanisms are properly tested
- Access control for admin functions is adequately restricted
Consider:
- Adding circuit breaker patterns for large withdrawals
- Implementing withdrawal limits
- Adding delay periods for significant operations
test/foundry/unit/ClientChainGateway.t.sol (1)
125-131: Consider adding address validation for config parameters
While the ImmutableConfig initialization is well-structured and consolidates the configuration parameters effectively, consider adding validation to ensure none of the addresses are zero addresses before initialization.
BootstrapStorage.ImmutableConfig memory config = BootstrapStorage.ImmutableConfig({
exocoreChainId: exocoreChainId,
beaconOracleAddress: address(beaconOracle),
vaultBeacon: address(vaultBeacon),
exoCapsuleBeacon: address(capsuleBeacon),
beaconProxyBytecode: address(beaconProxyBytecode)
});
+require(address(beaconOracle) != address(0), "Invalid beaconOracle address");
+require(address(vaultBeacon) != address(0), "Invalid vaultBeacon address");
+require(address(capsuleBeacon) != address(0), "Invalid capsuleBeacon address");
+require(address(beaconProxyBytecode) != address(0), "Invalid beaconProxyBytecode address");docs/native_deposit_workflow.wsd (1)
Line range hint 55-98: Align flowchart conditions with actual function logic
In the flowchart for verifyAndDepositNativeStake(), some conditional branches may not accurately reflect the implemented logic. Specifically:
- The check
if (msg.sender != gateway)should occur before interacting with theExoCapsule, as permission checks are typically done upfront. - The use of
forkmay be unnecessary if the branches are not truly parallel processes.
Apply this diff to correct the flowchart:
start
+:Check if caller is gateway;
if (msg.sender != gateway) then (yes)
- :Revert with "ExoCapsule: only client chain gateway could call this function";
+ :Revert with UnauthorizedCaller error;
stop
endif
:Get the capsule associated with the message sender;
if (capsule == address(0)) then (yes)
:Revert with CapsuleDoesNotExist error;
stop
endif
-:Call capsule.verifyDepositProof(validatorContainer, proof);
-fork
:Call capsule.verifyDepositProof(validatorContainer, proof);This revision ensures the flowchart mirrors the function's actual control flow, improving maintainability and readability.
script/7_DeployBootstrap.s.sol (2)
67-68: Fix comment formatting for clarity
There is an extra /// on line 68~, which may be unintended. Adjusting the comment formatting will improve code readability.
Apply this diff to correct the comment:
-/// deploy vault implementation contract, capsule implementation contract, reward vault implementation contract
-///
+// Deploy vault, capsule, and reward vault implementation contracts72-72: Adjust comment for consistency and clarity
Ensure that the comment on line 72~ accurately describes the subsequent code and follows consistent formatting.
Apply this diff:
-/// deploy the vault beacon, capsule beacon, reward vault beacon that store the implementation contract address
+// Deploy the vault and capsule beacons that store the implementation contract addressestest/foundry/BootstrapDepositNST.t.sol (1)
145-168: Add error handling for file operations
When reading the validator data from a JSON file, the code does not handle potential errors such as the file not existing or the data being malformed. Adding error handling will make the tests more robust and prevent unexpected failures.
test/foundry/DepositWithdrawPrinciple.t.sol (2)
304-304: Improve comment clarity and grammar
Consider rephrasing the comment for better readability:
-// 1. next depositor call clientGateway.verifyAndDepositNativeStake to deposit into Exocore from client chain through layerzero
+// 1. Depositor calls clientGateway.verifyAndDepositNativeStake to deposit into Exocore from client chain through LayerZero334-334: Improve comment clarity and grammar
Consider rephrasing the comment for clarity:
-/// call verifyAndDepositNativeStake to see if these events are emitted as expected
+/// Call verifyAndDepositNativeStake to confirm that the expected events are emitted as expectedsrc/core/Bootstrap.sol (2)
442-454: Placeholder Functions for Future Reward Mechanisms
The functions submitReward, claimRewardFromExocore, and withdrawReward currently revert with Errors.NotYetSupported(). This indicates that reward mechanisms are planned for future implementation.
Would you like assistance in outlining or implementing these reward functions?
788-796: Implement processBeaconChainWithdrawal Functionality
The processBeaconChainWithdrawal function currently reverts with Errors.NotYetSupported(). Implementing this function is crucial for handling withdrawal proofs from the beacon chain.
Would you like assistance in drafting the implementation for this function?
test/foundry/unit/Bootstrap.t.sol (2)
32-33: Remove duplicate import of Create2.
There is a duplicate import of Create2 at lines 32 and 33. Removing the redundant import will clean up the code.
Apply this diff to remove the duplicate import:
-import {Create2} from "@openzeppelin/contracts/utils/Create2.sol";
1671-1680: Use assertEq instead of assertTrue for value comparisons.
Using assertEq in your assertions provides clearer output when tests fail by displaying the expected and actual values. Consider replacing assertTrue with assertEq for better test diagnostics.
Apply this diff to improve the assertions:
- assertTrue(
- bootstrap.totalDepositAmounts(addrs[0], VIRTUAL_STAKED_ETH_ADDRESS) == initialDeposit + expectedDepositValue
- );
+ assertEq(
+ bootstrap.totalDepositAmounts(addrs[0], VIRTUAL_STAKED_ETH_ADDRESS),
+ initialDeposit + expectedDepositValue,
+ "Total deposit amount mismatch after verifying and depositing native stake."
+ );
- assertTrue(
- bootstrap.withdrawableAmounts(addrs[0], VIRTUAL_STAKED_ETH_ADDRESS)
- == initialWithdrawable + expectedDepositValue
- );
+ assertEq(
+ bootstrap.withdrawableAmounts(addrs[0], VIRTUAL_STAKED_ETH_ADDRESS),
+ initialWithdrawable + expectedDepositValue,
+ "Withdrawable amount mismatch after verifying and depositing native stake."
+ );
- assertTrue(
- bootstrap.depositsByToken(VIRTUAL_STAKED_ETH_ADDRESS) == initialDepositsByToken + expectedDepositValue
- );
+ assertEq(
+ bootstrap.depositsByToken(VIRTUAL_STAKED_ETH_ADDRESS),
+ initialDepositsByToken + expectedDepositValue,
+ "Deposits by token mismatch after verifying and depositing native stake."
+ );📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (19)
- docs/native_deposit_workflow.wsd (2 hunks)
- script/12_RedeployClientChainGateway.s.sol (2 hunks)
- script/13_DepositValidator.s.sol (1 hunks)
- script/14_CorrectBootstrapErrors.s.sol (3 hunks)
- script/2_DeployBoth.s.sol (2 hunks)
- script/7_DeployBootstrap.s.sol (3 hunks)
- script/integration/1_DeployBootstrap.s.sol (5 hunks)
- src/core/Bootstrap.sol (5 hunks)
- src/core/ClientChainGateway.sol (1 hunks)
- src/core/NativeRestakingController.sol (1 hunks)
- src/interfaces/INativeRestakingController.sol (2 hunks)
- src/storage/BootstrapStorage.sol (6 hunks)
- src/storage/ClientChainGatewayStorage.sol (1 hunks)
- test/foundry/BootstrapDepositNST.t.sol (1 hunks)
- test/foundry/DepositWithdrawPrinciple.t.sol (2 hunks)
- test/foundry/ExocoreDeployer.t.sol (2 hunks)
- test/foundry/Governance.t.sol (2 hunks)
- test/foundry/unit/Bootstrap.t.sol (15 hunks)
- test/foundry/unit/ClientChainGateway.t.sol (2 hunks)
🧰 Additional context used
📓 Learnings (1)
test/foundry/unit/ClientChainGateway.t.sol (1)
Learnt from: MaxMustermann2
PR: ExocoreNetwork/exocore-contracts#16
File: test/foundry/Bootstrap.t.sol:156-156
Timestamp: 2024-10-09T01:26:20.424Z
Learning: The `setClientChainGatewayLogic` method in `src/core/Bootstrap.sol` now includes checks to ensure the address is non-zero and the initialization data is at least 4 bytes long to include the function selector.
🔇 Additional comments (43)
src/interfaces/INativeRestakingController.sol (1)
Line range hint 35-39: LGTM! Function rename improves clarity.
The renaming from depositBeaconChainValidator to verifyAndDepositNativeStake better reflects the function's dual responsibility of verification and deposit handling.
script/12_RedeployClientChainGateway.s.sol (2)
7-7: LGTM! Import changes are well-organized.
The new BootstrapStorage import is properly placed with other core imports and is necessary for the ImmutableConfig struct usage.
Also applies to: 9-9
61-73: Well-structured configuration changes, verify constructor parameters.
The refactoring to use ImmutableConfig improves code organization and maintainability. The struct encapsulates related parameters logically, aligning with the PR's architectural changes.
Let's verify the constructor parameter alignment:
✅ Verification successful
Constructor parameters match the deployment script correctly
The verification confirms that the constructor parameters in the deployment script match the implementation:
- The
ClientChainGatewayconstructor signature matches exactly:
constructor(address endpoint_, ImmutableConfig memory params_, address rewardVaultBeacon_)- The
ImmutableConfigstruct inBootstrapStorage.solcontains all the expected fields in the correct order:
struct ImmutableConfig {
uint32 exocoreChainId;
address beaconOracleAddress;
address vaultBeacon;
address exoCapsuleBeacon;
address beaconProxyBytecode;
}The deployment script correctly provides all required parameters in the proper order:
address(clientChainLzEndpoint)forendpoint_configstruct with all required fields forparams_address(rewardVaultBeacon)forrewardVaultBeacon_
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify ClientChainGateway constructor signature matches the deployment script
# Test: Search for ClientChainGateway constructor definition
ast-grep --pattern 'contract ClientChainGateway {
$$$
constructor($$$) {
$$$
}
$$$
}'
Length of output: 99
Script:
#!/bin/bash
# Let's try a different approach to find the ClientChainGateway constructor
# First, let's find the file containing ClientChainGateway
fd "ClientChainGateway" --type f
# Then use ripgrep to search for constructor with context
rg "constructor.*ClientChainGateway" -A 10
Length of output: 315
Script:
#!/bin/bash
# Let's examine the ClientChainGateway.sol file directly
cat src/core/ClientChainGateway.sol | rg "constructor" -A 10 -B 2
# Also check the storage contract for the ImmutableConfig struct definition
cat src/storage/ClientChainGatewayStorage.sol | rg "struct ImmutableConfig" -A 10
Length of output: 1066
Script:
#!/bin/bash
# Let's check the storage contract directly since grep didn't find the struct
cat src/storage/ClientChainGatewayStorage.sol
Length of output: 3786
Script:
#!/bin/bash
# Let's check the BootstrapStorage.sol since it contains the ImmutableConfig struct
cat src/storage/BootstrapStorage.sol
Length of output: 23424
src/core/NativeRestakingController.sol (2)
Line range hint 90-104: Add validation and error handling for deposit verification.
The function needs additional safeguards and clarification:
- No validation of validatorContainer format/length
- No minimum deposit value check
- Comment states "_processRequest is a must-succeed action" but no explanation of failure handling
Please clarify:
- What happens if _processRequest fails?
- Are there any recovery mechanisms?
#!/bin/bash
# Search for _processRequest implementation and error handling
rg -A 10 "_processRequest.*Action\.REQUEST_DEPOSIT_NST"Line range hint 38-60: Add input validation for BLS public key and signature.
While the function correctly validates the ETH amount and follows the checks-effects-interactions pattern, it lacks validation for the BLS public key and signature format. This could lead to failed deposits if malformed data is provided.
Consider adding these validations:
- Verify pubkey length (48 bytes for BLS12-381)
- Verify signature length (96 bytes for BLS12-381)
- Check if validator already exists to prevent duplicate deposits
script/14_CorrectBootstrapErrors.s.sol (5)
22-22: LGTM: Import aligns with new configuration structure.
The addition of BootstrapStorage import supports the new configuration management approach using ImmutableConfig.
76-81: LGTM: Enhanced validation for critical components.
The additional checks for beaconOracle and capsuleBeacon addresses ensure all required components for native restaking are properly configured.
93-101: LGTM: Well-structured configuration management.
The ImmutableConfig struct provides a clean and organized way to manage configuration parameters, improving code maintainability and reducing the risk of initialization errors.
102-103: Verify upgrade compatibility.
The upgrade process involves both a new implementation and initialization. Please ensure:
- The new Bootstrap implementation is backward compatible with existing state
- The reinitializer version (2) mentioned in the comment is correctly set in the Bootstrap contract
#!/bin/bash
# Check reinitializer version in Bootstrap contract
ast-grep --pattern 'function initialize($$$) $_ {
$$$
reinitializer(2)
$$$
}'Also applies to: 116-117
102-102: Verify Bootstrap constructor parameters.
While the configuration looks correct, please ensure that the clientChainLzEndpoint and config parameters are properly validated in the Bootstrap contract's constructor.
✅ Verification successful
Based on the code examination, I can now generate the final response:
Constructor parameters are properly validated in Bootstrap contract
The constructor parameters are properly validated through multiple mechanisms:
- The
endpoint_parameter is validated in the parentOAppCoreUpgradeableconstructor - The
paramsstruct is validated in the parentBootstrapStorageconstructor - The constructor immediately disables initializers as a security measure
Additionally, from the learnings, we know that setClientChainGatewayLogic method (which is part of the initialization flow) includes checks to ensure addresses are non-zero and initialization data is valid.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check Bootstrap contract's constructor validation
ast-grep --pattern 'contract Bootstrap {
$$$
constructor(address $_endpoint, $_ memory $_config) {
$$$
require($_endpoint != address(0), $_);
$$$
}
}'
Length of output: 241
Script:
#!/bin/bash
# Let's try to find and examine the Bootstrap contract's constructor
rg "constructor.*Bootstrap" -A 10
Length of output: 767
Script:
#!/bin/bash
# Let's try to find the Bootstrap contract file first and then examine its contents
fd Bootstrap.sol --type f --exec cat {}
Length of output: 35626
script/2_DeployBoth.s.sol (3)
13-13: LGTM: Import statement is correctly placed and necessary.
The addition of BootstrapStorage import aligns with the new ImmutableConfig struct usage.
63-74: LGTM: Implementation and beacon deployments follow best practices.
The deployment sequence is well-structured with each implementation paired with its corresponding beacon. The simplified comment style maintains clarity while being more concise.
75-87: Verify parameter validation in ClientChainGateway constructor.
While grouping configuration parameters into an ImmutableConfig struct improves code organization, we should ensure proper validation of these parameters in the ClientChainGateway constructor to prevent initialization with zero addresses.
Let's verify the parameter validation in the ClientChainGateway constructor:
✅ Verification successful
Parameter validation is properly implemented in parent contracts
The validation concern is addressed through proper inheritance chain:
ClientChainGatewayinherits fromClientChainGatewayStorageandOAppCoreUpgradeableClientChainGatewayStorageinherits fromBootstrapStorageBootstrapStorageimplements comprehensive validation for all ImmutableConfig parameters:- Validates exocoreChainId is not zero
- Validates beaconOracleAddress is not zero address
- Validates vaultBeacon is not zero address
- Additionally,
ClientChainGatewayStoragevalidates the rewardVaultBeacon parameter
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if ClientChainGateway validates ImmutableConfig parameters
# Test: Search for parameter validation in the constructor
ast-grep --pattern 'constructor($config, $_) {
$$$
require($config.$_ != address(0), $_);
$$$
}'
# Test: Search for any other validation patterns
rg -A 3 'require.*config\.' src/
Length of output: 139
Script:
#!/bin/bash
# Let's try a broader search to find the ClientChainGateway constructor implementation
# Search for ClientChainGateway constructor
ast-grep --pattern 'contract ClientChainGateway {
$$$
constructor($$$) {
$$$
}
$$$
}'
# Also search for any require statements in ClientChainGateway
rg -B 2 -A 5 'ClientChainGateway' --type sol
Length of output: 172
Script:
#!/bin/bash
# Let's try finding the file first and then examine its contents
fd ClientChainGateway
# Then search for constructor implementation
rg -B 2 -A 10 "constructor.*ImmutableConfig"
Length of output: 4412
test/foundry/Governance.t.sol (4)
38-38: LGTM: Import statement is correctly placed and necessary.
The import of BootstrapStorage is required for using the ImmutableConfig struct in the test file.
155-162: LGTM: ImmutableConfig struct is properly initialized with all required parameters.
The configuration includes all necessary components for the native restaking functionality, with proper initialization of beacon addresses and chain configuration.
164-165: LGTM: ClientChainGateway constructor updated correctly with new parameter structure.
The constructor call properly uses the new ImmutableConfig struct along with the required endpoint and reward vault beacon addresses.
154-166: Verify test coverage for native restaking functionality.
While the configuration changes are properly implemented, consider adding specific test cases for:
- Native restaking initialization
- Validation of ImmutableConfig parameters
- Integration with beacon chain withdrawal credentials
test/foundry/unit/ClientChainGateway.t.sol (2)
24-24: LGTM: Import statement properly added
The import for BootstrapStorage is correctly placed and necessary for the ImmutableConfig struct usage.
133-133: LGTM: ClientChainGateway initialization updated correctly
The initialization is properly updated to use the new ImmutableConfig struct and includes the rewardVaultBeacon parameter. The change is well-tested through the existing test cases in the Initialize contract.
test/foundry/ExocoreDeployer.t.sol (2)
47-47: LGTM: Import statement is correctly placed.
The import for BootstrapStorage is appropriately placed with other storage-related imports.
366-366: Verify ClientChainGateway initialization.
The constructor call includes critical components for the system's functionality. Let's verify the initialization is properly tested.
✅ Verification successful
ClientChainGateway initialization is properly tested
The initialization is thoroughly verified in the Initialize contract within ClientChainGateway.t.sol. The test suite includes comprehensive checks for:
- Exocore chain ID initialization
- LayerZero endpoint initialization
- Vault beacon initialization
- Beacon proxy bytecode initialization
- Beacon oracle initialization
- ExoCapsule beacon initialization
- Owner initialization
- Initial pause state
- Bootstrap state
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for tests that verify the initialization of ClientChainGateway
rg -A 5 "new ClientChainGateway.*config" --type sol
Length of output: 81
Script:
#!/bin/bash
# Search for tests that verify the initialization of ClientChainGateway
rg -A 5 "new ClientChainGateway"
Length of output: 4352
Script:
#!/bin/bash
# Let's check the test file content to verify initialization tests
cat test/foundry/unit/ClientChainGateway.t.sol
Length of output: 16222
script/7_DeployBootstrap.s.sol (5)
21-21: Ensure new imports are necessary and correctly used
The imports BootstrapStorage and EigenLayerBeaconOracle have been added. Verify that these contracts are required and are correctly utilized in the script.
65-65: Verify implementation of _deployBeaconOracle()
The function _deployBeaconOracle() is called to deploy beaconOracle, but its implementation is not shown here. Ensure that this function is properly defined, perhaps in the inherited BaseScript contract, and that it correctly deploys and returns an instance of EigenLayerBeaconOracle.
76-83: Verify the initialization of ImmutableConfig struct
Confirm that all parameters passed into the ImmutableConfig struct are correct and correspond to the expected values. Double-check that the addresses assigned to beaconOracleAddress, vaultBeacon, exoCapsuleBeacon, and beaconProxyBytecode are accurate and necessary for initializing the Bootstrap and ClientChainGateway contracts.
86-86: Confirm instantiation of Bootstrap with ImmutableConfig
The Bootstrap contract is instantiated using the config struct. This should align with the updated constructor in the Bootstrap contract. Ensure that all required parameters are included and correctly referenced.
91-93: Ensure correct instantiation of ClientChainGateway
The ClientChainGateway is instantiated with the config struct and the rewardVaultBeacon address. Verify that these parameters match the updated constructor of ClientChainGateway and that the initialization logic is correctly implemented.
script/integration/1_DeployBootstrap.s.sol (4)
7-19: Imports are appropriately added
The new import statements for EigenLayerBeaconOracle, BootstrapStorage, ExoCapsule, and IExoCapsule are necessary for the added functionalities and are correctly referenced.
59-63: New state variables introduced correctly
The introduction of state variables beaconOracle, capsuleImplementation, vaultBeacon, and capsuleBeacon is appropriate for managing the new oracle and capsule implementations.
117-119: Beacon oracle deployment is correctly implemented
The deployment of the EigenLayerBeaconOracle using the _deployBeaconOracle() function is properly integrated into the deployContract function.
134-143: ImmutableConfig struct initialization enhances configuration management
The creation and initialization of the ImmutableConfig struct consolidates configuration parameters, improving code organization and maintainability.
test/foundry/BootstrapDepositNST.t.sol (2)
274-282: Ensure correct mocking of external contract calls
When mocking the call to ETH_POS.deposit, verify that the mock accurately reflects the expected behavior of the deposit function. Misconfigured mocks can cause the test to pass or fail incorrectly.
Confirm that the deposit function in IETHPOSDeposit does not return a value, and adjust the mock accordingly:
-vm.mockCall(address(ETH_POS), abi.encodeWithSelector(ETH_POS.deposit.selector), abi.encode(true));
+vm.mockCall(address(ETH_POS), abi.encodeWithSelector(ETH_POS.deposit.selector), "");185-189: Verify event emission expectations
The test expects certain events to be emitted using vm.expectEmit. Ensure that the events are correctly defined and that the expectEmit parameters match the event signatures. Incorrect configurations may cause the test to pass or fail incorrectly.
src/storage/BootstrapStorage.sol (2)
320-326: Ensure Modifier is Applied to Relevant Functions
Please verify that the nativeRestakingEnabled modifier is applied to all functions that require native restaking to be enabled. This ensures that any functions dependent on native restaking are correctly restricted when the feature is disabled.
146-148: Verify ETH_POS Deposit Contract Address
Confirm that the address 0x00000000219ab540356cBB839Cbe05303d7705Fa assigned to ETH_POS is correct and corresponds to the official ETH2.0 deposit contract. Using the correct contract address is crucial for the security and functionality of deposit operations.
test/foundry/DepositWithdrawPrinciple.t.sol (1)
336-336: Update reflects correct method usage
The method clientGateway.verifyAndDepositNativeStake is correctly called with the appropriate parameters, aligning with the updated functionality.
src/core/Bootstrap.sol (7)
9-9: Addition of Create2 Import
The import of Create2 is appropriate for deploying contracts with deterministic addresses, which is essential for the capsule creation functionality.
20-20: Adding Necessary Interfaces and Libraries
The imports of INativeRestakingController, IExoCapsule, and BeaconChainProofs are necessary for implementing the native restaking features.
Also applies to: 23-23, 27-27
47-47: Implementing INativeRestakingController Interface
Including INativeRestakingController in the list of implemented interfaces correctly extends the contract to support native restaking functionalities.
52-54: Updating Constructor to Use ImmutableConfig Struct
The constructor now accepts an ImmutableConfig struct, streamlining the initialization process and enhancing code maintainability.
709-727: Proper Handling of 32 ETH Stake in stake Function
The stake function correctly ensures that exactly 32 ETH is staked, aligning with Ethereum's validator requirements. The creation of a capsule if it doesn't exist enhances user experience.
735-755: Secure Implementation of createExoCapsule Function
The createExoCapsule function uses Create2 for deterministic deployment and correctly prevents multiple capsules per user. The use of the checks-effects-interactions pattern enhances security by updating state before external calls.
802-810: Access Control Verification in withdrawNonBeaconChainETHFromCapsule
The function withdrawNonBeaconChainETHFromCapsule allows users to withdraw ETH from their capsule. Ensure that access control is correctly enforced so only the capsule owner can perform withdrawals.
You can verify the access control with the following script:
✅ Verification successful
Let me try a different pattern to locate the _getCapsule function implementation.
Let me check the mapping between owners and capsules to verify the access control.
Based on the gathered information, I can now provide the final response:
Access Control is Properly Implemented in withdrawNonBeaconChainETHFromCapsule
The function correctly enforces access control through:
- The
_getCapsulefunction which verifies capsule ownership by checkingownerToCapsule[msg.sender] - Reverts with
CapsuleDoesNotExistif the caller doesn't own a capsule - The mapping
ownerToCapsulecan only be modified during capsule creation and is properly validated
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that only the capsule owner can call `withdrawNonBeaconChainETHFromCapsule`.
# Expected result: The function should only be callable by the owner of the capsule.
# Command to search for the `_getCapsule` function and its usage.
ast-grep --pattern $'function _getCapsule($_) {
$$ body
}' --json
# Check that `_getCapsule` retrieves the capsule based on `msg.sender` and that there are no alternate ways to access capsules.
Length of output: 73
Script:
#!/bin/bash
# Search for _getCapsule function with a more flexible pattern
ast-grep --pattern '_getCapsule'
# Also search using ripgrep for better context
rg "_getCapsule" -A 5
Length of output: 8282
Script:
#!/bin/bash
# Search for ownerToCapsule mapping and its usage
rg "ownerToCapsule" -A 5
# Also search for any other functions that might modify this mapping
ast-grep --pattern 'ownerToCapsule[$_] = $_'
Length of output: 7115
test/foundry/unit/Bootstrap.t.sol (1)
1556-1823: Comprehensive test cases for native restaking functionality.
The added test functions thoroughly cover various scenarios for Ethereum native restaking, including positive and negative test cases. This significantly enhances the test coverage and ensures the robustness of the new features.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (7)
src/core/ClientChainGateway.sol (1)
45-46: Enhance NatSpec documentation for better clarity.Consider expanding the parameter documentation to include:
- The purpose and structure of the
ImmutableConfigstruct- The relationship between these parameters and the contract's functionality
Apply this diff to improve the documentation:
- /// @param config is the struct containing the immutable state variables + /// @param config The configuration struct containing immutable state variables: + /// - exocoreChainId: The chain ID of the Exocore network + /// - beaconOracleAddress: The address of the beacon chain oracle + /// - vaultBeacon: The beacon contract address for Vault proxies + /// - exoCapsuleBeacon: The beacon contract address for ExoCapsule proxiessrc/libraries/Errors.sol (1)
63-64: LGTM! Consider enhancing the error documentation.The new error is well-placed and aligns with the PR's objectives of supporting Ethereum native restaking. The error will help validate immutable configurations during contract initialization.
Consider expanding the documentation to list specific validation criteria:
- /// @dev Bootstrap: invalid immutable config, typically due to zero address or zero value + /// @dev Bootstrap: invalid immutable config + /// @dev Thrown when the immutable config validation fails due to: + /// @dev - Zero addresses in required address fields + /// @dev - Zero values in required numeric fields + /// @dev - Invalid ranges or relationships between config parameterssrc/storage/BootstrapStorage.sol (1)
146-147: Consider making ETH_POS address configurable.The ETH_POS deposit contract address is hardcoded to the Ethereum mainnet address. This could make testing difficult and might not work on other networks.
Consider making this address configurable through the constructor or a setter function, especially for testing environments.
test/foundry/DepositWithdrawPrinciple.t.sol (1)
Line range hint
304-336: Consider enhancing test coverage and consistency.
- The comment style is inconsistent (mixing
//and///). Consider standardizing to one style.- Consider adding assertions to verify the capsule state after deposit.
- /// client chain layerzero endpoint should emit the message packet including deposit payload. + // client chain layerzero endpoint should emit the message packet including deposit payload. // Add capsule state verification + assertEq(capsule.owner(), depositor.addr); + assertEq(capsule.gateway(), address(clientGateway));src/core/Bootstrap.sol (3)
442-454: Stub Implementations for Reward FunctionsThe functions
submitReward,claimRewardFromExocore, andwithdrawRewardcurrently revert withErrors.NotYetSupported(). Ensure these functions are implemented when reward functionality is added.
762-786: Ensure Capsule Existence Before Verification inverifyAndDepositNativeStakeWhile
_getCapsule(msg.sender)reverts if the capsule does not exist, consider adding a clear error message to improve code readability and provide better feedback to users.Apply this diff to add an explicit require statement:
IExoCapsule capsule = _getCapsule(msg.sender); + // Explicit check for capsule existence + require(address(capsule) != address(0), "Capsule does not exist for caller"); uint256 depositValue = capsule.verifyDepositProof(validatorContainer, proof);
788-798: Clarify Comments inprocessBeaconChainWithdrawalFunctionThe comment mentions that this function is not yet supported but suggests that stakers could call it after bootstrapping. To avoid confusion, update the comment to clearly state when this function will be available.
Apply this diff to update the comment:
/// @notice Verifies a withdrawal proof from the beacon chain and forwards the information to Exocore. /// @notice This function is not yet supported. It will be available after bootstrapping is complete, -/// but staker could call this function after bootstrapping to withdraw their stake. +/// allowing stakers to withdraw their stake. function processBeaconChainWithdrawal( bytes32[] calldata, BeaconChainProofs.ValidatorContainerProof calldata, bytes32[] calldata, BeaconChainProofs.WithdrawalProof calldata ) external payable whenNotPaused nativeRestakingEnabled { revert Errors.NotYetSupported(); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (8)
script/13_DepositValidator.s.sol(1 hunks)src/core/Bootstrap.sol(5 hunks)src/core/ClientChainGateway.sol(1 hunks)src/libraries/Errors.sol(1 hunks)src/storage/BootstrapStorage.sol(6 hunks)src/storage/ClientChainGatewayStorage.sol(2 hunks)test/foundry/BootstrapDepositNST.t.sol(1 hunks)test/foundry/DepositWithdrawPrinciple.t.sol(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- test/foundry/BootstrapDepositNST.t.sol
🧰 Additional context used
📓 Learnings (1)
src/core/Bootstrap.sol (2)
Learnt from: adu-web3
PR: ExocoreNetwork/exocore-contracts#121
File: docs/native_deposit_workflow.wsd:23-23
Timestamp: 2024-10-31T01:55:53.516Z
Learning: In the `verifyAndDepositNativeStake(validatorContainer, proof)` function of `NativeStakingController` in `docs/native_deposit_workflow.wsd`, the verification work is performed by calling `ExoCapsule`, so internal validations within `NativeStakingController` are minimal or handled by `ExoCapsule`.
Learnt from: bwhour
PR: ExocoreNetwork/exocore-contracts#121
File: src/core/Bootstrap.sol:762-786
Timestamp: 2024-10-30T11:22:55.750Z
Learning: In the Solidity contract `src/core/Bootstrap.sol`, within the `verifyAndDepositNativeStake` function, there is no need to check if the capsule exists before calling `_getCapsule(msg.sender)` because `_getCapsule` already reverts if the capsule does not exist.
🔇 Additional comments (13)
src/storage/ClientChainGatewayStorage.sol (2)
6-6: LGTM: Import organization.
The added empty line improves code organization by grouping related imports together.
72-78: Constructor refactoring looks good but verify storage layout.
The constructor changes appropriately consolidate parameters using ImmutableConfig and include proper validation. However, the removal of state variables raises concerns about storage layout compatibility.
Let's analyze the storage layout impact:
✅ Verification successful
Storage layout changes are safely handled with storage gaps
The storage layout changes in ClientChainGatewayStorage are safe because:
- The contract maintains a storage gap of 40 slots (
uint256[40] private __gap) - The inheritance chain (
ClientChainGatewayStorage->BootstrapStorage->GatewayStorage) each have their own storage gaps - The removed state variables (
ownerToCapsule,BEACON_ORACLE_ADDRESS,EXO_CAPSULE_BEACON) are being compensated by the existing storage gaps - The contract is part of an upgradeable contract system as evidenced by the presence of proxy patterns and upgrade scripts
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for potential storage layout incompatibilities
# Test 1: Look for deployed addresses or deployment scripts
echo "Checking for deployment references..."
rg -l "ClientChainGatewayStorage" | grep -iE "deploy|address|migration"
# Test 2: Check if there are existing storage layout artifacts
echo "Checking for storage layout artifacts..."
fd -g "*storage*layout*.json"
# Test 3: Look for upgrade references
echo "Looking for upgrade patterns..."
rg "function upgrade" -A 5
rg "function implementation" -A 5
Length of output: 711
Script:
#!/bin/bash
# Let's try a different approach to analyze storage layout concerns
# Check for storage inheritance hierarchy
echo "Checking storage inheritance..."
ast-grep --pattern 'contract ClientChainGatewayStorage is $$$'
# Look for storage slots and variables in parent contracts
echo "Checking BootstrapStorage..."
rg -A 20 "contract BootstrapStorage"
# Check for storage gaps
echo "Looking for storage gaps..."
rg "__gap" -B 5 -A 5
# Check for upgrade related contracts/interfaces
echo "Checking for upgrade patterns..."
rg -l "Upgradeable|UUPS|Proxy|Beacon"
Length of output: 11061
script/13_DepositValidator.s.sol (1)
83-83:
Critical operation requires proper error handling and validation.
The current implementation has several concerns:
- The removal of error handling makes the script less robust for this critical operation
- The
nativeFeevalue is used without validation - As a must-succeed operation for validator deposits, failure handling is essential
Apply this diff to add proper error handling and validation:
+ require(nativeFee > 0, "Invalid native fee");
+ try clientGateway.verifyAndDepositNativeStake{value: nativeFee}(validatorContainer, validatorProof) {
+ console.log("Validator deposit successful");
+ } catch Error(string memory reason) {
+ console.log("Validator deposit failed:", reason);
+ revert("Validator deposit failed - must succeed operation");
+ } catch (bytes memory /*lowLevelData*/) {
+ console.log("Validator deposit failed with low-level error");
+ revert("Validator deposit failed - must succeed operation");
+ }
- clientGateway.verifyAndDepositNativeStake{value: nativeFee}(validatorContainer, validatorProof);Let's verify the implementation of verifyAndDepositNativeStake to ensure our error handling aligns with the contract's expectations:
src/core/ClientChainGateway.sol (1)
47-49: LGTM! Verify ImmutableConfig validation.
The refactoring to use ImmutableConfig improves code organization. However, we should verify that the struct parameters are properly validated.
Let's check the validation in the base contracts:
✅ Verification successful
ImmutableConfig parameters are properly validated
The validation is implemented in BootstrapStorage.sol with proper zero-address checks for all config parameters:
require(!(
config.exocoreChainId == 0 ||
config.beaconOracleAddress == address(0) ||
config.vaultBeacon == address(0) ||
config.exoCapsuleBeacon == address(0) ||
config.beaconProxyBytecode == address(0)
), "Bootstrap: invalid config");🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for ImmutableConfig validation in base contracts
rg -A 10 "ImmutableConfig.*{"
Length of output: 131
Script:
#!/bin/bash
# Search for ImmutableConfig struct definition and usage
rg -A 5 "struct ImmutableConfig"
# Search for validation of ImmutableConfig parameters
rg -A 10 "function.*ImmutableConfig.*memory.*config"
Length of output: 484
Script:
#!/bin/bash
# Search for ClientChainGatewayStorage constructor and initialization
rg -A 10 "contract ClientChainGatewayStorage"
# Search for any validation or require statements related to config parameters
rg -A 3 "require\(.*config\."
rg -A 3 "if.*config\."
# Search for usage of these config parameters
rg "beaconOracleAddress|vaultBeacon|exoCapsuleBeacon|beaconProxyBytecode"
Length of output: 15396
src/storage/BootstrapStorage.sol (4)
275-284: LGTM! Well-structured events with proper indexing.
The new events are well-designed with:
- Clear naming that reflects their purpose
- Proper use of indexing for efficient filtering
- Comprehensive documentation
301-315: LGTM! Well-structured configuration struct.
The ImmutableConfig struct is a good solution that:
- Avoids "stack too deep" errors in the constructor
- Improves code organization
- Has comprehensive documentation for each field
320-326: LGTM! Well-implemented modifier.
The nativeRestakingEnabled modifier properly checks if native restaking is enabled through the whitelist mechanism.
378-386: LGTM! Consistent internal helper function.
The _getCapsule function follows the same pattern as _getVault:
- Proper error handling for non-existent capsules
- Consistent with the codebase's style
- Well-documented
test/foundry/DepositWithdrawPrinciple.t.sol (2)
30-31: LGTM: Well-structured event declarations.
The events are properly defined with indexed parameters, following Solidity best practices and providing clear tracking for capsule creation and staking operations.
Line range hint 304-336: LGTM: Native deposit test updated correctly.
The test has been properly updated to use the new verifyAndDepositNativeStake function name, maintaining test coverage for the native staking functionality.
src/core/Bootstrap.sol (3)
Line range hint 9-32: Imports and Inheritance Updated for Native Restaking Support
The added imports and inheritance correctly integrate INativeRestakingController, IExoCapsule, BeaconChainProofs, and BootstrapLzReceiver to support Ethereum native restaking functionality.
52-56: Constructor Updated with Immutable Configuration
The constructor now accepts an ImmutableConfig struct and properly initializes the base contracts OAppCoreUpgradeable and BootstrapStorage with the provided parameters.
800-812: Access Control in withdrawNonBeaconChainETHFromCapsule
Ensure that the withdrawNonBeaconChainETHFromCapsule function properly restricts withdrawals to authorized users to prevent unauthorized access to funds.
Run the following script to verify access control:
|
Storage layout compatibility check failed. This could be due to a mismatch between Bootstrap and ClientChainGateway, or incompatibility with deployed contracts on Sepolia. Please check the logs for details. |
|
This PR would lead to failure of upgrading the current |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
src/storage/BootstrapStorage.sol (1)
353-360: Constructor validation logic can be refinedWhile the validation checks are comprehensive, consider separating zero address checks and the chain ID check for more precise error handling. This can improve clarity on which parameter caused the failure.
Apply this diff to refine the validation logic:
constructor(ImmutableConfig memory config) { - if ( - config.exocoreChainId == 0 || config.beaconOracleAddress == address(0) || config.vaultBeacon == address(0) - || config.exoCapsuleBeacon == address(0) || config.beaconProxyBytecode == address(0) - ) { - revert Errors.InvalidImmutableConfig(); - } + if (config.exocoreChainId == 0) { + revert Errors.InvalidExocoreChainId(); + } + if ( + config.beaconOracleAddress == address(0) || + config.vaultBeacon == address(0) || + config.exoCapsuleBeacon == address(0) || + config.beaconProxyBytecode == address(0) + ) { + revert Errors.ZeroAddress(); + }This change provides more specific error messages, helping users identify configuration issues more easily.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
src/storage/BootstrapStorage.sol(7 hunks)
🔇 Additional comments (10)
src/storage/BootstrapStorage.sol (10)
6-7: Imports are properly added for new interfaces
The added imports for IETHPOSDeposit and IExoCapsule correctly bring in the necessary interfaces for the new functionalities.
133-135: Immutable ExoCapsule Beacon is correctly declared
The declaration of the immutable EXO_CAPSULE_BEACON variable ensures that the ExoCapsule implementation remains consistent throughout the contract's lifecycle.
136-138: Immutable Beacon Oracle Address is correctly declared
The BEACON_ORACLE_ADDRESS is properly set as an immutable variable, which aligns with its intended unchangeable nature.
142-144: ETH_POS deposit contract address is accurately defined
The ETH_POS constant is correctly set to the official ETH2.0 deposit contract address.
167-170: Storage gap is appropriately defined for future upgrades
Defining the storage gap after existing variables helps maintain storage layout compatibility and allows for future contract upgrades.
171-176: Mapping ownerToCapsule is correctly moved and declared
The ownerToCapsule mapping has been moved appropriately to maintain storage layout compatibility. Including parameter names enhances readability, which is preferred in this codebase.
281-291: Events for native restaking are well-defined and indexed
The CapsuleCreated and StakedWithCapsule events are correctly defined with indexed parameters, facilitating efficient event filtering and log queries.
307-322: ImmutableConfig struct enhances constructor parameter management
Introducing the ImmutableConfig struct simplifies the constructor and avoids the stack too deep error. This approach improves code maintainability and clarity.
323-329: nativeRestakingEnabled modifier is correctly implemented
The modifier properly checks if native restaking is enabled by verifying the whitelisting of VIRTUAL_NST_ADDRESS. The revert uses an appropriate custom error.
381-390: _getCapsule function is properly implemented
The internal function _getCapsule correctly retrieves the ExoCapsule for a given owner and handles the case where it does not exist by reverting with an appropriate error.
Description
As we need to support Ethereum native restaking during bootstrap, we should allow the stakers to deposit staked ETH of beacon chain to Exocore, by pointing withdrawal credentials to capsule contract the staker creates and owns.
TODO:
BootstrapinheritINativeRestakingControllerand implement relative functionsownerToCapsulestate fromClientChainGatewayStoragetoBootstrapStorageBootstrapStorageconstructor function by usingImmutableConfigstruct to work aroundstack too deepcompilation errorBootstrap.t.soland integration test inBootstrapDepositNSTTest.t.solSummary by CodeRabbit
New Features
verifyAndDepositNativeStakefunction for enhanced deposit verification and processing.withdrawNonBeaconChainETHFromCapsulefunction for ETH withdrawals from ExoCapsule.ImmutableConfigstruct consolidates configuration parameters for improved contract initialization.Bug Fixes
InvalidImmutableConfigerror for better error management in configuration.Tests
Documentation