fix(sui): process multiple gateway events per tx#4596
fix(sui): process multiple gateway events per tx#4596alan747271363-art wants to merge 2 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe PR implements transaction block caching within each inbound observer scan to enable efficient processing of multiple Gateway events from the same Sui transaction, and adds comprehensive test coverage for this multi-event scenario across both the main observer and tracker recovery paths. ChangesMulti-event Sui Gateway processing
Sequence DiagramsequenceDiagram
participant Scan as Scan Loop
participant Cache as txCache Map
participant ProcessFunc as processInboundEvent
participant RPC as Sui RPC
Scan->>Cache: Create per-scan txCache
loop For each inbound event
Scan->>ProcessFunc: Call with event and txCache
ProcessFunc->>Cache: Lookup by TxHash
alt Transaction in cache
Cache-->>ProcessFunc: Return cached block
else Transaction not in cache
ProcessFunc->>RPC: Fetch SuiTransactionBlockResponse
RPC-->>ProcessFunc: Transaction block
ProcessFunc->>Cache: Store by TxHash
end
ProcessFunc->>ProcessFunc: Process Gateway event
end
🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
|
Follow-up pushed in What changed:
Validation:
|
Summary
Tests
Closes #4584
Greptile Summary
This PR fixes the Sui inbound observer to correctly handle transactions that emit more than one gateway event (e.g. a contract depositing to multiple accounts in a single PTB). Previously a hard guard rejected any event whose
EventIndexwas non-zero, which silently skipped all but the first event in such transactions.event.EventIndex != 0early-return inprocessInboundEvent, allowing every gateway event in a transaction to be parsed, validated, and voted on independently using its ownEventIndex.ObserveInbound) and recovery (ProcessInboundTrackers) paths, verifying that two distinct inbound votes are produced with the correct receiver, amount, and event index for a two-event transaction.SampleEventWithSeqtest helper to construct mock events with an arbitrary sequence number.Confidence Score: 4/5
Safe to merge; the fix is focused, cursor advancement and vote idempotency are handled correctly, and both recovery paths are covered by tests.
The production change is a five-line deletion of a guard that blocked multi-event transactions. Cursor management, compliance skipping, and errTxNotFound/errVoteInbound retry semantics all continue to work correctly for the multi-event case. The one non-critical concern is that the block-scan path issues a redundant SuiGetTransactionBlock call per event for the same digest; the test even hard-codes two identical OnGetTx registrations for the same hash, confirming the behaviour is present but benign at current scale.
No files require special attention; inbound.go is the only production change and it is small and well-tested.
Important Files Changed
Sequence Diagram
sequenceDiagram participant Observer participant SuiRPC participant ZetaCore Note over Observer: Block-scan path (ObserveInbound) Observer->>SuiRPC: QueryModuleEvents(cursor) SuiRPC-->>Observer: "[Event{TxA, seq=0}, Event{TxA, seq=1}, ...]" loop for each event Observer->>SuiRPC: SuiGetTransactionBlock(TxA) SuiRPC-->>Observer: tx (checkpoint, effects) Observer->>ZetaCore: "VoteInbound(TxA, EventIndex=N)" Observer->>Observer: setCursor(packageID, event.Id) end Note over Observer: Tracker path (ProcessInboundTrackers) Observer->>SuiRPC: "SuiGetTransactionBlock(TxA, ShowEvents=true)" SuiRPC-->>Observer: "tx with Events[seq=0, seq=1]" loop for each tx.Event Observer->>ZetaCore: "VoteInbound(TxA, EventIndex=N)" endComments Outside Diff (1)
zetaclient/chains/sui/observer/inbound.go, line 73-102 (link)When a single transaction emits N gateway events,
observeGatewayInboundwill callSuiGetTransactionBlockonce per event (N times in total), even though all events share the sameTxDigest. The test explicitly registersOnGetTxtwice for the same hash (ts.OnGetTx(txHash, "10000", true, false, nil)× 2) confirming the current behavior.Under normal operation this is just wasteful, but it becomes a concern at scale when the same tx produces many events and the RPC node is rate-limited or has latency. Consider caching the fetched
*models.SuiTransactionBlockResponsebyTxDigestwithin the scan loop (a simplemap[string]*txis enough) and passing the cached value toprocessInboundEventon subsequent events for the same digest.Prompt To Fix With AI
Prompt To Fix All With AI
Reviews (1): Last reviewed commit: "fix(sui): process multiple gateway event..." | Re-trigger Greptile
Summary by CodeRabbit
Performance
Tests