Skip to content

Commit

Permalink
Merge pull request #8 from dfns/hd-wallets
Browse files Browse the repository at this point in the history 
Support HD derivation and taproot tweak in full signing
  • Loading branch information
@survived
survived authored Aug 21, 2024
2 parents 72c2f79 + 168f51f commit 35e7f63
Show file tree
Hide file tree
Showing 8 changed files with 408 additions and 104 deletions.
9 changes: 9 additions & 0 deletions Cargo.lock
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

46 changes: 34 additions & 12 deletions givre/src/signing/aggregate.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -97,11 +97,12 @@ pub struct AggregateOptions<'a, C: Ciphersuite> {
/// Additive shift derived from HD path
hd_additive_shift: Option<Scalar<C::Curve>>,
/// Possible values:
/// * `None` if script tree is empty
/// * `Some(root)` if script tree is not empty
/// * `None` if it wasn't specified
/// * `Some(None)` if script tree is empty
/// * `Some(Some(root))` if script tree is not empty
///
/// It only matters when `C::IS_TAPROOT` is `true`
taproot_merkle_root: Option<[u8; 32]>,
/// It must be `None` when `C::IS_TAPROOT` is `true`, and it must be `Some(_)` otherwise
taproot_merkle_root: Option<Option<[u8; 32]>>,
}

