cln: update to v23.08 to allow self payments

[vincenzopalazzo]

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

[codecov[bot]]

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

```diff @@ 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](https://app.codecov.io/gh/fedimint/fedimint/pull/2960?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=fedimint) | Coverage Δ | | |---|---|---| | [gateway/ln-gateway/src/bin/cln\_extension.rs](https://app.codecov.io/gh/fedimint/fedimint/pull/2960?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=fedimint#diff-Z2F0ZXdheS9sbi1nYXRld2F5L3NyYy9iaW4vY2xuX2V4dGVuc2lvbi5ycw==) | `0.00% <0.00%> (ø)` | | ... and [7 files with indirect coverage changes](https://app.codecov.io/gh/fedimint/fedimint/pull/2960/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=fedimint)

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[m1sterc001guy]

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]

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

[vincenzopalazzo]

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]

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:

https://github.com/fedimint/fedimint/blob/290472841f38f8ad165d0d5906fc5625840d499b/gateway/ln-gateway/src/lnd.rs#L292-L357

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]

@vincenzopalazzo @elsirion There's also CARGO_BUILD_JOBS

[vincenzopalazzo]

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

[elsirion]

Thx a lot, great job! CI is currently a bit flaky and also broken from the GitHub side.

[elsirion]

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

[vincenzopalazzo]

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]

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]

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]

@dpc could you look into this issue? https://github.com/fedimint/fedimint/pull/2960#issuecomment-1743490693. 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]

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]

https://github.com/fedimint/fedimint/pull/3336

[vincenzopalazzo]

Thanks, when it is merged https://github.com/fedimint/fedimint/pull/3336#pullrequestreview-1662639160 I will rebase on it

[dpc]

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

[vincenzopalazzo]

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

[dpc]

just format it

[dpc]

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.