chore: upgrade bitcoin-core to 28.0 #3191
Conversation
gartnera
requested review from
brewmaster012,
kingpinXD,
lumtis,
ws4charlie,
skosito,
swift1337 and
fbac
as code owners
|
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 significant modifications to the 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
|
![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
🧹 Outside diff range and nitpick comments (2)
e2e/runner/setup_bitcoin.go (2)
73-78: Add documentation about bitcoin-core version requirements.The implementation correctly handles the wallet creation for bitcoin-core 27.2, but would benefit from documentation explaining the version-specific requirements.
Add a comment block explaining the version requirements:
// SetupBtcAddress setups the deployer Bitcoin address +// Note: This implementation is specific to bitcoin-core >= 27.2 and requires +// the -deprecatedrpc=create_bdb configuration option to support wallet +// creation with descriptors=false. func (r *E2ERunner) SetupBtcAddress(createWallet bool) {
101-104: Enhance error handling specificity.The current error handling checks for a generic "Database already exists" message. Consider making this more specific to avoid catching unrelated errors.
Consider this more specific approach:
- if err != nil { - require.ErrorContains(r, err, "Database already exists") - } + if err != nil { + // Ignore only if the wallet already exists, fail for other errors + if !strings.Contains(err.Error(), "Database already exists") { + require.NoError(r, err, "failed to create wallet") + } + }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (2)
contrib/localnet/docker-compose.yml(2 hunks)e2e/runner/setup_bitcoin.go(3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
e2e/runner/setup_bitcoin.go (1)
Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
📓 Learnings (1)
e2e/runner/setup_bitcoin.go (1)
Learnt from: gartnera
PR: zeta-chain/node#3105
File: e2e/runner/setup_bitcoin.go:51-69
Timestamp: 2024-11-12T13:20:12.658Z
Learning: In test code (`e2e/runner/setup_bitcoin.go`), adding security measures for private key handling in the `GetBtcAddress` method is not required.
🔇 Additional comments (4)
e2e/runner/setup_bitcoin.go (3)
5-5: LGTM: Import addition aligns with new JSON handling requirements.
The addition of the encoding/json import is necessary for constructing the raw JSON RPC request in the SetupBtcAddress method.
38-38: LGTM: Method call updated to match new signature.
The simplified method call aligns with the updated SetupBtcAddress signature, maintaining functionality while reducing parameter complexity.
86-94: Verify alignment with bitcoin-core 27.2 requirements.
The implementation correctly sets descriptors=false and includes all required parameters for wallet creation in bitcoin-core 27.2.
Let's verify the bitcoin-core version and configuration in the docker setup:
✅ Verification successful
Implementation aligns with bitcoin-core 27.2 requirements
The verification confirms that:
- Docker image uses bitcoin-core 27.2 (
ghcr.io/zeta-chain/bitcoin-core-docker:27.2) - Wallet creation parameters are correctly implemented with
descriptors=falseas required - Implementation includes proper error handling for existing wallets
- All required parameters for bitcoin-core 27.2 wallet creation are present and correctly ordered
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify bitcoin-core version and configuration in docker-compose.yml
# Check bitcoin service configuration
rg -A 5 'bitcoin:' | rg -A 5 'image:|command:'
Length of output: 888
Script:
#!/bin/bash
# Let's verify the bitcoin configuration and wallet creation parameters in the codebase
# Check bitcoin configuration files
fd -e conf bitcoin
fd -e conf -x cat {} \;
# Check other bitcoin setup related files
rg -l "createwallet|bitcoin.*27" --type go
Length of output: 155
Script:
#!/bin/bash
# Let's examine the bitcoin setup implementation in detail
# Check the bitcoin setup implementation
rg -A 10 "createwallet" e2e/runner/setup_bitcoin.go
# Check for any bitcoin version references
rg "bitcoin.*27" e2e/runner/setup_bitcoin.go
# Check the entire setup_bitcoin.go file for context
cat e2e/runner/setup_bitcoin.go
Length of output: 4287
contrib/localnet/docker-compose.yml (1)
Line range hint 198-213: LGTM: Bitcoin-core upgrade changes are properly configured
The changes align well with the PR objectives:
- Updated to bitcoin-core v27.2 using the custom Docker image
- Added
-deprecatedrpc=create_bdbcommand to supportcreatewalletwithdescriptors=false
Let's verify the Docker image exists and contains the expected version:
✅ Verification successful
Bitcoin Core upgrade changes are properly configured
The verification confirms that:
- The Bitcoin Core image upgrade to v27.2 is properly integrated into the test environment
- The
-deprecatedrpc=create_bdbflag is correctly configured for backward compatibility - The setup is well-integrated with the e2e testing framework, including proper Bitcoin deposit/withdrawal test accounts and configurations
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the Docker image exists and contains bitcoin-core v27.2
# Check if the image exists in the registry
gh api \
-H "Accept: application/vnd.oci.image.index.v1+json" \
/v2/zeta-chain/bitcoin-core-docker/manifests/27.2 \
--jq '.schemaVersion'
# Note: We can't verify the actual bitcoin-core version inside the image
# as we don't have access to the running container
Length of output: 437
Script:
#!/bin/bash
# Let's try a different approach to verify the Docker image
# 1. Check if the image is referenced in other configuration files
# 2. Look for any documentation about the image
# Search for references to the Docker image
rg -l "ghcr.io/zeta-chain/bitcoin-core-docker" --type yaml
# Check for documentation about Bitcoin setup
fd "README|CONTRIBUTING|SETUP" --extension md --exec grep -l "bitcoin" {} \;
# Look for any scripts that might be using this image
fd ".*\.(sh|bash)" --exec grep -l "bitcoin-core-docker" {} \;
Length of output: 308
Script:
#!/bin/bash
# Let's check the documentation in the localnet directory for Bitcoin setup details
cat contrib/localnet/README.md
# Also check the e2e documentation as it might contain relevant information
cat e2e/README.md
# Look for any Bitcoin-related configuration or setup scripts in the localnet directory
fd . contrib/localnet --type f --exec grep -l "bitcoin" {} \;
Length of output: 6211
Script:
#!/bin/bash
# Let's check the orchestrator and zetacored scripts for Bitcoin setup details
cat contrib/localnet/scripts/start-zetacored.sh
cat contrib/localnet/orchestrator/start-zetae2e.sh
# Also check if there are any Bitcoin-related environment variables or configuration
rg "bitcoin" contrib/localnet/ --type sh -A 5 -B 5
Length of output: 33548
gartnera
changed the title
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
* chore: upgrade bitcoin-core to 27.2 * Upgrade to 28.0
Upgrade to bitcoin-core 28.0
This requires four things:
createwalletrpc with thedescriptorsargument set to false (it defaults to true in bitcoin-core 23+)-deprecatedrpc=create_bdbto allowcreatewalletwithdescriptors=false. This is required in bitcoin-core 26+.-deprecatedrpc=warningsto avoid the issue described here: getnetworkinfo core RPC introduced new field in json object. btcsuite/btcd#2224. Can be removed once btcd does a new release.We need to upgrade to bitcoin-core 28.0 for testnet4 support. Related to #2242
Summary by CodeRabbit
New Features
eth2,upgrade-host, andupgrade-orchestratorservices for enhanced functionality.bitcoinservice to support deprecated RPC.Improvements
bitcoinservice image for better performance.Bug Fixes