From 2d10bf257ffb177306b9cd10f7fc57f92d34b7e1 Mon Sep 17 00:00:00 2001 From: Bitflash Date: Fri, 31 Jul 2026 16:37:13 -0300 Subject: wallet: stop paying the recipient the transaction fee on a fee retry (#74) CreateTransaction kept the payee amount in nValue and then did int64 nValueOut = nValue; nValue += nFee; with both lines inside the retry loop. The first pass is fine. When the fee turns out to be too low the loop starts over, and on the second pass nValueOut is read from an nValue that already carries the fee -- so the recipient is sent the amount plus the fee, out of the sender's pocket, and it compounds on every further retry. Reachable, but it takes two things at once, which is why it has sat here: the user must have set a transaction fee in Options (with the default of zero, nValue += nFee adds nothing on the first pass and nothing is corrupted), and the transaction must be big enough for GetMinFee to demand more than that -- over 10 KB, so a wallet paying from many small inputs. Keep nValue meaning what the payee asked for and carry nValue + nFee in a separate nTotalValue, the way Bitcoin fixed it in 107d9e288. Verified by building the same transaction on both versions, with a synthetic wallet of 200 coins of 50, a configured fee of 0.05, and a request of 5,000 that needs 101 inputs and 28 KB -- enough for GetMinFee to demand 0.12 and force the retry: before payee gets 5,000.05 change 49.83 after payee gets 5,000.00 change 49.88 Two other fixes from the same era were considered and left out. Satoshi's CENT floor on the change output (496823249) exists to avoid a dust output that his fee rules then penalised; this fee schedule has no such rule, so adopting it would only donate sub-cent change to the miner. The 1-confirmation rule (a790fa46f) is a spending-policy change, not a bug fix, and deserves its own decision. --- src/main.cpp | 26 ++++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-) (limited to 'src/main.cpp') diff --git a/src/main.cpp b/src/main.cpp index c3fb33b..5610de9 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -3437,22 +3437,36 @@ bool CreateTransaction(CScript scriptPubKey, int64 nValue, CWalletTx& wtxNew, in wtxNew.vout.clear(); if (nValue < 0) return false; - int64 nValueOut = nValue; - nValue += nFee; + + // nValue is the amount the payee asked for and must stay that. + // + // This used to read "nValueOut = nValue; nValue += nFee;", and + // both lines are inside the retry loop. The first pass was + // fine. But when the fee turned out to be too low the loop + // starts over, and on the second pass nValueOut is read from + // an nValue that already had the fee added -- so the payee is + // sent the amount plus the fee, and it compounds on every + // further retry, with the sender covering it. + // + // The retry fires whenever GetMinFee exceeds the fee we + // guessed, which for this fee schedule means any transaction + // over 10 KB: a wallet paying from many small inputs. Bitcoin + // fixed this in 107d9e288. + int64 nTotalValue = nValue + nFee; // Choose coins to use set setCoins; - if (!SelectCoins(nValue, setCoins)) + if (!SelectCoins(nTotalValue, setCoins)) return false; int64 nValueIn = 0; foreach(CWalletTx* pcoin, setCoins) nValueIn += pcoin->GetCredit(); // Fill vout[0] to the payee - wtxNew.vout.push_back(CTxOut(nValueOut, scriptPubKey)); + wtxNew.vout.push_back(CTxOut(nValue, scriptPubKey)); // Fill vout[1] back to self with any change - if (nValueIn > nValue) + if (nValueIn > nTotalValue) { // Use the same key as one of the coins vector vchPubKey; @@ -3467,7 +3481,7 @@ bool CreateTransaction(CScript scriptPubKey, int64 nValue, CWalletTx& wtxNew, in // Fill vout[1] to ourself CScript scriptPubKey; scriptPubKey << vchPubKey << OP_CHECKSIG; - wtxNew.vout.push_back(CTxOut(nValueIn - nValue, scriptPubKey)); + wtxNew.vout.push_back(CTxOut(nValueIn - nTotalValue, scriptPubKey)); } // Fill vin -- cgit v1.2.3