Skip to content

Conversation

@luke-jr
Copy link
Member

@luke-jr luke-jr commented Mar 30, 2016


Each soft fork deployment is specified by the following per-chain parameters (further elaborated below):

# The '''name''' specifies a very brief description of the soft fork, reasonable for use as an identifier.
Copy link
Contributor

Choose a reason for hiding this comment

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

I would suggest the for most of the time, most softforks will role out one consensus BIP so the sensible name would be bipxxxx unless you have a case like with the bip68+112+113 softfork, in which case we labelled that csv. Therefore I would suggest default of bipxxxx unless it's a bundle.

Thinking more, we probably need a table of soft forks somewhere with name, bip(s), starttime, timeout.

Copy link
Member Author

Choose a reason for hiding this comment

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

This is outside the scope of this PR, IMO.

@sipa
Copy link
Member

sipa commented Apr 5, 2016

The nVersion field of a block never affects the rules applied, so a miner can always be free to set the nVersion to anything.

@luke-jr
Copy link
Member Author

luke-jr commented Apr 5, 2016

@sipa Which part are you referring to? GBT servers may very well want to restrict the nVersion to things they can handle.

@sipa
Copy link
Member

sipa commented Apr 5, 2016

I had interpreted "the server requires support for" as rules the server demanded you to enforce, rather than rules the server demands you to set bits for. It's a bit confusing that rules and rulesenforced are about consensus rules, and rulesrequires is about nVersion bits to be set. Perhaps replace rulesrequired with versionrequired, with an integer of bits the server requires you to set in nversion?

@luke-jr
Copy link
Member Author

luke-jr commented Apr 5, 2016

I would have grouped "rules" and "rulesrequired" together, since they deal with bits available/allowed and required; vs "rulesenforced" which is the actual consensus rules being used. Maybe "vbits" and "vbitsrequired"?

@jl2012
Copy link
Contributor

jl2012 commented Apr 5, 2016

ACK BIP141

@sipa
Copy link
Member

sipa commented Apr 5, 2016

@luke-jr vbits would just be the same as version, I think?

Isn't this sufficient:

  • vbrequired: bits in nVersion that the server demands to be set (so an integer)
  • rules: list of strings of deployments whose rules are in effect

?

@luke-jr
Copy link
Member Author

luke-jr commented Apr 5, 2016

@sipa How will clients know what bits are available and what they mean in context?

@sipa
Copy link
Member

sipa commented Apr 5, 2016

@luke-jr What do you mean by "available"?

@luke-jr
Copy link
Member Author

luke-jr commented Apr 5, 2016

@sipa Bits which have a defined meaning.

@sipa
Copy link
Member

sipa commented Apr 5, 2016

@luke-jr Why does that matter? If the bits are not active, they have no relevance for mining the block, as there are never any consensus rules in BIP9 that prevent you from setting any combination at all of version bits. The server may insist that you set some to vote for, but apart from that, that information is not relevant to the client.

@luke-jr
Copy link
Member Author

luke-jr commented Apr 5, 2016

The client needs the information to know which bits it wants to or doesn't want to set.

@sipa
Copy link
Member

sipa commented Apr 5, 2016 via email

@luke-jr
Copy link
Member Author

luke-jr commented Apr 5, 2016

So put each and every BIP into the GBT client, even before it has activated?

Consider that the server trusted for name:bit mappings could very well be your own local bitcoind.

@luke-jr
Copy link
Member Author

luke-jr commented Apr 5, 2016

Also, even if each BIP were put into the GBT client, the client doesn't have the required blockchain context to determine if the BIP has activated or is still pending.

@sipa
Copy link
Member

sipa commented Apr 5, 2016

If the client software doesn't know about a BIP, why does it care?

How does it help that the server says "these arbitrary string names exist",
if the client doesn't know what protocol changes they correspond to? If
they care about the rules, they know the rule's name too.

@sipa
Copy link
Member

sipa commented Apr 5, 2016

@luke-jr Yes, saying with rules are active is useful. Saying which exist
isn't. They're just arbitrary strings with no meaning to the client. If
they do know about them, they don't need to be told.

@luke-jr
Copy link
Member Author

luke-jr commented Apr 6, 2016

For example, a miner might want to use --veto-vbits=bip109 to instruct their software to automatically unset the BIP 109 bit, and reject pools that require it. Or --require-vbits=bip109 for the opposite.

@sipa
Copy link
Member

sipa commented Apr 6, 2016

@luke-jr What if the server doesn't know about a particular BIP yet, but the miner does? If the miner has an opinion about a particular BIP/deployment, they also know what bits to use.

@luke-jr
Copy link
Member Author

luke-jr commented Apr 6, 2016

If the server doesn't support it, it shouldn't be indicating it does regardless.

@sipa
Copy link
Member

sipa commented Apr 25, 2016

The changes in BIP22 are just formatting?

@luke-jr
Copy link
Member Author

luke-jr commented Apr 25, 2016

Yes, someone noticed the {{templates}} were not rendering on GitHub.

@jl2012
Copy link
Contributor

jl2012 commented Apr 25, 2016

ACK 141

@theuni
Copy link
Member

theuni commented May 5, 2016

Concept ACK on GBT changes. It's a bit hazy though as to what the server should do if a softfork rule is missing from the request, and there's the possibility of altering the block (potentially in a nasty way, like coinbase-only) to make it compatible. In segwit's case, would the lack of a "segwit" rule in the request cause an error post-activation, or would witness transactions be filtered out?

@luke-jr
Copy link
Member Author

luke-jr commented May 6, 2016

It's a bit hazy though as to what the server should do if a softfork rule is missing from the request, and there's the possibility of altering the block (potentially in a nasty way, like coinbase-only) to make it compatible. In segwit's case, would the lack of a "segwit" rule in the request cause an error post-activation, or would witness transactions be filtered out?

The server has a few options:

  • Filter out witness transactions and use rules/force
  • Produce the full witness-enabled template, and do not use rules/force. The client in this case must refuse to proceed unless it can handle segwit (listed in "rules")
  • Throw an error

@theuni
Copy link
Member

theuni commented May 7, 2016

Right, and that ambiguity is kinda nasty without a proposed implementation. I'd go as far as saying that it should never throw an error if there's the possibility of somehow creating a valid block. Maybe another input flag could be added for "fail for degraded blocks".

Regardless, this needs to be laid out in the BIP.

@luke-jr
Copy link
Member Author

luke-jr commented May 7, 2016

Which method a server uses in this situation is implementation-specific, not part of the standard. In any case, it is already documented clearly here: https://github.com/bitcoin/bips/pull/365/files#diff-6a9961b118279d111faf86ad6ebbfedbR204

Please feel free to proposed improved language.

@theuni
Copy link
Member

theuni commented May 7, 2016

NACK that. Where preferences can be communicated, they should be, and they should be respected by the server. Otherwise what ends up being "part of the standard" is the default behavior of the leading implementation.

@luke-jr
Copy link
Member Author

luke-jr commented May 7, 2016

I don't understand what you're NACKing.

@theuni
Copy link
Member

theuni commented May 7, 2016

I'm NACKing the language that makes the behavior implementation-specific. I'm suggest changes to either:

  • Add a rules/degraded value to the template, so that the miner is aware that the block has been downgraded in some way as a result of a missing input rule. This is different from the rules/force key, in that the latter does not necessarily mean that the block has been degraded.
    or
  • Add an optional "error_degraded" key to the request that signals that the server should not try to degrade the block as a result of a missing input rule, but throw an error instead.

Along with the addition of text along the lines of Unless signaled (by whichever mechanism above), the server should always attempt to provide the client with a template that will allow the client to create a valid block, degraded only as much as is necessary. Only in the case where no solution from the client would be valid should the server return an error.

With those changes, it would be clear what the implementation for Core should look like.

@luke-jr
Copy link
Member Author

luke-jr commented May 7, 2016

Any item in "rules" the client does not understand implies it must error or (if "rules/force" is allowed) "is degraded".

@theuni
Copy link
Member

theuni commented May 7, 2016

Force (as I read it) doesn't necessarily mean degraded. It could simply mean that a serialization change would break the block, but that transactions haven't been removed. For example some future soft-fork that specifies a required order for transactions.

More importantly though, the text above guarantees backwards-compatibility when possible, as opposed to letting the implementation decide.

@luke-jr
Copy link
Member Author

luke-jr commented May 7, 2016

A softfork requiring ordered transactions would necessarily need to a new "rules" entry. When there are no rules that the client doesn't understand, "rules/force" is a no-op.

It's not the business of the spec to force backward compatibility. Implementations that don't want to provide it should be free not to.

On the other hand, when this prefix is used, it indicates a more subtle change to the block structure or generation transaction; examples of this would be BIP 34 (because it modifies coinbase construction) and 141 (since it modifies the txid hashing and adds a commitment to the generation transaction).
A client that does not understand a rule prefixed by '!' must not attempt to process the template, and must not attempt to use it for mining even unmodified.

Servers should only signal this mutation when they have verified a client behaving this way will not produce invalid blocks.
Copy link
Member

Choose a reason for hiding this comment

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

Does this section still match the text before?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, it was left over. Removed.

Sigops per block is currently limited to 20,000. We change this restriction as follows:

''Sigop cost'' is defined. The cost of a sigop in traditional script is 4, while the cost of a sigop in witness program is 1.
Sigops in the current pubkey script, signature script, and P2SH check script are counted at 4 times their previous value.
Copy link
Member

Choose a reason for hiding this comment

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

Any reason for removing the sigop cost terminology? I thought it was useful to distinguish from the original sigops definition.

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't see how the original sigops definition is relevant post-segwit. ie, what's the point?

Copy link
Member

Choose a reason for hiding this comment

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

Avoiding confusion :)

Copy link
Member Author

Choose a reason for hiding this comment

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

I personally find it more confusing to have two terms for essentially the same thing of different versions... should GBT be changed to remove sigops/sigoplimit and add sigopcost/sigopcostlimit as well?

@sipa
Copy link
Member

sipa commented May 13, 2016

@theuni Have your concerns been addressed?

@luke-jr luke-jr force-pushed the segwit_gbt_updates_20160330 branch from e8e5d5b to ce426ab Compare May 15, 2016 21:32
@luke-jr luke-jr changed the title BIP 9, 141, and 145: GBT updates for VB & SW BIP 141 and 145: GBT updates for SegWit May 31, 2016
@sipa
Copy link
Member

sipa commented Jun 1, 2016

ACK

@luke-jr luke-jr merged commit c917abe into bitcoin:master Jun 1, 2016
@luke-jr luke-jr deleted the segwit_gbt_updates_20160330 branch April 21, 2017 21:39
@dgenr8 dgenr8 mentioned this pull request Sep 28, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants