The goal of this actor is to maintain a local cache of all channel data, that can be used by the API, instead of querying thousands of channel actors. Surprisingly, this is turns out to not be noticeably faster than the ask+Future.sequence pattern.

Maybe we should remove this? Or we could keep it and use that for all remaining API calls, because it saves some burden of the channel actors? I'm not sure.

I think it's useful to have it, when we'll have more load on the channel actors it should be noticeably useful, don't you think?

nit: MutualCloseBalance would be more consistent IMO

Actually, this is just additional information about mutual close transactions, useful to track evictions. The mutual close balance is tracked as part of onchain balance, see:

  /**
   * Mutual close transactions are always immediately published, and will be taken into account by bitcoin core, so we
   * don't count them in the balance.
   */
  case class ClosingBalance

But now that I think of it, we should probably treat mutual closes like other transactions, in the offchain balance, and prune it afterwards. The goal of pruning was initially to remove duplicates for transactions that were published (and possibly confirmed but less than mindepth), but it should also nicely handle evicted transactions.

Ok, I remember now why the mutual close transactions were handled like this. The problem is that there can be several competing mutual closes, and in that case the simple pruning logic that we have to make sure we count the amount exactly once doesn't work. This would lead us to possibly count twice the amount in the competing-mutual-closes scenario.

On the other hand, what we had before meant that we didn't count the amount in the case where the mutual close was evicted. Not perfect either, and this is why we had the additional MutualCloseStatus item.

In general I would say that it is better to undershoot rather than overshoot the balance computation, but here the trade-off makes sense for consistency, and also it is easy to tell when we are potentially counting a mutual close twice: the mutualCloseBalance should always be zero after pruning.

So I went ahead and did it with 4b78392, with a detailed comment on what is going on.

We can leave it like that for now, but I'm not a big fan of adding new unrelated checks inside existing tests (especially when these tests are already quite long and complex).

It's a bit more code, but I would not put any balance test inside this file, I would instead write these tests in CheckBalanceSpec (where you would setup channels and HTLCs similarly to what's done here, but with the flexibility of changing the scenarios tested without impacting the rest of the test).