Skip to content

Commit

Permalink
Merge #1448: bump(deps): upgrade rust bitcoin to 0.32.0, `miniscr…
Browse files Browse the repository at this point in the history
…ipt` to `0.12.0` and others

1120081 chore(wallet): rm dup code (志宇)
2a45640 deps(bdk): bump `bitcoin` to `0.32.0`, miniscript to `12.0.0` (Leonardo Lima)

Pull request description:

  fixes #1422
  <!-- You can erase any parts of this template not applicable to your Pull Request. -->

  ### Description

  This PR focuses on upgrading the `rust-bitcoin` and `miniscript` versions, to `0.32.0` and `0.12.0`, respectively. It also bumps the versions of other BDK ecosystem crates that also rely on both `rust-bitcoin` and `miniscript`, being:

  - electrum-client bitcoindevkit/rust-electrum-client#133
  - esplora-client bitcoindevkit/rust-esplora-client#85
  - hwi bitcoindevkit/rust-hwi#99

  <ins>I structured the PR in multiple commits, with closed scope, being one for each BDK crate being upgraded, and one for each kind of fix and upgrade required, it seems like a lot of commits (**that should be squashed before merging**), but I think it'll make it easier during review phase.</ins>

  In summary I can already mention some of the main changes:
  - using `compute_txid()` instead of deprecated `txid()`
  - using `minimal_non_dust()` instead of `dust_value()`
  - using the renamed `signature` and `sighash_type` fields
  - using proper `sighash::P2wpkhError`,  `sighash::TaprootError` instead of older `sighash::Error`
  - conversion from `Network` to new expected `NetworkKind` #1465
  - conversion from the new `Weight` type to current expected `usize` #1466
  - using `.into()` to convert from AbsLockTime and `RelLockTime` to `absolute::LockTime` and `relative::LockTime`
  - using Message::from_digest() instead of relying on deprecated `ThirtyTwoByteHash` trait.
  - updating the miniscript policy and dsl to proper expect and handle new `Threshold` type, instead of the previous two parameters.

  <!-- Describe the purpose of this PR, what's being adding and/or fixed -->

  ### Notes to the reviewers
  <ins>Again, I structured the PR in multiple commits, with closed scope, being one for each BDK crate being upgraded, and one for each kind of fix and upgrade required, it seems like a lot of commits (**that should be squashed before merging**), but I think it'll make it easier during review phase.</ins>

  It should definitely be carefully reviewed, especially the last commits for the wallet crate scope, the ones with the semantic `fix(wallet)`.

  I would also appreciate if @tcharding reviewed it, as he gave a try in the past (#1400 ), and I relied on some of it for the  policy part of it, other rust-bitcoin maintainers reviews are a definitely welcome 😁

  <!-- In this section you can include notes directed to the reviewers, like explaining why some parts
  of the PR were done in a specific way -->

  ### Changelog notice
  > // TODO(@oleonardolima): Do another pass and double check the changes
  - Use `compute_txid()` instead of deprecated `txid()`
  - Use `minimal_non_dust()` instead of `dust_value()`
  - Use `signature` and `sighash_type` fields, instead of previous `sig` and `hash_type`
  - Use `sighash::P2wpkhError`,  and `sighash::TaprootError` instead of older `sighash::Error`
  - Converts from `Network` to `NetworkKind`, where expected
  - Converts from `Weight` type to current used `usize`
  - Use `.into()` to convert from `AbsLockTime` and `RelLockTime` to `absolute::LockTime` and `relative::LockTime`
  - Remove use of  deprecated `ThirtyTwoByteHash` trait, use `Message::from_digest()`
  - Update the miniscript policy and dsl macros to proper expect and handle new `Threshold` type, instead of the previous two parameters.

  <!-- Notice the release manager should include in the release tag message changelog -->
  <!-- See https://keepachangelog.com/en/1.0.0/ for examples -->

  ### Checklists

  #### All Submissions:

  * [x] I've signed all my commits
  * [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)
  * [x] I ran `cargo fmt` and `cargo clippy` before committing

  #### New Features:

  * [ ] I've added tests for the new feature
  * [ ] I've added docs for the new feature

  #### Bugfixes:

  * [ ] This pull request breaks the existing API
  * [ ] I've added tests to reproduce the issue which are now passing
  * [ ] I'm linking the issue being fixed by this PR

ACKs for top commit:
  notmandatory:
    ACK 1120081

Tree-SHA512: ba1ab64603b41014d3f0866d846167f77d31959ca6f1d9c3181d5e543964f5d772e05651d63935ba7bbffeba41a66868d27de4c32129739b9ca50f3bbaf9f2a1
  • Loading branch information
notmandatory committed Jun 13, 2024
2 parents 473ef97 + 1120081 commit 3b040a7
Show file tree
Hide file tree
Showing 41 changed files with 430 additions and 305 deletions.
10 changes: 5 additions & 5 deletions .github/workflows/cont_integration.yml
Original file line number Diff line number Diff line change
Expand Up @@ -58,15 +58,15 @@ jobs:
- name: Check bdk_chain
working-directory: ./crates/chain
# TODO "--target thumbv6m-none-eabi" should work but currently does not
run: cargo check --no-default-features --features bitcoin/no-std,miniscript/no-std,hashbrown
run: cargo check --no-default-features --features miniscript/no-std,hashbrown
- name: Check bdk wallet
working-directory: ./crates/wallet
# TODO "--target thumbv6m-none-eabi" should work but currently does not
run: cargo check --no-default-features --features bitcoin/no-std,miniscript/no-std,bdk_chain/hashbrown
run: cargo check --no-default-features --features miniscript/no-std,bdk_chain/hashbrown
- name: Check esplora
working-directory: ./crates/esplora
# TODO "--target thumbv6m-none-eabi" should work but currently does not
run: cargo check --no-default-features --features bitcoin/no-std,miniscript/no-std,bdk_chain/hashbrown
run: cargo check --no-default-features --features miniscript/no-std,bdk_chain/hashbrown

check-wasm:
name: Check WASM
Expand All @@ -92,10 +92,10 @@ jobs:
uses: Swatinem/[email protected]
- name: Check bdk wallet
working-directory: ./crates/wallet
run: cargo check --target wasm32-unknown-unknown --no-default-features --features bitcoin/no-std,miniscript/no-std,bdk_chain/hashbrown,dev-getrandom-wasm
run: cargo check --target wasm32-unknown-unknown --no-default-features --features miniscript/no-std,bdk_chain/hashbrown,dev-getrandom-wasm
- name: Check esplora
working-directory: ./crates/esplora
run: cargo check --target wasm32-unknown-unknown --no-default-features --features bitcoin/no-std,miniscript/no-std,bdk_chain/hashbrown,async
run: cargo check --target wasm32-unknown-unknown --no-default-features --features miniscript/no-std,bdk_chain/hashbrown,async

fmt:
name: Rust fmt
Expand Down
5 changes: 2 additions & 3 deletions crates/bitcoind_rpc/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,8 @@ readme = "README.md"
# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]
# For no-std, remember to enable the bitcoin/no-std feature
bitcoin = { version = "0.31", default-features = false }
bitcoincore-rpc = { version = "0.18" }
bitcoin = { version = "0.32.0", default-features = false }
bitcoincore-rpc = { version = "0.19.0" }
bdk_chain = { path = "../chain", version = "0.15", default-features = false }

[dev-dependencies]
Expand Down
16 changes: 8 additions & 8 deletions crates/bitcoind_rpc/tests/test_emitter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ fn test_into_tx_graph() -> anyhow::Result<()> {
.graph
.txs
.iter()
.map(|tx| tx.txid())
.map(|tx| tx.compute_txid())
.collect::<BTreeSet<Txid>>(),
exp_txids,
"changeset should have the 3 mempool transactions",
Expand Down Expand Up @@ -440,7 +440,7 @@ fn mempool_avoids_re_emission() -> anyhow::Result<()> {
let emitted_txids = emitter
.mempool()?
.into_iter()
.map(|(tx, _)| tx.txid())
.map(|(tx, _)| tx.compute_txid())
.collect::<BTreeSet<Txid>>();
assert_eq!(
emitted_txids, exp_txids,
Expand Down Expand Up @@ -509,7 +509,7 @@ fn mempool_re_emits_if_tx_introduction_height_not_reached() -> anyhow::Result<()
emitter
.mempool()?
.into_iter()
.map(|(tx, _)| tx.txid())
.map(|(tx, _)| tx.compute_txid())
.collect::<BTreeSet<_>>(),
tx_introductions.iter().map(|&(_, txid)| txid).collect(),
"first mempool emission should include all txs",
Expand All @@ -518,7 +518,7 @@ fn mempool_re_emits_if_tx_introduction_height_not_reached() -> anyhow::Result<()
emitter
.mempool()?
.into_iter()
.map(|(tx, _)| tx.txid())
.map(|(tx, _)| tx.compute_txid())
.collect::<BTreeSet<_>>(),
tx_introductions.iter().map(|&(_, txid)| txid).collect(),
"second mempool emission should still include all txs",
Expand All @@ -538,7 +538,7 @@ fn mempool_re_emits_if_tx_introduction_height_not_reached() -> anyhow::Result<()
let emitted_txids = emitter
.mempool()?
.into_iter()
.map(|(tx, _)| tx.txid())
.map(|(tx, _)| tx.compute_txid())
.collect::<BTreeSet<_>>();
assert_eq!(
emitted_txids, exp_txids,
Expand Down Expand Up @@ -596,7 +596,7 @@ fn mempool_during_reorg() -> anyhow::Result<()> {
emitter
.mempool()?
.into_iter()
.map(|(tx, _)| tx.txid())
.map(|(tx, _)| tx.compute_txid())
.collect::<BTreeSet<_>>(),
env.rpc_client()
.get_raw_mempool()?
Expand Down Expand Up @@ -633,7 +633,7 @@ fn mempool_during_reorg() -> anyhow::Result<()> {
let mempool = emitter
.mempool()?
.into_iter()
.map(|(tx, _)| tx.txid())
.map(|(tx, _)| tx.compute_txid())
.collect::<BTreeSet<_>>();
let exp_mempool = tx_introductions
.iter()
Expand All @@ -648,7 +648,7 @@ fn mempool_during_reorg() -> anyhow::Result<()> {
let mempool = emitter
.mempool()?
.into_iter()
.map(|(tx, _)| tx.txid())
.map(|(tx, _)| tx.compute_txid())
.collect::<BTreeSet<_>>();
let exp_mempool = tx_introductions
.iter()
Expand Down
5 changes: 2 additions & 3 deletions crates/chain/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,12 @@ readme = "README.md"
# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]
# For no-std, remember to enable the bitcoin/no-std feature
bitcoin = { version = "0.31.0", default-features = false }
bitcoin = { version = "0.32.0", default-features = false }
serde_crate = { package = "serde", version = "1", optional = true, features = ["derive", "rc"] }

# Use hashbrown as a feature flag to have HashSet and HashMap from it.
hashbrown = { version = "0.9.1", optional = true, features = ["serde"] }
miniscript = { version = "11.0.0", optional = true, default-features = false }
miniscript = { version = "12.0.0", optional = true, default-features = false }

[dev-dependencies]
rand = "0.8"
Expand Down
2 changes: 1 addition & 1 deletion crates/chain/src/descriptor_ext.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ impl DescriptorExt for Descriptor<DescriptorPublicKey> {
self.at_derivation_index(0)
.expect("descriptor can't have hardened derivation")
.script_pubkey()
.dust_value()
.minimal_non_dust()
.to_sat()
}

Expand Down
6 changes: 3 additions & 3 deletions crates/chain/src/indexed_tx_graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ where
let mut graph = tx_graph::ChangeSet::default();
for (tx, anchors) in txs {
if self.index.is_tx_relevant(tx) {
let txid = tx.txid();
let txid = tx.compute_txid();
graph.append(self.graph.insert_tx(tx.clone()));
for anchor in anchors {
graph.append(self.graph.insert_anchor(txid, anchor));
Expand Down Expand Up @@ -234,7 +234,7 @@ where
for (tx_pos, tx) in block.txdata.iter().enumerate() {
changeset.indexer.append(self.index.index_tx(tx));
if self.index.is_tx_relevant(tx) {
let txid = tx.txid();
let txid = tx.compute_txid();
let anchor = A::from_block_position(block, block_id, tx_pos);
changeset.graph.append(self.graph.insert_tx(tx.clone()));
changeset
Expand All @@ -261,7 +261,7 @@ where
let mut graph = tx_graph::ChangeSet::default();
for (tx_pos, tx) in block.txdata.iter().enumerate() {
let anchor = A::from_block_position(&block, block_id, tx_pos);
graph.append(self.graph.insert_anchor(tx.txid(), anchor));
graph.append(self.graph.insert_anchor(tx.compute_txid(), anchor));
graph.append(self.graph.insert_tx(tx.clone()));
}
let indexer = self.index_tx_graph_changeset(&graph);
Expand Down
2 changes: 1 addition & 1 deletion crates/chain/src/keychain/txout_index.rs
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@ impl<K: Clone + Ord + Debug> Indexer for KeychainTxOutIndex<K> {
fn index_tx(&mut self, tx: &bitcoin::Transaction) -> Self::ChangeSet {
let mut changeset = super::ChangeSet::<K>::default();
for (op, txout) in tx.output.iter().enumerate() {
changeset.append(self.index_txout(OutPoint::new(tx.txid(), op as u32), txout));
changeset.append(self.index_txout(OutPoint::new(tx.compute_txid(), op as u32), txout));
}
changeset
}
Expand Down
2 changes: 1 addition & 1 deletion crates/chain/src/spk_txout_index.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ impl<I: Clone + Ord> SpkTxOutIndex<I> {
/// 2. When getting new data from the chain, you usually scan it before incorporating it into your chain state.
pub fn scan(&mut self, tx: &Transaction) -> BTreeSet<I> {
let mut scanned_indices = BTreeSet::new();
let txid = tx.txid();
let txid = tx.compute_txid();
for (i, txout) in tx.output.iter().enumerate() {
let op = OutPoint::new(txid, i as u32);
if let Some(spk_i) = self.scan_txout(op, txout) {
Expand Down
6 changes: 3 additions & 3 deletions crates/chain/src/tx_data_traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ use alloc::vec::Vec;
/// let mut graph_a = TxGraph::<BlockId>::default();
/// let _ = graph_a.insert_tx(tx.clone());
/// graph_a.insert_anchor(
/// tx.txid(),
/// tx.compute_txid(),
/// BlockId {
/// height: 1,
/// hash: Hash::hash("first".as_bytes()),
Expand All @@ -58,7 +58,7 @@ use alloc::vec::Vec;
/// let mut graph_b = TxGraph::<ConfirmationHeightAnchor>::default();
/// let _ = graph_b.insert_tx(tx.clone());
/// graph_b.insert_anchor(
/// tx.txid(),
/// tx.compute_txid(),
/// ConfirmationHeightAnchor {
/// anchor_block: BlockId {
/// height: 2,
Expand All @@ -76,7 +76,7 @@ use alloc::vec::Vec;
/// let mut graph_c = TxGraph::<ConfirmationTimeHeightAnchor>::default();
/// let _ = graph_c.insert_tx(tx.clone());
/// graph_c.insert_anchor(
/// tx.txid(),
/// tx.compute_txid(),
/// ConfirmationTimeHeightAnchor {
/// anchor_block: BlockId {
/// height: 2,
Expand Down
23 changes: 12 additions & 11 deletions crates/chain/src/tx_graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -445,7 +445,7 @@ impl<A> TxGraph<A> {
&'g self,
tx: &'g Transaction,
) -> impl Iterator<Item = (usize, Txid)> + '_ {
let txid = tx.txid();
let txid = tx.compute_txid();
tx.input
.iter()
.enumerate()
Expand Down Expand Up @@ -516,9 +516,10 @@ impl<A: Clone + Ord> TxGraph<A> {
pub fn insert_tx<T: Into<Arc<Transaction>>>(&mut self, tx: T) -> ChangeSet<A> {
let tx = tx.into();
let mut update = Self::default();
update
.txs
.insert(tx.txid(), (TxNodeInternal::Whole(tx), BTreeSet::new(), 0));
update.txs.insert(
tx.compute_txid(),
(TxNodeInternal::Whole(tx), BTreeSet::new(), 0),
);
self.apply_update(update)
}

Expand All @@ -533,7 +534,7 @@ impl<A: Clone + Ord> TxGraph<A> {
) -> ChangeSet<A> {
let mut changeset = ChangeSet::<A>::default();
for (tx, seen_at) in txs {
changeset.append(self.insert_seen_at(tx.txid(), seen_at));
changeset.append(self.insert_seen_at(tx.compute_txid(), seen_at));
changeset.append(self.insert_tx(tx));
}
changeset
Expand Down Expand Up @@ -642,7 +643,7 @@ impl<A: Clone + Ord> TxGraph<A> {
pub fn apply_changeset(&mut self, changeset: ChangeSet<A>) {
for wrapped_tx in changeset.txs {
let tx = wrapped_tx.as_ref();
let txid = tx.txid();
let txid = tx.compute_txid();

tx.input
.iter()
Expand All @@ -660,7 +661,7 @@ impl<A: Clone + Ord> TxGraph<A> {
}
Some((TxNodeInternal::Whole(tx), _, _)) => {
debug_assert_eq!(
tx.as_ref().txid(),
tx.as_ref().compute_txid(),
txid,
"tx should produce txid that is same as key"
);
Expand Down Expand Up @@ -825,7 +826,7 @@ impl<A: Anchor> TxGraph<A> {
// resulting array will also include `tx`
let unconfirmed_ancestor_txs =
TxAncestors::new_include_root(self, tx.clone(), |_, ancestor_tx: Arc<Transaction>| {
let tx_node = self.get_tx_node(ancestor_tx.as_ref().txid())?;
let tx_node = self.get_tx_node(ancestor_tx.as_ref().compute_txid())?;
// We're filtering the ancestors to keep only the unconfirmed ones (= no anchors in
// the best chain)
for block in tx_node.anchors {
Expand All @@ -843,7 +844,7 @@ impl<A: Anchor> TxGraph<A> {
// and our unconf descendants' last seen.
let unconfirmed_descendants_txs = TxDescendants::new_include_root(
self,
tx.as_ref().txid(),
tx.as_ref().compute_txid(),
|_, descendant_txid: Txid| {
let tx_node = self.get_tx_node(descendant_txid)?;
// We're filtering the ancestors to keep only the unconfirmed ones (= no anchors in
Expand Down Expand Up @@ -884,7 +885,7 @@ impl<A: Anchor> TxGraph<A> {
return Ok(None);
}
if conflicting_tx.last_seen_unconfirmed == *last_seen
&& conflicting_tx.as_ref().txid() > tx.as_ref().txid()
&& conflicting_tx.as_ref().compute_txid() > tx.as_ref().compute_txid()
{
// Conflicting tx has priority if txid of conflicting tx > txid of original tx
return Ok(None);
Expand Down Expand Up @@ -1255,7 +1256,7 @@ impl<A> ChangeSet<A> {
tx.output
.iter()
.enumerate()
.map(move |(vout, txout)| (OutPoint::new(tx.txid(), vout as _), txout))
.map(move |(vout, txout)| (OutPoint::new(tx.compute_txid(), vout as _), txout))
})
.chain(self.txouts.iter().map(|(op, txout)| (*op, txout)))
}
Expand Down
6 changes: 3 additions & 3 deletions crates/chain/tests/common/tx_template.rs
Original file line number Diff line number Diff line change
Expand Up @@ -125,14 +125,14 @@ pub fn init_graph<'a, A: Anchor + Clone + 'a>(
.collect(),
};

tx_ids.insert(tx_tmp.tx_name, tx.txid());
tx_ids.insert(tx_tmp.tx_name, tx.compute_txid());
spk_index.scan(&tx);
let _ = graph.insert_tx(tx.clone());
for anchor in tx_tmp.anchors.iter() {
let _ = graph.insert_anchor(tx.txid(), anchor.clone());
let _ = graph.insert_anchor(tx.compute_txid(), anchor.clone());
}
if let Some(seen_at) = tx_tmp.last_seen {
let _ = graph.insert_seen_at(tx.txid(), seen_at);
let _ = graph.insert_seen_at(tx.compute_txid(), seen_at);
}
}
(graph, spk_index, tx_ids)
Expand Down
Loading

0 comments on commit 3b040a7

Please sign in to comment.