Skip to content

psbt: Clean up a few TODOs #1043

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
apoelstra merged 6 commits into from
Sep 18, 2026
Merged

psbt: Clean up a few TODOs #1043

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
f8897ea
psbt: Check pubkey-hashes equivalence
Abeeujah Sep 3, 2026
c97922a
psbt: Update usage of Input::ecdsa_hash_ty
Abeeujah Sep 3, 2026
f5ec0d0
psbt: Clean up typo from PsbtInputSatisfier docs
Abeeujah Sep 3, 2026
6edd418
psbt: Combine find and map calls to find_map
Abeeujah Sep 3, 2026
cc260a8
psbt: Refactor PsbtExt finalize_mut methods
Abeeujah Sep 3, 2026
b3c091c
psbt: Nitpick finalizer module
Abeeujah Sep 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
120 changes: 45 additions & 75 deletions src/psbt/finalizer.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,26 +38,20 @@ fn construct_tap_witness(
// When miniscript tries to finalize the PSBT, it doesn't have the full descriptor (which contained a pkh() fragment)
// and instead resorts to parsing the raw script sig, which is translated into a "expr_raw_pkh" internally.
let mut map: BTreeMap<hash160::Hash, bitcoin::key::XOnlyPublicKey> = BTreeMap::new();
let psbt_inputs = &sat.psbt.inputs;
for psbt_input in psbt_inputs {
sat.psbt.inputs.iter().for_each(|input| {
// We need to satisfy or dissatisfy any given key. `tap_key_origin` is the only field of PSBT Input which consist of
// all the keys added on a descriptor and thus we get keys from it.
let public_keys = psbt_input.tap_key_origins.keys();
for key in public_keys {
let bitcoin_key = *key;
let hash = bitcoin_key.to_pubkeyhash(SigType::Schnorr);
map.insert(hash, bitcoin_key);
}
}
input.tap_key_origins.keys().for_each(|key| {
map.insert(key.to_pubkeyhash(SigType::Schnorr), *key);
})
});
assert!(spk.is_p2tr());

// try the key spend path firsti
if let Some(ref key) = sat.psbt_input().tap_internal_key {
if let Some(sig) =
<PsbtInputSatisfier as Satisfier<XOnlyPublicKey>>::lookup_tap_key_spend_sig(sat, key)
{
return Ok(vec![sig.to_vec()]);
}
if let Some(sig) = sat.psbt_input().tap_internal_key.as_ref().and_then(|key| {
<PsbtInputSatisfier as Satisfier<XOnlyPublicKey>>::lookup_tap_key_spend_sig(sat, key)
}) {
return Ok(vec![sig.to_vec()]);
}
// Next script spends
let (mut min_wit, mut min_wit_len) = (None, None);
Expand Down Expand Up @@ -165,37 +159,26 @@ fn get_descriptor(psbt: &Psbt, index: usize) -> Result<Descriptor<PublicKey>, In
}
} else if script_pubkey.is_p2pkh() {
// 2. `Pkh`: creates a `PkH` descriptor if partial_sigs has the corresponding pk
let partial_sig_contains_pk = inp.partial_sigs.iter().find(|&(&pk, _sig)| {
// Indirect way to check the equivalence of pubkey-hashes.
// Create a pubkey hash and check if they are the same.
// THIS IS A BUG AND *WILL* PRODUCE WRONG SATISFACTIONS FOR UNCOMPRESSED KEYS
// Partial sigs loses the compressed flag that is necessary
// TODO: See https://github.com/rust-bitcoin/rust-bitcoin/pull/836
// The type checker will fail again after we update to 0.28 and this can be removed
let addr = bitcoin::Address::p2pkh(pk, bitcoin::Network::Bitcoin);
*script_pubkey == addr.script_pubkey()
});
match partial_sig_contains_pk {
Some((pk, _sig)) => Descriptor::new_pkh(*pk).map_err(InputError::from),
None => Err(InputError::MissingPubkey),
}
let (pk, _) = inp
.partial_sigs
.iter()
.find(|&(pk, _sig)| script_pubkey.as_bytes()[3..23] == pk.pubkey_hash()[..])
.ok_or(InputError::MissingPubkey)?;
Descriptor::new_pkh(*pk).map_err(InputError::from)
} else if script_pubkey.is_p2wpkh() {
// 3. `Wpkh`: creates a `wpkh` descriptor if the partial sig has corresponding pk.
let partial_sig_contains_pk = inp.partial_sigs.iter().find(|&(&pk, _sig)| {
match bitcoin::key::CompressedPublicKey::try_from(pk) {
Ok(compressed) => {
// Indirect way to check the equivalence of pubkey-hashes.
// Create a pubkey hash and check if they are the same.
let addr = bitcoin::Address::p2wpkh(&compressed, bitcoin::Network::Bitcoin);
*script_pubkey == addr.script_pubkey()
}
Err(_) => false,
}
});
match partial_sig_contains_pk {
Some((pk, _sig)) => Ok(Descriptor::new_wpkh(*pk)?),
None => Err(InputError::MissingPubkey),
}
let (pk, _) = inp
.partial_sigs
.iter()
.find(|&(pk, _)| {
bitcoin::key::CompressedPublicKey::try_from(*pk)
.map(|compressed| {
compressed.pubkey_hash()[..] == script_pubkey.as_bytes()[2..22]
})
.unwrap_or(false)
})
.ok_or(InputError::MissingPubkey)?;
Descriptor::new_wpkh(*pk).map_err(InputError::from)
} else if script_pubkey.is_p2wsh() {
// 4. `Wsh`: creates a `Wsh` descriptor
if inp.redeem_script.is_some() {
Expand Down Expand Up @@ -241,22 +224,18 @@ fn get_descriptor(psbt: &Psbt, index: usize) -> Result<Descriptor<PublicKey>, In
}
} else if redeem_script.is_p2wpkh() {
// 6. `ShWpkh` case
let partial_sig_contains_pk = inp.partial_sigs.iter().find(|&(&pk, _sig)| {
match bitcoin::key::CompressedPublicKey::try_from(pk) {
Ok(compressed) => {
let addr = bitcoin::Address::p2wpkh(
&compressed,
bitcoin::Network::Bitcoin,
);
*redeem_script == addr.script_pubkey()
}
Err(_) => false,
}
});
match partial_sig_contains_pk {
Some((pk, _sig)) => Ok(Descriptor::new_sh_wpkh(*pk)?),
None => Err(InputError::MissingPubkey),
}
let (pk, _) = inp
.partial_sigs
.iter()
.find(|&(&pk, _sig)| {
bitcoin::key::CompressedPublicKey::try_from(pk)
.map(|compressed| {
compressed.pubkey_hash()[..] == redeem_script.as_bytes()[2..22]
})
.unwrap_or(false)
})
.ok_or(InputError::MissingPubkey)?;
Ok(Descriptor::new_sh_wpkh(*pk)?)
} else {
//7. regular p2sh
if inp.witness_script.is_some() {
Expand Down Expand Up @@ -298,17 +277,16 @@ pub fn interpreter_check<C: secp256k1::Verification>(
) -> Result<(), Error> {
let utxos = prevouts(psbt)?;
let utxos = &Prevouts::All(&utxos);
let empty_script_sig = ScriptBuf::new();
let empty_witness = Witness::default();
for (index, input) in psbt.inputs.iter().enumerate() {
let empty_script_sig = ScriptBuf::new();
let empty_witness = Witness::default();
let script_sig = input.final_script_sig.as_ref().unwrap_or(&empty_script_sig);
let witness = input
.final_script_witness
.as_ref()
.map(|wit_slice| Witness::from_slice(&wit_slice.to_vec())) // TODO: Update rust-bitcoin psbt API to use witness
.unwrap_or(empty_witness);
.unwrap_or(&empty_witness);

interpreter_inp_check(psbt, secp, index, utxos, &witness, script_sig)?;
interpreter_inp_check(psbt, secp, index, utxos, witness, script_sig)?;
}
Ok(())
}
Expand Down Expand Up @@ -445,16 +423,8 @@ pub(super) fn finalize_input<C: secp256k1::Verification>(
let input = &mut psbt.inputs[index];
input.non_witness_utxo = original.non_witness_utxo;
input.witness_utxo = original.witness_utxo;
input.final_script_sig = if script_sig.is_empty() {
None
} else {
Some(script_sig)
};
input.final_script_witness = if witness.is_empty() {
None
} else {
Some(witness)
};
input.final_script_sig = (!script_sig.is_empty()).then_some(script_sig);
input.final_script_witness = (!witness.is_empty()).then_some(witness);
}

Ok(())
Expand Down
61 changes: 18 additions & 43 deletions src/psbt/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ impl From<bitcoin::key::FromSliceError> for InputError {
fn from(e: bitcoin::key::FromSliceError) -> Self { Self::KeyErr(e) }
}

/// Psbt satisfier for at inputs at a particular index.
/// Psbt satisfier for inputs at a particular index.
///
/// Holds a `&psbt` because multiple inputs may share
/// the same psbt structure
Expand Down Expand Up @@ -283,8 +283,9 @@ impl<Pk: MiniscriptKey + ToPublicKey> Satisfier<Pk> for PsbtInputSatisfier<'_> {
self.psbt_input()
.bip32_derivation
.iter()
.find(|&(pubkey, _)| pubkey.to_pubkeyhash(SigType::Ecdsa) == *pkh)
.map(|(pubkey, _)| bitcoin::PublicKey::new(*pubkey))
.find_map(|(&pk, _)| {
(pk.to_pubkeyhash(SigType::Ecdsa) == *pkh).then_some(bitcoin::PublicKey::new(pk))
})
}

fn lookup_tap_control_block_map(
Expand All @@ -300,10 +301,9 @@ impl<Pk: MiniscriptKey + ToPublicKey> Satisfier<Pk> for PsbtInputSatisfier<'_> {
self.psbt_input()
.tap_script_sigs
.iter()
.find(|&((pubkey, lh), _sig)| {
pubkey.to_pubkeyhash(SigType::Schnorr) == pkh.0 && *lh == pkh.1
.find_map(|(&(pk, lh), &sig)| {
(pk.to_pubkeyhash(SigType::Schnorr) == pkh.0 && lh == pkh.1).then_some((pk, sig))
})
.map(|((x_only_pk, _leaf_hash), sig)| (*x_only_pk, *sig))
}

fn lookup_ecdsa_sig(&self, pk: &Pk) -> Option<bitcoin::ecdsa::Signature> {
Expand All @@ -320,8 +320,7 @@ impl<Pk: MiniscriptKey + ToPublicKey> Satisfier<Pk> for PsbtInputSatisfier<'_> {
self.psbt_input()
.partial_sigs
.iter()
.find(|&(pubkey, _sig)| pubkey.to_pubkeyhash(SigType::Ecdsa) == *pkh)
.map(|(pk, sig)| (*pk, *sig))
.find_map(|(&pk, &sig)| (pk.to_pubkeyhash(SigType::Ecdsa) == *pkh).then_some((pk, sig)))
}

fn check_after(&self, n: absolute::LockTime) -> bool {
Expand Down Expand Up @@ -387,13 +386,9 @@ fn sanity_check(psbt: &Psbt) -> Result<(), Error> {

// Check well-formedness of input data
for (index, input) in psbt.inputs.iter().enumerate() {
// TODO: fix this after https://github.com/rust-bitcoin/rust-bitcoin/issues/838
let target_ecdsa_sighash_ty = match input.sighash_type {
Some(psbt_hash_ty) => psbt_hash_ty
.ecdsa_hash_ty()
.map_err(|e| Error::InputError(InputError::NonStandardSighashType(e), index))?,
None => sighash::EcdsaSighashType::All,
};
let target_ecdsa_sighash_ty = input
.ecdsa_hash_ty()
.map_err(|e| Error::InputError(InputError::NonStandardSighashType(e), index))?;
for (key, ecdsa_sig) in &input.partial_sigs {
let flag = sighash::EcdsaSighashType::from_standard(ecdsa_sig.sighash_type as u32)
.map_err(|_| {
Expand Down Expand Up @@ -599,20 +594,10 @@ impl PsbtExt for Psbt {
secp: &secp256k1::Secp256k1<C>,
) -> Result<(), Vec<Error>> {
// Actually construct the witnesses
let mut errors = vec![];
for index in 0..self.inputs.len() {
match finalizer::finalize_input(self, index, secp, /*allow_mall*/ false) {
Ok(..) => {}
Err(e) => {
errors.push(e);
}
}
}
if errors.is_empty() {
Ok(())
} else {
Err(errors)
}
let errors = (0..self.inputs.len())
.filter_map(|i| finalizer::finalize_input(self, i, secp, /*allow_mall*/ false).err())
.collect::<Vec<Error>>();
errors.is_empty().then_some(()).ok_or(errors)
}

fn finalize<C: secp256k1::Verification>(
Expand All @@ -629,20 +614,10 @@ impl PsbtExt for Psbt {
&mut self,
secp: &secp256k1::Secp256k1<C>,
) -> Result<(), Vec<Error>> {
let mut errors = vec![];
for index in 0..self.inputs.len() {
match finalizer::finalize_input(self, index, secp, /*allow_mall*/ true) {
Ok(..) => {}
Err(e) => {
errors.push(e);
}
}
}
if errors.is_empty() {
Ok(())
} else {
Err(errors)
}
let errors = (0..self.inputs.len())
.filter_map(|i| finalizer::finalize_input(self, i, secp, /*allow_mall*/ true).err())
.collect::<Vec<Error>>();
errors.is_empty().then_some(()).ok_or(errors)
}

fn finalize_mall<C: secp256k1::Verification>(
Expand Down
Loading