lnbc1QWFyb24 / lnmessage

Talk to Lightning nodes from the Browser
MIT License
22 stars 7 forks source link

Question: NodeJs support #3

Closed niteshbalusu11 closed 1 year ago

niteshbalusu11 commented 2 years ago

Hey - does this work with NodeJs as well? also, does it support —experimental-websocket-port as well or just the P2P port?

And what’s the role of the Hex Encoded private key in this communication?

lnbc1QWFyb24 commented 2 years ago

Hey @niteshbalusu11 this library will not currently work as is with NodeJS, it is built specifically for the browser. I could probably add support by checking the window property and loading the equivalent libraries dynamically if not in a browser env, I would just need to double check that it would not break browser bundlers like Vite. If you were using it in node there would be no need for a WebSocket connection as you can just use a TCP Socket and connect directly to the node over a regular socket. WebSockets are only needed in the browser since they do not support a regular socket.

Can I ask what is your use case for NodeJS support?

You can have the library connect to a node on any port that you like by passing in a port value in the constructor.

The purpose of the private key is so that you can have a persistent node id for lnmessage. The main reason I added this is so that the node id can be presented to the user and then they can use that node id to restrict the rune they give to the app so that only the app could use that rune if it ever leaked somehow.

So the flow is that if the user is a first time visitor, then don't pass a value for the privateKey. Lnmessage will generate a random one for you. You can then display the corresponding public key to the user so they can use that to restrict the rune to that key and then the rune and private key can be saved together for connection next time the user returns to the app.

niteshbalusu11 commented 2 years ago

Thanks for explaining.

The use case for NodeJs support is, working with sockets is just easier to work with the current state of core-lightning gRPC and they don't have like in-house support for REST API as well. So the only other way is the Unix Socket which doesn't work well at all with docker. So I found this really useful, like generate a fake private/public keypair, present it to the user on the CLI App, they create a rune with restricted access and then talk to it via CLI commands.

LNSocket https://github.com/jb55/lnsocket does have NodeJS support but it does not take in a private key so you can't restrict your rune.

lnbc1QWFyb24 commented 2 years ago

Ah ok no worries. I'll see if I can get a Node compatible version working soon and i'll keep you posted here.

I am finding that there are some limitations with this way of accessing the RPC which I am yet to document, so it is not a complete substitute for the gRPC. For instance long running calls to RPC methods like waitinvoice and waitanyinvoice can sometimes break the decryption parameters when the keys are rotated internally. I'm not sure if this is due to a problem with the code as it is, or if this is a limitation of the messaging protocol in Lightning itself. This can worked around by polling listinvoice instead though, so not a big deal, but I thought is worth mentioning.

niteshbalusu11 commented 2 years ago

Cool, yeah. Ideally I just want to work with gRPC but there are just not enough methods for me to work with. Too limited.

niteshbalusu11 commented 2 years ago

I can't seem to get it working in the browser. @aaronbarnardsound Am i missing something?

Uncaught TypeError: Error resolving module specifier “rxjs”. Relative module specifiers must start with “./”, “../” or “/”.
lnbc1QWFyb24 commented 2 years ago

rxjs should be getting installed as a dependency, what bundler are you using?

niteshbalusu11 commented 2 years ago

Installing from npm already gives me a bundled version with all deps needed right?

lnbc1QWFyb24 commented 2 years ago

It won't be bundled straight from NPM. It exports an esm module which will run in the browser, but the dependencies are marked as external and will need to bundled with it.

niteshbalusu11 commented 2 years ago

Cool ok, thanks.

escapedcat commented 2 years ago

Hey @aaronbarnardsound , sorry to jump onto this. Happy to create a new issue as well.

If lnmessage is supporting browser only for now anyways would it be possible to remove type: "module" from the package.json?
Currently we need to add a resolver to webpack to handle this "not fully specified" module. If other formats should be supported the build script can be modified as well I guess.
Happy to propose a PR or give more context/information.

lnbc1QWFyb24 commented 2 years ago

Hey @escapedcat, yeah if you could create a separate issue that would be great.

My understanding is that since this package exports a esm module, that it was best practice to add type: module to indicate that it is an esm module. But I have not seen that Webpack option needing to be used before. I'll look in to it some more and create a Webpack project to test with, but happy to remove it in the meantime as long as it doesn't break other bundlers (Vite, Rollup etc).

