From 080089567ca766d4c1cde8ec0e5c7df48f566e07 Mon Sep 17 00:00:00 2001
From: Sebastian Falbesoner <sebastian.falbesoner@gmail.com>
Date: Mon, 20 Nov 2023 10:34:03 +0100
Subject: [PATCH 1/2] bench: add benchmark for `SignTransaction`

---
 src/Makefile.bench.include     |  1 +
 src/bench/sign_transaction.cpp | 70 ++++++++++++++++++++++++++++++++++
 2 files changed, 71 insertions(+)
 create mode 100644 src/bench/sign_transaction.cpp

diff --git a/src/Makefile.bench.include b/src/Makefile.bench.include
index 7ba0111fa68..204a1cc1622 100644
--- a/src/Makefile.bench.include
+++ b/src/Makefile.bench.include
@@ -52,6 +52,7 @@ bench_bench_bitcoin_SOURCES = \
   bench/rollingbloom.cpp \
   bench/rpc_blockchain.cpp \
   bench/rpc_mempool.cpp \
+  bench/sign_transaction.cpp \
   bench/streams_findbyte.cpp \
   bench/strencodings.cpp \
   bench/util_time.cpp \
diff --git a/src/bench/sign_transaction.cpp b/src/bench/sign_transaction.cpp
new file mode 100644
index 00000000000..7a2e26e339f
--- /dev/null
+++ b/src/bench/sign_transaction.cpp
@@ -0,0 +1,70 @@
+// Copyright (c) 2023 The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#include <bench/bench.h>
+#include <addresstype.h>
+#include <coins.h>
+#include <key.h>
+#include <primitives/transaction.h>
+#include <pubkey.h>
+#include <script/interpreter.h>
+#include <script/script.h>
+#include <script/sign.h>
+#include <uint256.h>
+#include <util/translation.h>
+
+enum class InputType {
+    P2WPKH, // segwitv0, witness-pubkey-hash (ECDSA signature)
+    P2TR,   // segwitv1, taproot key-path spend (Schnorr signature)
+};
+
+static void SignTransactionSingleInput(benchmark::Bench& bench, InputType input_type)
+{
+    ECC_Context ecc_context{};
+
+    FlatSigningProvider keystore;
+    std::vector<CScript> prev_spks;
+
+    // Create a bunch of keys / UTXOs to avoid signing with the same key repeatedly
+    for (int i = 0; i < 32; i++) {
+        CKey privkey = GenerateRandomKey();
+        CPubKey pubkey = privkey.GetPubKey();
+        CKeyID key_id = pubkey.GetID();
+        keystore.keys.emplace(key_id, privkey);
+        keystore.pubkeys.emplace(key_id, pubkey);
+
+        // Create specified locking script type
+        CScript prev_spk;
+        switch (input_type) {
+        case InputType::P2WPKH: prev_spk = GetScriptForDestination(WitnessV0KeyHash(pubkey)); break;
+        case InputType::P2TR:   prev_spk = GetScriptForDestination(WitnessV1Taproot(XOnlyPubKey{pubkey})); break;
+        default: assert(false);
+        }
+        prev_spks.push_back(prev_spk);
+    }
+
+    // Simple 1-input tx with artificial outpoint
+    // (note that for the purpose of signing with SIGHASH_ALL we don't need any outputs)
+    COutPoint prevout{/*hashIn=*/Txid::FromUint256(uint256::ONE), /*nIn=*/1337};
+    CMutableTransaction unsigned_tx;
+    unsigned_tx.vin.emplace_back(prevout);
+
+    // Benchmark.
+    int iter = 0;
+    bench.minEpochIterations(100).run([&] {
+        CMutableTransaction tx{unsigned_tx};
+        std::map<COutPoint, Coin> coins;
+        CScript prev_spk = prev_spks[(iter++) % prev_spks.size()];
+        coins[prevout] = Coin(CTxOut(10000, prev_spk), /*nHeightIn=*/100, /*fCoinBaseIn=*/false);
+        std::map<int, bilingual_str> input_errors;
+        bool complete = SignTransaction(tx, &keystore, coins, SIGHASH_ALL, input_errors);
+        assert(complete);
+    });
+}
+
+static void SignTransactionECDSA(benchmark::Bench& bench)   { SignTransactionSingleInput(bench, InputType::P2WPKH); }
+static void SignTransactionSchnorr(benchmark::Bench& bench) { SignTransactionSingleInput(bench, InputType::P2TR);   }
+
+BENCHMARK(SignTransactionECDSA, benchmark::PriorityLevel::HIGH);
+BENCHMARK(SignTransactionSchnorr, benchmark::PriorityLevel::HIGH);

From fe92c15f0c44d1405b9048306736bd0eae868506 Mon Sep 17 00:00:00 2001
From: Sebastian Falbesoner <sebastian.falbesoner@gmail.com>
Date: Tue, 21 Nov 2023 16:47:35 +0100
Subject: [PATCH 2/2] script/sign: avoid duplicated signature verification
 after signing

`ProduceSignature` already calls `VerifyScript` internally as last step in
order to check whether the signature data is complete. If and only if that is
the case, the `complete` field of the `SignatureData` is set accordingly and
there is no need then to verify the script after again, as we already know that
it would succeed.

This leads to a rough ~20% speed-up for `SignTransaction` for single-input
ECDSA or Taproot inputs, according to the `SignTransaction{ECDSA,Taproot}`
benchmarks.
---
 src/script/sign.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/script/sign.cpp b/src/script/sign.cpp
index 22ac062a633..6e26ec11e05 100644
--- a/src/script/sign.cpp
+++ b/src/script/sign.cpp
@@ -831,7 +831,7 @@ bool SignTransaction(CMutableTransaction& mtx, const SigningProvider* keystore,
         }
 
         ScriptError serror = SCRIPT_ERR_OK;
-        if (!VerifyScript(txin.scriptSig, prevPubKey, &txin.scriptWitness, STANDARD_SCRIPT_VERIFY_FLAGS, TransactionSignatureChecker(&txConst, i, amount, txdata, MissingDataBehavior::FAIL), &serror)) {
+        if (!sigdata.complete && !VerifyScript(txin.scriptSig, prevPubKey, &txin.scriptWitness, STANDARD_SCRIPT_VERIFY_FLAGS, TransactionSignatureChecker(&txConst, i, amount, txdata, MissingDataBehavior::FAIL), &serror)) {
             if (serror == SCRIPT_ERR_INVALID_STACK_OPERATION) {
                 // Unable to sign input and verification failed (possible attempt to partially sign).
                 input_errors[i] = Untranslated("Unable to sign input, invalid stack size (possibly missing key)");