fix: cherry pick athens3 hotfixes #3253
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 Walkthrough📝 WalkthroughWalkthroughThe pull request introduces multiple enhancements and fixes to the ZetaChain node, including the addition of a new telemetry endpoint Changes
Possibly related PRs
Suggested labels
Suggested reviewers
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
|
ws4charlie
added
zetaclient
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #3253 +/- ##
========================================
Coverage 61.74% 61.75%
========================================
Files 431 431
Lines 30772 30772
========================================
+ Hits 19001 19004 +3
+ Misses 10914 10911 -3
Partials 857 857
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (2)
zetaclient/chains/solana/observer/inbound_tracker.go (1)
67-67: Enhance warning message with transaction contextThe warning message could be more helpful for debugging by including additional context.
- ob.Logger().Inbound.Warn().Stringer("tx.signature", signature).Msg("skip inbound tracker hash") + ob.Logger().Inbound.Warn(). + Stringer("tx.signature", signature). + Str("chain_id", chainID.String()). + Msg("skipping inbound tracker: unsupported transaction version")zetaclient/chains/solana/rpc/rpc_live_test.go (1)
93-95: Consider extracting test constantsThe hardcoded signature could become invalid if the devnet state changes.
Consider moving test constants to a dedicated test constants file:
// testdata/constants.go var DevnetTestSignatures = map[chains.Chain]string{ chains.SolanaDevnet: "39fSgD2nteJCQRQP3ynqEcDMZAFSETCbfb61AUqLU6y7qbzSJL5rn2DHU2oM35zsf94Feb5C5QWd5L5UnncBsAay", }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (6)
changelog.md(1 hunks)zetaclient/chains/solana/observer/inbound_tracker.go(2 hunks)zetaclient/chains/solana/rpc/rpc.go(2 hunks)zetaclient/chains/solana/rpc/rpc_live_test.go(3 hunks)zetaclient/chains/solana/signer/signer.go(1 hunks)zetaclient/keys/relayer_key.go(2 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
zetaclient/keys/relayer_key.go (1)
Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
zetaclient/chains/solana/observer/inbound_tracker.go (1)
Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
zetaclient/chains/solana/rpc/rpc_live_test.go (1)
Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
zetaclient/chains/solana/rpc/rpc.go (1)
Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
zetaclient/chains/solana/signer/signer.go (1)
Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
🪛 GitHub Check: codecov/patch
zetaclient/chains/solana/observer/inbound_tracker.go
[warning] 64-69: zetaclient/chains/solana/observer/inbound_tracker.go#L64-L69
Added lines #L64 - L69 were not covered by tests
zetaclient/chains/solana/rpc/rpc.go
[warning] 88-89: zetaclient/chains/solana/rpc/rpc.go#L88-L89
Added lines #L88 - L89 were not covered by tests
[warning] 136-136: zetaclient/chains/solana/rpc/rpc.go#L136
Added line #L136 was not covered by tests
zetaclient/chains/solana/signer/signer.go
[warning] 166-169: zetaclient/chains/solana/signer/signer.go#L166-L169
Added lines #L166 - L169 were not covered by tests
🪛 LanguageTool
changelog.md
[style] ~22-~22: ‘prior to’ might be wordy. Consider a shorter alternative.
Context: ... version 0 queries and move tss keysign prior to relayer key checking ## v23.0.0 ### F...
(EN_WORDINESS_PREMIUM_PRIOR_TO)
🔇 Additional comments (4)
zetaclient/keys/relayer_key.go (1)
72-72: 🛠️ Refactor suggestion
Use structured logging consistently
The warning uses the global logger directly. Consider using structured logging for consistency.
- log.Logger.Warn().Msgf("relayer key file not found: %s", fileName)
+ log.Warn().
+ Str("file", fileName).
+ Str("network", network.String()).
+ Msg("relayer key file not found")Likely invalid or redundant comment.
zetaclient/chains/solana/rpc/rpc.go (2)
88-89: Improve test coverage for error handling logic.
The error handling for unsupported transaction versions is a critical path that should be covered by tests.
Run the following script to check existing test coverage:
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 88-89: zetaclient/chains/solana/rpc/rpc.go#L88-L89
Added lines #L88 - L89 were not covered by tests
136-136: Verify commitment level configuration.
The commitment level is hardcoded to rpc.CommitmentFinalized. Consider making this configurable through chain parameters if different commitment levels are needed for different scenarios.
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 136-136: zetaclient/chains/solana/rpc/rpc.go#L136
Added line #L136 was not covered by tests
changelog.md (1)
22-22: LGTM! Changelog entry is accurate and complete.
The changelog entry correctly documents both changes:
- Fix for Solana inbound version 0 queries
- Moving TSS keysign prior to relayer key checking
These align with the code changes observed in rpc.go and signer.go.
🧰 Tools
🪛 LanguageTool
[style] ~22-~22: ‘prior to’ might be wordy. Consider a shorter alternative.
Context: ... version 0 queries and move tss keysign prior to relayer key checking ## v23.0.0 ### F...
(EN_WORDINESS_PREMIUM_PRIOR_TO)
Description
This PR includes two zetaclient hotfixes in Athens3:
How Has This Been Tested?
Summary by CodeRabbit
Release Notes
New Features
/systemtimein the ZetaChain client.Bug Fixes
Tests
Refactor