-
Notifications
You must be signed in to change notification settings - Fork 5.9k
BIP-Draft: 64-bit arithmetic in Script #2073
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
base: master
Are you sure you want to change the base?
Conversation
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.
Thanks for your submission. Due to the proposal’s welcome brevity, I was able to give it a quick skim immediately. I noticed that this document seems to be missing a Specification section, so I’m changing the pull request to Draft status. Please set the pull request to Ready for Review, when the Specification section has been added.
Co-authored-by: Mark "Murch" Erhardt <murch@murch.one>
|
|
||
| ==Specification== | ||
|
|
||
| All existing opcodes<ref name="numeric-opcodes" /> that interpret stack elements as numbers are modified to support 64-bit operands. |
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.
The Backward Compatibility section mentions OP_SUCCESSx, but here you state that the behavior of existing opcodes is modified. The former would imply that a new set of opcodes is introduced and in that case, this document should specify which OP_SUCCESS codes are used to do so. The latter seems to imply a hardfork, so it seems that those two sections need some reconciliation.
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 intended the reconciliation to happen in the deployment section.
I think the most optimal deployment story is this BIP gets bundled with a wider set of BIPs that actually allocate a new opcode - ideally any of the 4 BIPs cited in the motivation section. Multiple BIPs were deployed as part of Taproot and segwit so I don't think this would be a problem.
An alternative that has been discussed is an OP_ENABLE64BIT opcode that uses the OP_SUCCESS semantics if this proposal was deployed on its own. I would advocate for taking the former path rather than allocating a speicific opcode like OP_ENABLE64BIT though.
With that being said, if you think this can be reworded to be clearer, or perhaps deleting parts that are confusing, I'm interested in hearing your thoughts.
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 see no issue with including a description of one or more of these options as suggested deployment paths at this stage of the document, and would consider that an improvement over the current description.
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.
Perhaps I'm being a bit dense, but isn't that what I'm already saying the deployment section. Is the OP_ENABLE64BIT opcode the part you would like me to add?
Or do you want me to add a section about the deployment story in the specification section?
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.
The way I understand the first line of the Specification (“All existing opcodes that interpret stack elements as numbers are modified to support 64-bit operands”) and the first line of the Abstract (“This BIP proposes to extend the range of numeric operands in Script […]”), it sounds that your proposal would modify all number processing opcodes in all versions of Script , but the Backward Compatibility section states that it should be deployed as a soft fork, and the Deployment section mentions OP_SUCCESSx semantics. It seems to me that there is an unstated assumption here that’s the missing link. Extending the accepted inputs of opcodes in Script or Tapscript would be a hard fork, so you may be thinking that the 64-bit arithmetic would only be introduced for a new script leaf version, but if so, I missed where the proposal explains this restricted scope.
I was hoping that you would update the phrasing of those sections to clarify in what scope the changed opcode behavior is introduced, so that the sections are compatible and it becomes obvious how this proposal could be deployed as a soft fork.
Mailing list post: https://groups.google.com/g/bitcoindev/c/j1zEky-3QEE
Delving bitcoin discussion: https://delvingbitcoin.org/t/64-bit-arithmetic-soft-fork/397
S/o to the "Write a BIP" talk at btc++ by Ava Chow to motivate me to actually submit this :-)
If you are interested in previous versions of this idea, see #1538 and #1533