Onion Updates to Match Latest Spec (and test vectors!) #4921

rustyrussell · 2021-11-18T02:38:45Z

Once again, we support old and new onions. First I remove the old-old spec versions. Then I rename the new to obs2. Then add the new ones. Then make sure we send and receive both OK.

This should comply with the latest route blinding lightning/bolts#765 and onion message lightning/bolts#759 proposals.

Thanks to @t-bast and @thomash-acinq and @joostjager for all the feedback!

gossipd/test/run-onion_message.c

vincenzopalazzo

LGTM, Github action point me out an error, where I don't know if there are any better solutions than the ones that I proposed in the comment.

Anyway, it is only a comments

tests/test_pay.py

vincenzopalazzo

I think here the mistake it is the space and not the end line, if I'm not wrong python have 2 end-line conventions between function.

So, in this case, looks good the change, but I think in the file there is a space " "

rustyrussell · 2021-11-28T00:45:39Z

Rebase to fix flakes...

vincenzopalazzo · 2021-11-28T08:34:20Z

I think we have a list part to migrate in the new spec version, that it is test_pay.py

lightning/tests/test_pay.py

Lines 3284 to 3301 in 35c6f90

    
           route = [{'id': l2.info['id'], 
        
                     'channel': l1.get_channel_scid(l2), 
        
                     'amount_msat': Millisatoshi(1002), 
        
                     'delay': 21, 
        
                     'blinding': blinding, 
        
                     'enctlv': p1enc}, 
        
                    {'id': p2, 
        
                     'amount_msat': Millisatoshi(1001), 
        
                     'delay': 15, 
        
                     # FIXME: this is a dummy! 
        
                     'channel': '0x0x0', 
        
                     'enctlv': p2enc}, 
        
                    {'id': p3, 
        
                     # FIXME: this is a dummy! 
        
                     'channel': '0x0x0', 
        
                     'amount_msat': Millisatoshi(1000), 
        
                     'delay': 9, 
        
                     'style': 'tlv'}]

thomash-acinq · 2021-11-29T13:02:52Z

This PR is compatible with ACINQ/eclair@59b4035, eclair and c-lightning can send and relay messages to each other.

rustyrussell · 2021-11-29T23:17:37Z

I think we have a list part to migrate in the new spec version, that it is test_pay.py

lightning/tests/test_pay.py

Lines 3284 to 3301 in 35c6f90

route = [{'id': l2.info['id'],

'channel': l1.get_channel_scid(l2),

'amount_msat': Millisatoshi(1002),

'delay': 21,

'blinding': blinding,

'enctlv': p1enc},

{'id': p2,

'amount_msat': Millisatoshi(1001),

'delay': 15,

# FIXME: this is a dummy!

'channel': '0x0x0',

'enctlv': p2enc},

{'id': p3,

# FIXME: this is a dummy!

'channel': '0x0x0',

'amount_msat': Millisatoshi(1000),

'delay': 9,

'style': 'tlv'}]

Yes. While HTLCs with blinded routing are not completely specced, they're pretty obvious. I'll see if it's better to implement or just disable for now...

Not all plugins depended on their headers. Keep it simple: all plugins depend on all plugin headers. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

Temporarily disable sendpay_blinding test which uses obsolete onionmsg; there's still some debate on the PR about how blinded HTLCs will work. Changelog-EXPERIMENTAL: onionmessage: removed support for v0.10.1 onion messages. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

Yes, we changed the spec again. Hopefully for the last time! Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

This is from 6e99c5feaf60cb797507d181fe583224309318e9 We renamed the enctlv field to encrypted_recipient_data in the spec, and the new onion_message is message 513. We don't handle it until the next patch. Two renames: 1. blinding_seed -> blinding_point. 2. enctlv -> encrypted_recipient_data. We don't do a compat cycle for our JSON APIs for these experimental features only used by our own plugins, we just rename. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

…dpath" RPC. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

It's very similar to the previous, but there are a few changes: 1. The enctlv fields are numbered differently. 2. The message itself is a different number. The onionmsg_path type is the same, however, so we keep that constant at least. The result is a lot of cut & paste, but we will delete the old one next release. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

And wire it through to the hook; update the plugins to recognize modern vs obs2 onions. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

…st vector. And include the final destination enctlv. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

This builds on the enctlv vectors, but actually goes all the way to creating a modern onionmessage. Thanks to Thomas H for corrections! Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

As noted by Joost. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

As of 2b923a0367c5f9154fcec706e3302cc4658dd889. Recurrence quotes need to be marked separately, since they're no longer in offers main bolt. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

We also move recurrence fields into a separate spec patch. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

