wallet: believe the chain about what has already been spent (#62)

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.

3 files changed, 71 insertions, 0 deletions

diff --git a/src/main.cpp b/src/main.cpp
index dbb2c41..85f89d9 100644
--- a/src/main.cpp
+++ b/src/main.cpp
@@ -693,6 +693,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 eea10b1..2d5cb86 100644
--- a/src/main.h
+++ b/src/main.h
@@ -117,6 +117,8 @@ bool AddKey(const CKey& key);
 vector<unsigned char> 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 99bcb6f..c593162 100644
--- a/src/main_gui.cpp
+++ b/src/main_gui.cpp
@@ -243,6 +243,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)
     {