finalizer: BIP174 compliance + sighash/signature-size validation

[evanlinjin]

Description

Hardens Finalizer to follow BIP174 more closely and refuse to emit a transaction that would be malformed or unsafe to broadcast. The finalizer now validates signatures before assembling the final witness/scriptSig, and reports failures through a dedicated FinalizeError instead of leaking miniscript::Error.

More context here: #74 (comment)

What changed:

• Retain unknown/proprietary input fields. Per BIP174 a finalizer must clear all input data except the UTXO and unknown/proprietary fields; finalization now preserves unknown and proprietary.
• SighashMismatch — a signature's sighash type disagrees with the input's declared PSBT_IN_SIGHASH_TYPE. Mandated by BIP174.
• SighashNotAllowed — no PSBT_IN_SIGHASH_TYPE is declared, yet a signature uses neither DEFAULT nor ALL. Stricter-than-spec safeguard against silently changing signing semantics.
• SignatureTooLarge — a satisfied schnorr witness is larger than the size the spending Plan committed to (e.g. a 65-byte SIGHASH_ALL sig where 64-byte SIGHASH_DEFAULT was planned). Because the plan's weight/feerate estimate derives from the committed size, a heavier witness makes the tx undershoot its target feerate and risk being unbroadcastable. Only the larger direction is rejected; a smaller witness merely overpays and is allowed.
• Dedicated FinalizeError — finalize_input / FinalizeMap now return FinalizeError, modelling the failures above plus Satisfaction(miniscript::Error).

Notes to the reviewers

SignatureTooLarge is intentionally not unit-tested: it is only reachable in release builds. In debug, miniscript's satisfy_self hits a debug_assert! on the signature size and panics before our check runs - an upstream bug.

Out of scope, for follow-up:

• ECDSA signatures are estimated at their 73-byte max, so they can't exceed the plan and need no check today (TODO in code). An upstream change could give ECDSA placeholders a committed size to capture low-R grinding fee savings safely.
• The upstream rust-miniscript debug_assert → recoverable-error fix.

Changelog notice

Changed:
- **(breaking)** `Finalizer::finalize_input`, `FinalizeMap`, and `FinalizeMap::results` now return the new `FinalizeError` type instead of `miniscript::Error`.

Added:
- `FinalizeError` enum with `SighashMismatch`, `SighashNotAllowed`, `SignatureTooLarge`, and `Satisfaction` variants.
- Finalization now validates that each signature's sighash type respects `PSBT_IN_SIGHASH_TYPE` (or is `DEFAULT`/`ALL` when unset) and that satisfied witness sizes do not exceed the spending plan.

Fixed:
- Finalization now retains `unknown` and `PSBT_IN_PROPRIETARY` input fields, as required by BIP174.

Before submitting

• I followed the contribution guidelines
• This PR breaks the existing API

[nymius]

cACK 0291c17

[nymius]

How should a recovery mechanism look like in the case we want to save the extra sats? Should we return an enum here with "Finalized, NotFinalized, Overshoot" or something similar? Should we compare the target fee rate and inner fee rate at the end of the tx creation process and rewind if they don't match?

[evanlinjin]

Interesting question. I think we should reserve this PR purely for safety. Everything else can be tackled in separate PRs. Could you create a ticket? Thanks!

[nymius]

This is the only way I found to reproduce the SignatureTooLarge error:

    #[test]
    #[cfg(not(debug_assertions))]
    fn test_finalize_signature_too_large() -> anyhow::Result<()> {
        let (non_default_input, input, keymap) = {
            let secp = Secp256k1::new();
            let (desc, keymap) = Descriptor::parse_descriptor(&secp, TR_XPRV)?;
            let def_desc = desc.at_derivation_index(0)?;
            let script_pubkey = def_desc.script_pubkey();

            let assets = keymap.keys().fold(Assets::new(), |a, k| a.add(k.clone()));
            let mut non_default_assets = keymap.keys().fold(Assets::new(), |a, k| a.add(k.clone()));
            non_default_assets.keys = std::mem::take(&mut non_default_assets.keys)
                .into_iter()
                .map(|(key_source, mut can_sign)| {
                    can_sign.taproot.sighash_default = false;
                    (key_source, can_sign)
                })
                .collect();

            let plan = def_desc.clone().plan(&assets).expect("failed to create plan");
            let non_default_plan = def_desc.plan(&non_default_assets).expect("failed to create plan");

            let prev_tx = Transaction {
                version: transaction::Version::TWO,
                lock_time: absolute::LockTime::ZERO,
                input: vec![TxIn::default()],
                output: vec![TxOut {
                    script_pubkey,
                    value: Amount::from_sat(100_000),
                }],
            };

            let status = crate::ConfirmationStatus::new(1_000, Some(500_000_000))?;
            let non_default_input = crate::Input::from_prev_tx(non_default_plan, prev_tx.clone(), 0, Some(status))?;
            let input = crate::Input::from_prev_tx(plan, prev_tx, 0, Some(status))?;
            (non_default_input, input, keymap)
        };
        let output = Output::with_script(ScriptBuf::new(), Amount::from_sat(9_000));
        let non_default_selection = Selection::new(vec![non_default_input], vec![output.clone()]);

        let mut psbt = non_default_selection.create_psbt(PsbtParams::default())?;

        psbt.inputs[0].sighash_type = Some(TapSighashType::All.into());
        psbt.sign(&Signer(keymap), &Secp256k1::new())
            .expect("signing failed");

        psbt.inputs[0].sighash_type = None;

        let selection = Selection::new(vec![input], vec![output]);
        let finalizer = selection.into_finalizer();
        let err = finalizer.finalize_input(&mut psbt, 0).unwrap_err();
        assert!(matches!(err, FinalizeError::SignatureTooLarge { .. }));

        Ok(())
    }

I run it with:

cargo test --release -- test_finalize_signature_too_large

because there is a debug_assert in the satisfier that was panicking earlier and preventing the error to happen.

[nymius]

Suggested change

	/// * [`SighashMismatch`] - a signature's sighash type disagrees with the input's declared
	/// `PSBT_IN_SIGHASH_TYPE`.
	/// * [`SighashMismatch`] - a signature's sighash type disagrees with the input's declared in
	/// `PSBT_IN_SIGHASH_TYPE`.