net: cap future-MTP headers commitments #35208

pull l0rinc wants to merge 1 commits into bitcoin:master from l0rinc:l0rinc/headerssync-future-mtp-cap changing 3 files +19 −6

l0rinc commented at 10:35 AM on May 5, 2026: contributor

Problem

Headers presync computes m_max_commitments from the elapsed time since the chain-start MTP plus MAX_FUTURE_BLOCK_TIME. When the local clock is more than MAX_FUTURE_BLOCK_TIME behind the chain-start MTP, that elapsed value is negative, but it is used in arithmetic assigned to the unsigned commitment cap. This can turn the intended zero bound into a large cap, letting low-work headers presync continue instead of aborting at the first commitment.

Fix

Keep the elapsed-time value signed and assign an explicit zero commitment cap when the chain-start MTP is still too far in the future for a locally acceptable extension to exist. The first stored commitment then exceeds the cap and aborts presync.

The unit test covers this boundary with commitment_period=1, and the headers_sync_state fuzz target now allows the same clock-skew case instead of always choosing a time at or after the chain-start MTP.
DrahtBot added the label P2P on May 5, 2026
DrahtBot commented at 10:36 AM on May 5, 2026: contributor


The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.



Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/35208.



Reviews

See the guideline for information on the review process.

Type Reviewers

ACK dergoegge

If your review is incorrectly listed, please copy-paste <code></code> into the comment that the bot should ignore.



Conflicts

Reviewers, this pull request conflicts with the following ones:
- #35114 (test: NodeClockContext follow-ups by seduless)
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

Type	Reviewers
ACK	dergoegge

net: cap future-MTP headers commitments

Headers presync derives its commitment limit from the signed time elapsed since the chain-start MTP plus `MAX_FUTURE_BLOCK_TIME`.
If the local clock is further behind than that allowance, the elapsed value is negative but the division by the unsigned commitment period converts it to a large unsigned bound.

Assign an explicit zero commitment cap for that case so the first stored commitment aborts presync instead of extending the low-work chain.
Add a regression test with `commitment_period=1` to cover the future-MTP boundary, and keep the `headers_sync_state` fuzz target from excluding the same clock-skew case.

e201bdb74b

in src/test/fuzz/headerssync.cpp:66 in 529757189b

  59 | @@ -60,7 +60,11 @@ FUZZ_TARGET(headers_sync_state, .init = initialize_headers_sync_state_fuzz)
  60 |      CBlockHeader genesis_header{Params().GenesisBlock()};
  61 |      CBlockIndex start_index(genesis_header);
  62 |  
  63 | -    NodeClockContext clock_ctx{ConsumeTime(fuzzed_data_provider, /*min=*/start_index.GetMedianTimePast())};
  64 | +    const int64_t chain_start_mtp{start_index.GetMedianTimePast()};
  65 | +    const NodeSeconds now{fuzzed_data_provider.ConsumeBool() ?
  66 | +                              ConsumeTime(fuzzed_data_provider, /*min=*/chain_start_mtp) :
  67 | +                              NodeSeconds{std::chrono::seconds{chain_start_mtp - MAX_FUTURE_BLOCK_TIME - 1}}};

maflcko commented at 12:49 PM on May 5, 2026:

nit: I think instead of a branch, this can be a plain ConsumeTime call with just the min adjusted to go further back. This makes the fuzz format a bit more static, and the fuzz engine should still be able to figure out the edge case. You may also extend the input space by allowing time further back than one MAX_FUTURE_BLOCK_TIME?

l0rinc force-pushed on May 5, 2026
dergoegge approved
dergoegge commented at 10:32 AM on May 8, 2026: member

utACK e201bdb74b994ebb34999a38612401167b02c4a9

Contributors

Labels

net: cap future-MTP headers commitments #35208

Problem

Fix

Code Coverage & Benchmarks

Reviews

Conflicts