Update Channel Monitor without broadcasting transactions

[yellowred]

Allowing Channel Monitor to be updated without executing any commands outside ChannelMonitor. This allows reading Channel Monitor in MonitorUpdatingPersister without using broadcaster and fee estimator and broadcast claims when the node is ready for it.
For example, in a multi tenant environment, a server could load channels data in one place, initialize auxiliary services and perform node configuration, then send the result to another process, that would start a node with all required background processes and one of these processes would broadcast the transactions.

• Is it safe to loose returned transactions?

[yellowred]

@TheBlueMatt Replaced private update_monitor with update_monitor_opt_broadcast. Appreciate if you could take a look to confirm if it goes in the right direction. Specifically If it is ok to make broadcaster optional in private functions so that they may return claims rather than broadcasting them.

[TheBlueMatt]

Instead of touching all the code here, wouldn't it be simpler to just build a Broadcaster that stores the packages and use that as the Broadcaster passed to the inner methods?

[TheBlueMatt]

I'm not entirely sold on returning the packages to broadcast here - if a user is running in a situation where they don't have the ability to register a Broadcaster that can broadcast why can they broadcast a set of transactions we hand to them here? Instead, should we consider storing the packages in the ChannelMonitor to broadcast as soon as we see the node being up (eg a block gets connected)?

[yellowred]

Instead of touching all the code here, wouldn't it be simpler to just build a Broadcaster that stores the packages and use that as the Broadcaster passed to the inner methods?

Followed this advice and implemented VecBroadcaster that does not broadcast transactions immediately, but accumulates them first:

/// Transaction broadcaster that does not broadcast transactions, but accumulates
/// them in a Vec instead. This could be used to delay broadcasts until the system
/// is ready.
pub struct VecBroadcaster {
    channel_id: ChannelId,
    transactions: Mutex<Vec<Transaction>>,
}

impl VecBroadcaster {
    /// Create a new broadcaster for a channel
    pub fn new(channel_id: ChannelId) -> Self {
        Self {
            channel_id,
            transactions: Mutex::new(Vec::new()),
        }
    }

    /// Used to actually broadcast stored transactions to the network.
    #[instrument(skip_all, fields(channel = %self.channel_id))]
    pub fn release_transactions(&self, broadcaster: Arc<dyn BroadcasterInterface>) {
        let transactions = self.transactions.lock();
        info!(
            "Releasing transactions for channel_id={}, len={}",
            self.channel_id,
            transactions.len()
        );
        broadcaster.broadcast_transactions(&transactions.iter().collect::<Vec<&Transaction>>())
    }
}

impl BroadcasterInterface for VecBroadcaster {
    fn broadcast_transactions(&self, txs: &[&Transaction]) {
        let mut tx_storage = self.transactions.lock();
        for tx in txs {
            tx_storage.push((*tx).to_owned())
        }
    }
}

Will close this PR.

[TheBlueMatt]

We could still take that upstream as a part of ChannelMonitor, though indeed its not all that hard to build outside of LDK.

[yellowred]

@TheBlueMatt Hey, yeah, that would be useful to have. I drafted another PR with CMon that stores claims in inner and then releases them on block_connected #3428