Repository navigation
Expose the node's KV store to applications #1174
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -190,8 +190,8 @@ use probing::{run_prober, Prober}; | |
| use runtime::Runtime; | ||
| pub use tokio; | ||
| use types::{ | ||
| Broadcaster, BumpTransactionEventHandler, ChainMonitor, ChannelManager, DynStore, Graph, | ||
| KeysManager, OnionMessenger, PaymentStore, PeerManager, Router, Scorer, Sweeper, Wallet, | ||
| Broadcaster, BumpTransactionEventHandler, ChainMonitor, ChannelManager, DynStore, DynStoreRef, | ||
| Graph, KeysManager, OnionMessenger, PaymentStore, PeerManager, Router, Scorer, Sweeper, Wallet, | ||
| }; | ||
| pub use types::{ | ||
| ChannelCounterparty, ChannelDetails, CustomTlvRecord, PeerDetails, ReserveType, UserChannelId, | ||
|
|
@@ -982,6 +982,19 @@ impl Node { | |
| self.config.as_ref().clone() | ||
| } | ||
|
|
||
| /// Returns a handle to the [`KVStore`] the [`Node`] persists its state to. | ||
| /// | ||
| /// This allows applications to persist their own data in the same store as the node, e.g., to | ||
| /// keep it in the same database and back it up together. | ||
| /// | ||
| /// **Note:** Applications must only read and write namespaces of their own. Modifying or | ||
| /// removing any data written by the node itself may lead to loss of funds. | ||
| /// | ||
| /// [`KVStore`]: lightning::util::persist::KVStore | ||
| pub fn kv_store(&self) -> DynStoreRef { | ||
| DynStoreRef(Arc::clone(&self.kv_store)) | ||
| } | ||
|
Comment on lines
+994
to
+996
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since this hands out a clone of the Could we document that all handles need to be dropped before building a new node on the same store? Probably also worth mentioning that lease-loss panics can now surface in app tasks when they write through this. |
||
|
|
||
| /// Returns the next event in the event queue, if currently available. | ||
| /// | ||
| /// Will return `Some(..)` if an event is available and `None` otherwise. | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -562,6 +562,34 @@ async fn address_pool_is_reloaded_on_restart() { | |
| expect_channel_ready_event!(node_b, node_a.node_id()); | ||
| } | ||
|
|
||
| #[tokio::test(flavor = "multi_thread", worker_threads = 1)] | ||
| async fn application_data_persists_in_node_kv_store() { | ||
| let (bitcoind, electrsd) = setup_bitcoind_and_electrsd(); | ||
| let chain_source = random_chain_source(&bitcoind, &electrsd); | ||
| let mut config = random_config(); | ||
| config.store_type = TestStoreType::Sqlite; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
|
|
||
| let (primary_namespace, secondary_namespace) = ("app", "data"); | ||
| let node = setup_node(&chain_source, config.clone()); | ||
| let kv_store = node.kv_store(); | ||
| KVStore::write(&kv_store, primary_namespace, secondary_namespace, "key", vec![42; 32]) | ||
| .await | ||
| .unwrap(); | ||
| node.stop().unwrap(); | ||
| drop(node); | ||
| drop(kv_store); | ||
|
|
||
| // Data written through the handle is persisted alongside the node's state and survives a | ||
| // restart. | ||
| let node = setup_node(&chain_source, config); | ||
| let kv_store = node.kv_store(); | ||
| let data = KVStore::read(&kv_store, primary_namespace, secondary_namespace, "key").await; | ||
| assert_eq!(data.unwrap(), vec![42; 32]); | ||
| let keys = KVStore::list(&kv_store, primary_namespace, secondary_namespace).await.unwrap(); | ||
| assert_eq!(keys, vec!["key".to_string()]); | ||
| node.stop().unwrap(); | ||
| } | ||
|
|
||
| #[cfg(feature = "chain-bitcoind")] | ||
| #[tokio::test(flavor = "multi_thread", worker_threads = 1)] | ||
| async fn channel_full_cycle() { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How would an app know which namespaces are its own? The node and LDK use "", monitors, payments, bdk_wallet, etc., but those constants are all pub(crate). There's also nothing stopping a future release from adding a namespace an app already uses.
Given the loss-of-funds risk, could we enforce this instead of only documenting it? E.g. return a small wrapper that rejects the empty primary namespace and the ones we use internally, or require some prefix like
app_. At minimum it'd be good to list the reserved ones here. What do you think?