Repository: rust-bitcoin/rust-bitcoin (rust)

Issue:
`Script::dust_value` method name disagrees with the documentation
Dust is a value that is not economically spendable and can not be broadcasted. The method is called `dust_value` which suggest it returns a value that is too small (and always returning 0 would be a valid implementation) but the documentation says "Returns the **minimum value* an output with this script should have in order **to be broadcastable** on today’s Bitcoin network." (emphasis mine) which implies it's a non-dust value.

Since writing a method that always returns 0 is useless I assume the documentation is right but the method is named incorrectly. I was not able to verify the implementation yet.

Pull request description:
Fix: TxOut::minimal_non_dust and Script::dust_value
Fixes #2192 

TxOut::minimal_non_dust has 3 problems.

1. There is an invisible dependency on Bitcoin Core's default minrelaytxfee value. It has been made explicit.
2. There is an off by one error. The dust limit comparison uses < and therefore `+ 1` was not needed. It has been fixed.
3. It was not returning 0 amount for OP_RETURN outputs.

Script::dust_value has 2 problems.

1. The dust amount depends on minrelaytxfee which is configurable in Bitcoin Core. This method was not configurable.
2. The division operation was done before multiplying the byte amount, which can cause small differences when using uncommon scripts and minrelaytxfee values.

Patch:
diff --git a/bitcoin/src/blockdata/script/borrowed.rs b/bitcoin/src/blockdata/script/borrowed.rs
index 117e34d0..828a23b9 100644
--- a/bitcoin/src/blockdata/script/borrowed.rs
+++ b/bitcoin/src/blockdata/script/borrowed.rs
@@ -10,6 +10,7 @@
 use secp256k1::{Secp256k1, Verification};
 
 use super::PushBytes;
+use crate::blockdata::fee_rate::FeeRate;
 use crate::blockdata::opcodes::all::*;
 use crate::blockdata::opcodes::{self, Opcode};
 use crate::blockdata::script::witness_version::WitnessVersion;
