Skip to content

Commit c08ea1d

Browse files
committed
Rename namespace validity constants to be more concise
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. Here we take the first step towards condensing them by making them somewhat more concise, dropping words which are obvious from context.
1 parent e42e74e commit c08ea1d

File tree

3 files changed

+26
-30
lines changed

3 files changed

+26
-30
lines changed

lightning-persister/src/test_utils.rs

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use lightning::ln::functional_test_utils::{
55
};
66
use lightning::util::persist::{
77
migrate_kv_store_data, read_channel_monitors, KVStoreSync, MigratableKVStore,
8-
KVSTORE_NAMESPACE_KEY_ALPHABET, KVSTORE_NAMESPACE_KEY_MAX_LEN,
8+
NAMESPACE_ALPHABET, NAMESPACE_MAX_LEN,
99
};
1010
use lightning::util::test_utils;
1111
use lightning::{check_added_monitors, check_closed_broadcast, check_closed_event};
@@ -46,8 +46,8 @@ pub(crate) fn do_read_write_remove_list_persist<K: KVStoreSync + RefUnwindSafe>(
4646
assert_eq!(listed_keys.len(), 0);
4747

4848
// Ensure we have no issue operating with primary_namespace/secondary_namespace/key being
49-
// KVSTORE_NAMESPACE_KEY_MAX_LEN
50-
let max_chars = "A".repeat(KVSTORE_NAMESPACE_KEY_MAX_LEN);
49+
// NAMESPACE_MAX_LEN
50+
let max_chars = "A".repeat(NAMESPACE_MAX_LEN);
5151
kv_store.write(&max_chars, &max_chars, &max_chars, data.clone()).unwrap();
5252

5353
let listed_keys = kv_store.list(&max_chars, &max_chars).unwrap();
@@ -76,17 +76,16 @@ pub(crate) fn do_test_data_migration<S: MigratableKVStore, T: MigratableKVStore>
7676
let primary_namespace = if i == 0 {
7777
String::new()
7878
} else {
79-
format!("testspace{}", KVSTORE_NAMESPACE_KEY_ALPHABET.chars().nth(i).unwrap())
79+
format!("testspace{}", NAMESPACE_ALPHABET.chars().nth(i).unwrap())
8080
};
8181
for j in 0..num_secondary_namespaces {
8282
let secondary_namespace = if i == 0 || j == 0 {
8383
String::new()
8484
} else {
85-
format!("testsubspace{}", KVSTORE_NAMESPACE_KEY_ALPHABET.chars().nth(j).unwrap())
85+
format!("testsubspace{}", NAMESPACE_ALPHABET.chars().nth(j).unwrap())
8686
};
8787
for k in 0..num_keys {
88-
let key =
89-
format!("testkey{}", KVSTORE_NAMESPACE_KEY_ALPHABET.chars().nth(k).unwrap());
88+
let key = format!("testkey{}", NAMESPACE_ALPHABET.chars().nth(k).unwrap());
9089
source_store
9190
.write(&primary_namespace, &secondary_namespace, &key, dummy_data.clone())
9291
.unwrap();

lightning-persister/src/utils.rs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,8 @@
11
use lightning::types::string::PrintableString;
2-
use lightning::util::persist::{KVSTORE_NAMESPACE_KEY_ALPHABET, KVSTORE_NAMESPACE_KEY_MAX_LEN};
2+
use lightning::util::persist::{NAMESPACE_ALPHABET, NAMESPACE_MAX_LEN};
33

44
pub(crate) fn is_valid_kvstore_str(key: &str) -> bool {
5-
key.len() <= KVSTORE_NAMESPACE_KEY_MAX_LEN
6-
&& key.chars().all(|c| KVSTORE_NAMESPACE_KEY_ALPHABET.contains(c))
5+
key.len() <= NAMESPACE_MAX_LEN && key.chars().all(|c| NAMESPACE_ALPHABET.contains(c))
76
}
87

98
pub(crate) fn check_namespace_key_validity(

lightning/src/util/persist.rs

Lines changed: 18 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -41,11 +41,11 @@ use crate::util::ser::{Readable, ReadableArgs, Writeable};
4141
use crate::util::wakers::Notifier;
4242

4343
/// The alphabet of characters allowed for namespaces and keys.
44-
pub const KVSTORE_NAMESPACE_KEY_ALPHABET: &str =
44+
pub const NAMESPACE_ALPHABET: &str =
4545
"abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789_-";
4646

4747
/// The maximum number of characters namespaces and keys may have.
48-
pub const KVSTORE_NAMESPACE_KEY_MAX_LEN: usize = 120;
48+
pub const NAMESPACE_MAX_LEN: usize = 120;
4949

5050
/// The primary namespace under which the [`ChannelManager`] will be persisted.
5151
///
@@ -126,15 +126,14 @@ pub const MONITOR_UPDATING_PERSISTER_PREPEND_SENTINEL: &[u8] = &[0xFF; 2];
126126
/// ways, as long as per-namespace key uniqueness is asserted.
127127
///
128128
/// Keys and namespaces are required to be valid ASCII strings in the range of
129-
/// [`KVSTORE_NAMESPACE_KEY_ALPHABET`] and no longer than [`KVSTORE_NAMESPACE_KEY_MAX_LEN`]. Empty
130-
/// primary namespaces and secondary namespaces (`""`) are assumed to be a valid, however, if
131-
/// `primary_namespace` is empty, `secondary_namespace` is required to be empty, too. This means
132-
/// that concerns should always be separated by primary namespace first, before secondary
133-
/// namespaces are used. While the number of primary namespaces will be relatively small and is
134-
/// determined at compile time, there may be many secondary namespaces per primary namespace. Note
135-
/// that per-namespace uniqueness needs to also hold for keys *and* namespaces in any given
136-
/// namespace, i.e., conflicts between keys and equally named
137-
/// primary namespaces/secondary namespaces must be avoided.
129+
/// [`NAMESPACE_ALPHABET`] and no longer than [`NAMESPACE_MAX_LEN`]. Empty primary namespaces and
130+
/// secondary namespaces (`""`) are assumed to be a valid, however, if `primary_namespace` is empty,
131+
/// `secondary_namespace` is required to be empty, too. This means that concerns should always be
132+
/// separated by primary namespace first, before secondary namespaces are used. While the number of
133+
/// primary namespaces will be relatively small and is determined at compile time, there may be many
134+
/// secondary namespaces per primary namespace. Note that per-namespace uniqueness needs to also
135+
/// hold for keys *and* namespaces in any given namespace, i.e., conflicts between keys and equally
136+
/// named primary namespaces/secondary namespaces must be avoided.
138137
///
139138
/// **Note:** Users migrating custom persistence backends from the pre-v0.0.117 `KVStorePersister`
140139
/// interface can use a concatenation of `[{primary_namespace}/[{secondary_namespace}/]]{key}` to
@@ -255,15 +254,14 @@ where
255254
/// ways, as long as per-namespace key uniqueness is asserted.
256255
///
257256
/// Keys and namespaces are required to be valid ASCII strings in the range of
258-
/// [`KVSTORE_NAMESPACE_KEY_ALPHABET`] and no longer than [`KVSTORE_NAMESPACE_KEY_MAX_LEN`]. Empty
259-
/// primary namespaces and secondary namespaces (`""`) are assumed to be a valid, however, if
260-
/// `primary_namespace` is empty, `secondary_namespace` is required to be empty, too. This means
261-
/// that concerns should always be separated by primary namespace first, before secondary
262-
/// namespaces are used. While the number of primary namespaces will be relatively small and is
263-
/// determined at compile time, there may be many secondary namespaces per primary namespace. Note
264-
/// that per-namespace uniqueness needs to also hold for keys *and* namespaces in any given
265-
/// namespace, i.e., conflicts between keys and equally named
266-
/// primary namespaces/secondary namespaces must be avoided.
257+
/// [`NAMESPACE_ALPHABET`] and no longer than [`NAMESPACE_MAX_LEN`]. Empty primary namespaces and
258+
/// secondary namespaces (`""`) are assumed to be a valid, however, if `primary_namespace` is
259+
/// empty, `secondary_namespace` is required to be empty, too. This means that concerns should
260+
/// always be separated by primary namespace first, before secondary namespaces are used. While the
261+
/// number of primary namespaces will be relatively small and is determined at compile time, there
262+
/// may be many secondary namespaces per primary namespace. Note that per-namespace uniqueness
263+
/// needs to also hold for keys *and* namespaces in any given namespace, i.e., conflicts between
264+
/// keys and equally named primary namespaces/secondary namespaces must be avoided.
267265
///
268266
/// **Note:** Users migrating custom persistence backends from the pre-v0.0.117 `KVStorePersister`
269267
/// interface can use a concatenation of `[{primary_namespace}/[{secondary_namespace}/]]{key}` to

0 commit comments

Comments
 (0)