Skip to content

Fix 11-year-old mis-categorized error code in OP_IF evaluation #32143

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fix 11-year-old mis-categorized error code in OP_IF evaluation #32143

wants to merge 1 commit into from

Conversation

cculianu
Copy link

@cculianu cculianu commented Mar 26, 2025 •
edited
Loading

This was introduced by commit ab9edbd.

It appears the original author may have gotten tired and pasted the wrong error code into this 1 place. Every other situation where the value stack lacks the required number of arguments for the op-code, SCRIPT_ERR_INVALID_STACK_OPERATION is reported. Not so here.

This commit fixes the situation.

EDIT: Note this turns out to be a dupe of the abandoned #30359 .

@DrahtBot
Copy link
Contributor

DrahtBot commented Mar 26, 2025 •
edited
Loading

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/32143.

Reviews

See the guideline for information on the review process.

Type Reviewers
Stale ACK laanwj

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

No conflicts as of last run.

@DrahtBot
Copy link
Contributor

🚧 At least one of the CI tasks failed.
Debug: https://github.com/bitcoin/bitcoin/runs/39412787907

Hints

Try to run the tests locally, according to the documentation. However, a CI failure may still
happen due to a number of reasons, for example:

  • Possibly due to a silent merge conflict (the changes in this pull request being
    incompatible with the current code in the target branch). If so, make sure to rebase on the latest
    commit of the target branch.

  • A sanitizer issue, which can only be found by compiling with the sanitizer and running the
    affected test.

  • An intermittent issue.

Leave a comment here, if you need help tracking down a confusing failure.

@laanwj
Copy link
Member

laanwj commented Mar 26, 2025 •
edited
Loading

This is failing the "test each commit" check. Please squash test commits where needed to make sure that the intermediate commits pass the tests, too.

