offer: make the merkle tree signature public #3892

vincenzopalazzo · 2025-06-26T10:30:20Z

This is helpful for users who want to use the Merkle tree signature in their own code, for example, to verify the signature of bolt12 invoices or recreate it.

Very useful for people who are building command line tools for the bolt12 offers.

I am opening this, but I do not know if it is something that you want to do, but at the same time, IMHO, this is very useful to expose because it allows to use LDK in command line tools and in learning tools.

However, this is not strictly necessary because Bolt12Invoice::try_from already verifies the signature, but I would open this to know your point of view.

ldk-reviews-bot · 2025-06-26T10:30:22Z

👋 I see @TheBlueMatt was un-assigned.
If you'd like another reviewer assignment, please click here.

dunxen · 2025-06-26T11:51:14Z

I'm not personally opposed to this and it may be nice to expose it for the use cases you mention.
However, maybe the doc comments for these should be updated to indicate that these don't need to be used directly and they're really for internal use, but made public for use in applications like you specified. We should redirect them to the appropriate methods for production use.

I'm also not sure about any API guarantees for these methods. I don't think it's too much of a hassle to treat them the same way we treat any other public API.

codecov · 2025-06-26T15:08:45Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.80%. Comparing base (0fe51c5) to head (e60b87c).
Report is 71 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3892      +/-   ##
==========================================
- Coverage   89.66%   88.80%   -0.86%     
==========================================
  Files         164      166       +2     
  Lines      134661   119539   -15122     
  Branches   134661   119539   -15122     
==========================================
- Hits       120743   106161   -14582     
+ Misses      11237    11038     -199     
+ Partials     2681     2340     -341

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

valentinewallace · 2025-06-27T16:20:23Z

lightning/src/offers/merkle.rs

