Consensus: Modified BIP8 implementation #10462

This is @shaolinfry 's implementation of bip8, but rebased, with some minor nits by myself freely applied, some squashes and with some sensible renames left out for later/before (whatever is decided, that's just self-documentation; orthogonal. Didn't separate s/BIP9SoftForkDescPushBack/VBSoftForkDescPushBack/ because I don't think it's worth it: it helps for review IMO and it's minimal).

Please no merge before we decide what to do with my new TODO comment lines (I want to add something to bip8 for warnings), but I think review for bip8 as it is now can begin at any time. Kudos again to @shaolinfry not just for the idea but also for the cleanness: I enjoyed reviewing and rebasing this.

EDIT:

This modifies the original BIP specification to enable warnings for unkown deployments being locked in by bip8 timeout as proposed in https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2017-May/014417.html

Dependencies:

• Introduce static DoWarning (simplify UpdateTip) #10464

@jtimon It was not squashed because that harms review, people need to see each atomic change. Squash can be trivial after the code review. FWIW I have deliberately held back on any PRs to Bitcoin Core until nearer the time. I don't think "NOMERGE" pull requests are helpful, better to just have a serious pull request.

It was not squashed because that harms review, people need to see each atomic change. Squash can be trivial after the code review.

I can separate it back, but I disagree that having this in a single commit hurts review or that the commit doesn't represent an atomic change. And it's still quite simple.

FWIW I have deliberately held back on any PRs to Bitcoin Core until nearer the time.

What time? BIP8 is a generic mechanism that can be used for any future deployment.

I don't think "NOMERGE" pull requests are helpful, better to just have a serious pull request.

I think this PR is very serious as it provides the implementation for the current bip8 specification. I added "NOMERGE" because of the TODOs, because I want bip8's specification to change (and then change this PR to add the suggested addition), please, see https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2017-May/014417.html

@shaolinfry updated the code to include my proposed modifications to bip8, updated OP and tittle. If you like the idea, I'm happy to help modifying the specification, otherwise I guess I will create a competing an extension one.

This modification doesnt make sense and I already went through a similar iteration during development.

The warning system will already notify users of unknown bits being signalled, and considering UASF deployment will be for a year. For upgraded clients they will also additionally emit warnings once LOCKED_IN has been reached. Unknown bits is also backwards compatible with BIP9 aware nodes.

Asking miners to set a bit in order to trigger warnings seems fraught with trouble as well as for false triggering. I dont think it would work for parallel deployments.

Soft forks are always deployed with considerable planning, and ecosystem collaboration, we cannot rely just on technical means, and actually, too many error messages or false warning just numbs users into ignoring them. We already say that happen with the chain split detection system which was removed a while ago.

Thanks for the review!

The warning system will already notify users of unknown bits being signalled

Only if the same bit is active for 95% of a diff adjustment period (ie only when an unkown bip9 deployment seems to have locked in).

and considering UASF deployment will be for a year.

This depends on the concrete deployment, bip8 is general like bip9.

For upgraded clients they will also additionally emit warnings once LOCKED_IN has been reached. Unknown bits is also backwards compatible with BIP9 aware nodes.

Only if they lock in by bip9, not by bip8 (what this modification covers).

Asking miners to set a bit in order to trigger warnings seems fraught with trouble as well as for false triggering. I dont think it would work for parallel deployments.

If they don't signal, the warning after lock in via bip8 won't be shown, but that's no worse than the current bip8 without this modification. False signaling is more concerning, probably the code should allow for bip8 warnings to be disabled optionally. False signaling is also possible today (although requires 95% to trigger the warning), so perhaps we should also add an option to disable bip9 warnings.

I think there's little incentive for miners to false signal here (or with plain bip9) though.

Closing. I will maybe reopen when @shaolinfry opens his own PR with https://github.com/bitcoin/bitcoin/compare/master...shaolinfry:bip-uaversionbits