Update BIP9 assignments list #400

pull mappum wants to merge 3 commits into bitcoin:master from mappum:update-bip9-assignments changing 1 files +4 −4

mappum commented at 8:46 AM on June 8, 2016: contributor

This PR updates the BIP9 bit assignments list since the segwit deployment is now active on testnet. It also changes the csv testnet activation height from "770111" to "770112" since that is the first block where the deployment is active.
Fixed CSV testnet state in BIP9 assignments list 08c068ed0b
Updated segwit testnet state in BIP9 assignments list cb2952c05e
luke-jr added the label Proposed BIP modification on Jun 8, 2016
in bip-0009/assignments.mediawiki:None in cb2952c05e outdated
```
  22 | +| 2017-05-01 00:00:00
  23 |  | defined
  24 |  | 2016-03-01 00:00:00
  25 |  | 2017-05-01 00:00:00
  26 | -| active since #770111
  27 | +| active since #770112
```
laanwj commented at 3:11 PM on June 28, 2016:

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.

btcdrak commented at 3:39 PM on June 28, 2016:

Maybe it should say "activated on #770112"
laanwj commented at 4:07 PM on June 28, 2016:

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) is THRESHOLD_LOCKED_IN

VersionBitsBlockState(chainActive[770111], consensusParams, Consensus::DEPLOYMENT_CSV) is THRESHOLD_ACTIVE

VersionBitsBlockState(chainActive[770112], consensusParams, Consensus::DEPLOYMENT_CSV) is THRESHOLD_ACTIVE
btcdrak commented at 4:15 PM on June 28, 2016:

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.

luke-jr commented at 4:40 PM on June 28, 2016:

@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...

laanwj commented at 7:02 AM on June 29, 2016:

I'd suggest to change the text to "enforced since #XXX" to prevent confusion then.

sipa commented at 7:23 AM on June 29, 2016:

@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.

laanwj commented at 7:28 AM on June 29, 2016:

OK. Someone else must have made that mistake too then, otherwise this number wouldn't be one-off in the first place.
in bip-0009/assignments.mediawiki:None in cb2952c05e outdated
```
  31 | @@ -32,6 +32,6 @@ State can be defined, active, failed. Dates are in UTC.
  32 |  | -
  33 |  | 2016-05-01 00:00:00
  34 |  | 2017-05-01 00:00:00
  35 | -| defined
  36 | +| active since #834624
```
laanwj commented at 3:13 PM on June 28, 2016:

Checked that this block # is correct for segwit (but with same nit as above - depending on how we want to count this "since", inclusive or not).
btcdrak commented at 12:19 AM on July 5, 2016: contributor

@mappum You can add CSV activation on mainnet now #419328
Added CSV mainnet activation to BIP9 assignments list 95947c5d77
mappum commented at 4:47 PM on July 5, 2016: contributor

It seems a majority in this thread agree that the activation heights in the document should contain the first block which has state ACTIVE. Can this be merged now?
btcdrak commented at 5:32 PM on July 5, 2016: contributor

@mappum please change "active since #XXX" to "enforced since #XXX" to prevent confusion. Was also suggested by @laanwj here
sipa commented at 5:38 PM on July 5, 2016: member

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.
btcdrak commented at 6:19 PM on July 5, 2016: contributor

@sipa OK then. You should ACK this so it can get merged.
laanwj commented at 8:09 AM on July 8, 2016: member

Yes, let's just merge this. I had the definition wrong (probably my own fault, not the BIP's).
laanwj cross-referenced this on Jul 8, 2016 from issue [Doc] Update bips.md for CSV softfork. by fanquake
sipa commented at 10:31 AM on July 8, 2016: member

ACK
laanwj merged this on Jul 11, 2016
laanwj closed this on Jul 11, 2016

Contributors

Labels

Proposed BIP modification