fix: fix Sui example deployment by removing gateway object reference #4414
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughWalkthroughThis pull request removes the Gateway object parameter and related dependencies from the Sui example contract and its client-side transaction builder. The Gateway reference is eliminated from the on_call function signature, import statements, and transaction argument construction, with corresponding documentation updates regarding breaking changes and upgrade requirements. Changes
Sequence DiagramsequenceDiagram
participant Client as zetaclient
participant Contract as example.move
rect rgb(240, 248, 255)
note over Client,Contract: After Changes (Gateway Removed)
Client->>Client: Build PTB with message context
Client->>Contract: on_call(message_context, withdrawn_coins, ...)
activate Contract
Contract->>Contract: Decode payload
Contract->>Contract: Validate authorization & transfer
deactivate Contract
Contract-->>Client: Success
end
rect rgb(255, 240, 245)
note over Client,Contract: Before Changes (Gateway Included)
Client->>Client: Build PTB with message context
Client->>Client: Construct gateway object reference
Client->>Contract: on_call(gateway, message_context, withdrawn_coins, ...)
activate Contract
Contract->>Contract: Validate active_message_context via gateway
Contract->>Contract: Decode payload
Contract->>Contract: Validate authorization & transfer
deactivate Contract
Contract-->>Client: Success
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
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. Comment |
ws4charlie
changed the title
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
e2e/contracts/sui/example/sources/example.move (1)
19-19: Remove unused error constant.The
EInactiveMessageContextconstant is no longer referenced after removing the gateway-based message context validation. This is dead code that should be removed.Apply this diff to remove the unused constant:
-const EInactiveMessageContext: u64 = 4; - const EPackageMismatch: u64 = 5;
🧹 Nitpick comments (1)
zetaclient/chains/sui/signer/withdraw_and_call.go (1)
424-425: Fix typo in comment.The gateway argument has been correctly removed from the Arguments list, and the code properly matches the updated Move contract signature. However, there is a minor typo in the comment.
Apply this diff to fix the typo:
- // [message context + withdrawns coins + payload objects + message] + // [message context + withdrawn coins + payload objects + message] Arguments: append([]suiptb.Argument{argMsgContext, argWithdrawnCoins}, onCallArgs...),
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
changelog.md(1 hunks)e2e/contracts/sui/example/sources/example.move(1 hunks)zetaclient/chains/sui/signer/withdraw_and_call.go(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.go
⚙️ CodeRabbit configuration file
Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
Files:
zetaclient/chains/sui/signer/withdraw_and_call.go
🧠 Learnings (5)
📓 Common learnings
Learnt from: gartnera
Repo: zeta-chain/node PR: 3070
File: cmd/zetae2e/init.go:0-0
Timestamp: 2024-10-30T17:56:16.341Z
Learning: In code reviews for Go files like `cmd/zetae2e/init.go` in the ZetaChain project, avoid suggesting unrelated refactoring. Focus comments on changes relevant to the PR objectives.
Learnt from: lumtis
Repo: zeta-chain/node PR: 3500
File: pkg/contracts/sui/gateway.go:62-62
Timestamp: 2025-02-12T13:42:27.999Z
Learning: The `from` and `to` parameters in the `queryInbounds` function of the Sui gateway are temporary placeholders that will be removed or implemented in a future PR.
📚 Learning: 2025-05-30T16:31:30.275Z
Learnt from: skosito
Repo: zeta-chain/node PR: 3939
File: go.mod:52-52
Timestamp: 2025-05-30T16:31:30.275Z
Learning: The ethermint dependency updates in the zeta-chain/node repository are typically moves between feature branches and main branch of the same fork, not breaking API changes. CI status should be verified before assuming compilation issues.
Applied to files:
changelog.md
📚 Learning: 2024-07-11T13:10:39.153Z
Learnt from: lumtis
Repo: zeta-chain/node PR: 2459
File: e2e/config/config.go:285-293
Timestamp: 2024-07-11T13:10:39.153Z
Learning: Tests should not be suggested in the `e2e` package for the `zeta-chain/node` repository.
Applied to files:
changelog.md
📚 Learning: 2025-09-15T13:42:17.594Z
Learnt from: lumtis
Repo: zeta-chain/node PR: 4199
File: zetaclient/chains/evm/signer/signer_admin.go:25-26
Timestamp: 2025-09-15T13:42:17.594Z
Learning: In PR 4199, the CLI references to CmdMigrateTssFunds in x/crosschain/client/cli/ files are intentionally not updated as they are out of scope for this specific refactor focused on removing ERC20 custody messages.
Applied to files:
zetaclient/chains/sui/signer/withdraw_and_call.go
📚 Learning: 2024-10-30T17:56:16.341Z
Learnt from: gartnera
Repo: zeta-chain/node PR: 3070
File: cmd/zetae2e/init.go:0-0
Timestamp: 2024-10-30T17:56:16.341Z
Learning: In code reviews for Go files like `cmd/zetae2e/init.go` in the ZetaChain project, avoid suggesting unrelated refactoring. Focus comments on changes relevant to the PR objectives.
Applied to files:
zetaclient/chains/sui/signer/withdraw_and_call.go
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: build-zetanode
- GitHub Check: gosec
- GitHub Check: lint
- GitHub Check: build-and-test
- GitHub Check: analyze (go)
- GitHub Check: build
🔇 Additional comments (3)
changelog.md (1)
5-12: Changelog entry accurately documents the fix.The addition of the Fixes entry for PR 4414 clearly describes the purpose of removing the gateway object reference. The formatting is consistent with the existing changelog structure.
e2e/contracts/sui/example/sources/example.move (1)
63-98: Gateway parameter successfully removed from on_call.The function signature and implementation correctly reflect the removal of the Gateway dependency. The authorization and validation logic remains intact using the MessageContext, and the function flow is clear and correct.
zetaclient/chains/sui/signer/withdraw_and_call.go (1)
160-162: Gateway correctly retained for withdraw_impl while removed from on_call.The code properly maintains the gateway object reference for the
withdraw_implcall (which still requires it) while correctly excluding it from theon_callarguments. This selective removal aligns with the Move contract changes.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #4414 +/- ##
===========================================
- Coverage 64.78% 64.78% -0.01%
===========================================
Files 469 469
Lines 28526 28516 -10
===========================================
- Hits 18481 18473 -8
+ Misses 9025 9024 -1
+ Partials 1020 1019 -1
🚀 New features to boost your workflow:
|
| assert!(false, ENonceMismatch); | ||
| }; | ||
|
|
||
| // check if the message context is active |
There was a problem hiding this comment.
Did you investigate how we can ensure the dev is using the right MessageContext? since it might be issued again.
Should we consider telling the devs to hardcode the ID in the program?
There was a problem hiding this comment.
I had checked this. In the issue_message_context_impl we do the following
// remove existing message context ID if any
df::remove_if_exists<vector<u8>, ID>(&mut gateway.id, ACTIVE_MESSAGE_CONTEXT_KEY);
So, the 2nd call to issue_message_context_impl will automatically remove old one. This logic guarantees two things:
- The gateway object has one single message context object under
ACTIVE_MESSAGE_CONTEXT_KEYkey. - The one single message context object is ALWAYS the active.
There was a problem hiding this comment.
I mean the dev should still check in the program that the active one is passed the contract since the old messageContext will still exist a an object, this require to pass the gateway object.
I think the consideration we can have is to skip the check of the messageContext ID since it's unlikely to be changed in practice (need TSS compromision, even for TSS migration, the old messageContext will simply be locked forever), users can also hardcode the object id.
These considerations should be documented somewhere though.
There was a problem hiding this comment.
Another solution that I just thought about is to have a new introduced "ActiveMessageContext" that contains the active message context, the object would be shared meaning it can be updated with the adminCap without requiring ownership, so can be updated in case of TSS compromised.
The object is issued at the same time as the MessageContext and then can be both used with the same chain ID
Description
This PR fix the Sui example package deployment failure. See background: zeta-chain/protocol-contracts-sui#72
How Has This Been Tested?
Summary by CodeRabbit