Repository navigation
spv: Validate merkle root for all fetched blocks - #2644
Merged
Merged
Conversation
Blocks are now always fetched from SPV peers through a single blocksFromPeer helper, which checks the transaction trees against the merkle root commitments in the header and disconnects the peer when they do not match. This not only removes duplication but also adds the merkle root check to two call sites which were previously missing it (cacheMissingCommitments in discovery.go and handleBlockInvs in sync.go).
Since the merkle root of blocks fetched from SPV peers are now validated immediately, and blocks from a local node are already considered to be trusted, there is no need for the merkle root to be checked again.
davecgh
approved these changes
Aug 5, 2026
| // request. Every block obtained from a remote peer must therefore have | ||
| // its transaction trees checked against the merkle root commitments of | ||
| // the header before the transactions are used for anything. | ||
| err := validate.MerkleRoots(b) |
Member
There was a problem hiding this comment.
While it is not something specifically for this PR since it's effectively just refactoring code that already exists, this probably should be made more intelligent at some point.
It isn't horribly expensive since the standalone funcs its defers to are overall quite efficient, but the specific blocks heights/hashes that DCP0005 activated at are now well known (and even documented in the DCP).
I would expect that it should be faster to determine if the block under test is a descendant of the activation block for the current network and call the right one accordingly.
jrick
approved these changes
Aug 18, 2026
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
3 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.
Blocks are now always fetched from SPV peers through a single
blocksFromPeerhelper, which checks the transaction trees against the merkle root commitments in the header and disconnects the peer when they do not match.This not only removes duplication but also adds the merkle root check to two call sites which were previously missing it (
cacheMissingCommitmentsindiscovery.goandhandleBlockInvsinsync.go).Bug Bounty Note
This fixes a reported vulnerability where the missing merkle root check in
cacheMissingCommitmentscould allow a malicious SPV peer to announce a genuine block byinvand return forged transaction trees under the real header, causing the wallet to permanently record a confirmed transaction that does not exist on chain.In practice this vulnerability is not practical to exploit because: