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

Add lnbits-legend #496

Closed
wants to merge 1 commit into from
Closed

Conversation

afilini
Copy link
Contributor

@afilini afilini commented Jun 4, 2022

This PR adds "lnbits legend" (https://github.com/lnbits/lnbits-legend) with support for both clightning and lnd.

Unfortunately it requires quite a few python packages that are not shipped by nixos, so I had to add them here as well.

This is my first relatively large contribution to a project in nix, so I apologize if there are any obvious mistakes.

@prusnak
Copy link
Contributor

prusnak commented Jun 5, 2022

Couple of questions:

  • why not lnbits-infinity? it's much easier to package golang stuff
  • what use-case do you want to bring to nix-bitcoin? if it's just L3 custodial LN wallets, maybe lndhub.go is better choice? (but it only supports lnd for the time being, until Feature/cln getAlby/lndhub.go#123 is resolved)

Otherwise the PR looks great from technical perspective!

@afilini
Copy link
Contributor Author

afilini commented Jun 6, 2022

why not lnbits-infinity? it's much easier to package golang stuff

On their website they say "(Legend is the commonly used and tested version, Infinity is more experimental)", so I figured "legend" is still the main implementation, at least for the time being.

what use-case do you want to bring to nix-bitcoin?

To be honest I don't know many of the alternatives and their tradeoffs compared to lnbits, the reason why I decided to package this specifically is that I personally need a backend for a Bleskomat and since I had to package lnbits for myself I thought I might as well share it in case somebody else finds it useful.

From my perspective I also feel like lnbits is still the most popular backend in used today, or at least this is what I see mentioned the most on twitter. There are a few interesting plugins that I'd like to try out and being in Python it also seems very easy to extend.

So in summary I'm not exactly trying to add a specific functionality to nix-bitcoin, I'm just adding a piece of software that I personally need and will use in the future and I guess other people might want to try as well.

@prusnak
Copy link
Contributor

prusnak commented Jun 6, 2022

Thanks for explanation Alekos!

@erikarvstedt
Copy link
Collaborator

This looks great, thank you!

Python packages indeed add a great maintenance burden: They often break on upstream nixpkgs updates and require lengthy manual tweaking to unblock.
Would you be okay with initially adding lndbits as a Flake?
It could live under the fort-nix org, be referenced in our README, and I'll help maintaining it.
If maintenance proves to be smooth and sustainable, we can promote it to a native nix-bitcoin module.

@afilini
Copy link
Contributor Author

afilini commented Jun 13, 2022

Would you be okay with initially adding lndbits as a Flake?

I've never used flakes, so this may take a while, but I can give it a shot!

@erikarvstedt
Copy link
Collaborator

@afilini, I'll happily do the flakes conversion and report back soon.

@arcbtc
Copy link

arcbtc commented Jul 1, 2022

Damn, if only I knew the existence of this PR!

Thanks for making this

We have another lnbitsnix attempt LNbits flake working yesterday (which being python, did require a bunch of work) https://github.com/MatthewCroughan/lnbits-legend/tree/matthewcroughan/nixify. It still requires some more work our end to improve further. Perhaps you can take a look and see if there is anything we can merge between to two attempts?

An issue with nixbitcoin you have touched upon above, is people who dont know much about nix, pop along and think its a registry. It's prob better that projects maintain their own flakes anyway, so perhaps nixbitcoin could also add a registry of useful bitcoin/node projects on https://search.nixos.org/flakes, that would take the pressure off nixbitcoin becoming the maintainers of all nix/bitcoin things?

@arcbtc
Copy link

arcbtc commented Jul 1, 2022

@afilini is also 100% correct. Infinity is on a back burner for most of the community until we get legend out of beta, then we will all pile in on infinity. Both have slightly different approaches, which I am sure will serve different uses, so although its a bit weird I imagine both continuing to exist

@MatthewCroughan
Copy link

MatthewCroughan commented Jul 1, 2022

It would be very helpful if @afilini could help maintain the Nix code I'm adding to lnbits-legend upstream, and if @jonasnick could add a list to the README that links out to projects that have added Nix properly to their own repositories, rather than having it all packaged here.

As part of what I have done in the link Ben shared, I have also converted lnbits-legend to poetry, and have had to modify a lot of the Python code to stop making assumptions about relative paths, and give absolute paths instead. There's a lot of Python refactor work that needs to be done in lnbits-legend to make it work correctly anyway. This is something we can do gradually over time.

I'm working on submitting a PR to lnbits-legend which notes all of the Python refactor work that needs to be done, along with some decent Nix code that works around all of the problems and introduces services.lnbits.enable along with a VM test. The VM test is something you can run right now:

If you do nix build github:matthewcroughan/lnbits-legend/matthewcroughan/nixify#checks.x86_64-linux.vmTest you'll run a test that:

  1. Builds lnbits
  2. Runs two virtual machines
  3. Proves that a client VM can talk to a server VM running LNBits correctly, and that curl returns the correct HTML title, rather than uvicorn crashing (which it did a lot in my testing, until I managed to make it work! :D)

@MatthewCroughan
Copy link

The PR I've made to lnbits-legend is now visible here lnbits/lnbits#721

@arcbtc
Copy link

arcbtc commented Jul 27, 2022

@HaosGames
Copy link
Contributor

I am currently trying to fix the flake for the upstream lnbits but am stuck at figuring out which other paths to symlink to mutable locations. lnbits/lnbits#1855
Maybe someone else is willing to help out

@prusnak
Copy link
Contributor

prusnak commented Oct 7, 2023

I think we can close this PR now that lnbits has functional flake in the upstream repo: https://github.com/lnbits/lnbits/blob/dev/flake.nix

and make an effort to integrate LNbits via flake to nix-bitcoin.

@MatthewCroughan
Copy link

@prusnak I am not sure it makes any sense to have nix-bitcoin contain a reference to lnbits. That kind of defeats the purpose and point of flakes. Flakes that vendor other's flakes don't make sense.

@jonasnick jonasnick closed this Oct 8, 2023
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

Successfully merging this pull request may close these issues.

7 participants