impl<'a, C: Ciphersuite> AggregateOptions<'a, C> {
Expand Down Expand Up @@ -133,7 +134,7 @@ impl<'a, C: Ciphersuite> AggregateOptions<'a, C> {
/// Returns error if the key doesn't support HD derivation, or if the path is invalid
#[cfg(feature = "hd-wallets")]
pub fn set_derivation_path<Index>(
mut self,
self,
path: impl IntoIterator<Item = Index>,
) -> Result<Self, crate::key_share::HdError<<slip_10::NonHardenedIndex as TryFrom<Index>>::Error>>
where
Expand All @@ -145,10 +146,22 @@ impl<'a, C: Ciphersuite> AggregateOptions<'a, C> {
.key_info
.extended_public_key()
.ok_or(HdError::DisabledHd)?;
self.hd_additive_shift =
Some(utils::derive_additive_shift(public_key, path).map_err(HdError::InvalidPath)?);
let additive_shift =
utils::derive_additive_shift(public_key, path).map_err(HdError::InvalidPath)?;

Ok(self)
Ok(self.dangerous_set_hd_additive_shift(additive_shift))
}

/// Specifies HD derivation additive shift
///
/// CAUTION: additive shift MUST BE derived from the extended public key obtained from
/// the key share which is used for signing by calling [`utils::derive_additive_shift`].
pub(crate) fn dangerous_set_hd_additive_shift(
mut self,
hd_additive_shift: Scalar<C::Curve>,
) -> Self {
self.hd_additive_shift = Some(hd_additive_shift);
self
}

/// Tweaks the key with specified merkle root following [BIP-341]
Expand All @@ -170,7 +183,7 @@ impl<'a, C: Ciphersuite> AggregateOptions<'a, C> {
return Err(Reason::NonTaprootCiphersuite.into());
}

self.taproot_merkle_root = merkle_root;
self.taproot_merkle_root = Some(merkle_root);
Ok(self)
}

Expand Down Expand Up @@ -222,7 +235,7 @@ fn aggregate_inner<C: Ciphersuite>(
hd_additive_shift: Option<Scalar<C::Curve>>,
#[rustfmt::skip]
#[cfg_attr(not(feature = "taproot"), allow(unused_variables))]
taproot_merkle_root: Option<[u8; 32]>,
taproot_merkle_root: Option<Option<[u8; 32]>>,
signers: &[(SignerIndex, PublicCommitments<C::Curve>, SigShare<C::Curve>)],
msg: &[u8],
) -> Result<Signature<C>, AggregateError> {
Expand Down Expand Up @@ -253,7 +266,8 @@ fn aggregate_inner<C: Ciphersuite>(
#[cfg(feature = "taproot")]
let pk = if C::IS_TAPROOT {
// Taproot: tweak the key share
let t = crate::signing::taproot::tweak::<C>(pk, taproot_merkle_root)
let merkle_root = taproot_merkle_root.ok_or(Reason::MissingTaprootMerkleRoot)?;
let t = crate::signing::taproot::tweak::<C>(pk, merkle_root)
.ok_or(Reason::TaprootTweakUndefined)?;
let pk = *pk + Point::generator() * t;
let pk = NonZero::from_point(pk).ok_or(Reason::TaprootChildPkZero)?;
Expand Down Expand Up @@ -315,6 +329,8 @@ enum Reason {
InvalidSig,
HdChildPkZero,
#[cfg(feature = "taproot")]
MissingTaprootMerkleRoot,
#[cfg(feature = "taproot")]
NonTaprootCiphersuite,
#[cfg(feature = "taproot")]
TaprootTweakUndefined,
Expand All @@ -338,6 +354,11 @@ impl fmt::Display for AggregateError {
Reason::InvalidSig => f.write_str("invalid signature"),
Reason::HdChildPkZero => f.write_str("HD derivation error: child pk is zero"),
#[cfg(feature = "taproot")]
Reason::MissingTaprootMerkleRoot => f.write_str(
"taproot merkle tree is missing: it must be specified \
for taproot ciphersuite via `SigningOptions::set_taproot_tweak`",
),
#[cfg(feature = "taproot")]
Reason::NonTaprootCiphersuite => {
f.write_str("ciphersuite doesn't support taproot tweaks")
}
Expand All @@ -358,7 +379,8 @@ impl std::error::Error for AggregateError {
| Reason::InvalidSig
| Reason::HdChildPkZero => None,
#[cfg(feature = "taproot")]
Reason::NonTaprootCiphersuite
Reason::MissingTaprootMerkleRoot
| Reason::NonTaprootCiphersuite
| Reason::TaprootTweakUndefined
| Reason::TaprootChildPkZero => None,
}
Expand Down
124 changes: 116 additions & 8 deletions givre/src/signing/full_signing.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use crate::{
Ciphersuite, SignerIndex,
};

use super::{aggregate::Signature, round1::PublicCommitments, round2::SigShare};
use super::{aggregate::Signature, round1::PublicCommitments, round2::SigShare, utils};

/// Message of FROST Signing Protocol
#[derive(Debug, Clone, Copy, round_based::ProtocolMessage)]
Expand All @@ -37,6 +37,9 @@ pub struct SigningBuilder<'a, C: Ciphersuite> {
key_share: &'a KeyShare<C::Curve>,
signers: &'a [SignerIndex],
msg: &'a [u8],

hd_additive_shift: Option<generic_ec::Scalar<C::Curve>>,
taproot_merkle_root: Option<Option<[u8; 32]>>,
}

impl<'a, C: Ciphersuite> SigningBuilder<'a, C> {
Expand All @@ -54,13 +57,63 @@ impl<'a, C: Ciphersuite> SigningBuilder<'a, C> {
key_share,
signers,
msg,
hd_additive_shift: None,
taproot_merkle_root: None,
}
}

/// Specifies HD derivation path
///
/// If called twice, the second call overwrites the first.
///
/// Returns error if the key doesn't support HD derivation, or if the path is invalid
#[cfg(feature = "hd-wallets")]
pub fn set_derivation_path<Index>(
mut self,
path: impl IntoIterator<Item = Index>,
) -> Result<Self, crate::key_share::HdError<<slip_10::NonHardenedIndex as TryFrom<Index>>::Error>>
where
slip_10::NonHardenedIndex: TryFrom<Index>,
{
use crate::key_share::HdError;

let public_key = self
.key_share
.extended_public_key()
.ok_or(HdError::DisabledHd)?;
let additive_shift =
utils::derive_additive_shift(public_key, path).map_err(HdError::InvalidPath)?;
self.hd_additive_shift = Some(additive_shift);
Ok(self)
}

/// Tweaks the key with specified merkle root following [BIP-341]
///
/// Note that the taproot spec requires that any key must be tweaked. By default, if this
/// method is not called for taproot-enabled ciphersuite, then an empty merkle root
/// is assumed.
///
/// The method returns an error if the ciphersuite doesn't support taproot, i.e. if
/// [`Ciphersuite::IS_TAPROOT`] is `false`
///
/// [BIP-341]: https://github.com/bitcoin/bips/blob/master/bip-0341.mediawiki
#[cfg(feature = "taproot")]
pub fn set_taproot_tweak(
mut self,
merkle_root: Option<[u8; 32]>,
) -> Result<Self, FullSigningError> {
if !C::IS_TAPROOT {
return Err(Reason::NonTaprootCiphersuite.into());
}

self.taproot_merkle_root = Some(merkle_root);
Ok(self)
}

/// Issues signature share
///
/// Signer will output a signature share. It'll be more efficient than [generating a full signature](Self::sign),
/// but it requires you to collect all sig shares in one place and [aggreate](crate::signing::aggregate::aggregate)
/// but it requires you to collect all sig shares in one place and [aggregate](crate::signing::aggregate::aggregate)
/// them.
pub async fn issue_sig_share<M, R>(
self,
Expand All @@ -78,6 +131,8 @@ impl<'a, C: Ciphersuite> SigningBuilder<'a, C> {
self.key_share,
self.signers,
self.msg,
self.hd_additive_shift,
self.taproot_merkle_root,
true,
)
.await?
Expand All @@ -100,6 +155,8 @@ impl<'a, C: Ciphersuite> SigningBuilder<'a, C> {
self.key_share,
self.signers,
self.msg,
self.hd_additive_shift,
self.taproot_merkle_root,
false,
)
.await?
Expand All @@ -117,6 +174,8 @@ async fn signing<C, M>(
key_share: &KeyShare<C::Curve>,
signers: &[SignerIndex],
msg: &[u8],
hd_additive_shift: Option<generic_ec::Scalar<C::Curve>>,
taproot_merkle_root: Option<Option<[u8; 32]>>,
output_sig_share: bool,
) -> Result<SigningOutput<C>, FullSigningError>
where
Expand Down Expand Up @@ -159,8 +218,25 @@ where
.map(|(&j, &comm)| (j, comm))
.collect::<Vec<_>>();

let sig_share = crate::signing::round2::sign::<C>(key_share, nonces, msg, &signers_list)
.map_err(Reason::Sign)?;
let mut options =
crate::signing::round2::SigningOptions::<C>::new(key_share, nonces, msg, &signers_list);
#[cfg(feature = "hd-wallets")]
if let Some(additive_shift) = hd_additive_shift {
options = options.dangerous_set_hd_additive_shift(additive_shift);
}
if cfg!(not(feature = "hd-wallets")) && hd_additive_shift.is_some() {
return Err(Bug::AdditiveShiftWithoutHdFeature.into());
}
#[cfg(feature = "taproot")]
if let Some(root) = taproot_merkle_root {
options = options
.set_taproot_tweak(root)
.map_err(Bug::SetTaprootTweakSign)?;
}
if cfg!(not(feature = "taproot")) && taproot_merkle_root.is_some() {
return Err(Bug::TaprootSpecifiedButDisabled.into());
}
let sig_share = options.sign().map_err(Reason::Sign)?;

if output_sig_share {
return Ok(SigningOutput::SigShare(sig_share));
Expand All @@ -171,7 +247,7 @@ where
.await
.map_err(IoError::send)?;

// Aggregate sigature
// Aggregate signature
let sig_shares = rounds.complete(round2).await.map_err(IoError::recv)?;

let signers_list = signers_list
Expand All @@ -181,8 +257,19 @@ where
.collect::<Vec<_>>();

let key_info: &KeyInfo<_> = key_share.as_ref();
let sig = crate::signing::aggregate::aggregate::<C>(key_info, &signers_list, msg)
.map_err(Reason::Aggregate)?;
let mut options =
crate::signing::aggregate::AggregateOptions::new(key_info, &signers_list, msg);
#[cfg(feature = "hd-wallets")]
if let Some(additive_shift) = hd_additive_shift {
options = options.dangerous_set_hd_additive_shift(additive_shift);
}
#[cfg(feature = "taproot")]
if let Some(root) = taproot_merkle_root {
options = options
.set_taproot_tweak(root)
.map_err(Bug::SetTaprootTweakAggregate)?;
}
let sig = options.aggregate().map_err(Reason::Aggregate)?;

Ok(SigningOutput::Signature(sig))
}
Expand All @@ -198,6 +285,7 @@ pub struct FullSigningError(Reason);

#[derive(Debug)]
enum Reason {
NonTaprootCiphersuite,
NOverflowsU16,
INotInRange,
UnexpectedNumberOfSigners,
Expand Down Expand Up @@ -225,11 +313,16 @@ impl IoError {
#[derive(Debug)]
enum Bug {
UnexpectedOutput,
AdditiveShiftWithoutHdFeature,
SetTaprootTweakSign(crate::signing::round2::SigningError),
SetTaprootTweakAggregate(crate::signing::aggregate::AggregateError),
TaprootSpecifiedButDisabled,
}

impl fmt::Display for FullSigningError {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match &self.0 {
Reason::NonTaprootCiphersuite => f.write_str("ciphersuite doesn't support taproot"),
Reason::NOverflowsU16 => f.write_str("number of signers overflows u16"),
Reason::INotInRange => f.write_str("signer index not in range (it must be 0 <= i < n)"),
Reason::UnexpectedNumberOfSigners => f.write_str(
Expand All @@ -250,6 +343,14 @@ impl fmt::Display for Bug {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self {
Bug::UnexpectedOutput => f.write_str("unexpected output"),
Bug::AdditiveShiftWithoutHdFeature => {
f.write_str("additive shift is specified, but hd wallets are disabled")
}
Bug::SetTaprootTweakSign(_) => f.write_str("set taproot tweak failed (sign)"),
Bug::SetTaprootTweakAggregate(_) => f.write_str("set taproot tweak failed (aggregate)"),
Bug::TaprootSpecifiedButDisabled => {
f.write_str("taproot merkle root is specified, but taproot feature is not enabled")
}
}
}
}
Expand All @@ -258,7 +359,10 @@ impl fmt::Display for Bug {
impl std::error::Error for FullSigningError {
fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
match &self.0 {
Reason::NOverflowsU16 | Reason::INotInRange | Reason::UnexpectedNumberOfSigners => None,
Reason::NonTaprootCiphersuite
| Reason::NOverflowsU16
| Reason::INotInRange
| Reason::UnexpectedNumberOfSigners => None,
Reason::IoError(IoError::Send(err)) | Reason::IoError(IoError::Recv(err)) => {
Some(&**err)
}
Expand All @@ -274,6 +378,10 @@ impl std::error::Error for Bug {
fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
match self {
Bug::UnexpectedOutput => None,
Bug::AdditiveShiftWithoutHdFeature => None,
Bug::SetTaprootTweakSign(err) => Some(err),
Bug::SetTaprootTweakAggregate(err) => Some(err),
Bug::TaprootSpecifiedButDisabled => None,
}
}
}
Expand Down
Loading

0 comments on commit 35e7f63

Please sign in to comment.