test/script_tests.cpp(908): Entering test case "script_json_test"
2025-03-26T09:15:59.630554Z [test] [test/util/random.cpp:48] [SeedRandomStateForTest] Setting random seed for current tests to RANDOM_CTX_SEED=be1d635f1a8e48c6048a40b25f2bd5e1d235e5ec1571241fe7157b541468c337
2025-03-26T09:15:59.630977Z [test] [init/common.cpp:151] [LogPackageVersion] Bitcoin Core version v29.99.0-da65703b810d (release build)
test/script_tests.cpp(127): error: in "script_tests/script_json_test": INVALID_STACK_OPERATION where UNBALANCED_CONDITIONAL expected: ["NOP","IF 1 ENDIF","P2SH,STRICTENC","UNBALANCED_CONDITIONAL","The following tests check the if(stack.size() < N) tests in each opcode"]
test/script_tests.cpp(127): error: in "script_tests/script_json_test": INVALID_STACK_OPERATION where UNBALANCED_CONDITIONAL expected: ["NOP","NOTIF 1 ENDIF","P2SH,STRICTENC","UNBALANCED_CONDITIONAL","They are here to catch copy-and-paste errors"]
test/script_tests.cpp(127): error: in "script_tests/script_json_test": INVALID_STACK_OPERATION where UNBALANCED_CONDITIONAL expected: ["0x03 0x635168","HASH160 0x14 0xe7309652a8e3f600f06f5d8d52d6df03d2176cc3 EQUAL","P2SH,WITNESS,MINIMALIF","UNBALANCED_CONDITIONAL"]
test/script_tests.cpp(127): error: in "script_tests/script_json_test": INVALID_STACK_OPERATION where UNBALANCED_CONDITIONAL expected: ["0x03 0x645168","HASH160 0x14 0x0c3f8fe3d6ca266e76311ecda544c67d15fdd5b0 EQUAL","P2SH,WITNESS,MINIMALIF","UNBALANCED_CONDITIONAL"]
test/script_tests.cpp(127): error: in "script_tests/script_json_test": INVALID_STACK_OPERATION where UNBALANCED_CONDITIONAL expected: [["635168",0.00000001],"","0 0x20 0xc7eaf06d5ae01a58e376e126eb1e6fab2036076922b96b2711ffbec1e590665d","P2SH,WITNESS","UNBALANCED_CONDITIONAL"]
test/script_tests.cpp(127): error: in "script_tests/script_json_test": INVALID_STACK_OPERATION where UNBALANCED_CONDITIONAL expected: [["635168",0.00000001],"","0 0x20 0xc7eaf06d5ae01a58e376e126eb1e6fab2036076922b96b2711ffbec1e590665d","P2SH,WITNESS,MINIMALIF","UNBALANCED_CONDITIONAL"]
test/script_tests.cpp(127): error: in "script_tests/script_json_test": INVALID_STACK_OPERATION where UNBALANCED_CONDITIONAL expected: [["645168",0.00000001],"","0 0x20 0xf913eacf2e38a5d6fc3a8311d72ae704cb83866350a984dd3e5eb76d2a8c28e8","P2SH,WITNESS","UNBALANCED_CONDITIONAL"]
test/script_tests.cpp(127): error: in "script_tests/script_json_test": INVALID_STACK_OPERATION where UNBALANCED_CONDITIONAL expected: [["645168",0.00000001],"","0 0x20 0xf913eacf2e38a5d6fc3a8311d72ae704cb83866350a984dd3e5eb76d2a8c28e8","P2SH,WITNESS,MINIMALIF","UNBALANCED_CONDITIONAL"]
test/script_tests.cpp(127): error: in "script_tests/script_json_test": INVALID_STACK_OPERATION where UNBALANCED_CONDITIONAL expected: [["635168",0.00000001],"0x22 0x0020c7eaf06d5ae01a58e376e126eb1e6fab2036076922b96b2711ffbec1e590665d","HASH160 0x14 0x9b27ee6d9010c21bf837b334d043be5d150e7ba7 EQUAL","P2SH,WITNESS","UNBALANCED_CONDITIONAL"]
test/script_tests.cpp(127): error: in "script_tests/script_json_test": INVALID_STACK_OPERATION where UNBALANCED_CONDITIONAL expected: [["635168",0.00000001],"0x22 0x0020c7eaf06d5ae01a58e376e126eb1e6fab2036076922b96b2711ffbec1e590665d","HASH160 0x14 0x9b27ee6d9010c21bf837b334d043be5d150e7ba7 EQUAL","P2SH,WITNESS,MINIMALIF","UNBALANCED_CONDITIONAL"]
test/script_tests.cpp(127): error: in "script_tests/script_json_test": INVALID_STACK_OPERATION where UNBALANCED_CONDITIONAL expected: [["645168",0.00000001],"0x22 0x0020f913eacf2e38a5d6fc3a8311d72ae704cb83866350a984dd3e5eb76d2a8c28e8","HASH160 0x14 0xdbb7d1c0a56b7a9c423300c8cca6e6e065baf1dc EQUAL","P2SH,WITNESS","UNBALANCED_CONDITIONAL"]
test/script_tests.cpp(127): error: in "script_tests/script_json_test": INVALID_STACK_OPERATION where UNBALANCED_CONDITIONAL expected: [["645168",0.00000001],"0x22 0x0020f913eacf2e38a5d6fc3a8311d72ae704cb83866350a984dd3e5eb76d2a8c28e8","HASH160 0x14 0xdbb7d1c0a56b7a9c423300c8cca6e6e065baf1dc EQUAL","P2SH,WITNESS,MINIMALIF","UNBALANCED_CONDITIONAL"]
test/script_tests.cpp(908): Leaving test case "script_json_test"; testing time: 184114us

@cculianu
Fix 11-year-old mis-categorized error code in OP_IF evaluation
a7b5814 
This was introduced by commit ab9edbd.

It appears the original author may have gotten tired and pasted the wrong
error code into this 1 place. Every other situation where the value stack
lacks the required number of arguments for the op-code,
SCRIPT_ERR_INVALID_STACK_OPERATION is reported. Not so here.

This commit fixes the situation.

Also in this commit:

- Fix script_tests to adjust to the corrected error message
- Fix p2p_invalid_tx functional test to produce the desired error message
@cculianu
Copy link
Author

Ok, squashed.

@DrahtBot DrahtBot mentioned this pull request Jul 19, 2025
Draft
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
Consensus Validation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@cculianu @DrahtBot @laanwj