SUI - fix ExtraData parsing (LOOP)#21796
Conversation
krebernisak
requested review from
AmrMohamedRezk,
agusaldasoro,
b-gopalswami,
chris-de-leon-cll,
Copilot,
ogtownsend,
tt-cll and
winder
|
👋 krebernisak, thanks for creating this pull request! To help reviewers, please consider creating future PRs as drafts first. This allows you to self-review and make any final changes before notifying the team. Once you're ready, you can mark it as "Ready for review" to request feedback. Thanks! |
|
I see you updated files related to
|
|
✅ No conflicts with other open PRs targeting |
There was a problem hiding this comment.
Pull request overview
Risk Rating: HIGH — changes touch CCIP message hashing inputs for Sui; mistakes here can cause incorrect hashes and failed/off-by-one message verification.
This PR aims to make Sui ExtraData decoding resilient to LOOP gRPC type conversions (e.g., [32]byte → []byte, uint32 → int64) similar to the Solana fix referenced in #21790.
Changes:
- Update Sui
parseExtraDataMapto accepttokenReceiveras either[32]byteor LOOP-converted[]byte(with length validation). - Update Sui
extractDestGasAmountFromMapto accept LOOP-convertedint64in addition touint32. - Add unit tests for
parseExtraDataMaptokenReceiver parsing; minor formatting/comment cleanup elsewhere.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| deployment/ccip/shared/stateview/state.go | Whitespace/formatting-only change in SupportedChains(). |
| core/capabilities/ccip/ccipsui/msghasher.go | Adds LOOP-compatible parsing for tokenReceiver and destGasAmount (helper). |
| core/capabilities/ccip/ccipsui/msghasher_test.go | Adds tests for [32]byte vs []byte tokenReceiver parsing, including invalid length case. |
| core/capabilities/ccip/ccipaptos/msghasher.go | Removes an incorrect/outdated inline comment. |
| switch v := fieldValue.(type) { | ||
| case uint32: | ||
| return v, nil | ||
| case int64: // LOOP converts expected uint32 to int64 | ||
| if v > math.MaxUint32 { | ||
| return 0, fmt.Errorf("destGasAmount exceeds uint32 max, got %d", v) | ||
| } | ||
| return uint32(v), nil //nolint:gosec // G115: validated to be within uint32 max above | ||
| default: | ||
| return 0, errors.New("invalid type for destgasamount, expected uint32 or int64") | ||
| } |
There was a problem hiding this comment.
New behavior is added here to accept LOOP-converted destGasAmount values (int64 → uint32 with bounds checks), but there are no tests covering this conversion (including boundary cases like negative values and values > MaxUint32). Please add unit tests for extractDestGasAmountFromMap similar to the existing parseExtraDataMap tests.
|
* SUI - fix ExtraData parsing (LOOP) * Add test * Fix destgasamount parsing
When a non-EVM source chain (e.g. TON) sends a message to SUI, the source chain's ExtraDataCodec runs in a LOOP plugin. The decoded map[string]any goes through gRPC protobuf serialization which changes Go types:
Supports
#21790