Skip to content

Use or delete the tap dependency #314

Description

@vadim-anfv

tap is declared as a dependency in Cargo.toml, but is never used:

$ cargo +nightly udeps --all-features --all-targets
...
unused dependencies:
`bdk-cli v4.0.0 (/home/v/src/bitcoindevkit/bdk-cli)`
└─── dependencies
     └─── "tap"
...

Rather than dropping it, I'd suggest putting it to use. Two places where it would make the code read better:

CompileCommand::execute in src/handlers/descriptor.rs:

// now
let legacy_policy: Miniscript<String, Legacy> = policy
    .compile()
    .map_err(|e| Error::Generic(e.to_string()))?;
let segwit_policy: Miniscript<String, Segwitv0> = policy
    .compile()
    .map_err(|e| Error::Generic(e.to_string()))?;
let taproot_policy: Miniscript<String, Tap> = policy
    .compile()
    .map_err(|e| Error::Generic(e.to_string()))?;

let descriptor = match self.script_type.as_str() {
    "sh" => Descriptor::new_sh(legacy_policy),
    "wsh" => Descriptor::new_wsh(segwit_policy),
    "sh-wsh" => Descriptor::new_sh_wsh(segwit_policy),
    // ...
}?;

// with tap::Pipe
let descriptor = match self.script_type.as_str() {
    "sh" => policy.compile::<Legacy>()?.pipe(Descriptor::new_sh),
    "wsh" => policy.compile::<Segwitv0>()?.pipe(Descriptor::new_wsh),
    "sh-wsh" => policy.compile::<Segwitv0>()?.pipe(Descriptor::new_sh_wsh),
    // ...
}?;

There is also a bug in that spot: the policy is compiled for all three contexts up front, so a policy that only fits the requested type can still be rejected by one of the other two. I don't want to expand this issue with the details, but the rewrite above fixes it as a side effect. If we decide to remove tap instead, I'll open a separate issue for the bug.

BumpFeeCommand::execute in src/handlers/offline.rs:

// now
let mut tx_builder = wallet.build_fee_bump(self.txid)?;
tx_builder.fee_rate(fee_rate);

if let Some(address) = &self.shrink_address {
    tx_builder.drain_to(address.script_pubkey());
}
if self.offline_signer {
    tx_builder.add_global_xpubs();
}

let psbt = tx_builder.finish()?;

// with tap::Tap
let psbt = wallet
    .build_fee_bump(self.txid)?
    .tap_mut(|b| { b.fee_rate(fee_rate); })
    .tap_mut(|b| if let Some(address) = &self.shrink_address {
        b.drain_to(address.script_pubkey());
    })
    .tap_mut(|b| if self.offline_signer { b.add_global_xpubs(); })
    .finish()?;

If that doesn't seem worth a dependency, then the other way is to remove the tap from dependencies.
@tvpeter your call - happy to open a PR either way.

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

Status
Done

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions