ZmnSCPxj / clboss

Automated Core Lightning Node Manager
MIT License
207 stars 30 forks source link

ChannelCreationDecider moderation to consider the candidates (quantity/quality). #89

Open hoplaaa opened 2 years ago

hoplaaa commented 2 years ago

Following #81 and #84

We can have an empty list of candidates at least when :

After that, as long as there are 2 or more real candidates in the list, ChannelCreationDecidercan trigger again, creating a new batch of channel creation.

It loops on creating small batches of channels and with it on chain fees.

Instead, ChannelCreationDecidercould start only when the candidate list is mature. We can start with mature beeing candidate list full or 80% full and evolve to something better. ChannelCreationDecider or Janitor or ChannelCandidateInvestigator could monitor the candidate list and decide if it is mature.

That idea was already mentioned earlier and the issue is that at node creation we need to create channels to popular nodes peers to initiate something (ChannelFinderByPopularity), and then be able to find new candidates with other channel finder modules. A solution to that is to allow ChannelCreationDeciderto trigger ChannelCreatorwhen the node is not well connected which could be total number of channels < minimum number of initial channels (~5?)

ZmnSCPxj commented 2 years ago

A solution to that is to allow ChannelCreationDecider to trigger ChannelCreator when the node is not well connected which could be total number of channels < minimum number of initial channels (~5?)

More directly to code, if I understand you correctly: you are proposing that ChannelCreationDecider should:

?

Seems like a good idea, thanks.

hoplaaa commented 2 years ago

The idea is to create channels only when the candidate list is mature, unless the node is badly connected. Said otherwise, we exit ChannelCreationDecider when candidate list is not mature and node is well connected.

In ChannelCreationDecider , line 177 :

bool node_is_well_connected = number of channels >= some threshhold (5?)
if ( !candidate_list_is mature && node_is_well_connected ){
        return decide("Candidates list is not mature. Waiting for better proposals.", nullptr);
}

In ChannelCreationDecider , line 94/95 : Get candidate_list_is_mature state from ChannelCandidateInvestigator. We can start with candidate_list_is_mature if candidates.size()>26

hoplaaa commented 2 years ago

About candidate_list_is_mature state.

ChannelCandidateInvestigator could have a sub-module responsible to control the maturation process of the list. Think about :Aging_(food). I'm not native, the Refiner ? Refiner could be responsible to decide when the overall list is mature. The GumShoe which controls the candidates connectivity could be a sub module of Refiner. More sub-module could be added, see ideas in #83 : by candidates ages, by fees, ...

We can consider opening a new issue to discuss that specific idea if you like.

ZmnSCPxj commented 2 years ago

Ok, thanks. Was thinking Auditor would consider this "is-mature" algo. It gets access to Secretary on construction so it can look at the candidate list directly. It then responds to RequestIsChannelCandidateListMature messages with a ResponseIsChannelCandidateListMature and is accessed by ChannelCreationDecider via a ModG::ReqResp.

hoplaaa commented 2 years ago

Yes, as long as the Auditor's role is passive : to determine IsChannelCandidateListMature.

I was also proposing a more active approach to the maturing process within that new object, but I realize that this is already the role of ChannelCandidateInvestigator module itself, right ?

hoplaaa commented 2 years ago

So we have this trio working on candidates :

To keep the logic, we can extend Janitor role to kick candidates with bad connectivity score.

To extend it, we can add parameters, for example, age in the list: age_in_the_list to start at 0 when adding a candidate in the list. Gumshoedo +=1 any time he checks one candidate. Auditordecides that the list is not mature if a % of the candidates are too young.

ZmnSCPxj commented 2 years ago

kick candidates with bad connectivity score.

This is already done by the Manager and Secretary.

hoplaaa commented 2 years ago

kick candidates with bad connectivity score.

This is already done by the Manager and Secretary.

Yes, and I was proposing to move that part in the Janitor as this is also kicking badly noted candidates.

ZmnSCPxj commented 2 years ago

Hmm, how about this for maturity?

INFO plugin-clboss: ChannelCandidateInvestigator::Auditor: Candidate list is not yet mature: it has only X candidates, we want at least 5; the median onlineness score is Y, we want at least 6. This could take a while, sit back, relax and let CLBOSS keep investigating node uptime for maybe ~N more hours.

hoplaaa commented 2 years ago

Nice !

We could add more candidate quality parameters over time, like :

  1. channel_balanceness : for example 80% of channels are with less than 10% liquidity on one side of the channel is not good
  2. overall inbound/outbound balance : for example 80% of total liquidity on one side is not good
  3. response time : Average ping more than X second is not good. Maybe that one is already included in onelineness.

Actually 1 and 2 might be for a previous selection step. I didn't dig the code since a few months. If you are interested I can open a new issue for 1&2. 3 might be added directly to onlineness if not already implemented.

ZmnSCPxj commented 2 years ago

3 is a nice idea to add to the channel candidate investigator gumshoe. Unfortunately, if you do a lightning-cli connect to a random node (i.e. one you do not have a channel to) followed by lightning-cli ping, you will get a Peer bad state error message, at least on C-Lightning 0.10.2. Worse the error code is the very generic -1, sigh, which makes it difficult to differentiate from some other failure. The alternative is to look up the addresses via lightning-cli listnodes and do ICMP-level pings, but (1) ICMP is often gated behind root access, which we really really really want not to require for CLBOSS since it does too many things for such a thing to be safe, and (2) what about .onions. Given that a successful connect involves a fair amount of TCP-level handshaking which exceeds the one-time packet exchange that a BOLT-level ping-pong is, perhaps it would be better to just time-limit the connect attempt and treat it as failed if it took too long.

1 and 2 are hampered by our own liquidity; we can only probe their channel state by locking up nontrivial amounts on our own channels, and sending out a remote doomed payment runs the risk that something along the path (i.e. not the node we want to test) going down and locking our funds needlessly. Hmmm.