Skip to content

wallet: one quiet batch is not far enough to stop a restore - #115

Merged
Bitflash-sh merged 1 commit into
mainfrom
restore-gap-limit
Aug 2, 2026
Merged

wallet: one quiet batch is not far enough to stop a restore#115
Bitflash-sh merged 1 commit into
mainfrom
restore-gap-limit

Conversation

@Bitflash-sh

Copy link
Copy Markdown
Owner

Found while proving #109 with real coin, and it is a consequence of #109. A wallet could hold money that the phrase derives, that the phrase covers, and that restoring from the phrase does not find.

The failure, on a real wallet

The sequence that produced it — all on the live chain, current main:

  1. -newphrase, then a payment from another node to -newaddress → lands on derived index 1
  2. delete wallet.dat, -restorephrase → recovers it, and leaves nHDNext at 201
  3. -sendto a 1 BTF spend → payment goes to index 201, change to index 302 (this is Keep HD change recoverable and bound message stalls #109 working: change is derived, not returned to an input's key)
  4. delete wallet.dat, -restorephrase again:
  101 addresses checked, 1 transaction(s) recovered
  201 addresses checked, 1 transaction(s) recovered

Restored: 1 transaction(s) across 201 derived addresses

The spend is gone. Both the payment and the 4 BTF of change. -restoredepth=500 on the same wallet reported 2 transactions, which is what said this was reach and not loss.

Why

The loop stopped after the first batch of a hundred that turned up nothing, and the wallet digs a gap wider than that all by itself:

  • a restore leaves nHDNext at the depth it scanned (201)
  • the key pool then derives KEYPOOL_SIZE more (201–300)
  • change takes the next index after those (302)

So the address used after a restore sits two hundred indices past the last one used before it, with nothing in between. The gap limit was smaller than the gap the wallet creates.

This is worse after #109, not before it. Change used to go back to an input's key — a low index the scan had already passed — so it was always found. Fixing the recoverability of change moved it out to where the scan stopped looking. One way to lose money traded for another.

The change

Three quiet batches instead of one. Same wallet, same chain, same phrase:

  301 addresses checked, 2 transaction(s) recovered
  ...
Restored: 2 transaction(s) across 601 derived addresses

What this does not fix

The gap is self-inflicted and it grows: each restore leaves nHDNext at the new scan depth, so the next spend sits further out than the last. A long enough history of restores can outrun any fixed number of quiet batches.

The structural answer is to pull nHDNext back to just past the highest index a transaction actually used, so the wallet stops digging. That is deliberately not in this PR: it rewinds a counter that hands out addresses, and it deserves its own testing rather than riding along with a fix that is already measured. Filed as the follow-up.

Testing

make tests passes. The before/after above is the same wallet with real coin on the live chain, restored twice.

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.
@Bitflash-sh
Bitflash-sh merged commit c33d013 into main Aug 2, 2026
Bitflash-sh added a commit that referenced this pull request Aug 3, 2026
CmdSendTo() has been in walletcmd.cpp since #115 and nothing declared
it or called it. The flag was dead: `bitflash -sendto=ADDRESS,AMOUNT`
started a node and said nothing, because an unrecognised argument is
not an error here.

The v1.2.14 notes announce it under "A headless node can finally
spend", and the headless Linux binary is exactly the one that does not
have it -- `-help` there lists /newaddress and stops. The published
Windows binary does have it, because it was built from a working tree
that carried these two files uncommitted. So the artifacts disagree
with each other and neither matches the tag.

This adds only what was missing: the declaration in walletcmd.h, the
dispatch in main(), and the -help line. No behaviour change to
CmdSendTo itself.

Checked on a build of this branch, against a throwaway data directory:

  -help                          lists /sendto=ADDRESS,AMOUNT
  -sendto=NOTANADDRESS,1         "Not a valid address", exit 1
  -sendto=<valid>,-5             "Not a valid amount", exit 1
  -sendto=<valid>,999            refused on an empty wallet, exit 1

All five self-tests still pass.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant