From 7b6181ecdb6a07c5b020a9ebcd57b9e0283e30c0 Mon Sep 17 00:00:00 2001
From: Luke Parker <lukeparker5132@gmail.com>
Date: Fri, 20 Oct 2023 08:11:42 -0400
Subject: [PATCH] Remove Plan ID non-determinism leading Monero to have
 distinct TX fees

Monero would select decoys with a new RNG seed, which may have used more bytes,
increasing the fee.

There's a few comments here.

1) Non-determinism wasn't removed via distinguishing the edits. It was done by
   removing part of the transcript. A TODO exists to improve this.
2) Distinct TX fees is a test failure, not an issue in prod *unless* the distinct
   fee is greater. So long as the distinct fee is lesser, it's fine.
3) Removing outputs is expected to only decrease fees.
---
 processor/src/networks/mod.rs | 18 +++++++++++++++---
 processor/src/plan.rs         |  6 ++++++
 2 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/processor/src/networks/mod.rs b/processor/src/networks/mod.rs
index 1b2a85e0..7d1f8b98 100644
--- a/processor/src/networks/mod.rs
+++ b/processor/src/networks/mod.rs
@@ -358,7 +358,7 @@ pub trait Network: 'static + Send + Sync + Clone + PartialEq + Eq + Debug {
       });
     };
 
-    // Amortize a fee over the plan's payments
+    // Amortize the fee over the plan's payments
     let (post_fee_branches, mut operating_costs) = (|| {
       // If we're creating a change output, letting us recoup coins, amortize the operating costs
       // as well
@@ -417,9 +417,21 @@ pub trait Network: 'static + Send + Sync + Clone + PartialEq + Eq + Debug {
       }
 
       // Drop payments now worth 0
-      plan.payments = plan.payments.drain(..).filter(|payment| payment.amount != 0).collect();
+      let plan_id = plan.id();
+      plan.payments = plan
+        .payments
+        .drain(..)
+        .filter(|payment| {
+          if payment.amount != 0 {
+            true
+          } else {
+            log::debug!("dropping dust payment from plan {}", hex::encode(&plan_id));
+            false
+          }
+        })
+        .collect();
 
-      // Sanity check the fee wa successfully amortized
+      // Sanity check the fee was successfully amortized
       let new_outputs = plan.payments.iter().map(|payment| payment.amount).sum::<u64>();
       assert!((new_outputs + total_fee) <= original_outputs);
 
diff --git a/processor/src/plan.rs b/processor/src/plan.rs
index f85372e1..c009c852 100644
--- a/processor/src/plan.rs
+++ b/processor/src/plan.rs
@@ -113,10 +113,16 @@ impl<N: Network> Plan<N> {
       transcript.append_message(b"input", input.id());
     }
 
+    // Don't transcript the payments as these will change between the intended Plan and the actual
+    // Plan, once various fee logics have executed
+    // TODO: Distinguish IntendedPlan and ActualPlan, or make actual payments a distinct field,
+    // letting us transcript this
+    /*
     transcript.domain_separate(b"payments");
     for payment in &self.payments {
       payment.transcript(&mut transcript);
     }
+    */
 
     if let Some(change) = &self.change {
       transcript.append_message(b"change", change.to_string());