[feature] Use LNC to connect to the LND node #98
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Roasbeef
requested changes
Jun 7, 2023
positiveblue
force-pushed
the
lnc
branch
9 times, most recently
from
4b0a926
to
581d7b3
Compare
positiveblue
force-pushed
the
lnc
branch
5 times, most recently
from
261ed8c
to
8bea271
Compare
|
@positiveblue, remember to re-request review from reviewers when ready |
positiveblue
force-pushed
the
lnc
branch
2 times, most recently
from
65d0fd6
to
0f2b63f
Compare
ellemouton
requested changes
Jun 30, 2023
There was a problem hiding this comment.
Something needs to block calls until the connection is ready. Also maybe some info logs to say where the connection is at.
Ran into this panic while testing.
2023/06/30 11:36:31 http: panic serving 127.0.0.1:65481: runtime error: invalid memory address or nil pointer dereference [6/477]
goroutine 84 [running]:
net/http.(*conn).serve.func1()
/usr/local/go/src/net/http/server.go:1850 +0xb0
panic({0x1056daf60, 0x106493e90})
/usr/local/go/src/runtime/panic.go:890 +0x258
github.com/lightninglabs/aperture.(*LndChallenger).NewChallenge(0x0, 0x14000bec000?)
/Users/elle/ll/aperture/challenger.go:269 +0x28
github.com/lightninglabs/aperture/mint.(*Mint).MintLSAT(0x140007929c0, {0x105927f08, 0x140001a2000}, {0x140003504e0, 0x1, 0x1})
/Users/elle/ll/aperture/mint/mint.go:110 +0x68
github.com/lightninglabs/aperture/auth.(*LsatAuthenticator).FreshChallengeHeader(0x140008bc000, 0x1400029a700, {0x14000040700, 0x8}, 0xa)
/Users/elle/ll/aperture/auth/authenticator.go:86 +0xb8
github.com/lightninglabs/aperture/proxy.(*Proxy).handlePaymentRequired(0x1400018e050, {0x105926fd0, 0x1400037e460}, 0x1400029a700, {0x14000040700, 0x8}, 0x1018?)
/Users/elle/ll/aperture/proxy/proxy.go:403 +0x70
github.com/lightninglabs/aperture/proxy.(*Proxy).ServeHTTP(0x1400018e050, {0x105926fd0, 0x1400037e460}, 0x1400029a700)
/Users/elle/ll/aperture/proxy/proxy.go:177 +0x2f8
net/http.HandlerFunc.ServeHTTP(0x14000080948?, {0x105926fd0?, 0x1400037e460?}, 0x40?)
/usr/local/go/src/net/http/server.go:2109 +0x38
golang.org/x/net/http2/h2c.h2cHandler.ServeHTTP({{0x10591dca0?, 0x140003fe480?}, 0x1400018e140?}, {0x105926fd0, 0x1400037e460}, 0x1400029a700)
/Users/elle/go/pkg/mod/golang.org/x/net@v0.10.0/http2/h2c/h2c.go:125 +0x41c
net/http.serverHandler.ServeHTTP({0x14000bacd20?}, {0x105926fd0, 0x1400037e460}, 0x1400029a700)
Roasbeef
requested changes
Jun 30, 2023
Stop creating the connection inside the `NewLndChallenger`. That will allow us reuse the function with an LNC connection.
Whenever we use the LightningClient from an LNC connection we need to add the macaroon to the headers.
The new package contains a new interface (`Challenger`) that any new challenger must implement. Because aperture uses the new interface instead of using directly the `LndChallenger` struct I added the `Stop()` method to the `mint.Challenger`. Instead of also adding the `Start()` method the constructor returns a Challenger already "started".
Roasbeef
reviewed
Jul 4, 2023
| if !a.cfg.Authenticator.Disable { | ||
| genInvoiceReq := func(price int64) (*lnrpc.Invoice, error) { | ||
| return &lnrpc.Invoice{ | ||
| Memo: "LSAT", |
There was a problem hiding this comment.
Non-blocking, but in the future we can insert relevant endpoint/context information here so someone can build a UI/dashboard on top of lnd/aperture to do things like track the amount earned from a given endpoint, etc.
To do this, we'd need to expose the memo as part of the challenger interface, as only much later in the pipeline do we have the full request context information.
| @@ -263,7 +239,9 @@ func (l *LndChallenger) Stop() { | |||
| // request (invoice) and the corresponding payment hash. | |||
| // | |||
| // NOTE: This is part of the mint.Challenger interface. | |||
| func (l *LndChallenger) NewChallenge(price int64) (string, lntypes.Hash, error) { | |||
| func (l *LndChallenger) NewChallenge(price int64) (string, lntypes.Hash, | |||
There was a problem hiding this comment.
Re the above comment, we could pass in the additional context here as an optional argument. Then the invoice creation has a chance to tag any extra details.
Roasbeef
reviewed
Jul 4, 2023
|
|
||
| // MailboxAddress is the address of the mailbox that the client will | ||
| // use for the LNC connection. | ||
| MailboxAddress string `long:"mailboxaddress" description:"the host:port of the mailbox server to be used"` |
There was a problem hiding this comment.
We should have a default mailbox addr we use here.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Allow aperture to connect to LND using LNC