script: Remove dead code from OP_CHECKMULTISIG #33977

1. 
   vostrnad commented at 2:29 am on December 1, 2025: none

   The stack size check before the dummy element check in OP_CHECKMULTISIG is redundant. The stack is previously validated to have least i elements (where i is number of pubkeys + number of signatures + 3), and then i - 1 elements are popped off the stack, leaving at least one element still in.

   The redundant check was added in #3843 (commit 6380180821917c22ecfd89128ee60aae6f4cac33). There it’s easy to see why it’s redundant – it’s inserted into a code path where another popstack would previously follow.

   Note the missing code coverage: https://maflcko.github.io/b-c-cov/total.coverage/src/script/interpreter.cpp.gcov.html#L1200

2. script: Remove dead code from OP_CHECKMULTISIG

   Removes a redundant stack size check from OP_CHECKMULTISIG that was added in #3843 (commit 6380180821917c22ecfd89128ee60aae6f4cac33).

   53ca0dc3cf
3. DrahtBot added the label Consensus on Dec 1, 2025
4. 
   DrahtBot commented at 2:29 am on December 1, 2025: contributor

   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/33977.

   Reviews

   See the guideline for information on the review process.

   Type	Reviewers
   ACK	l0rinc, bensig, billymcbip

   If your review is incorrectly listed, please copy-paste <!–meta-tag:bot-skip–> into the comment that the bot should ignore.

5. 
   l0rinc commented at 10:36 am on December 1, 2025: contributor

   Code review ACK 53ca0dc3cfdad8c9fd994c3758a477e319379db1

   The lines are safe to remove since they’re side-effect free and the mentioned extra element is included in the initial stack size validation while the cleanup loop stops before popping it. The comment above it likely doesn’t need updating since it’s seems to refer to the line below. The SCRIPT_ERR_INVALID_STACK_OPERATION script error is used elsewhere, so we don’t need to clean that up either.

6. 
   billymcbip commented at 2:17 pm on December 1, 2025: contributor

   I think it wouldn’t hurt to have an assert(!stack.empty()); instead, just in case the code above is ever refactored. Nvm, agree this isn’t required. Suggest adding a comment though, see below.
7. 
   vostrnad commented at 2:26 pm on December 1, 2025: none

   The script interpreter code is so thoroughly tested, so infrequently refactored, and any change to it reviewed with such extreme caution that I don’t think we need to bother with an assert, but I can add it if others think it’s a good idea.
8. 
   billymcbip commented at 11:31 am on December 24, 2025: contributor

   tACK 53ca0dc3cfdad8c9fd994c3758a477e319379db1

   Verified using existing tests and a new test that omits the dummy element: #34145

   The previous stack size check already fails script evaluation when stack size < i, where i equals 3 + nKeysCount + nSigsCount.

9. 
   bensig commented at 0:14 am on January 4, 2026: contributor

   ACK 53ca0dc3cfdad8c9fd994c3758a477e319379db1

   Verified the logic - after popping i - 1 elements from a stack guaranteed to have >= i elements, at least 1 element always remains. The size check is unreachable. script_tests pass.

10. 
    achow101 commented at 11:47 pm on January 5, 2026: member

    Concept -0

    While this is probably true, the implementation of OP_CHECKMULTISIG is complicated enough that walking through the logic to determine whether it is redundant does not seem like a good use of reviewer time for something that has little to no impact.

11. 
    billymcbip commented at 5:28 pm on January 6, 2026: contributor

    The stack is previously validated to have least i elements (where i is number of pubkeys + number of signatures + 3)

    Adding this as a code comment just before the final stack-size check would make it easier to eyeball the code and see that the stack operations are safe:

    0    i += nSigsCount;
    1+++ // i is now 3 + nKeysCount + nSigsCount
    2    if ((int)stack.size() < i)
    

Contributors
vostrnad DrahtBot l0rinc billymcbip bensig achow101

Labels
Consensus