Skip to content
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

Added clarifications to BIP0009. #219

Merged
merged 2 commits into from Mar 1, 2016

Conversation

CodeShark
Copy link
Contributor

I added explicit parameters for deployment at the beginning of the specification. I think this greatly clarifies the spec for someone approaching it for the first time.

We've decided for now not to consensus-enforce the bit states, making the RFC language that had been previously proposed a bit superfluous. The only thing that remains consensus-enforced is the moment of activation which should be clear from context.

@CodeShark CodeShark force-pushed the BIP0009_revisions branch 3 times, most recently from 9383add to e76d16b Compare October 11, 2015 15:11
Each soft fork deployment is specified by the following parameters (further elaborated below):

# The '''bit''' determines which bit in the nVersion field of the block is to be used to signal the soft fork lock-in and activation.
# The '''threshold''' specifies how many blocks within a single retarget period (2016 blocks) MUST have the bit set before we lock in the deploment.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/deploment/deployment/

@CodeShark CodeShark force-pushed the BIP0009_revisions branch 2 times, most recently from 224623c to ee8b3c3 Compare October 11, 2015 15:21
@btcdrak
Copy link
Contributor

btcdrak commented Oct 11, 2015

ACK

more of the 2016 blocks within a retarget period, it is considered
''locked-in''. Miners should continue setting bit B, so uptake is
visible.
For each retarget interval, the number of blocks in which bit B is set is counted. If bit B is set in at least some threshold number of blocks (1916 on mainnet, 1512 on testnet) of the 2016 blocks within a retarget period, it is considered ''locked-in''. Miners SHOULD continue setting bit B so uptake is visible.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MUST

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it MUST? I thought we agreed that until activation it was still considered optional or else lock-in itself would be a soft fork.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Eric Lombrozo notifications@github.com writes:

 if (BState != activated && BState != failed) {
     SetBInBlock();
 }

'''Success: Lock-in Threshold'''
-If bit B is set in 1916 (1512 on testnet) or
-more of the 2016 blocks within a retarget period, it is considered
-''locked-in''. Miners should continue setting bit B, so uptake is
-visible.
+For each retarget interval, the number of blocks in which bit B is set is counted. If bit B is set in at least some threshold number of blocks (1916 on mainnet, 1512 on testnet) of the 2016 blocks within a retarget period, it is considered ''locked-in''. Miners SHOULD continue setting bit B so uptake is visible.

Is it MUST? I thought we agreed that until activation it was still considered optional or else lock-in itself would be a soft fork.

No, that's enforcement. Setting the bit is compulsory, and was the
intent of the last BIP change.

Cheers,
Rusty.

@luke-jr
Copy link
Member

luke-jr commented Oct 16, 2015

Probably it should be clarified that even the strongest "rules" here are ultimately up to the softforking BIP, and can in theory be overridden?

@CodeShark
Copy link
Contributor Author

I removed the RFC language for now but kept in the parameters at the beginning of the specification, which IMO was the more important change I had made.

@CodeShark CodeShark force-pushed the BIP0009_revisions branch 2 times, most recently from f69462f to 6c0d9c9 Compare October 19, 2015 03:30
@andre-amorim
Copy link

your money means nothing for bitcoin XT

On 19 October 2015 at 04:16, Eric Lombrozo notifications@github.com wrote:

I removed the RFC language for now but kept in the parameters at the
beginning of the specification, which IMO was the more important change I
had made.


Reply to this email directly or view it on GitHub
#219 (comment).

@CodeShark
Copy link
Contributor Author

I like the idea of retaining the flexibility to specify a different threshold for each fork or to change it for all forks in the future. The way I see it, the thresholds used for mainnet and testnet are recommendations.

@rustyrussell
Copy link
Contributor

Eric Lombrozo notifications@github.com writes:

I like the idea of retaining the flexibility to specify a different threshold for each fork or to change it for all forks in the future. The way I see it, the thresholds used for mainnet and testnet are recommendations.

Sure, you could do that but you'll break everyone's warning system.
And it's hard enough to propose a good soft fork without worrying about
even more parameters.

Cheers,
Rusty.

@CodeShark
Copy link
Contributor Author

I'm not saying we should be changing these parameters arbitrarily on an ad hoc basis - I just think the flexibility costs nothing and it makes the spec more clear...and the BIP document even talks about potentially modifying thresholds in the future, so might as well have that flexibility from the beginning.

@rustyrussell
Copy link
Contributor

Eric Lombrozo notifications@github.com writes:

I'm not saying we should be changing these parameters arbitrarily on an ad hoc basis - I just think the flexibility costs nothing and it makes the spec more clear...and the BIP document even talks about potentially modifying thresholds in the future, so might as well have that flexibility from the beginning.

I don't understand?

It's just a document, you can take or leave it. It does describe some
of the considerations if you decide to do something different.

But the way these things actually work is:

  1. Grumpy engineer who has better things to do gets referred to document
    as another hurdle they have to jump before finishing what they're
    trying to do.
  2. They skim the document to see if they have to bother.
  3. If it clearly, says they have to they read the minimum amout to see
    what they need.

So, yeah, the default language says "DO THIS. THEN THIS. THEN THIS.
DONE". Because I guarantee that nobody will read the rest of it.

Cheers,
Rusty.

@CodeShark CodeShark force-pushed the BIP0009_revisions branch 2 times, most recently from fd45a90 to 64fda4e Compare November 17, 2015 12:28
@luke-jr
Copy link
Member

luke-jr commented Nov 28, 2015

@btcdrak This needs an ACK from at least one of @sipa @petertodd @gmaxwell @rustyrussell to be merged.

@sipa
Copy link
Member

sipa commented Nov 28, 2015

NAK, seems controversial (the consensus logic affected by a start date).

@CodeShark
Copy link
Contributor Author

@sipa I thought we agreed that a start time is needed. How do you propose we accomplish this, then?

@sipa
Copy link
Member

sipa commented Nov 28, 2015

See @rustyrussell's comment and the discussion above.

@CodeShark
Copy link
Contributor Author

@sipa regardless, the thrust of this PR was actually adding the deployment parameters to the top of the spec in an easy-to-reference list. I removed the start time, then readded it after talking to you about it and you agreeing it was necessary.

I'll get rid of the start time, if nobody wants it.

@sipa
Copy link
Member

sipa commented Nov 28, 2015

I'm not the only one to decide that (anymore).

@CodeShark
Copy link
Contributor Author

Can we at least merge this for now pending resolution of the controversy?

@rustyrussell
Copy link
Contributor

Ack. Specifying the parameters up front is nice (though in practice the BIP recommends specific threshold and timeout values).

@luke-jr
Copy link
Member

luke-jr commented Jan 8, 2016

@sipa Do you still NACK this, or can it be merged?

@sipa
Copy link
Member

sipa commented Feb 29, 2016

ACK

@sipa
Copy link
Member

sipa commented Feb 29, 2016

