Skip to content

Conversation

@luke-jr
Copy link

@luke-jr luke-jr commented Jun 27, 2017

No description provided.

// Enforce rule that the coinbase starts with serialized block height
if (nHeight >= consensusParams.BIP34Height)
{
if (block.nTime >= consensusParams.HardforkTime) {
Copy link

Choose a reason for hiding this comment

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

Why not use HardforkHeight instead ?

Copy link
Author

Choose a reason for hiding this comment

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

This is just part of a larger hardfork. The height isn't known at header-validation time.

Copy link

Choose a reason for hiding this comment

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

how is the height not known at header-validation time? Don't you know the height of your predecessor block?

Copy link
Author

Choose a reason for hiding this comment

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

I'm referring to CheckBlockHeader, not ContextualCheckBlockHeader. This would mean we cannot verify the PoW until we have the block context, which could possibly (?) make a DoS vector. (But maybe some restructuring of the code to ignore all headers without context makes sense?)

Copy link
Author

Choose a reason for hiding this comment

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

In any case, there's little harm to using time here. This is intended to only be deployed after the final SHA2 block is found, which means there is a very well-defined time to specify (in fact, PR #2 requires the time specified is exactly the second after the final SHA2 block). No real benefit to using height.

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