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 Autopilot sub-RPC server #2039

Merged
merged 10 commits into from Dec 14, 2018

Conversation

Projects
None yet
3 participants
@halseth
Copy link
Collaborator

halseth commented Oct 12, 2018

This PR adds new subserver that manages lnds autopilot, enabled by providing the autopilotrpc flag at build time.

With this enabled, a new RPC for getting the status if the autopilot agent, and enabling/disabling the agent at runtime is set up on the gRPC server.

For builds without the autopilotrpc flag, no behavior is changed, and the autopilot.enable flag in the config will still determine whether the agent is enabled or not.

@halseth halseth force-pushed the halseth:autopilot-rpcserver branch 2 times, most recently from d9458d8 to 5c0ee85 Oct 12, 2018

@halseth

This comment has been minimized.

Copy link
Collaborator

halseth commented Oct 31, 2018

Blocked by #2081

@halseth halseth added the blocked label Oct 31, 2018

@halseth halseth referenced this pull request Oct 31, 2018

Merged

Scoring based autopilot attachment heuristic #2006

1 of 5 tasks complete

@halseth halseth force-pushed the halseth:autopilot-rpcserver branch from 5c0ee85 to e7773fd Nov 12, 2018

@halseth halseth changed the title [experimental] Add support for enabling/disabling autopilot at runtime Add Autopilot sub-RPC server Nov 12, 2018

@halseth

This comment has been minimized.

Copy link
Collaborator

halseth commented Nov 12, 2018

Rebased on top of the subserver PR.

@halseth halseth referenced this pull request Nov 23, 2018

Open

[WIP] Bos score enabled Autopilot #2212

4 of 6 tasks complete

@halseth halseth force-pushed the halseth:autopilot-rpcserver branch from e7773fd to 9f39246 Nov 29, 2018

@halseth halseth removed the blocked label Nov 29, 2018

@halseth

This comment has been minimized.

Copy link
Collaborator

halseth commented Nov 29, 2018

Rebased on subserver-enabled master, ready for review and testing! 💃

@AndrewSamokhvalov

This comment has been minimized.

Copy link
Member

AndrewSamokhvalov commented Dec 1, 2018

Would you consider adding here something similar to sending payment RPC?

  1. router => agent
  2. queryroutes => querynodesscores
  3. sendtoroute => openchannelswith

The end goal is to be able to use the wisdom of lnd itself, but adding some custom node filtering, based on internal third-party metrics.

As far as I understand there are two ways how third-party could influence the process of opening channel today is either (1) work directly with openchannel/closechannel rpc and avoid agent, which means the recreation of agent logic, or (2) rewrite heuristic interface to use remote metrics fetching, but in this case lose information from ConstrainedPrefAttachment heuristic.

As an example of additional metric might be “percentage of time node is being online”, or “percentage of success payments routed through node”. Based on this metrics third-party could filter nodes and than send the result back to agent., avoiding need of sending PRs in lnd, with new heuristic implementations.

@Roasbeef

This comment has been minimized.

Copy link
Member

Roasbeef commented Dec 3, 2018

rewrite heuristic interface to use remote metrics fetching, but in this case lose information from ConstrainedPrefAttachment heuristic.

No, they can simply be composed.

As an example of additional metric might be “percentage of time node is being online”, or “percentage of success payments routed through node”.

Check out the PR, all of these can be added as new score based heuristics.

Show resolved Hide resolved autopilot/manager.go
Show resolved Hide resolved autopilot/manager.go Outdated
Show resolved Hide resolved autopilot/manager.go
Show resolved Hide resolved autopilot/manager.go
Show resolved Hide resolved autopilot/manager.go
/**
Enable is used to enable or disable the autopilot agent.
*/
rpc Enable(EnableRequest) returns (EnableResponse);

This comment has been minimized.

@Roasbeef

Roasbeef Dec 4, 2018

Member

Possible can name it something like ModifyStatus? As it's a bit awkward that atm you need to call Enable with a false bool rather than something like Disable.

This comment has been minimized.

@halseth

halseth Dec 5, 2018

Collaborator

What do you think about SetEnabled or SetActive?

This comment has been minimized.

@Roasbeef

Roasbeef Dec 12, 2018

Member

Those could work as well, personally I lean towards ModifyStatus as in the future we can do things like modify the status of things like it's "operating type", so: end user, routing node, etc. Of course, this can also be a follow up later.

This comment has been minimized.