@@ -390,21 +391,51 @@ pub fn p2wpkh_script_code(&self) -> Option<ScriptBuf> {
 
     /// Returns the minimum value an output with this script should have in order to be
     /// broadcastable on today's Bitcoin network.
-    pub fn dust_value(&self) -> crate::Amount {
+    ///
+    /// Dust depends on the -dustrelayfee value of the Bitcoin Core node you are broadcasting to.
+    /// This function uses the default value of 0.00003 BTC/kB (3 sat/vByte).
+    ///
+    /// To use a custom value, use [`minimal_non_dust_custom`].
+    ///
+    /// [`minimal_non_dust_custom`]: Script::minimal_non_dust_custom
+    pub fn minimal_non_dust(&self) -> crate::Amount {
+        self.minimal_non_dust_inner(DUST_RELAY_TX_FEE.into())
+    }
+
+    /// Returns the minimum value an output with this script should have in order to be
+    /// broadcastable on today's Bitcoin network.
+    ///
+    /// Dust depends on the -dustrelayfee value of the Bitcoin Core node you are broadcasting to.
+    /// This function lets you set the fee rate used in dust calculation.
+    ///
+    /// The current default value in Bitcoin Core (as of v26) is 3 sat/vByte.
+    ///
+    /// To use the default Bitcoin Core value, use [`minimal_non_dust`].
+    ///
+    /// [`minimal_non_dust`]: Script::minimal_non_dust
+    pub fn minimal_non_dust_custom(&self, dust_relay_fee: FeeRate) -> crate::Amount {
+        self.minimal_non_dust_inner(dust_relay_fee.to_sat_per_kwu() * 4)
+    }
+
+    fn minimal_non_dust_inner(&self, dust_relay_fee: u64) -> crate::Amount {
         // This must never be lower than Bitcoin Core's GetDustThreshold() (as of v0.21) as it may
         // otherwise allow users to create transactions which likely can never be broadcast/confirmed.
-        let sats = DUST_RELAY_TX_FEE as u64 / 1000 * // The default dust relay fee is 3000 satoshi/kB (i.e. 3 sat/vByte)
-        if self.is_op_return() {
-            0
-        } else if self.is_witness_program() {
-            32 + 4 + 1 + (107 / 4) + 4 + // The spend cost copied from Core
-            8 + // The serialized size of the TxOut's amount field
-            self.consensus_encode(&mut sink()).expect("sinks don't error") as u64 // The serialized size of this script_pubkey
-        } else {
-            32 + 4 + 1 + 107 + 4 + // The spend cost copied from Core
-            8 + // The serialized size of the TxOut's amount field
-            self.consensus_encode(&mut sink()).expect("sinks don't error") as u64 // The serialized size of this script_pubkey
-        };
+        let sats = dust_relay_fee.checked_mul(
+                if self.is_op_return() {
+                    0
+                } else if self.is_witness_program() {
+                    32 + 4 + 1 + (107 / 4) + 4 + // The spend cost copied from Core
+                    8 + // The serialized size of the TxOut's amount field
+                    self.consensus_encode(&mut sink()).expect("sinks don't error") as u64 // The serialized size of this script_pubkey
+                } else {
+                    32 + 4 + 1 + 107 + 4 + // The spend cost copied from Core
+                    8 + // The serialized size of the TxOut's amount field
+                    self.consensus_encode(&mut sink()).expect("sinks don't error") as u64 // The serialized size of this script_pubkey
+                }
+            ).expect("dust_relay_fee or script length should not be absurdly large") /
+            1000; // divide by 1000 like in Core to get value as it cancels out DEFAULT_MIN_RELAY_TX_FEE
+            // Note: We ensure the division happens at the end, since Core performs the division at the end.
+            //       This will make sure none of the implicit floor operations mess with the value.
 
         crate::Amount::from_sat(sats)
     }
diff --git a/bitcoin/src/blockdata/transaction.rs b/bitcoin/src/blockdata/transaction.rs
index 20146c1e..db64f14d 100644
--- a/bitcoin/src/blockdata/transaction.rs
+++ b/bitcoin/src/blockdata/transaction.rs
@@ -18,6 +18,7 @@
 use internals::write_err;
 
 use super::Weight;
+use crate::blockdata::fee_rate::FeeRate;
 use crate::blockdata::locktime::absolute::{self, Height, Time};
 use crate::blockdata::locktime::relative;
 use crate::blockdata::script::{Script, ScriptBuf};
@@ -534,19 +535,33 @@ pub fn size(&self) -> usize { size_from_script_pubkey(&self.script_pubkey) }
     /// Creates a `TxOut` with given script and the smallest possible `value` that is **not** dust
     /// per current Core policy.
     ///
-    /// The current dust fee rate is 3 sat/vB.
+    /// Dust depends on the -dustrelayfee value of the Bitcoin Core node you are broadcasting to.
+    /// This function uses the default value of 0.00003 BTC/kB (3 sat/vByte).
+    ///
+    /// To use a custom value, use [`minimal_non_dust_custom`].
+    ///
+    /// [`minimal_non_dust_custom`]: TxOut::minimal_non_dust_custom
     pub fn minimal_non_dust(script_pubkey: ScriptBuf) -> Self {
-        let len = size_from_script_pubkey(&script_pubkey);
-        let len = len
-            + if script_pubkey.is_witness_program() {
-                32 + 4 + 1 + (107 / 4) + 4
-            } else {
-                32 + 4 + 1 + 107 + 4
-            };
-        let dust_amount = (len as u64) * 3;
+        TxOut {
+            value: script_pubkey.minimal_non_dust(),
+            script_pubkey,
+        }
+    }
 
+    /// Creates a `TxOut` with given script and the smallest possible `value` that is **not** dust
+    /// per current Core policy.
+    ///
+    /// Dust depends on the -dustrelayfee value of the Bitcoin Core node you are broadcasting to.
+    /// This function lets you set the fee rate used in dust calculation.
+    ///
+    /// The current default value in Bitcoin Core (as of v26) is 3 sat/vByte.
+    ///
+    /// To use the default Bitcoin Core value, use [`minimal_non_dust`].
+    ///
+    /// [`minimal_non_dust`]: TxOut::minimal_non_dust
+    pub fn minimal_non_dust_custom(script_pubkey: ScriptBuf, dust_relay_fee: FeeRate) -> Self {
         TxOut {
-            value: Amount::from_sat(dust_amount + 1), // minimal non-dust amount is one higher than dust amount
+            value: script_pubkey.minimal_non_dust_custom(dust_relay_fee),
             script_pubkey,
         }
     }
diff --git a/bitcoin/src/crypto/sighash.rs b/bitcoin/src/crypto/sighash.rs
index 42e9d5b1..6fb2ddbe 100644
--- a/bitcoin/src/crypto/sighash.rs
+++ b/bitcoin/src/crypto/sighash.rs
@@ -1187,34 +1187,25 @@ fn taproot_cache<T: Borrow<TxOut>>(&mut self, prevouts: &[T]) -> &TaprootCache {
 }
 
 impl<R: BorrowMut<Transaction>> SighashCache<R> {
-    /// Allows modification of witnesses.
+    /// When the `SighashCache` is initialized with a mutable reference to a transaction instead of
+    /// a regular reference, this method is available to allow modification to the witnesses.
     ///
-    /// As a lint against accidental changes to the transaction that would invalidate the cache and
-    /// signatures, `SighashCache` borrows the Transaction so that modifying it is not possible
-    /// without hacks with `UnsafeCell` (which is hopefully a strong indication that something is
-    /// wrong). However modifying witnesses never invalidates the cache and is actually useful - one
-    /// usually wants to put the signature generated for an input into the witness of that input.
-    ///
-    /// This method allows doing exactly that if the transaction is owned by the `SighashCache` or
-    /// borrowed mutably.
-    ///
-    /// # Examples
-    ///
-    /// ```compile_fail
-    /// let mut sighasher = SighashCache::new(&mut tx_to_sign);
-    /// let sighash = sighasher.p2wpkh_signature_hash(input_index, &utxo.script_pubkey, amount, sighash_type)?;
-    ///
-    /// let signature = {
-    ///     // Sign the sighash using secp256k1
-    /// };
-    ///
-    /// *sighasher.witness_mut(input_index).unwrap() = Witness::p2wpkh(&signature, &pk);
+    /// This allows in-line signing such as
     /// ```
+    /// use bitcoin::{absolute, transaction, Amount, Transaction, Script};
+    /// use bitcoin::sighash::{EcdsaSighashType, SighashCache};
     ///
-    /// For full signing code see the [`segwit v0`] and [`taproot`] signing examples.
+    /// let mut tx_to_sign = Transaction { version: transaction::Version::TWO, lock_time: absolute::LockTime::ZERO, input: Vec::new(), output: Vec::new() };
+    /// let input_count = tx_to_sign.input.len();
     ///
-    /// [`segwit v0`]: <https://github.com/rust-bitcoin/rust-bitcoin/blob/master/bitcoin/examples/sign-tx-segwit-v0.rs>
-    /// [`taproot`]: <https://github.com/rust-bitcoin/rust-bitcoin/blob/master/bitcoin/examples/sign-tx-taproot.rs>
+    /// let mut sig_hasher = SighashCache::new(&mut tx_to_sign);
+    /// for inp in 0..input_count {
+    ///     let prevout_script = Script::new();
+    ///     let _sighash = sig_hasher.p2wpkh_signature_hash(inp, prevout_script, Amount::ONE_SAT, EcdsaSighashType::All);
+    ///     // ... sign the sighash
+    ///     sig_hasher.witness_mut(inp).unwrap().push(&Vec::new());
+    /// }
+    /// ```
     pub fn witness_mut(&mut self, input_index: usize) -> Option<&mut Witness> {
         self.tx.borrow_mut().input.get_mut(input_index).map(|i| &mut i.witness)
     }


Test patch:
diff --git a/bitcoin/src/blockdata/script/tests.rs b/bitcoin/src/blockdata/script/tests.rs
index c0a4c10b..e0a60f4e 100644
--- a/bitcoin/src/blockdata/script/tests.rs
+++ b/bitcoin/src/blockdata/script/tests.rs
@@ -6,6 +6,7 @@
 use hex_lit::hex;
 
 use super::*;
+use crate::FeeRate;
 use crate::blockdata::opcodes;
 use crate::consensus::encode::{deserialize, serialize};
 use crate::crypto::key::{PubkeyHash, PublicKey, WPubkeyHash, XOnlyPublicKey};
@@ -649,7 +650,8 @@ fn defult_dust_value_tests() {
     // well-known scriptPubKey types.
     let script_p2wpkh = Builder::new().push_int(0).push_slice([42; 20]).into_script();
     assert!(script_p2wpkh.is_p2wpkh());
-    assert_eq!(script_p2wpkh.dust_value(), crate::Amount::from_sat(294));
+    assert_eq!(script_p2wpkh.minimal_non_dust(), crate::Amount::from_sat(294));
+    assert_eq!(script_p2wpkh.minimal_non_dust_custom(FeeRate::from_sat_per_vb_unchecked(6)), crate::Amount::from_sat(588));
 
     let script_p2pkh = Builder::new()
         .push_opcode(OP_DUP)
@@ -659,7 +661,8 @@ fn defult_dust_value_tests() {
         .push_opcode(OP_CHECKSIG)
         .into_script();
     assert!(script_p2pkh.is_p2pkh());
-    assert_eq!(script_p2pkh.dust_value(), crate::Amount::from_sat(546));
+    assert_eq!(script_p2pkh.minimal_non_dust(), crate::Amount::from_sat(546));
+    assert_eq!(script_p2pkh.minimal_non_dust_custom(FeeRate::from_sat_per_vb_unchecked(6)), crate::Amount::from_sat(1092));
 }
 
 #[test]