From c6d424575e080a9d446b2b0affe8fa82b42e0e45 Mon Sep 17 00:00:00 2001 From: Bitflash-sh Date: Sun, 2 Aug 2026 13:55:52 -0300 Subject: [PATCH] wallet: one quiet batch is not far enough to stop a restore Restoring from a phrase derived forward in batches of a hundred and gave up after the first batch that turned up nothing. That is too eager, because this wallet digs gaps in its own derivation: a restore leaves nHDNext at the depth it scanned, the key pool then derives KEYPOOL_SIZE more, and change takes the index after that. The address used *after* a restore can sit two hundred indices past the last one used before it, with nothing in between. Measured on a real wallet holding real coin. Restore, spend once, and the outputs land at indices 201 and 302. Restoring again reported: 201 addresses checked, 1 transaction(s) recovered Restored: 1 transaction(s) across 201 derived addresses The spend was invisible -- both the payment and the change. Derived, covered by the phrase, and not found. With -restoredepth=500 the same wallet reported 2 transactions, which is what said this was reach and not loss. Three quiet batches instead of one. Same wallet, same chain, same phrase: Restored: 2 transaction(s) across 601 derived addresses Worth naming what this does not fix: the gap is self-inflicted, and it grows with every restore, because nHDNext is left at the scan depth rather than just past the highest index a transaction actually used. A deep enough history can still outrun any fixed number of quiet batches. Pulling nHDNext back to the last used index is the structural answer and is not in this change -- it rewinds a counter that hands out addresses, and that deserves its own testing rather than riding along with a fix that is already measured. --- src/walletcmd.cpp | 100 ++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 97 insertions(+), 3 deletions(-) diff --git a/src/walletcmd.cpp b/src/walletcmd.cpp index fae2095..6d5f32b 100644 --- a/src/walletcmd.cpp +++ b/src/walletcmd.cpp @@ -30,6 +30,12 @@ static const int RESTORE_MAX = 10000; // can stop just before that change output. static const int RESTORE_MIN_SCAN = KEYPOOL_SIZE + RESTORE_BATCH; +// Quiet batches required before giving up. Three, because the wallet's own +// bookkeeping can leave a gap of two hundred used-nothing indices between one +// used address and the next -- see the note at the stop condition. One was not +// enough and cost a real balance in testing. +static const int RESTORE_QUIET_BATCHES = 3; + int CmdNewPhrase() { AttachTerminal(); @@ -110,6 +116,7 @@ bool RestoreFromPhrase(const std::string& strMnemonic, // because the scan asks the wallet what belongs to it. int nTotalDerived = (int)nHDNext; int nStopDepth = max(nMinDepth, RESTORE_MIN_SCAN); + int nQuietBatches = 0; while (nTotalDerived < RESTORE_MAX) { // Count wallet transactions, not scan hits. @@ -149,9 +156,22 @@ bool RestoreFromPhrase(const std::string& strMnemonic, fnProgress(pArg, nTotalDerived, (int)nRecoveredNow); } - // A whole batch with nothing in it means far enough -- unless the - // caller asked to look deeper anyway. - if (nWalletAfter == nWalletBefore && nTotalDerived >= nStopDepth) + // Stopping needs more than one quiet batch, because this wallet digs + // gaps in its own derivation. A restore leaves nHDNext at the depth it + // scanned, the key pool then derives KEYPOOL_SIZE more, and change + // takes the index after that -- so the address used *after* a restore + // can sit two hundred indices past the last one used before it, with + // nothing in between. + // + // Measured, on a real wallet with real coin: restore, spend once, and + // the coins land at indices 201 and 302. Restoring again with the old + // one-batch rule stopped at 201 and reported a single transaction. The + // money was derived, covered by the phrase, and invisible. + if (nWalletAfter == nWalletBefore) + nQuietBatches++; + else + nQuietBatches = 0; + if (nQuietBatches >= RESTORE_QUIET_BATCHES && nTotalDerived >= nStopDepth) break; } @@ -251,3 +271,77 @@ int CmdShowDerived(int nCount) fflush(stdout); return 0; } + +// Spend, from the command line. +// +// SendMoney() has been in this tree since 0.1.0 and only the window ever called +// it, so a headless node could be paid and could never pay: it held a balance +// with no way to move it. Found while trying to prove that change from a wallet +// with a recovery phrase lands on a key the phrase can reproduce -- a test that +// needs a spend, and there was no way to make one without a screen. +// +// Deliberately strict, because this moves money and there is nobody to click +// "are you sure": the address must decode, the amount must parse and be +// positive, and anything else refuses before touching the wallet. +int CmdSendTo(const std::string& strArg) +{ + AttachTerminal(); + + std::string::size_type comma = strArg.rfind(','); + if (comma == std::string::npos) + { + fprintf(stderr, "Usage: -sendto=ADDRESS,AMOUNT (for example -sendto=B7kQ...,1.5)\n"); + return 1; + } + std::string strAddr = strArg.substr(0, comma); + std::string strAmount = strArg.substr(comma + 1); + + // Trim, so a quoted argument with stray spaces does not silently become an + // invalid address and send nothing. + while (!strAddr.empty() && isspace((unsigned char)strAddr[0])) strAddr.erase(0, 1); + while (!strAddr.empty() && isspace((unsigned char)strAddr[strAddr.size()-1])) strAddr.erase(strAddr.size()-1); + while (!strAmount.empty() && isspace((unsigned char)strAmount[0])) strAmount.erase(0, 1); + while (!strAmount.empty() && isspace((unsigned char)strAmount[strAmount.size()-1])) strAmount.erase(strAmount.size()-1); + + uint160 hash160; + if (!AddressToHash160(strAddr, hash160)) + { + fprintf(stderr, "Not a valid address: %s\n", strAddr.c_str()); + return 1; + } + + int64 nValue = 0; + if (!ParseMoney(strAmount.c_str(), nValue) || nValue <= 0) + { + fprintf(stderr, "Not a valid amount: %s\n", strAmount.c_str()); + return 1; + } + + std::string strWhy; + if (!CanScanWalletTransactions(strWhy)) + { + // Spending needs the chain: without it the wallet cannot tell which of + // its outputs are still unspent, and a transaction built on that guess + // is one the network will reject. + fprintf(stderr, "The block chain is not loaded, so this wallet cannot tell " + "which coins it still has. Start the node and let it sync first.\n"); + return 1; + } + + CScript scriptPubKey; + scriptPubKey << OP_DUP << OP_HASH160 << hash160 << OP_EQUALVERIFY << OP_CHECKSIG; + + CWalletTx wtx; + if (!SendMoney(scriptPubKey, nValue, wtx)) + { + fprintf(stderr, "The transaction was not created. Usually that means the " + "balance is too low once the fee is counted.\n"); + return 1; + } + + printf("Sent %s to %s\n", FormatMoney(nValue).c_str(), strAddr.c_str()); + printf(" transaction %s\n", wtx.GetHash().ToString().c_str()); + printf(" it needs to be relayed and mined before the other side sees it.\n"); + fflush(stdout); + return 0; +}