txscript: reject OP_CODESEPARATOR in unexecuted branches for non-segwit#2485
Open
Roasbeef wants to merge 1 commit intobtcsuite:masterfrom
Open
txscript: reject OP_CODESEPARATOR in unexecuted branches for non-segwit#2485Roasbeef wants to merge 1 commit intobtcsuite:masterfrom
Roasbeef wants to merge 1 commit intobtcsuite:masterfrom
Conversation
In this commit, we fix a policy-level divergence with Bitcoin Core when handling OP_CODESEPARATOR inside unexecuted OP_IF branches in non-segwit scripts with the ScriptVerifyConstScriptCode flag. Bitcoin Core's EvalScript (interpreter.cpp:474-476) places the SCRIPT_VERIFY_CONST_SCRIPTCODE check for OP_CODESEPARATOR before the fExec branch-execution gate, causing it to fire unconditionally on every OP_CODESEPARATOR encountered during script iteration -- even inside OP_FALSE OP_IF ... OP_ENDIF envelopes. Previously, btcd's equivalent check lived inside the opcodeCodeSeparator handler, which was never reached for opcodes in unexecuted branches due to the early return in executeOpcode that skips non-conditional opcodes when isBranchExecuting() is false. This meant a script like: OP_FALSE OP_IF OP_CODESEPARATOR OP_ENDIF <validation> would be rejected by Bitcoin Core's mempool but accepted by btcd's. The fix moves the check before the branch-execution gate in executeOpcode, matching Bitcoin Core's structure. This follows the existing pattern in btcd where isOpcodeDisabled and isOpcodeAlwaysIllegal checks already fire regardless of branch execution state. Note: SCRIPT_VERIFY_CONST_SCRIPTCODE is purely a policy flag (included in STANDARD_SCRIPT_VERIFY_FLAGS but not MANDATORY_SCRIPT_VERIFY_FLAGS), so this was not a consensus divergence. Both implementations would accept such transactions if mined in a block. Found via differential fuzzing by Bruno from bitcoinfuzz.
Pull Request Test Coverage Report for Build 22081381425Details
💛 - Coveralls |
brunoerg
approved these changes
Feb 18, 2026
Collaborator
|
cc: @gijswijs for review |
| // script is rejected even in an unexecuted branch. This mirrors | ||
| // Bitcoin Core's behavior where the check fires unconditionally | ||
| // before the branch execution gate. | ||
| if op.value == OP_CODESEPARATOR && vm.taprootCtx == nil && |
Collaborator
There was a problem hiding this comment.
Bit weird that we check for non-segwitness by asserting witnessProgram and taprootCtx is nil but this works.
Collaborator
|
Checked against Core at commit But can we add a segwit case? It's not tested here and would be good to have. Something like this maybe: diff --git a/txscript/engine_test.go b/txscript/engine_test.go
index dea9f2f7..311a9038 100644
--- a/txscript/engine_test.go
+++ b/txscript/engine_test.go
@@ -6,6 +6,7 @@
package txscript
import (
+ "crypto/sha256"
"testing"
"github.com/btcsuite/btcd/chaincfg/chainhash"
@@ -466,6 +467,7 @@ func TestCodeSepUnexecutedBranch(t *testing.T) {
name string
script string
flags ScriptFlags
+ segwit bool
wantErr bool
errCode ErrorCode
}{
@@ -503,13 +505,37 @@ func TestCodeSepUnexecutedBranch(t *testing.T) {
wantErr: true,
errCode: ErrCodeSeparator,
},
+ {
+ // OP_CODESEPARATOR in a segwit P2WSH witness script
+ // should succeed even with const scriptcode, since
+ // the flag only applies to non-segwit scripts.
+ name: "codesep in segwit P2WSH with const scriptcode",
+ script: "CODESEPARATOR TRUE",
+ flags: ScriptVerifyConstScriptCode | ScriptVerifyWitness | ScriptBip16,
+ segwit: true,
+ wantErr: false,
+ },
}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
- pkScript := mustParseShortForm(test.script)
+ scriptBytes := mustParseShortForm(test.script)
+
+ pkScript := scriptBytes
+ testTx := tx
+ if test.segwit {
+ hash := sha256.Sum256(scriptBytes)
+ pkScript, _ = NewScriptBuilder().
+ AddOp(OP_0).AddData(hash[:]).Script()
+ testTx = tx.Copy()
+ testTx.TxIn[0].SignatureScript = nil
+ testTx.TxIn[0].Witness = wire.TxWitness{
+ scriptBytes,
+ }
+ }
+
vm, err := NewEngine(
- pkScript, tx, 0, test.flags, nil, nil,
+ pkScript, testTx, 0, test.flags, nil, nil,
-1, nil,
)
if err != nil { |
Member
Author
|
@kcalvinalvin PTAL. One other thing we can do is remove the old check. Since we know check it each time, we no longer need that logic within the op code invocation itself. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
In this commit, we fix a policy-level divergence with Bitcoin Core when handling OP_CODESEPARATOR inside unexecuted OP_IF branches in non-segwit scripts with the ScriptVerifyConstScriptCode flag.
Bitcoin Core's EvalScript (interpreter.cpp:474-476) places the SCRIPT_VERIFY_CONST_SCRIPTCODE check for OP_CODESEPARATOR before the fExec branch-execution gate, causing it to fire unconditionally on every OP_CODESEPARATOR encountered during script iteration -- even inside OP_FALSE OP_IF ... OP_ENDIF envelopes.
Previously, btcd's equivalent check lived inside the opcodeCodeSeparator handler, which was never reached for opcodes in unexecuted branches due to the early return in executeOpcode that skips non-conditional opcodes when isBranchExecuting() is false. This meant a script like:
OP_FALSE OP_IF OP_CODESEPARATOR OP_ENDIF
would be rejected by Bitcoin Core's mempool but accepted by btcd's.
The fix moves the check before the branch-execution gate in executeOpcode, matching Bitcoin Core's structure. This follows the existing pattern in btcd where isOpcodeDisabled and isOpcodeAlwaysIllegal checks already fire regardless of branch execution state.
Note: SCRIPT_VERIFY_CONST_SCRIPTCODE is purely a policy flag (included in STANDARD_SCRIPT_VERIFY_FLAGS but not MANDATORY_SCRIPT_VERIFY_FLAGS), so this was not a consensus divergence. Both implementations would accept such transactions if mined in a block.
Found via differential fuzzing by Bruno from bitcoinfuzz.