WIP: A new plugin to find good peers | explore the LN #82

darosior · 2020-01-12T19:05:44Z

The algorithm is a bit sketchy but it can be improved over time, and I already made some good finds with the plugin.

cdecker

Very nice plugin, just some minor suggestions.

Seeing that this overlaps in scope with the autopilot plugin, would it make sense to merge the two?

goodpeer/goodpeer.py

cdecker · 2020-01-17T16:15:39Z

goodpeer/goodpeer.py

+
+def get_medians(channels):
+    """
+    Return the (median_base_fee, median_fees_ppm) of the given peer.


Which direction are you considering here? Or does that not matter? I would have expected merchants to look for nodes with a small incoming fee, and end-users for a small outgoing fee.

Ah right, I was only considering end-user here !

(Considered outgoing fees)

I added another optional parameter

Actually the direction argument doesn't make that much sense, as when scanning nodes I take the fees this node sets (source), taking the fees its peers set (destination) doesn't seem relevant.

I would have expected merchants to look for nodes with a small incoming fee, and end-users for a small outgoing fee.

Re-thinking it I had outgoing fees in mind in any case, and assumed that if we fund a channel to a peer which always set somewhat small outgoing fees, it would set it to us too. So that we can route through it, and receive through it.

goodpeer/goodpeer.py

darosior · 2020-01-18T14:27:55Z

Seeing that this overlaps in scope with the autopilot plugin, would it make sense to merge the two?

Ah I've never read the autopilot. Since this is just part of it maybe it can just be duplicated ? I'd like to use this without autopilot, maybe so do other users.

darosior · 2020-01-18T16:21:03Z

Removed the just added direction parameter

cdecker · 2020-01-19T17:23:40Z

Seeing that this overlaps in scope with the autopilot plugin, would it make sense to merge the two?

Ah I've never read the autopilot. Since this is just part of it maybe it can just be duplicated ? I'd like to use this without autopilot, maybe so do other users.

The autopilot doesn't really do anything unless you call the exposed RPC commands, and even most of the commands are just suggestions, kind of a discovery mechanism. This is pretty similar in scope to that, that's why I'm suggesting it.

It's just a bit confusing for users to have multiple options to achieve the same goal, I think.

darosior · 2020-01-23T16:37:04Z

I had to (re) open channels recently, which permitted me to tweak the results:

The filters were too low. 20$ isnt enough
The average fees can be a secondary metric
Add the possibility to filter out old channels, fees policy may have changed since

Median is great, but a lot of the results will have the same score if we restrict the scoring to just that. We apply a <1 factor to the mean though, as it would otherwise distort the result otherwise.

Useful to filter out channels established a while ago

renepickhardt

I would strongly suggest to extend lib autopilot. And have everything with suggesting nodes / channels in one place.

I have more ideas of suggesting channels based on the potential ability to rebalance making use of the recent research results. As far as I understand your plugin this does actually go in this direction.

darosior · 2020-02-05T10:28:37Z

Late reply: I need to read the autopilot to integrate it, but I've thought a bit more about that and I think I'm going to extend this to be more kind of an "explorer" as soon as I can. Marking WIP then.

m-schmoock

ACK. Tested and works as promised.
This has been dormant for a while, are we go with this?
Needs rebase for the readme... and anyway some nits:

I have a docstring warning in the RPC method:

INFO    plugin-goodpeer.py: RPC method 'goodpeers' does not have a docstring.

Additional we have some W504 nits:

goodpeer/goodpeer.py:55:45: W504 line break after binary operator
goodpeer/goodpeer.py:82:49: W504 line break after binary operator
goodpeer/goodpeer.py:84:47: W504 line break after binary operator

@cdecker I think we can allow W504 (maybe also in main repo) as I think it increases readability when used like in the above occurences.

m-schmoock · 2020-11-30T10:07:25Z

goodpeer/descriptions.py

+goodpeers_small_desc = "Get a list of good enough peers to connect to, reversely sorted."
+
+goodpeers_desc = \
+"""


Nit: E122 continuation line missing indentation or outdented

m-schmoock · 2020-11-30T10:25:10Z

goodpeer/goodpeer.py

+@plugin.method("goodpeers", desc=goodpeers_small_desc,
+               long_desc=goodpeers_desc)
+def goodpeers(plugin, bias=None, min_height=None, **kwargs):
+    nodes = get_nodes(plugin)


We should use the docstring here, maybe instead of the desc_long=... above.

darosior · 2020-11-30T11:09:05Z

Thanks for giving it a look :-) I have mixed feelings about this plugin: - I think it should certainly not be in autopilot, now that we have CLBOSS i think we should focus here on smaller parts of what consists a comprehensive autopilot so people (and us actually) can choose to run only smaller parts. - I think it's great and got me great finds. However, the best peers to connect to are.. A way more complicated matter. If we merge this i should probably rename it to "blackfridaypeers" :p - I'm still thinking about the explorer i mentioned previously. It will probably be called "edges" and will allow research of peers sorted by new edges created, which could be weighted. It will be a completely new plugin however and should not condition the fortune of this one :) ‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐ Le lundi, novembre 30, 2020 11:28 AM, Michael Schmoock <notifications@github.com> a écrit :

…