@rustyrussell Regarding the previous point brought up in this PR, I'd very much like to argue in favor of a start date regardless that is part of the BIP9 consensus logic. The reason is that:

  • Experience has shown that some miners and other nodes do in fact run pre-release code, and this does cause problems.
  • The suggestion of not setting a bit until a certain date leads to ambiguous behaviour (for example, the warning logic would need to be more complicated and take bits that you wouldn't set yourself into account).
  • Using a start and end date is a much cleaner way of avoiding bits that depend on each other than defining them recursively (if that's ever needed).

luke-jr added a commit that referenced this pull request Mar 1, 2016
@luke-jr luke-jr merged commit 2b27bb5 into bitcoin:master Mar 1, 2016
@jtimon
Copy link
Contributor

jtimon commented Mar 1, 2016

Mhmm, I thought that we wanted a star time per deployment, but not a different activation threshold per development (the bip implicitly states that there's at least one per chain, by specifying testnet's threshold).

@rustyrussell
Copy link
Contributor

Pieter Wuille notifications@github.com writes:

@rustyrussell Regarding the previous point brought up in this PR, I'd
very much like to argue in favor of a start date regardless that is
part of the BIP9 consensus logic. The reason is that:

  • Experience has shown that some miners and other nodes do in fact run
    pre-release code, and this does cause problems.

OK, I respectfully disagree, but I'm often wrong :)

  • Using a start and end date is a much cleaner way of avoiding bits that depend on each other than defining them recursively (if that's ever needed).

I look forward to reading your BIP patch which shows BIP authors how to
choose the start date.

Perhaps we should make this the only decision, and make "end date =
start date + 3 years" (ie. 94608000 seconds)? That still gives us
failed softfork every 6 weeks still.

Thanks,
Rusty.

@btcdrak
Copy link
Contributor

btcdrak commented Mar 8, 2016

@rustyrussell BIP65 was deployed by 20% of the hash power before the stable release.

I am very much for a start date and I dont think it needs needs to be complicated. The idea is simply to set the start date reasonably to a time after you plan a stable release.
NACK against automatically setting the end date. That would create a lack of flexibility and for the time being it's clear 3 year expiry is way too long considering all softforks have been enforced in just a few months. Unless mining decentralises a bit I see no reason for it to take longer unless they actually do reject it, in which case a year is probably a good window.

tl;dr, The start and end dates should be configurable per softfork. The way it is implemented in core PR 7575 is ideal.

@NicolasDorier
Copy link
Contributor

I slightly prefer having possibility to set startdate and enddate manually.

If I understand @rustyrussell point, this is about not having to think about more parameters during the soft fork creation. But even if there was no startdate to configure programmatically in BIP9, the startdate will need to be debated anyway by deciding when the soft fork will be merged.

At the end of the day, I don't think this matter a lot.

@rustyrussell
Copy link
Contributor

฿tcDrak notifications@github.com writes:

@rustyrussell BIP65 was deployed by 20% of the hash power before the stable release.

I know; and that's a long way from 95%. But I've already conceded the
start date.

I am very much for a start date and I dont think it needs needs to be complicated. The idea is simply to set the start date reasonably to a time after you plan a stable release.

As a BIP author, this means I need to know when it's likely to be
merged, and what the release cycle timing will be. So some advice about
how to select this would be nice. Perhaps suggest aiming for 3 months
after an expected production release?

NACK against automatically setting the end date. That would create a lack of flexibility and for the time being it's clear 3 year expiry is way too long considering all softforks have been enforced in just a few months. Unless mining decentralises a bit I see no reason for it to take longer unless they actually do reject it, in which case a year is probably a good window.

OK, let's make it a year then, but let's choose something for BIP9.
You could change it for each BIP, just like you could change the
thresholds, but why would one softfork choose 1 year and one choose 3
years? How would I, as a random BIP author, tell?

We can always have a new BIP which changes that advice if necessary, but
I feel that punting to each BIP author is reneging on our
responsibilities.

Cheers,
Rusty.

@btcdrak
Copy link
Contributor

btcdrak commented Mar 9, 2016

@rustyrussell I agree BIP9 could recommend some sane defaults. In any case, each softfork BIP already must specify the exact deployment details, for example which bit they intend to use. A typical deployment section for a BIP9 deployment might say "This will be deployed using BIP9 with bit 5 set with a start date of 2016-04-01 00:00:00 and a timeout of 1 year".

I would also point out that draft softfork BIPs usually have the deployment section blank as "TBD" until exact deployment methodology is known (which is the case for the current BIP68,112 and 113).

Additionally, we've tended to separate the softfork feature patch and the deployment code. In fact sometimes it's even been necessary to release mempool only code first as was the case of BIP113.

How about wording for BIP9 something to the effect of:-

"It is recommended that the '''starttime''' be at least 1 month after expected production release of the softfork proposal, with a '''timeout''' of 1 year."

@rustyrussell
Copy link
Contributor

฿tcDrak notifications@github.com writes:

How about wording for BIP9 something to the effect of:-

"It is recommended that the '''starttime''' be at least 1 month after expected production release of the softfork proposal, with a '''timeout''' of 1 year."

OK, here's a patch (also available at
https://github.com/rustyrussell/bips/tree/bip9-recommendations ) which
makes the recommendations more explicit. If people like it after
bikeshedding, I'll open a new PR:

(Moved to a separate section immediately following ==Specification==):

===Selection guidelines===

The following guidelines are suggested for selecting these parameters for a soft fork:

'''bit''' should be selected such that no two concurrent softforks use the same bit.

'''starttime''' should be set to some date in the future, approximately one month software release date including the soft fork. This allows for some release delays, while preventing triggers as a result of parties running pre-release software.

'''timeout''' should be 1 year (31536000 seconds) after starttime.

A later deployment using the same bit is possible as long as the starttime is after the previous one's
timeout or activation, though it is recommended to have a pause in between to detect buggy software.

Cheers,
Rusty.

@sipa
Copy link
Member

sipa commented Mar 10, 2016 via email

@morcos
Copy link
Member

morcos commented Mar 10, 2016

I think we should recommend that a previously unused bit is used if available, otherwise it becomes a bit annoying in regtest to figure out how to activate both soft forks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
9 participants