Skip to content

Commit 98e6a8e

Browse files
TheBlueMattTheBlueMatt
committed
Drop verbose *_SECONDARY_NAMESPACE consts that are always ""
The constants in `lightning::util::persist` are sufficiently long that its often difficult eyeball their correctness which nearly led to several bugs when adding async support. As it turns out, all of the `*_SECONDARY_NAMESPACE` constants were simply "", so having a huge pile of them everywhere is quite verbose and makes scanning the `*_NAMESPACE` constants more difficult. Here, we simply drop the `*_SECONDARY_NAMESPACE` constants entirely.
1 parent 1660c72 commit 98e6a8e

File tree

4 files changed

+100
-209
lines changed

4 files changed

+100
-209
lines changed

‎lightning-background-processor/src/lib.rs‎

Lines changed: 37 additions & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -53,10 +53,8 @@ use lightning::sign::{
5353
};
5454
use lightning::util::logger::Logger;
5555
use lightning::util::persist::{
56-
KVStore, KVStoreSync, KVStoreSyncWrapper, CHANNEL_MANAGER_KEY,
57-
CHANNEL_MANAGER_PRIMARY_NAMESPACE, CHANNEL_MANAGER_SECONDARY_NAMESPACE, NETWORK_GRAPH_KEY,
58-
NETWORK_GRAPH_PRIMARY_NAMESPACE, NETWORK_GRAPH_SECONDARY_NAMESPACE, SCORER_KEY,
59-
SCORER_PRIMARY_NAMESPACE, SCORER_SECONDARY_NAMESPACE,
56+
KVStore, KVStoreSync, KVStoreSyncWrapper, CHANNEL_MANAGER_KEY, CHANNEL_MANAGER_NAMESPACE,
57+
NETWORK_GRAPH_KEY, NETWORK_GRAPH_NAMESPACE, SCORER_KEY, SCORER_NAMESPACE,
6058
};
6159
use lightning::util::sweep::{OutputSweeper, OutputSweeperSync};
6260
#[cfg(feature = "std")]
@@ -941,14 +939,8 @@ where
941939
if let Some(duration_since_epoch) = fetch_time() {
942940
if update_scorer(scorer, &event, duration_since_epoch) {
943941
log_trace!(logger, "Persisting scorer after update");
944-
if let Err(e) = kv_store
945-
.write(
946-
SCORER_PRIMARY_NAMESPACE,
947-
SCORER_SECONDARY_NAMESPACE,
948-
SCORER_KEY,
949-
scorer.encode(),
950-
)
951-
.await
942+
if let Err(e) =
943+
kv_store.write(SCORER_NAMESPACE, "", SCORER_KEY, scorer.encode()).await
952944
{
953945
log_error!(logger, "Error: Failed to persist scorer, check your disk and permissions {}", e);
954946
// We opt not to abort early on persistence failure here as persisting
@@ -1079,8 +1071,8 @@ where
10791071
let fut = async {
10801072
kv_store
10811073
.write(
1082-
CHANNEL_MANAGER_PRIMARY_NAMESPACE,
1083-
CHANNEL_MANAGER_SECONDARY_NAMESPACE,
1074+
CHANNEL_MANAGER_NAMESPACE,
1075+
"",
10841076
CHANNEL_MANAGER_KEY,
10851077
channel_manager.get_cm().encode(),
10861078
)
@@ -1142,8 +1134,8 @@ where
11421134
let fut = async {
11431135
if let Err(e) = kv_store
11441136
.write(
1145-
NETWORK_GRAPH_PRIMARY_NAMESPACE,
1146-
NETWORK_GRAPH_SECONDARY_NAMESPACE,
1137+
NETWORK_GRAPH_NAMESPACE,
1138+
"",
11471139
NETWORK_GRAPH_KEY,
11481140
network_graph.encode(),
11491141
)
@@ -1182,14 +1174,8 @@ where
11821174
log_trace!(logger, "Persisting scorer");
11831175
}
11841176
let fut = async {
1185-
if let Err(e) = kv_store
1186-
.write(
1187-
SCORER_PRIMARY_NAMESPACE,
1188-
SCORER_SECONDARY_NAMESPACE,
1189-
SCORER_KEY,
1190-
scorer.encode(),
1191-
)
1192-
.await
1177+
if let Err(e) =
1178+
kv_store.write(SCORER_NAMESPACE, "", SCORER_KEY, scorer.encode()).await
11931179
{
11941180
log_error!(
11951181
logger,
@@ -1300,30 +1286,18 @@ where
13001286
// ChannelMonitor update(s) persisted without a corresponding ChannelManager update.
13011287
kv_store
13021288
.write(
1303-
CHANNEL_MANAGER_PRIMARY_NAMESPACE,
1304-
CHANNEL_MANAGER_SECONDARY_NAMESPACE,
1289+
CHANNEL_MANAGER_NAMESPACE,
1290+
"",
13051291
CHANNEL_MANAGER_KEY,
13061292
channel_manager.get_cm().encode(),
13071293
)
13081294
.await?;
13091295
if let Some(ref scorer) = scorer {
1310-
kv_store
1311-
.write(
1312-
SCORER_PRIMARY_NAMESPACE,
1313-
SCORER_SECONDARY_NAMESPACE,
1314-
SCORER_KEY,
1315-
scorer.encode(),
1316-
)
1317-
.await?;
1296+
kv_store.write(SCORER_NAMESPACE, "", SCORER_KEY, scorer.encode()).await?;
13181297
}
13191298
if let Some(network_graph) = gossip_sync.network_graph() {
13201299
kv_store
1321-
.write(
1322-
NETWORK_GRAPH_PRIMARY_NAMESPACE,
1323-
NETWORK_GRAPH_SECONDARY_NAMESPACE,
1324-
NETWORK_GRAPH_KEY,
1325-
network_graph.encode(),
1326-
)
1300+
.write(NETWORK_GRAPH_NAMESPACE, "", NETWORK_GRAPH_KEY, network_graph.encode())
13271301
.await?;
13281302
}
13291303
Ok(())
@@ -1525,12 +1499,9 @@ impl BackgroundProcessor {
15251499
.expect("Time should be sometime after 1970");
15261500
if update_scorer(scorer, &event, duration_since_epoch) {
15271501
log_trace!(logger, "Persisting scorer after update");
1528-
if let Err(e) = kv_store.write(
1529-
SCORER_PRIMARY_NAMESPACE,
1530-
SCORER_SECONDARY_NAMESPACE,
1531-
SCORER_KEY,
1532-
scorer.encode(),
1533-
) {
1502+
if let Err(e) =
1503+
kv_store.write(SCORER_NAMESPACE, "", SCORER_KEY, scorer.encode())
1504+
{
15341505
log_error!(logger, "Error: Failed to persist scorer, check your disk and permissions {}", e)
15351506
}
15361507
}
@@ -1626,8 +1597,8 @@ impl BackgroundProcessor {
16261597
if channel_manager.get_cm().get_and_clear_needs_persistence() {
16271598
log_trace!(logger, "Persisting ChannelManager...");
16281599
(kv_store.write(
1629-
CHANNEL_MANAGER_PRIMARY_NAMESPACE,
1630-
CHANNEL_MANAGER_SECONDARY_NAMESPACE,
1600+
CHANNEL_MANAGER_NAMESPACE,
1601+
"",
16311602
CHANNEL_MANAGER_KEY,
16321603
channel_manager.get_cm().encode(),
16331604
))?;
@@ -1665,8 +1636,8 @@ impl BackgroundProcessor {
16651636
duration_since_epoch.as_secs(),
16661637
);
16671638
if let Err(e) = kv_store.write(
1668-
NETWORK_GRAPH_PRIMARY_NAMESPACE,
1669-
NETWORK_GRAPH_SECONDARY_NAMESPACE,
1639+
NETWORK_GRAPH_NAMESPACE,
1640+
"",
16701641
NETWORK_GRAPH_KEY,
16711642
network_graph.encode(),
16721643
) {
@@ -1693,12 +1664,9 @@ impl BackgroundProcessor {
16931664
.expect("Time should be sometime after 1970");
16941665
log_trace!(logger, "Calling time_passed and persisting scorer");
16951666
scorer.write_lock().time_passed(duration_since_epoch);
1696-
if let Err(e) = kv_store.write(
1697-
SCORER_PRIMARY_NAMESPACE,
1698-
SCORER_SECONDARY_NAMESPACE,
1699-
SCORER_KEY,
1700-
scorer.encode(),
1701-
) {
1667+
if let Err(e) =
1668+
kv_store.write(SCORER_NAMESPACE, "", SCORER_KEY, scorer.encode())
1669+
{
17021670
log_error!(logger, "Error: Failed to persist scorer, check your disk and permissions {}", e);
17031671
}
17041672
}
@@ -1734,23 +1702,18 @@ impl BackgroundProcessor {
17341702
// some races where users quit while channel updates were in-flight, with
17351703
// ChannelMonitor update(s) persisted without a corresponding ChannelManager update.
17361704
kv_store.write(
1737-
CHANNEL_MANAGER_PRIMARY_NAMESPACE,
1738-
CHANNEL_MANAGER_SECONDARY_NAMESPACE,
1705+
CHANNEL_MANAGER_NAMESPACE,
1706+
"",
17391707
CHANNEL_MANAGER_KEY,
17401708
channel_manager.get_cm().encode(),
17411709
)?;
17421710
if let Some(ref scorer) = scorer {
1743-
kv_store.write(
1744-
SCORER_PRIMARY_NAMESPACE,
1745-
SCORER_SECONDARY_NAMESPACE,
1746-
SCORER_KEY,
1747-
scorer.encode(),
1748-
)?;
1711+
kv_store.write(SCORER_NAMESPACE, "", SCORER_KEY, scorer.encode())?;
17491712
}
17501713
if let Some(network_graph) = gossip_sync.network_graph() {
17511714
kv_store.write(
1752-
NETWORK_GRAPH_PRIMARY_NAMESPACE,
1753-
NETWORK_GRAPH_SECONDARY_NAMESPACE,
1715+
NETWORK_GRAPH_NAMESPACE,
1716+
"",
17541717
NETWORK_GRAPH_KEY,
17551718
network_graph.encode(),
17561719
)?;
@@ -1843,10 +1806,8 @@ mod tests {
18431806
use lightning::types::payment::PaymentHash;
18441807
use lightning::util::config::UserConfig;
18451808
use lightning::util::persist::{
1846-
KVStoreSync, KVStoreSyncWrapper, CHANNEL_MANAGER_KEY, CHANNEL_MANAGER_PRIMARY_NAMESPACE,
1847-
CHANNEL_MANAGER_SECONDARY_NAMESPACE, NETWORK_GRAPH_KEY, NETWORK_GRAPH_PRIMARY_NAMESPACE,
1848-
NETWORK_GRAPH_SECONDARY_NAMESPACE, SCORER_KEY, SCORER_PRIMARY_NAMESPACE,
1849-
SCORER_SECONDARY_NAMESPACE,
1809+
KVStoreSync, KVStoreSyncWrapper, CHANNEL_MANAGER_KEY, CHANNEL_MANAGER_NAMESPACE,
1810+
NETWORK_GRAPH_KEY, NETWORK_GRAPH_NAMESPACE, SCORER_KEY, SCORER_NAMESPACE,
18501811
};
18511812
use lightning::util::ser::Writeable;
18521813
use lightning::util::sweep::{
@@ -2101,19 +2062,15 @@ mod tests {
21012062
fn write(
21022063
&self, primary_namespace: &str, secondary_namespace: &str, key: &str, buf: Vec<u8>,
21032064
) -> lightning::io::Result<()> {
2104-
if primary_namespace == CHANNEL_MANAGER_PRIMARY_NAMESPACE
2105-
&& secondary_namespace == CHANNEL_MANAGER_SECONDARY_NAMESPACE
2106-
&& key == CHANNEL_MANAGER_KEY
2107-
{
2065+
if primary_namespace == CHANNEL_MANAGER_NAMESPACE && key == CHANNEL_MANAGER_KEY {
2066+
assert_eq!(secondary_namespace, "");
21082067
if let Some((error, message)) = self.manager_error {
21092068
return Err(std::io::Error::new(error, message).into());
21102069
}
21112070
}
21122071

2113-
if primary_namespace == NETWORK_GRAPH_PRIMARY_NAMESPACE
2114-
&& secondary_namespace == NETWORK_GRAPH_SECONDARY_NAMESPACE
2115-
&& key == NETWORK_GRAPH_KEY
2116-
{
2072+
if primary_namespace == NETWORK_GRAPH_NAMESPACE && key == NETWORK_GRAPH_KEY {
2073+
assert_eq!(secondary_namespace, "");
21172074
if let Some(sender) = &self.graph_persistence_notifier {
21182075
match sender.send(()) {
21192076
Ok(()) => {},
@@ -2128,10 +2085,8 @@ mod tests {
21282085
}
21292086
}
21302087

2131-
if primary_namespace == SCORER_PRIMARY_NAMESPACE
2132-
&& secondary_namespace == SCORER_SECONDARY_NAMESPACE
2133-
&& key == SCORER_KEY
2134-
{
2088+
if primary_namespace == SCORER_NAMESPACE && key == SCORER_KEY {
2089+
assert_eq!(secondary_namespace, "");
21352090
if let Some((error, message)) = self.scorer_error {
21362091
return Err(std::io::Error::new(error, message).into());
21372092
}

‎lightning/src/ln/chanmon_update_fail_tests.rs‎

Lines changed: 10 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,7 @@ use crate::ln::types::ChannelId;
2828
use crate::sign::NodeSigner;
2929
use crate::util::native_async::FutureQueue;
3030
use crate::util::persist::{
31-
MonitorName, MonitorUpdatingPersisterAsync, CHANNEL_MONITOR_PRIMARY_NAMESPACE,
32-
CHANNEL_MONITOR_SECONDARY_NAMESPACE, MONITOR_UPDATE_PRIMARY_NAMESPACE,
31+
MonitorName, MonitorUpdatingPersisterAsync, CHANNEL_MONITOR_NAMESPACE, MONITOR_UPDATE_NAMESPACE,
3332
};
3433
use crate::util::ser::{ReadableArgs, Writeable};
3534
use crate::util::test_channel_signer::TestChannelSigner;
@@ -4938,11 +4937,7 @@ fn native_async_persist() {
49384937

49394938
let funding_txo = OutPoint { txid: funding_tx.compute_txid(), index: 0 };
49404939
let key = MonitorName::V1Channel(funding_txo).to_string();
4941-
let pending_writes = kv_store.list_pending_async_writes(
4942-
CHANNEL_MONITOR_PRIMARY_NAMESPACE,
4943-
CHANNEL_MONITOR_SECONDARY_NAMESPACE,
4944-
&key,
4945-
);
4940+
let pending_writes = kv_store.list_pending_async_writes(CHANNEL_MONITOR_NAMESPACE, "", &key);
49464941
assert_eq!(pending_writes.len(), 1);
49474942

49484943
// Once we complete the future, the write will still be pending until the future gets `poll`ed.
@@ -4970,21 +4965,19 @@ fn native_async_persist() {
49704965
persist_futures.poll_futures();
49714966
assert_eq!(async_chain_monitor.release_pending_monitor_events().len(), 0);
49724967

4973-
let pending_writes =
4974-
kv_store.list_pending_async_writes(MONITOR_UPDATE_PRIMARY_NAMESPACE, &key, "1");
4968+
let pending_writes = kv_store.list_pending_async_writes(MONITOR_UPDATE_NAMESPACE, &key, "1");
49754969
assert_eq!(pending_writes.len(), 1);
4976-
let pending_writes =
4977-
kv_store.list_pending_async_writes(MONITOR_UPDATE_PRIMARY_NAMESPACE, &key, "2");
4970+
let pending_writes = kv_store.list_pending_async_writes(MONITOR_UPDATE_NAMESPACE, &key, "2");
49784971
assert_eq!(pending_writes.len(), 1);
49794972

4980-
kv_store.complete_async_writes_through(MONITOR_UPDATE_PRIMARY_NAMESPACE, &key, "1", usize::MAX);
4973+
kv_store.complete_async_writes_through(MONITOR_UPDATE_NAMESPACE, &key, "1", usize::MAX);
49814974
persist_futures.poll_futures();
49824975
// While the `ChainMonitor` could return a `MonitorEvent::Completed` here, it currently
49834976
// doesn't. If that ever changes we should validate that the `Completed` event has the correct
49844977
// `monitor_update_id` (1).
49854978
assert!(async_chain_monitor.release_pending_monitor_events().is_empty());
49864979

4987-
kv_store.complete_async_writes_through(MONITOR_UPDATE_PRIMARY_NAMESPACE, &key, "2", usize::MAX);
4980+
kv_store.complete_async_writes_through(MONITOR_UPDATE_NAMESPACE, &key, "2", usize::MAX);
49884981
persist_futures.poll_futures();
49894982
let completed_persist = async_chain_monitor.release_pending_monitor_events();
49904983
assert_eq!(completed_persist.len(), 1);
@@ -5003,18 +4996,16 @@ fn native_async_persist() {
50034996
persist_futures.poll_futures();
50044997
assert_eq!(async_chain_monitor.release_pending_monitor_events().len(), 0);
50054998

5006-
let pending_writes =
5007-
kv_store.list_pending_async_writes(MONITOR_UPDATE_PRIMARY_NAMESPACE, &key, "3");
4999+
let pending_writes = kv_store.list_pending_async_writes(MONITOR_UPDATE_NAMESPACE, &key, "3");
50085000
assert_eq!(pending_writes.len(), 1);
5009-
let pending_writes =
5010-
kv_store.list_pending_async_writes(MONITOR_UPDATE_PRIMARY_NAMESPACE, &key, "4");
5001+
let pending_writes = kv_store.list_pending_async_writes(MONITOR_UPDATE_NAMESPACE, &key, "4");
50115002
assert_eq!(pending_writes.len(), 1);
50125003

5013-
kv_store.complete_async_writes_through(MONITOR_UPDATE_PRIMARY_NAMESPACE, &key, "4", usize::MAX);
5004+
kv_store.complete_async_writes_through(MONITOR_UPDATE_NAMESPACE, &key, "4", usize::MAX);
50145005
persist_futures.poll_futures();
50155006
assert_eq!(async_chain_monitor.release_pending_monitor_events().len(), 0);
50165007

5017-
kv_store.complete_async_writes_through(MONITOR_UPDATE_PRIMARY_NAMESPACE, &key, "3", usize::MAX);
5008+
kv_store.complete_async_writes_through(MONITOR_UPDATE_NAMESPACE, &key, "3", usize::MAX);
50185009
persist_futures.poll_futures();
50195010
let completed_persist = async_chain_monitor.release_pending_monitor_events();
50205011
assert_eq!(completed_persist.len(), 1);

0 commit comments

Comments
 (0)