Remove the "OP_" prefix from transaction script decodes. #5392
Conversation
|
Ehh, I find the OP_ prefix more readable, and though I dont feel strongly, I dont like the idea of changing the rpc outptut just for this. |
|
Thanks for the feedback. #5264 was a change to decode the scriptSig signature hash types. The reason for making this set of "OP_" changes at the same time was to break any dependencies on the output pretty clearly. Helping readability by reducing the "OP_" redundancy was a secondary goal. I think the growing consensus from the #5264 discussion is that no guarantees to hold these outputs constant have been made. As these changes are made, more unit tests are being added which will firm up these output formats and increase support for those that do want to script against them in the future. So, that's at least why this set of changes was put forward. |
|
@TheBlueMatt The rationale for this is that as #5264 changes the asm syntax to add (...) comments, which will break current parsers anyhow, a cleanup like removing OP_ can be done at the same time. So I'm fine with this if it is decided to merge that pull. |
|
Yes, doing them at the same time is reasonable, though I still think the OP_ prefix reads better to me...but, really...who cares? |
|
Prefixes or not, the added value is in the new unit tests that firm up the format. I'd be just as happy to remove my changes that remove the "OP_" prefixes if doing that will get the new unit tests merged (and I'll update them to verify that the "OP_" prefixes are there). Please let me know if I should do that. This is also about consistency in the "asm" outputs. @TheBlueMatt could you comment on #5264 about if you like that change or not (#5264 adds SIGHASH decodes to the "asm" outputs)? That would help move that ticket closer to a result either way too. Thanks! |
|
Typical asm-style languages do not contain a redundant, common prefix. It's useful for C++ global namespace disambiguation, but the "native" names should not contain OP_ |
|
@jgarzik interesting perspective, and little nugget of knowledge, thanks! |
|
Withdrew #5264 so I'm closing this too as it's not important enough on its own. |
4 participants
This set of changes was born out of the discussion of #5264.
The main impact of these changes is on the scriptPubKey "asm" value in the output for transaction decodes. There are also more minor logging and bitcoin-tx references.
Basically, the goal was to just remove the "OP_" prefixes.
Example P2PKH
Before:
OP_DUP OP_HASH160 b9c670c7aa955c9808af7eeb9643b9fd6285cbf5 OP_EQUALVERIFY OP_CHECKSIG
After:
DUP HASH160 b9c670c7aa955c9808af7eeb9643b9fd6285cbf5 EQUALVERIFY CHECKSIG
Example P2SH
Before:
OP_HASH160 2a5edea39971049a540474c6a99edf0aa4074c58 OP_EQUAL
After:
HASH160 2a5edea39971049a540474c6a99edf0aa4074c58 EQUAL
If these changes gain acceptance, I'll add some release notes too when that time comes.