Skip to content

refactor: optimize: avoid allocations in script & policy verification #33645

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
Raimo33 wants to merge 4 commits into
base: master
Choose a base branch
from
Open

refactor: optimize: avoid allocations in script & policy verification #33645

Raimo33 wants to merge 4 commits into from
+21 −21

Conversation

@Raimo33
Copy link
Contributor

@Raimo33 Raimo33 commented Oct 17, 2025 •
edited
Loading

Currently, some policy and script related methods are inefficiently allocating/reallocating containers where it is completely unnecessary.

This PR aims at optimizing policy verifications by reducing redundant heap allocations without losing performance even in worst case scenarios, effectively reducing the overall memory footprint.

@DrahtBot
Copy link
Contributor

DrahtBot commented Oct 17, 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/33645.

Reviews

See the guideline for information on the review process.

Type Reviewers
Concept ACK l0rinc

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #32729 (test,refactor: extract script template helpers & widen sigop count coverage by l0rinc)
  • #29060 (Policy: Report debug message why inputs are non standard by ismaelsadeeq)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

@DrahtBot DrahtBot mentioned this pull request Oct 18, 2025
Open
cedwies
cedwies reviewed Oct 21, 2025
View reviewed changes
Copy link

@cedwies cedwies left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • reusing vector capacity across loop iterations is makes sense
  • no functional or policy logic changes introduced
  • Code refactoring (range-based loops, variable renames) improves readability

Here is my bench on MacOS M2 MAX:

Before:

ns/op op/s err% total benchmark
134,805.18 7,418.11 0.7% 3.30 AssembleBlock
338.44 2,954,694.93 0.5% 3.29 CCoinsCaching

After:

ns/op op/s err% total benchmark
134,100.45 7,457.10 0.1% 3.29 AssembleBlock
286.00 3,496,467.59 0.3% 3.29 CCoinsCaching

Not so sure about the Assemble Block bench though. Doesn't it measure block assembly after admission, therefore not timing the policy checks you changed?

@Raimo33
Copy link
Contributor Author

Raimo33 commented Oct 22, 2025

Not so sure about the Assemble Block bench though. Doesn't it measure block assembly after admission, therefore not timing the policy checks you changed?

Thanks for the review & benchmarks. I'm positive about the fact that the AssembleBlock bench calls some of the impacted functions. I've not delved into details, and I think it's irrelevant considering performance is the same.

@l0rinc
Copy link
Contributor

l0rinc commented Oct 22, 2025 •
edited
Loading

I don't really see any speedup for AssembleBlock
image

Change: gcc=-0.71%, clang=-0.82%

CCoinsCaching does seem to be improved a bit
image

Change: gcc=+18.87%, clang=+11.79%

Benchmark 
 for compiler in gcc clang; do \
  if [ "$compiler" = "gcc" ]; then CC=gcc; CXX=g++; COMP_VER=$(gcc -dumpfullversion); \
  else CC=clang; CXX=clang++; COMP_VER=$(clang -dumpversion); fi && \
  echo "> Compiler: $compiler $COMP_VER" && \
  for commit in 64a7c7cbb975cb3c3f25a3f784779f32cd95ebaa aa4c5b81ce686a160a883394f00a38604d81ccdd 7ef44fe6876dcf69f043df82a944e36a8a787b16 5d4b008728e13e923cd9d9315620b486c92b225b; do \
    git fetch origin $commit >/dev/null 2>&1 && git checkout $commit >/dev/null 2>&1 && git log -1 --pretty='%h %s' && \
    rm -rf build && \
    cmake -B build -DBUILD_BENCH=ON -DENABLE_IPC=OFF -DCMAKE_BUILD_TYPE=RelWithDebInfo -DCMAKE_C_COMPILER=$CC -DCMAKE_CXX_COMPILER=$CXX >/dev/null 2>&1 && \
    cmake --build build -j$(nproc) >/dev/null 2>&1 && \
    for i in 1 2; do \
      build/bin/bench_bitcoin -filter='AssembleBlock|CCoinsCaching' -min-time=5000; \
    done; \
  done; \
done
Measurements

Compiler: gcc 15.0.1

64a7c7c Merge #33558: ci: Use native platform for win-cross task

ns/op op/s err% ins/op cyc/op IPC bra/op miss% total benchmark
332,700.15 3,005.71 0.1% 2,892,252.71 1,193,396.57 2.424 262,758.41 0.4% 5.31 AssembleBlock
613.60 1,629,733.71 0.1% 5,790.00 2,204.34 2.627 819.00 0.1% 5.50 CCoinsCaching
ns/op op/s err% ins/op cyc/op IPC bra/op miss% total benchmark
333,387.37 2,999.51 0.0% 2,889,152.01 1,195,756.63 2.416 262,647.34 0.4% 5.32 AssembleBlock
620.71 1,611,064.09 0.1% 5,790.00 2,229.94 2.596 819.00 0.2% 5.51 CCoinsCaching

aa4c5b81ce refactor: use range-based iteration

ns/op op/s err% ins/op cyc/op IPC bra/op miss% total benchmark
336,394.98 2,972.70 0.0% 2,892,400.57 1,206,262.36 2.398 262,630.80 0.4% 5.31 AssembleBlock
525.13 1,904,279.72 0.1% 5,229.00 1,886.12 2.772 802.00 0.1% 5.50 CCoinsCaching
ns/op op/s err% ins/op cyc/op IPC bra/op miss% total benchmark
336,470.91 2,972.03 0.1% 2,887,727.08 1,206,756.91 2.393 262,840.00 0.4% 5.21 AssembleBlock
605.07 1,652,703.31 0.0% 5,759.00 2,173.77 2.649 816.00 0.1% 5.48 CCoinsCaching

7ef44fe687 refactor: rename script stack

ns/op op/s err% ins/op cyc/op IPC bra/op miss% total benchmark
332,237.42 3,009.90 0.0% 2,891,082.62 1,191,956.55 2.425 262,820.66 0.4% 5.31 AssembleBlock
525.33 1,903,566.06 0.1% 5,229.00 1,887.29 2.771 802.00 0.1% 5.50 CCoinsCaching
ns/op op/s err% ins/op cyc/op IPC bra/op miss% total benchmark
330,947.55 3,021.63 0.0% 2,888,455.08 1,187,248.72 2.433 262,606.41 0.4% 5.30 AssembleBlock
526.32 1,899,969.76 0.0% 5,229.00 1,890.73 2.766 802.00 0.1% 5.50 CCoinsCaching

5d4b008728 optimize: reuse containers across iterations

ns/op op/s err% ins/op cyc/op IPC bra/op miss% total benchmark
334,038.19 2,993.67 0.0% 2,894,146.80 1,198,114.94 2.416 262,962.33 0.5% 5.31 AssembleBlock
562.38 1,778,156.15 0.1% 5,412.00 2,019.23 2.680 736.00 0.0% 5.49 CCoinsCaching
ns/op op/s err% ins/op cyc/op IPC bra/op miss% total benchmark
336,807.71 2,969.05 0.0% 2,894,460.95 1,208,318.07 2.395 262,725.50 0.4% 5.31 AssembleBlock
482.11 2,074,216.12 0.0% 4,882.00 1,731.89 2.819 722.00 0.0% 5.50 CCoinsCaching

Compiler: clang 22.0.0

64a7c7c Merge #33558: ci: Use native platform for win-cross task

ns/op op/s err% ins/op cyc/op IPC bra/op miss% total benchmark
316,532.30 3,159.24 0.0% 2,799,107.72 1,135,483.03 2.465 252,868.15 0.4% 5.29 AssembleBlock
482.62 2,072,038.85 0.2% 4,715.00 1,733.69 2.720 688.00 0.2% 5.51 CCoinsCaching
ns/op op/s err% ins/op cyc/op IPC bra/op miss% total benchmark
319,172.99 3,133.10 0.1% 2,804,493.20 1,145,219.92 2.449 252,971.35 0.4% 5.29 AssembleBlock
484.08 2,065,772.51 0.1% 4,715.00 1,738.81 2.712 688.00 0.1% 5.50 CCoinsCaching

aa4c5b81ce refactor: use range-based iteration

ns/op op/s err% ins/op cyc/op IPC bra/op miss% total benchmark
319,298.02 3,131.87 0.1% 2,806,937.58 1,145,187.88 2.451 253,098.73 0.4% 5.29 AssembleBlock
491.35 2,035,216.81 0.1% 4,686.00 1,765.17 2.655 687.00 0.2% 5.51 CCoinsCaching
ns/op op/s err% ins/op cyc/op IPC bra/op miss% total benchmark
319,413.24 3,130.74 0.1% 2,798,169.36 1,145,659.16 2.442 252,777.51 0.4% 5.29 AssembleBlock
496.25 2,015,105.84 0.0% 4,686.00 1,782.54 2.629 687.00 0.2% 5.50 CCoinsCaching

7ef44fe687 refactor: rename script stack

ns/op op/s err% ins/op cyc/op IPC bra/op miss% total benchmark
319,993.93 3,125.06 0.1% 2,801,660.61 1,147,280.13 2.442 252,912.30 0.4% 5.30 AssembleBlock
489.84 2,041,475.07 0.1% 4,686.00 1,759.79 2.663 687.00 0.2% 5.49 CCoinsCaching
ns/op op/s err% ins/op cyc/op IPC bra/op miss% total benchmark
317,083.74 3,153.74 0.1% 2,802,373.03 1,136,973.75 2.465 252,921.05 0.4% 5.29 AssembleBlock
490.88 2,037,151.54 0.1% 4,686.00 1,763.06 2.658 687.00 0.2% 5.50 CCoinsCaching

5d4b008728 optimize: reuse containers across iterations

ns/op op/s err% ins/op cyc/op IPC bra/op miss% total benchmark
321,831.90 3,107.21 0.1% 2,804,969.78 1,154,021.10 2.431 252,995.03 0.4% 5.30 AssembleBlock
432.01 2,314,757.74 0.0% 4,301.00 1,551.96 2.771 608.00 0.0% 5.41 CCoinsCaching
ns/op op/s err% ins/op cyc/op IPC bra/op miss% total benchmark
319,109.25 3,133.72 0.1% 2,797,937.98 1,144,492.87 2.445 252,714.07 0.4% 5.30 AssembleBlock
432.73 2,310,885.42 0.3% 4,301.00 1,554.58 2.767 608.00 0.0% 5.42 CCoinsCaching

But the code seems more complicated than before - is there a way to retain the speedup without complicating the code (i.e. minimizing the diff)?

@cedwies
Copy link

cedwies commented Oct 22, 2025 •
edited
Loading

Not so sure about the Assemble Block bench though. Doesn't it measure block assembly after admission, therefore not timing the policy checks you changed?

Thanks for the review & benchmarks. I'm positive about the fact that the AssembleBlock bench calls some of the impacted functions. I've not delved into details, and I think it's irrelevant considering performance is the same.

It does call code on the policy path, but only during setup, not during the timed region.
In static void AssembleBlock(benchmark::Bench& bench) in src/bench/block_assemble.cpp

    bench.run([&] {
        PrepareBlock(test_setup->m_node, options);
    });

Don't want to be nitpicking here, but just want to point out that if a bench is presented as evidence of a performance claim (faster/slower/same), it should measure the code which was changed. Fine if we use it as a sanity-check, just wanted to mention that AssembleBlock is (AFAIK) expected to stay the same, since no timed code changes. On the other hand CCoinsCaching actually times AreInputsStandard(...)

@Raimo33
Copy link
Contributor Author

Raimo33 commented Oct 22, 2025

@l0rinc @cedwies the bench is not presented as evidence. it is presented as a transparent way of saying performance hasn't degraded. the PR body clearly states that it remained the same. I included it for the sake of transparency.

@cedwies
Copy link

cedwies commented Oct 22, 2025 •
edited
Loading

But the code seems more complicated than before - is there a way to retain the speedup without complicating the code (i.e. minimizing the diff)?

It is more complicated. I would:

Keep:

  • change from index-based to range-based loops.
  • In AreInputsStandard(...): Move vSolutions outside the for loop (this brings the performance gain in CCoinsCaching)
  • In IsWitnessStandard(...): Keep the variable name change from stack to script_stack

Discard to minimize diff (these changes don't give a (relevant) performance gain):

  • In AreInputsStandard(...): Don't move the stack outside the for loop
  • In IsWitnessStandard(...): Don't move stack and witnessprogram outside the for loop
  • In SpendsNonAnchorWitnessProg(...): Don't move stack outside the for loop.

Optional: Add a comment/hint to clarify that moving vSolutions outside the loop is intentional?

This way we minimize the diff, avoiding making the code more complicated than necessary, while taking advantage of the performance gain. Happy to hear your thoughts.

@Raimo33
Copy link
Contributor Author

Raimo33 commented Oct 22, 2025

  • In AreInputsStandard(...): Don't move the stack outside the for loop

Agreed: it is not in the hot path (or the most likely path).

  • In IsWitnessStandard(...): Don't move stack and witnessprogram outside the for loop

Agreed for stack: it is only needed for P2SH which is unlikely
Disagree for witnessprogram: it is used in the hot path (P2WPKH is the most likely)

  • In SpendsNonAnchorWitnessProg(...): Don't move stack outside the for loop.

Agreed: it is not in the hot path (or the most likely path).

@l0rinc
Copy link
Contributor

l0rinc commented Oct 22, 2025

Since std::vector only allocates on first use

Actually it can reallocate on every size exhaustion, i.e. every resize.
I don't like the new reuse, that can actually be slower than before because now the code paths depend on each other.
Wouldn't reserving the vectors help here instead?

@Raimo33 Raimo33 force-pushed the optimize-tx-policy-verification branch from 5d4b008 to cedfff1 Compare October 22, 2025 14:50
@Raimo33
Copy link
Contributor Author

Raimo33 commented Oct 22, 2025

I've removed some of the less impacting changes, considering @cedwies suggestions. It now shows a 9% improvement on CCoinsCache on my end (up from a 7.7% previously)

@Raimo33
Copy link
Contributor Author

Raimo33 commented Oct 22, 2025 •
edited
Loading

now the code paths depend on each other

That's not the case, at least not with the latest push:

  • vSolutions is used regardless of the code path. It's used before branching.
  • witnessprogram is used late, after a couple of branches, but this can't possibly impact performance since std::vector starts allocating only after first use.

Wouldn't reserving the vectors help here instead?

This was my initial intuition, but it actually goes against your previous point. .reserve() would mean allocating regardless of the code path. It would mean allocating even when unnecessary.

@cedwies
Copy link

cedwies commented Oct 22, 2025

Actually it can reallocate on every size exhaustion, i.e. every resize.
I don't like the new reuse, that can actually be slower than before because now the code paths depend on each other.
Wouldn't reserving the vectors help here instead?

You are referring to iteration-to-iteration coupling? Yes, the iterations depend on each other, however in practice we still allocate less than before, so there should not be a slowdown. Are you suggesting something like:

witnessprogram.reserve(40); // segwit witness program length <= 40 bytes

? I would not use reserve() for vSolutions because it would be mostly guesswork with little payoff and hoisting alone already gets the win in performance.
Or am I missing your point @l0rinc ?

@l0rinc
Copy link
Contributor

l0rinc commented Oct 23, 2025

I like the new version better, you have indeed identified an inconsistency that we could fix in a way that would make the source could also more readable.

However, I think the current version still makes the code less readable than it was before - now the loop iterations depend on each other by reusing the same vector. However, vSolutions that you're reusing isn't actually needed at all (maybe that's what the compiler detected after your change). The cleaner alternative (that I also bumped into previously) is to make the vector optional:

Cleaner alternative 
diff --git a/src/addresstype.cpp b/src/addresstype.cpp
index 67e643943d..979983b705 100644
--- a/src/addresstype.cpp
+++ b/src/addresstype.cpp
@@ -49,7 +49,7 @@ WitnessV0ScriptHash::WitnessV0ScriptHash(const CScript& in)
 bool ExtractDestination(const CScript& scriptPubKey, CTxDestination& addressRet)
 {
     std::vector<valtype> vSolutions;
-    TxoutType whichType = Solver(scriptPubKey, vSolutions);
+    TxoutType whichType = Solver(scriptPubKey, &vSolutions);
 
     switch (whichType) {
     case TxoutType::PUBKEY: {
diff --git a/src/common/bloom.cpp b/src/common/bloom.cpp
index efb4178cab..c4133221c0 100644
--- a/src/common/bloom.cpp
+++ b/src/common/bloom.cpp
@@ -123,8 +123,7 @@ bool CBloomFilter::IsRelevantAndUpdate(const CTransaction& tx)
                     insert(COutPoint(hash, i));
                 else if ((nFlags & BLOOM_UPDATE_MASK) == BLOOM_UPDATE_P2PUBKEY_ONLY)
                 {
-                    std::vector<std::vector<unsigned char> > vSolutions;
-                    TxoutType type = Solver(txout.scriptPubKey, vSolutions);
+                    TxoutType type = Solver(txout.scriptPubKey);
                     if (type == TxoutType::PUBKEY || type == TxoutType::MULTISIG) {
                         insert(COutPoint(hash, i));
                     }
diff --git a/src/core_write.cpp b/src/core_write.cpp
index 14836f5148..4aea803997 100644
--- a/src/core_write.cpp
+++ b/src/core_write.cpp
@@ -159,8 +159,7 @@ void ScriptToUniv(const CScript& script, UniValue& out, bool include_hex, bool i
         out.pushKV("hex", HexStr(script));
     }
 
-    std::vector<std::vector<unsigned char>> solns;
-    const TxoutType type{Solver(script, solns)};
+    const TxoutType type{Solver(script)};
 
     if (include_address && ExtractDestination(script, address) && type != TxoutType::PUBKEY) {
         out.pushKV("address", EncodeDestination(address));
diff --git a/src/policy/policy.cpp b/src/policy/policy.cpp
index 454f3f9021..8f1c0e9b8d 100644
--- a/src/policy/policy.cpp
+++ b/src/policy/policy.cpp
@@ -79,7 +79,7 @@ std::vector<uint32_t> GetDust(const CTransaction& tx, CFeeRate dust_relay_rate)
 bool IsStandard(const CScript& scriptPubKey, TxoutType& whichType)
 {
     std::vector<std::vector<unsigned char> > vSolutions;
-    whichType = Solver(scriptPubKey, vSolutions);
+    whichType = Solver(scriptPubKey, &vSolutions);
 
     if (whichType == TxoutType::NONSTANDARD) {
         return false;
@@ -220,12 +220,10 @@ bool AreInputsStandard(const CTransaction& tx, const CCoinsViewCache& mapInputs)
         return false;
     }
 
-    std::vector<std::vector<unsigned char> > vSolutions;
     for (const CTxIn& txin : tx.vin) {
         const CTxOut& prev = mapInputs.AccessCoin(txin.prevout).out;
 
-        vSolutions.clear();
-        TxoutType whichType = Solver(prev.scriptPubKey, vSolutions);
+        TxoutType whichType = Solver(prev.scriptPubKey);
         if (whichType == TxoutType::NONSTANDARD || whichType == TxoutType::WITNESS_UNKNOWN) {
             // WITNESS_UNKNOWN failures are typically also caught with a policy
             // flag in the script interpreter, but it can be helpful to catch
diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp
index 1a8edc594f..4d1a90bf47 100644
--- a/src/rpc/rawtransaction.cpp
+++ b/src/rpc/rawtransaction.cpp
@@ -534,7 +534,7 @@ static RPCHelpMan decodescript()
     ScriptToUniv(script, /*out=*/r, /*include_hex=*/false, /*include_address=*/true);
 
     std::vector<std::vector<unsigned char>> solutions_data;
-    const TxoutType which_type{Solver(script, solutions_data)};
+    const TxoutType which_type{Solver(script, &solutions_data)};
 
     const bool can_wrap{[&] {
         switch (which_type) {
diff --git a/src/script/descriptor.cpp b/src/script/descriptor.cpp
index bd819d365a..5ba2d43576 100644
--- a/src/script/descriptor.cpp
+++ b/src/script/descriptor.cpp
@@ -2569,7 +2569,7 @@ std::unique_ptr<DescriptorImpl> InferScript(const CScript& script, ParseScriptCo
     }
 
     std::vector<std::vector<unsigned char>> data;
-    TxoutType txntype = Solver(script, data);
+    TxoutType txntype = Solver(script, &data);
 
     if (txntype == TxoutType::PUBKEY && (ctx == ParseScriptContext::TOP || ctx == ParseScriptContext::P2SH || ctx == ParseScriptContext::P2WSH)) {
         CPubKey pubkey(data[0]);
diff --git a/src/script/sign.cpp b/src/script/sign.cpp
index 33cbc38be4..980ddc9abd 100644
--- a/src/script/sign.cpp
+++ b/src/script/sign.cpp
@@ -406,7 +406,7 @@ static bool SignStep(const SigningProvider& provider, const BaseSignatureCreator
     std::vector<unsigned char> sig;
 
     std::vector<valtype> vSolutions;
-    whichTypeRet = Solver(scriptPubKey, vSolutions);
+    whichTypeRet = Solver(scriptPubKey, &vSolutions);
 
     switch (whichTypeRet) {
     case TxoutType::NONSTANDARD:
@@ -625,7 +625,7 @@ SignatureData DataFromTransaction(const CMutableTransaction& tx, unsigned int nI
 
     // Get scripts
     std::vector<std::vector<unsigned char>> solutions;
-    TxoutType script_type = Solver(txout.scriptPubKey, solutions);
+    TxoutType script_type = Solver(txout.scriptPubKey, &solutions);
     SigVersion sigversion = SigVersion::BASE;
     CScript next_script = txout.scriptPubKey;
 
@@ -636,7 +636,7 @@ SignatureData DataFromTransaction(const CMutableTransaction& tx, unsigned int nI
         next_script = std::move(redeem_script);
 
         // Get redeemScript type
-        script_type = Solver(next_script, solutions);
+        script_type = Solver(next_script, &solutions);
         stack.script.pop_back();
     }
     if (script_type == TxoutType::WITNESS_V0_SCRIPTHASH && !stack.witness.empty() && !stack.witness.back().empty()) {
@@ -646,7 +646,7 @@ SignatureData DataFromTransaction(const CMutableTransaction& tx, unsigned int nI
         next_script = std::move(witness_script);
 
         // Get witnessScript type
-        script_type = Solver(next_script, solutions);
+        script_type = Solver(next_script, &solutions);
         stack.witness.pop_back();
         stack.script = std::move(stack.witness);
         stack.witness.clear();
@@ -751,7 +751,7 @@ bool IsSegWitOutput(const SigningProvider& provider, const CScript& script)
     if (script.IsWitnessProgram(version, program)) return true;
     if (script.IsPayToScriptHash()) {
         std::vector<valtype> solutions;
-        auto whichtype = Solver(script, solutions);
+        auto whichtype = Solver(script, &solutions);
         if (whichtype == TxoutType::SCRIPTHASH) {
             auto h160 = uint160(solutions[0]);
             CScript subscript;
diff --git a/src/script/solver.cpp b/src/script/solver.cpp
index 783baf0708..2e45c7d305 100644
--- a/src/script/solver.cpp
+++ b/src/script/solver.cpp
@@ -138,16 +138,15 @@ std::optional<std::pair<int, std::vector<std::span<const unsigned char>>>> Match
     return std::pair{*threshold, std::move(keyspans)};
 }
 
-TxoutType Solver(const CScript& scriptPubKey, std::vector<std::vector<unsigned char>>& vSolutionsRet)
+TxoutType Solver(const CScript& scriptPubKey, std::vector<std::vector<unsigned char>>* vSolutionsRet)
 {
-    vSolutionsRet.clear();
+    if (vSolutionsRet) vSolutionsRet->clear();
 
     // Shortcut for pay-to-script-hash, which are more constrained than the other types:
     // it is always OP_HASH160 20 [20 byte hash] OP_EQUAL
-    if (scriptPubKey.IsPayToScriptHash())
-    {
+    if (scriptPubKey.IsPayToScriptHash()) {
         std::vector<unsigned char> hashBytes(scriptPubKey.begin()+2, scriptPubKey.begin()+22);
-        vSolutionsRet.push_back(hashBytes);
+        if (vSolutionsRet) vSolutionsRet->push_back(hashBytes);
         return TxoutType::SCRIPTHASH;
     }
 
@@ -155,23 +154,25 @@ TxoutType Solver(const CScript& scriptPubKey, std::vector<std::vector<unsigned c
     std::vector<unsigned char> witnessprogram;
     if (scriptPubKey.IsWitnessProgram(witnessversion, witnessprogram)) {
         if (witnessversion == 0 && witnessprogram.size() == WITNESS_V0_KEYHASH_SIZE) {
-            vSolutionsRet.push_back(std::move(witnessprogram));
+            if (vSolutionsRet) vSolutionsRet->push_back(std::move(witnessprogram));
             return TxoutType::WITNESS_V0_KEYHASH;
         }
         if (witnessversion == 0 && witnessprogram.size() == WITNESS_V0_SCRIPTHASH_SIZE) {
-            vSolutionsRet.push_back(std::move(witnessprogram));
+            if (vSolutionsRet) vSolutionsRet->push_back(std::move(witnessprogram));
             return TxoutType::WITNESS_V0_SCRIPTHASH;
         }
         if (witnessversion == 1 && witnessprogram.size() == WITNESS_V1_TAPROOT_SIZE) {
-            vSolutionsRet.push_back(std::move(witnessprogram));
+            if (vSolutionsRet) vSolutionsRet->push_back(std::move(witnessprogram));
             return TxoutType::WITNESS_V1_TAPROOT;
         }
         if (scriptPubKey.IsPayToAnchor()) {
             return TxoutType::ANCHOR;
         }
         if (witnessversion != 0) {
-            vSolutionsRet.push_back(std::vector<unsigned char>{(unsigned char)witnessversion});
-            vSolutionsRet.push_back(std::move(witnessprogram));
+            if (vSolutionsRet) {
+                vSolutionsRet->push_back(std::vector<unsigned char>{(unsigned char)witnessversion});
+                vSolutionsRet->push_back(std::move(witnessprogram));
+            }
             return TxoutType::WITNESS_UNKNOWN;
         }
         return TxoutType::NONSTANDARD;
@@ -188,25 +189,27 @@ TxoutType Solver(const CScript& scriptPubKey, std::vector<std::vector<unsigned c
 
     std::vector<unsigned char> data;
     if (MatchPayToPubkey(scriptPubKey, data)) {
-        vSolutionsRet.push_back(std::move(data));
+        if (vSolutionsRet) vSolutionsRet->push_back(std::move(data));
         return TxoutType::PUBKEY;
     }
 
     if (MatchPayToPubkeyHash(scriptPubKey, data)) {
-        vSolutionsRet.push_back(std::move(data));
+        if (vSolutionsRet) vSolutionsRet->push_back(std::move(data));
         return TxoutType::PUBKEYHASH;
     }
 
     int required;
     std::vector<std::vector<unsigned char>> keys;
     if (MatchMultisig(scriptPubKey, required, keys)) {
-        vSolutionsRet.push_back({static_cast<unsigned char>(required)}); // safe as required is in range 1..20
-        vSolutionsRet.insert(vSolutionsRet.end(), keys.begin(), keys.end());
-        vSolutionsRet.push_back({static_cast<unsigned char>(keys.size())}); // safe as size is in range 1..20
+        if (vSolutionsRet) {
+            vSolutionsRet->push_back({static_cast<unsigned char>(required)}); // safe as required is in range 1..20
+            vSolutionsRet->insert(vSolutionsRet->end(), keys.begin(), keys.end());
+            vSolutionsRet->push_back({static_cast<unsigned char>(keys.size())}); // safe as size is in range 1..20
+        }
         return TxoutType::MULTISIG;
     }
 
-    vSolutionsRet.clear();
+    if (vSolutionsRet) vSolutionsRet->clear();
     return TxoutType::NONSTANDARD;
 }
 
diff --git a/src/script/solver.h b/src/script/solver.h
index d2b7fb8881..f708a00f82 100644
--- a/src/script/solver.h
+++ b/src/script/solver.h
@@ -52,7 +52,7 @@ constexpr bool IsPushdataOp(opcodetype opcode)
  * @param[out]  vSolutionsRet  Vector of parsed pubkeys and hashes
  * @return                     The script type. TxoutType::NONSTANDARD represents a failed solve.
  */
-TxoutType Solver(const CScript& scriptPubKey, std::vector<std::vector<unsigned char>>& vSolutionsRet);
+TxoutType Solver(const CScript& scriptPubKey, std::vector<std::vector<unsigned char>>* vSolutionsRet = nullptr);
 
 /** Generate a P2PK script for the given pubkey. */
 CScript GetScriptForRawPubKey(const CPubKey& pubkey);
diff --git a/src/test/fuzz/key.cpp b/src/test/fuzz/key.cpp
index e4bedff8b5..5660310935 100644
--- a/src/test/fuzz/key.cpp
+++ b/src/test/fuzz/key.cpp
@@ -160,13 +160,13 @@ FUZZ_TARGET(key, .init = initialize_key)
         assert(which_type_tx_multisig == TxoutType::MULTISIG);
 
         std::vector<std::vector<unsigned char>> v_solutions_ret_tx_pubkey;
-        const TxoutType outtype_tx_pubkey = Solver(tx_pubkey_script, v_solutions_ret_tx_pubkey);
+        const TxoutType outtype_tx_pubkey = Solver(tx_pubkey_script, &v_solutions_ret_tx_pubkey);
         assert(outtype_tx_pubkey == TxoutType::PUBKEY);
         assert(v_solutions_ret_tx_pubkey.size() == 1);
         assert(v_solutions_ret_tx_pubkey[0].size() == 33);
 
         std::vector<std::vector<unsigned char>> v_solutions_ret_tx_multisig;
-        const TxoutType outtype_tx_multisig = Solver(tx_multisig_script, v_solutions_ret_tx_multisig);
+        const TxoutType outtype_tx_multisig = Solver(tx_multisig_script, &v_solutions_ret_tx_multisig);
         assert(outtype_tx_multisig == TxoutType::MULTISIG);
         assert(v_solutions_ret_tx_multisig.size() == 3);
         assert(v_solutions_ret_tx_multisig[0].size() == 1);
diff --git a/src/test/fuzz/script.cpp b/src/test/fuzz/script.cpp
index dfad0b7184..38608b48be 100644
--- a/src/test/fuzz/script.cpp
+++ b/src/test/fuzz/script.cpp
@@ -91,7 +91,8 @@ FUZZ_TARGET(script, .init = initialize_script)
     (void)RecursiveDynamicUsage(script);
 
     std::vector<std::vector<unsigned char>> solutions;
-    (void)Solver(script, solutions);
+    (void)Solver(script);
+    (void)Solver(script, &solutions);
 
     (void)script.HasValidOps();
     (void)script.IsPayToAnchor();
diff --git a/src/test/script_standard_tests.cpp b/src/test/script_standard_tests.cpp
index 9a63426e7d..49d7b164bc 100644
--- a/src/test/script_standard_tests.cpp
+++ b/src/test/script_standard_tests.cpp
@@ -42,14 +42,14 @@ BOOST_AUTO_TEST_CASE(script_standard_Solver_success)
     // TxoutType::PUBKEY
     s.clear();
     s << ToByteVector(pubkeys[0]) << OP_CHECKSIG;
-    BOOST_CHECK_EQUAL(Solver(s, solutions), TxoutType::PUBKEY);
+    BOOST_CHECK_EQUAL(Solver(s, &solutions), TxoutType::PUBKEY);
     BOOST_CHECK_EQUAL(solutions.size(), 1U);
     BOOST_CHECK(solutions[0] == ToByteVector(pubkeys[0]));
 
     // TxoutType::PUBKEYHASH
     s.clear();
     s << OP_DUP << OP_HASH160 << ToByteVector(pubkeys[0].GetID()) << OP_EQUALVERIFY << OP_CHECKSIG;
-    BOOST_CHECK_EQUAL(Solver(s, solutions), TxoutType::PUBKEYHASH);
+    BOOST_CHECK_EQUAL(Solver(s, &solutions), TxoutType::PUBKEYHASH);
     BOOST_CHECK_EQUAL(solutions.size(), 1U);
     BOOST_CHECK(solutions[0] == ToByteVector(pubkeys[0].GetID()));
 
@@ -57,7 +57,7 @@ BOOST_AUTO_TEST_CASE(script_standard_Solver_success)
     CScript redeemScript(s); // initialize with leftover P2PKH script
     s.clear();
     s << OP_HASH160 << ToByteVector(CScriptID(redeemScript)) << OP_EQUAL;
-    BOOST_CHECK_EQUAL(Solver(s, solutions), TxoutType::SCRIPTHASH);
+    BOOST_CHECK_EQUAL(Solver(s, &solutions), TxoutType::SCRIPTHASH);
     BOOST_CHECK_EQUAL(solutions.size(), 1U);
     BOOST_CHECK(solutions[0] == ToByteVector(CScriptID(redeemScript)));
 
@@ -67,7 +67,7 @@ BOOST_AUTO_TEST_CASE(script_standard_Solver_success)
         ToByteVector(pubkeys[0]) <<
         ToByteVector(pubkeys[1]) <<
         OP_2 << OP_CHECKMULTISIG;
-    BOOST_CHECK_EQUAL(Solver(s, solutions), TxoutType::MULTISIG);
+    BOOST_CHECK_EQUAL(Solver(s, &solutions), TxoutType::MULTISIG);
     BOOST_CHECK_EQUAL(solutions.size(), 4U);
     BOOST_CHECK(solutions[0] == std::vector<unsigned char>({1}));
     BOOST_CHECK(solutions[1] == ToByteVector(pubkeys[0]));
@@ -80,7 +80,7 @@ BOOST_AUTO_TEST_CASE(script_standard_Solver_success)
         ToByteVector(pubkeys[1]) <<
         ToByteVector(pubkeys[2]) <<
         OP_3 << OP_CHECKMULTISIG;
-    BOOST_CHECK_EQUAL(Solver(s, solutions), TxoutType::MULTISIG);
+    BOOST_CHECK_EQUAL(Solver(s, &solutions), TxoutType::MULTISIG);
     BOOST_CHECK_EQUAL(solutions.size(), 5U);
     BOOST_CHECK(solutions[0] == std::vector<unsigned char>({2}));
     BOOST_CHECK(solutions[1] == ToByteVector(pubkeys[0]));
@@ -94,13 +94,13 @@ BOOST_AUTO_TEST_CASE(script_standard_Solver_success)
         std::vector<unsigned char>({0}) <<
         std::vector<unsigned char>({75}) <<
         std::vector<unsigned char>({255});
-    BOOST_CHECK_EQUAL(Solver(s, solutions), TxoutType::NULL_DATA);
+    BOOST_CHECK_EQUAL(Solver(s, &solutions), TxoutType::NULL_DATA);
     BOOST_CHECK_EQUAL(solutions.size(), 0U);
 
     // TxoutType::WITNESS_V0_KEYHASH
     s.clear();
     s << OP_0 << ToByteVector(pubkeys[0].GetID());
-    BOOST_CHECK_EQUAL(Solver(s, solutions), TxoutType::WITNESS_V0_KEYHASH);
+    BOOST_CHECK_EQUAL(Solver(s, &solutions), TxoutType::WITNESS_V0_KEYHASH);
     BOOST_CHECK_EQUAL(solutions.size(), 1U);
     BOOST_CHECK(solutions[0] == ToByteVector(pubkeys[0].GetID()));
 
@@ -111,21 +111,21 @@ BOOST_AUTO_TEST_CASE(script_standard_Solver_success)
 
     s.clear();
     s << OP_0 << ToByteVector(scriptHash);
-    BOOST_CHECK_EQUAL(Solver(s, solutions), TxoutType::WITNESS_V0_SCRIPTHASH);
+    BOOST_CHECK_EQUAL(Solver(s, &solutions), TxoutType::WITNESS_V0_SCRIPTHASH);
     BOOST_CHECK_EQUAL(solutions.size(), 1U);
     BOOST_CHECK(solutions[0] == ToByteVector(scriptHash));
 
     // TxoutType::WITNESS_V1_TAPROOT
     s.clear();
     s << OP_1 << ToByteVector(uint256::ZERO);
-    BOOST_CHECK_EQUAL(Solver(s, solutions), TxoutType::WITNESS_V1_TAPROOT);
+    BOOST_CHECK_EQUAL(Solver(s, &solutions), TxoutType::WITNESS_V1_TAPROOT);
     BOOST_CHECK_EQUAL(solutions.size(), 1U);
     BOOST_CHECK(solutions[0] == ToByteVector(uint256::ZERO));
 
     // TxoutType::WITNESS_UNKNOWN
     s.clear();
     s << OP_16 << ToByteVector(uint256::ONE);
-    BOOST_CHECK_EQUAL(Solver(s, solutions), TxoutType::WITNESS_UNKNOWN);
+    BOOST_CHECK_EQUAL(Solver(s, &solutions), TxoutType::WITNESS_UNKNOWN);
     BOOST_CHECK_EQUAL(solutions.size(), 2U);
     BOOST_CHECK(solutions[0] == std::vector<unsigned char>{16});
     BOOST_CHECK(solutions[1] == ToByteVector(uint256::ONE));
@@ -133,7 +133,7 @@ BOOST_AUTO_TEST_CASE(script_standard_Solver_success)
     // TxoutType::ANCHOR
     s.clear();
     s << OP_1 << ANCHOR_BYTES;
-    BOOST_CHECK_EQUAL(Solver(s, solutions), TxoutType::ANCHOR);
+    BOOST_CHECK_EQUAL(Solver(s, &solutions), TxoutType::ANCHOR);
     BOOST_CHECK(solutions.empty());
 
     // Sanity-check IsPayToAnchor
@@ -146,7 +146,7 @@ BOOST_AUTO_TEST_CASE(script_standard_Solver_success)
     // TxoutType::NONSTANDARD
     s.clear();
     s << OP_9 << OP_ADD << OP_11 << OP_EQUAL;
-    BOOST_CHECK_EQUAL(Solver(s, solutions), TxoutType::NONSTANDARD);
+    BOOST_CHECK_EQUAL(Solver(s, &solutions), TxoutType::NONSTANDARD);
 }
 
 BOOST_AUTO_TEST_CASE(script_standard_Solver_failure)
@@ -160,67 +160,67 @@ BOOST_AUTO_TEST_CASE(script_standard_Solver_failure)
     // TxoutType::PUBKEY with incorrectly sized pubkey
     s.clear();
     s << std::vector<unsigned char>(30, 0x01) << OP_CHECKSIG;
-    BOOST_CHECK_EQUAL(Solver(s, solutions), TxoutType::NONSTANDARD);
+    BOOST_CHECK_EQUAL(Solver(s, &solutions), TxoutType::NONSTANDARD);
 
     // TxoutType::PUBKEYHASH with incorrectly sized key hash
     s.clear();
     s << OP_DUP << OP_HASH160 << ToByteVector(pubkey) << OP_EQUALVERIFY << OP_CHECKSIG;
-    BOOST_CHECK_EQUAL(Solver(s, solutions), TxoutType::NONSTANDARD);
+    BOOST_CHECK_EQUAL(Solver(s, &solutions), TxoutType::NONSTANDARD);
 
     // TxoutType::SCRIPTHASH with incorrectly sized script hash
     s.clear();
     s << OP_HASH160 << std::vector<unsigned char>(21, 0x01) << OP_EQUAL;
-    BOOST_CHECK_EQUAL(Solver(s, solutions), TxoutType::NONSTANDARD);
+    BOOST_CHECK_EQUAL(Solver(s, &solutions), TxoutType::NONSTANDARD);
 
     // TxoutType::MULTISIG 0/2
     s.clear();
     s << OP_0 << ToByteVector(pubkey) << OP_1 << OP_CHECKMULTISIG;
-    BOOST_CHECK_EQUAL(Solver(s, solutions), TxoutType::NONSTANDARD);
+    BOOST_CHECK_EQUAL(Solver(s, &solutions), TxoutType::NONSTANDARD);
 
     // TxoutType::MULTISIG 2/1
     s.clear();
     s << OP_2 << ToByteVector(pubkey) << OP_1 << OP_CHECKMULTISIG;
-    BOOST_CHECK_EQUAL(Solver(s, solutions), TxoutType::NONSTANDARD);
+    BOOST_CHECK_EQUAL(Solver(s, &solutions), TxoutType::NONSTANDARD);
 
     // TxoutType::MULTISIG n = 2 with 1 pubkey
     s.clear();
     s << OP_1 << ToByteVector(pubkey) << OP_2 << OP_CHECKMULTISIG;
-    BOOST_CHECK_EQUAL(Solver(s, solutions), TxoutType::NONSTANDARD);
+    BOOST_CHECK_EQUAL(Solver(s, &solutions), TxoutType::NONSTANDARD);
 
     // TxoutType::MULTISIG n = 1 with 0 pubkeys
     s.clear();
     s << OP_1 << OP_1 << OP_CHECKMULTISIG;
-    BOOST_CHECK_EQUAL(Solver(s, solutions), TxoutType::NONSTANDARD);
+    BOOST_CHECK_EQUAL(Solver(s, &solutions), TxoutType::NONSTANDARD);
 
     // TxoutType::NULL_DATA with other opcodes
     s.clear();
     s << OP_RETURN << std::vector<unsigned char>({75}) << OP_ADD;
-    BOOST_CHECK_EQUAL(Solver(s, solutions), TxoutType::NONSTANDARD);
+    BOOST_CHECK_EQUAL(Solver(s, &solutions), TxoutType::NONSTANDARD);
 
     // TxoutType::WITNESS_V0_{KEY,SCRIPT}HASH with incorrect program size (-> consensus-invalid, i.e. non-standard)
     s.clear();
     s << OP_0 << std::vector<unsigned char>(19, 0x01);
-    BOOST_CHECK_EQUAL(Solver(s, solutions), TxoutType::NONSTANDARD);
+    BOOST_CHECK_EQUAL(Solver(s, &solutions), TxoutType::NONSTANDARD);
 
     // TxoutType::WITNESS_V1_TAPROOT with incorrect program size (-> undefined, but still policy-valid)
     s.clear();
     s << OP_1 << std::vector<unsigned char>(31, 0x01);
-    BOOST_CHECK_EQUAL(Solver(s, solutions), TxoutType::WITNESS_UNKNOWN);
+    BOOST_CHECK_EQUAL(Solver(s, &solutions), TxoutType::WITNESS_UNKNOWN);
     s.clear();
     s << OP_1 << std::vector<unsigned char>(33, 0x01);
-    BOOST_CHECK_EQUAL(Solver(s, solutions), TxoutType::WITNESS_UNKNOWN);
+    BOOST_CHECK_EQUAL(Solver(s, &solutions), TxoutType::WITNESS_UNKNOWN);
 
     // TxoutType::ANCHOR but wrong witness version
     s.clear();
     s << OP_2 << ANCHOR_BYTES;
     BOOST_CHECK(!s.IsPayToAnchor());
-    BOOST_CHECK_EQUAL(Solver(s, solutions), TxoutType::WITNESS_UNKNOWN);
+    BOOST_CHECK_EQUAL(Solver(s, &solutions), TxoutType::WITNESS_UNKNOWN);
 
     // TxoutType::ANCHOR but wrong 2-byte data push
     s.clear();
     s << OP_1 << std::vector<unsigned char>{0xff, 0xff};
     BOOST_CHECK(!s.IsPayToAnchor());
-    BOOST_CHECK_EQUAL(Solver(s, solutions), TxoutType::WITNESS_UNKNOWN);
+    BOOST_CHECK_EQUAL(Solver(s, &solutions), TxoutType::WITNESS_UNKNOWN);
 }
 
 BOOST_AUTO_TEST_CASE(script_standard_ExtractDestination)
diff --git a/src/test/script_tests.cpp b/src/test/script_tests.cpp
index fc4550bb7b..4e4dbd5f89 100644
--- a/src/test/script_tests.cpp
+++ b/src/test/script_tests.cpp
@@ -1157,13 +1157,6 @@ BOOST_AUTO_TEST_CASE(script_CHECKMULTISIG23)
     BOOST_CHECK_MESSAGE(err == SCRIPT_ERR_INVALID_STACK_OPERATION, ScriptErrorString(err));
 }
 
-/** Return the TxoutType of a script without exposing Solver details. */
-static TxoutType GetTxoutType(const CScript& output_script)
-{
-    std::vector<std::vector<uint8_t>> unused;
-    return Solver(output_script, unused);
-}
-
 #define CHECK_SCRIPT_STATIC_SIZE(script, expected_size)                   \
     do {                                                                  \
         BOOST_CHECK_EQUAL((script).size(), (expected_size));              \
@@ -1193,14 +1186,14 @@ BOOST_AUTO_TEST_CASE(script_size_and_capacity_test)
     // Small OP_RETURN has direct allocation
     {
         const auto script{CScript() << OP_RETURN << std::vector<uint8_t>(10, 0xaa)};
-        BOOST_CHECK_EQUAL(GetTxoutType(script), TxoutType::NULL_DATA);
+        BOOST_CHECK_EQUAL(Solver(script), TxoutType::NULL_DATA);
         CHECK_SCRIPT_STATIC_SIZE(script, 12);
     }
 
     // P2WPKH has direct allocation
     {
         const auto script{GetScriptForDestination(WitnessV0KeyHash{PKHash{dummy_pubkey}})};
-        BOOST_CHECK_EQUAL(GetTxoutType(script), TxoutType::WITNESS_V0_KEYHASH);
+        BOOST_CHECK_EQUAL(Solver(script), TxoutType::WITNESS_V0_KEYHASH);
         CHECK_SCRIPT_STATIC_SIZE(script, 22);
     }
 
@@ -1214,7 +1207,7 @@ BOOST_AUTO_TEST_CASE(script_size_and_capacity_test)
     // P2PKH has direct allocation
     {
         const auto script{GetScriptForDestination(PKHash{dummy_pubkey})};
-        BOOST_CHECK_EQUAL(GetTxoutType(script), TxoutType::PUBKEYHASH);
+        BOOST_CHECK_EQUAL(Solver(script), TxoutType::PUBKEYHASH);
         CHECK_SCRIPT_STATIC_SIZE(script, 25);
     }
 
@@ -1228,14 +1221,14 @@ BOOST_AUTO_TEST_CASE(script_size_and_capacity_test)
     // P2TR has direct allocation
     {
         const auto script{GetScriptForDestination(WitnessV1Taproot{XOnlyPubKey{dummy_pubkey}})};
-        BOOST_CHECK_EQUAL(GetTxoutType(script), TxoutType::WITNESS_V1_TAPROOT);
+        BOOST_CHECK_EQUAL(Solver(script), TxoutType::WITNESS_V1_TAPROOT);
         CHECK_SCRIPT_STATIC_SIZE(script, 34);
     }
 
     // Compressed P2PK has direct allocation
     {
         const auto script{GetScriptForRawPubKey(dummy_pubkey)};
-        BOOST_CHECK_EQUAL(GetTxoutType(script), TxoutType::PUBKEY);
+        BOOST_CHECK_EQUAL(Solver(script), TxoutType::PUBKEY);
         CHECK_SCRIPT_STATIC_SIZE(script, 35);
     }
 
@@ -1246,14 +1239,14 @@ BOOST_AUTO_TEST_CASE(script_size_and_capacity_test)
         const CPubKey uncompressed_pubkey{uncompressed_key.GetPubKey()};
 
         const auto script{GetScriptForRawPubKey(uncompressed_pubkey)};
-        BOOST_CHECK_EQUAL(GetTxoutType(script), TxoutType::PUBKEY);
+        BOOST_CHECK_EQUAL(Solver(script), TxoutType::PUBKEY);
         CHECK_SCRIPT_DYNAMIC_SIZE(script, 67, 67);
     }
 
     // Bare multisig needs extra allocation
     {
         const auto script{GetScriptForMultisig(1, std::vector{2, dummy_pubkey})};
-        BOOST_CHECK_EQUAL(GetTxoutType(script), TxoutType::MULTISIG);
+        BOOST_CHECK_EQUAL(Solver(script), TxoutType::MULTISIG);
         CHECK_SCRIPT_DYNAMIC_SIZE(script, 71, 103);
     }
 }
diff --git a/src/test/transaction_tests.cpp b/src/test/transaction_tests.cpp
index b2a70057d5..47f42fb231 100644
--- a/src/test/transaction_tests.cpp
+++ b/src/test/transaction_tests.cpp
@@ -1125,7 +1125,6 @@ BOOST_AUTO_TEST_CASE(spends_witness_prog)
     CMutableTransaction tx_create{}, tx_spend{};
     tx_create.vout.emplace_back(0, CScript{});
     tx_spend.vin.emplace_back(Txid{}, 0);
-    std::vector<std::vector<uint8_t>> sol_dummy;
 
     // CNoDestination, PubKeyDestination, PKHash, ScriptHash, WitnessV0ScriptHash, WitnessV0KeyHash,
     // WitnessV1Taproot, PayToAnchor, WitnessUnknown.
@@ -1135,14 +1134,14 @@ BOOST_AUTO_TEST_CASE(spends_witness_prog)
 
     // P2PK
     tx_create.vout[0].scriptPubKey = GetScriptForDestination(PubKeyDestination{pubkey});
-    BOOST_CHECK_EQUAL(Solver(tx_create.vout[0].scriptPubKey, sol_dummy), TxoutType::PUBKEY);
+    BOOST_CHECK_EQUAL(Solver(tx_create.vout[0].scriptPubKey), TxoutType::PUBKEY);
     tx_spend.vin[0].prevout.hash = tx_create.GetHash();
     AddCoins(coins, CTransaction{tx_create}, 0, false);
     BOOST_CHECK(!::SpendsNonAnchorWitnessProg(CTransaction{tx_spend}, coins));
 
     // P2PKH
     tx_create.vout[0].scriptPubKey = GetScriptForDestination(PKHash{pubkey});
-    BOOST_CHECK_EQUAL(Solver(tx_create.vout[0].scriptPubKey, sol_dummy), TxoutType::PUBKEYHASH);
+    BOOST_CHECK_EQUAL(Solver(tx_create.vout[0].scriptPubKey), TxoutType::PUBKEYHASH);
     tx_spend.vin[0].prevout.hash = tx_create.GetHash();
     AddCoins(coins, CTransaction{tx_create}, 0, false);
     BOOST_CHECK(!::SpendsNonAnchorWitnessProg(CTransaction{tx_spend}, coins));
@@ -1150,7 +1149,7 @@ BOOST_AUTO_TEST_CASE(spends_witness_prog)
     // P2SH
     auto redeem_script{CScript{} << OP_1 << OP_CHECKSIG};
     tx_create.vout[0].scriptPubKey = GetScriptForDestination(ScriptHash{redeem_script});
-    BOOST_CHECK_EQUAL(Solver(tx_create.vout[0].scriptPubKey, sol_dummy), TxoutType::SCRIPTHASH);
+    BOOST_CHECK_EQUAL(Solver(tx_create.vout[0].scriptPubKey), TxoutType::SCRIPTHASH);
     tx_spend.vin[0].prevout.hash = tx_create.GetHash();
     tx_spend.vin[0].scriptSig = CScript{} << OP_0 << ToByteVector(redeem_script);
     AddCoins(coins, CTransaction{tx_create}, 0, false);
@@ -1160,7 +1159,7 @@ BOOST_AUTO_TEST_CASE(spends_witness_prog)
     // native P2WSH
     const auto witness_script{CScript{} << OP_12 << OP_HASH160 << OP_DUP << OP_EQUAL};
     tx_create.vout[0].scriptPubKey = GetScriptForDestination(WitnessV0ScriptHash{witness_script});
-    BOOST_CHECK_EQUAL(Solver(tx_create.vout[0].scriptPubKey, sol_dummy), TxoutType::WITNESS_V0_SCRIPTHASH);
+    BOOST_CHECK_EQUAL(Solver(tx_create.vout[0].scriptPubKey), TxoutType::WITNESS_V0_SCRIPTHASH);
     tx_spend.vin[0].prevout.hash = tx_create.GetHash();
     AddCoins(coins, CTransaction{tx_create}, 0, false);
     BOOST_CHECK(::SpendsNonAnchorWitnessProg(CTransaction{tx_spend}, coins));
@@ -1168,7 +1167,7 @@ BOOST_AUTO_TEST_CASE(spends_witness_prog)
     // P2SH-wrapped P2WSH
     redeem_script = tx_create.vout[0].scriptPubKey;
     tx_create.vout[0].scriptPubKey = GetScriptForDestination(ScriptHash(redeem_script));
-    BOOST_CHECK_EQUAL(Solver(tx_create.vout[0].scriptPubKey, sol_dummy), TxoutType::SCRIPTHASH);
+    BOOST_CHECK_EQUAL(Solver(tx_create.vout[0].scriptPubKey), TxoutType::SCRIPTHASH);
     tx_spend.vin[0].prevout.hash = tx_create.GetHash();
     tx_spend.vin[0].scriptSig = CScript{} << ToByteVector(redeem_script);
     AddCoins(coins, CTransaction{tx_create}, 0, false);
@@ -1178,7 +1177,7 @@ BOOST_AUTO_TEST_CASE(spends_witness_prog)
 
     // native P2WPKH
     tx_create.vout[0].scriptPubKey = GetScriptForDestination(WitnessV0KeyHash{pubkey});
-    BOOST_CHECK_EQUAL(Solver(tx_create.vout[0].scriptPubKey, sol_dummy), TxoutType::WITNESS_V0_KEYHASH);
+    BOOST_CHECK_EQUAL(Solver(tx_create.vout[0].scriptPubKey), TxoutType::WITNESS_V0_KEYHASH);
     tx_spend.vin[0].prevout.hash = tx_create.GetHash();
     AddCoins(coins, CTransaction{tx_create}, 0, false);
     BOOST_CHECK(::SpendsNonAnchorWitnessProg(CTransaction{tx_spend}, coins));
@@ -1186,7 +1185,7 @@ BOOST_AUTO_TEST_CASE(spends_witness_prog)
     // P2SH-wrapped P2WPKH
     redeem_script = tx_create.vout[0].scriptPubKey;
     tx_create.vout[0].scriptPubKey = GetScriptForDestination(ScriptHash(redeem_script));
-    BOOST_CHECK_EQUAL(Solver(tx_create.vout[0].scriptPubKey, sol_dummy), TxoutType::SCRIPTHASH);
+    BOOST_CHECK_EQUAL(Solver(tx_create.vout[0].scriptPubKey), TxoutType::SCRIPTHASH);
     tx_spend.vin[0].prevout.hash = tx_create.GetHash();
     tx_spend.vin[0].scriptSig = CScript{} << ToByteVector(redeem_script);
     AddCoins(coins, CTransaction{tx_create}, 0, false);
@@ -1196,7 +1195,7 @@ BOOST_AUTO_TEST_CASE(spends_witness_prog)
 
     // P2TR
     tx_create.vout[0].scriptPubKey = GetScriptForDestination(WitnessV1Taproot{XOnlyPubKey{pubkey}});
-    BOOST_CHECK_EQUAL(Solver(tx_create.vout[0].scriptPubKey, sol_dummy), TxoutType::WITNESS_V1_TAPROOT);
+    BOOST_CHECK_EQUAL(Solver(tx_create.vout[0].scriptPubKey), TxoutType::WITNESS_V1_TAPROOT);
     tx_spend.vin[0].prevout.hash = tx_create.GetHash();
     AddCoins(coins, CTransaction{tx_create}, 0, false);
     BOOST_CHECK(::SpendsNonAnchorWitnessProg(CTransaction{tx_spend}, coins));
@@ -1204,7 +1203,7 @@ BOOST_AUTO_TEST_CASE(spends_witness_prog)
     // P2SH-wrapped P2TR (undefined, non-standard)
     redeem_script = tx_create.vout[0].scriptPubKey;
     tx_create.vout[0].scriptPubKey = GetScriptForDestination(ScriptHash(redeem_script));
-    BOOST_CHECK_EQUAL(Solver(tx_create.vout[0].scriptPubKey, sol_dummy), TxoutType::SCRIPTHASH);
+    BOOST_CHECK_EQUAL(Solver(tx_create.vout[0].scriptPubKey), TxoutType::SCRIPTHASH);
     tx_spend.vin[0].prevout.hash = tx_create.GetHash();
     tx_spend.vin[0].scriptSig = CScript{} << ToByteVector(redeem_script);
     AddCoins(coins, CTransaction{tx_create}, 0, false);
@@ -1214,7 +1213,7 @@ BOOST_AUTO_TEST_CASE(spends_witness_prog)
 
     // P2A
     tx_create.vout[0].scriptPubKey = GetScriptForDestination(PayToAnchor{});
-    BOOST_CHECK_EQUAL(Solver(tx_create.vout[0].scriptPubKey, sol_dummy), TxoutType::ANCHOR);
+    BOOST_CHECK_EQUAL(Solver(tx_create.vout[0].scriptPubKey), TxoutType::ANCHOR);
     tx_spend.vin[0].prevout.hash = tx_create.GetHash();
     AddCoins(coins, CTransaction{tx_create}, 0, false);
     BOOST_CHECK(!::SpendsNonAnchorWitnessProg(CTransaction{tx_spend}, coins));
@@ -1222,7 +1221,7 @@ BOOST_AUTO_TEST_CASE(spends_witness_prog)
     // P2SH-wrapped P2A (undefined, non-standard)
     redeem_script = tx_create.vout[0].scriptPubKey;
     tx_create.vout[0].scriptPubKey = GetScriptForDestination(ScriptHash(redeem_script));
-    BOOST_CHECK_EQUAL(Solver(tx_create.vout[0].scriptPubKey, sol_dummy), TxoutType::SCRIPTHASH);
+    BOOST_CHECK_EQUAL(Solver(tx_create.vout[0].scriptPubKey), TxoutType::SCRIPTHASH);
     tx_spend.vin[0].prevout.hash = tx_create.GetHash();
     tx_spend.vin[0].scriptSig = CScript{} << ToByteVector(redeem_script);
     AddCoins(coins, CTransaction{tx_create}, 0, false);
@@ -1231,7 +1230,7 @@ BOOST_AUTO_TEST_CASE(spends_witness_prog)
 
     // Undefined version 1 witness program
     tx_create.vout[0].scriptPubKey = GetScriptForDestination(WitnessUnknown{1, {0x42, 0x42}});
-    BOOST_CHECK_EQUAL(Solver(tx_create.vout[0].scriptPubKey, sol_dummy), TxoutType::WITNESS_UNKNOWN);
+    BOOST_CHECK_EQUAL(Solver(tx_create.vout[0].scriptPubKey), TxoutType::WITNESS_UNKNOWN);
     tx_spend.vin[0].prevout.hash = tx_create.GetHash();
     AddCoins(coins, CTransaction{tx_create}, 0, false);
     BOOST_CHECK(::SpendsNonAnchorWitnessProg(CTransaction{tx_spend}, coins));
@@ -1239,7 +1238,7 @@ BOOST_AUTO_TEST_CASE(spends_witness_prog)
     // P2SH-wrapped undefined version 1 witness program
     redeem_script = tx_create.vout[0].scriptPubKey;
     tx_create.vout[0].scriptPubKey = GetScriptForDestination(ScriptHash(redeem_script));
-    BOOST_CHECK_EQUAL(Solver(tx_create.vout[0].scriptPubKey, sol_dummy), TxoutType::SCRIPTHASH);
+    BOOST_CHECK_EQUAL(Solver(tx_create.vout[0].scriptPubKey), TxoutType::SCRIPTHASH);
     tx_spend.vin[0].prevout.hash = tx_create.GetHash();
     tx_spend.vin[0].scriptSig = CScript{} << ToByteVector(redeem_script);
     AddCoins(coins, CTransaction{tx_create}, 0, false);
@@ -1251,7 +1250,7 @@ BOOST_AUTO_TEST_CASE(spends_witness_prog)
     const auto program{ToByteVector(XOnlyPubKey{pubkey})};
     for (int i{2}; i <= 16; ++i) {
         tx_create.vout[0].scriptPubKey = GetScriptForDestination(WitnessUnknown{i, program});
-        BOOST_CHECK_EQUAL(Solver(tx_create.vout[0].scriptPubKey, sol_dummy), TxoutType::WITNESS_UNKNOWN);
+        BOOST_CHECK_EQUAL(Solver(tx_create.vout[0].scriptPubKey), TxoutType::WITNESS_UNKNOWN);
         tx_spend.vin[0].prevout.hash = tx_create.GetHash();
         AddCoins(coins, CTransaction{tx_create}, 0, false);
         BOOST_CHECK(::SpendsNonAnchorWitnessProg(CTransaction{tx_spend}, coins));
@@ -1259,7 +1258,7 @@ BOOST_AUTO_TEST_CASE(spends_witness_prog)
         // It's also detected within P2SH.
         redeem_script = tx_create.vout[0].scriptPubKey;
         tx_create.vout[0].scriptPubKey = GetScriptForDestination(ScriptHash(redeem_script));
-        BOOST_CHECK_EQUAL(Solver(tx_create.vout[0].scriptPubKey, sol_dummy), TxoutType::SCRIPTHASH);
+        BOOST_CHECK_EQUAL(Solver(tx_create.vout[0].scriptPubKey), TxoutType::SCRIPTHASH);
         tx_spend.vin[0].prevout.hash = tx_create.GetHash();
         tx_spend.vin[0].scriptSig = CScript{} << ToByteVector(redeem_script);
         AddCoins(coins, CTransaction{tx_create}, 0, false);
diff --git a/src/wallet/rpc/addresses.cpp b/src/wallet/rpc/addresses.cpp
index a04e0903b9..0ddee1fa49 100644
--- a/src/wallet/rpc/addresses.cpp
+++ b/src/wallet/rpc/addresses.cpp
@@ -267,7 +267,7 @@ public:
     {
         // Always present: script type and redeemscript
         std::vector<std::vector<unsigned char>> solutions_data;
-        TxoutType which_type = Solver(subscript, solutions_data);
+        TxoutType which_type = Solver(subscript, &solutions_data);
         obj.pushKV("script", GetTxnOutputType(which_type));
         obj.pushKV("hex", HexStr(subscript));
 
diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp
index a78f32f890..2f23861259 100644
--- a/src/wallet/scriptpubkeyman.cpp
+++ b/src/wallet/scriptpubkeyman.cpp
@@ -86,7 +86,7 @@ IsMineResult LegacyWalletIsMineInnerDONOTUSE(const LegacyDataSPKM& keystore, con
     IsMineResult ret = IsMineResult::NO;
 
     std::vector<valtype> vSolutions;
-    TxoutType whichType = Solver(scriptPubKey, vSolutions);
+    TxoutType whichType = Solver(scriptPubKey, &vSolutions);
 
     CKeyID keyID;
     switch (whichType) {
@@ -350,7 +350,7 @@ bool LegacyDataSPKM::LoadWatchOnly(const CScript &dest)
 static bool ExtractPubKey(const CScript &dest, CPubKey& pubKeyOut)
 {
     std::vector<std::vector<unsigned char>> solutions;
-    return Solver(dest, solutions) == TxoutType::PUBKEY &&
+    return Solver(dest, &solutions) == TxoutType::PUBKEY &&
         (pubKeyOut = CPubKey(solutions[0])).IsFullyValid();
 }
 
@@ -1346,7 +1346,7 @@ std::optional<PSBTError> DescriptorScriptPubKeyMan::FillPSBT(PartiallySignedTran
 
             // Taproot output pubkey
             std::vector<std::vector<unsigned char>> sols;
-            if (Solver(script, sols) == TxoutType::WITNESS_V1_TAPROOT) {
+            if (Solver(script, &sols) == TxoutType::WITNESS_V1_TAPROOT) {
                 sols[0].insert(sols[0].begin(), 0x02);
                 pubkeys.emplace_back(sols[0]);
                 sols[0][0] = 0x03;
diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp
index 146fb49ea7..b36925381a 100644
--- a/src/wallet/spend.cpp
+++ b/src/wallet/spend.cpp
@@ -452,7 +452,7 @@ CoinsResult AvailableCoins(const CWallet& wallet,
 
         // Obtain script type
         std::vector<std::vector<uint8_t>> script_solutions;
-        TxoutType type = Solver(output.scriptPubKey, script_solutions);
+        TxoutType type = Solver(output.scriptPubKey, &script_solutions);
 
         // If the output is P2SH and solvable, we want to know if it is
         // a P2SH (legacy) or one of P2SH-P2WPKH, P2SH-P2WSH (P2SH-Segwit). We can determine
@@ -462,7 +462,7 @@ CoinsResult AvailableCoins(const CWallet& wallet,
         if (type == TxoutType::SCRIPTHASH && solvable) {
             CScript script;
             if (!provider->GetCScript(CScriptID(uint160(script_solutions[0])), script)) continue;
-            type = Solver(script, script_solutions);
+            type = Solver(script, &script_solutions);
             is_from_p2sh = true;
         }
image

Concept ACK. if you decide to use my alternative, please split it into smaller, focused commits.

@Raimo33
Copy link
Contributor Author

Raimo33 commented Oct 23, 2025

You're right, vSolutions is completely unnecessary in AreInputsStandard(). I'm not a fan of raw pointers but in this case it seems the best practical solution overall. smart pointers or optionals don't fit quite well.

I'll use your commits with small tweaks and add you as co-author for now

@Raimo33 Raimo33 changed the title refactor: optimize: reuse containers in transaction policy verification refactor: optimize: avoid allocations in script & policy verification Oct 23, 2025
@Raimo33 Raimo33 force-pushed the optimize-tx-policy-verification branch from cedfff1 to 55574ee Compare October 23, 2025 14:55
@Raimo33
Copy link
Contributor Author

Raimo33 commented Oct 23, 2025 •
edited
Loading

I've found other unnecessary related memory allocations. Instead of using @l0rinc 's approach of modifying the already existing methods (adding branch complexity) I opted for defining new methods. While this increases redundancy between similar methods, it produces a smaller diff and cleaner separate paths.

performance has further improved on my end. showing +15% for CCoinsCaching

@l0rinc
Copy link
Contributor

l0rinc commented Oct 23, 2025

I deliberately avoided duplication, not sure why you think that's simpler

@Raimo33
Copy link
Contributor Author

Raimo33 commented Oct 23, 2025 •
edited
Loading

I deliberately avoided duplication, not sure why you think that's simpler

  • wether the diff is simpler is debatable.
  • I argue that the separation of concerns makes both Solver and GetScriptPubKeyType more readable
  • theoretically the duplication approach is faster, avoiding branches that check if vSolutions is null

@l0rinc
Copy link
Contributor

l0rinc commented Oct 23, 2025 •
edited
Loading

Approach N​A​C​K, you're needlessly modifying critical code and needlessly duplicating existing tests for them.
Edit: retracting my review since the alternative I suggested isn't enough

@Raimo33 Raimo33 marked this pull request as draft October 24, 2025 08:44
@Raimo33 Raimo33 force-pushed the optimize-tx-policy-verification branch 3 times, most recently from 8bde358 to e9db221 Compare October 24, 2025 12:35
@Raimo33 Raimo33 force-pushed the optimize-tx-policy-verification branch from e9db221 to 6b6dcee Compare October 24, 2025 12:36
@Raimo33
Copy link
Contributor Author

Raimo33 commented Oct 24, 2025

I've noticed that the CCoinsCaching benchmark is a bit deceiving. What we are seeing in this PR is not a speedup in actual coin caching, but rather a speedup in AreInputsStandard()

benchmark code 
static void CCoinsCaching(benchmark::Bench& bench)
{
    ECC_Context ecc_context{};

    FillableSigningProvider keystore;
    CCoinsView coinsDummy;
    CCoinsViewCache coins(&coinsDummy);
    std::vector<CMutableTransaction> dummyTransactions =
        SetupDummyInputs(keystore, coins, {11 * COIN, 50 * COIN, 21 * COIN, 22 * COIN});

    CMutableTransaction t1;
    t1.vin.resize(3);
    t1.vin[0].prevout.hash = dummyTransactions[0].GetHash();
    t1.vin[0].prevout.n = 1;
    t1.vin[0].scriptSig << std::vector<unsigned char>(65, 0);
    t1.vin[1].prevout.hash = dummyTransactions[1].GetHash();
    t1.vin[1].prevout.n = 0;
    t1.vin[1].scriptSig << std::vector<unsigned char>(65, 0) << std::vector<unsigned char>(33, 4);
    t1.vin[2].prevout.hash = dummyTransactions[1].GetHash();
    t1.vin[2].prevout.n = 1;
    t1.vin[2].scriptSig << std::vector<unsigned char>(65, 0) << std::vector<unsigned char>(33, 4);
    t1.vout.resize(2);
    t1.vout[0].nValue = 90 * COIN;
    t1.vout[0].scriptPubKey << OP_1;

    // Benchmark.
    const CTransaction tx_1(t1);
    bench.run([&] {
        bool success{AreInputsStandard(tx_1, coins)};
        assert(success);
    });
}

@Raimo33 Raimo33 force-pushed the optimize-tx-policy-verification branch 4 times, most recently from 510b0f2 to 2537dba Compare October 24, 2025 14:57
@l0rinc
Copy link
Contributor

l0rinc commented Oct 24, 2025

Please only push after you're actually done, everyone gets a notification for each push, and people will just simply unsubscribe from this thread otherwise...
Also, please revert unrelated refactorings, the change is just getting more complicated after the simplification requests.

@cedwies
Copy link

cedwies commented Oct 24, 2025

I strongly suggest splitting this draft into smaller, more focused PRs (or separate drafts), each exploring one idea at a time.

The current draft mixes several independent changes (policy loop optimizations, Solver/IsWitnessProgram API changes, and stylistic updates), which makes it harder to benchmark and review effectively.

I’m happy to follow along and review, but it would be great to have clear, targeted benchmarks for future iterations (ones that measure the actual code (paths) being changed, so that we can make solid, data-driven decisions about which changes are worth merging.

For example, regarding:

"Disagree for witnessprogram: it is used in the hot path (P2WPKH is the most likely)"

I think whether it’s on the hot path or not is less important than having data to show a measurable gain. The benchmarks did not show a clear performance benefit. The performance benefit (AFAIK) came from hoisting vSolutions (even though we later found out vSolutions is not needed in that code). I prefer we do not simply hoist vectors (or similar) because we think/assume this improves performance, let's confirm with targeted measurements first.

@DrahtBot DrahtBot mentioned this pull request Oct 25, 2025
Open
@Raimo33 Raimo33 force-pushed the optimize-tx-policy-verification branch from 2537dba to 996acda Compare October 26, 2025 13:57
@Raimo33
Copy link
Contributor Author

Raimo33 commented Oct 26, 2025

As suggested, I've removed some commits to keep this PR simple. I'll open a follow up PR with the more complex but related commits that change the API.

As per the benchmarks, there currently is no benchmark that realistically measures the impacted methods, and I'm afraid I'll not be able to code one. we'd need to measure AreInputsStandard(), IsWitnessStandard() and Solver() which depend on too many variables.

@Raimo33 Raimo33 marked this pull request as ready for review October 28, 2025 12:57
ajtowns
ajtowns reviewed Oct 28, 2025
View reviewed changes
src/policy/policy.cpp

int witnessversion = 0;
std::vector<unsigned char> witnessprogram;
witnessprogram.clear();
Copy link
Contributor

@ajtowns ajtowns Oct 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be better to have IsWitnessProgram() populate a span rather than a vector if we're trying to minimise allocations, as far as I can see.

Copy link
Contributor Author

@Raimo33 Raimo33 Oct 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here the vector is totally useless. it's required by the IsWitnessProgram() API but not actually needed. I've made more significant changes on this private branch, for another PR: https://github.com/Raimo33/bitcoinknots/commits/optimize-tx-policy-verification-2/

Copy link
Contributor

@l0rinc l0rinc Oct 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like the reuses, seems like a workaround instead of a fix, pushed an alternative to #33757, added @Raimo33 as a coauthor

Copy link
Contributor Author

@Raimo33 Raimo33 Oct 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what about the other commits? @l0rinc.
there are some intresting zero-cost optimizations. maybe not worth for a PR alone, but could be included as small refactorings in bigger PRs.

Copy link
Contributor

@l0rinc l0rinc Nov 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have removed my nack here, see #33757 (comment)

I'm not sure about the focus for the other commits, they seem like problems that you can solve instead of problems that need solving.
The resulting code isn't obviously cleaner and the dependencies are often more complicated now (reusing vectors for example). I know that was already in place, but most of our work is untangling code, I'm more enthusiastic about changes that are both cleaner and faster - which I don't think is the case here.

But I have retracted my objection so that others are free to review and opine on it.

l0rinc added a commit to l0rinc/bitcoin that referenced this pull request Oct 31, 2025
@l0rinc @Raimo33
refactor: make script Solver's solutions parameter optional
2726abd 
Make the `vSolutionsRet` parameter of `Solver()` nullable pointer allowing callers that only need the return value to avoid allocating and populating the solutions vector.

Benchmark results show a 25.0% performance improvement (4,240,283 to
5,300,923 operations per second) for callers that don't need solutions.

This approach addresses the performance concern raised in bitcoin#33645 more
fundamentally than the vector reuse optimization, while simplifying the call sites at the same time.

Co-authored-by: Raimo33 <[email protected]>
@l0rinc l0rinc mentioned this pull request Oct 31, 2025
Closed
l0rinc added a commit to l0rinc/bitcoin that referenced this pull request Nov 2, 2025
@l0rinc @Raimo33
refactor: make script Solver's solutions parameter optional
f97f3d9 
Make the `vSolutionsRet` parameter of `Solver()` nullable pointer allowing callers that only need the return value to avoid allocating and populating the solutions vector.

Benchmark results show a 25.0% performance improvement (4,240,283 to
5,300,923 operations per second) for callers that don't need solutions.

This approach addresses the performance concern raised in bitcoin#33645 more
fundamentally than the vector reuse optimization, while simplifying the call sites at the same time.

Co-authored-by: Raimo33 <[email protected]>
@DrahtBot DrahtBot mentioned this pull request Nov 27, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

3 more reviewers

@ajtowns ajtowns ajtowns left review comments

@l0rinc l0rinc l0rinc left review comments

@cedwies cedwies cedwies left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Refactoring

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@Raimo33 @DrahtBot @l0rinc @cedwies @ajtowns