[script] simplify CheckMinimalPush checks, add safety assert #11900

pull instagibbs wants to merge 2 commits into bitcoin:master from instagibbs:checkminimal changing 1 files +10 −8

instagibbs commented at 7:23 PM on December 14, 2017: member

the two conditions could simply never be hit as true, as those opcodes have a push payload of size 0 in data.

Added the assert for clarity for future readers(matching the gating in the interpreter) and safety for future use.

This effects policy only.
simplify CheckMinimalPush checks, add safety assert 176db61474
instagibbs renamed this:
~~[policy] simplify CheckMinimalPush checks, add safety assert~~
[script] simplify CheckMinimalPush checks, add safety assert
on Dec 14, 2017
laanwj added the label Consensus on Dec 14, 2017
instagibbs commented at 6:13 PM on March 6, 2018: member

@laanwj this is not consensus, but policy.
laanwj added the label TX fees and policy on Mar 6, 2018
laanwj removed the label Consensus on Mar 6, 2018
laanwj commented at 6:17 PM on March 6, 2018: member

@instagibbs thanks, label was updated
sdaftuar commented at 3:23 PM on March 7, 2018: member

utACK

in src/script/interpreter.cpp:229 in 176db61474 outdated

 224 | +    assert(0 <= opcode && opcode <= OP_PUSHDATA4);
 225 |      if (data.size() == 0) {
 226 |          // Could have used OP_0.
 227 |          return opcode == OP_0;
 228 |      } else if (data.size() == 1 && data[0] >= 1 && data[0] <= 16) {
 229 |          // Could have used OP_1 .. OP_16.

kallewoof commented at 3:36 PM on March 7, 2018:

Would rewrite as "Should" since the previous comment actually indicated that this would have been valid.

in src/script/interpreter.cpp:232 in 176db61474 outdated

 228 |      } else if (data.size() == 1 && data[0] >= 1 && data[0] <= 16) {
 229 |          // Could have used OP_1 .. OP_16.
 230 | -        return opcode == OP_1 + (data[0] - 1);
 231 | +        return false;
 232 |      } else if (data.size() == 1 && data[0] == 0x81) {
 233 |          // Could have used OP_1NEGATE.

kallewoof commented at 3:36 PM on March 7, 2018:

Would rew... (see above)

kallewoof approved
kallewoof commented at 3:37 PM on March 7, 2018: member

utACK
instagibbs force-pushed on Mar 7, 2018
instagibbs commented at 3:51 PM on March 7, 2018: member

added a comment clarification as a new independent commit
CheckMinimalPush comments are prescriptive 0749808a7f
instagibbs force-pushed on Mar 7, 2018
kallewoof approved
kallewoof commented at 3:57 PM on March 7, 2018: member

utACK 0749808a7fc31250abfbc8c61963065e0d320824
sdaftuar commented at 4:08 PM on March 7, 2018: member

re-utACK
laanwj commented at 4:18 PM on March 7, 2018: member

utACK 0749808a7, checked and this makes sense.
laanwj merged this on Mar 7, 2018
laanwj closed this on Mar 7, 2018
laanwj referenced this in commit 842f61a675 on Mar 7, 2018
Mengerian referenced this in commit 04df29df4d on Aug 6, 2019
jonspock referenced this in commit 29df3a1181 on Sep 7, 2019
jonspock referenced this in commit a5606c6bb3 on Sep 7, 2019
proteanx referenced this in commit cc4e2c3c31 on Sep 9, 2019
PastaPastaPasta referenced this in commit 9e9ba619c7 on Dec 12, 2020
PastaPastaPasta referenced this in commit ddc6cb050c on Dec 12, 2020
PastaPastaPasta referenced this in commit 657b5246d4 on Dec 15, 2020
PastaPastaPasta referenced this in commit 28f1a09ee4 on Dec 15, 2020
PastaPastaPasta referenced this in commit 66a6a684e2 on Dec 18, 2020
MarcoFalke locked this on Dec 16, 2021

Contributors

Labels