From 8c6d411bc4c2f36740dd5c81eddbac2c4108bac0 Mon Sep 17 00:00:00 2001 From: Muhammad Date: Sun, 3 May 2026 09:36:34 +0100 Subject: [PATCH] fix(signer): respect allow_all_sighashes in sign_input --- src/wallet/error.rs | 2 +- src/wallet/signer.rs | 107 ++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 107 insertions(+), 2 deletions(-) diff --git a/src/wallet/error.rs b/src/wallet/error.rs index ddd07478..c1784447 100644 --- a/src/wallet/error.rs +++ b/src/wallet/error.rs @@ -132,7 +132,7 @@ impl From for LoadError { } /// Errors returned by miniscript when updating inconsistent PSBTs -#[derive(Debug, Clone)] +#[derive(Debug, Clone, PartialEq)] pub enum MiniscriptPsbtError { /// Descriptor key conversion error Conversion(miniscript::descriptor::ConversionError), diff --git a/src/wallet/signer.rs b/src/wallet/signer.rs index 88050db5..c0e5f499 100644 --- a/src/wallet/signer.rs +++ b/src/wallet/signer.rs @@ -134,7 +134,7 @@ impl From for SignerId { } /// Signing error -#[derive(Debug)] +#[derive(Debug, PartialEq)] pub enum SignerError { /// The private key is missing for the required public key MissingKey, @@ -464,6 +464,17 @@ impl InputSigner for SignerWrapper { return Ok(()); } + if !sign_options.allow_all_sighashes { + let sighash_type = psbt.inputs[input_index].sighash_type; + if sighash_type.is_some() + && sighash_type != Some(EcdsaSighashType::All.into()) + && sighash_type != Some(TapSighashType::All.into()) + && sighash_type != Some(TapSighashType::Default.into()) + { + return Err(SignerError::NonStandardSighash); + } + } + let pubkey = PublicKey::from_private_key(secp, self); match self.ctx { @@ -1010,6 +1021,100 @@ mod signers_container_tests { assert_matches!(signers.find(id_nonexistent), None); } + #[test] + fn sign_input_rejects_non_standard_sighash() { + use bitcoin::{ + absolute, psbt::Input, sighash::TapSighashType, transaction, Amount, OutPoint, + PrivateKey, ScriptBuf, Sequence, TxIn, TxOut, Witness, + }; + + let secp = Secp256k1::new(); + + // Build a private key and derive its x-only pubkey for taproot + let tprv = bip32::Xpriv::from_str(TPRV0_STR).unwrap(); + let priv_key = PrivateKey::new(tprv.private_key, bitcoin::NetworkKind::Test); + let pubkey = priv_key.public_key(&secp); + let x_only = bitcoin::key::XOnlyPublicKey::from(pubkey.inner); + + // Wrap as a taproot signer treating this key as the internal key + let signer = SignerWrapper::new( + priv_key, + SignerContext::Tap { + is_internal_key: true, + }, + ); + + // Build a minimal unsigned transaction with one input + let unsigned_tx = bitcoin::Transaction { + version: transaction::Version::TWO, + lock_time: absolute::LockTime::ZERO, + input: vec![TxIn { + previous_output: OutPoint::null(), + script_sig: ScriptBuf::default(), + sequence: Sequence::MAX, + witness: Witness::default(), + }], + output: vec![TxOut { + value: Amount::from_sat(90_000), + script_pubkey: ScriptBuf::default(), + }], + }; + + // Build a base PSBT from the unsigned transaction to be cloned for each assertion + let mut base_psbt = bitcoin::Psbt::from_unsigned_tx(unsigned_tx).unwrap(); + base_psbt.inputs[0] = Input { + tap_internal_key: Some(x_only), + witness_utxo: Some(TxOut { + value: Amount::from_sat(100_000), + script_pubkey: ScriptBuf::default(), + }), + ..Default::default() + }; + + let opts_reject = SignOptions { + allow_all_sighashes: false, + ..Default::default() + }; + + let opts_allow = SignOptions { + allow_all_sighashes: true, + ..Default::default() + }; + + // SIGHASH_NONE + allow_all_sighashes: false -> must reject + let mut psbt = base_psbt.clone(); + psbt.inputs[0].sighash_type = Some(TapSighashType::None.into()); + assert_eq!( + signer.sign_input(&mut psbt, 0, &opts_reject, &secp), + Err(SignerError::NonStandardSighash), + "expected NonStandardSighash when allow_all_sighashes is false" + ); + + // SIGHASH_ALL + allow_all_sighashes: false -> must pass guard + let mut psbt = base_psbt.clone(); + psbt.inputs[0].sighash_type = Some(TapSighashType::All.into()); + assert!( + signer.sign_input(&mut psbt, 0, &opts_reject, &secp).is_ok(), + "SIGHASH_ALL should not be rejected when allow_all_sighashes is false" + ); + + // SIGHASH_DEFAULT + allow_all_sighashes: false -> must pass guard + let mut psbt = base_psbt.clone(); + psbt.inputs[0].sighash_type = Some(TapSighashType::Default.into()); + assert!( + signer.sign_input(&mut psbt, 0, &opts_reject, &secp).is_ok(), + "SIGHASH_DEFAULT should not be rejected when allow_all_sighashes is false" + ); + + // SIGHASH_NONE + allow_all_sighashes: true -> guard must not reject + let mut psbt = base_psbt.clone(); + psbt.inputs[0].sighash_type = Some(TapSighashType::None.into()); + assert!( + signer.sign_input(&mut psbt, 0, &opts_allow, &secp).is_ok(), + "expected guard to pass when allow_all_sighashes is true" + ); + } + #[derive(Debug, Clone, Copy)] struct DummySigner { number: u64,