Skip to content

Update BIP 141 to include definition of Virtual transaction size and Transaction weight - #1

Merged
jnewbery merged 1 commit into
ChaincodeResidency:masterfrom
jnewbery:master
Sep 22, 2016
Merged

Update BIP 141 to include definition of Virtual transaction size and Transaction weight#1
jnewbery merged 1 commit into
ChaincodeResidency:masterfrom
jnewbery:master

Conversation

@jnewbery

Copy link
Copy Markdown
Collaborator

Completes PR for ChaincodeResidency/bitcoin#1

Comment thread bip-0141.mediawiki Outdated
The new rule is ''block weight'' ≤ 4,000,000.

===== Transaction size calculations =====

Copy link
Copy Markdown

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?

Copy link
Copy Markdown
Collaborator Author

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.

Copy link
Copy Markdown

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?).

Copy link
Copy Markdown

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.

@jnewbery

Copy link
Copy Markdown
Collaborator Author

@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?

@TheBlueMatt

Copy link
Copy Markdown

Good enough for me.

Comment thread 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.

Copy link
Copy Markdown

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 :)

Copy link
Copy Markdown
Collaborator Author

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.

@jnewbery
jnewbery merged commit c2213ed into ChaincodeResidency:master Sep 22, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants