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

Expose more granular data in TaggedHash struct #2687

Merged
merged 2 commits into from
Nov 7, 2023
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
14 changes: 8 additions & 6 deletions lightning/src/offers/invoice.rs
Original file line number Diff line number Diff line change
Expand Up @@ -439,6 +439,7 @@ impl UnsignedBolt12Invoice {
bytes: self.bytes,
contents: self.contents,
signature,
tagged_hash: self.tagged_hash,
})
}
}
Expand All @@ -463,6 +464,7 @@ pub struct Bolt12Invoice {
bytes: Vec<u8>,
contents: InvoiceContents,
signature: Signature,
tagged_hash: TaggedHash,
}

/// The contents of an [`Bolt12Invoice`] for responding to either an [`Offer`] or a [`Refund`].
Expand Down Expand Up @@ -707,7 +709,7 @@ impl Bolt12Invoice {

/// Hash that was used for signing the invoice.
pub fn signable_hash(&self) -> [u8; 32] {
merkle::message_digest(SIGNATURE_TAG, &self.bytes).as_ref().clone()
self.tagged_hash.as_digest().as_ref().clone()
}

/// Verifies that the invoice was for a request or refund created using the given key. Returns
Expand Down Expand Up @@ -1212,11 +1214,11 @@ impl TryFrom<ParsedMessage<FullInvoiceTlvStream>> for Bolt12Invoice {
None => return Err(Bolt12ParseError::InvalidSemantics(Bolt12SemanticError::MissingSignature)),
Some(signature) => signature,
};
let message = TaggedHash::new(SIGNATURE_TAG, &bytes);
let tagged_hash = TaggedHash::new(SIGNATURE_TAG, &bytes);
let pubkey = contents.fields().signing_pubkey;
merkle::verify_signature(&signature, message, pubkey)?;
merkle::verify_signature(&signature, &tagged_hash, pubkey)?;

Ok(Bolt12Invoice { bytes, contents, signature })
Ok(Bolt12Invoice { bytes, contents, signature, tagged_hash })
}
}

Expand Down Expand Up @@ -1431,7 +1433,7 @@ mod tests {
assert_eq!(invoice.signing_pubkey(), recipient_pubkey());

let message = TaggedHash::new(SIGNATURE_TAG, &invoice.bytes);
assert!(merkle::verify_signature(&invoice.signature, message, recipient_pubkey()).is_ok());
assert!(merkle::verify_signature(&invoice.signature, &message, recipient_pubkey()).is_ok());

let digest = Message::from_slice(&invoice.signable_hash()).unwrap();
let pubkey = recipient_pubkey().into();
Expand Down Expand Up @@ -1528,7 +1530,7 @@ mod tests {
assert_eq!(invoice.signing_pubkey(), recipient_pubkey());

let message = TaggedHash::new(SIGNATURE_TAG, &invoice.bytes);
assert!(merkle::verify_signature(&invoice.signature, message, recipient_pubkey()).is_ok());
assert!(merkle::verify_signature(&invoice.signature, &message, recipient_pubkey()).is_ok());

assert_eq!(
invoice.as_tlv_stream(),
Expand Down
4 changes: 2 additions & 2 deletions lightning/src/offers/invoice_request.rs
Original file line number Diff line number Diff line change
Expand Up @@ -876,7 +876,7 @@ impl TryFrom<Vec<u8>> for InvoiceRequest {
Some(signature) => signature,
};
let message = TaggedHash::new(SIGNATURE_TAG, &bytes);
merkle::verify_signature(&signature, message, contents.payer_id)?;
merkle::verify_signature(&signature, &message, contents.payer_id)?;

Ok(InvoiceRequest { bytes, contents, signature })
}
Expand Down Expand Up @@ -1013,7 +1013,7 @@ mod tests {
assert_eq!(invoice_request.payer_note(), None);

let message = TaggedHash::new(SIGNATURE_TAG, &invoice_request.bytes);
assert!(merkle::verify_signature(&invoice_request.signature, message, payer_pubkey()).is_ok());
assert!(merkle::verify_signature(&invoice_request.signature, &message, payer_pubkey()).is_ok());

assert_eq!(
invoice_request.as_tlv_stream(),
Expand Down
59 changes: 47 additions & 12 deletions lightning/src/offers/merkle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,20 +30,41 @@ tlv_stream!(SignatureTlvStream, SignatureTlvStreamRef, SIGNATURE_TYPES, {
///
/// [BIP 340]: https://github.com/bitcoin/bips/blob/master/bip-0340.mediawiki
/// [BOLT 12]: https://github.com/rustyrussell/lightning-rfc/blob/guilt/offers/12-offer-encoding.md#signature-calculation
#[derive(Debug, PartialEq)]
pub struct TaggedHash(Message);
#[derive(Clone, Debug, PartialEq)]
pub struct TaggedHash {
tag: String,
merkle_root: sha256::Hash,
digest: Message,
}

impl TaggedHash {
/// Creates a tagged hash with the given parameters.
///
/// Panics if `tlv_stream` is not a well-formed TLV stream containing at least one TLV record.
pub(super) fn new(tag: &str, tlv_stream: &[u8]) -> Self {
Self(message_digest(tag, tlv_stream))
let tag_hash = sha256::Hash::hash(tag.as_bytes());
let merkle_root = root_hash(tlv_stream);
let digest = Message::from_slice(&tagged_hash(tag_hash, merkle_root)).unwrap();
Self {
tag: tag.to_owned(),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bleh. This allocation feels incredibly unnecessary given there's only two possible values here and they're both constants. We could instead trivially make tag an &'static str and avoid it entirely. Probably a reasonable followup.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

merkle_root,
digest,
}
}

/// Returns the digest to sign.
pub fn as_digest(&self) -> &Message {
&self.0
&self.digest
}

/// Returns the tag used in the tagged hash.
pub fn tag(&self) -> &str {
&self.tag
}

/// Returns the merkle root used in the tagged hash.
pub fn merkle_root(&self) -> sha256::Hash {
self.merkle_root
}
}

Expand Down Expand Up @@ -91,20 +112,14 @@ where
/// Verifies the signature with a pubkey over the given message using a tagged hash as the message
/// digest.
pub(super) fn verify_signature(
signature: &Signature, message: TaggedHash, pubkey: PublicKey,
signature: &Signature, message: &TaggedHash, pubkey: PublicKey,
) -> Result<(), secp256k1::Error> {
let digest = message.as_digest();
let pubkey = pubkey.into();
let secp_ctx = Secp256k1::verification_only();
secp_ctx.verify_schnorr(signature, digest, &pubkey)
}

pub(super) fn message_digest(tag: &str, bytes: &[u8]) -> Message {
let tag = sha256::Hash::hash(tag.as_bytes());
let merkle_root = root_hash(bytes);
Message::from_slice(&tagged_hash(tag, merkle_root)).unwrap()
}

/// Computes a merkle root hash for the given data, which must be a well-formed TLV stream
/// containing at least one TLV record.
fn root_hash(data: &[u8]) -> sha256::Hash {
Expand Down Expand Up @@ -258,12 +273,13 @@ mod tests {
use super::{SIGNATURE_TYPES, TlvStream, WithoutSignatures};

use bitcoin::hashes::{Hash, sha256};
use bitcoin::secp256k1::{KeyPair, Secp256k1, SecretKey};
use bitcoin::secp256k1::{KeyPair, Message, Secp256k1, SecretKey};
use bitcoin::secp256k1::schnorr::Signature;
use core::convert::Infallible;
use crate::offers::offer::{Amount, OfferBuilder};
use crate::offers::invoice_request::InvoiceRequest;
use crate::offers::parse::Bech32Encode;
use crate::offers::test_utils::{payer_pubkey, recipient_pubkey};
use crate::util::ser::Writeable;

#[test]
Expand Down Expand Up @@ -322,6 +338,25 @@ mod tests {
);
}

#[test]
fn compute_tagged_hash() {
let unsigned_invoice_request = OfferBuilder::new("foo".into(), recipient_pubkey())
.amount_msats(1000)
.build().unwrap()
.request_invoice(vec![1; 32], payer_pubkey()).unwrap()
.payer_note("bar".into())
.build().unwrap();

// Simply test that we can grab the tag and merkle root exposed by the accessor
// functions, then use them to succesfully compute a tagged hash.
let tagged_hash = unsigned_invoice_request.as_ref();
let expected_digest = unsigned_invoice_request.as_ref().as_digest();
let tag = sha256::Hash::hash(tagged_hash.tag().as_bytes());
let actual_digest = Message::from_slice(&super::tagged_hash(tag, tagged_hash.merkle_root()))
.unwrap();
assert_eq!(*expected_digest, actual_digest);
}

#[test]
fn skips_encoding_signature_tlv_records() {
let secp_ctx = Secp256k1::new();
Expand Down