Rename OP_NOP2 to OP_CHECKLOCKTIMEVERIFY#7213
Conversation
|
Concept ACK Could you please squash those commits? Commits should be as small as possible but still atomic. |
mb300sd
force-pushed
the
rename-opnop2
branch
from
784b408 to
d19686d
Compare
|
It'd be a good idea to note that the name has changed in the release notes, as this has the (small) potential to break some code that depends on what decodescript returns; we probably won't release this until v0.13 |
There was a problem hiding this comment.
I am not sure this "compatibility" should be retained. If someone is specifying OP_NOP2, they may actually be expecting OP_NOP2 behaviour. Better to error and let them explicitly change to OP_CHECKLOCKTIMEVERIFY.
|
@petertodd If they depend on OP_NOP2 being returned, they are likely to be more subtley broken by not updating this... |
|
Agreed with @luke-jr, the meaning has changed, a break would be the best-case result for anyone depending on this. |
mb300sd
force-pushed
the
rename-opnop2
branch
from
d19686d to
ccd7ebc
Compare
|
Updated with release note update and the compatibility define removed. |
mb300sd
force-pushed
the
rename-opnop2
branch
from
ccd7ebc to
0bf46e0
Compare
|
concept/once-over utACK 0bf46e0 |
|
@paveljanik Only if the verbose option is specified, the json output contains an asm script field. Standard hex output is not. |
|
@mb300sd Right. Thank you. Can you please add "(in verbose mode)"? |
mb300sd
force-pushed
the
rename-opnop2
branch
from
0bf46e0 to
37d271d
Compare
|
utACK 37d271d |
|
@luke-jr That's a reasonable idea; let's remove OP_NOP2. Release notes look good, thanks! |
|
utACK (but the tests are sufficient here) |
|
tested ACK |
37d271d Rename OP_NOP2 to OP_CHECKLOCKTIMEVERIFY. (mb300sd)
Github-Pull: bitcoin#7213 Rebased-From: 37d271d
Github-Pull: bitcoin#7213 Rebased-From: 37d271d
Github-Pull: bitcoin#7213 Rebased-From: 37d271d
8 participants
As suggested by petertodd here https://www.reddit.com/r/Bitcoin/comments/3wt1b9/bip65_cltv_just_went_into_full_enforcement/cxywon5
:)