librustzcash icon indicating copy to clipboard operation
librustzcash copied to clipboard

zcash_primitives: Rework the transparent bundle builder to be more like the shielded bundle builders

Open str4d opened this issue 2 months ago • 0 comments

The main pattern is the same: construct a Builder, add spends and outputs, build a Bundle<Unauthorized>, and apply signatures. However, there are some annoying API differences that cause problems at the transaction building level:

  • Builder::add_input takes the signing key as an argument, which prevents us from using signing keys that are stored in an HSM / hardware wallet.
    • The Sapling builder has the same issue here; it takes a Sapling extended spending key when it could just take a proof authorizing key, but the latter has weird UX so we initially went with extsk instead.
    • The Orchard builder takes an FVK, which has the intended semantics.
  • Builder::build stores the transparent signing keys inside Bundle<Unauthorized>, which similarly prevents HSM usage.
    • Currently the transaction builder stores the Sapling and Orchard spending keys, but this is a central touchpoint that could be replaced by an HSM; their built bundles do not store spending key material.
  • Bundle::<Unauthorized>::apply_signatures takes a &TransactionData<tx::Unauthorized>, which forces us to have computed Sapling proofs even for v5 transactions.
    • This will be the hardest to decouple, and probably won't be done in this issue, but I'm recording it here for completeness.
  • Bundle::<Unauthorized>::apply_signatures returns Bundle<Authorized> directly, which forces us to have #[cfg(feature = "transparent-inputs")] on its arguments, which directly encodes spooky-breakage-at-a-distance into a very low-level API.
    • Orchard has a similar one-pass method, but it is just a helper method on top of a underlying more granular API.

We should rework these APIs:

  • Decouple the transparent spending keys from the bundle.
  • Separate Bundle::<Unauthorized>::apply_signatures into several methods, so we can conditionally call the signing method when transparent-inputs is enabled, and then uniformly call a Bundle::finalize method that fails if any signatures are missing.

We might also want to think about how we will want to handle P2SH spends in the API, but that shouldn't block this refactor.

str4d avatar May 01 '24 14:05 str4d