Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

blockchain: Separate full data context checks. #1509

Merged

Conversation

davecgh
Copy link
Member

@davecgh davecgh commented Nov 2, 2018

This separates the block and header context checks to make it more explicit exactly which checks require the availability of the full data for all ancestors and which checks do not.

Currently, all ancestor data is always available due to the way the download and block processing logic is implemented, however, ultimately, the plan is to decouple the chain processing and connection code from the download logic. In order to help pave the way towards that goal, this more clearly delineates the checks by redefining the current meaning of the concept of contextual checks and creating a new concept of positional checks.

Positional checks are defined as those which depend on the position of a block or header within the block chain and have all of the ancestor block headers available, but do NOT have all of the full block data for
all of the ancestor blocks.

Contextual checks are now defined as those which depend on having the full block data for all of the ancestors available, which also implies all of the ancestor block headers are available too.

To that end, this introduces two new functions named checkBlockHeaderPositional and checkBlockPositional, moves all of the checks which adhere to the aforementioned semantics to the new
functions, and updates all call sites which invoke the contextual variants to call the positional variants first.

@davecgh davecgh added this to the 1.4.0 milestone Nov 2, 2018
@davecgh davecgh force-pushed the blockchain_separate_full_data_context_checks branch from db4eef5 to a3f66e5 Compare November 2, 2018 01:28
blockchain/accept.go Outdated Show resolved Hide resolved
blockchain/chain.go Outdated Show resolved Hide resolved
@davecgh davecgh force-pushed the blockchain_separate_full_data_context_checks branch 2 times, most recently from caf78f7 to 230c127 Compare November 2, 2018 17:37
This separates the block and header context checks to make it more
explicit exactly which checks require the availability of the full data
for all ancestors and which checks do not.

Currently, all ancestor data is always available due to the way the
download and block processing logic is implemented, however, ultimately,
the plan is to decouple the chain processing and connection code from the
download logic.  In order to help pave the way towards that goal, this
more clearly delineates the checks by redefining the current meaning of
the concept of contextual checks and creating a new concept of
positional checks.

Positional checks are defined as those which depend on the position of a
block or header within the block chain and have all of the ancestor
block headers available, but do NOT have all of the full block data for
all of the ancestor blocks.

Contextual checks are now defined as those which depend on having the
full block data for all of the ancestors available, which also implies
all of the ancestor block headers are available too.

To that end, this introduces two new functions named
checkBlockHeaderPositional and checkBlockPositional, moves all of the
checks which adhere to the aforementioned semantics to the new
functions, and updates all call sites which invoke the contextual
variants to call the positional variants first.
@davecgh davecgh force-pushed the blockchain_separate_full_data_context_checks branch from 230c127 to 1dece72 Compare November 8, 2018 00:21
@davecgh davecgh merged commit 1dece72 into decred:master Nov 8, 2018
@davecgh davecgh deleted the blockchain_separate_full_data_context_checks branch November 8, 2018 00:27
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.

3 participants