Skip to content
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

Question: NodeJs support #3

Closed
niteshbalusu11 opened this issue Oct 13, 2022 · 27 comments
Closed

Question: NodeJs support #3

niteshbalusu11 opened this issue Oct 13, 2022 · 27 comments

Comments

@niteshbalusu11
Copy link

niteshbalusu11 commented Oct 13, 2022

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
Copy link
Owner

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
Copy link
Author

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
Copy link
Owner

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
Copy link
Author

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
Copy link
Author

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
Copy link
Owner

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

@niteshbalusu11
Copy link
Author

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

@lnbc1QWFyb24
Copy link
Owner

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
Copy link
Author

niteshbalusu11 commented Oct 17, 2022

Cool ok, thanks.

@escapedcat
Copy link
Contributor

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
Copy link
Owner

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
Copy link
Contributor

bumi commented Nov 5, 2022

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
Copy link
Owner

@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
Copy link
Contributor

bumi commented Nov 7, 2022

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

@escapedcat
Copy link
Contributor

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
Copy link
Author

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

@lnbc1QWFyb24
Copy link
Owner

@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
Copy link
Owner

@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
Copy link
Author

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
Copy link
Owner

@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
Copy link
Author

Oh nice! Thanks!

@bumi
Copy link
Contributor

bumi commented Nov 11, 2022

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
Copy link
Contributor

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
Copy link
Contributor

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:

  • Add/modify polyfills.ts file to provide missing dependencies. lnmessage threw not defined errors for crypto & WebSocket. Providing them like below in polyfills was the solution.
import WebSocket from 'ws';

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

if (!(<any>global).WebSocket) {
  (<any>global).WebSocket = WebSocket;
}
  • Include polyfills.ts file in your tsconfig.json's include section.

  • Import and initialise polyfills.ts at the start of your project.

  • Optionally, only if these dependencies are missing, install them with npm install --save-dev @types/node, ws, @types/ws.

@lnbc1QWFyb24
Copy link
Owner

@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
Copy link
Owner

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

@niteshbalusu11
Copy link
Author

Yay! thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants