-
Notifications
You must be signed in to change notification settings - Fork 5.3k
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
BIP8: directly support special cases #1023
Conversation
bip-0008.mediawiki
Outdated
} else { | ||
return FAILED; | ||
} | ||
} else if (block.height >= startheight) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could also make it:
} else if (block.height >= startheight) {
if (lockinontimeout && block.height + 2016 >= timeoutheight) {
return MUST_SIGNAL;
} else {
return STARTED;
}
}
which would also remove the edge case from #1019.
Rather than always having the genesis block be DEFINED, allow {0,0,true} to act as ALWAYS_ACTIVE and {0,0,false} to act as NEVER_ACTIVE, and {n,n,true} to act as a buried deployment.
c2d4b38
to
18b6e36
Compare
I wonder if this is needed in the BIP, or just implementation details that complicate the BIP. |
Fair question. I think it makes the special cases ("what if timeout is less than start? what if they're not equal but in the same retarget phase?") a bit clearer, without complicating the BIP very much. It also might let the implementation follow the BIP pseudocode more exactly which might make it easier to see both are correct. |
|
||
case DEFINED: | ||
if (block.height >= startheight) { | ||
return STARTED; | ||
if (block.height >= timeoutheight) { | ||
return (lockinontimeout ? ACTIVE : FAILED); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The genesis block handling is slightly better than before, but imo this is more difficult to understand than the current "timoutheight must be 2 retarget periods after startheight".
Closing this -- it only covers cases that can't reasonably arise in practice on either mainnet or testnet, and can be treated as an implementation detail elsewhere. |
Rather than always having the genesis block be DEFINED, allow {0,0,true} to act as ALWAYS_ACTIVE and {0,0,false} to act as NEVER_ACTIVE, and {n,n,true} to act as a buried deployment.
This cleans up the implementation a bit I think, removing the need for special case handling. (Note that in the {n,n,true} case n will still be rounded up to the next retarget period however)