[signer] Replace force_non_witness_utxo with only_witness_utxo

Instead of providing an opt-in option to force the addition of the
`non_witness_utxo`, we will now add them by default and provide the
option to disable them when they aren't considered necessary.
This commit is contained in:
Alekos Filini 2021-04-20 14:58:33 +02:00
parent b5e9589803
commit 17bcd8ed7d
No known key found for this signature in database
GPG Key ID: 431401E4A4530061
3 changed files with 74 additions and 38 deletions

View File

@ -17,6 +17,7 @@ Timelocks are considered (optionally) in building the `satisfaction` field
- Changed `Wallet::{sign, finalize_psbt}` now take a `&mut psbt` rather than consuming it. - Changed `Wallet::{sign, finalize_psbt}` now take a `&mut psbt` rather than consuming it.
- Require and validate `non_witness_utxo` for SegWit signatures by default, can be adjusted with `SignOptions` - Require and validate `non_witness_utxo` for SegWit signatures by default, can be adjusted with `SignOptions`
- Replace the opt-in builder option `force_non_witness_utxo` with the opposite `only_witness_utxo`. From now on we will provide the `non_witness_utxo`, unless explicitly asked not to.
## [v0.6.0] - [v0.5.1] ## [v0.6.0] - [v0.5.1]

View File

