-
-
Notifications
You must be signed in to change notification settings - Fork 11
Open
Labels
enhancementNew feature or requestNew feature or requestgood first issueGood for newcomersGood for newcomershelp wantedExtra attention is neededExtra attention is needed
Description
Summary
Introduce a private decode core with a bool skipValidation switch so callers can optionally skip invoice validation and still inspect parsed fields when something fails. Keep the existing public Decode(...) API unchanged and add a new, explicitly named public method that calls the private core with validation skipped.
Background / Motivation
- During troubleshooting and tooling scenarios, consumers may want to parse BOLT11 invoices to inspect tagged fields and partial state even when validation would fail.
- Centralizing validation after parsing (see dependency below) enables a clean separation: parse first, optionally validate after.
- Current public API must not change; we will provide a new, clearly dangerous/explicit method to opt out of validation.
Dependency
- Depends on issue Move uniqueness and mutual‑exclusivity checks from
TaggedFieldList.AddtoInvoiceValidationService#69 (move uniqueness and mutual‑exclusivity checks fromTaggedFieldList.AddtoInvoiceValidationService). That refactor must land first so parsing can proceed without throwing pre‑validation.
Scope
- Create a new private core method for
Invoice.Decodethat accepts abool skipValidation. - Keep the existing public
Decode(...)method with the same signature; make it delegate to the new private core withskipValidation: false. - Add a new, explicitly named public method that makes it unmistakable validation is skipped; it delegates to the private core with
skipValidation: true.
Proposed Changes
- In
src/NLightning.Bolt11/Models/Invoice.cs:- Extract the core logic from the current
public static Invoice Decode(...)into a newprivate static Invoice DecodeInternal(string? invoiceString, BitcoinNetwork? expectedNetwork = null, bool skipValidation = false)(name is illustrative; any consistent internal name is fine). - Update existing
public static Invoice Decode(string? invoiceString, BitcoinNetwork? expectedNetwork = null)to callDecodeInternal(invoiceString, expectedNetwork, false). - Add a new public method with a very verbose name (example):
public static Invoice DecodeSkippingAllValidation(string? invoiceString, BitcoinNetwork? expectedNetwork = null)- This should call
DecodeInternal(invoiceString, expectedNetwork, true).
- Inside the private core, gate the validation section:
- If
skipValidationisfalse, runInvoiceValidationService.ValidateInvoice(invoice)(as today) and throw on errors. - If
skipValidationistrue, skip the validation step entirely, still perform signature handling decisions consistent with the goal (either skip signature check as part of validation or keep it if it does not prevent field inspection — clarify in implementation notes/tests).
- If
- Extract the core logic from the current
Acceptance Criteria
- A new
privatecore decode method exists that takesbool skipValidationand returns anInvoiceeven when it would otherwise fail validation. - The existing public
Decode(...)signature remains unchanged and delegates to the private core withskipValidation: false. - A new public method with an explicit, verbose name (e.g.,
DecodeSkippingAllValidationSoYouCanInspectParsedFieldsOnly) delegates to the private core withskipValidation: true. - XML docs clearly warn that the new method is unsafe for production and is intended solely for diagnostics/inspection.
- Unit tests cover both pathways (normal validate vs. skip validate) and confirm that when skipping, fields are still accessible even for invalid invoices.
- This change is implemented only after issue Move uniqueness and mutual‑exclusivity checks from
TaggedFieldList.AddtoInvoiceValidationService#69 is merged; PR notes and commit messages reference the dependency.
Testing Plan
- Add tests in
test/NLightning.Bolt11.Testsfor:- Normal path:
Decode(...)still validates and throws (viaInvoiceSerializationException) on invalid invoices. - Skip path:
DecodeSkippingAllValidationSoYouCanInspectParsedFieldsOnly(...)returns anInvoicewith parsedTaggedFieldListdespite invalid combinations; tests assert on the presence of fields and that no validation exception is thrown. - Edge cases: malformed Bech32 or irrecoverable parse errors should still throw from the decode core (skip does not bypass deserialization failures, only validation).
- Normal path:
Risks / Considerations
- Make sure skip‑validation does not accidentally bypass essential integrity checks that would lead to undefined state (e.g., signature parsing errors vs. signature validation): parsing should succeed, but cryptographic verification may be skipped or gated behind
skipValidationif it would throw. Document any behavior. - Very explicit method naming and XML docs are important to avoid misuse.
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
enhancementNew feature or requestNew feature or requestgood first issueGood for newcomersGood for newcomershelp wantedExtra attention is neededExtra attention is needed