-
Notifications
You must be signed in to change notification settings - Fork 407
offers: parse invoice and invoice request #3800
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -65,6 +65,8 @@ | |
//! # } | ||
//! ``` | ||
|
||
use core::str::FromStr; | ||
|
||
use crate::blinded_path::message::BlindedMessagePath; | ||
use crate::blinded_path::payment::BlindedPaymentPath; | ||
use crate::io; | ||
|
@@ -79,7 +81,7 @@ use crate::offers::offer::{ | |
Amount, ExperimentalOfferTlvStream, ExperimentalOfferTlvStreamRef, Offer, OfferContents, | ||
OfferId, OfferTlvStream, OfferTlvStreamRef, EXPERIMENTAL_OFFER_TYPES, OFFER_TYPES, | ||
}; | ||
use crate::offers::parse::{Bolt12ParseError, Bolt12SemanticError, ParsedMessage}; | ||
use crate::offers::parse::{Bech32Encode, Bolt12ParseError, Bolt12SemanticError, ParsedMessage}; | ||
use crate::offers::payer::{PayerContents, PayerTlvStream, PayerTlvStreamRef}; | ||
use crate::offers::signer::{Metadata, MetadataMaterial}; | ||
use crate::onion_message::dns_resolution::HumanReadableName; | ||
|
@@ -1284,6 +1286,30 @@ impl TryFrom<Vec<u8>> for UnsignedInvoiceRequest { | |
} | ||
} | ||
|
||
impl AsRef<[u8]> for InvoiceRequest { | ||
fn as_ref(&self) -> &[u8] { | ||
&self.bytes | ||
} | ||
} | ||
|
||
impl Bech32Encode for InvoiceRequest { | ||
const BECH32_HRP: &'static str = "lnr"; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similarly here. Additionally, this clashes with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This one is in the spec though, or at least it makes it seem
https://github.com/lightning/bolts/blob/master/12-offer-encoding.md#invoice-requests There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I raised the issue at one point: lightning/bolts#798 (comment) Ultimately, if you overload it, then your parser can't just look at the human-readable part to determine which code to call. It needs to try parsing with one and, if it fails, fall back to the other. |
||
} | ||
|
||
impl FromStr for InvoiceRequest { | ||
type Err = Bolt12ParseError; | ||
|
||
fn from_str(s: &str) -> Result<Self, <Self as FromStr>::Err> { | ||
Self::from_bech32_str(s) | ||
} | ||
} | ||
|
||
impl core::fmt::Display for InvoiceRequest { | ||
fn fmt(&self, f: &mut core::fmt::Formatter) -> Result<(), core::fmt::Error> { | ||
self.fmt_bech32_str(f) | ||
} | ||
} | ||
|
||
impl TryFrom<Vec<u8>> for InvoiceRequest { | ||
type Error = Bolt12ParseError; | ||
|
||
|
@@ -2219,6 +2245,22 @@ mod tests { | |
} | ||
} | ||
|
||
#[test] | ||
fn parses_bech32_encoded_invoice_requests() { | ||
let invoice_requests = [ | ||
"lnr1qqsg7jpsyzz4hcsj0hu6rvjevwhmkceurq7sd5ez8ne3js4qt8acvxcgqgp7szsqzsqpvggzhdvttlk22pw8fmwqqrvzst792mj35ypylj886ljkcmug03wg6he9yqs86ptqzqjcyypqk4jf95qryjcsqywr6kktzrf366ex4yp8cr5r8m32cre3kfea7w0sgzegrzqgucwd37cjyvkgg2lfae8j6wyyx7dj3aqe8j2ncrthhszl8r69lecma5cxclmft4kh8x39jaeqtdl2yy5gsfdqcpvxczf5x0sw" | ||
]; | ||
for encoded in invoice_requests { | ||
let decoded = match encoded.parse::<InvoiceRequest>() { | ||
Ok(decoded) => decoded, | ||
Err(e) => panic!("Invalid invoice request ({:?}): {}", e, encoded), | ||
}; | ||
|
||
let reencoded = decoded.to_string(); | ||
assert_eq!(reencoded, encoded, "Re-encoded invoice does not match original"); | ||
} | ||
} | ||
|
||
#[test] | ||
fn parses_invoice_request_with_metadata() { | ||
let expanded_key = ExpandedKey::new([42; 32]); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm... we shouldn't introduce a bech32 encoding that isn't in the spec. BOLT12 invoices are only transferred over the wire, so one isn't defined. They also only make sense in the context of a preceding
offer
/invoice_request
(i.e., you'll never see them in a context where a user would scan it).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FWIW, could that be a spec bug? Seems CLN is supporting the
lni
format, so maybe it should be added to the spec, or was that omitted on purpose?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was deliberately removed from the spec. IIRC, @TheBlueMatt pushed for having it removed. CLN uses it in some tests and its CLI, I believe.