Keep HD change recoverable and bound message stalls - #109
Merged
Bitflash-sh merged 1 commit intoAug 2, 2026
Merged
Conversation
Bitflash-sh
added a commit
that referenced
this pull request
Aug 2, 2026
CheckBlock() validated block size against MAX_SIZE, the serializer's generic 32 MB guard rather than a block rule; MAX_BLOCK_SIZE did not exist in this tree at all, because Bitcoin's 1 MB cap arrived in 0.3.x, after the 0.1.0 this fork came from. A miner could produce a 32 MB block and every node would download, verify and store it. Original work by mtg0001 in #110, rebased over #109 with the self-test conflict resolved, plus a comment explaining why the inherited count-against-bytes test stays. Measured before choosing the number: 5709 blocks read from blk0001.dat give a largest block of 34,290 bytes and a median of 216, so 1 MB leaves 29x of headroom and rejects nothing that exists. A node built from this branch then synced genesis to 5706 on the live network with zero size-limit rejections and zero CheckBlock errors -- the same test that caught BIP30 rejecting a real block at height 859.
This was referenced Aug 2, 2026
Bitflash-sh
added a commit
that referenced
this pull request
Aug 2, 2026
Restoring from a phrase gave up after the first batch of a hundred addresses that turned up nothing, and the wallet digs a wider gap than that by itself: a restore leaves nHDNext at the depth it scanned, the key pool derives KEYPOOL_SIZE more, and change takes the index after those. Measured on a real wallet with real coin -- restore, spend once, and the outputs land at indices 201 and 302; restoring again found only the first transaction and reported the spend as gone. With -restoredepth=500 the same wallet found both, which is what said this was reach and not loss. Three quiet batches instead of one: 2 transaction(s) across 601 derived addresses on the same wallet, chain and phrase. Worse after #109, not before it: change used to return to an input's key, a low index the scan had already passed. Making change recoverable moved it out to where the scan stopped looking. Does not fix the gap growing with every restore -- pulling nHDNext back to the last used index is the structural answer and needs its own testing.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
MAX_SIZEand disconnect peers that keep an incomplete message pending too long.net-messageself-test for complete, incomplete, stale incomplete, and oversized message headers.Why
A wallet with a recovery phrase can still hold coins on pre-phrase keys. Reusing an input key for change silently moves value back to a key the phrase cannot reproduce. HD wallets should fail to create a transaction rather than create non-recoverable change.
The message parser also had a shared-thread stall: when a header announced more bytes than had arrived, it consumed the header, inserted it back at the front of the receive buffer, slept for 100 ms, and moved on. A peer that drips a large message can make every pass pay that cost and keep the connection alive by continuing to send bytes.
This PR deliberately leaves the block-size consensus cap out. That should be a separate change after a full resync from genesis with the proposed limit.
Testing
git diff --checkmake -f makefile.mingw testsfrom MSYS2 UCRT64 insrc/