BOLT12 fixes#3
Closed
m0wer wants to merge 5 commits into
Closed
Conversation
The fallbacks branch in to_lnaddr was unreachable in practice and
contained two bugs:
1. data.get('invoice_fallbacks', []) returned the wrong shape; per the
onion_wire CSV the value is a dict {'fallbacks': [...]}, so the
iteration always yielded nothing on a well-formed invoice.
2. The filter expression had a misplaced parenthesis,
len(x['address'] <= 40), which would raise TypeError on a bytes
address. Additionally, 'version' is decoded by the lnmsg subtype
parser as a 1-byte bytes object, so comparing it to int 16 would
also raise TypeError.
Fixes the parsing, adds regression tests covering both the populated
and empty fallback cases.
Two issues in the blinded-payment receive path: 1. An assert was used to enforce the BOLT 04 rule that a final hop payload over a blinded path must only contain a specific set of TLV fields. A peer sending an unexpected field would raise AssertionError instead of returning the spec-mandated INCORRECT_OR_UNKNOWN_PAYMENT_DETAILS onion error. 2. self.lnworker._pathids[htlc.payment_hash] would raise KeyError when no path_ids had been registered for the payment_hash (e.g. a spurious HTLC). Now treated as an unknown path_id and the proper onion failure is returned.
UpdateAddHtlc.to_json now produces a 6-tuple (with the trailing 'blinding' field) instead of the previous 5-tuple. The new code reads either shape, but older electrum versions call from_tuple(*tuple) positionally and would raise TypeError on a 6-tuple entry. Bumping FINAL_SEED_VERSION ensures older clients refuse to open wallets that may now contain blinded HTLC entries, rather than failing mid-deserialization with an unclear error.
Previously, a malformed bolt12 invoice missing invoice_node_id,
invoice_created_at, or invoice_payment_hash would crash with a
non-descriptive AttributeError ('NoneType' has no attribute 'get')
deep in conversion. Now raise Bolt12InvoiceError with a clear message
identifying the missing field.
The LRU cache for processed onions keyed entries on onion_hash + payment_hash + is_trampoline only. With route blinding a receiver may advertise multiple blinded paths for the same payment; two incoming HTLCs that share onion_hash and payment_hash but use different blinding bytes would otherwise be served the same cached result. Including blinding in the key removes the ambiguity. Also drops a commented-out debug line from the new bolt12 regtest case.
Author
|
Stale target branch. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Some fixes/improvements for spesmilo#10110
bolt12.py::to_lnaddrwould have been unreachable without raising on any invoice carryinginvoice_fallbacks(commit df8366b).lnpeer.pyblinded final-hop handling crashed withAssertionError/KeyErroron malformed or spurious HTLCs instead of returning the spec-mandated onion failure (commit 7c26c5b).UpdateAddHtlcstorage gained a 6th field without a wallet-db version bump; bumpedFINAL_SEED_VERSIONto 67 (commit 7425e12).to_lnaddrraised non-descriptiveAttributeErroron malformed invoices; nowBolt12InvoiceError(commit fd19abb)._processed_onion_cachekey omittedblinding, allowing cache collisions across distinct blinded paths for the samepayment_hash; key now includes it (commit a8d64d9).End-to-end regtest
test_bolt12passes (Alice -> Bob -> Carol BOLT12 payment over a blinded path).BOLT12_INVOICE_PREFIXstorage hack would benefit from documentation and a unit test confirming round-trip.