feat: Add a deposit-then-delegate function #24
Conversation
WalkthroughThe update introduces a streamlined way for stakers to deposit and delegate tokens in a single transaction, enhancing user experience by reducing cross-chain communication costs. Changes span several files, incorporating new functions, renaming actions, and updating enums, primarily to support this combined deposit-and-delegate functionality. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ClientChainGateway
participant ExocoreGateway
participant Bootstrap
User->>ClientChainGateway: depositThenDelegateTo(token, amount, operator)
ClientChainGateway->>ExocoreGateway: requestDepositThenDelegateTo()
ExocoreGateway->>Bootstrap: depositThenDelegateTo(token, amount, operator)
Bootstrap-->>ExocoreGateway: success
ExocoreGateway-->>ClientChainGateway: response
ClientChainGateway-->>User: confirmation
Assessment against linked issues
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 Configration 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: 2
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (14)
- src/core/Bootstrap.sol (8 hunks)
- src/core/ClientChainGateway.sol (1 hunks)
- src/core/ClientGatewayLzReceiver.sol (1 hunks)
- src/core/ExocoreGateway.sol (2 hunks)
- src/core/LSTRestakingController.sol (1 hunks)
- src/interfaces/ILSTRestakingController.sol (1 hunks)
- src/storage/BootstrapStorage.sol (1 hunks)
- src/storage/ExocoreGatewayStorage.sol (1 hunks)
- src/storage/GatewayStorage.sol (1 hunks)
- test/foundry/Delegation.t.sol (1 hunks)
- test/foundry/DepositThenDelegateTo.t.sol (1 hunks)
- test/foundry/DepositWithdrawPrinciple.t.sol (1 hunks)
- test/foundry/ExocoreDeployer.t.sol (1 hunks)
- test/foundry/WithdrawReward.t.sol (1 hunks)
Files skipped from review due to trivial changes (4)
- src/storage/ExocoreGatewayStorage.sol
- test/foundry/Delegation.t.sol
- test/foundry/DepositWithdrawPrinciple.t.sol
- test/foundry/WithdrawReward.t.sol
Additional comments not posted (8)
src/core/LSTRestakingController.sol (1)
40-51: Implementation ofdepositThenDelegateTolooks solid and well-integrated.The use of multiple modifiers for validation and the reuse of the
_processRequestmethod are good practices that enhance maintainability and security.src/interfaces/ILSTRestakingController.sol (1)
50-62: Well-documented addition ofdepositThenDelegateToin the interface.The detailed documentation provides clear expectations for the implementation, which is crucial for maintaining the integrity of the interface contract.
test/foundry/DepositThenDelegateTo.t.sol (1)
44-154: Comprehensive tests forDepositThenDelegateTofunctionality.The tests are well-structured and cover both request and response scenarios comprehensively. Consider verifying the integration with the actual contract to ensure that the tests reflect the real-world behavior accurately.
src/core/ClientGatewayLzReceiver.sol (1)
171-190: The implementation ofafterReceiveDepositThenDelegateToResponsecorrectly handles the response for the combined deposit and delegate action. Ensure that edge cases and error scenarios are thoroughly tested.src/core/ExocoreGateway.sol (1)
226-252: The implementation ofrequestDepositThenDelegateTocorrectly handles the combined deposit and delegate request. Ensure that error handling and integration with other system components are thoroughly tested.test/foundry/ExocoreDeployer.t.sol (1)
279-289: The implementation ofgenerateUIDcorrectly generates unique identifiers for transactions. Ensure that this function is used consistently across the system for tracking and auditing purposes.Verification successful
The
generateUIDfunction is used consistently across the system, as evidenced by multiple instances found in the codebase.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify consistent usage of generateUID across the system. # Test: Search for usages of generateUID. Expect: Consistent usage across the system. rg --type solidity $'generateUID'Length of output: 65
Script:
#!/bin/bash # Description: Verify consistent usage of generateUID across the system. # Find all Solidity files and search for usages of generateUID within them. fd -e sol -x rg 'generateUID' {}Length of output: 1081
src/storage/BootstrapStorage.sol (1)
320-335: The new eventDepositThenDelegateResultis well-defined and aligns with the PR objectives to handle combined deposit and delegation actions.src/core/Bootstrap.sol (1)
308-308: The implementation ofdepositThenDelegateToinBootstrap.solcorrectly integrates the deposit and delegation processes into a single transaction, which aligns with the PR objectives to improve user experience by reducing transaction costs.Also applies to: 441-454
- rename MARK_BOOTSTRAP to REQUEST_MARK_BOOTSTRAP - move REQUEST_DEPOSIT_THEN_DELEGATE_TO upwards
There was a problem hiding this comment.
Actionable comments posted: 3
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (6)
- src/core/Bootstrap.sol (9 hunks)
- src/core/ClientChainGateway.sol (2 hunks)
- src/core/ExocoreGateway.sol (3 hunks)
- src/storage/GatewayStorage.sol (1 hunks)
- test/foundry/Bootstrap.t.sol (3 hunks)
- test/mocks/ExocoreGatewayMock.sol (2 hunks)
Additional comments not posted (5)
src/core/ExocoreGateway.sol (1)
70-71: The addition of theREQUEST_DEPOSIT_THEN_DELEGATE_TOaction to_whiteListFunctionSelectorsis correctly implemented. This ensures that the new functionrequestDepositThenDelegateTocan be called internally following the LayerZero messaging pattern.test/mocks/ExocoreGatewayMock.sol (1)
39-40: The addition ofREQUEST_DEPOSIT_THEN_DELEGATE_TOto theActionenum in the mock contract is correctly implemented. This ensures that the mock environment aligns with the changes in the main contract, facilitating accurate testing.test/foundry/Bootstrap.t.sol (3)
817-817: Ensure the correct encoding of the action in thelzReceivefunction.Verification successful
Ensure the correct encoding of the action in the
lzReceivefunction.The encoding of
REQUEST_MARK_BOOTSTRAPappears consistent across the codebase.
test/foundry/Bootstrap.t.sol: Multiple instances ofabi.encodePacked(GatewayStorage.Action.REQUEST_MARK_BOOTSTRAP, "")src/core/ExocoreGateway.sol:_sendInterchainMsg(uint32(clientChainId), Action.REQUEST_MARK_BOOTSTRAP, "")src/core/Bootstrap.sol:_whiteListFunctionSelectors[Action.REQUEST_MARK_BOOTSTRAP] = this.markBootstrapped.selectorsrc/storage/GatewayStorage.sol:REQUEST_MARK_BOOTSTRAPsrc/core/ClientChainGateway.sol:delete _whiteListFunctionSelectors[Action.REQUEST_MARK_BOOTSTRAP]Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the encoding of the action REQUEST_MARK_BOOTSTRAP is consistent across all contracts. # Test: Search for the encoding usage. Expect: Consistent usage across the codebase. rg --type solidity $'REQUEST_MARK_BOOTSTRAP'Length of output: 76
Script:
#!/bin/bash # Description: Verify that the encoding of the action REQUEST_MARK_BOOTSTRAP is consistent across all contracts. # Test: Search for the encoding usage. Expect: Consistent usage across the codebase. rg 'REQUEST_MARK_BOOTSTRAP' --glob '*.sol'Length of output: 1133
847-852: Ensure proper handling of unsupported requests.Verification successful
The search results indicate that the
UnsupportedRequesterror is defined and used in multiple places across the codebase, including in theBootstrap.t.solfile. This suggests that unsupported requests are indeed being handled, typically by reverting the transaction.
test/mocks/ExocoreGatewayMock.sol: Defines and revertsUnsupportedRequest.src/storage/GatewayStorage.sol: DefinesUnsupportedRequest.src/core/ExocoreGateway.sol: RevertsUnsupportedRequest.src/core/BootstrapLzReceiver.sol: RevertsUnsupportedRequest.src/core/ClientGatewayLzReceiver.sol: RevertsUnsupportedRequest.These findings confirm that unsupported requests are being handled properly by reverting the transaction with the
UnsupportedRequesterror.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that unsupported requests are handled correctly across all contracts. # Test: Search for the handling of unsupported requests. Expect: Proper error handling and reverts. rg --type solidity $'UnsupportedRequest'Length of output: 72
Script:
#!/bin/bash # Description: Verify that unsupported requests are handled correctly across all contracts. # Test: Search for the handling of unsupported requests. Expect: Proper error handling and reverts. rg --type sol $'UnsupportedRequest'Length of output: 62
Script:
#!/bin/bash # Description: Verify that unsupported requests are handled correctly across all contracts. # Test: Search for the handling of unsupported requests. Expect: Proper error handling and reverts. fd -e sol | xargs rg 'UnsupportedRequest'Length of output: 656
836-836: Check the timing conditions for marking bootstrap.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- src/storage/GatewayStorage.sol (1 hunks)
- test/foundry/Bootstrap.t.sol (3 hunks)
- test/mocks/ExocoreGatewayMock.sol (2 hunks)
Files skipped from review as they are similar to previous changes (3)
- src/storage/GatewayStorage.sol
- test/foundry/Bootstrap.t.sol
- test/mocks/ExocoreGatewayMock.sol
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- src/core/Bootstrap.sol (9 hunks)
- src/core/ExocoreGateway.sol (3 hunks)
- test/foundry/Bootstrap.t.sol (4 hunks)
Files skipped from review as they are similar to previous changes (3)
- src/core/Bootstrap.sol
- src/core/ExocoreGateway.sol
- test/foundry/Bootstrap.t.sol
|
function _processRequest(
address token,
address sender,
uint256 amount,
Action action,
string memory operator // Optional parameter, empty string when not needed.
) internal {
if (token != VIRTUAL_STAKED_ETH_ADDRESS) {
IVault vault = _getVault(token);
if (action == Action.REQUEST_DEPOSIT) {
vault.deposit(sender, amount); // Logic specific to the REQUEST_DEPOSIT action.
}
}
outboundNonce++;
bool hasOperator = bytes(operator).length > 0;
// Use a single abi.encode call via ternary operators to handle both cases.
_registeredRequests[outboundNonce] =
hasOperator ? abi.encode(token, operator, sender, amount) : abi.encode(token, sender, amount);
_registeredRequestActions[outboundNonce] = action;
// Use a single abi.encodePacked call via ternary operators to handle both cases.
bytes memory actionArgs = hasOperator
? abi.encodePacked(bytes32(bytes20(token)), bytes32(bytes20(sender)), bytes(operator), amount)
: abi.encodePacked(bytes32(bytes20(token)), bytes32(bytes20(sender)), amount);
_sendMsgToExocore(action, actionArgs);
} |
Ensure that user is actually charged
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- src/core/BaseRestakingController.sol (1 hunks)
- test/foundry/Bootstrap.t.sol (4 hunks)
- test/foundry/DepositThenDelegateTo.t.sol (1 hunks)
- test/mocks/DepositMock.sol (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- test/foundry/Bootstrap.t.sol
Additional comments not posted (3)
test/mocks/DepositMock.sol (2)
9-9: Visibility change ofprincipleBalancesto public is appropriate for a mock contract used in testing.
32-32: The updated return statement inwithdrawPrincipleimproves function clarity and usability.test/foundry/DepositThenDelegateTo.t.sol (1)
46-185: The test implementation inDepositThenDelegateToTestis thorough, covering various important aspects of the deposit-then-delegate functionality, including balance checks and event emissions.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- test/foundry/DepositThenDelegateTo.t.sol (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- test/foundry/DepositThenDelegateTo.t.sol
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- test/foundry/DepositThenDelegateTo.t.sol (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- test/foundry/DepositThenDelegateTo.t.sol
|
Test passed with following user scenarioes.
|
Fixes: #4
ILSTRestakingControllercalleddepositThenDelegateTo.BootstrapLSTRestakingController, which is inherited byClientChainGatewayExocoreGatewayClientChainGatewaygenerateUIDfunction common in tests inheriting fromExocoreDeployer. With unit tests, we verify that (1) deposit is received, (2) delegate is received, and (3) the assets are withdrawn from the user's balance and increased in the vault.Based on previous discussions, it is assumed that the deposit part does not fail and any failure returned by
ExocoreGatewaytoClientChainGatewayrefers to the delegation. This is correctly reflected in the event definition as provided below. Note that thedelegatedAmountwill be, by definition, equal to the amount that is being deposited since both the steps happen atomically.Summary by CodeRabbit
New Features
Enhancements
Bug Fixes
Tests