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.

1 files changed, 44 insertions, 1 deletions

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);