bolt12: add Invoice codec and structural validators - #10941
Conversation
github-actions
Bot
added
the
severity-critical
PR Severity: CRITICAL
Critical (1 file)
Medium (7 files)
Low (1 file)
Excluded from counting (5 files)
AnalysisThe critical classification is driven by changes to The bulk of this PR (~2,300 non-test lines) is new BOLT 12 invoice parsing/validation logic in the The To override, add a |
03cc142 to
2ab1ae2
Compare
github-actions
Bot
added
severity-high
🟠 PR Severity: HIGH
🟡 Medium (4 files)
🟢 Low (1 file)
AnalysisThe changed files are all in the Severity bump applied: 1,278 non-test lines changed exceeds the 500-line threshold, bumping the classification from MEDIUM → HIGH. Test files excluded from counting: To override, add a |
|
/gateway review |
|
✅ Review posted: #10941 (review) 6 finding(s); 6 inline, 0 in body. 🔁 Need a re-review after pushing changes? Reply with |
![lightninglabs-gateway[bot]](https://avatars.githubusercontent.com/in/3635431?s=60&v=4){kind=small}
There was a problem hiding this comment.
This PR adds the BOLT 12 Invoice message, its blinded_payinfo/fallback_address subtype codecs, a truncated-uint32 TLV type, and the structural reader/writer/cross-message validators, and threads a runtime feature-bit catalogue into the offer and invoice_request readers. The codec work is careful and unusually well-tested: round-trip bijection, truncation rejection, unknown-field mirroring, and every reader/writer rejection path are all pinned by table tests, and the deferred concerns (signing, expiry clock, on-chain fallback rules) are documented inline.
The concerns worth resolving before merge are not in the new codec math but at its edges. First, the feature-catalogue refactor is a breaking signature change to three already-shipped validators, and this diff contains no non-test caller updates even though rpcserver.go was dropped from the PR since the last severity classification — confirm every caller compiles. Second, the same refactor silently removes the write-side unknown-even-feature-bit rejection that landed three weeks ago in #10832. Third, ValidateInvoiceRead authenticates nothing cryptographically yet; that is documented and intentional, but the identity binding it does perform has a gap when offer_issuer_id is absent, so callers must not mistake a passing invoice for an authentic one.
Findings: 🔴 0 Blocker · 🟠 3 Major · 🟡 3 Minor · 🔵 0 Nit
| return err | ||
| } | ||
|
|
||
| // check UTF-8 constraints and BIP 353 |
There was a problem hiding this comment.
I removed feature bit validation from writer validation as otherwise we'd need to pass in known features. I think it should be ok to rely on ourselves to set feature bits correctly.
| // - MUST reject the invoice if this leaves no usable paths. | ||
| var usablePaths int | ||
| for i := range bp.Infos { | ||
| fv := bp.Infos[i].Features |
There was a problem hiding this comment.
Have added the docstring and signature validation will come later, in addition to checking against the blinded path.
| // BlindedPayInfos holds a list of BlindedPayInfo entries for the | ||
| // invoice_blindedpay field. | ||
| type BlindedPayInfos struct { | ||
| Infos []BlindedPayInfo |
There was a problem hiding this comment.
This is a known issue as documented. I favored using the internal data structure for tlv mechanics over keeping the byte-wise representation. Added an error path to detect this explicitly.
| return err | ||
| } | ||
|
|
||
| // - MUST reject the invoice if signature is not a valid signature using |
There was a problem hiding this comment.
Yes, documented explicitly. Imo validation should only read and not modify in this case, so this has to be checked by the pathfinding system later.
|
🤖 gateway audit metadata for this PR — auto-generated, please don't edit. |
2ab1ae2 to
0f49d8e
Compare
| return err | ||
| } | ||
|
|
||
| // check UTF-8 constraints and BIP 353 |
There was a problem hiding this comment.
I removed feature bit validation from writer validation as otherwise we'd need to pass in known features. I think it should be ok to rely on ourselves to set feature bits correctly.
| // - MUST reject the invoice if this leaves no usable paths. | ||
| var usablePaths int | ||
| for i := range bp.Infos { | ||
| fv := bp.Infos[i].Features |
There was a problem hiding this comment.
Have added the docstring and signature validation will come later, in addition to checking against the blinded path.
| // BlindedPayInfos holds a list of BlindedPayInfo entries for the | ||
| // invoice_blindedpay field. | ||
| type BlindedPayInfos struct { | ||
| Infos []BlindedPayInfo |
There was a problem hiding this comment.
This is a known issue as documented. I favored using the internal data structure for tlv mechanics over keeping the byte-wise representation. Added an error path to detect this explicitly.
| return err | ||
| } | ||
|
|
||
| // - MUST reject the invoice if signature is not a valid signature using |
There was a problem hiding this comment.
Yes, documented explicitly. Imo validation should only read and not modify in this case, so this has to be checked by the pathfinding system later.
| // WARNING: RawFeatureVector re-encodes to minimal length, so setting | ||
| // non-minimal feature bytes (trailing zeros) yields different wire | ||
| // bytes than were read and invalidates the invoice signature. |
There was a problem hiding this comment.
I believe it would be worth a change in the spec to only allow minimal encoded features in all lightning types.
This would be similar to what is being done with channel_announcement and node_announcement:
lightning/bolts#1341 (review)
erickcestari
left a comment
erickcestari
left a comment
There was a problem hiding this comment.
LGTM! Nice work!🏅
There are only some nits that are non-blocking from my side.
| RPC response rather than being emitted as an empty struct. | ||
|
|
||
| * [BOLT 12 invoice | ||
| codec](https://github.com/lightningnetwork/lnd/pull/10999): add the |
There was a problem hiding this comment.
nit:
| codec](https://github.com/lightningnetwork/lnd/pull/10999): add the | |
| codec](https://github.com/lightningnetwork/lnd/pull/10941): add the |
There was a problem hiding this comment.
Thanks, fixed the link to point at 10941.
|
|
||
| // InvoiceNodeID is the public key of the recipient node, used to verify | ||
| // the signature. | ||
| InvoiceNodeID tlv.OptionalRecordT[tlv.TlvType176, [33]byte] |
There was a problem hiding this comment.
nit:
| InvoiceNodeID tlv.OptionalRecordT[tlv.TlvType176, [33]byte] | |
| InvoiceNodeID tlv.OptionalRecordT[tlv.TlvType176, *btcec.PublicKey] |
There was a problem hiding this comment.
Good call, done. invoice_node_id is now a *btcec.PublicKey to match offer_issuer_id and invreq_payer_id. I added a present but nil guard in both the writer and reader validators, and the comparison against offer_issuer_id now uses IsEqual on the parsed keys. I also added rejection tests for the nil case on both validators.
| // - For each invoice_blindedpay.payinfo: | ||
| // - MUST NOT use the corresponding invoice_paths.path if | ||
| // payinfo.features has any unknown even bits set. | ||
| // - MUST reject the invoice if this leaves no usable paths. | ||
| // NOTE: This loop only counts usable paths to ensure at least one | ||
| // exists. Callers MUST re-apply the same knownBlindedFeatures filter | ||
| // when selecting a path downstream to avoid using an unusable path, | ||
| // as the unfiltered list is returned. | ||
| var usablePaths int | ||
| for i := range bp.Infos { | ||
| fv := bp.Infos[i].Features | ||
| wrapped := lnwire.NewFeatureVector(&fv, knownBlindedFeatures) | ||
| if len(wrapped.UnknownRequiredFeatures()) == 0 { | ||
| usablePaths++ | ||
| } | ||
| } | ||
|
|
||
| if usablePaths == 0 { | ||
| return ErrNoUsablePaths | ||
| } |
There was a problem hiding this comment.
nit: We could have a similar method of UsableFallbackAddresses, but for the BlindedPaths fields.
// UsablePath pairs a blinded path with its payment parameters, as returned by
// UsablePaths after the BOLT 12 reader's feature filter has been applied.
type UsablePath struct {
// Path is the blinded path to the recipient.
Path lnwire.BlindedPath
// PayInfo is the blinded_payinfo for Path.
PayInfo BlindedPayInfo
}
// UsablePaths returns the invoice_paths entries a payer may use, each paired
// with its blinded_payinfo, after applying the BOLT 12 reader rule that a path
// MUST NOT be used when its payinfo.features has unknown required (even) bits
// set. knownBlindedFeatures names the feature bits the reader understands.
//
// The result is empty when invoice_paths or invoice_blindedpay is absent, or
// when the two lists differ in length; ValidateInvoiceRead rejects those cases
// separately, so a caller that validates first can treat an empty result as
// "no usable paths".
func (inv *Invoice) UsablePaths(
knownBlindedFeatures map[lnwire.FeatureBit]string) []UsablePath {
paths := inv.InvoicePaths.ValOpt().UnwrapOr(lnwire.BlindedPaths{})
bp := inv.InvoiceBlindedPay.ValOpt().UnwrapOr(BlindedPayInfos{})
// Entries pair by index; a length mismatch is rejected upstream by
// ValidateInvoiceRead, so guard here to stay in bounds.
if len(paths.Paths) != len(bp.Infos) {
return nil
}
var usable []UsablePath
for i := range bp.Infos {
// MUST NOT use the path if payinfo.features has any unknown even
// bits set.
fv := bp.Infos[i].Features
wrapped := lnwire.NewFeatureVector(&fv, knownBlindedFeatures)
if len(wrapped.UnknownRequiredFeatures()) > 0 {
continue
}
usable = append(usable, UsablePath{
Path: paths.Paths[i],
PayInfo: bp.Infos[i],
})
}
return usable
}Then:
| // - For each invoice_blindedpay.payinfo: | |
| // - MUST NOT use the corresponding invoice_paths.path if | |
| // payinfo.features has any unknown even bits set. | |
| // - MUST reject the invoice if this leaves no usable paths. | |
| // NOTE: This loop only counts usable paths to ensure at least one | |
| // exists. Callers MUST re-apply the same knownBlindedFeatures filter | |
| // when selecting a path downstream to avoid using an unusable path, | |
| // as the unfiltered list is returned. | |
| var usablePaths int | |
| for i := range bp.Infos { | |
| fv := bp.Infos[i].Features | |
| wrapped := lnwire.NewFeatureVector(&fv, knownBlindedFeatures) | |
| if len(wrapped.UnknownRequiredFeatures()) == 0 { | |
| usablePaths++ | |
| } | |
| } | |
| if usablePaths == 0 { | |
| return ErrNoUsablePaths | |
| } | |
| // - For each invoice_blindedpay.payinfo: | |
| // - MUST NOT use the corresponding invoice_paths.path if | |
| // payinfo.features has any unknown even bits set. | |
| // - MUST reject the invoice if this leaves no usable paths. | |
| // UsablePaths applies that filter; a caller selecting a path downstream | |
| // should use it rather than the unfiltered invoice_paths. | |
| if len(inv.UsablePaths(knownBlindedFeatures)) == 0 { | |
| return ErrNoUsablePaths | |
| } |
There was a problem hiding this comment.
Nice suggestion. I added Invoice.UsablePaths, which returns each usable path paired with its blinded_payinfo, and ValidateInvoiceRead now uses it in place of the ad hoc counting loop. I added a test for it as well.
|
/gateway dismiss F1 per #10941 (comment) |
|
🚫 Dismissed F1 (major) by @saubyk — per #10941 (comment) Open findings on this PR: 🟠 F2 (major) · 🟠 F3 (major) · 🟡 F4 (minor) · 🟡 F5 (minor) · 🟡 F6 (minor) |
|
/gateway dismiss F6 per #10941 (comment) |
|
Thanks for you review @erickcestari, addressed your comments and also the ones for lightninglabs-gateway. @vctt94 @Abdulkbk your reviews are highly welcome, if you find the time (I can't request reviews from you on github). |
|
/gateway dismiss F2 per #10941 (comment) |
|
🚫 Dismissed F2 (major) by @saubyk — per #10941 (comment) Open findings on this PR: 🟡 F7 (minor) · 🟡 F8 (minor) · 🟡 F9 (minor) |
|
/gateway re-review |
|
✅ Re-review posted: #10941 (review) Prior findings: 4 addressed, 0 still unresolved, 0 withdrawn. 🔁 Need another re-review after pushing changes? Reply with |
There was a problem hiding this comment.
The four outstanding non-dismissed findings from the prior review are all cleanly addressed: ValidateInvoiceExpiry now uses a strict > boundary comparison matching the reader spec (F7); ValidateInvoiceAgainstRequest enforces the native offer-amount lower bound via checkInvoiceAmountMeetsOffer with an overflow guard when invreq_amount is absent (F8); the two same-typed feature catalogues are grouped into the named InvoiceFeatureCatalogues struct so they can no longer be transposed positionally (F9); and the subtype decoders retain their explicit maxBlindedPayInfos/maxFallbackAddrs caps (F5). Boundary and delegation tests were added for each.
No new blocker- or major-severity issues survived verification. The specialists surfaced several major candidates, but each was either a restatement of a maintainer-dismissed finding, or an artifact of the reviewer working from a lossy digest rather than the source (verified against the actual diff: the mirror-range upper bound and the 7200s expiry default are both correct).
One genuinely-new minor remains: decoded blinded_payinfo HTLC bounds are accepted without a relational sanity check. This does not gate merge.
Findings: 🔴 0 Blocker · 🟠 0 Major · 🟡 1 Minor · 🔵 0 Nit
Status of prior findings
- F5 addressed: Fixed —
decodeBlindedPayInfosanddecodeFallbackAddrsinbolt12/subtypes.gonow carry explicit entry-count caps (if len(bp.Infos) >= maxBlindedPayInfos { return ErrTooManyBlindedPayInfos }and themaxFallbackAddrsequivalent), documenting the real bound rather than leaning on the enclosing record-size limit. - F7 addressed: Fixed in
bolt12/validate.go—ValidateInvoiceExpirynow rejects only whencarry == 0 && uint64(now.Unix()) > expiry(strict>), so the exact expiry second is still valid, matching the BOLT 12 reader rule and the strict comparisonValidateOfferReaduses.TestValidateInvoiceExpiryadds "at default expiry boundary" (now = created+7200, no error) and "past default expiry" (created+7201, expired) cases. - F8 addressed: Fixed in
bolt12/validate.go— wheninvreq_amountis absent,ValidateInvoiceAgainstRequestnow returnscheckInvoiceAmountMeetsOffer(inv), which enforcesinvoice_amount >= offer_amount * invreq_quantityfor the native case with abits.Mul64overflow guard, and delegates theoffer_currencycase to the caller as documented.TestValidateInvoiceAgainstRequestOfferAmountcovers at/above/below the bound, quantity scaling, and the currency-delegation skip. - F9 addressed: Fixed in
bolt12/validate.go—ValidateInvoiceRead(inv, activeChain, features InvoiceFeatureCatalogues)replaces the two adjacentmap[lnwire.FeatureBit]stringparameters with the namedInvoiceFeatureCatalogues{Invoice, Blinded}struct, making a positional transposition of the invoice-features vs blinded-path catalogues impossible.TestValidateFeaturesWithCatalogueexercises both fields independently.
a5c47c0 to
2fd5980
Compare
|
/gateway re-review |
|
✅ Re-review posted: #10941 (review) Prior findings: 5 addressed, 0 still unresolved, 0 withdrawn. 🔁 Need another re-review after pushing changes? Reply with |
Add the truncated uint32 (tu32) TLV type used by invoice_relative_expiry and the dynamic invoice subtypes BlindedPayInfo and FallbackAddress, along with their encode/decode helpers and round-trip tests. These primitives are the building blocks for the BOLT 12 Invoice message struct that follows. Isolating them keeps that codec commit focused on the message shape rather than its component records.
…alidators Inject known feature-bit catalogues into the read-side validators to enable correct must-understand capability checks, and remove write-side feature enforcement entirely. Whether a feature bit is "unknown" is a runtime property of the reading node, not of the wire format or pure codec.
Add the BOLT 12 Invoice message: a struct mirroring the invoice_request fields (types 0-91) plus the invoice-specific fields (types 160-176) and the signature (type 240), together with its pure-TLV Encode/DecodeInvoice codec and the UsableFallbackAddresses accessor that applies the spec's MUST-ignore filter. Additionally, add the NewInvoiceFromRequest constructor to build an Invoice from a corresponding request. This copies all non-signature fields from the request (including unknown signed-range TLVs via the decodedTLVs sidecar) and mirrors invreq_amount into invoice_amount.
2fd5980 to
ea08c41
Compare
Implement the structural validators for the BOLT 12 invoice, adding ValidateInvoiceWrite, ValidateInvoiceRead, ValidateInvoiceExpiry, and ValidateInvoiceAgainstRequest. The validators implement the spec writer and reader requirements in the order the spec lists them. The reader confirms the signature TLV is present but defers actual Schnorr verification until the merkle and signing primitives land, mirroring the ValidateInvoiceRequestRead precedent.
Add release notes for the BOLT 12 invoice codec.
ea08c41 to
1ed6719
Compare
vctt94
left a comment
vctt94
left a comment
There was a problem hiding this comment.
LGTM! Left only a small nit.
great job!
| // be odd"). The signature range (240-1000) is exempt for the same | ||
| // reason, matching the invoice_request reader and the Merkle path. | ||
| for _, t := range sortedTypes(inv.decodedTLVs) { | ||
| if bolt12InUnsignedRange(t) { |
There was a problem hiding this comment.
Non-blocking: should this match the invoice_request pattern and only exempt the signature range from range checks, not from unknown-even checks?
Right now an unknown even TLV in the 240-1000 signature range would be skipped before ErrUnknownEvenType is applied. ValidateInvoiceRequestRead still rejects unknown even types inside that range, which seems like the safer/more consistent behavior.
|
/gateway re-review |
There was a problem hiding this comment.
All prior findings are resolved. F10 (the sole remaining unresolved concern — an unvalidated htlc_min > htlc_max range in decodeBlindedPayInfos) is now fixed with a typed ErrInvalidHtlcRange guard at decode time, matching the sibling ErrNonMinimalFeatures check. The five maintainer-dismissed findings are not re-raised. Specialist review surfaced no genuinely-new defects: the candidate findings were either false positives against the actual source or restatements of already-dismissed concerns.
One prior structural gap worth a maintainer's continued awareness (not a new finding, and already flagged non-blocking by a human reviewer): ValidateInvoiceRead skips both the range and unknown-even checks for TLV types in the signature range (240–1000), whereas ValidateInvoiceRequestRead still rejects unknown-even types there. This asymmetry was acknowledged in review and left as-is.
Status of prior findings
- F5 addressed: Addressed.
decodeBlindedPayInfosanddecodeFallbackAddrsnow enforce explicit entry caps (maxBlindedPayInfos/maxFallbackAddrs= 32) inside thefor lr.N > 0loop, consistent withdecodeChainsRecord. - F7 addressed: Addressed in
bolt12/validate.go—ValidateInvoiceExpiryuses the strict comparatoruint64(now.Unix()) > expiry, so the exact boundary second is still valid, matchingValidateOfferRead. Boundary tests cover the exact second and one past it. - F8 addressed: Addressed.
ValidateInvoiceAgainstRequestnow callscheckInvoiceAmountMeetsOfferfor the invreq_amount-absent case, enforcinginvoice_amount >= offer_amount * invreq_quantitywith abits.Mul64overflow guard; the offer_currency case is documented as delegated to the caller. - F9 addressed: Addressed. The two same-typed feature maps are grouped into the named
InvoiceFeatureCatalogues{Invoice, Blinded}struct, making positional transposition impossible; in-package callers and tests updated. - F10 addressed: Addressed in
bolt12/subtypes.go—decodeBlindedPayInfosnow rejects an inverted range withErrInvalidHtlcRangewheninfo.HtlcMinimumMsat > info.HtlcMaximumMsat, right at decode so thehtlc_min <= htlc_maxinvariant holds for every downstream consumer. A decode test covers the inverted case.
|
/gateway approve |
There was a problem hiding this comment.
✅ Approved — all prior findings addressed.
Findings recap
Findings:
F1(major) — unresolved: This diff removes thecheckFeatures(ir.InvreqFeatures)call fromValidateInvoiceRequestWritewhile the reader (`ValidateInvoiceRequestRe…F2(major) — unresolved:ValidateOfferRead,ValidateInvoiceRequestRead, and the genericcheckFeaturesall gain a required `knownFeatures map[lnwire.FeatureBit]…F3(major) — unresolved:ValidateInvoiceReadonly checksinv.Signature.IsSome(); Schnorr verification is a documentedTODO(bolt12)deferred to the caller, and …F4(minor) — unresolved:BlindedPayInfoalready carries a WARNING thatRawFeatureVectorre-encodes to minimal length. On the read path,decodeBlindedPayInfosa…F5(minor) — addressed:decodeBlindedPayInfosanddecodeFallbackAddrsboth loopfor lr.N > 0with no explicit entry-count bound, relying solely on the enclosi…F6(minor) — unresolved: The usable-paths loop inValidateInvoiceReadincrementsusablePathsfor eachblinded_payinfowhose features carry no unknown required …F7(minor) — addressed:ValidateInvoiceExpirytreats the invoice as expired whencarry == 0 && uint64(now.Unix()) >= expiry, where `expiry = invoice_created_at …F8(minor) — addressed:ValidateInvoiceAgainstRequest's only amount cross-check isreq.InvreqAmountpresent →inv.InvoiceAmountmust equal it. When the payer …F9(minor) — addressed:ValidateInvoiceRead(inv, activeChain, knownFeatures, knownBlindedFeatures map[lnwire.FeatureBit]string)places two parameters of identical…F10(minor) — addressed:decodeBlindedPayInfosreadshtlc_minimum_msatandhtlc_maximum_msat(and the fee fields) straight off the wire as rawuint64with no…
Dismissed:
F1by @saubyk — per #10941 (comment)F6by @saubyk — per #10941 (comment)F3by @saubyk — per #10941 (comment)F4by @saubyk — per #10941 (comment)F2by @saubyk — per #10941 (comment)
Approved by @saubyk via /gateway approve. Last reviewed at 1ed6719. Skill v0.3.0, model claude-opus-4-8.
ziggie1984
left a comment
ziggie1984
left a comment
There was a problem hiding this comment.
LGTM, I think we will see potential gaps only when the whole thing is implemented and we start creating Offers and Invoices in real time
Still take a look at my comments and decide by yourself if you want to address them here or in a followup PR, nothing really blocking this PR.
| // be odd"). The signature range (240-1000) is exempt for the same | ||
| // reason, matching the invoice_request reader and the Merkle path. | ||
| for _, t := range sortedTypes(inv.decodedTLVs) { | ||
| if bolt12InUnsignedRange(t) { |
|
|
||
| // Guard against overflow of offer_amount * quantity. | ||
| hi, expectedAmt := bits.Mul64(offerAmt, qty) | ||
| if hi != 0 { |
There was a problem hiding this comment.
nit: the overflow guard here (hi != 0) has no test coverage — it looks like the only fully-uncovered branch in the new invoice validation. The request-side twin checkInvreqAmountMeetsOffer is exercised by TestValidateInvoiceRequestAmountOverflow, but the invoice side isn't. A case in TestValidateInvoiceAgainstRequestOfferAmount with offer_amount = math.MaxUint64, invreq_quantity = 2, and invreq_amount absent, asserting ErrAmountBelowExpected, would close it.
| ) | ||
| require.Error(t, err) | ||
| require.Contains(t, err.Error(), tc.errSubstr) | ||
| }) |
There was a problem hiding this comment.
nit: these cases assert on the error string (require.Contains(err.Error(), ...)) even though the decoder wraps %w sentinels — ErrTooManyBlindedPayInfos, ErrNonMinimalFeatures, ErrInvalidHtlcRange (and ErrTooManyFallbackAddrs in the fallback test below). TestDecodeChainsRecord already uses require.ErrorIs(err, ErrTooManyChains); switching these to require.ErrorIs makes them robust to error-text drift.
| // We only reject unknown even bits here; advertising a feature is the | ||
| // caller's decision. | ||
| if err := checkFeatures(ir.InvreqFeatures); err != nil { | ||
| // caller's decision. Since the writer lacks a catalogue in scope, we |
There was a problem hiding this comment.
what do you mean by lacking a catalogue, you mean there are currently no even features which should be supported the bolt12 validation ?
| // caller's decision. Since the writer lacks a catalogue in scope, we | ||
| // pass nil for the catalogue, treating all even feature bits as | ||
| // unknown. | ||
| if err := checkFeatures(ir.InvreqFeatures, nil); err != nil { |
There was a problem hiding this comment.
hmm the final version however has:
if err := checkFeatures(ir.InvreqFeatures, knownFeatures); err != nil {
?
| if err := checkFeatures(ir.InvreqFeatures, nil); err != nil { | ||
| return err | ||
| } | ||
| // We rely on the writer to set feature bits correctly as those are |
There was a problem hiding this comment.
Nit: maybe prefix it with a NOTE: so it is easier when parsing the code here.
There was a problem hiding this comment.
I wonder how you decide when we do validate things on the writer side and when we don't, I think it is good to have a proper consistency and don't decide case by case, wdyt ?
| // checkInvoiceNodeID enforces the spec rule that, when offer_issuer_id is | ||
| // present, invoice_node_id MUST equal it. Both fields live on the invoice, so | ||
| // this is verifiable without the originating offer. The offer_paths branch | ||
| // (invoice_node_id equals the final blinded_node_id on the arrival path) needs |
There was a problem hiding this comment.
could you clarify this a bit more, what is the blinded_node_id arrival path of an invoice ?
| // invoice_node_id is rejected separately as ErrNilPublicKey, so a nil here is | ||
| // treated as absent. | ||
| func checkInvoiceNodeID(inv *Invoice) error { | ||
| // A present-but-nil offer_issuer_id is rejected separately as |
There was a problem hiding this comment.
you mean it is rejected somewhere else in the code and this can bascially never happen if we check it here ?
|
|
||
| // defaultInvoiceRelativeExpiry is the spec-defined fallback when an invoice | ||
| // omits invoice_relative_expiry: two hours from creation. | ||
| const defaultInvoiceRelativeExpiry uint32 = 7200 |
There was a problem hiding this comment.
is this also accounted for in the blindedd path expiry (I think in the blinded path we check expiry by blocks so I wonder if this calculation determines the blinded path generation CLTV timeout as well then ?
| // offer_amount * invreq_quantity for the native (bitcoin) case. The | ||
| // offer_currency case needs a caller-supplied exchange rate and is delegated to | ||
| // the caller. | ||
| func ValidateInvoiceAgainstRequest(inv *Invoice, req *InvoiceRequest) error { |
There was a problem hiding this comment.
This makes sense, but I wonder why we do not have something similar comparing the invoice_request with the offer which also has mirrored fields ?
7 participants
Based on #10832, part of #10736.
Adds the BOLT 12
Invoicemessage struct withPureTLVMessage-basedEncode/Decode, alongside the structuralValidateInvoiceRead/Writeand cross-messageValidateInvoiceAgainstRequestvalidators.Additionally, this PR injects known feature-bit catalogues into the
OfferandInvoiceRequestread-side validators to enable must-understand capability checks at runtime, because features are defined outside of bolt12's boundaries.