Skip to content

Commit 1c76084

Browse files
Merge #779: Add signature grinding for ECDSA signatures
68dd6d2 Add signature grinding for ECDSA signatures (Vladimir Fomene) Pull request description: ### Description This PR adds a new field called `allow_grinding` in the Signer's `SignOptions` struct that is used to determine whether or not to grind an ECDSA signature during the signing process. ### Changelog notice Breaking change: the BDK Signer now produces low-R signatures by default, saving one byte. If you want to preserve the original behavior, set `allow_grinding` in the `SignOptions` to `false`. ### Notes to the reviewers This PR resolves issue #695 #### All Submissions: * [x] I've signed all my commits * [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md) * [x] I ran `cargo fmt` and `cargo clippy` before committing #### New Features: * [ ] I've added tests for the new feature * [x] I've added docs for the new feature #### Bugfixes: * [ ] This pull request breaks the existing API * [ ] I've added tests to reproduce the issue which are now passing * [x] I'm linking the issue being fixed by this PR ACKs for top commit: danielabrozzoni: ACK 68dd6d2 rajarshimaitra: ACK 68dd6d2 Tree-SHA512: 6472338c611b4b32986cf66fcd313ef84f17f5b0ae9e7991ea7da47142641ab812f8b325d4d18314e1a58abe462683101160e62e2363a048fdab3f18aee4d699
2 parents b627455 + 68dd6d2 commit 1c76084

File tree

2 files changed

+48
-1
lines changed

2 files changed

+48
-1
lines changed

src/wallet/mod.rs

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5524,6 +5524,7 @@ pub(crate) mod test {
55245524
SignOptions {
55255525
remove_partial_sigs: false,
55265526
try_finalize: false,
5527+
allow_grinding: false,
55275528
..Default::default()
55285529
},
55295530
)
@@ -5538,6 +5539,7 @@ pub(crate) mod test {
55385539
&mut psbt,
55395540
SignOptions {
55405541
remove_partial_sigs: false,
5542+
allow_grinding: false,
55415543
..Default::default()
55425544
},
55435545
)
@@ -5546,6 +5548,39 @@ pub(crate) mod test {
55465548
assert_fee_rate!(psbt, details.fee.unwrap_or(0), fee_rate);
55475549
}
55485550

5551+
#[test]
5552+
fn test_fee_rate_sign_grinding_low_r() {
5553+
// Our goal is to obtain a transaction with a signature with low-R (70 bytes)
5554+
// by setting the `allow_grinding` signing option as true.
5555+
// We then check that our fee rate and fee calculation is alright and that our
5556+
// signature is 70 bytes.
5557+
let (wallet, _, _) = get_funded_wallet("wpkh(tprv8ZgxMBicQKsPd3EupYiPRhaMooHKUHJxNsTfYuScep13go8QFfHdtkG9nRkFGb7busX4isf6X9dURGCoKgitaApQ6MupRhZMcELAxTBRJgS/*)");
5558+
let addr = wallet.get_address(New).unwrap();
5559+
let fee_rate = FeeRate::from_sat_per_vb(1.0);
5560+
let mut builder = wallet.build_tx();
5561+
builder
5562+
.drain_to(addr.script_pubkey())
5563+
.drain_wallet()
5564+
.fee_rate(fee_rate);
5565+
let (mut psbt, details) = builder.finish().unwrap();
5566+
5567+
wallet
5568+
.sign(
5569+
&mut psbt,
5570+
SignOptions {
5571+
remove_partial_sigs: false,
5572+
allow_grinding: true,
5573+
..Default::default()
5574+
},
5575+
)
5576+
.unwrap();
5577+
5578+
let key = psbt.inputs[0].partial_sigs.keys().next().unwrap();
5579+
let sig_len = psbt.inputs[0].partial_sigs[key].sig.serialize_der().len();
5580+
assert_eq!(sig_len, 70);
5581+
assert_fee_rate!(psbt, details.fee.unwrap_or(0), fee_rate);
5582+
}
5583+
55495584
#[cfg(feature = "test-hardware-signer")]
55505585
#[test]
55515586
fn test_create_signer() {

src/wallet/signer.rs

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -472,6 +472,7 @@ impl InputSigner for SignerWrapper<PrivateKey> {
472472
hash,
473473
hash_ty,
474474
secp,
475+
sign_options.allow_grinding,
475476
);
476477

477478
Ok(())
@@ -485,9 +486,14 @@ fn sign_psbt_ecdsa(
485486
hash: bitcoin::Sighash,
486487
hash_ty: EcdsaSighashType,
487488
secp: &SecpCtx,
489+
allow_grinding: bool,
488490
) {
489491
let msg = &Message::from_slice(&hash.into_inner()[..]).unwrap();
490-
let sig = secp.sign_ecdsa(msg, secret_key);
492+
let sig = if allow_grinding {
493+
secp.sign_ecdsa_low_r(msg, secret_key)
494+
} else {
495+
secp.sign_ecdsa(msg, secret_key)
496+
};
491497
secp.verify_ecdsa(msg, &sig, &pubkey.inner)
492498
.expect("invalid or corrupted ecdsa signature");
493499

@@ -718,6 +724,11 @@ pub struct SignOptions {
718724
///
719725
/// Defaults to `true`, i.e., we always try to sign with the taproot internal key.
720726
pub sign_with_tap_internal_key: bool,
727+
728+
/// Whether we should grind ECDSA signature to ensure signing with low r
729+
/// or not.
730+
/// Defaults to `true`, i.e., we always grind ECDSA signature to sign with low r.
731+
pub allow_grinding: bool,
721732
}
722733

723734
/// Customize which taproot script-path leaves the signer should sign.
@@ -751,6 +762,7 @@ impl Default for SignOptions {
751762
try_finalize: true,
752763
tap_leaves_options: TapLeavesOptions::default(),
753764
sign_with_tap_internal_key: true,
765+
allow_grinding: true,
754766
}
755767
}
756768
}

0 commit comments

Comments
 (0)