Rename OP_NOP2 to OP_CHECKLOCKTIMEVERIFY #7213

pull mb300sd wants to merge 1 commits into bitcoin:master from mb300sd:rename-opnop2 changing 11 files +61 −47

mb300sd commented at 7:36 PM on December 14, 2015: contributor

As suggested by petertodd here https://www.reddit.com/r/Bitcoin/comments/3wt1b9/bip65_cltv_just_went_into_full_enforcement/cxywon5

:)
MarcoFalke commented at 9:39 PM on December 14, 2015: member

Concept ACK

Could you please squash those commits? Commits should be as small as possible but still atomic.
mb300sd force-pushed on Dec 14, 2015
petertodd commented at 10:02 PM on December 14, 2015: contributor

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
in qa/rpc-tests/test_framework/script.py:None in d19686d4fb outdated
```
 225 | @@ -226,7 +226,8 @@ def __new__(cls, n):
 226 |  
 227 |  # expansion
 228 |  OP_NOP1 = CScriptOp(0xb0)
 229 | -OP_NOP2 = CScriptOp(0xb1)
 230 | +OP_CHECKLOCKTIMEVERIFY = CScriptOp(0xb1)
 231 | +OP_NOP2 = OP_CHECKLOCKTIMEVERIFY
```
luke-jr commented at 10:53 PM on December 14, 2015:

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.
luke-jr commented at 10:55 PM on December 14, 2015: member

@petertodd If they depend on OP_NOP2 being returned, they are likely to be more subtley broken by not updating this...
dcousens commented at 11:25 PM on December 14, 2015: contributor

Agreed with @luke-jr, the meaning has changed, a break would be the best-case result for anyone depending on this.
mb300sd force-pushed on Dec 14, 2015
mb300sd commented at 11:34 PM on December 14, 2015: contributor

Updated with release note update and the compatibility define removed.
mb300sd force-pushed on Dec 14, 2015
dcousens commented at 2:02 AM on December 15, 2015: contributor

concept/once-over utACK 0bf46e0
mb300sd commented at 6:42 AM on December 15, 2015: contributor

@paveljanik Only if the verbose option is specified, the json output contains an asm script field. Standard hex output is not.
paveljanik commented at 6:54 AM on December 15, 2015: contributor

@mb300sd Right. Thank you. Can you please add "(in verbose mode)"?
Rename OP_NOP2 to OP_CHECKLOCKTIMEVERIFY. 37d271d7cc
mb300sd force-pushed on Dec 15, 2015
MarcoFalke commented at 4:46 PM on December 17, 2015: member

utACK 37d271d
petertodd commented at 4:44 PM on December 19, 2015: contributor

@luke-jr That's a reasonable idea; let's remove OP_NOP2.

Release notes look good, thanks!
laanwj commented at 9:16 AM on December 22, 2015: member

utACK (but the tests are sufficient here) 37d271d7cce44885f835292ffe99b54399b014d6
maaku commented at 9:23 AM on December 22, 2015: contributor

tested ACK
laanwj added the label P2P on Dec 22, 2015
laanwj added the label RPC on Dec 22, 2015
laanwj removed the label P2P on Dec 22, 2015
laanwj merged this on Dec 22, 2015
laanwj closed this on Dec 22, 2015
laanwj referenced this in commit 595f93977c on Dec 22, 2015
luke-jr referenced this in commit 3fb66755c7 on Dec 28, 2015
luke-jr referenced this in commit 8f6b971a58 on Dec 28, 2015
luke-jr referenced this in commit 7c8e88142e on Jan 13, 2016
luke-jr referenced this in commit 605de4a88a on Jan 13, 2016
MarcoFalke locked this on Sep 8, 2021

Contributors

Labels