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

lndhub-go: integrate LndHub.go #519

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

prusnak
Copy link
Contributor

@prusnak prusnak commented Jul 21, 2022

LndHub.go is an accounting wrapper for the Lightning Network. It provides separate accounts for end-users and LndHub compatible API while written in Go.

upstream: https://github.com/getAlby/lndhub.go

This is a draft PR to gather feedback, also any help about how to test this is welcome!

Fixes #517

@erikarvstedt
Copy link
Collaborator

Thanks, Pavol, lndhub seems low maintenance and a good fit for nix-bitcoin. If it supported clightning, I'd use it right away.

Why do you set FEE_RESERVE to true (default is false)?

@prusnak prusnak force-pushed the lndhub branch 3 times, most recently from 1bbfc45 to 644b632 Compare July 21, 2022 09:55
@prusnak
Copy link
Contributor Author

prusnak commented Jul 21, 2022

If it supported clightning, I'd use it right away.

CLN might be supported in the future, see getAlby/lndhub.go#123

Why do you set FEE_RESERVE to true (default is false)?

Fixed in 644b632

@prusnak prusnak force-pushed the lndhub branch 2 times, most recently from a263052 to edd9def Compare July 21, 2022 10:30
@prusnak prusnak changed the title lndhub-go: integrate LndHub.go into Nix-Bitcoin lndhub-go: integrate LndHub.go Jul 21, 2022
@prusnak prusnak force-pushed the lndhub branch 5 times, most recently from 78ea328 to 875787f Compare July 21, 2022 11:03
@erikarvstedt
Copy link
Collaborator

erikarvstedt commented Jul 21, 2022

Initial fixups to make the service run.
The postgresql connection fails with:

Jul 21 13:12:16 nb-test lndhub.go[5251]: {"level":"fatal","time":"2022-07-21T13:12:16+02:00","message":"Error initializing db migrator: pgdriver: SASL: FATAL: password authentication failed for user \"lndhub-go\" (SQLSTATE=28P01)"}

The best way to fix this is to make lndhub-go allow connections via the postgresql socket.

@erikarvstedt
Copy link
Collaborator

erikarvstedt commented Jul 21, 2022

I've removed all app-specific settings and replaced them with option settings.
They can now be defined like this:

services.lndhub-go = {
  settings = {
    FEE_RESERVE = true;
    MAX_SEND_AMOUNT = 1000000;
  };
};

If you think some of these settings deserve a dedicated NixOS option, we can add them back in (maybe via a freeform module).

@prusnak
Copy link
Contributor Author

prusnak commented Jul 21, 2022

Merged your fixups into my branch, thanks! Added bfceb1e which fixes the database name issue.

@erikarvstedt
Copy link
Collaborator

Fixups.
The postgres connection fails with:

Jul 21 14:40:23 nb-test lndhub.go[575]: {"level":"fatal","time":"2022-07-21T14:40:23+02:00","message":"Error initializing db migrator: pgdriver: SASL: FATAL: password authentication failed for user \"lndhub-go\" (SQLSTATE=28P01)"}

We can add a password via the secrets mechanism, but I'd really prefer a socket connection. This should be simple to add to lndhub.

@prusnak
Copy link
Contributor Author

prusnak commented Jul 21, 2022

This should be simple to add to lndhub.

Maybe it is enough to just change the connection string to postgres:///lndhubgo?host=/var/run/postgresql/ ?

@erikarvstedt
Copy link
Collaborator

Ah, great.
With DATABASE_URI = "postgres:///lndhubgo?host=/run/postgresql"; connecting still fails:

Jul 21 16:03:10 nb-test lndhub.go[632]: {"level":"fatal","time":"2022-07-21T16:03:10+02:00","message":"Error initializing db migrator: dial unix /run/postgresql: connect: permission denied"}

But manually connecting via sudo -u lndhub-go psql lndhubgo --host=/run/postgresql works fine. Seems like a lndhub-go bug.

@prusnak
Copy link
Contributor Author

prusnak commented Jul 21, 2022

Maybe we need to specify also the user?

DATABASE_URI = "postgres:///lndhubgo?host=/run/postgresql&user=${cfg.user}";

@erikarvstedt
Copy link
Collaborator

No, this didn't help. Same with the equivalent postgres://lndhub-go@/lndhubgo?host=/run/postgresql

@prusnak
Copy link
Contributor Author

prusnak commented Jul 21, 2022

I figured it out. First, this PR has to be merged: getAlby/lndhub.go#215 and then one can use the following connection string: unix://${cfg.user}@lndhubgo/run/postgresql/.s.PGSQL.5432?sslmode=disable

@prusnak
Copy link
Contributor Author

prusnak commented Jul 22, 2022

When getAlby/lndhub.go#218 is merged, we can remove the xxd shenanigans

@jonasnick
Copy link
Member

Thanks, Pavol, lndhub seems low maintenance and a good fit for nix-bitcoin. If it supported clightning, I'd use it right away.

Agreed!

@prusnak
Copy link
Contributor Author

prusnak commented Jul 25, 2022

Rebased and squashed. Added fixup commits that need to wait until next release of LndHub.go is released, containing getAlby/lndhub.go#215 and getAlby/lndhub.go#218

@prusnak
Copy link
Contributor Author

prusnak commented Aug 8, 2022

lndhub-go 0.10.0 which contains the required fixes in getAlby/lndhub.go#215 and getAlby/lndhub.go#218 has been released and merged to nixpkgs via NixOS/nixpkgs#185649

@prusnak
Copy link
Contributor Author

prusnak commented Aug 21, 2022

Rebased after #537 has been merged. Removing draft status.

@prusnak prusnak marked this pull request as ready for review August 21, 2022 17:57
@prusnak prusnak force-pushed the lndhub branch 3 times, most recently from 0178abd to deeb21b Compare August 21, 2022 18:24
@erikarvstedt
Copy link
Collaborator

Fixups.

@prusnak
Copy link
Contributor Author

prusnak commented Aug 21, 2022

Fixups.

Pulled into the PR

Copy link
Collaborator

@erikarvstedt erikarvstedt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK cff12ba

Copy link
Collaborator

@erikarvstedt erikarvstedt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@prusnak
Copy link
Contributor Author

prusnak commented Aug 22, 2022

Final fixup

applied to a03a0b7

erikarvstedt
erikarvstedt previously approved these changes Aug 22, 2022
Copy link
Collaborator

@erikarvstedt erikarvstedt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK 6a195f6

@prusnak
Copy link
Contributor Author

prusnak commented Dec 6, 2022

Dropped 6a195f6 since NixOS/nixpkgs#204769 has been merged and no longer required.

Rebased ...

@erikarvstedt erikarvstedt added the enhancement New feature or request label Dec 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add LndHub.go module
3 participants