@m-schmoock approved this pull request. ACK. Tested and works as promised. This has been dormant for a while, are we go with this? Anyway some nits: I have a docstring warning in the RPC method: INFO plugin-goodpeer.py: RPC method 'goodpeers' does not have a docstring. Additional we have some W504 nits: goodpeer/goodpeer.py:55:45: W504 line break after binary operator goodpeer/goodpeer.py:82:49: W504 line break after binary operator goodpeer/goodpeer.py:84:47: W504 line break after binary operator ***@***.***(https://github.com/cdecker) I think we can allow W504 (maybe also in main repo) as I think it increases readability when used like in the above occurences. --------------------------------------------------------------- In [goodpeer/descriptions.py](#82 (comment)): > @@ -0,0 +1,15 @@ +goodpeers_small_desc = "Get a list of good enough peers to connect to, reversely sorted." + +goodpeers_desc = \ +""" Nit: E122 continuation line missing indentation or outdented --------------------------------------------------------------- In [goodpeer/goodpeer.py](#82 (comment)): > + return median_base, median_ppm, avg_base, avg_ppm + + +def reverse_sort(nodes): + """ + Reversely sort the nodes, the lower the score the better. + """ + # Python3.5 hasn't native dicts ordered.. So use OrderedDict + return OrderedDict(sorted(nodes.items(), + key=lambda item: item[1]["score"], reverse=True)) + + ***@***.*** ("goodpeers", desc=goodpeers_small_desc, + long_desc=goodpeers_desc) +def goodpeers(plugin, bias=None, min_height=None, **kwargs): + nodes = get_nodes(plugin) We should use the docstring here, maybe instead of the desc_long=... above. — You are receiving this because you authored the thread. Reply to this email directly, [view it on GitHub](#82 (review)), or [unsubscribe](https://github.com/notifications/unsubscribe-auth/AFLK3FYO4G6MBZQASVROMLTSSNXUJANCNFSM4KFZKLZA).

m-schmoock · 2020-11-30T11:15:45Z

@darosior "blackfridaypeers" :D

One thing that came to my mind is that it would be good to limit the output to just the best 20 peers or something, maybe a parameter that can be set to 100 or all if one wants more output.

cdecker reviewed Jan 17, 2020

View reviewed changes

darosior added 5 commits January 18, 2020 17:03

goodpeer: A plugin for finding good enough low-fees peers

d253b65

goodpeer: add a README

9e7cde8

goodpeer.py: add the possibility to tweak the scoring

f50e58f

goodpeer: add a basic starting test

0268ba9

README: Add goodpeer to the list

fa46bed

darosior force-pushed the goodpeers branch 2 times, most recently from 9f36355 to fa46bed Compare January 18, 2020 16:20

darosior added 3 commits January 23, 2020 17:40

goodpeer: increase the minimum channel amount

aa94fe2

goodpeer: take the average fees into account for the scoring

0e7a9b2

Median is great, but a lot of the results will have the same score if we restrict the scoring to just that. We apply a <1 factor to the mean though, as it would otherwise distort the result otherwise.

goodpeer: add a min_height argument

ae10abc

Useful to filter out channels established a while ago

darosior force-pushed the goodpeers branch from a2df4f3 to ae10abc Compare January 23, 2020 16:41

renepickhardt reviewed Jan 25, 2020

View reviewed changes

darosior changed the title ~~A new plugin to find peers~~ WIP: A new plugin to find good peers | explore the LN Feb 5, 2020

cdecker marked this pull request as draft August 28, 2020 14:45

m-schmoock approved these changes Nov 30, 2020

View reviewed changes

cdecker closed this Sep 25, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WIP: A new plugin to find good peers | explore the LN #82

WIP: A new plugin to find good peers | explore the LN #82

darosior commented Jan 12, 2020

cdecker left a comment

cdecker Jan 17, 2020

darosior Jan 18, 2020

darosior Jan 18, 2020

darosior Jan 18, 2020

darosior Jan 18, 2020

darosior commented Jan 18, 2020

darosior commented Jan 18, 2020

cdecker commented Jan 19, 2020

darosior commented Jan 23, 2020

renepickhardt left a comment

darosior commented Feb 5, 2020

m-schmoock left a comment •

edited

Loading

m-schmoock Nov 30, 2020

m-schmoock Nov 30, 2020

darosior commented Nov 30, 2020 via email

m-schmoock commented Nov 30, 2020

WIP: A new plugin to find good peers | explore the LN #82

WIP: A new plugin to find good peers | explore the LN #82

Conversation

darosior commented Jan 12, 2020

cdecker left a comment

Choose a reason for hiding this comment

cdecker Jan 17, 2020

Choose a reason for hiding this comment

darosior Jan 18, 2020

Choose a reason for hiding this comment

darosior Jan 18, 2020

Choose a reason for hiding this comment

darosior Jan 18, 2020

Choose a reason for hiding this comment

darosior Jan 18, 2020

Choose a reason for hiding this comment

darosior commented Jan 18, 2020

darosior commented Jan 18, 2020

cdecker commented Jan 19, 2020

darosior commented Jan 23, 2020

renepickhardt left a comment

Choose a reason for hiding this comment

darosior commented Feb 5, 2020

m-schmoock left a comment • edited Loading

Choose a reason for hiding this comment

m-schmoock Nov 30, 2020

Choose a reason for hiding this comment

m-schmoock Nov 30, 2020

Choose a reason for hiding this comment

darosior commented Nov 30, 2020 via email

m-schmoock commented Nov 30, 2020

m-schmoock left a comment •

edited

Loading