cln: update to v23.08 to allow self payments #2960

vincenzopalazzo · 2023-08-15T13:48:30Z

This include the recent version of core lightning to
allow self-payments.

codecov · 2023-08-15T14:03:00Z

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (0a67d55) 59.76% compared to head (daa3292) 59.77%.
Report is 5 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2960      +/-   ##
==========================================
+ Coverage   59.76%   59.77%   +0.01%     
==========================================
  Files         197      197              
  Lines       41149    41147       -2     
==========================================
+ Hits        24593    24596       +3     
+ Misses      16556    16551       -5

Files	Coverage Δ
gateway/ln-gateway/src/bin/cln_extension.rs	`0.00% <0.00%> (ø)`

... and 7 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

m1sterc001guy · 2023-08-24T15:05:50Z

This should be relatively straightforward if you want to take a shot at writing a test. See test_gateway_pay_valid_invoice in gateway/ln_gateway/src/ng/tests.rs as an example of a test that uses fedimint to pay an invoice.

The main thing that you'll need to change, is that instead of generating an invoice with other_lightning_client, you'll need to generate the invoice with CLN and then try to pay it with fedimint when CLN is set as the lightning gateway.

elsirion · 2023-08-25T07:32:26Z

This breaks our CI, probably some CLN command that changed.

vincenzopalazzo · 2023-08-25T08:55:58Z

This should be relatively straightforward if you want to take a shot at writing a test. See test_gateway_pay_valid_invoice in gateway/ln_gateway/src/ng/tests.rs as an example of a test that uses fedimint to pay an invoice.

Thanks to point me in the correct file, but test_gateway_pay_valid_invoice is not there.

The main thing that you'll need to change, is that instead of generating an invoice with other_lightning_client, you'll need to generate the invoice with CLN and then try to pay it with fedimint when CLN is set as the lightning gateway.

I see, I assume that CLN in this case is the user_client?

The use case that I would like to test is generate an invoice with fedi -> pay with user client

This breaks our CI, probably some CLN command that changed.

Yeah, I am not able to reproduce on my machine because my remote build machine apparently will OOM :/

elsirion · 2023-08-25T10:08:14Z

I think you can reproduce one (of potentially more) problems in just mprocs. It takes a ridiculous time to get read, apparently because the LND node doesn't return any route hints and lnd-gw keeps retrying.

2023-08-25T09:46:53.870439Z  INFO connect_fed: ln_gateway: LN node returned no route hints, trying again in 2s num_retries=16

After 30 retries it starts without route hints, leading to the user shell becoming read after over 160s. When checking the output of fedimint-cli list-gateways one of them doesn't have any route hints as predicted by this theory.

On master both the CLN and the LND gateway register quickly and have route hints. Is it possible that some CLN change lead to LND not recognizing a channel as active? The route hint filtering for LND is implemented here:

fedimint/gateway/ln-gateway/src/lnd.rs

Lines 292 to 357 in 2904728

    
           async fn routehints(&self) -> Result<GetRouteHintsResponse, LightningRpcError> { 
        
               let mut client = Self::connect( 
        
                   self.address.clone(), 
        
                   self.tls_cert.clone(), 
        
                   self.macaroon.clone(), 
        
               ) 
        
               .await?; 
        
               let channels = client 
        
                   .lightning() 
        
                   .list_channels(ListChannelsRequest { 
        
                       active_only: true, 
        
                       inactive_only: false, 
        
                       public_only: false, 
        
                       private_only: false, 
        
                       peer: vec![], 
        
                   }) 
        
                   .await 
        
                   .map_err(|status| LightningRpcError::FailedToGetRouteHints { 
        
                       failure_reason: format!("Failed to list channels {status:?}"), 
        
                   })? 
        
                   .into_inner(); 
        
               let mut route_hints: Vec<RouteHint> = vec![]; 
        
               for chan in channels.channels { 
        
                   let info = client 
        
                       .lightning() 
        
                       .get_chan_info(ChanInfoRequest { 
        
                           chan_id: chan.chan_id, 
        
                       }) 
        
                       .await 
        
                       .map_err(|status| LightningRpcError::FailedToGetRouteHints { 
        
                           failure_reason: format!("Failed to get channel info {status:?}"), 
        
                       })? 
        
                       .into_inner(); 
        
                   let policy = match info.node1_policy.clone() { 
        
                       Some(policy) => policy, 
        
                       None => continue, 
        
                   }; 
        
                   let src_node_id = PublicKey::from_str(&chan.remote_pubkey) 
        
                       .unwrap() 
        
                       .serialize() 
        
                       .to_vec(); 
        
                   let short_channel_id = chan.chan_id; 
        
                   let base_msat = policy.fee_base_msat as u32; 
        
                   let proportional_millionths = policy.fee_rate_milli_msat as u32; 
        
                   let cltv_expiry_delta = policy.time_lock_delta; 
        
                   let htlc_maximum_msat = Some(policy.max_htlc_msat); 
        
                   let htlc_minimum_msat = Some(policy.min_htlc as u64); 
        
                   let route_hint_hop = RouteHintHop { 
        
                       src_node_id, 
        
                       short_channel_id, 
        
                       base_msat, 
        
                       proportional_millionths, 
        
                       cltv_expiry_delta, 
        
                       htlc_minimum_msat, 
        
                       htlc_maximum_msat, 
        
                   }; 
        
                   route_hints.push(RouteHint { 
        
                       hops: vec![route_hint_hop], 
        
                   }); 
        
               } 
        
               Ok(GetRouteHintsResponse { route_hints }) 
        
           }

If you are running out of memory when compiling for just mprocs you can limit cargo's parallelism by applying the following patch:

diff --git a/scripts/build.sh b/scripts/build.sh
index 2dfae4e581..374e43f06f 100755
--- a/scripts/build.sh
+++ b/scripts/build.sh
@@ -38,6 +38,6 @@ cd $SRC_DIR || exit 1
 # Note: Respect 'CARGO_PROFILE' that crane uses
 
 if [ -z "${SKIP_CARGO_BUILD:-}" ]; then
-  cargo build --workspace --all-targets ${CARGO_PROFILE:+--profile ${CARGO_PROFILE}}
+  cargo build -j 2 --workspace --all-targets ${CARGO_PROFILE:+--profile ${CARGO_PROFILE}}
 fi
 export PATH="$PWD/target/${CARGO_PROFILE:-debug}:$PATH"

dpc · 2023-08-25T17:17:42Z

@vincenzopalazzo @elsirion There's also CARGO_BUILD_JOBS

vincenzopalazzo · 2023-09-01T23:58:16Z

Ok this should be ready now! Just waiting for the CI

gateway/ln-gateway/src/bin/cln_extension.rs

elsirion · 2023-09-07T15:21:26Z

gateway/ln-gateway/src/bin/cln_extension.rs

