Skip to content

Commit 00254b9

Browse files
wpaulinowpaulino
committed
Avoid persisting ChannelManager on handle_tx_* errors
These errors will only ever affect our in-memory state, so there's no need to persist the ChannelManager when we come across one. Note that `tx_abort` is not included here because there is a possibility we force close the channel, which we should persist.
1 parent 3e8b060 commit 00254b9

File tree

1 file changed

+43
-43
lines changed

1 file changed

+43
-43
lines changed

‎lightning/src/ln/channelmanager.rs‎

Lines changed: 43 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -2935,6 +2935,22 @@ impl<'a> PersistenceNotifierGuard<'a, fn() -> NotifyOption> {
29352935
Self::optionally_notify(cm, || -> NotifyOption { NotifyOption::DoPersist })
29362936
}
29372937

2938+
fn manually_notify<F: FnOnce(), C: AChannelManager>(
2939+
cm: &'a C, f: F,
2940+
) -> PersistenceNotifierGuard<'a, impl FnOnce() -> NotifyOption> {
2941+
let read_guard = cm.get_cm().total_consistency_lock.read().unwrap();
2942+
let force_notify = cm.get_cm().process_background_events();
2943+
2944+
f();
2945+
2946+
PersistenceNotifierGuard {
2947+
event_persist_notifier: &cm.get_cm().event_persist_notifier,
2948+
needs_persist_flag: &cm.get_cm().needs_persist_flag,
2949+
should_persist: Some(move || force_notify),
2950+
_read_guard: read_guard,
2951+
}
2952+
}
2953+
29382954
fn optionally_notify<F: FnOnce() -> NotifyOption, C: AChannelManager>(
29392955
cm: &'a C, persist_check: F,
29402956
) -> PersistenceNotifierGuard<'a, impl FnOnce() -> NotifyOption> {
@@ -11336,7 +11352,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
1133611352
>(
1133711353
&self, counterparty_node_id: &PublicKey, channel_id: ChannelId,
1133811354
tx_msg_handler: HandleTxMsgFn,
11339-
) -> Result<NotifyOption, MsgHandleErrInternal> {
11355+
) -> Result<(), MsgHandleErrInternal> {
1134011356
let per_peer_state = self.per_peer_state.read().unwrap();
1134111357
let peer_state_mutex = per_peer_state.get(counterparty_node_id).ok_or_else(|| {
1134211358
debug_assert!(false);
@@ -11351,7 +11367,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
1135111367
Ok(msg_send) => {
1135211368
let msg_send_event = msg_send.into_msg_send_event(*counterparty_node_id);
1135311369
peer_state.pending_msg_events.push(msg_send_event);
11354-
Ok(NotifyOption::SkipPersistHandleEvents)
11370+
Ok(())
1135511371
},
1135611372
Err(InteractiveTxMsgError {
1135711373
err,
@@ -11389,39 +11405,39 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
1138911405

1139011406
fn internal_tx_add_input(
1139111407
&self, counterparty_node_id: PublicKey, msg: &msgs::TxAddInput,
11392-
) -> Result<NotifyOption, MsgHandleErrInternal> {
11408+
) -> Result<(), MsgHandleErrInternal> {
1139311409
self.internal_tx_msg(&counterparty_node_id, msg.channel_id, |channel: &mut Channel<SP>| {
1139411410
channel.tx_add_input(msg, &self.logger)
1139511411
})
1139611412
}
1139711413

1139811414
fn internal_tx_add_output(
1139911415
&self, counterparty_node_id: PublicKey, msg: &msgs::TxAddOutput,
11400-
) -> Result<NotifyOption, MsgHandleErrInternal> {
11416+
) -> Result<(), MsgHandleErrInternal> {
1140111417
self.internal_tx_msg(&counterparty_node_id, msg.channel_id, |channel: &mut Channel<SP>| {
1140211418
channel.tx_add_output(msg, &self.logger)
1140311419
})
1140411420
}
1140511421

1140611422
fn internal_tx_remove_input(
1140711423
&self, counterparty_node_id: PublicKey, msg: &msgs::TxRemoveInput,
11408-
) -> Result<NotifyOption, MsgHandleErrInternal> {
11424+
) -> Result<(), MsgHandleErrInternal> {
1140911425
self.internal_tx_msg(&counterparty_node_id, msg.channel_id, |channel: &mut Channel<SP>| {
1141011426
channel.tx_remove_input(msg, &self.logger)
1141111427
})
1141211428
}
1141311429

1141411430
fn internal_tx_remove_output(
1141511431
&self, counterparty_node_id: PublicKey, msg: &msgs::TxRemoveOutput,
11416-
) -> Result<NotifyOption, MsgHandleErrInternal> {
11432+
) -> Result<(), MsgHandleErrInternal> {
1141711433
self.internal_tx_msg(&counterparty_node_id, msg.channel_id, |channel: &mut Channel<SP>| {
1141811434
channel.tx_remove_output(msg, &self.logger)
1141911435
})
1142011436
}
1142111437

1142211438
fn internal_tx_complete(
1142311439
&self, counterparty_node_id: PublicKey, msg: &msgs::TxComplete,
11424-
) -> Result<NotifyOption, MsgHandleErrInternal> {
11440+
) -> Result<(), MsgHandleErrInternal> {
1142511441
let per_peer_state = self.per_peer_state.read().unwrap();
1142611442
let peer_state_mutex = per_peer_state.get(&counterparty_node_id).ok_or_else(|| {
1142711443
debug_assert!(false);
@@ -11434,15 +11450,12 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
1143411450
let chan = chan_entry.get_mut();
1143511451
match chan.tx_complete(msg, &self.fee_estimator, &self.logger) {
1143611452
Ok(tx_complete_result) => {
11437-
let mut persist = NotifyOption::SkipPersistNoEvents;
11438-
1143911453
if let Some(interactive_tx_msg_send) =
1144011454
tx_complete_result.interactive_tx_msg_send
1144111455
{
1144211456
let msg_send_event =
1144311457
interactive_tx_msg_send.into_msg_send_event(counterparty_node_id);
1144411458
peer_state.pending_msg_events.push(msg_send_event);
11445-
persist = NotifyOption::SkipPersistHandleEvents;
1144611459
};
1144711460

1144811461
if let Some(unsigned_transaction) = tx_complete_result.event_unsigned_tx {
@@ -11456,7 +11469,8 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
1145611469
None,
1145711470
));
1145811471
// We have a successful signing session that we need to persist.
11459-
persist = NotifyOption::DoPersist;
11472+
self.needs_persist_flag.store(true, Ordering::Release);
11473+
self.event_persist_notifier.notify()
1146011474
}
1146111475

1146211476
if let Some(FundingTxSigned {
@@ -11501,10 +11515,11 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
1150111515
}
1150211516

1150311517
// We have a successful signing session that we need to persist.
11504-
persist = NotifyOption::DoPersist;
11518+
self.needs_persist_flag.store(true, Ordering::Release);
11519+
self.event_persist_notifier.notify()
1150511520
}
1150611521

11507-
Ok(persist)
11522+
Ok(())
1150811523
},
1150911524
Err(InteractiveTxMsgError {
1151011525
err,
@@ -16182,62 +16197,47 @@ impl<
1618216197
}
1618316198

1618416199
fn handle_tx_add_input(&self, counterparty_node_id: PublicKey, msg: &msgs::TxAddInput) {
16185-
let _persistence_guard = PersistenceNotifierGuard::optionally_notify(self, || {
16200+
let _persistence_guard = PersistenceNotifierGuard::manually_notify(self, || {
1618616201
let res = self.internal_tx_add_input(counterparty_node_id, msg);
16187-
let persist = match &res {
16188-
Err(_) => NotifyOption::DoPersist,
16189-
Ok(persist) => *persist,
16190-
};
16202+
debug_assert!(res.as_ref().err().map_or(true, |err| !err.closes_channel()));
1619116203
let _ = self.handle_error(res, counterparty_node_id);
16192-
persist
16204+
self.event_persist_notifier.notify();
1619316205
});
1619416206
}
1619516207

1619616208
fn handle_tx_add_output(&self, counterparty_node_id: PublicKey, msg: &msgs::TxAddOutput) {
16197-
let _persistence_guard = PersistenceNotifierGuard::optionally_notify(self, || {
16209+
let _persistence_guard = PersistenceNotifierGuard::manually_notify(self, || {
1619816210
let res = self.internal_tx_add_output(counterparty_node_id, msg);
16199-
let persist = match &res {
16200-
Err(_) => NotifyOption::DoPersist,
16201-
Ok(persist) => *persist,
16202-
};
16211+
debug_assert!(res.as_ref().err().map_or(true, |err| !err.closes_channel()));
1620316212
let _ = self.handle_error(res, counterparty_node_id);
16204-
persist
16213+
self.event_persist_notifier.notify();
1620516214
});
1620616215
}
1620716216

1620816217
fn handle_tx_remove_input(&self, counterparty_node_id: PublicKey, msg: &msgs::TxRemoveInput) {
16209-
let _persistence_guard = PersistenceNotifierGuard::optionally_notify(self, || {
16218+
let _persistence_guard = PersistenceNotifierGuard::manually_notify(self, || {
1621016219
let res = self.internal_tx_remove_input(counterparty_node_id, msg);
16211-
let persist = match &res {
16212-
Err(_) => NotifyOption::DoPersist,
16213-
Ok(persist) => *persist,
16214-
};
16220+
debug_assert!(res.as_ref().err().map_or(true, |err| !err.closes_channel()));
1621516221
let _ = self.handle_error(res, counterparty_node_id);
16216-
persist
16222+
self.event_persist_notifier.notify();
1621716223
});
1621816224
}
1621916225

1622016226
fn handle_tx_remove_output(&self, counterparty_node_id: PublicKey, msg: &msgs::TxRemoveOutput) {
16221-
let _persistence_guard = PersistenceNotifierGuard::optionally_notify(self, || {
16227+
let _persistence_guard = PersistenceNotifierGuard::manually_notify(self, || {
1622216228
let res = self.internal_tx_remove_output(counterparty_node_id, msg);
16223-
let persist = match &res {
16224-
Err(_) => NotifyOption::DoPersist,
16225-
Ok(persist) => *persist,
16226-
};
16229+
debug_assert!(res.as_ref().err().map_or(true, |err| !err.closes_channel()));
1622716230
let _ = self.handle_error(res, counterparty_node_id);
16228-
persist
16231+
self.event_persist_notifier.notify();
1622916232
});
1623016233
}
1623116234

1623216235
fn handle_tx_complete(&self, counterparty_node_id: PublicKey, msg: &msgs::TxComplete) {
16233-
let _persistence_guard = PersistenceNotifierGuard::optionally_notify(self, || {
16236+
let _persistence_guard = PersistenceNotifierGuard::manually_notify(self, || {
1623416237
let res = self.internal_tx_complete(counterparty_node_id, msg);
16235-
let persist = match &res {
16236-
Err(_) => NotifyOption::DoPersist,
16237-
Ok(persist) => *persist,
16238-
};
16238+
debug_assert!(res.as_ref().err().map_or(true, |err| !err.closes_channel()));
1623916239
let _ = self.handle_error(res, counterparty_node_id);
16240-
persist
16240+
self.event_persist_notifier.notify();
1624116241
});
1624216242
}
1624316243

0 commit comments

Comments
 (0)