-
Notifications
You must be signed in to change notification settings - Fork 115
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
fix(zetaclient): use chain name in metrics #3484
Conversation
📝 WalkthroughWalkthroughThis pull request refactors the handling of chain identifiers in cross-chain transaction listing. The Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Tip 🌐 Web search-backed reviews and chat
✨ Finishing Touches
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
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
zetaclient/chains/bitcoin/bitcoin.go (1)
133-134
: Consider combining chain object usage.While the implementation is correct, consider combining the chain object usage to reduce variable declarations:
- chain := b.observer.Chain() - chainID := chain.ChainId + chain := b.observer.Chain()Then update line 150 to use
chain.ChainId
directly:- if params.ReceiverChainId != chainID { + if params.ReceiverChainId != chain.ChainId {
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
pkg/rpc/clients_crosschain.go
(1 hunks)zetaclient/chains/bitcoin/bitcoin.go
(1 hunks)zetaclient/chains/evm/evm.go
(2 hunks)zetaclient/chains/interfaces/interfaces.go
(1 hunks)zetaclient/chains/solana/solana.go
(2 hunks)zetaclient/chains/ton/ton.go
(1 hunks)zetaclient/testutils/mocks/zetacore_client.go
(2 hunks)zetaclient/zetacore/client_crosschain.go
(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- pkg/rpc/clients_crosschain.go
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.go`: Review the Go code, point out issues relative to ...
**/*.go
: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
zetaclient/chains/evm/evm.go
zetaclient/chains/solana/solana.go
zetaclient/testutils/mocks/zetacore_client.go
zetaclient/chains/ton/ton.go
zetaclient/chains/bitcoin/bitcoin.go
zetaclient/zetacore/client_crosschain.go
zetaclient/chains/interfaces/interfaces.go
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: start-ton-test / e2e
- GitHub Check: start-solana-test / e2e
- GitHub Check: start-e2e-test / e2e
- GitHub Check: Analyze (go)
- GitHub Check: build-and-test
- GitHub Check: gosec
🔇 Additional comments (6)
zetaclient/zetacore/client_crosschain.go (1)
16-26
: LGTM! Clean implementation of chain name metrics.The changes effectively implement the requirement to use chain names in metrics, providing better readability and maintainability.
zetaclient/chains/ton/ton.go (1)
133-135
: LGTM! Clean chain object usage.The implementation correctly uses the chain object, maintaining consistency with other chain handlers.
zetaclient/chains/interfaces/interfaces.go (1)
79-79
: LGTM! Clean interface update.The interface change properly reflects the new design pattern of using chain objects directly.
zetaclient/chains/solana/solana.go (1)
137-140
: LGTM! Clean refactoring to use chain object.The changes improve code clarity by explicitly defining the chain object before accessing its ID. This aligns with the PR objective of using chain objects consistently throughout the codebase.
Also applies to: 151-151
zetaclient/chains/evm/evm.go (1)
133-136
: LGTM! Consistent refactoring with Solana implementation.The changes follow the same clean pattern of explicitly defining the chain object before accessing its ID, maintaining consistency across different chain implementations.
Also applies to: 155-155
zetaclient/testutils/mocks/zetacore_client.go (1)
653-688
: LGTM! Mock implementation correctly updated.The auto-generated mock implementation correctly reflects the interface changes, updating the
ListPendingCCTX
method to usechains.Chain
instead ofint64
.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #3484 +/- ##
========================================
Coverage 65.28% 65.29%
========================================
Files 441 441
Lines 30429 30430 +1
========================================
+ Hits 19865 19868 +3
+ Misses 9708 9706 -2
Partials 856 856
|
In v27.0.0 the
chain
changed from a string name to the integer ID:revert that to the v26 behavior:
No changelog as it will be released on v27.0.2
Summary by CodeRabbit
Documentation
Refactor