payout: ask the wallet why a send failed instead of guessing from the balance

A payment that the pool could not fund was being recorded as a fault
against the payment, and 1440 faults erased the debt with the coin still
in the operator's wallet.

The classifier read GetBalance() after the failure and asked whether it
covered `amount + nTransactionFee + CENT`. Every term of that was wrong.
nTransactionFee is 0 on this pool -- it is only ever assigned from the
GUI preferences dialog, which a headless node does not have -- so the
entire reserve was one CENT. The fee it is meant to reserve for is
GetMinFee(fDiscount=true), which is a step function of the serialised
size: 0 below 10,000 bytes and (1 + bytes/1000) * CENT at or above it.
There is no required-fee value between 1 and 11 CENT, so a one-CENT
reserve is never the right number -- below the step nothing needs
reserving, at the step it is short by a factor of eleven.

Worse, the trigger correlates with the failure. A pool wallet close to
broke selects many small inputs BECAUSE it is close to broke, and about
56 signed P2PKH inputs is 10 KB, which a pool that has been paying out
accumulates as change. So: balance of amount + 0.05, the pre-send gate
passes, CreateTransaction builds a transaction over 10 KB, the fee
becomes 0.11, SelectCoins(amount + 0.11) fails, SendMoney returns false
-- and (amount + 0.05) < (amount + 0 + 0.01) is false, so "funded", so a
fault. The condition is stable, so it recurred every 60 s until the
budget ran out and ForfeitPayoutRecipient erased the miner's money.

Three changes.

The wallet now says what it did. CreateTransaction takes an optional
WalletSendResult out-parameter and sets WALLET_SEND_INSUFFICIENT_FUNDS
on the one return that means a funding shortfall -- SelectCoins failing
to cover nValue plus whatever fee that pass is working with, which on
the second and later passes is GetMinFee() of the transaction the first
pass actually built. SendMoney gains a four-argument overload returning
that plus the fee that was required. The three-argument SendMoney stays,
as a separate declaration rather than a default argument, because
gui.cpp declares it itself instead of including main.h; walletcmd.cpp
and the GUI are unchanged. These are wallet functions, not consensus.

ClassifyPayoutSendFailure() then classifies the answer rather than
guessing: locked or short of coin defers, anything else is a fault.
There is no reserve left to get wrong, and the size-dependent fee is
accounted for exactly, because the wallet computed it.

And the give-up branch is gone. 1440 failures is not a decidable state,
it is a count of how long we have been asking, and this pool reached it
by misclassifying a permanent funding shortfall. A payment that keeps
failing now stays in flight, keeps its ledger entry, keeps being
reported as owed in pool_status.json, and keeps retrying once a minute.
A debt that is never written off cannot be written off wrongly. The
remaining call to ForfeitPayoutRecipient is the one decidable case --
the ledger names a string that is not a valid address, so no transaction
can pay it -- which previously only dropped the in-flight record and
left the ledger and the round proof claiming the debt forever.

Tested against a real wallet and the real CreateTransaction, not against
injected values, because the honest question is whether a wallet holding
many small coins reports the state this code assumes. 60 coins of 1 BTF,
balance 60.00, amount 59.95: CreateTransaction returns false with
WALLET_SEND_INSUFFICIENT_FUNDS and a required fee of 0.11 -- and since
GetMinFee returns 0 below 10,000 bytes, a required fee of 11 CENT is
itself the measurement that the transaction crossed the step. The same
test asserts that the old reserve judged that state "funded", and that
the same wallet pays a 3-input transaction with a required fee of 0.00,
which is why no fixed reserve is right for both.

Property: a send that failed because the wallet could not fund it is
deferred, whatever the transaction's size, and no number of failures
erases a debt.

3 files changed, 454 insertions, 79 deletions

diff --git a/src/main.cpp b/src/main.cpp
index 252911b..7b3e555 100644
--- a/src/main.cpp
+++ b/src/main.cpp
@@ -4907,9 +4907,17 @@ bool SelectCoins(int64 nTargetValue, set<CWalletTx*>& setCoinsRet)
 
 
 
-bool CreateTransaction(CScript scriptPubKey, int64 nValue, CWalletTx& wtxNew, int64& nFeeRequiredRet)
+// presultRet, when given, says WHY this returned false: the caller cannot work
+// it out afterwards. "Coin selection came up short" and "the wallet could not
+// build the transaction" are indistinguishable from the outside -- both are
+// just `false` -- and the difference decides whether a pool defers a payment
+// or counts it against the payment's retry budget. See WalletSendResult.
+bool CreateTransaction(CScript scriptPubKey, int64 nValue, CWalletTx& wtxNew, int64& nFeeRequiredRet,
+                       WalletSendResult* presultRet)
 {
     nFeeRequiredRet = 0;
+    if (presultRet)
+        *presultRet = WALLET_SEND_FAILED;
     CRITICAL_BLOCK(cs_main)
     {
         // txdb must be opened before the mapWallet lock
@@ -4940,10 +4948,21 @@ bool CreateTransaction(CScript scriptPubKey, int64 nValue, CWalletTx& wtxNew, in
                 // fixed this in 107d9e288.
                 int64 nTotalValue = nValue + nFee;
 
-                // Choose coins to use
+                // Choose coins to use.
+                //
+                // Failing here is a funding shortfall and nothing else: there
+                // is not enough spendable coin for nValue plus the fee this
+                // pass is working with. On the second and later passes that
+                // fee is GetMinFee() of the transaction the first pass built,
+                // so this is also how a wallet that can afford the amount but
+                // not the 11 * CENT charged at 10,000 bytes reports itself.
                 set<CWalletTx*> setCoins;
                 if (!SelectCoins(nTotalValue, setCoins))
+                {
+                    if (presultRet)
+                        *presultRet = WALLET_SEND_INSUFFICIENT_FUNDS;
                     return false;
+                }
                 int64 nValueIn = 0;
                 foreach(CWalletTx* pcoin, setCoins)
                     nValueIn += pcoin->GetCredit();
@@ -5013,6 +5032,8 @@ bool CreateTransaction(CScript scriptPubKey, int64 nValue, CWalletTx& wtxNew, in
             }
         }
     }
+    if (presultRet)
+        *presultRet = WALLET_SEND_OK;
     return true;
 }
 
@@ -5075,13 +5096,24 @@ bool CommitTransactionSpent(const CWalletTx& wtxNew)
 
 
 
-bool SendMoney(CScript scriptPubKey, int64 nValue, CWalletTx& wtxNew)
+// outcomeRet reports what happened rather than only whether it worked. A
+// caller that has to decide what a failure MEANS -- the pool payout thread --
+// cannot reconstruct it from a balance reading afterwards, because the fee
+// that was actually required depends on the size of a transaction that was
+// never built. See WalletSendResult.
+bool SendMoney(CScript scriptPubKey, int64 nValue, CWalletTx& wtxNew,
+               SendMoneyOutcome& outcomeRet)
 {
+    outcomeRet.result       = WALLET_SEND_FAILED;
+    outcomeRet.nFeeRequired = 0;
     CRITICAL_BLOCK(cs_main)
     {
         int64 nFeeRequired;
-        if (!CreateTransaction(scriptPubKey, nValue, wtxNew, nFeeRequired))
+        WalletSendResult createResult = WALLET_SEND_FAILED;
+        if (!CreateTransaction(scriptPubKey, nValue, wtxNew, nFeeRequired, &createResult))
         {
+            outcomeRet.result       = createResult;
+            outcomeRet.nFeeRequired = nFeeRequired;
             string strError;
             if (nValue + nFeeRequired > GetBalance())
                 // Not an oversized transaction -- this branch is simply "the
@@ -5119,6 +5151,13 @@ bool SendMoney(CScript scriptPubKey, int64 nValue, CWalletTx& wtxNew)
         }
         wtxNew.RelayWalletTransaction();
     }
+    outcomeRet.result = WALLET_SEND_OK;
     MainFrameRepaint();
     return true;
 }
+
+bool SendMoney(CScript scriptPubKey, int64 nValue, CWalletTx& wtxNew)
+{
+    SendMoneyOutcome ignored;
+    return SendMoney(scriptPubKey, nValue, wtxNew, ignored);
+}
diff --git a/src/main.h b/src/main.h
index 8bb536b..382dd9c 100644
--- a/src/main.h
+++ b/src/main.h
@@ -342,9 +342,52 @@ struct WalletRecoveryAudit
     }
 };
 WalletRecoveryAudit GetWalletRecoveryAudit();
-bool CreateTransaction(CScript scriptPubKey, int64 nValue, CWalletTx& txNew, int64& nFeeRequiredRet);
+
+// Why a send did not go through.
+//
+// The pool's payout thread has to decide, on every failed SendMoney, whether
+// the failure was the wallet's condition (wait for it to change) or a fault
+// (something is wrong with this payment). It used to guess by re-reading
+// GetBalance() afterwards and allowing a fixed CENT of headroom for the fee.
+// That cannot work: the fee here is a step function of the SERIALISED SIZE of
+// a transaction that no longer exists, and the step is from 0 to 11 * CENT at
+// 10,000 bytes -- so a one-CENT allowance is either unnecessary or an order of
+// magnitude short, never right. A pool wallet close to broke selects many
+// small inputs BECAUSE it is close to broke, which is what pushes the
+// transaction over 10 KB, so the guess is wrong exactly when the money is
+// tight.
+//
+// The wallet already knows the answer; it just did not say. SelectCoins()
+// failing is "there is not enough spendable coin to cover the amount plus
+// whatever fee this size of transaction requires", and it is the only reason
+// CreateTransaction() can fail that is about funding rather than about the
+// wallet being broken.
+enum WalletSendResult
+{
+    WALLET_SEND_OK = 0,
+    WALLET_SEND_INSUFFICIENT_FUNDS, // coin selection could not cover amount+fee
+    WALLET_SEND_FAILED              // anything else: could not build, sign or accept
+};
+
+// What the wallet did with a send request. nFeeRequired is the fee the
+// transaction turned out to need, which for a failed send is what coin
+// selection was asked for on top of the amount and could not find -- 0 if it
+// never got that far.
+struct SendMoneyOutcome
+{
+    WalletSendResult result       = WALLET_SEND_OK;
+    int64            nFeeRequired = 0;
+};
+
+bool CreateTransaction(CScript scriptPubKey, int64 nValue, CWalletTx& txNew, int64& nFeeRequiredRet,
+                       WalletSendResult* presultRet=NULL);
 bool CommitTransactionSpent(const CWalletTx& wtxNew);
+// The three-argument form is kept, and kept as a distinct declaration rather
+// than a defaulted argument, because gui.cpp declares SendMoney itself instead
+// of including this header.
 bool SendMoney(CScript scriptPubKey, int64 nValue, CWalletTx& wtxNew);
+bool SendMoney(CScript scriptPubKey, int64 nValue, CWalletTx& wtxNew,
+               SendMoneyOutcome& outcomeRet);
 
 
 
diff --git a/src/rpc.cpp b/src/rpc.cpp
index 403cb14..06de7f2 100644
--- a/src/rpc.cpp
+++ b/src/rpc.cpp
@@ -1700,8 +1700,11 @@ static void ForgetPayment(const PayingRecord& r)
 
         // When the last record for a round is gone the round is no longer in
         // flight, so a later flush may explode it again if the ledger still
-        // shows recipients owed (a give-up, for instance, leaves the entry
-        // standing).
+        // shows recipients owed. That is the intended behaviour and it is now
+        // the only way a stalled payment ever leaves this list: a payment that
+        // keeps failing is retried forever rather than given up on, so its
+        // record stays here, and the flush that would re-explode it is stopped
+        // by PaymentInFlightLocked.
         bool stillInFlight = false;
         for (const auto& e : gPaying)
             if (SamePayoutRound(e.blockHeight, e.blockHash,
@@ -1776,9 +1779,16 @@ static void RecordPayoutTxid(const PayingRecord& r, const std::string& txid)
         LogPrint("payout", "[payout] block %d fully settled\n", r.blockHeight);
 }
 
-// Write a recipient off. Only reached after a full budget of failures that
-// were NOT the wallet being locked or short of coin -- see the classification
-// in PayoutThreadFn -- against a block still on the main chain.
+// Write a recipient off.
+//
+// Only ever called for a state that is DECIDABLE -- currently one: the ledger
+// names an address that is not a valid address, so no transaction paying it
+// can be constructed by this or any other build. It is not called, and must
+// not be, because a payment has failed some number of times. "The wallet said
+// no N times" is not a decidable state, it is a description of how long we
+// have been asking; a debt written off on the strength of it is a debt the
+// operator keeps, and the arithmetic that decided WHY the wallet said no was
+// wrong for a year (see PayoutThreadFn).
 //
 // The amount is removed from the entry rather than marked paid, so the money
 // is not reported as sent. Once nothing is left owed the entry retires. The
@@ -1810,6 +1820,60 @@ static void ForfeitPayoutRecipient(const PayingRecord& r, const std::string& rea
                            r.address, reason);
 }
 
+// What a failed send means for the payment's retry budget.
+//
+// A deferral is "the wallet's condition is temporarily wrong": wait, and do
+// not hold it against this payment. A fault is "the wallet is funded and
+// unlocked and still would not do it", which is worth logging loudly and
+// retrying with backoff.
+//
+// This used to be decided by re-reading GetBalance() after the failure and
+// comparing it against `amount + nTransactionFee + CENT`. Every term of that
+// was wrong:
+//
+//   * nTransactionFee is 0 in production. It is only ever assigned from the
+//     GUI preferences dialog (gui.cpp), which a headless pool node does not
+//     have, so the whole reserve was the CENT.
+//
+//   * The fee the reserve is for is GetMinFee(fDiscount=true), a STEP
+//     function of the serialised size: 0 below 10,000 bytes, and
+//     (1 + bytes/1000) * CENT at or above it -- so 11 * CENT at the step.
+//     There is no required-fee value between 1 and 11 CENT. A one-CENT
+//     reserve is therefore never the right number: below the step nothing
+//     needs reserving, and at the step it is short by a factor of eleven.
+//
+//   * A pool wallet close to broke selects many small inputs BECAUSE it is
+//     close to broke, and ~56 signed P2PKH inputs is 10 KB. The trigger and
+//     the failure are positively correlated: the guess is wrong precisely
+//     when the pool is nearly out of money.
+//
+// Concretely, with a balance of amount + 0.05: the pre-send gate passes,
+// CreateTransaction builds a >10 KB transaction, the fee becomes 0.11,
+// SelectCoins(amount + 0.11) fails, SendMoney returns false -- and the old
+// arithmetic said (amount + 0.05) < (amount + 0.01) is false, so "funded",
+// so a fault. The condition is stable, so it recurred every 60 s, and 1440 of
+// them erased the debt with the coin still in the operator's wallet.
+//
+// So stop guessing. The wallet is asked to say what it did, and this only
+// classifies the answer. WALLET_SEND_INSUFFICIENT_FUNDS is coin selection
+// failing to cover amount plus whatever fee that size of transaction
+// required, whatever that turned out to be -- no reserve to get wrong.
+enum PayoutFailureClass {
+    PAYOUT_FAILURE_DEFER = 0, // conditions are wrong; wait for them to change
+    PAYOUT_FAILURE_FAULT      // funded and unlocked, and it still would not go
+};
+
+static PayoutFailureClass ClassifyPayoutSendFailure(WalletSendResult result,
+                                                    bool fWalletLocked)
+{
+    // A locked wallet cannot sign. Nothing about this payment is wrong.
+    if (fWalletLocked)
+        return PAYOUT_FAILURE_DEFER;
+    if (result == WALLET_SEND_INSUFFICIENT_FUNDS)
+        return PAYOUT_FAILURE_DEFER;
+    return PAYOUT_FAILURE_FAULT;
+}
+
 // Payout execution thread: works through gPaying, calls SendMoney.
 // Runs independently of the event loop so slow wallet operations
 // (SendMoney takes cs_main) never stall miner I/O.
@@ -1867,9 +1931,24 @@ static void PayoutThreadFn(void*)
 
             uint160 h160;
             if (!AddressToHash160(r.address, h160)) {
-                // Should not happen -- bad addresses are filtered in QueuePayouts
-                LogPrint("payout", "[payout] SKIPPED bad address '%s'\n",
-                         r.address.c_str());
+                // Should not happen -- bad addresses are filtered in
+                // QueuePayouts. If one is here anyway it is decidable and
+                // permanent: there is no transaction that pays a string which
+                // is not an address, so retrying is not waiting for anything.
+                //
+                // This used to only ForgetPayment(), which drops the in-flight
+                // record and leaves the ledger entry claiming the debt and
+                // pool_rounds.json advertising the recipient as unpaid
+                // forever, with the next flush re-exploding it. Write it off
+                // properly and say why.
+                const std::string reason =
+                    strprintf("'%s' is not a valid address, so no transaction"
+                              " can pay it", r.address.c_str());
+                LogPrint("payout", "[payout] SKIPPED bad address '%s' --"
+                         " %s owed for block %d cannot be paid\n",
+                         r.address.c_str(), FormatMoney(r.amount).c_str(),
+                         r.blockHeight);
+                ForfeitPayoutRecipient(r, reason);
                 ForgetPayment(r);
                 continue;
             }
@@ -1926,13 +2005,17 @@ static void PayoutThreadFn(void*)
             // the operator's wallet and nothing on disk saying it was owed.
             //
             // nTransactionFee is the part of the fee that can be known before
-            // the transaction exists. The rest is GetMinFee(), a function of
-            // the serialised size and therefore of how many inputs SelectCoins
-            // ends up picking, which cannot be predicted from here -- so this
-            // gate deliberately does NOT try to reserve it. Refusing to attempt
-            // a payment the wallet could actually have made would be its own
-            // way of not paying a miner. That residual is handled after the
-            // fact, by classifying the failure below.
+            // the transaction exists, and in production it is 0 -- it is only
+            // ever assigned from the GUI preferences dialog, which a pool node
+            // does not have -- so including it here changes nothing on this
+            // pool and is kept only because a GUI operator can set it. The
+            // rest is GetMinFee(), a function of the serialised size and
+            // therefore of how many inputs SelectCoins ends up picking, which
+            // cannot be predicted from here -- so this gate deliberately does
+            // NOT try to reserve it. Refusing to attempt a payment the wallet
+            // could actually have made would be its own way of not paying a
+            // miner. That residual is handled after the fact, by asking the
+            // wallet why it refused -- see ClassifyPayoutSendFailure.
             int64 nSpendable = GetBalance();
             if (nSpendable < r.amount + nTransactionFee) {
                 r.deferrals++;
@@ -1977,7 +2060,8 @@ static void PayoutThreadFn(void*)
             wtx.mapValue["pool_payout_address"] = r.address;
             wtx.mapValue["pool_payout_amount"]  = strprintf("%lld", r.amount);
 
-            if (SendMoney(sc, r.amount, wtx)) {
+            SendMoneyOutcome outcome;
+            if (SendMoney(sc, r.amount, wtx, outcome)) {
                 // The wallet holds the transaction before this returns, so a
                 // crash between here and the ledger write is recoverable: the
                 // probe above finds it by id+address+amount and does not
@@ -1989,74 +2073,67 @@ static void PayoutThreadFn(void*)
                          FormatMoney(r.amount).c_str(), r.address.c_str(),
                          wtx.GetHash().ToString().substr(0, 12).c_str());
             } else {
-                // Why did it fail? The answer decides whether this costs a
-                // retry, and the retry budget is what eventually erases a
-                // miner's money -- so guessing here is how a pool keeps coin
-                // it owes.
+                // Why did it fail? The answer used to decide whether the
+                // payment eventually got erased, and it was decided by
+                // guessing from a balance snapshot -- see
+                // ClassifyPayoutSendFailure for the arithmetic and what it
+                // cost. The wallet now says what it did, and nothing here
+                // erases anything.
                 //
-                // Re-read the conditions rather than trusting the ones checked
-                // before the attempt: SendMoney takes cs_main and can block for
-                // a long time, so the balance and the lock state may both have
-                // moved underneath it.
-                //
-                // Short of coin covers the size-dependent part of the fee that
-                // the gate above deliberately does not reserve: if the balance
-                // no longer covers amount + configured fee + one fee tier, the
-                // failure is a funding shortfall, not a fault.
-                int64 nAfter    = GetBalance();
-                bool  fLocked   = IsWalletLocked();
-                bool  fUnfunded = nAfter < r.amount + nTransactionFee + CENT;
-
-                if (fLocked || fUnfunded) {
-                    // Conditions are temporarily wrong. Wait; do not spend the
-                    // budget that leads to a permanent write-off.
+                // The lock state is re-read rather than trusted from before
+                // the attempt: SendMoney takes cs_main and can block for a
+                // long time, so it may have moved underneath us. The funding
+                // answer does not need re-reading -- it is a statement about
+                // the attempt that was actually made.
+                bool fLocked = IsWalletLocked();
+                int64 nAfter = GetBalance();
+
+                if (ClassifyPayoutSendFailure(outcome.result, fLocked)
+                        == PAYOUT_FAILURE_DEFER) {
+                    // Conditions are temporarily wrong. Wait.
                     r.deferrals++;
                     r.retryAfter = GetTime() + 60;
                     if (r.deferrals == 1 || r.deferrals % 60 == 0)
                         LogPrint("payout", "[payout] deferring %s to %s -- send"
-                                 " failed with %s (%s spendable, %s needed with"
-                                 " fee headroom); waiting %d min so far\n",
+                                 " failed with %s (%s spendable; the wallet"
+                                 " needed a fee of %s for a transaction this"
+                                 " size); waiting %d min so far\n",
                                  FormatMoney(r.amount).c_str(), r.address.c_str(),
                                  fLocked ? "the wallet locked" : "too little coin",
                                  FormatMoney(nAfter).c_str(),
-                                 FormatMoney(r.amount + nTransactionFee + CENT).c_str(),
+                                 FormatMoney(outcome.nFeeRequired).c_str(),
                                  r.deferrals);
                     ReschedulePayment(r);
-                } else if (r.retries < 1440) {
-                    // A genuine fault: funded, unlocked, block on the main
-                    // chain, and the wallet still would not build or accept
-                    // the transaction. Retry with backoff.
+                } else {
+                    // Funded, unlocked, block on the main chain, and the
+                    // wallet still would not build, sign or accept the
+                    // transaction. That is worth shouting about; it is not
+                    // grounds to keep the money.
+                    //
+                    // There is deliberately no give-up branch. It used to
+                    // forfeit the debt after 1440 of these, which is not a
+                    // decidable state -- it is a count of how long we have
+                    // been asking, and it was reached by misclassifying a
+                    // funding shortfall as a fault. A debt that is never
+                    // written off cannot be written off wrongly. The payment
+                    // stays in flight, keeps its ledger entry, keeps being
+                    // reported in pool_status.json as owed, and keeps
+                    // retrying once a minute until the operator or the wallet
+                    // makes it possible.
                     r.retries++;
                     r.retryAfter = GetTime() + 60;
                     ReschedulePayment(r);
-                    LogPrint("payout", "[payout] FAILED to pay %s to %s"
-                             " (%s spendable, wallet unlocked) -- retry %d/1440"
-                             " in 60s\n",
-                             FormatMoney(r.amount).c_str(), r.address.c_str(),
-                             FormatMoney(nAfter).c_str(), r.retries);
-                } else {
-                    // 1440 failures that were each individually impossible to
-                    // explain by maturity, funding, the wallet being locked, or
-                    // a reorg. That is the only state in which this payment can
-                    // be called impossible rather than merely blocked.
-                    const std::string reason = strprintf(
-                        "wallet refused this payment %d times while funded"
-                        " (%s spendable), unlocked, and block %d on the main"
-                        " chain", r.retries, FormatMoney(nAfter).c_str(),
-                        r.blockHeight);
-                    LogPrint("payout", "[payout] GIVING UP on %s to %s: %s"
-                             " -- payment forfeited\n",
-                             FormatMoney(r.amount).c_str(), r.address.c_str(),
-                             reason.c_str());
-                    // Write the debt off in the ledger as well as dropping the
-                    // in-flight record. Under the survives-until-fully-paid
-                    // model the entry would otherwise never complete, and every
-                    // later flush would explode it again -- a forfeit that
-                    // silently becomes an infinite retry. The round proof is
-                    // told too, so the public record does not keep claiming the
-                    // address is owed.
-                    ForfeitPayoutRecipient(r, reason);
-                    ForgetPayment(r);
+                    // Every minute at first, then hourly: a fault that is not
+                    // going away must not turn the log into one line a minute
+                    // forever, and must not go quiet either.
+                    if (r.retries <= 5 || r.retries % 60 == 0)
+                        LogPrint("payout", "[payout] FAILED to pay %s to %s"
+                                 " (%s spendable, wallet unlocked, block %d on"
+                                 " the main chain) -- attempt %d, retrying in"
+                                 " 60s; this debt stands until it is paid\n",
+                                 FormatMoney(r.amount).c_str(), r.address.c_str(),
+                                 FormatMoney(nAfter).c_str(), r.blockHeight,
+                                 r.retries);
                 }
             }
         }
@@ -2850,13 +2927,19 @@ static void PayoutTestRemoveDir(const std::string& dir)
     }
     RemoveDirectoryA(dir.c_str());
 #else
+    // One level of recursion, because CreateTransaction() opens a Berkeley DB
+    // environment and BDB puts its logs in a "database" subdirectory. Without
+    // this the scratch directory cannot be removed and the test leaves one
+    // behind on every run, which is what d5b3503 was for.
     DIR* d = opendir(dir.c_str());
     if (d) {
         struct dirent* ent;
         while ((ent = readdir(d)) != NULL) {
             std::string name = ent->d_name;
             if (name == "." || name == "..") continue;
-            unlink((dir + "/" + name).c_str());
+            const std::string path = dir + "/" + name;
+            if (unlink(path.c_str()) != 0)
+                PayoutTestRemoveDir(path);
         }
         closedir(d);
     }
@@ -2942,6 +3025,48 @@ static std::string PayoutTestAddWalletTx(const std::string& comment,
     return hash.GetHex();
 }
 
+// A real key, in the keystore, so a test wallet's outputs are IsMine() and can
+// actually be signed -- both are needed for CreateTransaction to build the
+// large transaction the fee schedule is being tested against. Inserted
+// straight into the key maps rather than through AddKey(), which would also
+// write it to a wallet.dat.
+static std::string PayoutTestAddSpendableKey()
+{
+    CKey key;
+    key.MakeNewKey();
+    std::vector<unsigned char> vchPubKey = key.GetPubKey();
+    CRITICAL_BLOCK(cs_mapKeys)
+    {
+        mapKeys[vchPubKey]                = key.GetPrivKey();
+        mapPubKeys[Hash160(vchPubKey)]    = vchPubKey;
+    }
+    return Hash160ToAddress(Hash160(vchPubKey));
+}
+
+// Give the wallet `n` spendable outputs of `each`, one transaction apiece --
+// which is the granularity SelectCoins works at, and therefore the number of
+// inputs the transaction spending them will carry.
+//
+// Deliberately not coinbases: a null prevout would make CWalletTx::GetCredit
+// return 0 until COINBASE_SPENDABLE_DEPTH, and there is no chain here.
+static void PayoutTestFundWallet(int n, int64 each, const std::string& address)
+{
+    const CScript sc = PayoutTestScriptFor(address);
+    CRITICAL_BLOCK(cs_mapWallet)
+    {
+        for (int i = 0; i < n; i++) {
+            CWalletTx wtx;
+            wtx.vin.resize(1);
+            wtx.vin[0].prevout.hash = (uint64)(i + 1);
+            wtx.vin[0].prevout.n    = 0;
+            wtx.vout.resize(1);
+            wtx.vout[0].nValue       = each;
+            wtx.vout[0].scriptPubKey = sc;
+            mapWallet[wtx.GetHash()] = wtx;
+        }
+    }
+}
+
 // Defined below with the event loop; the status-file publication test drives
 // it directly.
 static void RefreshStats(int blocksFoundThisSession, uint64 roundShareTotal);
@@ -2967,11 +3092,32 @@ int RunPayoutLedgerSelfTest()
 #else
     unsigned long pid = (unsigned long)getpid();
 #endif
-    std::string dir = strprintf("payout_selftest_%d_%lu", (int)GetTime(), pid);
+    // Absolute, and the process moves into it.
+    //
+    // Both are needed by the CreateTransaction section: Berkeley DB resolves
+    // its log directory relative to the environment home, so a relative
+    // datadir makes dbenv.open() fail with DB_RUNRECOVERY; and BDB opens
+    // db.log relative to the working directory, which is src/ during `make
+    // tests`. Moving in puts it inside the scratch directory, where it dies
+    // with everything else.
+    std::string cwd = ".";
+    {
+#ifdef _WIN32
+        char buf[MAX_PATH];
+        if (GetCurrentDirectoryA(MAX_PATH, buf) > 0) cwd = buf;
+#else
+        char buf[4096];
+        if (getcwd(buf, sizeof(buf)) != NULL) cwd = buf;
+#endif
+    }
+    std::string dir = cwd + "/" +
+                      strprintf("payout_selftest_%d_%lu", (int)GetTime(), pid);
 #ifdef _WIN32
     CreateDirectoryA(dir.c_str(), NULL);
+    SetCurrentDirectoryA(dir.c_str());
 #else
     mkdir(dir.c_str(), 0700);
+    if (chdir(dir.c_str()) != 0) { /* stay put; the paths are absolute anyway */ }
 #endif
     strSetDataDir = dir;
     fprintf(stdout, "  scratch datadir: %s\n", dir.c_str());
@@ -3494,7 +3640,141 @@ int RunPayoutLedgerSelfTest()
         strPoolRoundsFile = savedRoundsFile;
     }
 
-    // -- 11. No payout mutex is held across the status-file write ------------
+    // -- 11. A send that failed for want of coin is not a fault --------------
+    //
+    // The defect this replaces: the classifier decided "was this a funding
+    // problem?" by re-reading GetBalance() and allowing amount +
+    // nTransactionFee + CENT. nTransactionFee is 0 on a headless node, and
+    // GetMinFee() is a step function -- 0 below 10,000 bytes, 11 * CENT at it
+    // -- so the allowance was either unnecessary or eleven times too small,
+    // and the ledger's own retry budget then wrote the debt off after 1440
+    // repeats of the same stable condition.
+    //
+    // This is tested against a real wallet and the real CreateTransaction, not
+    // against injected values: the honest question is not "does my enum
+    // switch work" but "does a wallet holding many small coins actually report
+    // the state I think it does". 60 outputs of 1 BTF give ~180 signed P2PKH
+    // bytes each, which is where the fee schedule steps. Nothing is committed
+    // or broadcast -- CreateTransaction only builds.
+    {
+        PayoutTestResetState();
+        CRITICAL_BLOCK(cs_mapWallet)
+            mapWallet.clear();
+
+        // CreateTransaction opens CTxDB("r"), which throws if blkindex.dat is
+        // not there. Create it in the scratch datadir first.
+        bool dbOk = true;
+        try {
+            CTxDB txdb("cr");
+        } catch (const std::exception&) {
+            dbOk = false;
+        }
+        check(dbOk, "a scratch block index exists for CreateTransaction to open");
+
+        const std::string walletAddr = PayoutTestAddSpendableKey();
+        const std::string payee      = PayoutTestAddress(0x77);
+        const CScript     payeeScript = PayoutTestScriptFor(payee);
+
+        // -- the >= 10 KB case: 60 small coins, balance = amount + 0.05 ------
+        PayoutTestFundWallet(60, 1 * COIN, walletAddr);
+        const int64 balance = GetBalance();
+        check(balance == 60 * COIN,
+              "the test wallet holds 60 separately spendable coins");
+
+        const int64 amount = balance - 5 * CENT;
+        // The pre-send gate in PayoutThreadFn passes: the balance covers the
+        // amount and the only fee that can be known in advance.
+        check(balance >= amount + nTransactionFee,
+              "the pre-send gate would let this payment through");
+
+        CWalletTx        wtx;
+        SendMoneyOutcome outcome;
+        int64            feeRequired = 0;
+        WalletSendResult result      = WALLET_SEND_OK;
+        bool             built       = true;
+        if (dbOk) {
+            try {
+                built = CreateTransaction(payeeScript, amount, wtx, feeRequired,
+                                          &result);
+            } catch (const std::exception&) {
+                built = true;   // report as a failure below
+            }
+        }
+        check(dbOk && !built,
+              "a wallet of many small coins cannot fund amount + the fee its"
+              " own size demands");
+        check(dbOk && result == WALLET_SEND_INSUFFICIENT_FUNDS,
+              "the wallet reports that as a funding shortfall, not as a"
+              " failure to build");
+        // GetMinFee(fDiscount) returns 0 below 10,000 bytes and
+        // (1 + bytes/1000) * CENT at or above, so a required fee of 11 CENT or
+        // more IS the statement that the transaction crossed 10 KB. Measured
+        // through the real serialiser rather than asserted.
+        check(dbOk && feeRequired >= 11 * CENT,
+              "the transaction it built crossed 10,000 bytes, so the fee"
+              " stepped from 0 to at least 11 CENT");
+
+        // The defect, stated as arithmetic. The old classifier asked whether
+        // the balance covered amount + nTransactionFee + CENT; it did, so it
+        // called a permanent funding shortfall a fault and started counting
+        // towards a write-off.
+        check(!(balance < amount + nTransactionFee + CENT),
+              "the reserve this replaces would have judged this send a fault");
+        check(balance < amount + feeRequired,
+              "while the fee actually required was more than the balance had");
+
+        outcome.result       = result;
+        outcome.nFeeRequired = feeRequired;
+        check(dbOk && ClassifyPayoutSendFailure(outcome.result, false)
+                          == PAYOUT_FAILURE_DEFER,
+              "a >= 10 KB send that ran out of coin is deferred, not counted"
+              " against the payment");
+
+        // -- the < 10 KB case: no reserve is needed at all -------------------
+        //
+        // The other half of the arithmetic. Below the step the required fee is
+        // 0, so a reserve of any size is a reason to refuse a payment the
+        // wallet could have made -- which is its own way of not paying a
+        // miner.
+        CRITICAL_BLOCK(cs_mapWallet)
+            mapWallet.clear();
+        PayoutTestFundWallet(3, 1 * COIN, walletAddr);
+        const int64 smallAmount = 3 * COIN - 5 * CENT;
+        CWalletTx   smallTx;
+        int64       smallFee    = -1;
+        WalletSendResult smallResult = WALLET_SEND_FAILED;
+        bool        smallBuilt  = false;
+        if (dbOk) {
+            try {
+                smallBuilt = CreateTransaction(payeeScript, smallAmount, smallTx,
+                                               smallFee, &smallResult);
+            } catch (const std::exception&) {
+                smallBuilt = false;
+            }
+        }
+        check(dbOk && smallBuilt && smallResult == WALLET_SEND_OK,
+              "the same balance pays a small transaction with no headroom at"
+              " all");
+        check(dbOk && smallFee == 0,
+              "because below 10,000 bytes the required fee is zero");
+
+        // -- and the classification of every other answer --------------------
+        check(ClassifyPayoutSendFailure(WALLET_SEND_INSUFFICIENT_FUNDS, false)
+                  == PAYOUT_FAILURE_DEFER &&
+              ClassifyPayoutSendFailure(WALLET_SEND_INSUFFICIENT_FUNDS, true)
+                  == PAYOUT_FAILURE_DEFER &&
+              ClassifyPayoutSendFailure(WALLET_SEND_FAILED, true)
+                  == PAYOUT_FAILURE_DEFER,
+              "short of coin or unable to sign is always a deferral");
+        check(ClassifyPayoutSendFailure(WALLET_SEND_FAILED, false)
+                  == PAYOUT_FAILURE_FAULT,
+              "funded and unlocked and still refused is a fault");
+
+        CRITICAL_BLOCK(cs_mapWallet)
+            mapWallet.clear();
+    }
+
+    // -- 12. No payout mutex is held across the status-file write ------------
     //
     // RefreshStats runs on the event loop -- the thread that owns every
     // miner's socket -- and used to hold gStatsMutex and gPayoutLedgerMutex at
@@ -3628,6 +3908,19 @@ int RunPayoutLedgerSelfTest()
     PayoutTestResetState();
     CRITICAL_BLOCK(cs_mapWallet)
         mapWallet.clear();
+    // Out of the scratch directory before removing it, and out of the key maps
+    // too: the keys this test generated are process-local, but leaving them in
+    // the keystore would let a later test think a script was its own.
+    CRITICAL_BLOCK(cs_mapKeys)
+    {
+        mapKeys.clear();
+        mapPubKeys.clear();
+    }
+#ifdef _WIN32
+    SetCurrentDirectoryA(cwd.c_str());
+#else
+    if (chdir(cwd.c_str()) != 0) { /* nothing useful to do about it */ }
+#endif
     PayoutTestRemoveDir(dir);
     strSetDataDir = savedDataDir;
     nBestHeight = savedBestHeight;