This has been here for a while: self_id hangs around while we're calling the hook, but now it triggers sometimes. ``` E ValueError: E Node errors: E Global errors: E - Node /tmp/ltests-3mcyp67u/test_dev_rawrequest_1/lightning-1/ has memory leaks: [ E { E "backtrace": [ E "ccan/ccan/tal/tal.c:442 (tal_alloc_)", E "gossipd/gossipd_wiregen.c:528 (fromwire_gossipd_got_onionmsg_to_us)", E "lightningd/onion_message.c:152 (handle_onionmsg_to_us)", E "lightningd/gossip_control.c:137 (gossip_msg)", E "lightningd/subd.c:548 (sd_msg_read)", E "ccan/ccan/io/io.c:59 (next_plan)", E "ccan/ccan/io/io.c:407 (do_plan)", E "ccan/ccan/io/io.c:417 (io_ready)", E "ccan/ccan/io/poll.c:453 (io_loop)", E "lightningd/io_loop_with_timers.c:21 (io_loop_with_timers)", E "lightningd/lightningd.c:1164 (main)" E ], E "label": "gossipd/gossipd_wiregen.c:528:struct secret", E "parents": [ E "lightningd/onion_message.c:149:struct onion_message_hook_payload", E "lightningd/plugin_hook.c:81:struct hook_instance *[]" E ], E "value": "0x55cf3cbc9458" E } E ] ```

rustyrussell · 2021-11-30T03:06:34Z

Re-rebased on master.

vincenzopalazzo

ack afb91fd

rustyrussell requested a review from cdecker as a code owner November 18, 2021 02:38

rustyrussell force-pushed the guilt/onion-testvectors branch from 441a175 to ec64fd6 Compare November 18, 2021 04:07

vincenzopalazzo reviewed Nov 21, 2021

View reviewed changes

gossipd/test/run-onion_message.c Outdated Show resolved Hide resolved

vincenzopalazzo reviewed Nov 21, 2021

View reviewed changes

tests/test_pay.py Outdated Show resolved Hide resolved

vincenzopalazzo requested changes Nov 21, 2021

View reviewed changes

rustyrussell force-pushed the guilt/onion-testvectors branch 3 times, most recently from fde2140 to e480712 Compare November 28, 2021 00:45

thomash-acinq mentioned this pull request Nov 29, 2021

Relay onion messages ACINQ/eclair#2061

Merged

rustyrussell force-pushed the guilt/onion-testvectors branch from e480712 to 491e8d9 Compare November 29, 2021 23:49

rustyrussell mentioned this pull request Nov 30, 2021

lightningd: make shutdown smoother and safer #4897

Merged

1 task

rustyrussell added 14 commits November 30, 2021 13:35

plugins/Makefile: improve header dependencies.

499eee1

Not all plugins depended on their headers. Keep it simple: all plugins depend on all plugin headers. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

common: rename current onion message structures to obs2_.

419d723

Yes, we changed the spec again. Hopefully for the last time! Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

lightningd: return both obsolete and modern blindedpaths from "blinde…

c8ff5fe

…dpath" RPC. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

gossipd: handle receipt of modern onion message.

e546954

And wire it through to the hook; update the plugins to recognize modern vs obs2 onions. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

pytest: re-enable modern/obsolete fetchonion tests.

c6e0abb

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

test: hack in test for (now-obsolete!) test vector Thomas emailed me.

d698341

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

common/test/run-blindedpath_enctlv: use modern functions to create te…

d4261a0

…st vector. And include the final destination enctlv. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

test: make an onionmessage test vector.

beffb06

This builds on the enctlv vectors, but actually goes all the way to creating a modern onionmessage. Thanks to Thomas H for corrections! Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

features: don't announce onion_messages in bolt11 invoices.

9958f61

As noted by Joost. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

doc: clean up offers bolt quotes

aeeb853

As of 2b923a0367c5f9154fcec706e3302cc4658dd889. Recurrence quotes need to be marked separately, since they're no longer in offers main bolt. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

bolt12: remove deprecated chains fields

2df0eac

We also move recurrence fields into a separate spec patch. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

rustyrussell added 2 commits November 30, 2021 13:36

blinding: check against two of the test vectors in the PR.

50f6797

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

rustyrussell force-pushed the guilt/onion-testvectors branch from 491e8d9 to afb91fd Compare November 30, 2021 03:06

vincenzopalazzo approved these changes Nov 30, 2021

View reviewed changes

rustyrussell merged commit e2698c5 into ElementsProject:master Nov 30, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Onion Updates to Match Latest Spec (and test vectors!) #4921

Onion Updates to Match Latest Spec (and test vectors!) #4921

rustyrussell commented Nov 18, 2021 •

edited

Loading

vincenzopalazzo left a comment

vincenzopalazzo left a comment

rustyrussell commented Nov 28, 2021

vincenzopalazzo commented Nov 28, 2021

thomash-acinq commented Nov 29, 2021

rustyrussell commented Nov 29, 2021

rustyrussell commented Nov 30, 2021

vincenzopalazzo left a comment

Onion Updates to Match Latest Spec (and test vectors!) #4921

Onion Updates to Match Latest Spec (and test vectors!) #4921

Conversation

rustyrussell commented Nov 18, 2021 • edited Loading

vincenzopalazzo left a comment

Choose a reason for hiding this comment

vincenzopalazzo left a comment

Choose a reason for hiding this comment

rustyrussell commented Nov 28, 2021

vincenzopalazzo commented Nov 28, 2021

thomash-acinq commented Nov 29, 2021

rustyrussell commented Nov 29, 2021

rustyrussell commented Nov 30, 2021

vincenzopalazzo left a comment

Choose a reason for hiding this comment

rustyrussell commented Nov 18, 2021 •

edited

Loading