@@ -65,8 +65,7 @@ async fn main() -> Result<(), anyhow::Error> {
        .serve_with_shutdown(listen, async {
            // Wait for plugin to signal it's shutting down
            // Shut down everything else via TaskGroup regardless of error
-            let result = plugin.join().await;
-            assert!(result.is_ok(), "{:?}", result);
+            let _ = plugin.join().await;


Not sure if expecting here would be useful or not. I'm ok with it. If anything panics we should normally see it anyway.

elsirion · 2023-09-07T15:22:05Z

Just saw that the changes were single fixup commits, could you squash them please?

vincenzopalazzo · 2023-09-07T16:21:15Z

Just saw that the changes were single fixup commits, could you squash them please?

Yeah sorry I was just collecting all the review and the squash them

Now should be done :)

justinmoon · 2023-10-02T17:59:15Z

When I run just mprocs on this branch I get the following in the cln tab:

lightningd: Config file /var/folders/19/h_y32l8s1lxfrpxctd1673p00000gn/T/nix-shell.wNptoj/fm-INjH/cln/config line 2: bitcoin-rpcuser=bitcoin: unknown option

Also, I notice that flake.nix is trying to pin the lightningd version to 23.08 but in the terminal I'm seeing:

$ lightningd --version
v23.05

Perhaps this change isn't having the desired effect ...

vincenzopalazzo · 2023-10-02T18:03:18Z

Yeah, this was something that I was looking into.

We could drop https://github.com/fedimint/fedimint/pull/2960/files#diff-206b9ce276ab5971a2489d75eb1b12999d4bf3843b7988cbe8d687cfde61dea0R66-R73 because cln nix package get updated (?)

I will try to drop it and made a rebase now

When I run just mprocs on this branch I get the following in the cln tab:

Regarding this, I am not sure about it the fedi flake nix is quite complex to understand what it is going on, sorry

justinmoon · 2023-10-04T18:48:50Z

@dpc could you look into this issue? #2960 (comment). It doesn't seem like the patch lightningd is actually showing up at all. We expect lightningd --version to say v23.08 but it says v23.05.

dpc · 2023-10-04T20:14:52Z

When I run just mprocs on this branch I get the following in the cln tab:
lightningd: Config file /var/folders/19/h_y32l8s1lxfrpxctd1673p00000gn/T/nix-shell.wNptoj/fm-INjH/cln/config line 2: bitcoin-rpcuser=bitcoin: unknown option
Also, I notice that flake.nix is trying to pin the lightningd version to 23.08 but in the terminal I'm seeing:
$ lightningd --version
v23.05
Perhaps this change isn't having the desired effect ...

Instead of having it as a separate package, try overwriting the existing one in pkgs. to make sure all clightnings everywhere will be patched.

I was planning to do it anyway. Otherwise only places that refer to this variable get this patched binary. It could be that where you're trying it is actually using pkgs.clightning and not this variable.

dpc · 2023-10-06T17:34:52Z

#3336

vincenzopalazzo · 2023-10-06T19:03:20Z

Thanks, when it is merged #3336 (review) I will rebase on it

dpc · 2023-10-06T19:18:33Z

Please @ me in case of issue, or even ping me on Discord.

vincenzopalazzo · 2023-10-06T19:27:01Z

Rebased now to see what the CI tell us, thanks for the fix

flake.nix

dpc · 2023-10-06T19:34:10Z

just format it

This include the recent version of core lightning to allow self-payments. Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>

dpc · 2024-03-15T05:26:04Z

Last time I tried updating tests were failing. I also remember manmeet pinning cln-rpc because it was introducing backward-incompat changes.

What do we do about it?

I'm closing as it just lags in the PR backlog. If anyone feels like moving it forward, feel free to reopen.

vincenzopalazzo force-pushed the macros/cln-self-pay branch from 9a3fc38 to f9eac24 Compare August 15, 2023 13:52

vincenzopalazzo force-pushed the macros/cln-self-pay branch from f9eac24 to 369c8c1 Compare August 24, 2023 10:12

vincenzopalazzo changed the title ~~wip: working for a self payments~~ cln: update to v23.08 to allow self payments Aug 24, 2023

vincenzopalazzo marked this pull request as ready for review August 24, 2023 10:16

vincenzopalazzo requested a review from a team as a code owner August 24, 2023 10:16

vincenzopalazzo force-pushed the macros/cln-self-pay branch from 369c8c1 to 95238b5 Compare August 24, 2023 10:16

dpc previously approved these changes Aug 24, 2023

View reviewed changes

elsirion mentioned this pull request Aug 26, 2023

Document preventing out-of-memory when compiling #3036

Merged

vincenzopalazzo force-pushed the macros/cln-self-pay branch from 95238b5 to edb2525 Compare August 28, 2023 08:46

vincenzopalazzo dismissed dpc’s stale review via 6dae3b0 August 28, 2023 11:09

vincenzopalazzo requested review from a team as code owners August 28, 2023 11:09

vincenzopalazzo force-pushed the macros/cln-self-pay branch 4 times, most recently from 7a57934 to de71c13 Compare September 1, 2023 23:24

vincenzopalazzo requested a review from dpc September 1, 2023 23:24

vincenzopalazzo force-pushed the macros/cln-self-pay branch from de71c13 to 47d6cae Compare September 1, 2023 23:57

dpc reviewed Sep 3, 2023

View reviewed changes

gateway/ln-gateway/src/bin/cln_extension.rs Outdated Show resolved Hide resolved

vincenzopalazzo force-pushed the macros/cln-self-pay branch 4 times, most recently from 3043131 to 1242052 Compare September 4, 2023 09:26

elsirion disabled auto-merge September 7, 2023 15:19

elsirion reviewed Sep 7, 2023

View reviewed changes

vincenzopalazzo force-pushed the macros/cln-self-pay branch from 8fc6b60 to 0d39e56 Compare September 7, 2023 16:20

elsirion approved these changes Sep 8, 2023

View reviewed changes

elsirion previously approved these changes Sep 8, 2023

View reviewed changes

elsirion enabled auto-merge September 8, 2023 10:42

dpc self-assigned this Sep 21, 2023

auto-merge was automatically disabled October 2, 2023 19:21
Head branch was pushed to by a user without write access

vincenzopalazzo dismissed elsirion’s stale review via 83aabf8 October 2, 2023 19:21

vincenzopalazzo force-pushed the macros/cln-self-pay branch from 0d39e56 to 83aabf8 Compare October 2, 2023 19:21

vincenzopalazzo force-pushed the macros/cln-self-pay branch from 83aabf8 to c9c12b1 Compare October 6, 2023 19:26

dpc reviewed Oct 6, 2023

View reviewed changes

flake.nix Outdated Show resolved Hide resolved

vincenzopalazzo force-pushed the macros/cln-self-pay branch 2 times, most recently from 3759a9c to cf244ba Compare October 6, 2023 19:32

vincenzopalazzo force-pushed the macros/cln-self-pay branch from cf244ba to a90beab Compare October 6, 2023 20:12

cln: update to v23.08 to allow self payments

daa3292

This include the recent version of core lightning to allow self-payments. Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>

vincenzopalazzo force-pushed the macros/cln-self-pay branch from a90beab to daa3292 Compare October 6, 2023 20:57

dpc closed this Mar 15, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cln: update to v23.08 to allow self payments #2960

cln: update to v23.08 to allow self payments #2960

vincenzopalazzo commented Aug 15, 2023 •

edited

codecov bot commented Aug 15, 2023 •

edited

m1sterc001guy commented Aug 24, 2023

elsirion commented Aug 25, 2023

vincenzopalazzo commented Aug 25, 2023

elsirion commented Aug 25, 2023

dpc commented Aug 25, 2023

vincenzopalazzo commented Sep 1, 2023 •

edited

elsirion Sep 7, 2023

elsirion commented Sep 7, 2023

vincenzopalazzo commented Sep 7, 2023

justinmoon commented Oct 2, 2023 •

edited

vincenzopalazzo commented Oct 2, 2023 •

edited

justinmoon commented Oct 4, 2023

dpc commented Oct 4, 2023 •

edited

dpc commented Oct 6, 2023

vincenzopalazzo commented Oct 6, 2023

dpc commented Oct 6, 2023

vincenzopalazzo commented Oct 6, 2023

dpc commented Oct 6, 2023

dpc commented Mar 15, 2024

cln: update to v23.08 to allow self payments #2960

cln: update to v23.08 to allow self payments #2960

Conversation

vincenzopalazzo commented Aug 15, 2023 • edited

codecov bot commented Aug 15, 2023 • edited

Codecov Report

m1sterc001guy commented Aug 24, 2023

elsirion commented Aug 25, 2023

vincenzopalazzo commented Aug 25, 2023

elsirion commented Aug 25, 2023

dpc commented Aug 25, 2023

vincenzopalazzo commented Sep 1, 2023 • edited

elsirion Sep 7, 2023

Choose a reason for hiding this comment

elsirion commented Sep 7, 2023

vincenzopalazzo commented Sep 7, 2023

justinmoon commented Oct 2, 2023 • edited

vincenzopalazzo commented Oct 2, 2023 • edited

justinmoon commented Oct 4, 2023

dpc commented Oct 4, 2023 • edited

dpc commented Oct 6, 2023

vincenzopalazzo commented Oct 6, 2023

dpc commented Oct 6, 2023

vincenzopalazzo commented Oct 6, 2023

dpc commented Oct 6, 2023

dpc commented Mar 15, 2024

vincenzopalazzo commented Aug 15, 2023 •

edited

codecov bot commented Aug 15, 2023 •

edited

vincenzopalazzo commented Sep 1, 2023 •

edited

justinmoon commented Oct 2, 2023 •

edited

vincenzopalazzo commented Oct 2, 2023 •

edited

dpc commented Oct 4, 2023 •

edited