bumi commented 2 years ago

I am not really good at NPM packages and the state of JS bundlers and the different possible environments. Recently I used microbundle to generate multiple formats (CJS, UMD & ESM). as I think there is no real reason to limit it and it gives installation options to the user.

https://github.com/getAlby/alby-js-sdk/blob/master/package.json#L5-L17

lnbc1QWFyb24 commented 2 years ago

@bumi Yeah I believe I can just run the build multiple times and modify the output for each build to provide different options. I'll create an issue for it.

In the meantime, I added some fixes that should fix the resolution error that @escapedcat mentioned. So let me know if that looks good on your end now that you are running v0.0.11.

bumi commented 2 years ago

great, thanks! +1 yeah, I think microbundle does all that. (https://github.com/getAlby/alby-js-sdk/blob/master/package.json#L19-L23)

escapedcat commented 2 years ago

In the meantime, I added some fixes that should fix the resolution error that @escapedcat mentioned. So let me know if that looks good on your end now that you are running v0.0.11.

With v0.0.11 we still get these when using webpack without the special resolver:

ERROR in ./node_modules/lnmessage/dist/messages/InitMessage.js 3:0-38
Module not found: Error: Can't resolve './BitField' in '/Users/user/alby/node_modules/lnmessage/dist/messages'
Did you mean 'BitField.js'?
BREAKING CHANGE: The request './BitField' failed to resolve only because it was resolved as fully specified
(probably because the origin is strict EcmaScript Module, e. g. a module with javascript mimetype, a '*.mjs' file, or a '*.js' file where the package.json contains '"type": "module"').
The extension in the request is mandatory for it to be fully specified.
Add the extension to the request.

No need to rush this. I believe this is non-blocking for us. We can make Alby work with the special rule for now till the build is sorted out.

niteshbalusu11 commented 2 years ago

Can the browser build instructions be added to the readme? Whatever build tool you’re using and it’s config.

lnbc1QWFyb24 commented 2 years ago

@escapedcat @bumi Sorry yeah it looks like i missed a few files where I needed to add the .js extension which should now be fixed in version 0.0.12. Give that a try and let me know if that works.

lnbc1QWFyb24 commented 2 years ago

@niteshbalusu11 Are you having trouble getting this running in a browser build env? Are building with a framework such as React/Vue/Svelte or just going for Vanilla JS?

niteshbalusu11 commented 2 years ago

I haven't played with it enough but when I tried bundling with Vite and it needed some extra config and took me sometime to figure it out. I think it'll be nice if you can add Vite build instructions to the read me. Maybe even webpack? I generally use react.

lnbc1QWFyb24 commented 2 years ago

@escapedcat this should finally be fixed in 0.0.13. Sorry I should have created a repro in the first place to make sure it is fixed.

@niteshbalusu11 with version 0.0.13 you can just create a new project with Create React App and then install Lnmessage and you should be good to go.

niteshbalusu11 commented 2 years ago

Oh nice! Thanks!

bumi commented 2 years ago

yeah! looks fine. using it in a jest setup still requires some hack because of the module but in my webpack build setup it works nicely! thanks!

escapedcat commented 2 years ago

Sorry I should have created a repro in the first place to make sure it is fixed.

No worries, it's about the friends we made along the way ;)

ShahanaFarooqui commented 1 year ago

Till this issue is not resolved, lnmessage library can be used with the help of polyfills.

Below is the working solution from my NodeJS & Typescript project:

import WebSocket from 'ws';

if (!(<any>global).crypto) {
  (<any>global).crypto = crypto;
}

if (!(<any>global).WebSocket) {
  (<any>global).WebSocket = WebSocket;
}
lnbc1QWFyb24 commented 1 year ago

@ShahanaFarooqui that is a great solution! I have added this to the documentation as the go-to way to use Lnmessage in a Nodejs project.

I thought that maybe I could implement this polyfill in the library for convenience and dynamically import the crypto and ws deps when in a Node env, but the crypto module is needed within the constructor to create the ls and es values, so can't async import them there.

Other solutions involved adding more dependencies which will bloat the project and add more surface area for security problems, so I'd rather just document your polyfill solution for now.

lnbc1QWFyb24 commented 1 year ago

v0.1.0 now has support out of the box for NodeJS without the need for polyfilling!

niteshbalusu11 commented 1 year ago

Yay! thanks!