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
No status

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions