Two findings from the audit that I deliberately did not patch, because a hasty fix for either is more dangerous than the bug. Recording them so they get solved on purpose rather than forgotten.
1. Pool payouts are lost if the operator crashes at the wrong moment
FlushMaturePayouts erases a matured payout from gPendingPayouts, rewrites the file without it, and only then pushes it to gPayoutExecQueue — which is memory-only. SavePendingPayouts serialises gPendingPayouts and nothing else.
Crash between the rewrite and SendMoney and those miners are never paid, with nothing on disk recording the debt. The comment claiming recovery from an "exec-queue section of the file" was wrong and is corrected in #22.
Why not just persist the queue and replay it: a crash after SendMoney would then pay the same miners twice, draining the operator's wallet. Shortchanging a miner is bad; silently double-spending the operator's balance is worse.
What it actually needs: idempotent payment. Record the txid alongside each payout, and on restart look it up in the wallet — re-send only when absent. Wants testing against a live pool with real miners, which is not something the audit could exercise.
2. Change goes back to a key the wallet already used
CreateTransaction builds the change output from a pubkey pulled off one of the inputs:
CTransaction& txFirst = *(*setCoins.begin());
foreach(const CTxOut& txout, txFirst.vout)
if (txout.IsMine())
if (ExtractPubKey(txout.scriptPubKey, true, vchPubKey))
break;Bitcoin generates a fresh key here, with the comment "to keep it from being obvious which side is the change." Reusing an input's key makes the change output trivially identifiable, and links every transaction that touches that key into one cluster. For a project whose stated point is anonymity — .btf addressing, no IP advertised to peers — the wallet undoes on-chain what the network layer works to protect.
Why not just call GenerateNewKey(): there is no keypool. Every transaction would create a key that is not in the user's last wallet.dat backup, so restoring a backup taken before the send loses the change. That converts a privacy weakness into lost coins, which is not a trade worth making silently.
What it actually needs: a keypool — pre-generate keys so a backup covers the next N transactions — and then fresh change keys on top of it. That is how Bitcoin resolved the same tension.
Note on severity
Neither is remotely exploitable. They are correctness and privacy gaps, unlike the denial-of-service and theft issues in #12 and #16–#20. They are filed because both are the kind of thing that gets rediscovered painfully later, by someone who lost money.
Two findings from the audit that I deliberately did not patch, because a hasty fix for either is more dangerous than the bug. Recording them so they get solved on purpose rather than forgotten.
1. Pool payouts are lost if the operator crashes at the wrong moment
FlushMaturePayoutserases a matured payout fromgPendingPayouts, rewrites the file without it, and only then pushes it togPayoutExecQueue— which is memory-only.SavePendingPayoutsserialisesgPendingPayoutsand nothing else.Crash between the rewrite and
SendMoneyand those miners are never paid, with nothing on disk recording the debt. The comment claiming recovery from an "exec-queue section of the file" was wrong and is corrected in #22.Why not just persist the queue and replay it: a crash after
SendMoneywould then pay the same miners twice, draining the operator's wallet. Shortchanging a miner is bad; silently double-spending the operator's balance is worse.What it actually needs: idempotent payment. Record the txid alongside each payout, and on restart look it up in the wallet — re-send only when absent. Wants testing against a live pool with real miners, which is not something the audit could exercise.
2. Change goes back to a key the wallet already used
CreateTransactionbuilds the change output from a pubkey pulled off one of the inputs:Bitcoin generates a fresh key here, with the comment "to keep it from being obvious which side is the change." Reusing an input's key makes the change output trivially identifiable, and links every transaction that touches that key into one cluster. For a project whose stated point is anonymity —
.btfaddressing, no IP advertised to peers — the wallet undoes on-chain what the network layer works to protect.Why not just call
GenerateNewKey(): there is no keypool. Every transaction would create a key that is not in the user's lastwallet.datbackup, so restoring a backup taken before the send loses the change. That converts a privacy weakness into lost coins, which is not a trade worth making silently.What it actually needs: a keypool — pre-generate keys so a backup covers the next N transactions — and then fresh change keys on top of it. That is how Bitcoin resolved the same tension.
Note on severity
Neither is remotely exploitable. They are correctness and privacy gaps, unlike the denial-of-service and theft issues in #12 and #16–#20. They are filed because both are the kind of thing that gets rediscovered painfully later, by someone who lost money.