Skip to content
Merged
Changes from all commits
Commits
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
96 changes: 94 additions & 2 deletions src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1934,7 +1934,16 @@ fn build_with_store_internal(

// Initialize the ChannelManager
let channel_manager = {
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,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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 🤷‍♂️

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, well, NotFound is at least part of the KVStore API contract.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No we removed that when we fixed MonitorUpdatingPersister to no longer rely on it.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

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.

Err(e) => {
log_error!(logger, "Failed to read channel manager from store: {}", e);
return Err(BuildError::ReadFailed);
},
};

if let Some(reader) = channel_manager_bytes {
let channel_monitor_references =
channel_monitors.iter().map(|(_, chanmon)| chanmon).collect();
let read_args = ChannelManagerReadArgs::new(
Expand Down Expand Up @@ -2426,7 +2435,90 @@ pub(crate) fn sanitize_alias(alias_str: &str) -> Result<NodeAlias, BuildError> {

#[cfg(test)]
mod tests {
use super::{sanitize_alias, BuildError, NodeAlias};
use std::future::Future;
use std::sync::Arc;

use lightning::io;
use lightning::util::persist::{
KVStore, PageToken, PaginatedKVStore, PaginatedListResponse,
CHANNEL_MANAGER_PERSISTENCE_KEY, CHANNEL_MANAGER_PERSISTENCE_PRIMARY_NAMESPACE,
CHANNEL_MANAGER_PERSISTENCE_SECONDARY_NAMESPACE,
};

use super::{sanitize_alias, BuildError, NodeAlias, NodeBuilder};
use crate::entropy::NodeEntropy;
use crate::io::test_utils::InMemoryStore;
use crate::logger::Logger;

struct ChannelManagerReadFailingStore(InMemoryStore);

impl KVStore for ChannelManagerReadFailingStore {
fn read(
&self, primary_namespace: &str, secondary_namespace: &str, key: &str,
) -> impl Future<Output = Result<Vec<u8>, io::Error>> + 'static + Send {
let fail_read = primary_namespace == CHANNEL_MANAGER_PERSISTENCE_PRIMARY_NAMESPACE
&& secondary_namespace == CHANNEL_MANAGER_PERSISTENCE_SECONDARY_NAMESPACE
&& key == CHANNEL_MANAGER_PERSISTENCE_KEY;
let read = KVStore::read(&self.0, primary_namespace, secondary_namespace, key);
async move {
if fail_read {
Err(io::Error::new(io::ErrorKind::Other, "channel manager read failed"))
} else {
read.await
}
}
}

fn write(
&self, primary_namespace: &str, secondary_namespace: &str, key: &str, buf: Vec<u8>,
) -> impl Future<Output = Result<(), io::Error>> + 'static + Send {
KVStore::write(&self.0, primary_namespace, secondary_namespace, key, buf)
}

fn remove(
&self, primary_namespace: &str, secondary_namespace: &str, key: &str, lazy: bool,
) -> impl Future<Output = Result<(), io::Error>> + 'static + Send {
KVStore::remove(&self.0, primary_namespace, secondary_namespace, key, lazy)
}

fn list(
&self, primary_namespace: &str, secondary_namespace: &str,
) -> impl Future<Output = Result<Vec<String>, io::Error>> + 'static + Send {
KVStore::list(&self.0, primary_namespace, secondary_namespace)
}
}

impl PaginatedKVStore for ChannelManagerReadFailingStore {
fn list_paginated(
&self, primary_namespace: &str, secondary_namespace: &str,
page_token: Option<PageToken>,
) -> impl Future<Output = Result<PaginatedListResponse, io::Error>> + 'static + Send {
PaginatedKVStore::list_paginated(
&self.0,
primary_namespace,
secondary_namespace,
page_token,
)
}
}

#[test]
fn channel_manager_read_failure_fails_build() {
let builder = NodeBuilder::new();
let logger = Arc::new(Logger::new_log_facade());
#[cfg(not(feature = "uniffi"))]
let node_entropy = NodeEntropy::from_seed_bytes([42; 64]);
#[cfg(feature = "uniffi")]
let node_entropy = NodeEntropy::from_seed_bytes(vec![42; 64]).unwrap();

let result = builder.build_with_store_and_logger(
node_entropy,
ChannelManagerReadFailingStore(InMemoryStore::new()),
logger,
);

assert!(matches!(result, Err(BuildError::ReadFailed)));
}

#[test]
fn sanitize_empty_node_alias() {
Expand Down
Loading