← Watch feed
refactor: remove dead code in `CountWitnessSigOps`
Public commit record
100/100 · Strong
What the developer wrote
Authored by Lőrinc
refactor: remove dead code in `CountWitnessSigOps`
Found while reviewing #32840
The `nullptr` witness path was dead in normal code paths: removing it deletes unreachable logic.
Code coverage proof:
https://maflcko.github.io/b-c-cov/total.coverage/src/script/interpreter.cpp.gcov.html#L2135
✓ Specific, descriptive subject✓ Names a concrete action or component✓ Uses a recognizable type or scope✓ Provides detailed explanatory context✓ Mentions testing or verification✓ Links an issue, advisory, or supporting reference
What changed, and why it matters
This commit is a code cleanup: it removes an unused 'nullptr witness' code path from a Bitcoin Core function that counts signature operations in SegWit transactions. The change does not alter behavior in normal operation because the nullptr path was never reached. It is not a security fix.
Recommended action
No security action required. Treat as ordinary refactoring review.
Security signals we found
No strong security signals were identified.
Evidence from the diff
CountWitnessSigOps previously accepted a const CScriptWitness* pointer and substituted an empty witness if nullptr was passed. The commit changes the signature to accept a const CScriptWitness& and removes the empty-witness fallback. All call sites are updated to pass the witness object directly. The author notes the nullptr path was dead in normal code paths, supported by code-coverage data. No consensus rules or validation behavior change.
Changed components
src/consensus/tx_verify.cppsrc/script/interpreter.cppsrc/script/interpreter.hsrc/test/fuzz/coins_view.cppsrc/test/fuzz/script.cppInspect captured patch +7 / −9
diff --git a/src/consensus/tx_verify.cpp b/src/consensus/tx_verify.cpp
index 9d098725..c38b9643 100644
--- a/src/consensus/tx_verify.cpp
+++ b/src/consensus/tx_verify.cpp
@@ -156,7 +156,7 @@ int64_t GetTransactionSigOpCost(const CTransaction& tx, const CCoinsViewCache& i
const Coin& coin = inputs.AccessCoin(tx.vin[i].prevout);
assert(!coin.IsSpent());
const CTxOut &prevout = coin.out;
- nSigOps += CountWitnessSigOps(tx.vin[i].scriptSig, prevout.scriptPubKey, &tx.vin[i].scriptWitness, flags);
+ nSigOps += CountWitnessSigOps(tx.vin[i].scriptSig, prevout.scriptPubKey, tx.vin[i].scriptWitness, flags);
}
return nSigOps;
}
diff --git a/src/script/interpreter.cpp b/src/script/interpreter.cpp
index abd99fc3..6505ffe5 100644
--- a/src/script/interpreter.cpp
+++ b/src/script/interpreter.cpp
@@ -2132,10 +2132,8 @@ size_t static WitnessSigOps(int witversion, const std::vector<unsigned char>& wi
return 0;
}
-size_t CountWitnessSigOps(const CScript& scriptSig, const CScript& scriptPubKey, const CScriptWitness* witness, script_verify_flags flags)
+size_t CountWitnessSigOps(const CScript& scriptSig, const CScript& scriptPubKey, const CScriptWitness& witness, script_verify_flags flags)
{
- static const CScriptWitness witnessEmpty;
-
if ((flags & SCRIPT_VERIFY_WITNESS) == 0) {
return 0;
}
@@ -2144,7 +2142,7 @@ size_t CountWitnessSigOps(const CScript& scriptSig, const CScript& scriptPubKey,
int witnessversion;
std::vector<unsigned char> witnessprogram;
if (scriptPubKey.IsWitnessProgram(witnessversion, witnessprogram)) {
- return WitnessSigOps(witnessversion, witnessprogram, witness ? *witness : witnessEmpty);
+ return WitnessSigOps(witnessversion, witnessprogram, witness);
}
if (scriptPubKey.IsPayToScriptHash() && scriptSig.IsPushOnly()) {
@@ -2156,7 +2154,7 @@ size_t CountWitnessSigOps(const CScript& scriptSig, const CScript& scriptPubKey,
}
CScript subscript(data.begin(), data.end());
if (subscript.IsWitnessProgram(witnessversion, witnessprogram)) {
- return WitnessSigOps(witnessversion, witnessprogram, witness ? *witness : witnessEmpty);
+ return WitnessSigOps(witnessversion, witnessprogram, witness);
}
}
diff --git a/src/script/interpreter.h b/src/script/interpreter.h
index 7f8a1f7a..cbb137e0 100644
--- a/src/script/interpreter.h
+++ b/src/script/interpreter.h
@@ -377,7 +377,7 @@ bool EvalScript(std::vector<std::vector<unsigned char> >& stack, const CScript&
bool EvalScript(std::vector<std::vector<unsigned char> >& stack, const CScript& script, script_verify_flags flags, const BaseSignatureChecker& checker, SigVersion sigversion, ScriptError* error = nullptr);
bool VerifyScript(const CScript& scriptSig, const CScript& scriptPubKey, const CScriptWitness* witness, script_verify_flags flags, const BaseSignatureChecker& checker, ScriptError* serror = nullptr);
-size_t CountWitnessSigOps(const CScript& scriptSig, const CScript& scriptPubKey, const CScriptWitness* witness, script_verify_flags flags);
+size_t CountWitnessSigOps(const CScript& scriptSig, const CScript& scriptPubKey, const CScriptWitness& witness, script_verify_flags flags);
int FindAndDelete(CScript& script, const CScript& b);
diff --git a/src/test/fuzz/coins_view.cpp b/src/test/fuzz/coins_view.cpp
index c6870654..dceec3d2 100644
--- a/src/test/fuzz/coins_view.cpp
+++ b/src/test/fuzz/coins_view.cpp
@@ -283,7 +283,7 @@ void TestCoinsView(FuzzedDataProvider& fuzzed_data_provider, CCoinsView& backend
const auto flags = script_verify_flags::from_int(fuzzed_data_provider.ConsumeIntegral<script_verify_flags::value_type>());
if (!transaction.vin.empty() && (flags & SCRIPT_VERIFY_WITNESS) != 0 && (flags & SCRIPT_VERIFY_P2SH) == 0) {
// Avoid:
- // script/interpreter.cpp:1705: size_t CountWitnessSigOps(const CScript &, const CScript &, const CScriptWitness *, unsigned int): Assertion `(flags & SCRIPT_VERIFY_P2SH) != 0' failed.
+ // script/interpreter.cpp:1705: size_t CountWitnessSigOps(const CScript &, const CScript &, const CScriptWitness &, unsigned int): Assertion `(flags & SCRIPT_VERIFY_P2SH) != 0' failed.
return;
}
(void)GetTransactionSigOpCost(transaction, coins_view_cache, flags);
diff --git a/src/test/fuzz/script.cpp b/src/test/fuzz/script.cpp
index dfad0b71..9d3ce27a 100644
--- a/src/test/fuzz/script.cpp
+++ b/src/test/fuzz/script.cpp
@@ -125,7 +125,7 @@ FUZZ_TARGET(script, .init = initialize_script)
for (const auto& s : random_string_vector) {
wit.stack.emplace_back(s.begin(), s.end());
}
- (void)CountWitnessSigOps(script, *other_script, &wit, flags);
+ (void)CountWitnessSigOps(script, *other_script, wit, flags);
wit.SetNull();
}
}
Risk score
Our methodology →Why this scored 15/100
Human-validated context
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
No validated notes yet.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.