Codecov Report

Attention: 18 lines in your changes are missing coverage. Please review.

Comparison is base (7b48964) 57.65% compared to head (1c7a61c) 57.71%.

@@            Coverage Diff             @@
##           master    #3781       /-   ##
==========================================
  Coverage   57.65%   57.71%    0.05%     
==========================================
  Files         193      193              
  Lines       42053    42044       -9     
==========================================
  Hits        24244    24264       20     
  Misses      17809    17780      -29     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

I'm not entirely sure if LN gateway code had some good reason to build new clients instead of cloning existing ones. @m1sterc001guy @okjodom

CI is failing

I'm not entirely sure if LN gateway code had some good reason to build new clients instead of cloning existing ones. @m1sterc001guy @okjodom

I think needing to inject lnrpc into the client was the reason? Not sure of it though

Will debug when I find some energy.

@okjodom @m1sterc001guy so it looks like this fails because something hangs in lnd gateawy, 99% because of the ArcClient re-use.

I tried to debug it but I have trouble understanding the code around it (I created #3807 motivated by it), and I don't have a good pre-existing understanding of the gateway architecture.

I don't even undersatnd why the gateway would need multiple client to the same federation, while the code (and my dbg! debugging statements) indicate that this can actually be the case? This is most probably fixable in some way, but hard to tell why without undrestanding the requirements.

dev call: dpc is blocked on this one because some lightning stuff is failing cc @okjodom @m1sterc001guy

I think I know the crux of the issue here. Today, we have a number of properties on GatewayClientModule and these properties are setup when the clients are created. One of these properties is a connection to the lightning node (Arc<dyn ILnRpcClient>). So when the connection to the lightning node goes down, we need to re-create this connection. Today we naively re-create all of the clients. I remember running into an issue that indicated we couldn't reopen an already existing connection to the database when re-creating the connection, hence the workaround to have DatabaseSource::Reuse so that we would just re-use the already opened db.

Ideally, I don't think we should be re-creating the clients at all. We should just create the federation clients when the gateway boots up and never again. So instead of passing the lightning node connection Arc<dyn ILnRpcClient> to GatewayClientModule, we should just pass a copy of the Gateway struct. Gateway is already clonable and has convenient methods for checking if the lightning node is online and accessing it. This would allow us to completely delete DatabaseSource::Reuse since we'd only ever open the database once, because we only create the clients once.

Was done in another PR