net: reject headers presync when clock trails chain start#186
Draft
l0rinc wants to merge 3 commits into
Draft
Conversation
Record the current headers presync behavior when the local clock is more than MAX_FUTURE_BLOCK_TIME behind the chain-start MTP. This is not the intended behavior: presync accepts the first low-work header and requests more even though no locally acceptable extension can exist. Mark the assertion with a TODO so the next behavior-changing commit can update it without making the current expectation look intentional.
Capture the local node clock before constructing HeadersSyncState and pass that value through the constructor. This keeps the existing behavior while making the later validation use the same time value as the commitment bound calculation.
Low-work headers presync should not start from a chain-start block whose MTP is already too far in the future for the local clock. Check the chain-start MTP against the captured local clock before constructing HeadersSyncState. The constructor keeps an internal assertion over the same clock value, and the regression test pins the exact MAX_FUTURE_BLOCK_TIME boundary.
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
1 participant
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.
Problem: Headers presync derives its commitment memory bound from the local node clock relative to the known chain-start MTP. If the local clock is more than
MAX_FUTURE_BLOCK_TIMEbehind that chain-start MTP, the elapsed value becomes negative before it is used to derivem_max_commitments. This condition is relative to the known fork point, not genesis.Fix: Reject low-work headers presync before constructing
HeadersSyncStatewhen the captured local clock is earlier thanchain_start_mtp - MAX_FUTURE_BLOCK_TIME. The same capturednowis passed intoHeadersSyncState, where the derived non-negative bound remains asserted. The tests first characterize the existing behavior, then update it to pin the exactMAX_FUTURE_BLOCK_TIMEboundary.