builder: Use correct `PrecomputedTransactionData` for transparent sigs

Before merging 4.7.0-rc1 into the nu5-consensus branch, we were in a
split state:

- 4.7.0-rc1 included Orchard support in the transaction builder, which
  required special handling of Orchard bundles when computing sighashes.
  The `PrecomputedTransactionData` structure could be shared, because
  its digests were only relevant to transparent signatures (as shielded
  signatures signed the txid directly even in shielding transactions).

- nu5-consensus included the changes to ZIP 244, which required passing
  around a `PrecomputedTransactionData` that contained the set of all
  transparent inputs being spent, because shielding transactions now also
  need to commit to transparent inputs.

In the merge commit, we incorrectly handled the resolution: we correctly
derived a fresh `PrecomputedTransactionData` when signing the Orchard
bundle, but we reused the `PrecomputedTransactionData` that was
previously derived before checking whether or not we even had an Orchard
bundle, for transparent inputs. This meant that its commitments didn't
commit to the Orchard bundle, and so transparent signatures on
transactions with Orchard bundles would fail to verify.

Incidentally, this is the exact inverse of a bug we encounted while
implementing the ZIP 244 changes on the nu5-consensus branch: we were
correctly computing the transparent sighash, but we were relying on the
initial `TxDigests` derived within `PrecomputedTransactionData` for the
Orchard sighash, even though we were actively rewriting the transaction
to include the Orchard bundle. The fix there was similarly to re-compute
the `TxDigests` before computing the sighash.
This commit is contained in:
Jack Grigg 2022-03-23 18:27:12 +00:00
parent 85b5595519
commit d9c1b05a83
1 changed files with 2 additions and 1 deletions

View File

@ -628,7 +628,6 @@ TransactionBuilderResult TransactionBuilder::Build()
//
auto consensusBranchId = CurrentEpochBranchId(nHeight, consensusParams);
const PrecomputedTransactionData txdata(mtx, tIns);
// Empty output script.
uint256 dataToBeSigned;
@ -638,6 +637,7 @@ TransactionBuilderResult TransactionBuilder::Build()
dataToBeSigned = ProduceZip244SignatureHash(mtx, tIns, orchardBundle.value());
} else {
CScript scriptCode;
const PrecomputedTransactionData txdata(mtx, tIns);
dataToBeSigned = SignatureHash(scriptCode, mtx, NOT_AN_INPUT, SIGHASH_ALL, 0, consensusBranchId, txdata);
}
} catch (std::logic_error ex) {
@ -691,6 +691,7 @@ TransactionBuilderResult TransactionBuilder::Build()
// Transparent signatures
CTransaction txNewConst(mtx);
const PrecomputedTransactionData txdata(txNewConst, tIns);
for (int nIn = 0; nIn < mtx.vin.size(); nIn++) {
auto tIn = tIns[nIn];
SignatureData sigdata;