feat: use gas budget argument to refund TSS for sui withdraw cost #3671
Conversation
📝 WalkthroughWalkthroughThis pull request introduces a new feature documented in the changelog that refunds TSS for Sui withdrawal costs via a gas budget argument. Additionally, multiple end-to-end tests for Sui deposit, token deposit, token withdrawal, and withdrawal have been updated to record and assert TSS balance changes. The observer and signer components have been modified to expect an extra argument (gasBudget) in move call requests and update related constants and struct fields accordingly. Changes
Sequence Diagram(s)sequenceDiagram
participant Test as TestFunction
participant TSS as TSS Account
participant Tx as Transaction
Test->>TSS: Query initial balance (tssBalanceBefore)
Test->>Tx: Execute deposit/withdraw operation
Test->>TSS: Query post-transaction balance (tssBalanceAfter)
Test->>Test: Assert tssBalanceAfter ≥ tssBalanceBefore
sequenceDiagram
participant Client
participant Signer
participant Observer
Client->>Signer: Request withdrawal transaction
Signer->>Signer: Build MoveCallRequest (include gasBudget via CallOptions)
Signer->>Observer: Submit transaction
Observer->>Observer: Validate input count (expected 6 arguments)
Observer->>Client: Confirm transaction execution
Possibly related PRs
Suggested labels
Suggested reviewers
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 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
Documentation and Community
|
lumtis
changed the title
lumtis
changed the title
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #3671 +/- ##
===========================================
- Coverage 64.61% 64.57% -0.05%
===========================================
Files 470 470
Lines 32857 32907 +50
===========================================
+ Hits 21232 21249 +17
- Misses 10659 10690 +31
- Partials 966 968 +2
🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
zetaclient/chains/sui/observer/outbound.go (2)
22-22: Consider enhancing documentation for the expectedWithdrawArgs constantThe constant has been updated from 5 to 6 to accommodate the new gas budget argument for TSS refund functionality. For better maintainability, consider expanding the comments to document all expected arguments (including their types and positions), particularly highlighting the newly added gas budget argument and its purpose.
// https://github.com/zeta-chain/protocol-contracts-sui/blob/9d08a70817d8cc7cf799b9ae12c59b6e0b8aaab9/sources/gateway.move#L125 // (excluding last arg of `ctx`) -const expectedWithdrawArgs = 6 +// Expected arguments for withdrawal: +// 1. recipient: address +// 2. amount: u64 +// 3. nonce: u64 +// 4. message: vector<u8> +// 5. gasAssetId: ID +// 6. gasBudget: u64 - Used to refund TSS for Sui withdrawal costs +const expectedWithdrawArgs = 6
277-296: Consider extracting constants for argument indicesFor better code maintainability, consider extracting constants for all argument indices, not just the nonce index. This would make the code more readable and less prone to errors when the order of arguments changes.
+// Argument indices for withdraw inputs +const ( + recipientIdx = 0 + amountIdx = 1 + nonceIdx = 2 + messageIdx = 3 + gasAssetIdIdx = 4 + gasBudgetIdx = 5 +) -const nonceIdx = 2
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
changelog.md(1 hunks)e2e/e2etests/test_sui_deposit_and_call_revert.go(2 hunks)e2e/e2etests/test_sui_token_deposit_and_call_revert.go(2 hunks)e2e/e2etests/test_sui_token_withdraw.go(2 hunks)e2e/e2etests/test_sui_withdraw.go(2 hunks)zetaclient/chains/sui/observer/observer_test.go(1 hunks)zetaclient/chains/sui/observer/outbound.go(1 hunks)zetaclient/chains/sui/signer/signer_test.go(2 hunks)zetaclient/chains/sui/signer/signer_tx.go(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.go`: Review the Go code, point out issues relative to ...
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
e2e/e2etests/test_sui_token_withdraw.gozetaclient/chains/sui/signer/signer_test.gozetaclient/chains/sui/observer/observer_test.goe2e/e2etests/test_sui_token_deposit_and_call_revert.gozetaclient/chains/sui/signer/signer_tx.gozetaclient/chains/sui/observer/outbound.goe2e/e2etests/test_sui_withdraw.goe2e/e2etests/test_sui_deposit_and_call_revert.go
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: start-e2e-test / e2e
🔇 Additional comments (16)
zetaclient/chains/sui/signer/signer_test.go (2)
46-48: Good addition of CallOptions for gas budget testing.The test now correctly includes CallOptions with a GasLimit value to test the new gas budget functionality for TSS refunds.
68-68: Expected arguments updated to include gas budget value.The expected arguments now properly include "42000" (42 * 1000) as the gas budget value passed to the MoveCallRequest, correctly reflecting the implementation change.
changelog.md (1)
3-7: Well-structured changelog entry for the new feature.The changelog entry clearly documents the new feature for using a gas budget argument to refund TSS for Sui withdraw costs. The PR reference provides good traceability.
zetaclient/chains/sui/observer/observer_test.go (1)
259-259: Test updated to match new withdrawal argument count.The test case for ProcessOutboundTrackers has been correctly updated to include an additional argument stub, ensuring that withdrawal transaction validation will pass with the new gas budget parameter.
zetaclient/chains/sui/signer/signer_tx.go (2)
48-54: Good implementation of gas budget calculation.The code correctly calculates the gas budget by multiplying gas price by gas limit from call options, implementing the core functionality for refunding TSS for Sui withdrawal costs.
66-66: Updated withdrawal function arguments to include gas budget.The Arguments field now correctly includes the gas budget parameter in the transaction arguments list, enabling the Sui contract to refund the TSS for withdrawal costs.
e2e/e2etests/test_sui_withdraw.go (2)
18-18: Appropriate addition of TSS balance checkThis addition correctly captures the TSS balance before the withdrawal operation, establishing a baseline for comparison.
37-40: Well-implemented TSS refund verificationThe added verification logic correctly ensures that the TSS receives its gas budget refund after the transaction completes. The comments clearly explain the rationale, and the use of
GreaterOrEqualcorrectly handles potential edge cases where the refund might exactly match the initial cost.e2e/e2etests/test_sui_token_withdraw.go (2)
18-18: Appropriate addition of TSS balance checkThis addition correctly captures the TSS balance before the token withdrawal operation, establishing a baseline for comparison.
37-40: Well-implemented TSS refund verificationThe added verification logic correctly ensures that the TSS receives its gas budget refund after the token withdrawal transaction completes. The implementation maintains consistency with the other test files and properly validates the gas refund behavior.
e2e/e2etests/test_sui_deposit_and_call_revert.go (2)
21-21: Appropriate addition of TSS balance checkThis addition correctly captures the TSS balance before the deposit operation that will revert, establishing a baseline for comparison.
40-43: Well-implemented TSS refund verification for revert scenarioThe added verification logic ensures that the TSS receives its gas budget refund even in the case of reverted transactions. This is particularly important for validating that the gas refund mechanism works correctly in failure scenarios.
e2e/e2etests/test_sui_token_deposit_and_call_revert.go (2)
23-23: Appropriate addition of TSS balance checkThis addition correctly captures the TSS balance before the token deposit operation that will revert, establishing a baseline for comparison.
48-51: Well-implemented TSS refund verification for token revert scenarioThe added verification logic ensures that the TSS receives its gas budget refund even when token deposit transactions revert. The implementation is consistent with the other test files and properly validates the complex interaction between token operations and gas refunds.
zetaclient/chains/sui/observer/outbound.go (2)
203-205: The validation logic correctly uses the updated constantThe validation logic in
validateOutboundproperly uses theexpectedWithdrawArgsconstant to check the correct number of arguments, ensuring compatibility with the new gas budget parameter.
277-280:❓ Verification inconclusive
Validate parseNonceFromWithdrawInputs function for gasBudget compatibility
This function correctly validates that the inputs match the updated
expectedWithdrawArgsconstant. However, it doesn't parse or use the new gas budget argument. If the position of the nonce has changed due to the additional argument, or if the gas budget needs to be processed here, this function would need further updates.
🏁 Script executed:
#!/bin/bash # Check if the nonce position remains consistent and if there's any usage of the gas budget argument # Search for the declaration and usage of the gas budget argument in the signer implementation rg -A 10 "buildWithdrawal" --glob "*.go" | grep -A 20 "Arguments" # Verify if there's any processing of gas budget in the codebase rg -A 5 "gasBudget" --glob "*.go"Length of output: 1031
Action: Confirm Nonce Extraction Consistency Amidst Withdraw Argument Updates
The function
parseNonceFromWithdrawInputscorrectly checks that the number of inputs equalsexpectedWithdrawArgs. However, with the new gas budget argument now computed and injected (as seen insui/signer/signer_tx.go), please verify that the nonce remains in its expected position. Specifically:
- Ensure that the constant
expectedWithdrawArgsand the index used to extract the nonce are still valid after adding the gas budget.- If the argument order has shifted (e.g., nonce moved to a different index), update this function to correctly parse the nonce.
- Confirm that gas budget processing is intentionally handled elsewhere and does not require additional action in this function.
* feat(sui): custom sui_executeTransactionBlock (#3662) * Fix sui_executeTransactionBlock * rename * fix unit test observer * feat: use gas budget argument to refund TSS for sui withdraw cost (#3671) * build new gateway binary * gas budget params * fix order * changelog * add accounting checks * fix tests * fix unit test observer * fix changeogs * changelog --------- Co-authored-by: Dmitry S <11892559+swift1337@users.noreply.github.com> Co-authored-by: Alex Gartner <git@agartner.com>
Description
Closes #3583
Use changes from zeta-chain/protocol-contracts-sui#40
Summary by CodeRabbit
New Features
Documentation