Merge bitcoin/bitcoin#33891: kernel: Expose reusable PrecomputedTransactionData in script validation

44e006d [kernel] Expose reusable PrecomputedTransactionData in script valid (Josh Doman)

Pull request description:

  This PR exposes a reusable `PrecomputedTransactionData` object in script validation using libkernel.

  Currently, libkernel computes `PrecomputedTransactionData` each time `btck_script_pubkey_verify` is called, exposing clients to quadratic hashing when validating a transaction with multiple inputs. By externalizing `PrecomputedTransactionData` and making it reusable, libkernel can eliminate this attack vector.

  I discussed this problem in [this issue](sedited/rust-bitcoinkernel#46). The design of this PR is inspired by @sedited's comments.

  The PR introduces three new APIs for managing the `btck_PrecomputedTransactionData` object:
  ```c
  /**
   * @brief Create precomputed transaction data for script verification.
   *
   * @param[in] tx_to             Non-null.
   * @param[in] spent_outputs     Nullable for non-taproot verification. Points to an array of
   *                              outputs spent by the transaction.
   * @param[in] spent_outputs_len Length of the spent_outputs array.
   * @return                      The precomputed data, or null on error.
   */
  btck_PrecomputedTransactionData* btck_precomputed_transaction_data_create(
      const btck_Transaction* tx_to,
      const btck_TransactionOutput** spent_outputs, size_t spent_outputs_len) BITCOINKERNEL_ARG_NONNULL(1);

  /**
   * @brief Copy precomputed transaction data.
   *
   * @param[in] precomputed_txdata  Non-null.
   * @return                      The copied precomputed transaction data.
   */
  btck_PrecomputedTransactionData* btck_precomputed_transaction_data_copy(
      const btck_PrecomputedTransactionData* precomputed_txdata) BITCOINKERNEL_ARG_NONNULL(1);

  /**
   * Destroy the precomputed transaction data.
   */
  void btck_precomputed_transaction_data_destroy(btck_PrecomputedTransactionData* precomputed_txdata);
  ```

  The PR also modifies `btck_script_pubkey_verify` so that it accepts `precomputed_txdata` instead of `spent_outputs`:
  ```c
  /**
   * @brief Verify if the input at input_index of tx_to spends the script pubkey
   * under the constraints specified by flags. If the
   * `btck_ScriptVerificationFlags_WITNESS` flag is set in the flags bitfield, the
   * amount parameter is used. If the taproot flag is set, the precomputed data
   * must contain the spent outputs.
   *
   * @param[in] script_pubkey      Non-null, script pubkey to be spent.
   * @param[in] amount             Amount of the script pubkey's associated output. May be zero if
   *                               the witness flag is not set.
   * @param[in] tx_to              Non-null, transaction spending the script_pubkey.
   * @param[in] precomputed_txdata Nullable if the taproot flag is not set. Otherwise, precomputed data
   *                               for tx_to with the spent outputs must be provided.
   * @param[in] input_index        Index of the input in tx_to spending the script_pubkey.
   * @param[in] flags              Bitfield of btck_ScriptVerificationFlags controlling validation constraints.
   * @param[out] status            Nullable, will be set to an error code if the operation fails, or OK otherwise.
   * @return                       1 if the script is valid, 0 otherwise.
   */
  int btck_script_pubkey_verify(
      const btck_ScriptPubkey* script_pubkey,
      int64_t amount,
      const btck_Transaction* tx_to,
      const btck_PrecomputedTransactionData* precomputed_txdata,
      unsigned int input_index,
      btck_ScriptVerificationFlags flags,
      btck_ScriptVerifyStatus* status) BITCOINKERNEL_ARG_NONNULL(1, 3);
  ```

  As before, an error is thrown if the taproot flag is set and `spent_outputs` is not provided in `precomputed_txdata` (or `precomputed_txdata` is null). For simple single-input non-taproot verification, `precomputed_txdata` may be null, and the kernel will construct the precomputed data on-the-fly.

  Both the C++ wrapper and the test suite are updated with the new API. Tests cover both `precomputed_txdata` reuse and nullability.

  Appreciate feedback on this concept / approach!

ACKs for top commit:
  sedited:
    Re-ACK 44e006d
  stringintech:
    ACK 44e006d

Tree-SHA512: 1ed435173e6ff4ec82bc603194cf182c685cb79f167439a442b9b179a32f6c189c358f04d4cb56d153fab04e3424a11b73c31680e42b87b8a6efcc3ccefc366c

Author: fanquake

src/kernel/bitcoinkernel.cpp

@@ -495,6 +495,7 @@ struct btck_BlockHash : Handle<btck_BlockHash, uint256> {};
 struct btck_TransactionInput : Handle<btck_TransactionInput, CTxIn> {};
 struct btck_TransactionOutPoint: Handle<btck_TransactionOutPoint, COutPoint> {};
 struct btck_Txid: Handle<btck_Txid, Txid> {};
+struct btck_PrecomputedTransactionData : Handle<btck_PrecomputedTransactionData, PrecomputedTransactionData> {};
 
 btck_Transaction* btck_transaction_create(const void* raw_transaction, size_t raw_transaction_len)
 {
@@ -608,10 +609,46 @@ void btck_transaction_output_destroy(btck_TransactionOutput* output)
     delete output;
 }
 
+btck_PrecomputedTransactionData* btck_precomputed_transaction_data_create(
+    const btck_Transaction* tx_to,
+    const btck_TransactionOutput** spent_outputs_, size_t spent_outputs_len)
+{
+    try {
+        const CTransaction& tx{*btck_Transaction::get(tx_to)};
+        auto txdata{btck_PrecomputedTransactionData::create()};
+        if (spent_outputs_ != nullptr && spent_outputs_len > 0) {
+            assert(spent_outputs_len == tx.vin.size());
+            std::vector<CTxOut> spent_outputs;
+            spent_outputs.reserve(spent_outputs_len);
+            for (size_t i = 0; i < spent_outputs_len; i++) {
+                const CTxOut& tx_out{btck_TransactionOutput::get(spent_outputs_[i])};
+                spent_outputs.push_back(tx_out);
+            }
+            btck_PrecomputedTransactionData::get(txdata).Init(tx, std::move(spent_outputs));
+        } else {
+            btck_PrecomputedTransactionData::get(txdata).Init(tx, {});
+        }
+
+        return txdata;
+    } catch (...) {
+        return nullptr;
+    }
+}
+
+btck_PrecomputedTransactionData* btck_precomputed_transaction_data_copy(const btck_PrecomputedTransactionData* precomputed_txdata)
+{
+    return btck_PrecomputedTransactionData::copy(precomputed_txdata);
+}
+
+void btck_precomputed_transaction_data_destroy(btck_PrecomputedTransactionData* precomputed_txdata)
+{
+    delete precomputed_txdata;
+}
+
 int btck_script_pubkey_verify(const btck_ScriptPubkey* script_pubkey,
                               const int64_t amount,
                               const btck_Transaction* tx_to,
-                              const btck_TransactionOutput** spent_outputs_, size_t spent_outputs_len,
+                              const btck_PrecomputedTransactionData* precomputed_txdata,
                               const unsigned int input_index,
                               const btck_ScriptVerificationFlags flags,
                               btck_ScriptVerifyStatus* status)
@@ -624,31 +661,18 @@ int btck_script_pubkey_verify(const btck_ScriptPubkey* script_pubkey,
         return 0;
     }
 
-    if (flags & btck_ScriptVerificationFlags_TAPROOT && spent_outputs_ == nullptr) {
+    const CTransaction& tx{*btck_Transaction::get(tx_to)};
+    assert(input_index < tx.vin.size());
+
+    const PrecomputedTransactionData& txdata{precomputed_txdata ? btck_PrecomputedTransactionData::get(precomputed_txdata) : PrecomputedTransactionData(tx)};
+
+    if (flags & btck_ScriptVerificationFlags_TAPROOT && txdata.m_spent_outputs.empty()) {
         if (status) *status = btck_ScriptVerifyStatus_ERROR_SPENT_OUTPUTS_REQUIRED;
         return 0;
     }
 
     if (status) *status = btck_ScriptVerifyStatus_OK;
 
-    const CTransaction& tx{*btck_Transaction::get(tx_to)};
-    std::vector<CTxOut> spent_outputs;
-    if (spent_outputs_ != nullptr) {
-        assert(spent_outputs_len == tx.vin.size());
-        spent_outputs.reserve(spent_outputs_len);
-        for (size_t i = 0; i < spent_outputs_len; i++) {
-            const CTxOut& tx_out{btck_TransactionOutput::get(spent_outputs_[i])};
-            spent_outputs.push_back(tx_out);
-        }
-    }
-
-    assert(input_index < tx.vin.size());
-    PrecomputedTransactionData txdata{tx};
-
-    if (spent_outputs_ != nullptr && flags & btck_ScriptVerificationFlags_TAPROOT) {
-        txdata.Init(tx, std::move(spent_outputs));
-    }
-
     bool result = VerifyScript(tx.vin[input_index].scriptSig,
                                btck_ScriptPubkey::get(script_pubkey),
                                &tx.vin[input_index].scriptWitness,

src/kernel/bitcoinkernel.h

@@ -282,6 +282,16 @@ typedef struct btck_TransactionInput btck_TransactionInput;
  */
 typedef struct btck_TransactionOutPoint btck_TransactionOutPoint;
 
+/**
+ * Opaque data structure for holding precomputed transaction data.
+ *
+ * Reusable when verifying multiple inputs of the same transaction.
+ * This avoids recomputing transaction hashes for each input.
+ *
+ * Required when verifying a taproot input.
+ */
+typedef struct btck_PrecomputedTransactionData btck_PrecomputedTransactionData;
+
 typedef struct btck_Txid btck_Txid;
 
 /** Current sync state passed to tip changed callbacks. */
@@ -571,6 +581,40 @@ BITCOINKERNEL_API void btck_transaction_destroy(btck_Transaction* transaction);
 
 ///@}
 
+/** @name PrecomputedTransactionData
+ * Functions for working with precomputed transaction data.
+ */
+///@{
+
+/**
+ * @brief Create precomputed transaction data for script verification.
+ *
+ * @param[in] tx_to             Non-null.
+ * @param[in] spent_outputs     Nullable for non-taproot verification. Points to an array of
+ *                              outputs spent by the transaction.
+ * @param[in] spent_outputs_len Length of the spent_outputs array.
+ * @return                      The precomputed data, or null on error.
+ */
+BITCOINKERNEL_API btck_PrecomputedTransactionData* BITCOINKERNEL_WARN_UNUSED_RESULT btck_precomputed_transaction_data_create(
+    const btck_Transaction* tx_to,
+    const btck_TransactionOutput** spent_outputs, size_t spent_outputs_len) BITCOINKERNEL_ARG_NONNULL(1);
+
+/**
+ * @brief Copy precomputed transaction data.
+ *
+ * @param[in] precomputed_txdata Non-null.
+ * @return                       The copied precomputed transaction data.
+ */
+BITCOINKERNEL_API btck_PrecomputedTransactionData* BITCOINKERNEL_WARN_UNUSED_RESULT btck_precomputed_transaction_data_copy(
+    const btck_PrecomputedTransactionData* precomputed_txdata) BITCOINKERNEL_ARG_NONNULL(1);
+
+/**
+ * Destroy the precomputed transaction data.
+ */
+BITCOINKERNEL_API void btck_precomputed_transaction_data_destroy(btck_PrecomputedTransactionData* precomputed_txdata);
+
+///@}
+
 /** @name ScriptPubkey
  * Functions for working with script pubkeys.
  */
@@ -598,26 +642,25 @@ BITCOINKERNEL_API btck_ScriptPubkey* BITCOINKERNEL_WARN_UNUSED_RESULT btck_scrip
  * @brief Verify if the input at input_index of tx_to spends the script pubkey
  * under the constraints specified by flags. If the
  * `btck_ScriptVerificationFlags_WITNESS` flag is set in the flags bitfield, the
- * amount parameter is used. If the taproot flag is set, the spent outputs
- * parameter is used to validate taproot transactions.
- *
- * @param[in] script_pubkey     Non-null, script pubkey to be spent.
- * @param[in] amount            Amount of the script pubkey's associated output. May be zero if
- *                              the witness flag is not set.
- * @param[in] tx_to             Non-null, transaction spending the script_pubkey.
- * @param[in] spent_outputs     Nullable if the taproot flag is not set. Points to an array of
- *                              outputs spent by the transaction.
- * @param[in] spent_outputs_len Length of the spent_outputs array.
- * @param[in] input_index       Index of the input in tx_to spending the script_pubkey.
- * @param[in] flags             Bitfield of btck_ScriptVerificationFlags controlling validation constraints.
- * @param[out] status           Nullable, will be set to an error code if the operation fails, or OK otherwise.
- * @return                      1 if the script is valid, 0 otherwise.
+ * amount parameter is used. If the taproot flag is set, the precomputed data
+ * must contain the spent outputs.
+ *
+ * @param[in] script_pubkey      Non-null, script pubkey to be spent.
+ * @param[in] amount             Amount of the script pubkey's associated output. May be zero if
+ *                               the witness flag is not set.
+ * @param[in] tx_to              Non-null, transaction spending the script_pubkey.
+ * @param[in] precomputed_txdata Nullable if the taproot flag is not set. Otherwise, precomputed data
+ *                               for tx_to with the spent outputs must be provided.
+ * @param[in] input_index        Index of the input in tx_to spending the script_pubkey.
+ * @param[in] flags              Bitfield of btck_ScriptVerificationFlags controlling validation constraints.
+ * @param[out] status            Nullable, will be set to an error code if the operation fails, or OK otherwise.
+ * @return                       1 if the script is valid, 0 otherwise.
  */
 BITCOINKERNEL_API int BITCOINKERNEL_WARN_UNUSED_RESULT btck_script_pubkey_verify(
     const btck_ScriptPubkey* script_pubkey,
     int64_t amount,
     const btck_Transaction* tx_to,
-    const btck_TransactionOutput** spent_outputs, size_t spent_outputs_len,
+    const btck_PrecomputedTransactionData* precomputed_txdata,
     unsigned int input_index,
     btck_ScriptVerificationFlags flags,
     btck_ScriptVerifyStatus* status) BITCOINKERNEL_ARG_NONNULL(1, 3);

src/kernel/bitcoinkernel_wrapper.h

@@ -367,6 +367,7 @@ class UniqueHandle
     const CType* get() const { return m_ptr.get(); }
 };
 
+class PrecomputedTransactionData;
 class Transaction;
 class TransactionOutput;
 
@@ -385,7 +386,7 @@ class ScriptPubkeyApi
 public:
     bool Verify(int64_t amount,
                 const Transaction& tx_to,
-                std::span<const TransactionOutput> spent_outputs,
+                const PrecomputedTransactionData* precomputed_txdata,
                 unsigned int input_index,
                 ScriptVerificationFlags flags,
                 ScriptVerifyStatus& status) const;
@@ -626,29 +627,30 @@ class Transaction : public Handle<btck_Transaction, btck_transaction_copy, btck_
         : Handle{view} {}
 };
 
+class PrecomputedTransactionData : public Handle<btck_PrecomputedTransactionData, btck_precomputed_transaction_data_copy, btck_precomputed_transaction_data_destroy>
+{
+public:
+    explicit PrecomputedTransactionData(const Transaction& tx_to, std::span<const TransactionOutput> spent_outputs)
+        : Handle{btck_precomputed_transaction_data_create(
+            tx_to.get(),
+            reinterpret_cast<const btck_TransactionOutput**>(
+                const_cast<TransactionOutput*>(spent_outputs.data())),
+            spent_outputs.size())} {}
+};
+
 template <typename Derived>
 bool ScriptPubkeyApi<Derived>::Verify(int64_t amount,
                                       const Transaction& tx_to,
-                                      const std::span<const TransactionOutput> spent_outputs,
+                                      const PrecomputedTransactionData* precomputed_txdata,
                                       unsigned int input_index,
                                       ScriptVerificationFlags flags,
                                       ScriptVerifyStatus& status) const
 {
-    const btck_TransactionOutput** spent_outputs_ptr = nullptr;
-    std::vector<const btck_TransactionOutput*> raw_spent_outputs;
-    if (spent_outputs.size() > 0) {
-        raw_spent_outputs.reserve(spent_outputs.size());
-
-        for (const auto& output : spent_outputs) {
-            raw_spent_outputs.push_back(output.get());
-        }
-        spent_outputs_ptr = raw_spent_outputs.data();
-    }
     auto result = btck_script_pubkey_verify(
         impl(),
         amount,
         tx_to.get(),
-        spent_outputs_ptr, spent_outputs.size(),
+        precomputed_txdata ? precomputed_txdata->get() : nullptr,
         input_index,
         static_cast<btck_ScriptVerificationFlags>(flags),
         reinterpret_cast<btck_ScriptVerifyStatus*>(&status));