Don't scan outputs which are dust, track dust change as operating costs

Fixes #299.

coins/bitcoin/src/wallet/send.rs

@@ -26,14 +26,14 @@ use crate::{
 
 #[rustfmt::skip]
 // https://github.com/bitcoin/bitcoin/blob/306ccd4927a2efe325c8d84be1bdb79edeb29b04/src/policy/policy.h#L27
-const MAX_STANDARD_TX_WEIGHT: u64 = 400_000;
+pub const MAX_STANDARD_TX_WEIGHT: u64 = 400_000;
 
 #[rustfmt::skip]
 // https://github.com/bitcoin/bitcoin/blob/306ccd4927a2efe325c8d84be1bdb79edeb29b04/src/policy/policy.cpp#L26-L63
 // As the above notes, a lower amount may not be considered dust if contained in a SegWit output
 // This doesn't bother with delineation due to how marginal these values are, and because it isn't
 // worth the complexity to implement differentation
-const DUST: u64 = 546;
+pub const DUST: u64 = 546;
 
 #[rustfmt::skip]
 // The constant is from:
@@ -44,7 +44,7 @@ const DUST: u64 = 546;
 // https://github.com/bitcoin/bitcoin/blob/296735f7638749906243c9e203df7bd024493806/src/net_processing.cpp#L5721-L5732
 // And then the fee itself is fee per thousand units, not fee per unit
 // https://github.com/bitcoin/bitcoin/blob/306ccd4927a2efe325c8d84be1bdb79edeb29b04/src/policy/feerate.cpp#L23-L37
-const MIN_FEE_PER_KILO_VSIZE: u64 = 1000;
+pub const MIN_FEE_PER_KILO_VSIZE: u64 = 1000;
 
 #[derive(Clone, PartialEq, Eq, Debug, Error)]
 pub enum TransactionError {

processor/src/multisigs/mod.rs

@@ -745,9 +745,8 @@ impl<D: Db, N: Network> MultisigManager<D, N> {
           res.push((key, id, tx, eventuality));
         }
 
-        // TODO: If the TX is None, restore its inputs to the scheduler
-        // Otherwise, if the TX had a change output, dropping its inputs would burn funds
-        // Are there exceptional cases upon rotation?
+        // TODO: If the TX is None, restore its inputs to the scheduler for efficiency's sake
+        // If this TODO is removed, also reduce the operating costs
       }
       res
     };

processor/src/multisigs/scanner.rs

@@ -553,7 +553,9 @@ impl<N: Network, D: Db> Scanner<N, D> {
           // TODO: These lines are the ones which will cause a really long-lived lock acquisiton
           for output in network.get_outputs(&block, key).await {
             assert_eq!(output.key(), key);
-            outputs.push(output);
+            if output.amount() >= N::DUST {
+              outputs.push(output);
+            }
           }
 
           for (id, (block_number, tx)) in network

processor/src/multisigs/scheduler.rs

@@ -435,6 +435,7 @@ impl<N: Network> Scheduler<N> {
     let mut remainder = diff - (per_payment * payments_len);
 
     for payment in payments.iter_mut() {
+      // TODO: This usage of saturating_sub is invalid as we *need* to subtract this value
       payment.amount = payment.amount.saturating_sub(per_payment + remainder);
       // Only subtract the remainder once
       remainder = 0;

processor/src/networks/bitcoin.rs

@@ -562,21 +562,35 @@ impl Network for Bitcoin {
         return Ok(PreparedSend {
           tx: None,
           post_fee_branches: drop_branches(&plan),
-          // We expected a change output of sum(inputs) - sum(outputs)
+          // This plan expects a change output valued at sum(inputs) - sum(outputs)
           // Since we can no longer create this change output, it becomes an operating cost
-          operating_costs: operating_costs +
-            plan.inputs.iter().map(|input| input.amount()).sum::<u64>() -
-            plan.payments.iter().map(|payment| payment.amount).sum::<u64>(),
+          // TODO: Look at input restoration to reduce this operating cost
+          operating_costs: operating_costs + plan.expected_change(),
         });
       }
     };
 
-    let AmortizeFeeRes { post_fee_branches, operating_costs } =
+    let AmortizeFeeRes { post_fee_branches, mut operating_costs } =
       amortize_fee(&mut plan, operating_costs, tx_fee);
 
     let signable = signable(&plan, Some(tx_fee)).unwrap();
 
-    // TODO: If the change output was dropped by Bitcoin, increase operating costs
+    // Now that we've amortized the fee (which may raise the expected change value), grab it again
+    // Then, subtract the TX fee
+    //
+    // The first `expected_change` call gets the theoretically expected change from the theoretical
+    // Plan object, and accordingly doesn't subtract the fee (expecting the payments to bare it)
+    // This call wants the actual value, and since Plan is unaware of the fee, has to manually
+    // adjust
+    let on_chain_expected_change = plan.expected_change() - tx_fee;
+    // If the change value is less than the dust threshold, it becomes an operating cost
+    // This may be slightly inaccurate as dropping payments may reduce the fee, raising the change
+    // above dust
+    // That's fine since it'd have to be in a very precarious state AND then it's over-eager in
+    // tabulating costs
+    if on_chain_expected_change < Self::DUST {
+      operating_costs += on_chain_expected_change;
+    }
 
     let plan_binding_input = *plan.inputs[0].output.outpoint();
     let outputs = signable.outputs().to_vec();

processor/src/networks/monero.rs

@@ -527,22 +527,38 @@ impl Network for Monero {
         return Ok(PreparedSend {
           tx: None,
           post_fee_branches: drop_branches(&plan),
-          // We expected a change output of sum(inputs) - sum(outputs)
+          // This plan expects a change output valued at sum(inputs) - sum(outputs)
           // Since we can no longer create this change output, it becomes an operating cost
-          operating_costs: operating_costs +
-            plan.inputs.iter().map(|input| input.amount()).sum::<u64>() -
-            plan.payments.iter().map(|payment| payment.amount).sum::<u64>(),
+          // TODO: Look at input restoration to reduce this operating cost
+          operating_costs: operating_costs + plan.expected_change(),
         });
       }
     };
 
-    let AmortizeFeeRes { post_fee_branches, operating_costs } =
+    let AmortizeFeeRes { post_fee_branches, mut operating_costs } =
       amortize_fee(&mut plan, operating_costs, tx_fee);
+    let plan_expected_change = plan.expected_change();
 
-    // TODO: If the change output was dropped by Monero, increase operating costs
+    let signable = signable(plan, Some(tx_fee))?.unwrap();
 
-    let signable =
-      SignableTransaction { transcript, actual: signable(plan, Some(tx_fee))?.unwrap() };
+    // Now that we've amortized the fee (which may raise the expected change value), grab it again
+    // Then, subtract the TX fee
+    //
+    // The first `expected_change` call gets the theoretically expected change from the theoretical
+    // Plan object, and accordingly doesn't subtract the fee (expecting the payments to bare it)
+    // This call wants the actual value, and since Plan is unaware of the fee, has to manually
+    // adjust
+    let on_chain_expected_change = plan_expected_change - tx_fee;
+    // If the change value is less than the dust threshold, it becomes an operating cost
+    // This may be slightly inaccurate as dropping payments may reduce the fee, raising the change
+    // above dust
+    // That's fine since it'd have to be in a very precarious state AND then it's over-eager in
+    // tabulating costs
+    if on_chain_expected_change < Self::DUST {
+      operating_costs += on_chain_expected_change;
+    }
+
+    let signable = SignableTransaction { transcript, actual: signable };
     let eventuality = signable.actual.eventuality().unwrap();
     Ok(PreparedSend { tx: Some((signable, eventuality)), post_fee_branches, operating_costs })
   }

processor/src/plan.rs

@@ -132,6 +132,11 @@ impl<N: Network> Plan<N> {
     res
   }
 
+  pub fn expected_change(&self) -> u64 {
+    self.inputs.iter().map(|input| input.amount()).sum::<u64>() -
+      self.payments.iter().map(|payment| payment.amount).sum::<u64>()
+  }
+
   pub fn write<W: io::Write>(&self, writer: &mut W) -> io::Result<()> {
     writer.write_all(self.key.to_bytes().as_ref())?;
 

processor/src/tests/literal/mod.rs

@@ -1,6 +1,17 @@
 #[cfg(feature = "bitcoin")]
 mod bitcoin {
-  use crate::networks::Bitcoin;
+  use crate::networks::{Network, Bitcoin};
+
+  #[test]
+  fn test_dust_constant() {
+    struct IsTrue<const V: bool>;
+    trait True {}
+    impl True for IsTrue<true> {}
+    fn check<T: True>() {
+      core::hint::black_box(());
+    }
+    check::<IsTrue<{ Bitcoin::DUST >= bitcoin_serai::wallet::DUST }>>();
+  }
 
   async fn bitcoin() -> Bitcoin {
     let bitcoin = Bitcoin::new("http://serai:seraidex@127.0.0.1:18443".to_string()).await;