Skip to content

Commit

Permalink
Merge #1110: fix(esplora): use saturating_add in update_tx_graph()
Browse files Browse the repository at this point in the history
bf9a425 ci: fix MSRV build by pinning tokio-util to 0.7.8 (Steve Myers)
d35668e ci(esplora): fix wasm cargo check by setting workspace resolver to version 2 (Steve Myers)
31d52e1 ci: fix msrv dependency versions for rustls-webpki and zip (Steve Myers)
6a5c9d7 fix(esplora): use saturating_add in update_tx_graph() (Steve Myers)
4d1a9fd test(esplora): add async_ext and blocking_ext integration tests (Steve Myers)

Pull request description:

  ### Description

  This fixes overflow error when calling update_tx_graph() from update_tx_graph_without_keychain().

  ### Notes to the reviewers

  You can reproduce the error by reverting 66a2bf5.

  The tests could use some cleanup but get the job done for this PR.

  ### Changelog notice

  None

  ### 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

  #### Bugfixes:

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

Top commit has no ACKs.

Tree-SHA512: eace00e0c289a7ac161985c4b8e46ad660ec0d6777cd74027ef1f0ab245daea87e34258233281796efb60473cf4f18d2647c090a14c0f05f3dc8a1950ebe9dab
  • Loading branch information
