Conversation
| | 2016-03-01 00:00:00 | ||
| | 2017-05-01 00:00:00 | ||
| | active since #770111 | ||
| | active since #770112 |
There was a problem hiding this comment.
Whether this should be #770111 or #770112 depends on how the column is defined.
You're right that the block mining on top of block #770111 is the first for which the CSV softfork rules are activated, which is block #770112.
There was a problem hiding this comment.
Maybe it should say "activated on #770112"
There was a problem hiding this comment.
it's #770111 that changes the state to "active", remember that it checks the state for previous block to see what rules to enforce. #770112 is the second block that has "active" state, but the first for which the rules are enforced.
I tend to agree that mentining #770112 seems more intuitive, but this is a more subtle distinction than most people take into account :) Very easy to introduce off-by-one errors here.
To be precise:
VersionBitsBlockState(chainActive[770110], consensusParams, Consensus::DEPLOYMENT_CSV)isTHRESHOLD_LOCKED_INVersionBitsBlockState(chainActive[770111], consensusParams, Consensus::DEPLOYMENT_CSV)isTHRESHOLD_ACTIVEVersionBitsBlockState(chainActive[770112], consensusParams, Consensus::DEPLOYMENT_CSV)isTHRESHOLD_ACTIVE
There was a problem hiding this comment.
In that case "active since #770112" is correct, which was my original thinking. Alternatively "enforced since #770112" would make the distinction that #770112 is the first block on which the new rules are applied.
There was a problem hiding this comment.
@laanwj Sounds like you're talking about implementation details there... If #770112 is the first block the rules are enforced in, how nodes implement that logic is not really relevant...
There was a problem hiding this comment.
I'd suggest to change the text to "enforced since #XXX" to prevent confusion then.
There was a problem hiding this comment.
@laanwj You're calling the function incorrectly. The first argument to VersionBitsBlockState is pindexPrev. Using BIP9 terminology, block 770112 is the first which has state ACTIVE.
There was a problem hiding this comment.
OK.
Someone else must have made that mistake too then, otherwise this number wouldn't be one-off in the first place.
|
@mappum You can add CSV activation on mainnet now #419328 |
|
It seems a majority in this thread agree that the activation heights in the document should contain the first block which has state |
|
I think 'active' is perfect, as it is the name of the state in BIP9. If
there is confusion about what active means, we should clarify the BIP
itself.
|
|
@sipa OK then. You should ACK this so it can get merged. |
|
Yes, let's just merge this. I had the definition wrong (probably my own fault, not the BIP's). |
|
ACK |
5 participants
This PR updates the BIP9 bit assignments list since the
segwitdeployment is now active on testnet. It also changes thecsvtestnet activation height from "770111" to "770112" since that is the first block where the deployment is active.