Would suggest to use FlatSigningProvider instead (see #28307).
It will also save you one extra GetScriptForDestination call per created key.

Have to admit that I'm lacking knowledge here about the concrete differences between FillableSigningProvider and FlatSigningProvider, especially about the possible benefits of the latter in this PR. AFAIR, I chose the fillable provider as it provides a nice interface (.AddKey()), which the flat one doesn't. The following patch works:

diff --git a/src/bench/sign_transaction.cpp b/src/bench/sign_transaction.cpp
index 8cb5e63882..8bd2ecbcd0 100644
--- a/src/bench/sign_transaction.cpp
+++ b/src/bench/sign_transaction.cpp
@@ -23,7 +23,7 @@ static void SignTransactionSingleInput(benchmark::Bench& bench, InputType input_
 {
     ECC_Start();
 
-    FillableSigningProvider keystore;
+    FlatSigningProvider keystore;
     std::vector<CScript> prev_spks;
 
     // Create a bunch of keys / UTXOs to avoid signing with the same key repeatedly
@@ -31,7 +31,9 @@ static void SignTransactionSingleInput(benchmark::Bench& bench, InputType input_
         CKey privkey;
         privkey.MakeNewKey(/*fCompressed=*/true);
         CPubKey pubkey = privkey.GetPubKey();
-        keystore.AddKey(privkey);
+        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;

Is there anything more that could be changed? I don't see how it makes the derivation of the outpoint scriptPubKey (needed for the map of coins) any easier, as I still need the individual GetScriptForDestination calls dependend on the locking script type.

Have to admit that I'm lacking knowledge here about the concrete differences between FillableSigningProvider and FlatSigningProvider, especially about the possible benefits of the latter in this PR.

FillableSigningProvider is just the legacy class and contains legacy scripts limitations (thus #28307). It does not affect the current form of this PR but, because this benchmark uses segwit v0 and v1 outputs, it could cause issues in the future.

Is there anything more that could be changed? I don't see how it makes the derivation of the outpoint scriptPubKey (needed for the map of coins) any easier, as I still need the individual GetScriptForDestination calls dependend on the locking script type.

I never said that it was going to make derivation easier. I Just said that it will also save you an extra GetScriptForDestination call per script.

diff --git a/src/bench/sign_transaction.cpp b/src/bench/sign_transaction.cpp
--- a/src/bench/sign_transaction.cpp	(revision 90469c8b7c1cd1d88134f6837914552792cbef49)
+++ b/src/bench/sign_transaction.cpp	(date 1706283907363)
@@ -23,15 +23,16 @@
 {
     ECC_Start();
 
-    FillableSigningProvider keystore;
-    std::vector<CScript> prev_spks;
+    FlatSigningProvider keystore;
 
     // Create a bunch of keys / UTXOs to avoid signing with the same key repeatedly
     for (int i = 0; i < 32; i++) {
         CKey privkey;
         privkey.MakeNewKey(/*fCompressed=*/true);
         CPubKey pubkey = privkey.GetPubKey();
-        keystore.AddKey(privkey);
+        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;
@@ -40,7 +41,7 @@
         case InputType::P2TR:   prev_spk = GetScriptForDestination(WitnessV1Taproot(XOnlyPubKey{pubkey})); break;
         default: assert(false);
         }
-        prev_spks.push_back(prev_spk);
+        keystore.scripts.insert({CScriptID{prev_spk}, prev_spk});
     }
 
     // Simple 1-input tx with artificial outpoint
@@ -50,15 +51,16 @@
     unsigned_tx.vin.emplace_back(prevout);
 
     // Benchmark.
-    int iter = 0;
+    auto it = keystore.scripts.begin();
     bench.minEpochIterations(100).run([&] {
         CMutableTransaction tx{unsigned_tx};
         std::map<COutPoint, Coin> coins;
-        CScript prev_spk = prev_spks[(iter++) % prev_spks.size()];
+        const CScript& prev_spk = it->second;
         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);
+        if (++it == keystore.scripts.end()) it = keystore.scripts.begin();
     });
 
     ECC_Stop();

FillableSigningProvider is just the legacy class and contains legacy scripts limitations (thus #28307). It does not affect the current form of this PR but, because this benchmark uses segwit v0 and v1 outputs, it could cause issues in the future.

Ok, good to know.

I never said that it was going to make derivation easier. I Just said that it will also save you an extra GetScriptForDestination call per script.

Took me a while now to understand what exactly you mean, after looking deeper in what happens in FillableSigningProvider::AddKey() and realizing that leads to an indirect GetScriptForDestination call. I initially assumed that you talk about needing less GetScriptForDestination calls directly in the benchmark preparation loop (i.e. "save" interpreted in the sense of less code from me, rather than less run-time overhead), which would have surprised me.

I'm still confused on why we would want/need to fill the .scripts of the FlatSigningProvider though. Aren't those only relevant for spends where redeem scripts are involved, i.e. P2(W)SH? Your proposed patch uses the .scripts map of the provider instead of an array to store the prev_spks, but that map is never accessed inside SignTransaction.

I've changed the PR to take use of FlatSigningProvider (and GenerateRandomKey for privkey instantiation, which wasn't available in master back then), but didn't fill the .scripts map as it seems not necessary for the script types tested here.