frost/bdk
3
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge bitcoindevkit/bdk#505: Using dust value from rust-bitcoin in `is_dust`

Browse Source 
5ac51dfe74c51c83fe270570d7ffc50888cf554f fix and test is_dust (James Taylor)
a0c140bb29ab37103e4a24d296ac6fcea91da69e add doc comment for IsDust trait (James Taylor)
bf5994b14ade1fc0c37d3e3cda6946c26ceea631 fixed fee in test, removed unnecessary comment (James Taylor)
ca682819b396c44c9139086b1537d32b212499a1 using dust value from rust-bitcoin (James Taylor)

Pull request description:

  ### Description

  This PR aims to fix #472 . We can retrieve the dust value for a given ``bitcoin::blockdata::script::Script``, so I adjusted the ``is_dust`` function within the ``IsDust`` trait to receive such a ``&Script``. Thus, the ``is_dust`` function can make the proper comparison.
  Let me know if you think that there could be a better interface than this.

  Furthermore, because this new ``is_dust`` function provides a tighter upper bound on Bitcoin Core's ``GetDustThreshold()``, it actually invalidated a test. In the test, the drain output for a transaction was no longer considered dust and no longer included in the fee. Instead, the drain output was kicked back to the sender, invalidating the asserts in line 3436, 3437 and 3441 in ``src/wallet/mod.rs``. I increased the ``FeeRate`` in the test just enough that the drain output would be small enough to considered dust again and included in the total fee.

  ### 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:
  afilini:
    ACK 5ac51dfe74c51c83fe270570d7ffc50888cf554f
  notmandatory:
    re ACK 5ac51dfe74c51c83fe270570d7ffc50888cf554f

Tree-SHA512: addf38fe065de581ddfcd3b4e6db92cd35d5bfa8cac78bd08c01f7a01292724a203ef59b09f3f5cd8e0fa0bb6d89efe72afda36efc11ded0424fc8105326af3f
This commit is contained in:
Steve Myers 2022-01-12 17:47:58 +01:00
commit b1346d4ccf
No known key found for this signature in database
GPG Key ID: 8105A46B22C2D051
3 changed files with 35 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
src/testutils/blockchain_tests.rs
View File

