Ava Chow 16b4f75d04
Merge bitcoin/bitcoin#28923: script/sign: avoid duplicated signature verification after signing (+introduce signing benchmarks)
fe92c15f0c44d1405b9048306736bd0eae868506 script/sign: avoid duplicated signature verification after signing (Sebastian Falbesoner)
080089567ca766d4c1cde8ec0e5c7df48f566e07 bench: add benchmark for `SignTransaction` (Sebastian Falbesoner)

Pull request description:

  This PR is a small performance improvement on the `SignTransaction` function, which is used mostly by the wallet (obviously) and the `signrawtransactionwithkey` RPC. The lower-level function `ProduceSignature` already calls `VerifyScript` internally as last step in order to check whether the signature data is complete:
  daa56f7f66/src/script/sign.cpp (L568-L570)

  If and only if that is the case, the `complete` field of the `SignatureData` is set to `true` 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 transactions, according to the newly introduced `SignTransaction{ECDSA,Taproot}` benchmarks:

  ```
  $ ./src/bench/bench_bitcoin --filter=SignTransaction.*
  ```

  without commit 18185f4f578b8795fdaa75926630a691e9c8d0d4:

  |               ns/op |                op/s |    err% |     total | benchmark
  |--------------------:|--------------------:|--------:|----------:|:----------
  |          185,597.79 |            5,388.00 |    1.6% |      0.22 | `SignTransactionECDSA`
  |          141,323.95 |            7,075.94 |    2.1% |      0.17 | `SignTransactionSchnorr`

  with commit 18185f4f578b8795fdaa75926630a691e9c8d0d4:

  |               ns/op |                op/s |    err% |     total | benchmark
  |--------------------:|--------------------:|--------:|----------:|:----------
  |          149,757.86 |            6,677.45 |    1.4% |      0.18 | `SignTransactionECDSA`
  |          108,284.40 |            9,234.94 |    2.0% |      0.13 | `SignTransactionSchnorr`

  Note that there are already signing benchmarks in the secp256k1 library, but `SignTransaction` does much more than just the cryptographical parts, i.e.:
  * calculate the unsigned tx's `PrecomputedTransactionData` if necessary
  * apply Solver on the prevout scriptPubKey, fetch the relevant keys from the signing provider
  * perform the actual signing operation (for ECDSA signatures, that could be more than once due to low-R grinding)
  * verify if the signatures are correct by calling `VerifyScript` (more than once currently, which is fixed by this PR)

  so it probably makes sense to also have benchmarks from that higher-level application perspective.

ACKs for top commit:
  achow101:
    ACK fe92c15f0c44d1405b9048306736bd0eae868506
  furszy:
    utACK fe92c15f0c44
  glozow:
    light review ACK fe92c15f0c44d1405b9048306736bd0eae868506

Tree-SHA512: b7225ff9e8a640ca5222dea5b2a463a0f9b9de704e4330b5b9a7bce2d63a1f4620575c474a8186f4708d7d9534eab55d000393d99db79c0cfc046b35f0a4a778
2024-07-16 16:19:07 -04:00
..
2023-10-12 11:27:19 +02:00
2023-11-21 13:15:44 +00:00
2024-07-08 16:11:15 +02:00
2023-11-14 20:22:12 +01:00
2024-02-28 17:23:54 +00:00