-
Notifications
You must be signed in to change notification settings - Fork 109
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add Bitcoin Testnet4 static chain info #2911
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 WalkthroughWalkthroughThe changes introduced in this pull request involve the addition of static information for the Bitcoin testnet4 across multiple files. This includes updates to the changelog, OpenAPI specifications, chain configurations in Go, test cases, protocol definitions, and configuration files. The modifications ensure that the new testnet is recognized and integrated into the existing framework, facilitating broader compatibility and testing capabilities. Changes
Possibly related issues
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
|
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: 2
Outside diff range and nitpick comments (3)
zetaclient/config/types.go (1)
53-53
: Approve change and suggest parameter reordering.The addition of "testnet4" to the comment is appropriate and aligns with the PR objective. However, to enhance readability and maintainability, consider reordering the parameters in the comment alphabetically or based on a logical sequence (e.g., from most common to least common usage).
Consider applying the following change:
- RPCParams string // "regtest", "mainnet", "testnet3" , "signet", "testnet4" + RPCParams string // "mainnet", "regtest", "signet", "testnet3", "testnet4"This ordering places the main network first, followed by the test networks in alphabetical order, which may be more intuitive for developers.
pkg/chains/chains.go (1)
185-196
: Approve the addition of BitcoinTestnet4 with a minor suggestion.The
BitcoinTestnet4
variable is correctly defined with appropriate values. To enhance documentation, consider adding a brief comment explaining the purpose of Testnet4 and how it differs from other Bitcoin testnets.Consider adding a comment above the
BitcoinTestnet4
definition:// BitcoinTestnet4 represents Bitcoin's Testnet4, a newer testnet designed for specific testing scenarios. // It uses chain ID 18334 to distinguish it from Testnet3 (18332) and Signet (18333).changelog.md (1)
Line range hint
80-114
: Significant breaking changes affecting queries and module structure.This version introduces several breaking changes:
- Relocation of TSS and chain validation queries from
crosschain
toobserver
module.- Unification of observer sets across all chains.
- Merging of observer params and core params into chain params.
- Modification of the
GetTssAddress
query to require Bitcoin chain ID.These changes may require updates to dependent systems and client applications. Ensure thorough testing and communication with affected parties.
Consider providing a migration guide or deprecation notices to help users adapt to these breaking changes.
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files ignored due to path filters (2)
pkg/chains/chains.pb.go
is excluded by!**/*.pb.go
,!**/*.pb.go
typescript/zetachain/zetacore/pkg/chains/chains_pb.d.ts
is excluded by!**/*_pb.d.ts
Files selected for processing (6)
- changelog.md (1 hunks)
- docs/openapi/openapi.swagger.yaml (1 hunks)
- pkg/chains/chains.go (3 hunks)
- pkg/chains/chains_test.go (5 hunks)
- proto/zetachain/zetacore/pkg/chains/chains.proto (1 hunks)
- zetaclient/config/types.go (1 hunks)
Additional context used
Path-based instructions (4)
pkg/chains/chains.go (1)
Pattern
**/*.go
: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.pkg/chains/chains_test.go (1)
Pattern
**/*.go
: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.proto/zetachain/zetacore/pkg/chains/chains.proto (1)
Pattern
**/*.proto
: Review the Protobuf definitions, point out issues relative to compatibility, and expressiveness.zetaclient/config/types.go (1)
Pattern
**/*.go
: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
Additional comments not posted (20)
pkg/chains/chains_test.go (5)
46-46
: Addition of BitcoinTestnet4 to testnet chains is appropriate.The inclusion of
chains.BitcoinTestnet4
in the list of testnet chains is consistent with the purpose of theTestChainListByNetworkType
function. The placement within the list is logical, maintaining the order of Bitcoin-related chains.
99-99
: BitcoinTestnet4 correctly added to Bitcoin network chains.The addition of
chains.BitcoinTestnet4
to the list of Bitcoin network chains in theTestChainListByNetwork
function is appropriate. Its placement within the list maintains the logical order of Bitcoin-related chains.
149-149
: BitcoinTestnet4 appropriately included in default chain list.The inclusion of
chains.BitcoinTestnet4
in theTestDefaultChainList
function is correct. Its position in the list maintains the logical grouping of Bitcoin-related chains within the default chain list.
187-187
: BitcoinTestnet4 correctly added to observer gateway chains.The addition of
chains.BitcoinTestnet4
to the list of observer gateway chains in theTestChainListByGateway
function is appropriate. Its placement within the list maintains the logical order of Bitcoin-related chains for the observer gateway.
231-231
: BitcoinTestnet4 appropriately included in external chain list.The inclusion of
chains.BitcoinTestnet4
in theTestExternalChainList
function is correct. Its position in the list maintains the logical grouping of Bitcoin-related chains within the external chain list.To ensure consistency across the codebase, please run the following verification script:
pkg/chains/chains.go (1)
377-377
: Approve the inclusion of BitcoinTestnet4 in DefaultChainsList.The addition of
BitcoinTestnet4
to theDefaultChainsList
function is correct and appropriately placed within the list of chains.changelog.md (14)
Line range hint
14-31
: Comprehensive fixes addressing critical issues.The changes in this version include several important fixes:
- Ensuring external chain height always increases.
- Adjusting gas price to buffer EIP1559 increases.
- Modifying WhitelistERC20 authorization.
- Improving pending outbound transaction handling.
- Optimizing Bitcoin keysign scheduling and fee calculation.
- Addressing issues with specific transaction types on Goerli and Mumbai networks.
These fixes contribute to improved reliability and efficiency of the system.
Line range hint
33-34
: Removal of standalone network and adjustment of testing requirements.The removal of the standalone network and the adjustment to use the require testing package for the entire node folder suggests a move towards a more integrated testing approach. This change may lead to more comprehensive and consistent testing across the codebase.
Line range hint
36-39
: Enhanced testing coverage with E2E tests for chain headers and admin functionality.The addition of chain header tests in E2E tests and the fix for admin tests contribute to improved test coverage and reliability of the system.
Line range hint
41-43
: Emission distribution modification for fixed block rewards.The change to use fixed block rewards for emission distribution could lead to more predictable and stable reward mechanisms. Ensure that this change aligns with the overall tokenomics strategy.
Line range hint
45-58
: Comprehensive fixes addressing various issues.The fixes in this version address a wide range of issues, including:
- Preventing incorrect ballot voting.
- Improving chain parameter comparison logic.
- Optimizing gas price checks and fee deductions for system transactions.
- Enhancing keygen status management.
- Resolving zetaclient crashes and improving error handling.
- Optimizing chain parameter support checks.
These fixes contribute to the overall stability and reliability of the system.
Line range hint
60-62
: CI pipeline improvement for release cleanup.The fix for the release pipelines cleanup step should lead to more efficient and reliable CI processes.
Line range hint
64-67
: Documentation and upgrade handler updates.The update to release instructions and the addition of an upgrade handler for version v12.1.0 contribute to improved documentation and smoother upgrade processes.
Line range hint
69-72
: New features for gas limit optimization and inbound transaction verification.The support for lower gas limits in voting and the improved checks for inbound tracker transactions enhance the efficiency and security of the system.
Line range hint
74-78
: Code refactoring and optimization.The refactoring efforts, including code optimization, zetaclient reorganization, and improvements to EVM fee calculation, contribute to better code maintainability and performance.
Line range hint
116-124
: New features enhancing monitoring, state tracking, and chain support.The new features include:
- Addition of monitoring tools for localnet testing.
- Implementation of state variables for tracking aborted zeta amounts.
- Introduction of
snapshots
commands.- Support for dynamic gas pricing on zetachain.
- Addition of static chain data for Sepolia testnet.
- New metrics for tracking hotkey burn rates.
These features contribute to improved monitoring, state management, and chain support.
Line range hint
126-140
: Comprehensive fixes addressing various issues.The fixes in this version address a wide range of issues, including:
- Improvements to UTXO handling and EVM chain outbound transaction processing.
- Enhanced security measures for outbound tracker removal.
- Sanity checks for various events.
- Optimizations for block header posting and keysign operations.
- Resolutions for Athens-3 related issues.
These fixes contribute to improved reliability and security of the system.
Line range hint
142-154
: Significant refactoring and structural improvements.The refactoring efforts include:
- Changes to header verification requirements.
- Reorganization of codebase structure and naming conventions.
- Improvements to query pagination and TSS address handling.
- Updates to observer params and chain params organization.
These changes should lead to improved code maintainability and performance.
Line range hint
156-163
: Chores and minor improvements.The chores include various file renaming, script additions, and build process improvements. These changes contribute to better code organization and development workflows.
Line range hint
165-167
: Improved testing and CI processes.The addition of stateful e2e testing and removal of private runners contribute to more robust testing and CI processes.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #2911 +/- ##
========================================
Coverage 67.49% 67.49%
========================================
Files 380 380
Lines 21169 21170 +1
========================================
+ Hits 14287 14288 +1
Misses 6213 6213
Partials 669 669
|
Description
Note: we still need
btcd
to ship the support fortestnet4
to be able to integrate it. They had testnet4 support tracked here and a draft patch for testnet4How Has This Been Tested?
Summary by CodeRabbit
New Features
btc_testnet4
as an accepted chain name.Documentation
Bug Fixes