@ -1271,28 +1271,23 @@ where
match utxo { match utxo {
Utxo::Local(utxo) => { Utxo::Local(utxo) => {
*psbt_input = match self.get_psbt_input( *psbt_input =
utxo, match self.get_psbt_input(utxo, params.sighash, params.only_witness_utxo) {
params.sighash, Ok(psbt_input) => psbt_input,
params.force_non_witness_utxo, Err(e) => match e {
) { Error::UnknownUtxo => Input {
Ok(psbt_input) => psbt_input, sighash_type: params.sighash,
Err(e) => match e { ..Input::default()
Error::UnknownUtxo => Input { },
sighash_type: params.sighash, _ => return Err(e),
..Input::default()
}, },
_ => return Err(e), }
},
}
} }
Utxo::Foreign { Utxo::Foreign {
psbt_input: foreign_psbt_input, psbt_input: foreign_psbt_input,
outpoint, outpoint,
} => { } => {
if params.force_non_witness_utxo if !params.only_witness_utxo && foreign_psbt_input.non_witness_utxo.is_none() {
&& foreign_psbt_input.non_witness_utxo.is_none()
{
return Err(Error::Generic(format!( return Err(Error::Generic(format!(
"Missing non_witness_utxo on foreign utxo {}", "Missing non_witness_utxo on foreign utxo {}",
outpoint outpoint
@ -1336,7 +1331,7 @@ where
&self, &self,
utxo: LocalUtxo, utxo: LocalUtxo,
sighash_type: Option<SigHashType>, sighash_type: Option<SigHashType>,
force_non_witness_utxo: bool, only_witness_utxo: bool,
) -> Result<Input, Error> { ) -> Result<Input, Error> {
// Try to find the prev_script in our db to figure out if this is internal or external, // Try to find the prev_script in our db to figure out if this is internal or external,
// and the derivation index // and the derivation index
@ -1363,7 +1358,7 @@ where
if desc.is_witness() { if desc.is_witness() {
psbt_input.witness_utxo = Some(prev_tx.output[prev_output.vout as usize].clone()); psbt_input.witness_utxo = Some(prev_tx.output[prev_output.vout as usize].clone());
} }
if !desc.is_witness() || force_non_witness_utxo { if !desc.is_witness() || !only_witness_utxo {
psbt_input.non_witness_utxo = Some(prev_tx); psbt_input.non_witness_utxo = Some(prev_tx);
} }
} }
@ -2250,6 +2245,7 @@ mod test {
let mut builder = wallet.build_tx(); let mut builder = wallet.build_tx();
builder builder
.set_single_recipient(addr.script_pubkey()) .set_single_recipient(addr.script_pubkey())
.only_witness_utxo()
.drain_wallet(); .drain_wallet();
let (psbt, _) = builder.finish().unwrap(); let (psbt, _) = builder.finish().unwrap();
@ -2268,20 +2264,18 @@ mod test {
.drain_wallet(); .drain_wallet();
let (psbt, _) = builder.finish().unwrap(); let (psbt, _) = builder.finish().unwrap();
assert!(psbt.inputs[0].non_witness_utxo.is_none());
assert!(psbt.inputs[0].witness_utxo.is_some()); assert!(psbt.inputs[0].witness_utxo.is_some());
} }
#[test] #[test]
fn test_create_tx_both_non_witness_utxo_and_witness_utxo() { fn test_create_tx_both_non_witness_utxo_and_witness_utxo_default() {
let (wallet, _, _) = let (wallet, _, _) =
get_funded_wallet("wsh(pk(cVpPVruEDdmutPzisEsYvtST1usBR3ntr8pXSyt6D2YYqXRyPcFW))"); get_funded_wallet("wsh(pk(cVpPVruEDdmutPzisEsYvtST1usBR3ntr8pXSyt6D2YYqXRyPcFW))");
let addr = wallet.get_address(New).unwrap(); let addr = wallet.get_address(New).unwrap();
let mut builder = wallet.build_tx(); let mut builder = wallet.build_tx();
builder builder
.set_single_recipient(addr.script_pubkey()) .set_single_recipient(addr.script_pubkey())
.drain_wallet() .drain_wallet();
.force_non_witness_utxo();
let (psbt, _) = builder.finish().unwrap(); let (psbt, _) = builder.finish().unwrap();
assert!(psbt.inputs[0].non_witness_utxo.is_some()); assert!(psbt.inputs[0].non_witness_utxo.is_some());
@ -2419,6 +2413,7 @@ mod test {
let (wallet1, _, _) = get_funded_wallet(get_test_wpkh()); let (wallet1, _, _) = get_funded_wallet(get_test_wpkh());
let (wallet2, _, _) = let (wallet2, _, _) =
get_funded_wallet("wpkh(cVbZ8ovhye9AoAHFsqobCf7LxbXDAECy9Kb8TZdfsDYMZGBUyCnm)"); get_funded_wallet("wpkh(cVbZ8ovhye9AoAHFsqobCf7LxbXDAECy9Kb8TZdfsDYMZGBUyCnm)");
let addr = Address::from_str("2N1Ffz3WaNzbeLFBb51xyFMHYSEUXcbiSoX").unwrap(); let addr = Address::from_str("2N1Ffz3WaNzbeLFBb51xyFMHYSEUXcbiSoX").unwrap();
let utxo = wallet2.list_unspent().unwrap().remove(0); let utxo = wallet2.list_unspent().unwrap().remove(0);
let foreign_utxo_satisfaction = wallet2 let foreign_utxo_satisfaction = wallet2
@ -2434,6 +2429,7 @@ mod test {
let mut builder = wallet1.build_tx(); let mut builder = wallet1.build_tx();
builder builder
.add_recipient(addr.script_pubkey(), 60_000) .add_recipient(addr.script_pubkey(), 60_000)
.only_witness_utxo()
.add_foreign_utxo(utxo.outpoint, psbt_input, foreign_utxo_satisfaction) .add_foreign_utxo(utxo.outpoint, psbt_input, foreign_utxo_satisfaction)
.unwrap(); .unwrap();
let (mut psbt, details) = builder.finish().unwrap(); let (mut psbt, details) = builder.finish().unwrap();
@ -2453,14 +2449,30 @@ mod test {
"foreign_utxo should be in there" "foreign_utxo should be in there"
); );
let finished = wallet1.sign(&mut psbt, Default::default()).unwrap(); let finished = wallet1
.sign(
&mut psbt,
SignOptions {
trust_witness_utxo: true,
..Default::default()
},
)
.unwrap();
assert!( assert!(
!finished, !finished,
"only one of the inputs should have been signed so far" "only one of the inputs should have been signed so far"
); );
let finished = wallet2.sign(&mut psbt, Default::default()).unwrap(); let finished = wallet2
.sign(
&mut psbt,
SignOptions {
trust_witness_utxo: true,
..Default::default()
},
)
.unwrap();
assert!(finished, "all the inputs should have been signed now"); assert!(finished, "all the inputs should have been signed now");
} }
@ -2538,7 +2550,7 @@ mod test {
} }
#[test] #[test]
fn test_add_foreign_utxo_force_non_witness_utxo() { fn test_add_foreign_utxo_only_witness_utxo() {
let (wallet1, _, _) = get_funded_wallet(get_test_wpkh()); let (wallet1, _, _) = get_funded_wallet(get_test_wpkh());
let (wallet2, _, txid2) = let (wallet2, _, txid2) =
get_funded_wallet("wpkh(cVbZ8ovhye9AoAHFsqobCf7LxbXDAECy9Kb8TZdfsDYMZGBUyCnm)"); get_funded_wallet("wpkh(cVbZ8ovhye9AoAHFsqobCf7LxbXDAECy9Kb8TZdfsDYMZGBUyCnm)");
@ -2551,9 +2563,7 @@ mod test {
.unwrap(); .unwrap();
let mut builder = wallet1.build_tx(); let mut builder = wallet1.build_tx();
builder builder.add_recipient(addr.script_pubkey(), 60_000);
.add_recipient(addr.script_pubkey(), 60_000)
.force_non_witness_utxo();
{ {
let mut builder = builder.clone(); let mut builder = builder.clone();
@ -2570,6 +2580,22 @@ mod test {
); );
} }
{
let mut builder = builder.clone();
let psbt_input = psbt::Input {
witness_utxo: Some(utxo2.txout.clone()),
..Default::default()
};
builder
.only_witness_utxo()
.add_foreign_utxo(utxo2.outpoint, psbt_input, satisfaction_weight)
.unwrap();
assert!(
builder.finish().is_ok(),
"psbt_input with just witness_utxo should succeed when `only_witness_utxo` is enabled"
);
}
{ {
let mut builder = builder.clone(); let mut builder = builder.clone();
let tx2 = wallet2 let tx2 = wallet2
@ -2589,7 +2615,7 @@ mod test {
.unwrap(); .unwrap();
assert!( assert!(
builder.finish().is_ok(), builder.finish().is_ok(),
"psbt_input with non_witness_utxo should succeed with force_non_witness_utxo" "psbt_input with non_witness_utxo should succeed by default"
); );
} }
} }
@ -3619,7 +3645,15 @@ mod test {
psbt.inputs.push(dud_input); psbt.inputs.push(dud_input);
psbt.global.unsigned_tx.input.push(bitcoin::TxIn::default()); psbt.global.unsigned_tx.input.push(bitcoin::TxIn::default());
let is_final = wallet.sign(&mut psbt, Default::default()).unwrap(); let is_final = wallet
.sign(
&mut psbt,
SignOptions {
trust_witness_utxo: true,
..Default::default()
},
)
.unwrap();
assert!( assert!(
!is_final, !is_final,
"shouldn't be final since we can't sign one of the inputs" "shouldn't be final since we can't sign one of the inputs"

View File

@ -146,7 +146,7 @@ pub(crate) struct TxParams {
pub(crate) rbf: Option<RbfValue>, pub(crate) rbf: Option<RbfValue>,
pub(crate) version: Option<Version>, pub(crate) version: Option<Version>,
pub(crate) change_policy: ChangeSpendPolicy, pub(crate) change_policy: ChangeSpendPolicy,
pub(crate) force_non_witness_utxo: bool, pub(crate) only_witness_utxo: bool,
pub(crate) add_global_xpubs: bool, pub(crate) add_global_xpubs: bool,
pub(crate) include_output_redeem_witness_script: bool, pub(crate) include_output_redeem_witness_script: bool,
pub(crate) bumping_fee: Option<PreviousFee>, pub(crate) bumping_fee: Option<PreviousFee>,
@ -336,10 +336,10 @@ impl<'a, B, D: BatchDatabase, Cs: CoinSelectionAlgorithm<D>, Ctx: TxBuilderConte
/// 1. The `psbt_input` does not contain a `witness_utxo` or `non_witness_utxo`. /// 1. The `psbt_input` does not contain a `witness_utxo` or `non_witness_utxo`.
/// 2. The data in `non_witness_utxo` does not match what is in `outpoint`. /// 2. The data in `non_witness_utxo` does not match what is in `outpoint`.
/// ///
/// Note if you set [`force_non_witness_utxo`] any `psbt_input` you pass to this method must /// Note unless you set [`only_witness_utxo`] any `psbt_input` you pass to this method must
/// have `non_witness_utxo` set otherwise you will get an error when [`finish`] is called. /// have `non_witness_utxo` set otherwise you will get an error when [`finish`] is called.
/// ///
/// [`force_non_witness_utxo`]: Self::force_non_witness_utxo /// [`only_witness_utxo`]: Self::only_witness_utxo
/// [`finish`]: Self::finish /// [`finish`]: Self::finish
/// [`max_satisfaction_weight`]: miniscript::Descriptor::max_satisfaction_weight /// [`max_satisfaction_weight`]: miniscript::Descriptor::max_satisfaction_weight
pub fn add_foreign_utxo( pub fn add_foreign_utxo(
@ -464,12 +464,13 @@ impl<'a, B, D: BatchDatabase, Cs: CoinSelectionAlgorithm<D>, Ctx: TxBuilderConte
self self
} }
/// Fill-in the [`psbt::Input::non_witness_utxo`](bitcoin::util::psbt::Input::non_witness_utxo) field even if the wallet only has SegWit /// Only Fill-in the [`psbt::Input::witness_utxo`](bitcoin::util::psbt::Input::witness_utxo) field when spending from
/// descriptors. /// SegWit descriptors.
/// ///
/// This is useful for signers which always require it, like Trezor hardware wallets. /// This reduces the size of the PSBT, but some signers might reject them due to the lack of
pub fn force_non_witness_utxo(&mut self) -> &mut Self { /// the `non_witness_utxo`.
self.params.force_non_witness_utxo = true; pub fn only_witness_utxo(&mut self) -> &mut Self {
self.params.only_witness_utxo = true;
self self
} }