Skip to content

Post-#2314 Cleanups #2765

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Dec 6, 2023
Merged

Post-#2314 Cleanups #2765

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
65e37fd
Ensure `ChannelMonitor` `Logger`s are always wrapped with metadata
TheBlueMatt Dec 3, 2023
de480d0
Lean on `Logger` wrapping in `ChannelMonitor` rather than in channel
TheBlueMatt Dec 2, 2023
ab93d35
Add some missing `WithContext` wrappers in `peer_handler.rs`
TheBlueMatt Dec 2, 2023
18a18d8
Correct lock drop'ing in `ChainMonitor::update_channel`
TheBlueMatt Dec 2, 2023
ed75ce3
Drop `log_bytes!()` calls on `Display` types in `ChannelManager`
TheBlueMatt Dec 2, 2023
4f0d5ed
Remove redundant locking when creating `WithChannelMonitor`
TheBlueMatt Dec 3, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 9 additions & 14 deletions lightning/src/chain/chainmonitor.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -620,9 +620,8 @@ where C::Target: chain::Filter,
pub fn rebroadcast_pending_claims(&self) {
let monitors = self.monitors.read().unwrap();
for (_, monitor_holder) in &*monitors {
let logger = WithChannelMonitor::from(&self.logger, &monitor_holder.monitor);
monitor_holder.monitor.rebroadcast_pending_claims(
&*self.broadcaster, &*self.fee_estimator, &logger
&*self.broadcaster, &*self.fee_estimator, &self.logger
)
}
}
Expand All @@ -640,19 +639,17 @@ where
fn filtered_block_connected(&self, header: &Header, txdata: &TransactionData, height: u32) {
log_debug!(self.logger, "New best block {} at height {} provided via block_connected", header.block_hash(), height);
self.process_chain_data(header, Some(height), &txdata, |monitor, txdata| {
let logger = WithChannelMonitor::from(&self.logger, &monitor);
monitor.block_connected(
header, txdata, height, &*self.broadcaster, &*self.fee_estimator, &logger)
header, txdata, height, &*self.broadcaster, &*self.fee_estimator, &self.logger)
});
}

fn block_disconnected(&self, header: &Header, height: u32) {
let monitor_states = self.monitors.read().unwrap();
log_debug!(self.logger, "Latest block {} at height {} removed via block_disconnected", header.block_hash(), height);
for monitor_state in monitor_states.values() {
let logger = WithChannelMonitor::from(&self.logger, &monitor_state.monitor);
monitor_state.monitor.block_disconnected(
header, height, &*self.broadcaster, &*self.fee_estimator, &logger);
header, height, &*self.broadcaster, &*self.fee_estimator, &self.logger);
}
}
}
Expand All @@ -669,30 +666,28 @@ where
fn transactions_confirmed(&self, header: &Header, txdata: &TransactionData, height: u32) {
log_debug!(self.logger, "{} provided transactions confirmed at height {} in block {}", txdata.len(), height, header.block_hash());
self.process_chain_data(header, None, txdata, |monitor, txdata| {
let logger = WithChannelMonitor::from(&self.logger, &monitor);
monitor.transactions_confirmed(
header, txdata, height, &*self.broadcaster, &*self.fee_estimator, &logger)
header, txdata, height, &*self.broadcaster, &*self.fee_estimator, &self.logger)
});
}

fn transaction_unconfirmed(&self, txid: &Txid) {
log_debug!(self.logger, "Transaction {} reorganized out of chain", txid);
let monitor_states = self.monitors.read().unwrap();
for monitor_state in monitor_states.values() {
let logger = WithChannelMonitor::from(&self.logger, &monitor_state.monitor);
monitor_state.monitor.transaction_unconfirmed(txid, &*self.broadcaster, &*self.fee_estimator, &logger);
monitor_state.monitor.transaction_unconfirmed(txid, &*self.broadcaster, &*self.fee_estimator, &self.logger);
}
}

fn best_block_updated(&self, header: &Header, height: u32) {
log_debug!(self.logger, "New best block {} at height {} provided via best_block_updated", header.block_hash(), height);
self.process_chain_data(header, Some(height), &[], |monitor, txdata| {
let logger = WithChannelMonitor::from(&self.logger, &monitor);
// While in practice there shouldn't be any recursive calls when given empty txdata,
// it's still possible if a chain::Filter implementation returns a transaction.
debug_assert!(txdata.is_empty());
monitor.best_block_updated(
header, height, &*self.broadcaster, &*self.fee_estimator, &logger)
header, height, &*self.broadcaster, &*self.fee_estimator, &self.logger
)
});
}

Expand Down Expand Up @@ -758,8 +753,7 @@ where C::Target: chain::Filter,

fn update_channel(&self, funding_txo: OutPoint, update: &ChannelMonitorUpdate) -> ChannelMonitorUpdateStatus {
// Update the monitor that watches the channel referred to by the given outpoint.
let monitors_lock = self.monitors.read().unwrap();
let monitors = monitors_lock.deref();
let monitors = self.monitors.read().unwrap();
match monitors.get(&funding_txo) {
None => {
log_error!(self.logger, "Failed to update channel monitor: no such monitor registered");
Expand Down Expand Up @@ -802,6 +796,7 @@ where C::Target: chain::Filter,
ChannelMonitorUpdateStatus::UnrecoverableError => {
// Take the monitors lock for writing so that we poison it and any future
// operations going forward fail immediately.
core::mem::drop(pending_monitor_updates);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for catching this 😬

core::mem::drop(monitors);
let _poison = self.monitors.write().unwrap();
let err_str = "ChannelMonitor[Update] persistence failed unrecoverably. This indicates we cannot continue normal operation and must shut down.";
Expand Down
Loading