-
Notifications
You must be signed in to change notification settings - Fork 3
deps: update dependencies 12-15-25
#50
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
Conversation
9e2f7ed to
6e87319
Compare
ViktorT-11
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me 🔥, thanks 🙏!
We usually include an extra commit that bumps the version of lnc-core though, and then before merging it, we test that it works properly with lnc-web with yarn link.
Should we do the normal approach, or merge this as is first?
| "name": "@lightninglabs/lnc-core", | ||
| "version": "0.3.4-alpha", | ||
| "description": "Type definitions and utilities for Lightning Node Connect", | ||
| "main": "./dist/index.js", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We usually include a commit that bumps the version above as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bumped!
jamaljsr
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Appreciate you taking a stab at this. It's better if we all are familiar with the process to update these packages.
I just happened to notice that there was a new proto added to tapd in the latest release that needs to be added to our scripts:
https://github.com/lightninglabs/taproot-assets/blob/main/taprpc/authmailboxrpc/mailbox.proto
I then checked the other daemons and found a couple in lnd that we never added in the past. Would you mind updating the scripts to include these?
https://github.com/lightningnetwork/lnd/blob/master/lnrpc/peersrpc/peers.proto
https://github.com/lightningnetwork/lnd/blob/master/lnrpc/stateservice.proto
I ran yarn update-protos && yarn generate which produced an error.
tapcommon.proto: File not found.
taprootassets.proto:3:1: Import "tapcommon.proto" was not found or had errors.
taprootassets.proto:246:5: "OutPoint" is not defined.
taprootassets.proto:1452:5: "OutPoint" is not defined.
taprootassets.proto:1934:14: "taprpc.OutPoint" is not defined.
taprootassets.proto:1953:5: "taprpc.OutPoint" is not defined.
It looks like tapcommon.proto was never added to our scripts. Can you add this as well.
As @ViktorT-11 mentioned, it would be helpful to pair this with an lnc-web PR so that we can easily test this with the new LNC WASM version.
6e87319 to
c7287d7
Compare
|
@ViktorT-11 @jamaljsr I've created an lnc-web PR here: Note that CI is failing, as the tag/release has not been created yet. Typically do we create the PR after the tag is cut? I was able to run a Thanks for pointing out the missing protos @jamaljsr! I've updated this PR to include those. Might it make sense to add a |
jamaljsr
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that CI is failing, as the tag/release has not been created yet. Typically do we create the PR after the tag is cut? I was able to run a
yarn link && yarn buildstep withinlnc-webwithout issues.
I left a comment on that PR. Still one more update to make to the code there.
lightninglabs/lnc-web#139 (comment)
Thanks for pointing out the missing protos @jamaljsr! I've updated this PR to include those. Might it make sense to add a
yarn update-protos && yarn generatein CI to catch this in the future?
Yup, that's a great idea to avoid missing this in the future.
|
@jamaljsr think we are ready for another round -- see lightninglabs/lnc-web#139 (comment) I will update the dependencies / lockfile on |
jamaljsr
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tACK LGTM 🚀
Works with linked lnc-web and Terminal.
Summary
Update daemon proto versions:
lnd: v0.19.1-beta → v0.20.0-betatapd: v0.6.0 → v0.7.0lit: v0.15.0-alpha → v0.16.0-alphapool: v0.6.6-beta → v0.6.4-betafaraday: v0.2.16-alpha → v0.2.14-alphaAdd missing proto files:
lnd:peersrpc/peers.proto,stateservice.prototapd:tapcommon.proto,authmailboxrpc/mailbox.protoBump version to v0.3.5-alpha
Test plan
yarn update-protos && yarn generateto verify proto generation succeedsyarn buildto verify TypeScript compilationyarn linkto verify compatibility with new LNC WASM version