Skip to content

LSAT aware client interceptor #101

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

Merged
merged 7 commits into from
Dec 7, 2019

Conversation

guggero
Copy link
Member

@guggero guggero commented Nov 1, 2019

This PR makes the loop daemon aware of the LSAT authentication scheme.
An unary client gRPC connector is added that intercepts server responses and handles payment challenges sent by the server.
An LSAT token is automatically acquired and paid by the interceptor and then stored in the file .loop/<network>/lsat.token.

@guggero guggero force-pushed the lsat-integration branch 3 times, most recently from 1b53342 to 54a8ec0 Compare November 8, 2019 09:47
@guggero guggero changed the title wip: LSAT aware client interceptor LSAT aware client interceptor Nov 8, 2019
@guggero guggero marked this pull request as ready for review November 8, 2019 09:52
@guggero guggero requested review from Roasbeef and wpaulino November 8, 2019 09:52
@guggero guggero force-pushed the lsat-integration branch 2 times, most recently from 90007e7 to 0727aa0 Compare November 8, 2019 14:14
Copy link
Member

@Roasbeef Roasbeef left a comment

Choose a reason for hiding this comment

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

Nice work! It's exciting how close we are to deploying LSAT for Loop! One thing that came up, but which shouldn't block this PR is adding a new loopd-level RPC to allow the user to obtain and examine their current LSAT token. This may be useful for user support, and also just some insight into the user's current level, their tier, etc.

@wpaulino
Copy link
Contributor

Should we also consider maintaining a history of all LSATs paid for accounting purposes?

@guggero
Copy link
Member Author

guggero commented Nov 15, 2019

I updated the store to keep old tokens and not just overwrite them.
Also there is now a command that lists all tokens (current and expired) in the command line.

The reason I implemented a file based token store is so people wanting to integrate the interceptor into their code base don't necessarily need a boltDB.
The question is now, should we also add a boltDB compliant store for loopd itself or go with the file based store for now?

Copy link
Member

@Roasbeef Roasbeef left a comment

Choose a reason for hiding this comment

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

LGTM 🐲

Left some final comments w.r.t updated godoc comments as well as naming for the new command.

@guggero guggero force-pushed the lsat-integration branch 2 times, most recently from f62c59c to 0e96a9c Compare November 22, 2019 10:22
We need the ability to connect to a swap server that uses
a self-signed certificate. The LSAT proxy cannot proxy insecure
gRPC requests since they don't conform to the HTTP 1.1 standard.
Therefore the LSAT proxy fill only serve TLS connections.
This means, we need the TLS path option to specify the certificate
the test environment LSAT proxy uses.
@guggero guggero requested a review from wpaulino November 26, 2019 09:31
@wpaulino wpaulino removed their request for review November 26, 2019 18:02
@guggero guggero requested a review from wpaulino November 27, 2019 12:44
@guggero guggero force-pushed the lsat-integration branch 2 times, most recently from 023d79f to a15c993 Compare December 2, 2019 15:49
@wpaulino
Copy link
Contributor

wpaulino commented Dec 2, 2019

Linter failing with:

lsat/store_test.go:76:11: S1005: should omit value from range; this loop is equivalent to `for key := range ...` (gosimple)
	for key, _ := range tokens {
	         ^
lsat/store_test.go:111:11: S1005: should omit value from range; this loop is equivalent to `for key := range ...` (gosimple)
	for key, _ := range tokens {

@guggero guggero force-pushed the lsat-integration branch 2 times, most recently from 6295aca to 664508e Compare December 5, 2019 13:46
Copy link
Contributor

@wpaulino wpaulino left a comment

Choose a reason for hiding this comment

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

This is pretty much good to go! Linter is failing again with:

lsat/interceptor.go:125: File is not `goimports`-ed (goimports)
	// we know it's not valid.

@guggero
Copy link
Member Author

guggero commented Dec 6, 2019

Fixed last nits, this should be good to merge. @alexbosworth want to take a final look?

Copy link
Member

@Roasbeef Roasbeef left a comment

Choose a reason for hiding this comment

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

LGTM 🤺

Once we're ready on the server-side, we'll need to update the client to use our new proxy port.

@Roasbeef Roasbeef merged commit 53dc21f into lightninglabs:master Dec 7, 2019
@guggero guggero deleted the lsat-integration branch December 7, 2019 14:26
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.

4 participants