feat(analyzer): Implement SUI analyzer #482
Conversation
🦋 Changeset detectedLatest commit: c61f1c8 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
There was a problem hiding this comment.
Pull Request Overview
This PR implements SUI transaction analyzer functionality that enables the framework to decode and analyze SUI blockchain transactions. The implementation follows the existing pattern used for other blockchain families like Aptos and EVM.
Key changes:
- Added SUI-specific transaction analyzer with decoder integration
- Extended the universal proposal framework (UPF) to support SUI transactions
- Refactored shared utility functions from Aptos analyzer to a common utils module
Reviewed Changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| go.mod | Updated dependencies for SUI SDK and MCMS packages |
| experimental/analyzer/utils.go | New shared utility functions for converting decoded operations to named arguments |
| experimental/analyzer/upf/upf.go | Added SUI support to transaction encoding and batch operations processing |
| experimental/analyzer/test_helpers.go | Added shared test helper function for generating expected output format |
| experimental/analyzer/sui_analyzer_test.go | Test suite for SUI transaction analyzer functionality |
| experimental/analyzer/sui_analyzer.go | Core SUI transaction analyzer implementation |
| experimental/analyzer/aptos_analyzer_test.go | Updated tests to use shared helper function |
| experimental/analyzer/aptos_analyzer.go | Removed duplicated utility functions (moved to utils.go) |
| experimental/analyzer/analyze.go | Added SUI case to batch operations and single operations analysis |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| ) | ||
|
|
||
| func TestAnalyzeSuiTransactions(t *testing.T) { | ||
| var testAddress = "0xe86f0e5a8b9cb6ab31b656baa83a0d2eb761b32eb31b9a9c74abb7d0cffd26fa" |
There was a problem hiding this comment.
[nitpick] The testAddress variable is declared globally and has the same value as the constant in aptos_analyzer_test.go. Consider moving this to a shared test constants file or using a more descriptive variable name to differentiate SUI addresses from Aptos addresses.
Copilot uses AI. Check for mistakes.
rodrigombsoares
force-pushed
the
sui-analyzer
branch
from
224fbad to
62ba95a
Compare
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 9 out of 10 changed files in this pull request and generated 1 comment.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| if tt.wantErr { | ||
| t.Fatal("AnalyzeSuiTransactions() succeeded unexpectedly") | ||
| } | ||
| // TODO: update the condition below to compare got with tt.want. |
There was a problem hiding this comment.
This TODO comment indicates incomplete test implementation. The comment suggests the comparison logic needs to be updated, but the test is already using reflect.DeepEqual correctly.
Copilot uses AI. Check for mistakes.
|
hi @rodrigombsoares , you might want to add a changeset file using |
rodrigombsoares
dismissed stale reviews from JohnChangUK and RodrigoAD
via
ec7a775
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 11 out of 13 changed files in this pull request and generated no new comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| return nil, fmt.Errorf("error getting sui metadata from proposal: %w", err) | ||
| } | ||
| chain := cfg.blockchains.SuiChains()[uint64(chainSelector)] | ||
| entrypointEncoder := suibindings.NewCCIPEntrypointArgEncoder() |
There was a problem hiding this comment.
Does this mean the encoder won't work with contracts from other products? (if/when they are created)
There was a problem hiding this comment.
It means that new products will need their own product encoders, or extend the existing one.
There was a problem hiding this comment.
Then we should figure out a way to allow product teams to specify their encoders from within their domain and pass it to the mcms CLI. See, for example, how the encoder for EVM is implemented (though I think that implementation can be simplified).
There was a problem hiding this comment.
I agree. I'll look into the EVM encoder.
For this case I thought on allowing multiple Encoders to be passed to the MCMS lib, and probably the proposal would indicate the product is using, and select the Encoder based on it. I'll note to dive and add this into MCMS
| return nil, fmt.Errorf("error getting sui metadata from proposal: %w", err) | ||
| } | ||
| chain := cfg.blockchains.SuiChains()[uint64(chainSelector)] | ||
| entrypointEncoder := suibindings.NewCCIPEntrypointArgEncoder() |
There was a problem hiding this comment.
Just noticed the NewCCIPEntrypointArgEncoder doesn't receive the registryObjID.
Please use &CCIPEntrypointArgEncoder{registryObjID: <value>} to construct it
There was a problem hiding this comment.
Where can I get the value of registryObjID?
Depends on: smartcontractkit/mcms#487
Summary
Implement SUI transaction analyzer from SUI decoder.
Features
analyze.goupf.goutilsandtesthelperfiles