Fail closed on manager read errors - #1032
Conversation
Only a missing ChannelManager should create a fresh node. Propagate other storage errors so transient failures cannot replace live channel state with an empty manager. Fixes lightningdevkit#1026 Co-Authored-By: HAL 9000
|
I've assigned @TheBlueMatt as a reviewer! |
| if let Ok(reader) = channel_manager_bytes_res { | ||
| let channel_manager_bytes = match channel_manager_bytes_res { | ||
| Ok(reader) => Some(reader), | ||
| Err(e) if e.kind() == lightning::io::ErrorKind::NotFound => None, |
There was a problem hiding this comment.
Hmm, okay. I don't love reintroducing the assumption that the ErrorKind is "correct", but at worst we just refuse to start and the developer fixes their KVStore cause it should be obvious during development. Also I dunno how else to fix this so 🤷♂️
There was a problem hiding this comment.
Yeah, well, NotFound is at least part of the KVStore API contract.
There was a problem hiding this comment.
No we removed that when we fixed MonitorUpdatingPersister to no longer rely on it.
There was a problem hiding this comment.
No we removed that when we fixed MonitorUpdatingPersister to no longer rely on it.
Uh, but that's actually bad, as we do lean on it in LDK Node in several places. NotFound is always a special value for KVStores, irrespective of whether MonitorUpdatingPersister now returns it or not. We should re-add it then.
3 participants
Only a missing ChannelManager should create a fresh node. Propagate other storage errors so transient failures cannot replace live channel state with an empty manager.
Fixes #1026