-
Notifications
You must be signed in to change notification settings - Fork 0
Update BIP 141 to include definition of Virtual transaction size and Transaction weight #1
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
Conversation
bip-0141.mediawiki
Outdated
| The new rule is ''block weight'' ≤ 4,000,000. | ||
|
|
||
| ===== Transaction size calculations ===== | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Though not neccessarily normative, the authors suggest use of the following terms for describing transaction size", or something like that, maybe?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure what you mean here. Transaction weight and Transaction virtual size are used in the code, and transaction weight is enforced in IsStandardTx() policy.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think your language here is fine, but since these terms aren't used in the consensus limits anywhere, my only nit would be to consider moving them to a new section ("Additional definitions", maybe?).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mean that they're not mentioned elsewhere in the BIP, so, by standards-speak they are "non-normative" (ie not a part of the spec). Feel free to use english instead of verobse bullshit :p.
|
@TheBlueMatt - I've moved the new definitions into their own section as suggested by @sdaftuar . Does that address your concerns about this being non-normative, or do you think additional language is still required? |
|
Good enough for me. |
bip-0141.mediawiki
Outdated
|
|
||
| === Additional definitions === | ||
|
|
||
| The following definitions are used in IsStandardTx() policy and the RPC interface, but are not part of consensus limits. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So this makes the doc very bitcoin core specific, which BIPs aren't really meant to be.
I'd either just strike the sentence, or say something more bland like:
The following definitions are suggested to provide language consistent with the terminology introduced above.
Maybe you could make that sound better :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, I think I get the point about non-normative now. Strictly speaking, BIPs should be about the consensus protocol, so adding reference to policy and rfc commands doesn't really fit.
I'm happy with your language, but I'll explicitly say that the definitions aren't used for consensus limits.
…Transaction weight
Completes PR for ChaincodeResidency/bitcoin#1