Skip to content

Conversation

@orbitalturtle
Copy link
Contributor

@orbitalturtle orbitalturtle commented Jul 19, 2021

When using the Lightning Terminal in stateless integrated mode (described here: lightninglabs/lightning-terminal#160), Pool shouldn't create a new macaroon. This is a small change to add an option to stop macaroons from being created.

Also exports the AllPermissions slice to make baking a macaroon with Pool's permissions easier in Lightning Terminal's integrated mode.

Copy link
Contributor

@guggero guggero left a comment

Choose a reason for hiding this comment

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

Definitely looks better this way.

@orbitalturtle orbitalturtle force-pushed the macaroon-hex branch 3 times, most recently from ba9aa48 to e5e19f5 Compare September 14, 2021 02:49
@orbitalturtle
Copy link
Contributor Author

Thanks for the review @guggero ! Made those changes.

@orbitalturtle orbitalturtle changed the title When a macaroon string is passed in, don't create new macaroons Add option to stop creation of macaroons Sep 14, 2021
Copy link
Contributor

@guggero guggero left a comment

Choose a reason for hiding this comment

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

LGTM pending the dependent PRs in lndclient and lnd.

The lnd PR is very close and can be merged as soon as the three remaining issues are addressed. Then we can update the lndclient and this PR to use the actual versions in go.mod.

@orbitalturtle orbitalturtle force-pushed the macaroon-hex branch 3 times, most recently from 09dc898 to e6507b5 Compare September 24, 2021 01:16
@orbitalturtle
Copy link
Contributor Author

@guggero great, I updated lnd and lndclient with the actual versions. I also attempted to take care of all the changes required to bump those up... I'm especially not sure if I updated macaroons.NewService method correctly.

@guggero
Copy link
Contributor

guggero commented Sep 24, 2021

@orbitalturtle I forgot to mention this to you, sorry. There is a branch I pushed to the main Pool repo called lnd-14 that you can rebase on to get all the things in you need to make this work. Your PR will need to wait for the 0.14 lnd compatibility anyway, so can just make that a hard dependency.

…rminal

Export AllPermissions slice so that we can retrieve the permissions
required to run Pool's operations. We need these permissions in
Lightning Terminal in order to bake a super macaroon that allows
access to Pool.
@orbitalturtle orbitalturtle changed the base branch from master to lnd-14 September 24, 2021 22:23
@orbitalturtle orbitalturtle force-pushed the macaroon-hex branch 2 times, most recently from fb3c269 to 7584bd9 Compare September 25, 2021 08:28
@orbitalturtle
Copy link
Contributor Author

@guggero Got it, no problem, Just updated.

Copy link
Contributor

@guggero guggero left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

}

// Start the macaroon service and let it create its default macaroon in
// case it doesn't exist yet.
Copy link
Contributor

Choose a reason for hiding this comment

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

This comment gets outdated after noMacaroonCreation is set to false.

}()

// Start the macaroon service and let it create its default macaroon in
// case it doesn't exist yet.
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: change the comment so it reflects that it creates its default macaroon in case it doesn't exist yet and noMacaroonCreation is true

@guggero guggero deleted the branch lightninglabs:lnd-14 October 21, 2021 17:23
@guggero guggero closed this Oct 21, 2021
@guggero
Copy link
Contributor

guggero commented Nov 4, 2021

Can be rebased onto master now. Sorry for the indirect/unintended close.

@guggero
Copy link
Contributor

guggero commented Nov 4, 2021

I'm going to take over those PRs so they can hopefully make it into the lnd 0.14.0 release of LiT. I hope you don't mind!

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.

3 participants