@ -843,7 +843,7 @@ macro_rules! bdk_blockchain_tests {
assert_eq!(wallet.get_balance().unwrap(), details.received, "incorrect received after send");
let mut builder = wallet.build_fee_bump(details.txid).unwrap();
builder.fee_rate(FeeRate::from_sat_per_vb(5.0));
builder.fee_rate(FeeRate::from_sat_per_vb(5.1));
let (mut new_psbt, new_details) = builder.finish().unwrap();
let finalized = wallet.sign(&mut new_psbt, Default::default()).unwrap();
assert!(finalized, "Cannot finalize transaction");

12
src/wallet/mod.rs
View File

@ -53,7 +53,7 @@ use address_validator::AddressValidator;
use coin_selection::DefaultCoinSelectionAlgorithm;
use signer::{SignOptions, Signer, SignerOrdering, SignersContainer};
use tx_builder::{BumpFee, CreateTx, FeePolicy, TxBuilder, TxParams};
use utils::{check_nlocktime, check_nsequence_rbf, After, Older, SecpCtx, DUST_LIMIT_SATOSHI};
use utils::{check_nlocktime, check_nsequence_rbf, After, Older, SecpCtx};
use crate::blockchain::{Blockchain, Progress};
use crate::database::memory::MemoryDatabase;
@ -601,7 +601,7 @@ where
let recipients = params.recipients.iter().map(|(r, v)| (r, *v));
for (index, (script_pubkey, value)) in recipients.enumerate() {
if value.is_dust() && !script_pubkey.is_provably_unspendable() {
if value.is_dust(script_pubkey) && !script_pubkey.is_provably_unspendable() {
return Err(Error::OutputBelowDustLimit(index));
}
@ -677,9 +677,9 @@ where
if tx.output.is_empty() {
if params.drain_to.is_some() {
if drain_val.is_dust() {
if drain_val.is_dust(&drain_output.script_pubkey) {
return Err(Error::InsufficientFunds {
needed: DUST_LIMIT_SATOSHI,
needed: drain_output.script_pubkey.dust_value().as_sat(),
available: drain_val,
});
}
@ -688,7 +688,7 @@ where
}
}
if drain_val.is_dust() {
if drain_val.is_dust(&drain_output.script_pubkey) {
fee_amount += drain_val;
} else {
drain_output.value = drain_val;
@ -3424,7 +3424,7 @@ pub(crate) mod test {
.unwrap();
let mut builder = wallet.build_fee_bump(txid).unwrap();
builder.fee_rate(FeeRate::from_sat_per_vb(140.0));
builder.fee_rate(FeeRate::from_sat_per_vb(141.0));
let (psbt, details) = builder.finish().unwrap();
assert_eq!(

38
src/wallet/utils.rs
View File

@ -9,13 +9,11 @@
// You may not use this file except in accordance with one or both of these
// licenses.
use bitcoin::blockdata::script::Script;
use bitcoin::secp256k1::{All, Secp256k1};
use miniscript::{MiniscriptKey, Satisfier, ToPublicKey};
// De-facto standard "dust limit" (even though it should change based on the output type)
pub const DUST_LIMIT_SATOSHI: u64 = 546;
// MSB of the nSequence. If set there's no consensus-constraint, so it must be disabled when
// spending using CSV in order to enforce CSV rules
pub(crate) const SEQUENCE_LOCKTIME_DISABLE_FLAG: u32 = 1 << 31;
@ -28,18 +26,19 @@ pub(crate) const SEQUENCE_LOCKTIME_MASK: u32 = 0x0000FFFF;
// Threshold for nLockTime to be considered a block-height-based timelock rather than time-based
pub(crate) const BLOCKS_TIMELOCK_THRESHOLD: u32 = 500000000;
/// Trait to check if a value is below the dust limit
/// Trait to check if a value is below the dust limit.
/// We are performing dust value calculation for a given script public key using rust-bitcoin to
/// keep it compatible with network dust rate
// we implement this trait to make sure we don't mess up the comparison with off-by-one like a <
// instead of a <= etc. The constant value for the dust limit is not public on purpose, to
// encourage the usage of this trait.
// instead of a <= etc.
pub trait IsDust {
/// Check whether or not a value is below dust limit
fn is_dust(&self) -> bool;
fn is_dust(&self, script: &Script) -> bool;
}
impl IsDust for u64 {
fn is_dust(&self) -> bool {
*self <= DUST_LIMIT_SATOSHI
fn is_dust(&self, script: &Script) -> bool {
*self < script.dust_value().as_sat()
}
}
@ -141,10 +140,29 @@ pub(crate) type SecpCtx = Secp256k1<All>;
#[cfg(test)]
mod test {
use super::{
check_nlocktime, check_nsequence_rbf, BLOCKS_TIMELOCK_THRESHOLD,
check_nlocktime, check_nsequence_rbf, IsDust, BLOCKS_TIMELOCK_THRESHOLD,
SEQUENCE_LOCKTIME_TYPE_FLAG,
};
use crate::bitcoin::Address;
use crate::types::FeeRate;
use std::str::FromStr;
#[test]
fn test_is_dust() {
let script_p2pkh = Address::from_str("1GNgwA8JfG7Kc8akJ8opdNWJUihqUztfPe")
.unwrap()
.script_pubkey();
assert!(script_p2pkh.is_p2pkh());
assert!(545.is_dust(&script_p2pkh));
assert!(!546.is_dust(&script_p2pkh));
let script_p2wpkh = Address::from_str("bc1qxlh2mnc0yqwas76gqq665qkggee5m98t8yskd8")
.unwrap()
.script_pubkey();
assert!(script_p2wpkh.is_v0_p2wpkh());
assert!(293.is_dust(&script_p2wpkh));
assert!(!294.is_dust(&script_p2wpkh));
}
#[test]
fn test_fee_from_btc_per_kb() {