Skip to content

Commit

Permalink
Merge #1782: fix electrum conftime_req filter for needs_block_height
Browse files Browse the repository at this point in the history
20100aa ci: downgrade all workflows to ubuntu to 20.04 (Steve Myers)
53fb49f ci(clippy): fix disallow ref to static mut (Steve Myers)
f2e1632 ci: pin msrv dep version for ureq (Steve Myers)
fa53884 ci: pin msrv dep version for rustls and hashbrown (Steve Myers)
6a5b418 ci(clippy): fix missing docs errors for rust 1.83 (Steve Myers)
341b869 ci(clippy): fix updated ptr_arg check for rust 1.83 (Steve Myers)
24ec31b ci(clippy): fix new stricter needless_lifetime errors for rust 1.83 (Steve Myers)
4aa8fba ci: update examples/rpcwallet to use electrds/bitcoind_22_1 (Steve Myers)
8d190ba fix electrum conftime_req filter for needs_block_height (Zoe Faltibà)

Pull request description:

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

  ### Description

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

  This PR fixes a bug introduced in 04994e4#diff-57bf66f87897e694c5ebfdfe0fd366774dda30b43eab93c1a0fdc802d0eb8c8dR171

  In that commit `block_times.get(height).is_none()` was converted to `block_times.contains_key(height)`, inverting the code logic.

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

ACKs for top commit:
  notmandatory:
    ACK 20100aa
  oleonardolima:
    utACK 20100aa
  nymius:
    ACK 20100aa

Tree-SHA512: 85b41b4f00273d0936172d7b845d28343b519cf698b4127dba85d45ab4030c69b6696f029bec38a58e3c2b3b17ff77620970d4c8fe20529980b13082299f5241
  • Loading branch information
notmandatory committed Jan 8, 2025
2 parents 8081006 + 20100aa commit 68c78e4
Show file tree
Hide file tree
Showing 14 changed files with 49 additions and 46 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/code_coverage.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ jobs:
Codecov:
name: Code Coverage
if: false # disabled
runs-on: ubuntu-latest
runs-on: ubuntu-20.04
env:
RUSTFLAGS: "-Cinstrument-coverage"
RUSTDOCFLAGS: "-Cinstrument-coverage"
Expand Down
11 changes: 6 additions & 5 deletions .github/workflows/cont_integration.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ jobs:

build-test:
name: Build and test
runs-on: ubuntu-latest
runs-on: ubuntu-20.04
strategy:
matrix:
rust:
Expand Down Expand Up @@ -64,8 +64,9 @@ jobs:
cargo update -p regex --precise "1.7.3"
cargo update -p security-framework-sys --precise "2.11.1"
cargo update -p url --precise "2.5.0"
cargo update -p rustls@0.23.18 --precise "0.23.17"
cargo update -p hashbrown@0.15.1 --precise "0.15.0"
cargo update -p rustls@0.23.20 --precise "0.23.19"
cargo update -p hashbrown@0.15.2 --precise "0.15.0"
cargo update -p ureq --precise "2.10.1"
- name: Build
run: cargo build --features bitcoin/std,miniscript/std,${{ matrix.features }} --no-default-features
- name: Clippy
Expand All @@ -76,7 +77,7 @@ jobs:

test-readme-examples:
name: Test README.md examples
runs-on: ubuntu-latest
runs-on: ubuntu-20.04
steps:
- name: checkout
uses: actions/checkout@v4
Expand Down Expand Up @@ -166,7 +167,7 @@ jobs:

fmt:
name: Rust fmt
runs-on: ubuntu-latest
runs-on: ubuntu-20.04
steps:
- name: Checkout
uses: actions/checkout@v4
Expand Down
8 changes: 5 additions & 3 deletions .github/workflows/nightly_docs.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ on:
jobs:
build_docs:
name: Build docs
runs-on: ubuntu-latest
runs-on: ubuntu-20.04
steps:
- name: Checkout sources
uses: actions/checkout@v4
Expand All @@ -28,7 +28,9 @@ jobs:
- name: Install Rust toolchain
uses: dtolnay/rust-toolchain@nightly
with:
components: clippy
components: clippy, rustfmt
- name: Cargo update
run: cargo update
- name: Build docs
run: cargo rustdoc --verbose --features=compiler,electrum,esplora,use-esplora-blocking,compact_filters,rpc,key-value-db,sqlite,all-keys,verify,hardware-signer -- --cfg docsrs -Dwarnings
- name: Upload artifact
Expand All @@ -41,7 +43,7 @@ jobs:
name: 'Publish docs'
if: github.ref == 'refs/heads/master'
needs: [build_docs]
runs-on: ubuntu-latest
runs-on: ubuntu-20.04
steps:
- name: Checkout `bitcoindevkit.org`
uses: actions/checkout@v4
Expand Down
2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ path = "examples/policy.rs"
[[example]]
name = "rpcwallet"
path = "examples/rpcwallet.rs"
required-features = ["keys-bip39", "key-value-db", "rpc", "electrsd/bitcoind_22_0"]
required-features = ["keys-bip39", "key-value-db", "rpc", "electrsd/bitcoind_22_1"]

