rust-lightning icon indicating copy to clipboard operation
rust-lightning copied to clipboard

Add `unregister_tx`/`unregister_output` to `chain::Filter`

Open tnull opened this issue 3 years ago • 2 comments

Currently, a user only gets informed about the transactions to be monitored via Filter::register_tx/Filter::unregister_tx.

We however do not provide any feedback when it would be fine to stop monitoring any registered transactions. Once we're able to drop individual ChannelMonitors, we should therefore add unregister_tx/unregister_output methods to the chain::Filter trait.

tnull avatar Aug 11 '22 12:08 tnull

Hi @tnull,

Is this still relevant and part of the work you mentioned here?

enigbe avatar Oct 01 '24 11:10 enigbe

Hi @tnull,

Is this still relevant and part of the work you mentioned here?

Yes, I think it's generally still relevant as unregister_tx/unregister_output would allow our syncing logic to reduce the set of watched transactions/outputs, e.g., when a ChannelMonitor was archived. However, note that this is a really really minor optimization as we'd reset the state of watched items on restart anyways. That is, this optimization would only improve things for long-running nodes that use Filter-based syncing, i.e., Esplora/Electrum, or BIP157/158 backends. But usually long-running nodes would likely use full-block syncing, although if they're lighter weight they might use BIP157/158 in the future.

TLDR is: yes, this is a minor optimization that we should eventually consider but not super pressing as few users would actually benefit from it. And note it's not exactly 'part of' the linked issue, as Electrum syncing works totally fine without this optimization.

Btw, an alternative approach to 'subtracting' the transactions/outputs from the watched set via dedicated unregister_tx/unregister_output methods would be to just regularly reset the syncing client state and add back what we still need (gained via ChannelMonitor::load_outputs_to_watch).

tnull avatar Oct 01 '24 11:10 tnull