Skip to content

Conversation

jtimon
Copy link
Contributor

@jtimon jtimon commented Apr 10, 2015

To extend libconsensus' functionality, I propose to add bitcoinconsensus_verify_header() the partial checks CheckBlockHeader() and ContextualCheckBlockHeader could be exposed as well.

Unlike VerifyScript, VerifyBlockHeader depends on the nodes storage, as it requires data from the blockchain previous to the header being verified. How to exactly interface that data with VerifyBlockHeader can be a delicate matter and it should be of course open to discussion. Here there's just one proposal.

The offending dependency is CBlockIndex in chain.o, specially CBlockIndex::pprev which is a pointer to another object. All the other dependencies unacceptable for libconsensus have been removed first, to make things easier for someone who wants to propose to solve that final problem in some other way.

Dependencies:
-MOVEONLY: Move most consensus function declarations to consensus/consensus.h #6048
-Consensus: Refactor: Consensus version of CheckBlockHeader() #6035
-Separate CValidationState from main #5669
-Chainparams: Refactor: Decouple IsSuperMajority from Params() #5968
-Consensus: Decouple ContextualCheckBlockHeader from checkpoints #5975
-Consensus: Turn CBlockIndex::GetMedianTimePast into independent function #6009
-Consensus: Consensus version of pow functions #6037

@jtimon jtimon changed the title DEPENDENT: Consensus: API: Expose bitcoinconsensus_verify_header() in libconsensus DEPENDENT: API: Expose bitcoinconsensus_verify_header() in libconsensus Apr 10, 2015
@jtimon jtimon force-pushed the consensus_header branch 5 times, most recently from 75b466c to 3e23de9 Compare April 13, 2015 17:14
@jtimon jtimon force-pushed the consensus_header branch 2 times, most recently from e6adb85 to f086046 Compare April 15, 2015 12:04
@jtimon
Copy link
Contributor Author

jtimon commented Apr 15, 2015

Needed rebase


#include <algorithm>

static const unsigned int MEDIAN_TIME_SPAN = 11;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMHO this could get a comment.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feel free to suggest something. Probably better to comment in the smaller PRs for now as this branch will likely require more rebases and force pushes. In this case, the relevant PR is #6009

@jtimon jtimon force-pushed the consensus_header branch from 7e0cd14 to 00b9b22 Compare April 20, 2015 19:38
@jtimon
Copy link
Contributor Author

jtimon commented Apr 22, 2015

I realized this is mostly unreadable until something similar to #6048 is merged.
I will close this for now but I will still maintain the list of dependencies.

@jtimon jtimon closed this Apr 22, 2015
@jtimon jtimon mentioned this pull request Oct 19, 2015
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants