feat: initialize pkg/contracts/sui
#3500
Conversation
📝 WalkthroughWalkthroughThis pull request updates the Sui gateway integration across the project. The Changes
Sequence Diagram(s)sequenceDiagram
participant R as E2ERunner.SetupSui
participant Z as zetasui
participant C as SuiClient
R->>Z: GatewayBytecodeBase64()
Z-->>R: gateway bytecode (base64)
R->>C: PublishTransaction(PublishRequest with gateway bytecode)
C-->>R: TransactionResponse
sequenceDiagram
participant G as Gateway
participant API as Sui API Client
participant P as Parser
G->>API: QueryDepositInbounds(ctx, from, to)
API-->>G: PaginatedEventsResponse
G->>P: parseInbound(event)
P-->>G: Inbound struct (or error)
G-->>Caller: Return list of Inbound events
Possibly related PRs
Suggested reviewers
✨ Finishing Touches
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
Documentation and Community
|
gartnera
added
no-changelog
![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
🧹 Nitpick comments (7)
pkg/contracts/sui/inbound.go (3)
29-33: Consider handling the TODO more explicitly.
The TODO at line 31 references a dedicated error type for invalid event data. Implementing a custom error type, for instanceErrInvalidEventData, would streamline error handling and allow consumers to distinguish parse errors more accurately.Would you like help implementing this specific error type and updating the call sites accordingly?
34-69: Refine error checking forevent.ParsedJson.
This function leveragesparsedJSONheavily but does not explicitly check ifevent.ParsedJsonis nil. If the upstream API behavior changes,parsedJSONmight become nil and cause a runtime panic. Adding a guard clause or early return for nil would make the parsing logic more robust.func parseInbound(event models.SuiEventResponse, eventType string) (Inbound, error) { + if event.ParsedJson == nil { + return Inbound{}, errors.New("parsed JSON is nil") + } parsedJSON := event.ParsedJson ... }
98-113: Consider Go 1.13+ native error wrapping.
TheconvertPayloadfunction usesfmt.Errorf(...)but also relies onerrors.Wrapin other sections of the file. To keep the codebase consistent and embrace newer language features, switching to native error wrapping (fmt.Errorf("... %w", err)) is an option. This is purely a stylistic choice and not functionally mandatory.pkg/contracts/sui/inbound_test.go (1)
10-52: Expand test coverage to parse inbound data.
Currently, the test only covers theIsGasDepositmethod. Consider also testingparseInboundandconvertPayloadto ensure event fields (e.g.,CoinType,Amount,Sender, andPayload) are parsed correctly and error cases are handled as intended.e2e/runner/setup_sui.go (2)
33-33: Consider making the gas budget configurable.The gas budget is currently hardcoded to "5000000000". Consider making it configurable through a constant or configuration parameter for better maintainability and flexibility.
+const defaultGasBudget = "5000000000" + func (r *E2ERunner) SetupSui(faucetURL string) { // ... - GasBudget: "5000000000", + GasBudget: defaultGasBudget,
29-32: Consider documenting the hardcoded addresses.The dependencies array contains hardcoded addresses without explanation. Add comments explaining what these addresses represent.
Dependencies: []string{ + // Sui framework package ID "0x0000000000000000000000000000000000000000000000000000000000000001", + // Sui system package ID "0x0000000000000000000000000000000000000000000000000000000000000002", },pkg/contracts/sui/gateway_test.go (1)
22-22: Consider making the testnet endpoint configurable.The testnet endpoint is hardcoded. Consider making it configurable through environment variables or test flags.
+const defaultTestnetEndpoint = "https://sui-testnet-endpoint.blockvision.org" + +func getTestnetEndpoint() string { + if endpoint := os.Getenv("SUI_TESTNET_ENDPOINT"); endpoint != "" { + return endpoint + } + return defaultTestnetEndpoint +} + func TestLiveGateway_ReadInbounds(t *testing.T) { - client := sui.NewSuiClient("https://sui-testnet-endpoint.blockvision.org") + client := sui.NewSuiClient(getTestnetEndpoint())
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
e2e/runner/setup_sui.go(2 hunks)e2e/utils/sui/gateway.go(0 hunks)pkg/contracts/sui/gateway.go(1 hunks)pkg/contracts/sui/gateway_test.go(1 hunks)pkg/contracts/sui/inbound.go(1 hunks)pkg/contracts/sui/inbound_test.go(1 hunks)pkg/contracts/sui/mocks/mocks.go(1 hunks)pkg/contracts/sui/mocks/sui_client.go(1 hunks)
💤 Files with no reviewable changes (1)
- e2e/utils/sui/gateway.go
🧰 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.
pkg/contracts/sui/mocks/mocks.goe2e/runner/setup_sui.gopkg/contracts/sui/inbound_test.gopkg/contracts/sui/inbound.gopkg/contracts/sui/gateway_test.gopkg/contracts/sui/gateway.gopkg/contracts/sui/mocks/sui_client.go
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: start-sui-test / e2e
- GitHub Check: start-e2e-test / e2e
- GitHub Check: gosec
- GitHub Check: build-and-test
- GitHub Check: build
- GitHub Check: Analyze (go)
🔇 Additional comments (6)
pkg/contracts/sui/inbound.go (2)
25-27: Looks great!
TheIsGasDepositmethod is concise and clearly communicates intent.
72-84: Payload format concern overlaps with a previous reviewer’s feedback.
We see from past review notes that some payloads could arrive in a different format (e.g., base64-encoded string). The current logic strictly expects a float64 array. If future updates allow a string-based payload, it will break here.Please confirm whether you anticipate additional payload formats. If so, we can extend
convertPayloadto handle those cases.pkg/contracts/sui/mocks/mocks.go (1)
14-20: Mock setup looks good.
This approach effectively sets the mock's expectations forSuiXQueryEventswithin a single call. It keeps tests predictable and well structured.pkg/contracts/sui/gateway.go (1)
64-80: Time range query functionality is commented out.The time range query functionality is commented out due to an API issue. This should be tracked and fixed to ensure proper event filtering.
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Description: Check if the issue is still open gh issue view 3523 --json state,title,bodyLength of output: 454
Time Range Query Functionality Comment Verified: Issue Still Open
The GitHub CLI output confirms that GitHub issue #3523 remains open, which justifies the commented out time range query functionality in the code. This open issue should be tracked, and once it is resolved or a viable workaround is found, the functionality can be reinstated or replaced accordingly.
- File: pkg/contracts/sui/gateway.go (lines 64-80)
- Issue: API error when querying events by time range (#3523 remains open)
pkg/contracts/sui/gateway_test.go (1)
60-66: LGTM! Well-structured test table.The test table is well-organized with clear test cases and expected outcomes.
pkg/contracts/sui/mocks/sui_client.go (1)
1-1678: LGTM! Well-generated mock implementation.The mock implementation is comprehensive and follows good practices for testing. The auto-generated code provides good coverage of the Sui client interface.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## feat/sui-observer #3500 +/- ##
====================================================
Coverage ? 65.52%
====================================================
Files ? 445
Lines ? 30606
Branches ? 0
====================================================
Hits ? 20055
Misses ? 9697
Partials ? 854 |
Description
Closes #3471
Conflicts with #3485No changelog as no user facing changes
Summary by CodeRabbit
New Features
Tests
Chores