From ec9527cb40341a5338864566d17fd1006fe35f2c Mon Sep 17 00:00:00 2001 From: Bitflash-sh Date: Thu, 30 Jul 2026 19:18:12 -0300 Subject: [PATCH] wallet: believe the chain about what has already been spent fSpent lives in wallet.dat and is written when this node spends something. A wallet.dat restored from backup, or copied and used on another machine, carries whatever that flag was when the copy was taken -- so it can say "unspent" about coins the chain shows as gone. The balance reads high and nothing contradicts it until a send is attempted against coins that do not exist. #47 is about this shape of problem and #40 is someone living through it; Bitcoin fixed the same thing in 53d508072. On startup, after the block index is loaded and before anything reports a balance, walk the wallet and mark spent whatever the transaction index says is spent. One direction only. Marking spent when the chain says spent is safe; clearing the flag because the chain has not caught up would offer up coins already on their way out. fSpent is one flag per transaction rather than per output, which is how 0.1.0 has always worked -- CommitTransactionSpent already marks a whole previous transaction when it spends any part of it -- so this matches rather than changing the model. It reports every run, including the runs where it finds nothing, with a breakdown of why: RescanSpentFlags() : examined 17, already-spent 0, not-indexed 17, corrected 0 A check that speaks only when it finds something is indistinguishable from one that never ran, and that confusion has cost this project real time. The breakdown earned its place immediately: it is how the test below turned from a guess into an answer. Verified running against a wallet with 17 transactions and against an empty one, with no false corrections in either. The correcting branch itself is NOT exercised by a real case: on this chain no test wallet holds a transaction that is both in the transaction index and spent, and the counters show why -- all 17 were absent from the index, so the comparison never had anything to compare. --- src/main.cpp | 65 ++++++++++++++++++++++++++++++++++++++++++++++++ src/main.h | 2 ++ src/main_gui.cpp | 4 +++ 3 files changed, 71 insertions(+) diff --git a/src/main.cpp b/src/main.cpp index 15805ca..da7e500 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -684,6 +684,71 @@ bool CWalletTx::AcceptWalletTransaction(CTxDB& txdb, bool fCheckInputs) return true; } +int RescanSpentFlags() +{ + // Believe the chain, not the wallet, about what has already been spent. + // + // fSpent lives in wallet.dat and is written when this node spends + // something. A wallet.dat that was restored from backup, or copied and + // used on another machine, carries whatever that flag was at the moment + // the copy was taken -- so it can say "unspent" about coins the chain + // shows as gone. The balance then reads high, and the error only surfaces + // when a send is attempted against coins that no longer exist. + // + // Issue #47 is about this shape of problem and #40 reports living through + // it. Bitcoin fixed the same thing in 53d508072. + // + // One direction only. Marking spent when the chain says spent is safe; + // clearing the flag because the chain has not caught up yet would offer + // up coins that are already on their way out. + CTxDB txdb("r"); + int nCorrected = 0; + int nExamined = 0; + int nAlreadySpent = 0, nNotIndexed = 0; + CRITICAL_BLOCK(cs_mapWallet) + { + foreach(PAIRTYPE(const uint256, CWalletTx)& item, mapWallet) + { + CWalletTx& wtx = item.second; + nExamined++; + if (wtx.fSpent) + { nAlreadySpent++; continue; } + + CTxIndex txindex; + if (!txdb.ReadTxIndex(wtx.GetHash(), txindex)) + { nNotIndexed++; continue; } + + // fSpent is one flag for the whole transaction, not one per + // output -- CommitTransactionSpent already marks the entire + // previous transaction when it spends any part of it, so matching + // that here keeps the two consistent. + bool fSeenSpent = false; + for (int i = 0; i < (int)txindex.vSpent.size() && i < (int)wtx.vout.size(); i++) + if (!txindex.vSpent[i].IsNull() && wtx.vout[i].IsMine()) + fSeenSpent = true; + + if (fSeenSpent) + { + wtx.fSpent = true; + wtx.WriteToDisk(); + nCorrected++; + printf("RescanSpentFlags() : %s was spent on chain but the wallet did not know\n", + wtx.GetHash().ToString().substr(0,10).c_str()); + } + } + } + // Always say it ran. A check that reports only when it finds something is + // indistinguishable from a check that never executed, and this codebase has + // paid for that confusion more than once. + printf("RescanSpentFlags() : examined %d, already-spent %d, not-indexed %d, corrected %d\n", + nExamined, nAlreadySpent, nNotIndexed, nCorrected); + if (nCorrected) + printf("RescanSpentFlags() : this wallet was behind the chain about what it had already spent, " + "which is what a restored backup looks like\n"); + return nCorrected; +} + + void ReacceptWalletTransactions() { // Reaccept any txes of ours that aren't already in a block diff --git a/src/main.h b/src/main.h index 750d3f1..d4f3642 100644 --- a/src/main.h +++ b/src/main.h @@ -115,6 +115,8 @@ bool AddKey(const CKey& key); vector GenerateNewKey(); bool AddToWallet(const CWalletTx& wtxIn); void ReacceptWalletTransactions(); +// Returns how many wallet transactions the chain corrected. +int RescanSpentFlags(); void RelayWalletTransactions(); bool LoadBlockIndex(bool fAllowNew=true); void PrintBlockTree(); diff --git a/src/main_gui.cpp b/src/main_gui.cpp index 852ad4d..e67cbcf 100644 --- a/src/main_gui.cpp +++ b/src/main_gui.cpp @@ -238,6 +238,10 @@ int main(int argc, char* argv[]) try { if (!LoadWallet()) { fprintf(stderr, "LoadWallet failed\n"); return 1; } + + // After the block index, so there is a chain to compare the wallet + // against, and before anything reports a balance. + RescanSpentFlags(); } catch (const std::exception& e) {