[[example]]
name = "psbt_signer"
Expand Down
5 changes: 3 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,7 @@ cargo update -p home --precise "0.5.5"
cargo update -p regex --precise "1.7.3"
cargo update -p security-framework-sys --precise "2.11.1"
cargo update -p url --precise "2.5.0"
cargo update -p rustls@0.23.18 --precise "0.23.17"
cargo update -p hashbrown@0.15.1 --precise "0.15.0"
cargo update -p rustls@0.23.20 --precise "0.23.19"
cargo update -p hashbrown@0.15.2 --precise "0.15.0"
cargo update -p ureq --precise "2.10.1"
```
1 change: 0 additions & 1 deletion examples/compiler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ use bdk::{KeychainKind, Wallet};
/// can be derived from the policy.
///
/// This example demonstrates the interaction between a bdk wallet and miniscript policy.
fn main() -> Result<(), Box<dyn Error>> {
env_logger::init_from_env(
env_logger::Env::default().filter_or(env_logger::DEFAULT_FILTER_ENV, "info"),
Expand Down
1 change: 0 additions & 1 deletion examples/policy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ use bdk::wallet::signer::SignersContainer;
///
/// This example demos a Policy output for a 2of2 multisig between between 2 parties, where the wallet holds
/// one of the Extend Private key.
fn main() -> Result<(), Box<dyn Error>> {
env_logger::init_from_env(
env_logger::Env::default().filter_or(env_logger::DEFAULT_FILTER_ENV, "info"),
Expand Down
2 changes: 1 addition & 1 deletion src/blockchain/electrum.rs
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ impl WalletSync for ElectrumBlockchain {
let needs_block_height = conftime_req
.request()
.filter_map(|txid| txid_to_height.get(txid).cloned())
.filter(|height| block_times.contains_key(height))
.filter(|height| !block_times.contains_key(height))
.take(chunk_size)
.collect::<HashSet<u32>>();

Expand Down
43 changes: 21 additions & 22 deletions src/database/keyvalue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -405,12 +405,13 @@ impl BatchDatabase for Tree {
#[cfg(test)]
mod test {
use lazy_static::lazy_static;
use std::sync::atomic::{AtomicUsize, Ordering};
use std::sync::{Arc, Condvar, Mutex, Once};
use std::time::{SystemTime, UNIX_EPOCH};

use sled::{Db, Tree};

static mut COUNT: usize = 0;
static COUNT: AtomicUsize = AtomicUsize::new(0);

lazy_static! {
static ref DB: Arc<(Mutex<Option<Db>>, Condvar)> =
Expand All @@ -419,33 +420,31 @@ mod test {
}

fn get_tree() -> Tree {
unsafe {
let cloned = DB.clone();
let (mutex, cvar) = &*cloned;
let cloned = DB.clone();
let (mutex, cvar) = &*cloned;

INIT.call_once(|| {
let mut db = mutex.lock().unwrap();
INIT.call_once(|| {
let mut db = mutex.lock().unwrap();

let time = SystemTime::now().duration_since(UNIX_EPOCH).unwrap();
let mut dir = std::env::temp_dir();
dir.push(format!("mbw_{}", time.as_nanos()));
let time = SystemTime::now().duration_since(UNIX_EPOCH).unwrap();
let mut dir = std::env::temp_dir();
dir.push(format!("mbw_{}", time.as_nanos()));

*db = Some(sled::open(dir).unwrap());
cvar.notify_all();
});
*db = Some(sled::open(dir).unwrap());
cvar.notify_all();
});

let mut db = mutex.lock().unwrap();
while !db.is_some() {
db = cvar.wait(db).unwrap();
}
let mut db = mutex.lock().unwrap();
while !db.is_some() {
db = cvar.wait(db).unwrap();
}

COUNT += 1;
COUNT.fetch_add(1, Ordering::Relaxed);

db.as_ref()
.unwrap()
.open_tree(format!("tree_{}", COUNT))
.unwrap()
}
db.as_ref()
.unwrap()
.open_tree(format!("tree_{}", COUNT.load(Ordering::Relaxed)))
.unwrap()
}

#[test]
Expand Down
1 change: 1 addition & 0 deletions src/database/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,7 @@ pub(crate) trait DatabaseUtils: Database {
impl<T: Database> DatabaseUtils for T {}

#[cfg(test)]
#[allow(missing_docs)]
pub mod test {
use bitcoin::consensus::encode::deserialize;
use bitcoin::consensus::serialize;
Expand Down
4 changes: 1 addition & 3 deletions src/descriptor/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -134,9 +134,7 @@ impl IntoWalletDescriptor for (ExtendedDescriptor, KeyMap) {
network: Network,
}

impl<'s, 'd> miniscript::Translator<DescriptorPublicKey, String, DescriptorError>
for Translator<'s, 'd>
{
impl miniscript::Translator<DescriptorPublicKey, String, DescriptorError> for Translator<'_, '_> {
fn pk(&mut self, pk: &DescriptorPublicKey) -> Result<String, DescriptorError> {
let secp = &self.secp;

Expand Down
2 changes: 1 addition & 1 deletion src/keys/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -949,7 +949,7 @@ impl std::fmt::Display for KeyError {
impl std::error::Error for KeyError {}

#[cfg(test)]
pub mod test {
mod test {
use bitcoin::bip32;

use super::*;
Expand Down
7 changes: 5 additions & 2 deletions src/wallet/coin_selection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,8 +119,11 @@ use std::convert::TryInto;
/// overridden
#[cfg(not(test))]
pub type DefaultCoinSelectionAlgorithm = BranchAndBoundCoinSelection;

/// Default deterministic coin selection algorithm for testing used by [`TxBuilder`](super::tx_builder::TxBuilder) if not
/// overridden
#[cfg(test)]
pub type DefaultCoinSelectionAlgorithm = LargestFirstCoinSelection; // make the tests more predictable
pub type DefaultCoinSelectionAlgorithm = LargestFirstCoinSelection;

// Base weight of a Txin, not counting the weight needed for satisfying it.
// prev_txid (32 bytes) + prev_vout (4 bytes) + sequence (4 bytes)
Expand Down Expand Up @@ -868,7 +871,7 @@ mod test {
vec![utxo; utxos_number]
}

fn sum_random_utxos(mut rng: &mut StdRng, utxos: &mut Vec<WeightedUtxo>) -> u64 {
fn sum_random_utxos(mut rng: &mut StdRng, utxos: &mut [WeightedUtxo]) -> u64 {
let utxos_picked_len = rng.gen_range(2..utxos.len() / 2);
utxos.shuffle(&mut rng);
utxos[..utxos_picked_len]
Expand Down
6 changes: 3 additions & 3 deletions src/wallet/tx_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ impl std::default::Default for FeePolicy {
}
}

impl<'a, Cs: Clone, Ctx, D> Clone for TxBuilder<'a, D, Cs, Ctx> {
impl<Cs: Clone, Ctx, D> Clone for TxBuilder<'_, D, Cs, Ctx> {
fn clone(&self) -> Self {
TxBuilder {
wallet: self.wallet,
Expand Down Expand Up @@ -584,7 +584,7 @@ impl<'a, D: BatchDatabase, Cs: CoinSelectionAlgorithm<D>, Ctx: TxBuilderContext>
}
}

impl<'a, D: BatchDatabase, Cs: CoinSelectionAlgorithm<D>> TxBuilder<'a, D, Cs, CreateTx> {
impl<D: BatchDatabase, Cs: CoinSelectionAlgorithm<D>> TxBuilder<'_, D, Cs, CreateTx> {
/// Replace the recipients already added with a new list
pub fn set_recipients(&mut self, recipients: Vec<(ScriptBuf, u64)>) -> &mut Self {
self.params.recipients = recipients;
Expand Down Expand Up @@ -658,7 +658,7 @@ impl<'a, D: BatchDatabase, Cs: CoinSelectionAlgorithm<D>> TxBuilder<'a, D, Cs, C
}

// methods supported only by bump_fee
impl<'a, D: BatchDatabase> TxBuilder<'a, D, DefaultCoinSelectionAlgorithm, BumpFee> {
impl<D: BatchDatabase> TxBuilder<'_, D, DefaultCoinSelectionAlgorithm, BumpFee> {
/// Explicitly tells the wallet that it is allowed to reduce the amount of the output matching this
/// `script_pubkey` in order to bump the transaction fee. Without specifying this the wallet
/// will attempt to find a change output to shrink instead.
Expand Down

0 comments on commit 68c78e4

Please sign in to comment.