Add MempoolInterface to add mempool scan

[ariard]

Local access to the mempool view enables routing hops and LSP to perfom more efficiently two tasks:

• passing backward HTLC preimage from the outbound edge to the inbound one (see ChannelMonitor::get_pending_or_resolved_outbound_htlcs())
• detecting an unconfirmed dual-funding double spend and freeing up the locked UTXOS (cf. liquidity griefing for 0-conf dual-funded txs)

For the first task, if a) our counterparty realizes a force-close and b) there are non-expired received HTLC outputs on the force-closed commitment transaction and c) our counterparty broadcast one or more HTLC-success transactions on those outputs, once those latest transactions are well-propagated on the network mempools, we can fetch the preimage and transmit it to the ChannelManager for pending off-chain inbound HTLCs resolution (i.e sending an update_fulfill_htlc to our counterparty on the inbound edge). Liquidity is therefore unlock faster on the inbound edge without having to wait on counterparty's commitment transaction confirmation.

For the second task, once we have dual-funding functional, we might accept counterparty's TxAddInput in the V2 channel establishment and commit some of our own inputs in the collaborative transaction for liquidity balanced channel opening. However, as long as the channel is not confirmed, we're exposed to double-spend by our counterparty locking up our contributed UTXOs in a liquidity griefing, at the detrimental of others potential dual-funding flows we could enter into.

This PR implements #2341 by introducing a MempoolInterface. This interface has for now a single method watch_outpoint(), where the implementation should return any in-mempool descendant for the given OutPoint. This can be implemented given Bitcoin Core's getmempooldescendant (or I think a very similar rpc getutxomempooldescendant that I'll have to land there as a first step).

Few integration approaches for a default MempoolInterface implementation can be considered:

• make it a new trait object in ChainMonitor ?
• make it a simple trait like FeeEstimator or FeeEstimator and defer implementation to downstream LDK users ?

The first option has the advantage to have a more robust and consistent integration if the mempool interface becomes
more complex in function of mempool evolution works on the Core side (e.g package policy or mempool clusters).

Other mempool scanning interfaces approaches can be considered before to make the necesary changes on the Core side to have a functional end-to-end mempool scanning for review.

[codecov-commenter]

Codecov Report

Patch and project coverage have no change.

Comparison is base (e404c12) 90.31% compared to head (8ccafab) 90.31%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #2404   +/-   ##
=======================================
  Coverage   90.31%   90.31%           
=======================================
  Files         106      106           
  Lines       55166    55166           
  Branches    55166    55166           
=======================================
  Hits        49822    49822           
  Misses       5344     5344           

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

[TheBlueMatt]

This doesn't actually accomplish anything - is there some specific feedback you want? Otherwise we should discuss on #2341 whether we actually want to do this.

[ariard]

This doesn't actually accomplish anything - is there some specific feedback you want?

Yes I’m blocked on those questions to work on a concrete implementation (and the ChainMonitor direction involves dirty passing the new trait object in a bunch of places, I think that might scarred folks as a plausible approach).

Few integration approaches for a default MempoolInterface implementation can be considered:

make it a new trait object in ChainMonitor ?
make it a simple trait like FeeEstimator or FeeEstimator and defer implementation to downstream LDK users ?

Somehow there is the assumption than a ChainMonitor has already access to block-relay to watch channel utxo spends and access to transaction-relay to broadcast reaction like justice txn so you can assume it has already access to a local mempool. On the other hand, the second approach enables to have a set of “real” mempool (i.e multiple Bitcoin Core nodes) under a single MempoolInterface, which makes the robustness of the second use better (mitigate liquidity griefing for 0-conf dual-funded txs), I think the biggest architectural trade-off implications out of my mind ?

[TheBlueMatt]

Yes I’m blocked on those questions to work on a concrete implementation (and the ChainMonitor direction involves dirty passing the new trait object in a bunch of places, I think that might scarred folks as a plausible approach).

Right, my point is before there's something to review the discussion belongs on the issue, rather than as a PR :). More specifically, I'm still not convinced we should do this, I'd appreciate a longer discussion on the linked issue.

[ariard]

More specifically, I'm still not convinced we should do this, I'd appreciate a longer discussion on the linked issue.

I don’t deny we can benefit from a longer discussion, added a new comment on the issue. I’m thinking we might combine both FeeEstimator and MempoolInterface in a single interface, as they kinda assume the same resource on the implementation-side (e.g direct or delegated access to a mempool).

[TheBlueMatt]

Let's discuss it on the issue and not leave this PR open? I don't like PRs hanging around forever and polluting the PR count :).

[ariard]

Let's discuss it on the issue and not leave this PR open? I don't like PRs hanging around forever and polluting the PR count :).

Replied on the issue, if the use-case a) is thought as relevant I'm happy to propose a minimal implementation of it to kick the ball rolling. So good if we can let it open (at least until we disagree mempool scanning is bad idea for all the use-cases I mention on the issue).

[TheBlueMatt]

This isn't a PR, this is a discussion starter API draft with no code. Let's discuss it on the issue and we can reopen this when we figure out (a) if we want it and (b) what the API should look like. This PR's full content could easily fit into a comment on that issue :).

[ariard]

This isn't a PR, this is a discussion starter API draft with no code. Let's discuss it on the issue and we can reopen this when we figure out (a) if we want it and (b) what the API should look like. This PR's full content could easily fit into a comment on that issue :).

Replied and have a look as I start to be worried you don’t understand Lightning :)