lightningdevkit / rust-lightning

A highly modular Bitcoin Lightning library written in Rust. It's rust-lightning, not Rusty's Lightning!
Other
1.17k stars 367 forks source link

Update Channel Monitor without broadcasting transactions #3396

Closed yellowred closed 2 weeks ago

yellowred commented 3 weeks ago

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.

yellowred commented 3 weeks ago

@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 commented 3 weeks ago

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?

yellowred commented 2 weeks ago

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 commented 2 weeks ago

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

yellowred commented 7 hours ago

@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 https://github.com/lightningdevkit/rust-lightning/pull/3428