notmandatory committed Sep 26, 2023
2 parents f95506b + bf9a425 commit 4a1b96d
Show file tree
Hide file tree
Showing 8 changed files with 252 additions and 7 deletions.
5 changes: 4 additions & 1 deletion .github/workflows/cont_integration.yml
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,13 @@ jobs:
cargo update -p rustls:0.21.7 --precise "0.21.1"
cargo update -p rustls:0.20.9 --precise "0.20.8"
cargo update -p tokio:1.32.0 --precise "1.29.1"
cargo update -p tokio-util --precise "0.7.8"
cargo update -p flate2:1.0.27 --precise "1.0.26"
cargo update -p reqwest --precise "0.11.18"
cargo update -p h2 --precise "0.3.20"
cargo update -p rustls-webpki --precise "0.100.1"
cargo update -p rustls-webpki:0.100.3 --precise "0.100.1"
cargo update -p rustls-webpki:0.101.6 --precise "0.101.1"
cargo update -p zip:0.6.6 --precise "0.6.2"
- name: Build
run: cargo build ${{ matrix.features }}
- name: Test
Expand Down
1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
[workspace]
resolver = "2"
members = [
"crates/bdk",
"crates/chain",
Expand Down
10 changes: 8 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -75,14 +75,20 @@ cargo update -p rustls:0.21.7 --precise "0.21.1"
cargo update -p rustls:0.20.9 --precise "0.20.8"
# tokio 1.30 has MSRV 1.63.0+
cargo update -p tokio:1.32.0 --precise "1.29.1"
# tokio-util 0.7.9 doesn't build with MSRV 1.57.0
cargo update -p tokio-util --precise "0.7.8"
# flate2 1.0.27 has MSRV 1.63.0+
cargo update -p flate2:1.0.27 --precise "1.0.26"
# reqwest 0.11.19 has MSRV 1.63.0+
cargo update -p reqwest --precise "0.11.18"
# h2 0.3.21 has MSRV 1.63.0+
cargo update -p h2 --precise "0.3.20"
# rustls-webpki has MSRV 1.60.0+
cargo update -p rustls-webpki --precise "0.100.1"
# rustls-webpki 0.100.2 has MSRV 1.60.0+
cargo update -p rustls-webpki:0.100.3 --precise "0.100.1"
# rustls-webpki 0.101.2 has MSRV 1.60.0+
cargo update -p rustls-webpki:0.101.6 --precise "0.101.1"
# zip 0.6.3 has MSRV 1.59.0+
cargo update -p zip:0.6.6 --precise "0.6.2"
```

## License
Expand Down
4 changes: 4 additions & 0 deletions crates/esplora/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,10 @@ futures = { version = "0.3.26", optional = true }
bitcoin = { version = "0.30.0", optional = true, default-features = false }
miniscript = { version = "10.0.0", optional = true, default-features = false }

[target.'cfg(not(target_arch = "wasm32"))'.dev-dependencies]
electrsd = { version= "0.25.0", features = ["bitcoind_25_0", "esplora_a33e97e1", "legacy"] }
tokio = { version = "1", features = ["rt", "rt-multi-thread", "macros"] }

[features]
default = ["std", "async-https", "blocking"]
std = ["bdk_chain/std"]
Expand Down
6 changes: 3 additions & 3 deletions crates/esplora/src/async_ext.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ use crate::{anchor_from_status, ASSUME_FINAL_DEPTH};
pub trait EsploraAsyncExt {
/// Prepare an [`LocalChain`] update with blocks fetched from Esplora.
///
/// * `prev_tip` is the previous tip of [`LocalChain::tip`].
/// * `get_heights` is the block heights that we are interested in fetching from Esplora.
/// * `local_tip` is the previous tip of [`LocalChain::tip`].
/// * `request_heights` is the block heights that we are interested in fetching from Esplora.
///
/// The result of this method can be applied to [`LocalChain::apply_update`].
///
Expand Down Expand Up @@ -261,7 +261,7 @@ impl EsploraAsyncExt for esplora_client::AsyncClient {
}
}

if last_index > last_active_index.map(|i| i + stop_gap as u32) {
if last_index > last_active_index.map(|i| i.saturating_add(stop_gap as u32)) {
break;
}
}
Expand Down
2 changes: 1 addition & 1 deletion crates/esplora/src/blocking_ext.rs
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@ impl EsploraExt for esplora_client::BlockingClient {
}
}

if last_index > last_active_index.map(|i| i + stop_gap as u32) {
if last_index > last_active_index.map(|i| i.saturating_add(stop_gap as u32)) {
break;
}
}
Expand Down
117 changes: 117 additions & 0 deletions crates/esplora/tests/async_ext.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
use bdk_esplora::EsploraAsyncExt;
use electrsd::bitcoind::bitcoincore_rpc::RpcApi;
use electrsd::bitcoind::{self, anyhow, BitcoinD};
use electrsd::{Conf, ElectrsD};
use esplora_client::{self, AsyncClient, Builder};
use std::str::FromStr;
use std::thread::sleep;
use std::time::Duration;

use bdk_chain::bitcoin::{Address, Amount, BlockHash, Txid};

struct TestEnv {
bitcoind: BitcoinD,
#[allow(dead_code)]
electrsd: ElectrsD,
client: AsyncClient,
}

impl TestEnv {
fn new() -> Result<Self, anyhow::Error> {
let bitcoind_exe =
bitcoind::downloaded_exe_path().expect("bitcoind version feature must be enabled");
let bitcoind = BitcoinD::new(bitcoind_exe).unwrap();

let mut electrs_conf = Conf::default();
electrs_conf.http_enabled = true;
let electrs_exe =
electrsd::downloaded_exe_path().expect("electrs version feature must be enabled");
let electrsd = ElectrsD::with_conf(electrs_exe, &bitcoind, &electrs_conf)?;

let base_url = format!("http://{}", &electrsd.esplora_url.clone().unwrap());
let client = Builder::new(base_url.as_str()).build_async()?;

Ok(Self {
bitcoind,
electrsd,
client,
})
}

fn mine_blocks(
&self,
count: usize,
address: Option<Address>,
) -> anyhow::Result<Vec<BlockHash>> {
let coinbase_address = match address {
Some(address) => address,
None => self
.bitcoind
.client
.get_new_address(None, None)?
.assume_checked(),
};
let block_hashes = self
.bitcoind
.client
.generate_to_address(count as _, &coinbase_address)?;
Ok(block_hashes)
}
}

#[tokio::test]
pub async fn test_update_tx_graph_without_keychain() -> anyhow::Result<()> {
let env = TestEnv::new()?;
let receive_address0 =
Address::from_str("bcrt1qc6fweuf4xjvz4x3gx3t9e0fh4hvqyu2qw4wvxm")?.assume_checked();
let receive_address1 =
Address::from_str("bcrt1qfjg5lv3dvc9az8patec8fjddrs4aqtauadnagr")?.assume_checked();

let misc_spks = [
receive_address0.script_pubkey(),
receive_address1.script_pubkey(),
];

let _block_hashes = env.mine_blocks(101, None)?;
let txid1 = env.bitcoind.client.send_to_address(
&receive_address1,
Amount::from_sat(10000),
None,
None,
None,
None,
Some(1),
None,
)?;
let txid2 = env.bitcoind.client.send_to_address(
&receive_address0,
Amount::from_sat(20000),
None,
None,
None,
None,
Some(1),
None,
)?;
let _block_hashes = env.mine_blocks(1, None)?;
while env.client.get_height().await.unwrap() < 102 {
sleep(Duration::from_millis(10))
}

let graph_update = env
.client
.scan_txs(
misc_spks.into_iter(),
vec![].into_iter(),
vec![].into_iter(),
1,
)
.await?;

let mut graph_update_txids: Vec<Txid> = graph_update.full_txs().map(|tx| tx.txid).collect();
graph_update_txids.sort();
let mut expected_txids = vec![txid1, txid2];
expected_txids.sort();
assert_eq!(graph_update_txids, expected_txids);
Ok(())
}
114 changes: 114 additions & 0 deletions crates/esplora/tests/blocking_ext.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
use bdk_esplora::EsploraExt;
use electrsd::bitcoind::bitcoincore_rpc::RpcApi;
use electrsd::bitcoind::{self, anyhow, BitcoinD};
use electrsd::{Conf, ElectrsD};
use esplora_client::{self, BlockingClient, Builder};
use std::str::FromStr;
use std::thread::sleep;
use std::time::Duration;

use bdk_chain::bitcoin::{Address, Amount, BlockHash, Txid};

struct TestEnv {
bitcoind: BitcoinD,
#[allow(dead_code)]
electrsd: ElectrsD,
client: BlockingClient,
}

impl TestEnv {
fn new() -> Result<Self, anyhow::Error> {
let bitcoind_exe =
bitcoind::downloaded_exe_path().expect("bitcoind version feature must be enabled");
let bitcoind = BitcoinD::new(bitcoind_exe).unwrap();

let mut electrs_conf = Conf::default();
electrs_conf.http_enabled = true;
let electrs_exe =
electrsd::downloaded_exe_path().expect("electrs version feature must be enabled");
let electrsd = ElectrsD::with_conf(electrs_exe, &bitcoind, &electrs_conf)?;

let base_url = format!("http://{}", &electrsd.esplora_url.clone().unwrap());
let client = Builder::new(base_url.as_str()).build_blocking()?;

Ok(Self {
bitcoind,
electrsd,
client,
})
}

fn mine_blocks(
&self,
count: usize,
address: Option<Address>,
) -> anyhow::Result<Vec<BlockHash>> {
let coinbase_address = match address {
Some(address) => address,
None => self
.bitcoind
.client
.get_new_address(None, None)?
.assume_checked(),
};
let block_hashes = self
.bitcoind
.client
.generate_to_address(count as _, &coinbase_address)?;
Ok(block_hashes)
}
}

#[test]
pub fn test_update_tx_graph_without_keychain() -> anyhow::Result<()> {
let env = TestEnv::new()?;
let receive_address0 =
Address::from_str("bcrt1qc6fweuf4xjvz4x3gx3t9e0fh4hvqyu2qw4wvxm")?.assume_checked();
let receive_address1 =
Address::from_str("bcrt1qfjg5lv3dvc9az8patec8fjddrs4aqtauadnagr")?.assume_checked();

let misc_spks = [
receive_address0.script_pubkey(),
receive_address1.script_pubkey(),
];

let _block_hashes = env.mine_blocks(101, None)?;
let txid1 = env.bitcoind.client.send_to_address(
&receive_address1,
Amount::from_sat(10000),
None,
None,
None,
None,
Some(1),
None,
)?;
let txid2 = env.bitcoind.client.send_to_address(
&receive_address0,
Amount::from_sat(20000),
None,
None,
None,
None,
Some(1),
None,
)?;
let _block_hashes = env.mine_blocks(1, None)?;
while env.client.get_height().unwrap() < 102 {
sleep(Duration::from_millis(10))
}

let graph_update = env.client.scan_txs(
misc_spks.into_iter(),
vec![].into_iter(),
vec![].into_iter(),
1,
)?;

let mut graph_update_txids: Vec<Txid> = graph_update.full_txs().map(|tx| tx.txid).collect();
graph_update_txids.sort();
let mut expected_txids = vec![txid1, txid2];
expected_txids.sort();
assert_eq!(graph_update_txids, expected_txids);
Ok(())
}

0 comments on commit 4a1b96d

Please sign in to comment.