Conversation
📝 WalkthroughWalkthroughThe changes refactor the Solana gateway instruction parsing logic to support multiple gateway instructions per transaction and enforce nonce validation during parsing. The Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Observer
participant SolanaTx
Client->>Observer: ParseGatewayInstruction(txResult, gatewayID, coinType, expectedNonce)
Observer->>SolanaTx: Iterate all instructions
loop For each instruction
SolanaTx-->>Observer: If program matches gatewayID
Observer->>Observer: Try parse as increment nonce or outbound (by coinType)
Observer->>Observer: If nonce matches expectedNonce, collect
end
alt No matches
Observer-->>Client: Error (no matching outbound instruction with expected nonce)
else Multiple matches
Observer-->>Client: Error (multiple matching outbound instructions)
else Single match
Observer-->>Client: Return parsed outbound instruction
end
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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
|
skosito
changed the title
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #3957 +/- ##
===========================================
- Coverage 64.18% 64.18% -0.01%
===========================================
Files 474 474
Lines 34804 34803 -1
===========================================
- Hits 22338 22337 -1
- Misses 11437 11438 +1
+ Partials 1029 1028 -1
🚀 New features to boost your workflow:
|
![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
🧹 Nitpick comments (2)
zetaclient/chains/solana/observer/outbound.go (2)
351-355: Consider adding test coverage for edge cases.The static analysis indicates that the
increment_nonceinstruction parsing (lines 351-355) and theCmd/NoAssetCallcoin type handling (lines 363-368) lack test coverage. While the implementation looks correct, adding tests for these paths would improve confidence in the code.Would you like me to help generate test cases for these uncovered code paths?
Also applies to: 363-368
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 351-355: zetaclient/chains/solana/observer/outbound.go#L351-L355
Added lines #L351 - L355 were not covered by tests
391-393: Enhance error message for duplicate nonce detection.While detecting multiple instructions with the same nonce is good defensive programming, the error message could be more informative for debugging purposes.
// should not happen if matchCount > 1 { - return nil, fmt.Errorf("multiple outbounds with same nonce detected") + return nil, fmt.Errorf("multiple outbound instructions detected with nonce %d", expectedNonce) }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
changelog.md(1 hunks)zetaclient/chains/solana/observer/outbound.go(3 hunks)zetaclient/chains/solana/observer/outbound_test.go(9 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.go`: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
zetaclient/chains/solana/observer/outbound_test.gozetaclient/chains/solana/observer/outbound.go
🧬 Code Graph Analysis (1)
zetaclient/chains/solana/observer/outbound_test.go (1)
zetaclient/chains/solana/observer/outbound.go (1)
ParseGatewayInstruction(320-396)
🪛 GitHub Check: codecov/patch
zetaclient/chains/solana/observer/outbound.go
[warning] 128-128: zetaclient/chains/solana/observer/outbound.go#L128
Added line #L128 was not covered by tests
[warning] 351-355: zetaclient/chains/solana/observer/outbound.go#L351-L355
Added lines #L351 - L355 were not covered by tests
[warning] 363-366: zetaclient/chains/solana/observer/outbound.go#L363-L366
Added lines #L363 - L366 were not covered by tests
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: start-solana-test / e2e
🔇 Additional comments (5)
changelog.md (1)
12-12: LGTM!The changelog entry is correctly formatted and accurately describes the fix.
zetaclient/chains/solana/observer/outbound_test.go (2)
223-223: Test updates correctly reflect the API changes.All test calls to
ParseGatewayInstructionhave been properly updated to include theexpectedNonceparameter with value0.Also applies to: 246-246, 249-249, 264-264, 282-282, 300-300, 303-303, 318-318, 331-331, 935-935, 953-953
249-249: Error message assertions updated appropriately.The error message checks now correctly verify the new format that includes the expected nonce value.
Also applies to: 303-303
zetaclient/chains/solana/observer/outbound.go (2)
128-128: Function calls updated correctly with nonce parameter.Both
VoteOutboundIfConfirmedandCheckFinalizedTxnow pass the nonce toParseGatewayInstruction, maintaining consistency with the new API.Also applies to: 280-280
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 128-128: zetaclient/chains/solana/observer/outbound.go#L128
Added line #L128 was not covered by tests
320-396: Excellent refactoring to remove transaction format assumptions.The new implementation properly iterates through all instructions and validates against the expected nonce, making the parser more robust and flexible. The error aggregation provides better debugging information when parsing fails.
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 351-355: zetaclient/chains/solana/observer/outbound.go#L351-L355
Added lines #L351 - L355 were not covered by tests
[warning] 363-366: zetaclient/chains/solana/observer/outbound.go#L363-L366
Added lines #L363 - L366 were not covered by tests
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
changelog.md(1 hunks)zetaclient/chains/solana/observer/outbound.go(3 hunks)zetaclient/chains/solana/observer/outbound_test.go(9 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.go`: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
zetaclient/chains/solana/observer/outbound_test.gozetaclient/chains/solana/observer/outbound.go
🧬 Code Graph Analysis (1)
zetaclient/chains/solana/observer/outbound.go (1)
pkg/contracts/solana/instruction.go (4)
OutboundInstruction(135-147)ParseInstructionIncrementNonce(201-217)ParseInstructionWhitelist(698-712)ParseInstructionExecute(339-353)
🪛 GitHub Check: codecov/patch
zetaclient/chains/solana/observer/outbound.go
[warning] 128-128: zetaclient/chains/solana/observer/outbound.go#L128
Added line #L128 was not covered by tests
[warning] 351-355: zetaclient/chains/solana/observer/outbound.go#L351-L355
Added lines #L351 - L355 were not covered by tests
[warning] 363-366: zetaclient/chains/solana/observer/outbound.go#L363-L366
Added lines #L363 - L366 were not covered by tests
🔇 Additional comments (3)
changelog.md (1)
12-12: LGTM!The changelog entry accurately documents the fix for removing transaction format assumptions.
zetaclient/chains/solana/observer/outbound_test.go (1)
223-223: Test updates are comprehensive and correct.All test cases have been properly updated to pass the expected nonce parameter and verify the enhanced error messages that include nonce validation context.
Also applies to: 246-249, 264-264, 283-283, 300-303, 318-318, 331-331, 936-936, 954-954
zetaclient/chains/solana/observer/outbound.go (1)
391-393: Good defensive programming for nonce uniqueness.The check for multiple instructions with the same nonce prevents ambiguity and potential security issues. This is a critical safety measure.
3 participants
Description
https://github.com/zeta-chain/protocol-private/issues/235
How Has This Been Tested?
Summary by CodeRabbit
Bug Fixes
Documentation