wasm-client: upgrade to latest master litd version
#114
Conversation
This allows the wasm client to avoid importing the entire LNC repo. The wasm client, located within the LNC repo, depends on an older version of LNC. That version imported taproot-assets, which caused a symbol overflow and prevented the wasm client from compiling. However, the wasm client only requires the gbn and mailbox packages, which do not depend on taproot-assets. By turning these packages into separate modules, they can be used by the wasm client without pulling in all of LNC. This also allows LNC to continue using taproot-assets while reducing the wasm client's dependency scope. Also, bump LND.
This commit updates wasm-client dependencies to avoid linking the entire taproot-assets package, which previously caused a symbol overflow and broke the build. The client now uses the latest versions of litrpc, lit permissions, taprpc, and the newly modularized LNC packages to reduce dependency scope and enable successful compilation.
|
I will fix the linter. |
Fixes the following error: mailbox/client_conn.go:551:25: non-constant format string in call to fmt.Errorf.
| @@ -0,0 +1,36 @@ | |||
| module github.com/lightninglabs/lightning-node-connect/gbn | |||
There was a problem hiding this comment.
I think we might get away without this commit and make our lives a bit easier in the future.
Doing a quick search, it looks like we only use the litclient and perms packages from the lightning-terminal repository. And both of these are now their own modules.
So requiring the whole lightning-node-connect package from the WASM client should no longer pull in all of litd.
There was a problem hiding this comment.
This commit also gets rid of the github.com/lightninglabs/lightning-node-connect v0.3.3-alpha.0.20250303090341-f823ad04b9de dep from WASM-client. We could just replace to the relative parent directory for lnc. But it will block us from importing taproot-assets into any part of LNC in the future.
There was a problem hiding this comment.
Really awesome work with this @ffranr!! Thank you so much for addressing the issue 🎉
LGTM if you choose to ignore the /mobile/go.mod comment. If you do, let me know and I'll update my review to an ACK :)
| github.com/jessevdk/go-flags v1.4.0 | ||
| github.com/lightninglabs/faraday v0.2.13-alpha | ||
| github.com/lightninglabs/lightning-node-connect v0.3.3-alpha.0.20250303090341-f823ad04b9de | ||
| github.com/lightninglabs/lightning-terminal v0.14.0-alpha |
There was a problem hiding this comment.
I'm just a little worried that we'd end up missing protos that's part of litd but not litrpc in the future, but I think the looprpc and pool dependencies here should cover those. But we just need to be careful in the future, and ensure we keep all of them updated.
cmd/wasm-client/go.mod
Outdated
| github.com/lightninglabs/lightning-terminal/litrpc v1.0.2-0.20250506112654-08095d30dd1a | ||
| github.com/lightninglabs/lightning-terminal/perms v0.0.0-20250506112654-08095d30dd1a | ||
| github.com/lightninglabs/loop/looprpc v1.0.7 | ||
| github.com/lightninglabs/pool v0.6.5-beta.0.20250305125211-4e860ec4e77f |
There was a problem hiding this comment.
Should this be poolrpc :)?
There was a problem hiding this comment.
Good question! For consistency I will move perms in pool into poolrpc. And then this wasm-client dep can just be poolrpc.
There was a problem hiding this comment.
Created PR: lightninglabs/pool#503
We could further reduce wasm client symbol count with this change, but I suppose it's not strictly necessary right now.
There was a problem hiding this comment.
Awesome, thanks 🚀!
| @@ -217,4 +217,4 @@ replace google.golang.org/protobuf => github.com/lightninglabs/protobuf-go-hex-d | |||
| // replace can be removed in favour of importing that. | |||
| replace github.com/lightninglabs/faraday => github.com/lightninglabs/faraday v0.2.13-alpha.0.20241118202659-a3aba5b7ea49 | |||
|
|
|||
| go 1.22.6 | |||
| go 1.24.2 | |||
There was a problem hiding this comment.
Leaving a comment here for this file in general:
We should update the dependencies in this file similar to how you've updated them in the wasm client, as we need both of them to use the same dependencies during a release. But, feel free to ignore this feedback though, as I'll fix it during the release PR if you do :)
There was a problem hiding this comment.
Good to know. I think for context sake it probably makes sense to do that in the release PR.
I'm not sure how you'd like to deal with the github.com/lightninglabs/lightning-node-connect v0.3.3-alpha.0.20250303090341-f823ad04b9de dependency for example, now that wasm-client is using relative path to LNC modules.
And I'm not sure if this TODO can be removed:
// TODO: uncomment once LiT has been updated to point to the latest version of
// LNC and LND.
// replace github.com/lightninglabs/lightning-node-connect => ../../
There was a problem hiding this comment.
Ok thanks, I'll look into that for the release PR instead!
// TODO: uncomment once LiT has been updated to point to the latest version of
// LNC and LND.
// replace github.com/lightninglabs/lightning-node-connect => ../../
No, we intentionally kept this, as this was recently added. See this comment for background to why we have this TODO:
#112 (comment)
|
I'm going to update the wasm-client pool dependency, and then i think this should be ready to merge. |
Nice, thanks a lot! |
Modify wasm-client to depend on poolrpc directly rather than the full pool package. This change reduces the number of symbols included in the WASM build, leading to a smaller output size.
Closes #113
Upgrade dependencies: lnd, lit, and taprpc.
Also upgrades to golang to 1.24.2.
Modularize LNC packages
gbnandmailboxto reduce the import scope for the WASM client.Fixes a WASM client build failure caused by symbol overflow due to linking taproot-assets.