Skip to content
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.

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

Add remove_partial_sigs and try_finalize to SignOptions #621

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Signing Taproot PSBTs (key spend and script spend)
- Support for `tr()` descriptors in the `descriptor!()` macro
- Add support for Bitcoin Core 23.0 when using the `rpc` blockchain
- Add `remove_partial_sigs` and `try_finalize` to `SignOptions`

## [v0.18.0] - [v0.17.0]

Expand Down
73 changes: 72 additions & 1 deletion src/wallet/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1050,7 +1050,11 @@ where
}

// attempt to finalize
self.finalize_psbt(psbt, sign_options)
if sign_options.try_finalize {
self.finalize_psbt(psbt, sign_options)
} else {
Ok(false)
}
}

/// Return the spending policies for the wallet's descriptor
Expand Down Expand Up @@ -1161,6 +1165,9 @@ where
let psbt_input = &mut psbt.inputs[n];
psbt_input.final_script_sig = Some(tmp_input.script_sig);
psbt_input.final_script_witness = Some(tmp_input.witness);
if sign_options.remove_partial_sigs {
psbt_input.partial_sigs.clear();
}
}
Err(e) => {
debug!("satisfy error {:?} for input {}", e, n);
Expand Down Expand Up @@ -3878,6 +3885,70 @@ pub(crate) mod test {
)
}

#[test]
fn test_remove_partial_sigs_after_finalize_sign_option() {
let (wallet, _, _) = get_funded_wallet("wpkh(tprv8ZgxMBicQKsPd3EupYiPRhaMooHKUHJxNsTfYuScep13go8QFfHdtkG9nRkFGb7busX4isf6X9dURGCoKgitaApQ6MupRhZMcELAxTBRJgS/*)");

for remove_partial_sigs in &[true, false] {
let addr = wallet.get_address(New).unwrap();
let mut builder = wallet.build_tx();
builder.drain_to(addr.script_pubkey()).drain_wallet();
let mut psbt = builder.finish().unwrap().0;

assert!(wallet
.sign(
&mut psbt,
SignOptions {
remove_partial_sigs: *remove_partial_sigs,
..Default::default()
},
)
.unwrap());

psbt.inputs.iter().for_each(|input| {
if *remove_partial_sigs {
assert!(input.partial_sigs.is_empty())
} else {
assert!(!input.partial_sigs.is_empty())
}
});
}
}

#[test]
fn test_try_finalize_sign_option() {
let (wallet, _, _) = get_funded_wallet("wpkh(tprv8ZgxMBicQKsPd3EupYiPRhaMooHKUHJxNsTfYuScep13go8QFfHdtkG9nRkFGb7busX4isf6X9dURGCoKgitaApQ6MupRhZMcELAxTBRJgS/*)");

for try_finalize in &[true, false] {
let addr = wallet.get_address(New).unwrap();
let mut builder = wallet.build_tx();
builder.drain_to(addr.script_pubkey()).drain_wallet();
let mut psbt = builder.finish().unwrap().0;

let finalized = wallet
.sign(
&mut psbt,
SignOptions {
try_finalize: *try_finalize,
..Default::default()
},
)
.unwrap();

psbt.inputs.iter().for_each(|input| {
if *try_finalize {
assert!(finalized);
assert!(input.final_script_sig.is_some());
assert!(input.final_script_witness.is_some());
} else {
assert!(!finalized);
assert!(input.final_script_sig.is_none());
assert!(input.final_script_witness.is_none());
}
});
}
}

#[test]
fn test_sign_nonstandard_sighash() {
let sighash = EcdsaSighashType::NonePlusAnyoneCanPay;
Expand Down
10 changes: 10 additions & 0 deletions src/wallet/signer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -667,6 +667,14 @@ pub struct SignOptions {
///
/// Defaults to `false` which will only allow signing using `SIGHASH_ALL`.
pub allow_all_sighashes: bool,
/// Whether to remove partial_sigs from psbt inputs while finalizing psbt.
///
/// Defaults to `true` which will remove partial_sigs after finalizing.
pub remove_partial_sigs: bool,
/// Whether to try finalizing psbt input after the inputs are signed.
///
/// Defaults to `true` which will try fianlizing psbt after inputs are signed.
pub try_finalize: bool,
}

#[allow(clippy::derivable_impls)]
Expand All @@ -676,6 +684,8 @@ impl Default for SignOptions {
trust_witness_utxo: false,
assume_height: None,
allow_all_sighashes: false,
remove_partial_sigs: true,
try_finalize: true,
}
}
}
Expand Down