wallet: prevent backdating locktime too far

ishaanam · ishaanam · commit eb92a9a6b7b3 · 2023-07-10T11:04:22.000-04:00
diff --git a/src/wallet/feebumper.cpp b/src/wallet/feebumper.cpp
@@ -308,6 +308,10 @@ Result CreateRateBumpTransaction(CWallet& wallet, const uint256& txid, const CCo
     // Write back transaction
     mtx = CMutableTransaction(*txr.tx);
 
+    // Set the locktime of the new transaction to the same 
+    // locktime as the transaction being replaced
+    mtx.nLockTime = wtx.tx->nLockTime;
+
     return Result::OK;
 }
 
diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp
@@ -764,7 +764,8 @@ static bool IsCurrentForAntiFeeSniping(interfaces::Chain& chain, const uint256&
  * current chain tip unless we are not synced with the current chain
  */
 static void DiscourageFeeSniping(CMutableTransaction& tx, FastRandomContext& rng_fast,
-                                 interfaces::Chain& chain, const uint256& block_hash, int block_height)
+                                 interfaces::Chain& chain, const uint256& block_hash, int block_height,
+                                 unsigned int min_locktime)
 {
     // All inputs must be added by now
     assert(!tx.vin.empty());
@@ -796,8 +797,11 @@ static void DiscourageFeeSniping(CMutableTransaction& tx, FastRandomContext& rng
         // e.g. high-latency mix networks and some CoinJoin implementations, have
         // better privacy.
         if (rng_fast.randrange(10) == 0) {
-            tx.nLockTime = std::max(0, int(tx.nLockTime) - int(rng_fast.randrange(100)));
+            // Ensure that the back-dated timelock does not go beneath the min_timelock
+            int locktime_range = std::min(100, int(block_height - min_locktime));
+            if (locktime_range > 0) tx.nLockTime = std::max(int(min_locktime), int(tx.nLockTime) - int(rng_fast.randrange(locktime_range)));
         }
+        assert(tx.nLockTime >= min_locktime);
     } else {
         // If our chain is lagging behind, we can't discourage fee sniping nor help
         // the privacy of high-latency transactions. To avoid leaking a potentially
@@ -1011,7 +1015,20 @@ static util::Result<CreatedTransactionResult> CreateTransactionInternal(
     for (const auto& coin : selected_coins) {
         txNew.vin.push_back(CTxIn(coin->outpoint, CScript(), nSequence));
     }
-    DiscourageFeeSniping(txNew, rng_fast, wallet.chain(), wallet.GetLastBlockHash(), wallet.GetLastBlockHeight());
+
+    // Figure out the highest locktime of all of the unconfirmed inputs
+    // so that the nLockTime for this transaction does not go beneath that.
+    // If the nLockTime goes beneath any of the previous transactions'
+    // locktimes, that can be a wallet fingerprint.
+    std::set<unsigned int> input_tx_locktimes{0}; // If there aren't any unconfirmed inputs, the minimum locktime is 0
+    for (const auto& coin: selected_coins) {
+        if (coin->depth == 0) {
+            const CWalletTx* coin_wtx{wallet.GetWalletTx(coin->outpoint.hash)};
+            if (coin_wtx) input_tx_locktimes.insert(coin_wtx->tx->nLockTime);
+        }
+    }
+
+    DiscourageFeeSniping(txNew, rng_fast, wallet.chain(), wallet.GetLastBlockHash(), wallet.GetLastBlockHeight(), /*min_locktime=*/*input_tx_locktimes.rbegin());
 
     // Calculate the transaction fee
     TxSize tx_sizes = CalculateMaximumSignedTxSize(CTransaction(txNew), &wallet, &coin_control);

Original file line number	Diff line number	Diff line change
`@@ -308,6 +308,10 @@ Result CreateRateBumpTransaction(CWallet& wallet, const uint256& txid, const CCo`
`308`	`308`	`// Write back transaction`
`309`	`309`	`mtx = CMutableTransaction(*txr.tx);`
`310`	`310`
	`311`	`+ // Set the locktime of the new transaction to the same`
	`312`	`+ // locktime as the transaction being replaced`
	`313`	`+ mtx.nLockTime = wtx.tx->nLockTime;`
	`314`	`+`
`311`	`315`	`return Result::OK;`
`312`	`316`	`}`
`313`	`317`