@@ -41,7 +41,7 @@ impl TaggedHash {
 	/// Creates a tagged hash with the given parameters.
 	///
 	/// Panics if `bytes` is not a well-formed TLV stream containing at least one TLV record.
-	pub(super) fn from_valid_tlv_stream_bytes(tag: &'static str, bytes: &[u8]) -> Self {
+	pub fn from_valid_tlv_stream_bytes(tag: &'static str, bytes: &[u8]) -> Self {


Do we have a user for this? Feels pretty low-level to be exposing. Echo dunxen's sentiments about improving the docs if we do want to expose it.

Yea, if we're gonna expose this we should really add a validating wrapper version that checks the stream is valid and can Err instead of panicking.

Do we have a user for this? Feels pretty low-level to be exposing.

Yeah, there is. Everyone who wants to verify that an offer is linked to a bolt12 invoice, also if the big picture is that the bolt12 invoice will never be exposed, but for now that we do not have a better PoP a person should allow this kind of verification IMHO

A possible example of verification can be

/// Verify that an offer, invoice, and preimage are valid and consistent pub fn verify_offer_payment(offer: &str, invoice: &str, preimage: &str) -> Result<()> { log::info!("Starting verification process..."); log::info!("Offer: {}", offer); log::info!("Invoice: {}", invoice); log::info!("Preimage: {}", preimage); let offer = Offer::from_str(offer).map_err(|e| anyhow::anyhow!("Failed to parse offer: {:?}", e))?; // Parse the bolt12 invoice preimage let (hrp, data) = bech32::decode_without_checksum(invoice)?; if hrp.as_str() != "lni" { return Err(anyhow::anyhow!( "Invalid HRP: expected 'lni', found '{}'", hrp )); } let data = Vec::<u8>::from_base32(&data)?; let invoice = Bolt12Invoice::try_from(data) .map_err(|e| anyhow::anyhow!("Failed to parse BOLT12: {e:?}"))?; let issuer_sign_pubkey = if let Some(issue_sign_pubkey) = offer.issuer_signing_pubkey() { issue_sign_pubkey } else { let valid_signing_keys = offer .paths() .iter() .filter_map(|path| path.blinded_hops().last()) .map(|hop| hop.blinded_node_id) .collect::<Vec<_>>(); log::debug!("{:?}", valid_signing_keys); let valid_signing_keys = valid_signing_keys .first() .ok_or(anyhow::anyhow!( "Offer does not contain issuer signing pubkey" ))? .clone(); valid_signing_keys }; let inv_signing_pubkey = invoice.signing_pubkey(); if issuer_sign_pubkey != inv_signing_pubkey { return Err(anyhow::anyhow!( "Offer and invoice issuer signing pubkeys do not match" )); } let mut bytes = vec![]; invoice .write(&mut bytes) .map_err(|e| anyhow::anyhow!("Failed to serialize invoice: {:?}", e))?; let tagged_hash = TaggedHash::from_valid_tlv_stream_bytes(SIGNATURE_TAG, &bytes); if let Err(err) = merkle::verify_signature(&invoice.signature(), &tagged_hash, issuer_sign_pubkey) { anyhow::bail!("Failed to verify invoice signature: {:?}", err); } let preimage = hex::decode(preimage).map_err(|e| anyhow::anyhow!("Failed to decode preimage: {:?}", e))?; let preimage: [u8; 32] = preimage .try_into() .map_err(|_| anyhow::anyhow!("Preimage must be 32 bytes"))?; let preimage = PaymentPreimage(preimage); // Validate the Proof of Payment for the invoice let computed_hash = PaymentHash::from(preimage); let payment_hash = invoice.payment_hash(); if computed_hash != payment_hash { anyhow::bail!("Fail to perform PoP"); } log::info!("Parsed offer: {:?}", offer); log::info!("Parsed invoice: {:?}", invoice); Ok(()) }

Hmm... we verify the invoice's signature when parsing it. So it's just a matter of matching that the Offer given is the same one that is part of the Bolt12Invoice. This should be possible by comparing Offer::id -- which is just the tagged hash of the offer's merkle root -- against one obtained from the Bolt12Invoice. We could possibly add Bolt12Invoice::offer_id for this.

Note that while OfferId uses an LDK-specific tag, it is computed over the TLV stream bytes so it doesn't matter if the offer wasn't created with LDK.

Ok implemented it @jkczyz cfd6ad4 let me know what do you think.

However, I still think that ldk should expose a way to verify the signature with the public key that is inside the offer, at least till we do not get a better PoP with bolt12

Ok implemented it @jkczyz cfd6ad4 let me know what do you think.

Thanks, left some comments.

However, I still think that ldk should expose a way to verify the signature with the public key that is inside the offer, at least till we do not get a better PoP with bolt12

Given the signature is verified when parsing, shouldn't it be sufficient to check that Bolt12Invoice::signing_pubkey matches the one in the offer? The parser is already doing this when calling check_invoice_signing_pubkey, too, FWIW. Not sure I understand why we need to expose these for the user to piece together and call on their own.

At very least, there doesn't seem to be a need to expose TaggedHash::from_valid_tlv_stream_bytes if we can just have a method on Bolt12Invoice give the TaggedHash for manual signature verification.

Given the signature is verified when parsing, shouldn't it be sufficient to check that Bolt12Invoice::signing_pubkey matches the one in the offer?

Yes, good point, I think it is just fine for a proof of linking point of view.

FWIW. Not sure I understand why we need to expose these for the user to piece together and call on their own.

At this point is just for a learning tool purpose if some users want to do the versification as specified inside the spec. Probably, this is a separate discussion

At very least, there doesn't seem to be a need to expose TaggedHash::from_valid_tlv_stream_bytes if we can just have a method on Bolt12Invoice give the TaggedHash for manual signature verification.

This is a really good point, lets me experiment with it

ldk-reviews-bot · 2025-06-27T16:20:29Z

👋 The first review has been submitted!

Do you think this PR is ready for a second reviewer? If so, click here to assign a second reviewer.

lightning/src/offers/merkle.rs

ldk-reviews-bot · 2025-07-07T10:48:01Z

🔔 1st Reminder

Hey @TheBlueMatt @valentinewallace! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2025-07-07T10:48:02Z

🔔 1st Reminder

Hey @TheBlueMatt @valentinewallace! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

vincenzopalazzo · 2025-07-07T17:18:52Z

cc @jkczyz

jkczyz · 2025-07-08T20:32:08Z

lightning/src/offers/invoice.rs

+				let offer_tlv_stream = TlvStream::new(&self.bytes).range(OFFER_TYPES);
+				let experimental_offer_tlv_stream = TlvStream::new(&self.experimental_bytes).range(EXPERIMENTAL_OFFER_TYPES);
+				let combined_tlv_stream = offer_tlv_stream.chain(experimental_offer_tlv_stream);
+				let tagged_hash = TaggedHash::from_tlv_stream("LDK Offer ID", combined_tlv_stream);
+				Some(OfferId(tagged_hash.to_bytes()))


You can reuse OfferId::from_valid_invreq_tlv_stream (or something with a more appropriate name) rather than duplicating that code.

Make sense thanks!

Done created the from_invoice_bytes that IMHO made clear that we look at the range of the offer TLV, please let me know if you have any other suggestion on this

lightning/src/offers/invoice.rs

jkczyz · 2025-07-08T21:11:50Z

lightning/src/offers/merkle.rs

@@ -41,7 +41,7 @@ impl TaggedHash {
 	/// Creates a tagged hash with the given parameters.
 	///
 	/// Panics if `bytes` is not a well-formed TLV stream containing at least one TLV record.
-	pub(super) fn from_valid_tlv_stream_bytes(tag: &'static str, bytes: &[u8]) -> Self {
+	pub fn from_valid_tlv_stream_bytes(tag: &'static str, bytes: &[u8]) -> Self {


Ok implemented it @jkczyz cfd6ad4 let me know what do you think.

Thanks, left some comments.

However, I still think that ldk should expose a way to verify the signature with the public key that is inside the offer, at least till we do not get a better PoP with bolt12

Given the signature is verified when parsing, shouldn't it be sufficient to check that Bolt12Invoice::signing_pubkey matches the one in the offer? The parser is already doing this when calling check_invoice_signing_pubkey, too, FWIW. Not sure I understand why we need to expose these for the user to piece together and call on their own.

At very least, there doesn't seem to be a need to expose TaggedHash::from_valid_tlv_stream_bytes if we can just have a method on Bolt12Invoice give the TaggedHash for manual signature verification.

TheBlueMatt · 2025-07-09T01:06:27Z

Will take a look at this after @jkczyz's comments are addressed, assuming he thinks it needs a second reviewer at that point.

ldk-reviews-bot · 2025-07-09T10:49:03Z

🔔 2nd Reminder

Hey @valentinewallace! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

This is helpfull for the users that want to use the merkle tree signature in their own code, for example to verify the signature of bolt12 invoices or recreate it. Very useful for people that are building command line tools for the bolt12 offers. Signed-off-by: Vincenzo Palazzo <[email protected]>

vincenzopalazzo · 2025-07-09T11:19:29Z

Ok, I pushed the change that @jkczyz requested, and now the PR is organised in two commits as follows:

bed3812 where expose the sign and verify method to verify manually an invoice signature, this is nice for people that are building command line tools and want to show how the signature verification is described inside the spec
-e60b87c38c043114642eda5499864c3c6898aa64 expose the offer_id inside the Bolt12Invoice that allows to compare it with the Offer to make sure that the invoice is linked with the Bolt12 one. This is useful for a sanity check, but requires trusting that the decoding of ldk will always verify the signature of the invoice

Let me know if you are fine with the commit divided in this way, or you prefer e60b87c

lightning/src/offers/invoice.rs

- Add an Option<OfferId> field to Bolt12Invoice to track the originating offer. - Compute the offer_id for invoices created from offers by extracting the offer TLV records and hashing them with the correct tag. - Expose a public offer_id() accessor on Bolt12Invoice. - Add tests to ensure the offer_id in the invoice matches the originating Offer, and that refund invoices have no offer_id. - All existing and new tests pass. This enables linking invoices to their originating offers in a robust and spec-compliant way. Signed-off-by: Vincenzo Palazzo <[email protected]> Co-authored-by: graphite-app[bot] <96075541+graphite-app[bot]@users.noreply.github.com>

jkczyz · 2025-07-09T22:52:44Z

lightning/src/offers/invoice.rs

@@ -967,6 +980,11 @@ impl Bolt12Invoice {
 		self.tagged_hash.as_digest().as_ref().clone()
 	}

+	/// Returns the [`OfferId`] if this invoice corresponds to an [`crate::offers::offer::Offer`].


The docs would be cleaner as:

/// Returns the [`OfferId`] if this invoice corresponds to an [`Offer`]. /// /// [`Offer`]: crate::offers::offer::Offer

jkczyz · 2025-07-09T22:54:15Z

lightning/src/offers/invoice.rs

@@ -665,6 +665,14 @@ impl UnsignedBolt12Invoice {
 	pub fn tagged_hash(&self) -> &TaggedHash {
 		&self.tagged_hash
 	}
+
+	/// Computes the [`OfferId`] if this invoice corresponds to an [`Offer`].


Link will need to be fixed here as described in my other comment. Or just drop the docs since this is a private method and doesn't need much explanation.

jkczyz · 2025-07-09T22:58:39Z

lightning/src/offers/offer.rs

+		let offer_tlv_stream = TlvStream::new(bytes).range(OFFER_TYPES);
+		let experimental_offer_tlv_stream = TlvStream::new(bytes).range(EXPERIMENTAL_OFFER_TYPES);
+		let combined_tlv_stream = offer_tlv_stream.chain(experimental_offer_tlv_stream);
+		let tagged_hash = TaggedHash::from_tlv_stream(Self::ID_TAG, combined_tlv_stream);
+		Self(tagged_hash.to_bytes())


This should be the same code as from_valid_invreq_tlv_stream.

jkczyz · 2025-07-09T23:04:43Z

lightning/src/offers/merkle.rs

-pub 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


This belongs in the previous commit.

jkczyz · 2025-07-09T23:05:26Z

lightning/src/offers/invoice.rs

+	fn compute_offer_id(&self) -> Option<OfferId> {
+		match &self.contents {
+			InvoiceContents::ForOffer { .. } => Some(OfferId::from_invoice_bytes(&self.bytes)),
+			InvoiceContents::ForRefund { .. } => None,
+		}
+	}


Maybe we should just inline this given it's not reused in the parsing code.

ldk-reviews-bot requested a review from valentinewallace June 26, 2025 10:40

vincenzopalazzo force-pushed the macros/markle-signature branch from 0ffed61 to be23002 Compare June 26, 2025 10:44

valentinewallace reviewed Jun 27, 2025

View reviewed changes

vincenzopalazzo requested review from TheBlueMatt and valentinewallace July 5, 2025 10:47

graphite-app bot reviewed Jul 5, 2025

View reviewed changes

lightning/src/offers/merkle.rs Outdated Show resolved Hide resolved

vincenzopalazzo force-pushed the macros/markle-signature branch 2 times, most recently from dd046d5 to 29e5bd6 Compare July 8, 2025 20:46

jkczyz reviewed Jul 8, 2025

View reviewed changes

TheBlueMatt removed their request for review July 9, 2025 01:06

vincenzopalazzo force-pushed the macros/markle-signature branch from 0c7fe08 to 123e16f Compare July 9, 2025 10:19

vincenzopalazzo force-pushed the macros/markle-signature branch from 123e16f to 7429b6a Compare July 9, 2025 11:11

vincenzopalazzo force-pushed the macros/markle-signature branch from 7429b6a to 1b958e9 Compare July 9, 2025 11:13

vincenzopalazzo force-pushed the macros/markle-signature branch from 1b958e9 to 699f37f Compare July 9, 2025 11:23

graphite-app bot reviewed Jul 9, 2025

View reviewed changes

lightning/src/offers/invoice.rs Outdated Show resolved Hide resolved

vincenzopalazzo force-pushed the macros/markle-signature branch from 22ef851 to e60b87c Compare July 9, 2025 20:12

jkczyz reviewed Jul 9, 2025

View reviewed changes

offer: make the merkle tree signature public #3892

Are you sure you want to change the base?

offer: make the merkle tree signature public #3892

Conversation

vincenzopalazzo commented Jun 26, 2025

Uh oh!

ldk-reviews-bot commented Jun 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dunxen commented Jun 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Jun 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ldk-reviews-bot commented Jun 27, 2025

Uh oh!

Uh oh!

ldk-reviews-bot commented Jul 7, 2025

Uh oh!

ldk-reviews-bot commented Jul 7, 2025

Uh oh!

vincenzopalazzo commented Jul 7, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TheBlueMatt commented Jul 9, 2025

Uh oh!

ldk-reviews-bot commented Jul 9, 2025

Uh oh!

vincenzopalazzo commented Jul 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ldk-reviews-bot commented Jun 26, 2025 •

edited

Loading

dunxen commented Jun 26, 2025 •

edited

Loading

codecov bot commented Jun 26, 2025 •

edited

Loading

vincenzopalazzo commented Jul 9, 2025 •

edited

Loading