-
Notifications
You must be signed in to change notification settings - Fork 78
cleanup: bump bitcoin and elements dependencies #138
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
Conversation
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.
Pull request overview
This pull request updates the bitcoin and elements dependencies from significantly outdated versions (bitcoin 0.28 → 0.32.8 and elements 0.19.1 → 0.26.1). The changes involve extensive API migration work across the entire codebase to accommodate breaking changes in these libraries.
Changes:
- Updated dependency versions in Cargo.toml and Cargo.lock with updated transitive dependencies
- Added compatibility traits (TxidCompat, BlockSizeCompat, SegwitDetection) to abstract differences between bitcoin and elements APIs
- Migrated from deprecated hash/address APIs to new pattern-based APIs (e.g.,
from_hex→parse,to_hex→to_string) - Updated script type detection methods (e.g.,
is_v0_p2wsh→is_p2wshfor Bitcoin, added SegwitDetection trait for cross-library compatibility) - Changed value access patterns (direct field access →
to_sat()method calls for Bitcoin) - Updated block hash initialization from
default()toall_zeros() - Removed
is_provably_unspendable()in favor ofis_op_return()for spendability checks - Added Testnet4 network support throughout the codebase
Reviewed changes
Copilot reviewed 18 out of 20 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| Cargo.toml / Cargo.lock | Bumped bitcoin from 0.28 to 0.32.8 and elements from 0.19.1 to 0.26.1 with updated dependencies |
| src/chain.rs | Added compatibility traits (TxidCompat, BlockSizeCompat), updated network magic handling, Testnet4 support, Hrp parsing for Liquid |
| src/util/transaction.rs | Migrated from from_hex to parse, changed is_coin_base() to is_coinbase(), replaced is_provably_unspendable() with is_op_return() |
| src/util/script.rs | Added SegwitDetection trait to unify segwit script detection across Bitcoin/Elements, added ScriptBuf support |
| src/util/mod.rs | Updated hex encoding from to_hex() to hex::encode(), exported SegwitDetection trait |
| src/util/fees.rs | Updated value extraction using to_sat(), weight to WU conversion |
| src/util/block.rs | Changed BlockHash::default() to BlockHash::all_zeros() |
| src/rest.rs | Extensive hash/address API migrations, updated address validation logic for Testnet4, removed "provably_unspendable" script type |
| src/new_index/*.rs | Updated transaction ID access via TxidCompat trait, value extraction with to_sat() |
| src/elements/*.rs | Updated Elements-specific APIs, asset ID parsing, pegin/pegout data handling |
| src/electrum/client.rs | Updated hash construction using from_raw_hash |
| src/daemon.rs | Migrated hash serialization to to_string(), updated parsing to use parse() |
| src/bin/tx-fingerprint-stats.rs | Updated API usage for new bitcoin library version |
Comments suppressed due to low confidence (1)
src/rest.rs:391
- The "provably_unspendable" script type has been removed from the script type detection logic. This means outputs that were previously labeled as "provably_unspendable" in the API will now be categorized differently (likely falling through to the "nonstandard" catch-all). This is a breaking change to the API response format that clients may depend on. Consider whether this script type classification should be retained for backward compatibility.
} else if script.is_op_return() {
"op_return"
} else if script.is_p2pk() {
"p2pk"
} else if script.is_p2pkh() {
"p2pkh"
} else if script.is_p2sh() {
"p2sh"
} else if script.segwit_is_p2wpkh() {
"v0_p2wpkh"
} else if script.segwit_is_p2wsh() {
"v0_p2wsh"
} else if script.segwit_is_p2tr() {
"v1_p2tr"
} else if is_anchor(script) {
"anchor"
} else if is_bare_multisig(script) {
"multisig"
} else {
"unknown"
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
mononaut
left a comment
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.
Tested ACK @ [f8302d7]
builds & runs successfully, and I confirmed there are no regressions in any of the REST API endpoints or electrum RPC methods.
bitcoin and elements versions were falling behind pretty badly, so let's bump them to the latest versions.