Introduce per-input sighash type via Input::set_sighash_type#69
Introduce per-input sighash type via Input::set_sighash_type#69evanlinjin wants to merge 1 commit into
Input::set_sighash_type#69Conversation
556f0e1 to
9ffb42c
Compare
noahjoeris
left a comment
There was a problem hiding this comment.
Nice one!
ACK 9ffb42c
Replace `PsbtParams::sighash_type` with a per-input setter/getter on `Input`. `Selection::create_psbt` propagates each input's sighash type into the resulting PSBT input. `Input::set_sighash_type` accepts anything convertible into `PsbtSighashType`, including the standard `EcdsaSighashType` and `TapSighashType` from rust-bitcoin. BREAKING CHANGE: `PsbtParams::sighash_type` is removed. Callers should call `Input::set_sighash_type` on each input before selection.
9ffb42c to
95d4b35
Compare
This comment was marked as outdated.
This comment was marked as outdated.
|
Updated design for this PR. After working through it, the per-Input sighash API turns out to be the source of the problem rather than a feature. Replacing it with a per-PSBT-input policy on The problemFor Taproot, signature size depends on sighash: 64 bytes for
Why sighash belongs at the PSBT layer, not on
|
params.sighash_type.get(outpoint) |
plan.tap_sighash_default() |
Behavior |
|---|---|---|
Some(t) |
Some(plan_is_default) |
Validate: t's Taproot interpretation must agree with plan_is_default. Error on mismatch. Else write t. |
Some(t) |
None (ECDSA — sighash-invariant) |
No constraint. Write t. |
None |
Some(true) (Default-Taproot Plan) |
Auto-write TapSighashType::Default to lock the signer to 64-byte sigs and match the Plan's weight estimate. |
None |
Some(false) (non-Default-Taproot Plan) |
Error: Plan expects a 65-byte sig but no non-Default sighash was specified. |
None |
None (ECDSA) |
Leave the field unset; signer defaults to SIGHASH_ALL. |
PSBT-based Inputs (built via Input::from_psbt_input) are unaffected — their embedded psbt::Input already owns its sighash_type field. The map applies only to Plan-based inputs. Stray map entries for outpoints not in the selection are ignored.
Net effect
Inputis purely a spend description, cannot carry inconsistent sighash state.- All sighash logic lives in one place (
create_psbt), one rule per case. - Default-Taproot Plans get safe auto-lock with zero caller ceremony.
- Non-Default-Taproot Plans force the caller to declare a specific flag — loud failure, not silent fee drift.
- ECDSA Plans are unconstrained, matching reality.
nymius
left a comment
There was a problem hiding this comment.
Is this PR complete? I agree with the code here, but I wouldn't relay on a witness guessing scheme for extracting the SIGHASH type in the Plan variant.
It is not complete, waiting on upstream changes. Where did you think we were guessing the sighash type? Sometimes plan is based on Sighash DEFAULT - if the resultant tx uses anything else, the feerate/fee will be different to what CS assumed. The idea is to error on inconsistency. |
Description
Closes #60
Replace
PsbtParams::sighash_typewith a per-input setter/getter onInput.Selection::create_psbtpropagates each input's sighash type into the resulting PSBT input.Changelog notice
Before submitting