Introduce `TxTemplate` as a state between `Selection` and `Psbt`

[evanlinjin]

Background

After a recent revisit of the codebase, I was dissatisfied with the API shape as it is now.

Initially, I blamed it on how AFS was added in the codebase. I left a post-merge review on #5 here. The problems I pointed out can be summarized as:

• People who don't care about AFS now need to handle 2 additional error variants and an extra input.
• AFS is not "PSBT-creation-related". It's just an algorithm for deciding on locktime or input sequence values.
• There were some complex interactions between PsbtInput fields after the PR.

I suggested that AFS should be a step after create_psbt. @aagbotemi created a second PR to address my review here (#56).

After looking at this PR, I realized my suggestions were incomplete. I did another thorough investigation and I've come up with a more complete direction/approach (which I will address here). Let's start with talking about AFS.

Where should AFS go?

• AFS should be a separate step to PSBT-creation
  This is where my inital review of feat: enable random anti-fee sniping #5 got right. We don't want to pollute .create_psbt with inputs and error variants that are only relevant to AFS.

• We should have the AFS step BEFORE psbt creation
  This is my new realization. AFS is essentially just logic which decides on locktime/sequence. After the decision is made, this should be easily passed into creating a psbt. Additionally, this is how it's done in Bitcoin Core. DiscourageFeeSniping is a step before PSBT creation.

However, since AFS logic currently depends on values contained in PsbtParams, it's difficult to implement it elsewhere. We need an architectural refactor.

Proposal

I propose putting an additional stage in our transaction-creation pipeline called TxTemplate. This stage will sit between Selection and Psbt. Unlike the Selection type, TxTemplate will have complete information to create a bitcoin::Transaction (including the version, locktime and sequence fields which currently all reside in PsbtParams).

let mut rand = rand::thread_rng();

let mut (psbt, finalizer) = selection
    .into_tx_template(TxTemplateParams::default())
    .shuffle_inputs(&mut rand)
    .apply_anti_fee_sniping(tip_height, &mut rand)?
    .create_psbt(PsbtBuildParams::default())?;

let _ = psbt.sign(&signer, &secp);
let res = finalizer.finalize(&mut psbt);
assert!(res.is_finalized());

Benefits of TxTemplate

• Clearer separation of concern. TxTemplateParams can purely be for deciding on bitcoin transaction fields. PsbtParams can be for PSBT-specific options.
• Callers that do not wish to work with Psbts are no longer forced to. Additionally, when we add support for PSBT V2 in the future, we have a clean way to do it as a method on TxTemplate::create_psbt_v2().
• AFS can be a single method on TxTemplate.

Shape of TxTemplate and TxTemplateParams

/// Parameters for resolving a [`Selection`] into a [`TxTemplate`].
///
/// Carries only the *floors* and *defaults* that must be resolved at
/// template construction. Optional transformations (anti-fee-sniping,
/// shuffling, etc.) are exposed as methods on [`TxTemplate`].
#[derive(Debug, Clone)]
pub struct TxTemplateParams {
    /// Minimum tx version. Acts as a floor on `tx.version`.
    ///
    /// Default: [`transaction::Version::TWO`].
    pub min_version: transaction::Version,

    /// Minimum tx lock_time. Acts as a floor on `tx.lock_time`.
    ///
    /// Default: [`absolute::LockTime::ZERO`].
    pub min_locktime: absolute::LockTime,

    /// Default sequence used for plan-based inputs that don't specify their
    /// own (via plan-required relative timelock or via
    /// [`Input::set_sequence`]).
    ///
    /// Default: [`FALLBACK_SEQUENCE`] (Bitcoin Core's wallet default).
    pub fallback_sequence: Sequence,
}

/// A fully-resolved tx shape, intermediate between [`Selection`] and the
/// final [`Psbt`] / [`bitcoin::Transaction`].
#[derive(Debug, Clone)]
pub struct TxTemplate {
    pub(crate) version: transaction::Version,
    pub(crate) lock_time: absolute::LockTime,
    pub(crate) inputs: Vec<Input>,
    pub(crate) outputs: Vec<Output>,
}

In order for the above shape to work, the Input type should be able to specify the sequence and sighash values.

• PsbtParams::sighash_type does not make sense #60
• add Input::set_sequence for per-input sequence override #61

[nymius]

I presume when PsbtV2 support is in place, TxTemplate is going to be completely replaced rather than used as a pass through, because v2 doesn't force you to create the tx before. I would keep this simple to make that replacement also simple.

[evanlinjin]

I presume when PsbtV2 support is in place, TxTemplate is going to be completely replaced rather than used as a pass through, because v2 doesn't force you to create the tx before. I would keep this simple to make that replacement also simple.

For some context, TxTemplate has the following responsibilities:

• resolve version (auto-bump to v2 on relative timelocks)
• resolve lock_time from input CLTVs + set_locktime overrides
• apply AFS / shuffle / sort / per-input sequence overrides
• order inputs and outputs

These responsibilities predate any PSBT spec. In fact, I think it's good that we don't rely PSBTs up to this point. Frostsnap's signing protocol is not based on PSBT, neither are some coinjoin protocols (Wasabi).

How would you envision that API with PSBT v2? Are you suggesting that we introduce some flexibility that is allowed by the PSBT v2 spec? What happens to inputs/outputs in a v2-forward-thinking replacement, given coin selection runs on those?