Skip to content

offer: make the merkle tree signature public #3892

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
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
76 changes: 72 additions & 4 deletions lightning/src/offers/invoice.rs
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ use crate::offers::merkle::{
};
use crate::offers::nonce::Nonce;
use crate::offers::offer::{
Amount, ExperimentalOfferTlvStream, ExperimentalOfferTlvStreamRef, OfferTlvStream,
Amount, ExperimentalOfferTlvStream, ExperimentalOfferTlvStreamRef, OfferId, OfferTlvStream,
OfferTlvStreamRef, Quantity, EXPERIMENTAL_OFFER_TYPES, OFFER_TYPES,
};
use crate::offers::parse::{Bolt12ParseError, Bolt12SemanticError, ParsedMessage};
Expand Down Expand Up @@ -595,6 +595,7 @@ pub struct UnsignedBolt12Invoice {
experimental_bytes: Vec<u8>,
contents: InvoiceContents,
tagged_hash: TaggedHash,
offer_id: Option<OfferId>,
}

/// A function for signing an [`UnsignedBolt12Invoice`].
Expand Down Expand Up @@ -658,7 +659,11 @@ impl UnsignedBolt12Invoice {
let tlv_stream = TlvStream::new(&bytes).chain(TlvStream::new(&experimental_bytes));
let tagged_hash = TaggedHash::from_tlv_stream(SIGNATURE_TAG, tlv_stream);

Self { bytes, experimental_bytes, contents, tagged_hash }
let offer_id = match &contents {
InvoiceContents::ForOffer { .. } => Some(OfferId::from_valid_bolt12_tlv_stream(&bytes)),
InvoiceContents::ForRefund { .. } => None,
};
Self { bytes, experimental_bytes, contents, tagged_hash, offer_id }
Comment on lines +662 to +666
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jkczyz do you think that we should add in OfferId a maybe_from_valid_bolt12_tlv_stream(&bytes) -> Option<OfferId> to include the refund case without duplicate the match here?

Copy link
Contributor

Choose a reason for hiding this comment

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

We'd need to take in the InvoiceContents then, too. Leaning towards not doing this for unsigned invoices after all. See next comment.

}

/// Returns the [`TaggedHash`] of the invoice to sign.
Expand Down Expand Up @@ -686,6 +691,13 @@ macro_rules! unsigned_invoice_sign_method { ($self: ident, $self_type: ty $(, $s
// Append the experimental bytes after the signature.
$self.bytes.extend_from_slice(&$self.experimental_bytes);

let offer_id = match &$self.contents {
InvoiceContents::ForOffer { .. } => {
Some(OfferId::from_valid_bolt12_tlv_stream(&$self.bytes))
},
InvoiceContents::ForRefund { .. } => None,
};

Ok(Bolt12Invoice {
#[cfg(not(c_bindings))]
bytes: $self.bytes,
Expand All @@ -700,6 +712,7 @@ macro_rules! unsigned_invoice_sign_method { ($self: ident, $self_type: ty $(, $s
tagged_hash: $self.tagged_hash,
#[cfg(c_bindings)]
tagged_hash: $self.tagged_hash.clone(),
offer_id,
})
}
} }
Expand Down Expand Up @@ -734,6 +747,7 @@ pub struct Bolt12Invoice {
contents: InvoiceContents,
signature: Signature,
tagged_hash: TaggedHash,
offer_id: Option<OfferId>,
}

/// The contents of an [`Bolt12Invoice`] for responding to either an [`Offer`] or a [`Refund`].
Expand Down Expand Up @@ -1432,7 +1446,12 @@ impl TryFrom<Vec<u8>> for UnsignedBolt12Invoice {
.map_or(0, |last_record| last_record.end);
let experimental_bytes = bytes.split_off(offset);

Ok(UnsignedBolt12Invoice { bytes, experimental_bytes, contents, tagged_hash })
let offer_id = match &contents {
InvoiceContents::ForOffer { .. } => Some(OfferId::from_valid_bolt12_tlv_stream(&bytes)),
InvoiceContents::ForRefund { .. } => None,
};

Ok(UnsignedBolt12Invoice { bytes, experimental_bytes, contents, tagged_hash, offer_id })
}
}

Expand Down Expand Up @@ -1622,7 +1641,11 @@ impl TryFrom<ParsedMessage<FullInvoiceTlvStream>> for Bolt12Invoice {
let pubkey = contents.fields().signing_pubkey;
merkle::verify_signature(&signature, &tagged_hash, pubkey)?;

Ok(Bolt12Invoice { bytes, contents, signature, tagged_hash })
let offer_id = match &contents {
InvoiceContents::ForOffer { .. } => Some(OfferId::from_valid_bolt12_tlv_stream(&bytes)),
InvoiceContents::ForRefund { .. } => None,
};
Ok(Bolt12Invoice { bytes, contents, signature, tagged_hash, offer_id })
}
}

Expand Down Expand Up @@ -3556,4 +3579,49 @@ mod tests {
),
}
}

#[test]
fn invoice_offer_id_matches_offer_id() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add a test for StaticInvoice, too.

let expanded_key = ExpandedKey::new([42; 32]);
let entropy = FixedEntropy {};
let nonce = Nonce::from_entropy_source(&entropy);
let secp_ctx = Secp256k1::new();
let payment_id = PaymentId([1; 32]);

let offer = OfferBuilder::new(recipient_pubkey()).amount_msats(1000).build().unwrap();

let offer_id = offer.id();

let invoice_request = offer
.request_invoice(&expanded_key, nonce, &secp_ctx, payment_id)
.unwrap()
.build_and_sign()
.unwrap();

let invoice = invoice_request
.respond_with_no_std(payment_paths(), payment_hash(), now())
.unwrap()
.build()
.unwrap()
.sign(recipient_sign)
.unwrap();

assert_eq!(invoice.offer_id(), Some(offer_id));
}

#[test]
fn refund_invoice_has_no_offer_id() {
let refund =
RefundBuilder::new(vec![1; 32], payer_pubkey(), 1000).unwrap().build().unwrap();

let invoice = refund
.respond_with_no_std(payment_paths(), payment_hash(), recipient_pubkey(), now())
.unwrap()
.build()
.unwrap()
.sign(recipient_sign)
.unwrap();

assert_eq!(invoice.offer_id(), None);
}
}
7 changes: 7 additions & 0 deletions lightning/src/offers/invoice_macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,13 @@ macro_rules! invoice_accessors_common { ($self: ident, $contents: expr, $invoice
pub fn invoice_features(&$self) -> &Bolt12InvoiceFeatures {
$contents.features()
}

/// Returns the [`OfferId`] if this invoice corresponds to an [`Offer`].
///
/// [`Offer`]: crate::offers::offer::Offer
pub fn offer_id(&$self) -> Option<OfferId> {
$self.offer_id
}
} }

pub(super) use invoice_accessors_common;
Expand Down
8 changes: 3 additions & 5 deletions lightning/src/offers/merkle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ pub enum SignError {
}

/// A function for signing a [`TaggedHash`].
pub(super) trait SignFn<T: AsRef<TaggedHash>> {
pub trait SignFn<T: AsRef<TaggedHash>> {
/// Signs a [`TaggedHash`] computed over the merkle root of `message`'s TLV stream.
fn sign(&self, message: &T) -> Result<Signature, ()>;
}
Expand All @@ -117,9 +117,7 @@ where
///
/// [`Bolt12Invoice`]: crate::offers::invoice::Bolt12Invoice
/// [`InvoiceRequest`]: crate::offers::invoice_request::InvoiceRequest
pub(super) fn sign_message<F, T>(
f: F, message: &T, pubkey: PublicKey,
) -> Result<Signature, SignError>
pub fn sign_message<F, T>(f: F, message: &T, pubkey: PublicKey) -> Result<Signature, SignError>
where
F: SignFn<T>,
T: AsRef<TaggedHash>,
Expand All @@ -136,7 +134,7 @@ where

/// Verifies the signature with a pubkey over the given message using a tagged hash as the message
/// digest.
pub(super) fn verify_signature(
pub fn verify_signature(
signature: &Signature, message: &TaggedHash, pubkey: PublicKey,
) -> Result<(), secp256k1::Error> {
let digest = message.as_digest();
Expand Down
4 changes: 2 additions & 2 deletions lightning/src/offers/offer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ impl OfferId {
Self(tagged_hash.to_bytes())
}

fn from_valid_invreq_tlv_stream(bytes: &[u8]) -> Self {
pub(super) fn from_valid_bolt12_tlv_stream(bytes: &[u8]) -> Self {
let tlv_stream = Offer::tlv_stream_iter(bytes);
let tagged_hash = TaggedHash::from_tlv_stream(Self::ID_TAG, tlv_stream);
Self(tagged_hash.to_bytes())
Expand Down Expand Up @@ -987,7 +987,7 @@ impl OfferContents {
secp_ctx,
)?;

let offer_id = OfferId::from_valid_invreq_tlv_stream(bytes);
let offer_id = OfferId::from_valid_bolt12_tlv_stream(bytes);

Ok((offer_id, keys))
},
Expand Down
20 changes: 16 additions & 4 deletions lightning/src/offers/static_invoice.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ use crate::offers::merkle::{
use crate::offers::nonce::Nonce;
use crate::offers::offer::{
Amount, ExperimentalOfferTlvStream, ExperimentalOfferTlvStreamRef, Offer, OfferContents,
OfferTlvStream, OfferTlvStreamRef, Quantity, EXPERIMENTAL_OFFER_TYPES, OFFER_TYPES,
OfferId, OfferTlvStream, OfferTlvStreamRef, Quantity, EXPERIMENTAL_OFFER_TYPES, OFFER_TYPES,
};
use crate::offers::parse::{Bolt12ParseError, Bolt12SemanticError, ParsedMessage};
use crate::types::features::{Bolt12InvoiceFeatures, OfferFeatures};
Expand Down Expand Up @@ -70,6 +70,7 @@ pub struct StaticInvoice {
bytes: Vec<u8>,
contents: InvoiceContents,
signature: Signature,
offer_id: Option<OfferId>,
}

impl PartialEq for StaticInvoice {
Expand Down Expand Up @@ -198,6 +199,7 @@ pub struct UnsignedStaticInvoice {
experimental_bytes: Vec<u8>,
contents: InvoiceContents,
tagged_hash: TaggedHash,
offer_id: Option<OfferId>,
}

macro_rules! invoice_accessors { ($self: ident, $contents: expr) => {
Expand Down Expand Up @@ -330,7 +332,9 @@ impl UnsignedStaticInvoice {
let tlv_stream = TlvStream::new(&bytes).chain(TlvStream::new(&experimental_bytes));
let tagged_hash = TaggedHash::from_tlv_stream(SIGNATURE_TAG, tlv_stream);

Self { bytes, experimental_bytes, contents, tagged_hash }
// FIXME: we can have a static invoice for a Refund? if yes this should be optional
let offer_id = OfferId::from_valid_bolt12_tlv_stream(&bytes);
Self { bytes, experimental_bytes, contents, tagged_hash, offer_id: Some(offer_id) }
Comment on lines +335 to +337
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm... I guess we'll never need the Refund case since those are user-initiated actions (i.e., they scan a Refund and create a Bolt12Invoice to send).

}

/// Signs the [`TaggedHash`] of the invoice using the given function.
Expand All @@ -347,7 +351,13 @@ impl UnsignedStaticInvoice {
// Append the experimental bytes after the signature.
self.bytes.extend_from_slice(&self.experimental_bytes);

Ok(StaticInvoice { bytes: self.bytes, contents: self.contents, signature })
let offer_id = OfferId::from_valid_bolt12_tlv_stream(&self.bytes);
Ok(StaticInvoice {
bytes: self.bytes,
contents: self.contents,
signature,
offer_id: Some(offer_id),
})
}

invoice_accessors_common!(self, self.contents, UnsignedStaticInvoice);
Expand Down Expand Up @@ -627,7 +637,9 @@ impl TryFrom<ParsedMessage<FullInvoiceTlvStream>> for StaticInvoice {
let pubkey = contents.signing_pubkey;
merkle::verify_signature(&signature, &tagged_hash, pubkey)?;

Ok(StaticInvoice { bytes, contents, signature })
// this is coming always from an offer, so this is always Some.
let offer_id = OfferId::from_valid_bolt12_tlv_stream(&bytes);
Ok(StaticInvoice { bytes, contents, signature, offer_id: Some(offer_id) })
}
}

Expand Down
Loading