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

Remove connection manager #4730

Closed
palango opened this issue Sep 3, 2019 · 15 comments · Fixed by #6622
Closed

Remove connection manager #4730

palango opened this issue Sep 3, 2019 · 15 comments · Fixed by #6622
Assignees
Labels
Type / Optimization Issues that are performance related
Milestone

Comments

@palango
Copy link
Contributor

palango commented Sep 3, 2019

The connection manager in theory is a nice feature, but currently leads to UX problems (@kelsos can you chime in here) and also is the source of bugs:

We should think about removing it and supporting the use cases in other ways.

@LefterisJP
Copy link
Contributor

Hmm I wouldn't remove the connection manager. I see it as an important component of Raiden.

Lightning also has the same thing and it's called autopilot: https://github.com/lightningnetwork/lnd/tree/master/autopilot

It's one of their most used features.

I understand it's buggy, but wouldn't fixing the bugs be what we want? Instead of just removing it?

@kelsos
Copy link
Contributor

kelsos commented Sep 3, 2019

Yes, so I agree that the idea behind the connection manager is really nice however the current implementation for a user perspective is not really helpful.

There is no guarantee that the channels opened by the connection manager belong to nodes that are first online/available and secondly have some kind of capacity to be usable.

As a user starting Raiden for the first time, I would expect after using the connection manager to actually be able to use the open channels so that I can send to other nodes in the network. Currently, there is absolutely no guarantee that I will be able to do this because all the nodes might be offline.

I am also towards having a proper mechanism that can help users connect to the network. Currently finding online nodes is really hard for a user that wants just test raiden in any network.

@LefterisJP
Copy link
Contributor

There is no guarantee that the channels opened by the connection manager belong to nodes that are first online/available and secondly have some kind of capacity to be usable.

Open an issue for that and we can improve it.

As a user starting Raiden for the first time, I would expect after using the connection manager to actually be able to use the open channels so that I can send to other nodes in the network. Currently, there is absolutely no guarantee that I will be able to do this because all the nodes might be offline.

Should be handled by the above issue, of opening nodes only with online partners.
Plus if there are not enough online people available we should bail out and show a relevant message to the user.

Relevant old issue: #576

@kelsos
Copy link
Contributor

kelsos commented Sep 3, 2019

Ok, will do so.

@palango
Copy link
Contributor Author

palango commented Sep 3, 2019

I understand it's buggy, but wouldn't fixing the bugs be what we want? Instead of just removing it?

I think we can provide similar UX in a simpler way.
For example we could display a list of online nodes with good position in the network in the WebUI and have connect buttons to open channels with them. That would have the same effect but would simplify our code.
For the backend we could implement a simple script doing the same thing, while dropping the whole complexity if the future integrated into the core.

@LefterisJP
Copy link
Contributor

For the backend we could implement a simple script doing the same thing, while dropping the whole complexity if the future integrated into the core.

A simple script is not enough. It needs to be informed enough to be able to implement various strategies to achieve a good connection with the network. Best place to be able to do that is as a module of the raiden client.

Again check at what lightning does and what strategies they implement. Here is a nice github issue where you can get hints on their current autopilot default strategies and how different strategies can be implemented there: lightningnetwork/lnd#677

@palango
Copy link
Contributor Author

palango commented Sep 3, 2019

A simple script is not enough. It needs to be informed enough to be able to implement various strategies to achieve a good connection with the network. Best place to be able to do that is as a module of the raiden client.

I think the obvious source for this information is the PFS, and that could be queried by a script.

@rakanalh
Copy link
Contributor

rakanalh commented Sep 3, 2019

I think having a bad UX in a component shouldn't lead to it's removal but rather more improvement.

TBQH, i am all for simplification but against removing the connection manager. We can convert this issue to "refactor".

@christianbrb
Copy link
Contributor

Maybe raiden-network/raiden-services#684 is the way to go.

@karlb
Copy link
Contributor

karlb commented Oct 6, 2020

We have the decision to remove the connection manager from the python code base. The plan is to add a simple replacement in a higher layer (e.g. web UI) relying on the partner suggestion endpoint of the PFS. Can we share code for this between the light client and the python client's web UI? @manuelwedler

@manuelwedler
Copy link
Contributor

We don't share any code between them yet, so we would have to introduce some shared library. But if it is just querying the suggestions from the PFS and opening a channel with them, I don't see a necessity for it. For more complicated logic it might make sense.

@christianbrb
Copy link
Contributor

@karlb I like your suggestion :) Could you create an issue within the WebUI repo (https://github.com/raiden-network/webui/issues/new/choose) roughly describing your proposal. Thanks :)

@karlb
Copy link
Contributor

karlb commented Oct 6, 2020

WebUI issue is here: raiden-network/webui#547

See also https://hackmd.io/qVuEq6YTR-SZ3n9otJyEuw for additional context on the CM removal decision.

@christianbrb
Copy link
Contributor

Great, thanks :)

@karlb karlb added this to the Bespin milestone Oct 9, 2020
palango pushed a commit that referenced this issue Oct 12, 2020
The connection manager will be removed in
#4730, so we don't need
to run that scenario, anymore.

Closes #6591.

[skip tests]
@karlb
Copy link
Contributor

karlb commented Oct 20, 2020

Planned changes to the API in https://raiden-network.readthedocs.io/en/latest/rest_api.html#connection-management:

  • remove PUT
  • remove "funds" key from GET
  • keep the rest as is

@karlb karlb self-assigned this Oct 20, 2020
@konradkonrad konradkonrad added the Type / Optimization Issues that are performance related label Nov 9, 2020
fredo pushed a commit that referenced this issue Nov 9, 2020
The connection manager is removed from the raiden client and replaced by
a simplified version in the WebUI
(raiden-network/webui#547).

Closes #4730.

More documentation updates are still needed, but the duplication between
https://docs.raiden.network/ and https://raiden-network.readthedocs.io
should be cleared up, first.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type / Optimization Issues that are performance related
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants