Consensus: Modified BIP8 implementation#10462
Conversation
|
@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. |
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.
What time? BIP8 is a generic mechanism that can be used for any future deployment.
I think this PR is very serious as it provides the implementation for the current bip8 specification. |
If flag set, transition to LOCKED_IN on timeout. Also Add BIP8 reporting to getblockchaininfo RPC
|
@shaolinfry updated the code to include my proposed modifications to bip8, updated OP and tittle. |
| " },\n" | ||
| " }, ...\n" | ||
| " ],\n" | ||
| " \"bip8_softforks\": { (object) status of BIP9 softforks in progress\n" |
There was a problem hiding this comment.
Typo: should be BIP8 softforks
|
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!
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).
This depends on the concrete deployment, bip8 is general like bip9.
Only if they lock in by bip9, not by bip8 (what this modification covers).
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. 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 master...shaolinfry:bip-uaversionbits |
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: