Skip to content

Commit a76aaa2

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 7f21bb2 commit a76aaa2

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
@@ -55,10 +55,8 @@ use lightning::sign::EntropySource;
5555
use lightning::sign::OutputSpender;
5656
use lightning::util::logger::Logger;
5757
use lightning::util::persist::{
58-
KVStore, KVStoreSync, KVStoreSyncWrapper, CHANNEL_MANAGER_KEY,
59-
CHANNEL_MANAGER_PRIMARY_NAMESPACE, CHANNEL_MANAGER_SECONDARY_NAMESPACE, NETWORK_GRAPH_KEY,
60-
NETWORK_GRAPH_PRIMARY_NAMESPACE, NETWORK_GRAPH_SECONDARY_NAMESPACE, SCORER_KEY,
61-
SCORER_PRIMARY_NAMESPACE, SCORER_SECONDARY_NAMESPACE,
58+
KVStore, KVStoreSync, KVStoreSyncWrapper, CHANNEL_MANAGER_KEY, CHANNEL_MANAGER_NAMESPACE,
59+
NETWORK_GRAPH_KEY, NETWORK_GRAPH_NAMESPACE, SCORER_KEY, SCORER_NAMESPACE,
6260
};
6361
use lightning::util::sweep::OutputSweeper;
6462
#[cfg(feature = "std")]
@@ -793,14 +791,8 @@ where
793791
if let Some(duration_since_epoch) = fetch_time() {
794792
if update_scorer(scorer, &event, duration_since_epoch) {
795793
log_trace!(logger, "Persisting scorer after update");
796-
if let Err(e) = kv_store
797-
.write(
798-
SCORER_PRIMARY_NAMESPACE,
799-
SCORER_SECONDARY_NAMESPACE,
800-
SCORER_KEY,
801-
scorer.encode(),
802-
)
803-
.await
794+
if let Err(e) =
795+
kv_store.write(SCORER_NAMESPACE, "", SCORER_KEY, scorer.encode()).await
804796
{
805797
log_error!(logger, "Error: Failed to persist scorer, check your disk and permissions {}", e);
806798
// We opt not to abort early on persistence failure here as persisting
@@ -931,8 +923,8 @@ where
931923
let fut = async {
932924
kv_store
933925
.write(
934-
CHANNEL_MANAGER_PRIMARY_NAMESPACE,
935-
CHANNEL_MANAGER_SECONDARY_NAMESPACE,
926+
CHANNEL_MANAGER_NAMESPACE,
927+
"",
936928
CHANNEL_MANAGER_KEY,
937929
channel_manager.get_cm().encode(),
938930
)
@@ -994,8 +986,8 @@ where
994986
let fut = async {
995987
if let Err(e) = kv_store
996988
.write(
997-
NETWORK_GRAPH_PRIMARY_NAMESPACE,
998-
NETWORK_GRAPH_SECONDARY_NAMESPACE,
989+
NETWORK_GRAPH_NAMESPACE,
990+
"",
999991
NETWORK_GRAPH_KEY,
1000992
network_graph.encode(),
1001993
)
@@ -1034,14 +1026,8 @@ where
10341026
log_trace!(logger, "Persisting scorer");
10351027
}
10361028
let fut = async {
1037-
if let Err(e) = kv_store
1038-
.write(
1039-
SCORER_PRIMARY_NAMESPACE,
1040-
SCORER_SECONDARY_NAMESPACE,
1041-
SCORER_KEY,
1042-
scorer.encode(),
1043-
)
1044-
.await
1029+
if let Err(e) =
1030+
kv_store.write(SCORER_NAMESPACE, "", SCORER_KEY, scorer.encode()).await
10451031
{
10461032
log_error!(
10471033
logger,
@@ -1141,30 +1127,18 @@ where
11411127
// ChannelMonitor update(s) persisted without a corresponding ChannelManager update.
11421128
kv_store
11431129
.write(
1144-
CHANNEL_MANAGER_PRIMARY_NAMESPACE,
1145-
CHANNEL_MANAGER_SECONDARY_NAMESPACE,
1130+
CHANNEL_MANAGER_NAMESPACE,
1131+
"",
11461132
CHANNEL_MANAGER_KEY,
11471133
channel_manager.get_cm().encode(),
11481134
)
11491135
.await?;
11501136
if let Some(ref scorer) = scorer {
1151-
kv_store
1152-
.write(
1153-
SCORER_PRIMARY_NAMESPACE,
1154-
SCORER_SECONDARY_NAMESPACE,
1155-
SCORER_KEY,
1156-
scorer.encode(),
1157-
)
1158-
.await?;
1137+
kv_store.write(SCORER_NAMESPACE, "", SCORER_KEY, scorer.encode()).await?;
11591138
}
11601139
if let Some(network_graph) = gossip_sync.network_graph() {
11611140
kv_store
1162-
.write(
1163-
NETWORK_GRAPH_PRIMARY_NAMESPACE,
1164-
NETWORK_GRAPH_SECONDARY_NAMESPACE,
1165-
NETWORK_GRAPH_KEY,
1166-
network_graph.encode(),
1167-
)
1141+
.write(NETWORK_GRAPH_NAMESPACE, "", NETWORK_GRAPH_KEY, network_graph.encode())
11681142
.await?;
11691143
}
11701144
Ok(())
@@ -1369,12 +1343,9 @@ impl BackgroundProcessor {
13691343
.expect("Time should be sometime after 1970");
13701344
if update_scorer(scorer, &event, duration_since_epoch) {
13711345
log_trace!(logger, "Persisting scorer after update");
1372-
if let Err(e) = kv_store.write(
1373-
SCORER_PRIMARY_NAMESPACE,
1374-
SCORER_SECONDARY_NAMESPACE,
1375-
SCORER_KEY,
1376-
scorer.encode(),
1377-
) {
1346+
if let Err(e) =
1347+
kv_store.write(SCORER_NAMESPACE, "", SCORER_KEY, scorer.encode())
1348+
{
13781349
log_error!(logger, "Error: Failed to persist scorer, check your disk and permissions {}", e)
13791350
}
13801351
}
@@ -1470,8 +1441,8 @@ impl BackgroundProcessor {
14701441
if channel_manager.get_cm().get_and_clear_needs_persistence() {
14711442
log_trace!(logger, "Persisting ChannelManager...");
14721443
(kv_store.write(
1473-
CHANNEL_MANAGER_PRIMARY_NAMESPACE,
1474-
CHANNEL_MANAGER_SECONDARY_NAMESPACE,
1444+
CHANNEL_MANAGER_NAMESPACE,
1445+
"",
14751446
CHANNEL_MANAGER_KEY,
14761447
channel_manager.get_cm().encode(),
14771448
))?;
@@ -1502,8 +1473,8 @@ impl BackgroundProcessor {
15021473
duration_since_epoch.as_secs(),
15031474
);
15041475
if let Err(e) = kv_store.write(
1505-
NETWORK_GRAPH_PRIMARY_NAMESPACE,
1506-
NETWORK_GRAPH_SECONDARY_NAMESPACE,
1476+
NETWORK_GRAPH_NAMESPACE,
1477+
"",
15071478
NETWORK_GRAPH_KEY,
15081479
network_graph.encode(),
15091480
) {
@@ -1530,12 +1501,9 @@ impl BackgroundProcessor {
15301501
.expect("Time should be sometime after 1970");
15311502
log_trace!(logger, "Calling time_passed and persisting scorer");
15321503
scorer.write_lock().time_passed(duration_since_epoch);
1533-
if let Err(e) = kv_store.write(
1534-
SCORER_PRIMARY_NAMESPACE,
1535-
SCORER_SECONDARY_NAMESPACE,
1536-
SCORER_KEY,
1537-
scorer.encode(),
1538-
) {
1504+
if let Err(e) =
1505+
kv_store.write(SCORER_NAMESPACE, "", SCORER_KEY, scorer.encode())
1506+
{
15391507
log_error!(logger, "Error: Failed to persist scorer, check your disk and permissions {}", e);
15401508
}
15411509
}
@@ -1571,23 +1539,18 @@ impl BackgroundProcessor {
15711539
// some races where users quit while channel updates were in-flight, with
15721540
// ChannelMonitor update(s) persisted without a corresponding ChannelManager update.
15731541
kv_store.write(
1574-
CHANNEL_MANAGER_PRIMARY_NAMESPACE,
1575-
CHANNEL_MANAGER_SECONDARY_NAMESPACE,
1542+
CHANNEL_MANAGER_NAMESPACE,
1543+
"",
15761544
CHANNEL_MANAGER_KEY,
15771545
channel_manager.get_cm().encode(),
15781546
)?;
15791547
if let Some(ref scorer) = scorer {
1580-
kv_store.write(
1581-
SCORER_PRIMARY_NAMESPACE,
1582-
SCORER_SECONDARY_NAMESPACE,
1583-
SCORER_KEY,
1584-
scorer.encode(),
1585-
)?;
1548+
kv_store.write(SCORER_NAMESPACE, "", SCORER_KEY, scorer.encode())?;
15861549
}
15871550
if let Some(network_graph) = gossip_sync.network_graph() {
15881551
kv_store.write(
1589-
NETWORK_GRAPH_PRIMARY_NAMESPACE,
1590-
NETWORK_GRAPH_SECONDARY_NAMESPACE,
1552+
NETWORK_GRAPH_NAMESPACE,
1553+
"",
15911554
NETWORK_GRAPH_KEY,
15921555
network_graph.encode(),
15931556
)?;
@@ -1680,10 +1643,8 @@ mod tests {
16801643
use lightning::types::payment::PaymentHash;
16811644
use lightning::util::config::UserConfig;
16821645
use lightning::util::persist::{
1683-
KVStoreSync, KVStoreSyncWrapper, CHANNEL_MANAGER_KEY, CHANNEL_MANAGER_PRIMARY_NAMESPACE,
1684-
CHANNEL_MANAGER_SECONDARY_NAMESPACE, NETWORK_GRAPH_KEY, NETWORK_GRAPH_PRIMARY_NAMESPACE,
1685-
NETWORK_GRAPH_SECONDARY_NAMESPACE, SCORER_KEY, SCORER_PRIMARY_NAMESPACE,
1686-
SCORER_SECONDARY_NAMESPACE,
1646+
KVStoreSync, KVStoreSyncWrapper, CHANNEL_MANAGER_KEY, CHANNEL_MANAGER_NAMESPACE,
1647+
NETWORK_GRAPH_KEY, NETWORK_GRAPH_NAMESPACE, SCORER_KEY, SCORER_NAMESPACE,
16871648
};
16881649
use lightning::util::ser::Writeable;
16891650
use lightning::util::sweep::{OutputSpendStatus, OutputSweeperSync, PRUNE_DELAY_BLOCKS};
@@ -1934,19 +1895,15 @@ mod tests {
19341895
fn write(
19351896
&self, primary_namespace: &str, secondary_namespace: &str, key: &str, buf: Vec<u8>,
19361897
) -> lightning::io::Result<()> {
1937-
if primary_namespace == CHANNEL_MANAGER_PRIMARY_NAMESPACE
1938-
&& secondary_namespace == CHANNEL_MANAGER_SECONDARY_NAMESPACE
1939-
&& key == CHANNEL_MANAGER_KEY
1940-
{
1898+
if primary_namespace == CHANNEL_MANAGER_NAMESPACE && key == CHANNEL_MANAGER_KEY {
1899+
assert_eq!(secondary_namespace, "");
19411900
if let Some((error, message)) = self.manager_error {
19421901
return Err(std::io::Error::new(error, message).into());
19431902
}
19441903
}
19451904

1946-
if primary_namespace == NETWORK_GRAPH_PRIMARY_NAMESPACE
1947-
&& secondary_namespace == NETWORK_GRAPH_SECONDARY_NAMESPACE
1948-
&& key == NETWORK_GRAPH_KEY
1949-
{
1905+
if primary_namespace == NETWORK_GRAPH_NAMESPACE && key == NETWORK_GRAPH_KEY {
1906+
assert_eq!(secondary_namespace, "");
19501907
if let Some(sender) = &self.graph_persistence_notifier {
19511908
match sender.send(()) {
19521909
Ok(()) => {},
@@ -1961,10 +1918,8 @@ mod tests {
19611918
}
19621919
}
19631920

1964-
if primary_namespace == SCORER_PRIMARY_NAMESPACE
1965-
&& secondary_namespace == SCORER_SECONDARY_NAMESPACE
1966-
&& key == SCORER_KEY
1967-
{
1921+
if primary_namespace == SCORER_NAMESPACE && key == SCORER_KEY {
1922+
assert_eq!(secondary_namespace, "");
19681923
if let Some((error, message)) = self.scorer_error {
19691924
return Err(std::io::Error::new(error, message).into());
19701925
}

‎lightning/src/ln/chanmon_update_fail_tests.rs‎

Lines changed: 10 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,7 @@ use crate::ln::types::ChannelId;
2727
use crate::sign::NodeSigner;
2828
use crate::util::native_async::FutureQueue;
2929
use crate::util::persist::{
30-
MonitorName, MonitorUpdatingPersisterAsync, CHANNEL_MONITOR_PRIMARY_NAMESPACE,
31-
CHANNEL_MONITOR_SECONDARY_NAMESPACE, MONITOR_UPDATE_PRIMARY_NAMESPACE,
30+
MonitorName, MonitorUpdatingPersisterAsync, CHANNEL_MONITOR_NAMESPACE, MONITOR_UPDATE_NAMESPACE,
3231
};
3332
use crate::util::ser::{ReadableArgs, Writeable};
3433
use crate::util::test_channel_signer::TestChannelSigner;
@@ -4933,11 +4932,7 @@ fn native_async_persist() {
49334932

49344933
let funding_txo = OutPoint { txid: funding_tx.compute_txid(), index: 0 };
49354934
let key = MonitorName::V1Channel(funding_txo).to_string();
4936-
let pending_writes = kv_store.list_pending_async_writes(
4937-
CHANNEL_MONITOR_PRIMARY_NAMESPACE,
4938-
CHANNEL_MONITOR_SECONDARY_NAMESPACE,
4939-
&key,
4940-
);
4935+
let pending_writes = kv_store.list_pending_async_writes(CHANNEL_MONITOR_NAMESPACE, "", &key);
49414936
assert_eq!(pending_writes.len(), 1);
49424937

49434938
// Once we complete the future, the write will still be pending until the future gets `poll`ed.
@@ -4965,21 +4960,19 @@ fn native_async_persist() {
49654960
persist_futures.poll_futures();
49664961
assert_eq!(async_chain_monitor.release_pending_monitor_events().len(), 0);
49674962

4968-
let pending_writes =
4969-
kv_store.list_pending_async_writes(MONITOR_UPDATE_PRIMARY_NAMESPACE, &key, "1");
4963+
let pending_writes = kv_store.list_pending_async_writes(MONITOR_UPDATE_NAMESPACE, &key, "1");
49704964
assert_eq!(pending_writes.len(), 1);
4971-
let pending_writes =
4972-
kv_store.list_pending_async_writes(MONITOR_UPDATE_PRIMARY_NAMESPACE, &key, "2");
4965+
let pending_writes = kv_store.list_pending_async_writes(MONITOR_UPDATE_NAMESPACE, &key, "2");
49734966
assert_eq!(pending_writes.len(), 1);
49744967

4975-
kv_store.complete_async_writes_through(MONITOR_UPDATE_PRIMARY_NAMESPACE, &key, "1", usize::MAX);
4968+
kv_store.complete_async_writes_through(MONITOR_UPDATE_NAMESPACE, &key, "1", usize::MAX);
49764969
persist_futures.poll_futures();
49774970
// While the `ChainMonitor` could return a `MonitorEvent::Completed` here, it currently
49784971
// doesn't. If that ever changes we should validate that the `Completed` event has the correct
49794972
// `monitor_update_id` (1).
49804973
assert!(async_chain_monitor.release_pending_monitor_events().is_empty());
49814974

4982-
kv_store.complete_async_writes_through(MONITOR_UPDATE_PRIMARY_NAMESPACE, &key, "2", usize::MAX);
4975+
kv_store.complete_async_writes_through(MONITOR_UPDATE_NAMESPACE, &key, "2", usize::MAX);
49834976
persist_futures.poll_futures();
49844977
let completed_persist = async_chain_monitor.release_pending_monitor_events();
49854978
assert_eq!(completed_persist.len(), 1);
@@ -4998,18 +4991,16 @@ fn native_async_persist() {
49984991
persist_futures.poll_futures();
49994992
assert_eq!(async_chain_monitor.release_pending_monitor_events().len(), 0);
50004993

5001-
let pending_writes =
5002-
kv_store.list_pending_async_writes(MONITOR_UPDATE_PRIMARY_NAMESPACE, &key, "3");
4994+
let pending_writes = kv_store.list_pending_async_writes(MONITOR_UPDATE_NAMESPACE, &key, "3");
50034995
assert_eq!(pending_writes.len(), 1);
5004-
let pending_writes =
5005-
kv_store.list_pending_async_writes(MONITOR_UPDATE_PRIMARY_NAMESPACE, &key, "4");
4996+
let pending_writes = kv_store.list_pending_async_writes(MONITOR_UPDATE_NAMESPACE, &key, "4");
50064997
assert_eq!(pending_writes.len(), 1);
50074998

5008-
kv_store.complete_async_writes_through(MONITOR_UPDATE_PRIMARY_NAMESPACE, &key, "4", usize::MAX);
4999+
kv_store.complete_async_writes_through(MONITOR_UPDATE_NAMESPACE, &key, "4", usize::MAX);
50095000
persist_futures.poll_futures();
50105001
assert_eq!(async_chain_monitor.release_pending_monitor_events().len(), 0);
50115002

5012-
kv_store.complete_async_writes_through(MONITOR_UPDATE_PRIMARY_NAMESPACE, &key, "3", usize::MAX);
5003+
kv_store.complete_async_writes_through(MONITOR_UPDATE_NAMESPACE, &key, "3", usize::MAX);
50135004
persist_futures.poll_futures();
50145005
let completed_persist = async_chain_monitor.release_pending_monitor_events();
50155006
assert_eq!(completed_persist.len(), 1);

0 commit comments

Comments
 (0)