Completely remove disabled opcodes; modify script tests accordingly #1869

pull maaku wants to merge 3 commits into bitcoin:master from maaku:remove_disabled_opcodes changing 3 files +30 −218

maaku commented at 7:14 PM on September 26, 2012: contributor
This patch removes the disabled opcodes and the one dependent utility function that is orphaned.

Interestingly there appears to be a slight difference in behavior between explicitly disabled opcodes and reserved/unused opcodes. Previously VerifyScript() would fail any script containing a disabled opcode as soon as it is encountered, whereas reserved/unused opcodes would fail when they are executed. This is a subtle difference, but detectable. Take the following script from the invalid-script unit tests:
```
0 IF INVERT ELSE 1 ENDIF
```
This previously failed even though the INVERT is never executed. But now that the disabled opcodes are removed entirely, INVERT becomes 0x83 and is treated like any other unused opcode, meaning that script now passes validation. To fail, the IF-branch of the conditional would have to be taken:
```
1 IF 0x83 ELSE 1 ENDIF
```
The last commit of this pull request simply switches the branch taken in this and other unit tests such that the opcode is executed and fails.

However the construction of the unit test clearly indicates that failure even if the opcode is not executed was the desired result. A decision still needs to be made as to whether this behavior should be extended to all reserved/unused opcodes.

Documented bug in sign-extension behavior of opcodes OP_AND, OP_OR, and OP_XOR.

Due to a bug in the implementation of MakeSameSize(), using OP_AND, OP_OR, or OP_XOR with signed values of unequal size will result in the sign-value becoming part of the smaller integer, with nonsensical results. This patch documents the unexpected behavior and provides the basis of a solution should decision be made to fix the bug in the future.

95d7f00295

Completely removed disabled opcodes (and one support function, MakeSameSize(), which was only used by the disabled opcodes). de7c86e191
Correct unit test scripts such that IF-branch of the conditional is taken and the previously disabled, now unused opcodes are executed. f0b3fb366c
gavinandresen commented at 7:33 PM on September 26, 2012: contributor

NACK. Changing the behavior of VerifyScript in any detectable way is unacceptable, because of the risk of a blockchain fork.
maaku commented at 9:10 PM on September 26, 2012: contributor

This can't lead to a blockchain fork. These opcodes aren't on mainnet, and wouldn't be accepted on mainnet anyway by any existing client. They would be illegal before the change, and they would be illegal after the change. It only changes the behavior of a hypothetical client that relaxes IsStandard() script restrictions.
jgarzik commented at 9:13 PM on September 26, 2012: contributor

IsStandard() is not relevant to this discussion. Transactions may appear in the blockchain, even if they fail IsStandard().

The set of acceptable behaviors is much larger than the set of relay-able IsStandard() behaviors.
maaku commented at 9:59 PM on September 26, 2012: contributor

Ok understood.
maaku closed this on Sep 26, 2012
KolbyML referenced this in commit 525a45a3a9 on Dec 5, 2020
DrahtBot locked this on Sep 8, 2021

Contributors

maaku

gavinandresen

jgarzik