@halseth

halseth Dec 13, 2018

Collaborator

Renamed RPCs to Status and ModifyStatus.

// it doesn't already exist) will have.
macaroonOps = []bakery.Op{
{
Entity: "autopilot",

This comment has been minimized.

@Roasbeef

Roasbeef Dec 4, 2018

Member

So one question is if we want to require an entirely new macaroon for interaction with the pilot sub-sever, or if we want to use the existing entity mapping. FWIW, the latter route would allow the admin.macaroon to use the service as is without having to generate an additional set of macaroons. However, if we make these a series of "sub-commands" for lncli, then these sub-commands can themselves know where to fetch the proper macaroon.

This comment has been minimized.

@halseth

halseth Dec 5, 2018

Collaborator

I'm fine with reusing existing macaroons, as you don't really gain more access than what you already have with the regular macaroon.

However, it would be nice if subservers had a standardized macaroon setup.

This comment has been minimized.

@Roasbeef

Roasbeef Dec 12, 2018

Member

Yeah re macaroon set up, once the primary sub-servers are in, following your suggestion, we can modify them to simply present which macaroons they need, and then the (to be fleshed out) bakery would handle baking them.

This comment has been minimized.

@Roasbeef

Roasbeef Dec 12, 2018

Member

Had some thoughts about this earlier w.r.t a "catch all" capability in the admin macaroon. This is a follow up though, and would require one last (hopefully) change to the admin macaroon to basically give it a "God" capability. All other method calls would by pass their regular checks if a macaron with this capability was presented.

This comment has been minimized.

@halseth

halseth Dec 13, 2018

Collaborator

Ended up removing custom autopilot macaroons completely for now, and instead require the info/read permission for getting status, and offchain/write + onchain/write for modifying the status.

Show resolved Hide resolved .gitignore
Show resolved Hide resolved cmd/lncli/main.go Outdated
@halseth

This comment has been minimized.

Copy link
Collaborator

halseth commented Dec 5, 2018

@AndrewSamokhvalov Check out the PR #2212. It is still WIP, but I think it covers what you want :) It will let you provide your own scores for node, and choose the weight given to the active heuristics.

@halseth halseth force-pushed the halseth:autopilot-rpcserver branch 3 times, most recently from 43598ec to 325a9ee Dec 5, 2018

@Roasbeef

This comment has been minimized.

Copy link
Member

Roasbeef commented Dec 12, 2018

Ready to test this out after we rebase and squash the fixup commits!

@halseth halseth force-pushed the halseth:autopilot-rpcserver branch from 325a9ee to e87a7e3 Dec 13, 2018

halseth added some commits Dec 13, 2018

autopilot: add autopilot.Manager
This commit adds a new type Manager responsible for managing an
autopilot agent, making it possible to start and stop it at runtime.
pilot+lnd: let autopilot.Manager manage pilot-agent
This commit moves the responsibility of managing the life cycle of the
autopilot from main to the autopilot Manager. It utilizes the recently
introduced autopilot Manager, and just sets up the necessary interfaces
for the Manager to properly set up the required subscriptions when
starting the agent.

@halseth halseth force-pushed the halseth:autopilot-rpcserver branch from e87a7e3 to 64d13bd Dec 13, 2018

lnrpc: add autopilotrpc subservice
This commit adds a new service lnrpc/autopilot, that is to be used to
communicate with the running autopilot. Currently a RPC for getting the
status of the agent is included, as well as enabling/disabling at
runtime.

halseth added some commits Dec 13, 2018

@halseth halseth force-pushed the halseth:autopilot-rpcserver branch from 64d13bd to 5b0b2b9 Dec 13, 2018

@halseth

This comment has been minimized.

Copy link
Collaborator

halseth commented Dec 13, 2018

Rebased , squashed and last comments addressed.

lncli: add lncli autopilot commands
This commit adds commands 'status', 'enable' and 'disable' for
autopilot.

The commands will only be enabled for autopilotrpc builds of lncli.

@halseth halseth force-pushed the halseth:autopilot-rpcserver branch from 5b0b2b9 to 80ac8c1 Dec 13, 2018

@Roasbeef
Copy link
Member

Roasbeef left a comment

LGTM 🎉

@Roasbeef Roasbeef merged commit 0fafd5e into lightningnetwork:master Dec 14, 2018

1 of 2 checks passed

coverage/coveralls Coverage decreased (-0.09%) to 55.651%
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment