Skip to content
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

Offers compat fixes #5892

Merged

Conversation

rustyrussell
Copy link
Contributor

This addresses all but one (sending to a blinded path which starts at ourselves) of the issues reported by @t-bast in compat testing with Eclair in #5823

Unfortunately, it breaks compatibility again :( But omelette, eggs and engineering time...

…alse'" warning

```
lightningd/jsonrpc.c: In function ‘destroy_json_command’:
lightningd/jsonrpc.c:1180:63: error: the comparison will always evaluate as ‘false’ for the address of ‘canary’ will never be NULL [-Werror=address]
lightningd/jsonrpc.c:108:53: note: ‘canary’ declared here
```

Signed-off-by: Rusty Russell <[email protected]>
…ion-test.json

This was reported by @valentinewallace: Dave would only use padding to
make all his own encrypted_recipient_data equal-length.  We did it
across the entire path, which includes the hop added by Alice, which
Dave wouldn't know about.

Signed-off-by: Rusty Russell <[email protected]>
See: ElementsProject#5823
Reported-by: @t-bast
Signed-off-by: Rusty Russell <[email protected]>
Changelog-EXPERIMENTAL: `offers` breaking blinded payments change (update_add_tlvs fix, Eclair compat)
Thanks valgrind!

Signed-off-by: Rusty Russell <[email protected]>
You can use rs->nextcase, but we don't always keep that around, so
keep a flag in onion_payload.

We'll use this in the "do we need to return a blinded error code"
patch.

Signed-off-by: Rusty Russell <[email protected]>
```
make check-source-bolt CHECK_BOLT_PREFIX="--prefix=BOLT-route-blinding" BOLTVERSION=guilt/offers
```

Other than textual changes, this does:

1. Ensures we put total_amount_msat in onion final hop (reported by @t-bast).
2. Require that they put total_amount_msat in onion final hop.
3. Return `invalid_onion_blinding` exactly as defined by the spec (i.e. less
   aggressive when we're the final hop) (also reported by @t-bast, but I already
   knew).

See: ElementsProject#5823
Signed-off-by: Rusty Russell <[email protected]>
Changelog-EXPERIMENTAL: `offers` breaking blinded payments change (total_amount_sat required, Eclair compat)
Otherwise it's skipped!

Signed-off-by: Rusty Russell <[email protected]>
```
make check-source-bolt CHECK_BOLT_PREFIX="--prefix=BOLT-onion-message" BOLTVERSION=guilt/offers
```

Mainly textual, though I neatened the extra fields check for TLVs with
blinding, and implemented the "no other fields" requirement for
non-final onion message hops.

Signed-off-by: Rusty Russell <[email protected]>
```
make check-source-bolt CHECK_BOLT_PREFIX="--prefix=BOLT-offers" BOLTVERSION=guilt/offers
```

In this case, only trivial mods.

Signed-off-by: Rusty Russell <[email protected]>
Our pay code handles this correctly, but decode was still using an old model
where there was a payinfo per hop, not per path.

Reported-by: @t-bast
See: ElementsProject#5823
Signed-off-by: Rusty Russell <[email protected]>
@rustyrussell rustyrussell added Optech Make Me Famous! Look! Look! Look! COOL NEW FEATURE! offers labels Jan 12, 2023
@rustyrussell rustyrussell added this to the v23.02 milestone Jan 12, 2023
@t-bast
Copy link

t-bast commented Jan 12, 2023

Neat, I'll do another round of compat' testing based on that branch soon!

@t-bast
Copy link

t-bast commented Jan 18, 2023

You mentioned that cln would create blinded hops when its local channels are private, but that doesn't seem to work.

With a basic setup eclair -> cln and a single private channel between the two nodes, cln still doesn't create a blinded hop and doesn't create a dummy blinded hop either, it only provides a payload for itself:

lni1qqgz38r72neywf20tda3m7tzpt86uq3qqc3xu3s3rg94nj40zfsy866mhu5vxne6tcej5878k2mneuvgjy8s5predakx793pqwxsq2ghkn7kau0tnysud6a792x97nng6w7c6fdwe3q69npsl4wvv5pqqc3xu3s3rg94nj40zfsy866mhu5vxne6tcej5878k2mneuvgjy84yqucj6q9sggrl9y0xgru2exyum5mrqa8m65khp4ntk77d3pa8vdfcw0kj9edzwy6pxqr35qzj9a5l4h0r6uey8rwh0323305u6xnhkxjttkvgx3vcv8atnrqy2r7gwahzupmlwstx6e4suceq33nst0agmr9smlaceq7q493j73wqyp77sc7clwgkhpfsnkxkkeesu37gvmmcpr5lq4q3p0072uxpc2lutsqxffhtyzhffzfrg2hms6qe3wvax3lxpl5ykal0mef50jxtt7s70tpn6mglc3hz490qsv4vzjt93dtpsq7823pcqqqqqqqqqqqqqqq5qqqqqqqqqqqqqwjfvkl43fqqqqqqzjqgc78cnf2sgrkj9490vt6gw76xnv3wqscngv905w9uurd38pav5zttces4mruqj4q8xykszczzqudqq530d8admc7hxfpcm4mu25vta8x35aa35j6anzp5txrpl2ucmcyq3svsnd4hxn39dtwff8fugqutcet5talpxr9nqfqthqrr8pq88c4v3szkrlslruwypfzhepd8phvls782fq7pj2zyge3ywe3m5jc9sjq
{
   "type": "bolt12 invoice",
   "offer_id": "78362da27528af131bb63c9af89c544d1b94206040de3a87bf1f05b43b4c2d72",
   "offer_chains": [
      "06226e46111a0b59caaf126043eb5bbf28c34f3a5e332a1fc7b2b73cf188910f"
   ],
   "offer_description": "yolo",
   "offer_node_id": "038d002917b4fd6ef1eb9921c6ebbe2a8c5f4e68d3bd8d25aecc41a2cc30fd5cc6",
   "invreq_metadata": "289c7e54f247254f5b7b1df9620acfae",
   "invreq_payer_id": "03f948f3207c564c4e6e9b183a7dea96b86b35dbde6c43d3b1a9c39f69172d1389",
   "invreq_chain": "06226e46111a0b59caaf126043eb5bbf28c34f3a5e332a1fc7b2b73cf188910f",
   "invreq_amount_msat": "10000000msat",
   "invoice_paths": [
      {
         "first_node_id": "038d002917b4fd6ef1eb9921c6ebbe2a8c5f4e68d3bd8d25aecc41a2cc30fd5cc6",
         "blinding": "02287e43bb71703bfba0b36b35873190463382dfd46c6586ffdc641e054b197a2e",
         "payinfo": {
            "fee_base_msat": "0msat",
            "fee_proportional_millionths": 0,
            "cltv_expiry_delta": 10,
            "features": ""
         },
         "path": [
            {
               "blinded_node_id": "03ef431ec7dc8b5c2984ec6b5b398723e4337bc0474f82a0885eff2b860e15fe2e",
               "encrypted_recipient_data": "537590574a4491a157dc340cc5cce9a3f307f425bbf7ef29a3e465afd0f3d619eb68fe237154af0419560a4b2c5ab0c01e3a"
            }
         ]
      }
   ],
   "invoice_created_at": 1674036434,
   "invoice_relative_expiry": 7200,
   "invoice_payment_hash": "76916a57b17a43bda34d91702189a1857d1c5e706d89c3d6504b5e330aec7c04",
   "invoice_amount_msat": "10000000msat",
   "invoice_node_id": "038d002917b4fd6ef1eb9921c6ebbe2a8c5f4e68d3bd8d25aecc41a2cc30fd5cc6",
   "signature": "460c84db5b9a712b56e4a4e9e201c5e32ba2fbf09865981205dc0319c2039f1564602b0ff0f8f8e20522be42d386ecfc3c75241e0c9422233123b31dd2582c24",
   "valid": true
}

There are some test cases I'm thus unable to run, is there an API argument I'm missing that would unblock this?

Another issue I found is that cln doesn't seem to support splitting an outgoing payment across blinded paths?
I'm creating this graph:

                     100 000 sat   +-----+   25 000 sat
            +----------------------| Bob |-----------------------+
            |                      +-----+                       |
 50 000 sat |                                                    | 125 000 sat
            |                                                    |
        +-------+                                             +------+
        | Alice |                                             | Dave |
        +-------+                                             +------+
            |                                                    |
 50 000 sat |                                                    | 125 000 sat
            |                     +-------+                      |
            +---------------------| Carol |----------------------+
                    100 000 sat   +-------+   25 000 sat

Alice then creates a bolt 12 invoice for 150 000 sats with two blinded paths:

  • Bob -> Alice with capacity 100 000 sats
  • Carol -> Alice with capacity 100 000 sats

Dave immediately fails paying it with "no path found"...

Apart from that, the issues highlighted in #5823 seem to be fixed 🎉

@rustyrussell
Copy link
Contributor Author

rustyrussell commented Jan 30, 2023

Ah, this gets overridden by the fact that CLN knows the Eclair node is a dead-end, so won't create a hop (this shares the logic we use for route hints, where it makes more sense). If there's another node attached to the Eclair node, it will work! (with a public channel, obv)

@rustyrussell rustyrussell merged commit 2dec805 into ElementsProject:master Jan 30, 2023
@carlaKC
Copy link

carlaKC commented Feb 1, 2023

Ah, this gets overridden by the fact that CLN knows the Eclair node is a dead-end, so won't create a hop (this shares the logic we use for route hints, where it makes more sense). If there's another node attached to the Eclair node, it will work! (with a public channel, obv)

Running into a different issue trying to fetch an offer invoice with a blinded route on 12761c3:
Alice --- Bob --- Carol --(private)---Dave

lightning-cli-dave offer 1000 test
lightnig-cli-alice fetchinvoice lno...

Dave has dev-fast-gossip enabled, and listchannels has an up to date network view.
I can retrieve the invoice but it doesn't have a blinded path:

2023-02-01T14:18:45.096Z **BROKEN** plugin-offers: Could not parse listincoming: Parsing '{id:%,incoming_capacity_msat:%,htlc_min_msat:%,htlc_max_msat:%,fee_base_msat:%': json_to_u32 could not parse \"1000msat\"
2023-02-01T14:18:45.096Z UNUSUAL plugin-offers: No incoming channel for 1000msat, so no blinded path

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
offers Optech Make Me Famous! Look! Look! Look! COOL NEW FEATURE!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants