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 sign_options argument to Wallet::sign and Wallet::finalize_psbt methods #650

Merged
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
53 changes: 51 additions & 2 deletions bdk-ffi/src/bdk.udl
Original file line number Diff line number Diff line change
Expand Up @@ -454,6 +454,55 @@ interface FullScanScriptInspector {
/// A changeset for [`Wallet`].
interface ChangeSet {};

/// Options for a software signer.
///
/// Adjust the behavior of our software signers and the way a transaction is finalized.
dictionary SignOptions {
/// Whether the signer should trust the `witness_utxo`, if the `non_witness_utxo` hasn't been
/// provided
///
/// Defaults to `false` to mitigate the "SegWit bug" which could trick the wallet into
/// paying a fee larger than expected.
///
/// Some wallets, especially if relatively old, might not provide the `non_witness_utxo` for
/// SegWit transactions in the PSBT they generate: in those cases setting this to `true`
/// should correctly produce a signature, at the expense of an increased trust in the creator
/// of the PSBT.
///
/// For more details see: <https://blog.trezor.io/details-of-firmware-updates-for-trezor-one-version-1-9-1-and-trezor-model-t-version-2-3-1-1eba8f60f2dd>
boolean trust_witness_utxo;

/// Whether the wallet should assume a specific height has been reached when trying to finalize
/// a transaction
///
/// The wallet will only "use" a timelock to satisfy the spending policy of an input if the
/// timelock height has already been reached. This option allows overriding the "current height" to let the
/// wallet use timelocks in the future to spend a coin.
u32? assume_height;

/// Whether the signer should use the `sighash_type` set in the PSBT when signing, no matter
/// what its value is
///
/// Defaults to `false` which will only allow signing using `SIGHASH_ALL`.
boolean allow_all_sighashes;

/// Whether to try finalizing the PSBT after the inputs are signed.
///
/// Defaults to `true` which will try finalizing PSBT after inputs are signed.
boolean try_finalize;

/// Whether we should try to sign a taproot transaction with the taproot internal key
/// or not. This option is ignored if we're signing a non-taproot PSBT.
///
/// Defaults to `true`, i.e., we always try to sign with the taproot internal key.
boolean sign_with_tap_internal_key;

/// Whether we should grind ECDSA signature to ensure signing with low r
/// or not.
/// Defaults to `true`, i.e., we always grind ECDSA signature to sign with low r.
boolean allow_grinding;
};

// ------------------------------------------------------------------------
// bdk_wallet crate - wallet module
// ------------------------------------------------------------------------
Expand Down Expand Up @@ -583,7 +632,7 @@ interface Wallet {
/// signers will follow the options, but the "software signers" (WIF keys and `xprv`) defined
/// in this library will.
[Throws=SignerError]
boolean sign(Psbt psbt);
boolean sign(Psbt psbt, optional SignOptions? sign_options = null);

/// Finalize a PSBT, i.e., for each input determine if sufficient data is available to pass
/// validation and construct the respective `scriptSig` or `scriptWitness`. Please refer to
Expand All @@ -595,7 +644,7 @@ interface Wallet {
///
/// The [`SignOptions`] can be used to tweak the behavior of the finalizer.
[Throws=SignerError]
boolean finalize_psbt(Psbt psbt);
boolean finalize_psbt(Psbt psbt, optional SignOptions? sign_options = null);

/// Compute the `tx`'s sent and received [`Amount`]s.
///
Expand Down
1 change: 1 addition & 0 deletions bdk-ffi/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ use crate::types::Satisfaction;
use crate::types::SatisfiableItem;
use crate::types::ScriptAmount;
use crate::types::SentAndReceivedValues;
use crate::types::SignOptions;
use crate::types::SyncRequest;
use crate::types::SyncRequestBuilder;
use crate::types::SyncScriptInspector;
Expand Down
29 changes: 28 additions & 1 deletion bdk-ffi/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,11 @@ use bdk_wallet::chain::tx_graph::CanonicalTx as BdkCanonicalTx;
use bdk_wallet::chain::{
ChainPosition as BdkChainPosition, ConfirmationBlockTime as BdkConfirmationBlockTime,
};

use bdk_wallet::descriptor::policy::{
Condition as BdkCondition, PkOrF as BdkPkOrF, Policy as BdkPolicy,
Satisfaction as BdkSatisfaction, SatisfiableItem as BdkSatisfiableItem,
};
use bdk_wallet::signer::{SignOptions as BdkSignOptions, TapLeavesOptions};
use bdk_wallet::AddressInfo as BdkAddressInfo;
use bdk_wallet::Balance as BdkBalance;
use bdk_wallet::KeychainKind;
Expand Down Expand Up @@ -519,3 +519,30 @@ impl From<BdkCondition> for Condition {
}
}
}

// This is a wrapper type around the bdk type [SignOptions](https://docs.rs/bdk_wallet/1.0.0/bdk_wallet/signer/struct.SignOptions.html)
// because we did not want to expose the complexity behind the `TapLeavesOptions` type. When
// transforming from a SignOption to a BdkSignOptions, we simply use the default values for
// TapLeavesOptions.
pub struct SignOptions {
pub trust_witness_utxo: bool,
pub assume_height: Option<u32>,
pub allow_all_sighashes: bool,
pub try_finalize: bool,
pub sign_with_tap_internal_key: bool,
pub allow_grinding: bool,
}

impl From<SignOptions> for BdkSignOptions {
fn from(options: SignOptions) -> BdkSignOptions {
BdkSignOptions {
trust_witness_utxo: options.trust_witness_utxo,
assume_height: options.assume_height,
allow_all_sighashes: options.allow_all_sighashes,
try_finalize: options.try_finalize,
tap_leaves_options: TapLeavesOptions::default(),
sign_with_tap_internal_key: options.sign_with_tap_internal_key,
allow_grinding: options.allow_grinding,
}
}
}
27 changes: 21 additions & 6 deletions bdk-ffi/src/wallet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,15 @@ use crate::error::{
use crate::store::Connection;
use crate::types::{
AddressInfo, Balance, CanonicalTx, FullScanRequestBuilder, KeychainAndIndex, LocalOutput,
Policy, SentAndReceivedValues, SyncRequestBuilder, Update,
Policy, SentAndReceivedValues, SignOptions, SyncRequestBuilder, Update,
};

use bitcoin_ffi::{Amount, FeeRate, OutPoint, Script};

use bdk_wallet::bitcoin::{Network, Txid};
use bdk_wallet::rusqlite::Connection as BdkConnection;
use bdk_wallet::{KeychainKind, PersistedWallet, SignOptions, Wallet as BdkWallet};
use bdk_wallet::signer::SignOptions as BdkSignOptions;
use bdk_wallet::{KeychainKind, PersistedWallet, Wallet as BdkWallet};

use std::borrow::BorrowMut;
use std::str::FromStr;
Expand Down Expand Up @@ -162,18 +163,32 @@ impl Wallet {
pub(crate) fn sign(
&self,
psbt: Arc<Psbt>,
// sign_options: Option<SignOptions>,
sign_options: Option<SignOptions>,
) -> Result<bool, SignerError> {
let mut psbt = psbt.0.lock().unwrap();
let bdk_sign_options: BdkSignOptions = match sign_options {
Some(sign_options) => BdkSignOptions::from(sign_options),
None => BdkSignOptions::default(),
};

self.get_wallet()
.sign(&mut psbt, SignOptions::default())
.sign(&mut psbt, bdk_sign_options)
.map_err(SignerError::from)
}

pub fn finalize_psbt(&self, psbt: Arc<Psbt>) -> Result<bool, SignerError> {
pub fn finalize_psbt(
&self,
psbt: Arc<Psbt>,
sign_options: Option<SignOptions>,
) -> Result<bool, SignerError> {
let mut psbt = psbt.0.lock().unwrap();
let bdk_sign_options: BdkSignOptions = match sign_options {
Some(sign_options) => BdkSignOptions::from(sign_options),
None => BdkSignOptions::default(),
};

self.get_wallet()
.finalize_psbt(&mut psbt, SignOptions::default())
.finalize_psbt(&mut psbt, bdk_sign_options)
.map_err(SignerError::from)
}

Expand Down
Loading