Skip to content

Use ConsensusFactory when parsing block headers - #1335

Merged
NicolasDorier merged 1 commit into
MetacoSA:masterfrom
walkjivefly:fix/factory-aware-block-parsing
Sep 17, 2026
Merged

NicolasDorier merged 1 commit into
MetacoSA:masterfrom
walkjivefly:fix/factory-aware-block-parsing

Conversation

@walkjivefly

Copy link
Copy Markdown
Contributor

Summary

While reviewing the code to gain familiarity with NBitcoin I noticed what appear to be some oversights in the use of ConsensusFactory.

ConsensusFactory was introduced in 6cb308a but wasn't applied everywhere it could be. Some missed uses were corrected in 5f7c433c.

I found 3 additional missed cases in header parsing:
- BlockHeader.Parse selected a factory but directly constructed a base BlockHeader.
- raw RPC header parsing created the correct header type but did not preserve the network factory during deserialization.
- REST header parsing directly constructed base headers and split the response into fixed 80-byte chunks.

Changes

- Create statically parsed headers through ConsensusFactory.CreateBlockHeader() and deserialize them with the same factory.
- Preserve the client's network factory while parsing raw RPC headers.
- Parse REST header responses sequentially using factory-created headers, allowing custom header implementations to determine their serialized length.
- Reject truncated REST header data and guard against custom deserializers that consume no input.

No public API signatures are changed, and standard Bitcoin header serialization and parsing remain unchanged.

Tests

Added regression coverage using an artificial test-only ConsensusFactory, custom BlockHeader, and custom Block. The custom header includes an extended serialized field and a nested factory-created value, which verifies both concrete type creation and propagation of the factory into deserialization.

Post-rebase test results:

- focused ConsensusFactoryTests: 10 passed, 0 failed
- complete NBitcoin.Tests Release/net10.0 suite: 752 passed, 0 failed

@NicolasDorier
NicolasDorier merged commit 64c8c56 into MetacoSA:master Sep 17, 2026
6 checks passed
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.

2 participants