Skip to content

Commit 406bef6

Browse files
committed
Correct maximum_pending_updates of 0 in MonitorUpdatingPersister
Though users maybe shouldn't use `MonitorUpdatingPersister` if they don't actually want to persist `ChannelMonitorUpdate`s, we also shouldn't panic if `maximum_pending_updates` is set to zero.
1 parent 1ed6c4b commit 406bef6

File tree

1 file changed

+34
-27
lines changed

1 file changed

+34
-27
lines changed

lightning/src/util/persist.rs

Lines changed: 34 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -796,6 +796,7 @@ where
796796
const LEGACY_CLOSED_CHANNEL_UPDATE_ID: u64 = u64::MAX;
797797
if let Some(update) = update {
798798
let persist_update = update.update_id != LEGACY_CLOSED_CHANNEL_UPDATE_ID
799+
&& self.maximum_pending_updates != 0
799800
&& update.update_id % self.maximum_pending_updates != 0;
800801
if persist_update {
801802
let monitor_key = monitor_name.to_string();
@@ -1188,12 +1189,9 @@ mod tests {
11881189
}
11891190

11901191
// Exercise the `MonitorUpdatingPersister` with real channels and payments.
1191-
#[test]
1192-
fn persister_with_real_monitors() {
1193-
// This value is used later to limit how many iterations we perform.
1194-
let persister_0_max_pending_updates = 7;
1195-
// Intentionally set this to a smaller value to test a different alignment.
1196-
let persister_1_max_pending_updates = 3;
1192+
fn do_persister_with_real_monitors(persisters_max_pending_updates: (u64, u64)) {
1193+
let persister_0_max_pending_updates = persisters_max_pending_updates.0;
1194+
let persister_1_max_pending_updates = persisters_max_pending_updates.1;
11971195
let chanmon_cfgs = create_chanmon_cfgs(4);
11981196
let persister_0 = MonitorUpdatingPersister {
11991197
kv_store: &TestStore::new(false),
@@ -1256,35 +1254,35 @@ mod tests {
12561254
assert_eq!(mon.get_latest_update_id(), $expected_update_id);
12571255

12581256
let monitor_name = mon.persistence_key();
1259-
assert_eq!(
1260-
KVStoreSync::list(
1261-
&*persister_0.kv_store,
1262-
CHANNEL_MONITOR_UPDATE_PERSISTENCE_PRIMARY_NAMESPACE,
1263-
&monitor_name.to_string()
1264-
)
1265-
.unwrap()
1266-
.len() as u64,
1267-
mon.get_latest_update_id() % persister_0_max_pending_updates,
1268-
"Wrong number of updates stored in persister 0",
1257+
let expected_updates = if persister_0_max_pending_updates == 0 {
1258+
0
1259+
} else {
1260+
mon.get_latest_update_id() % persister_0_max_pending_updates
1261+
};
1262+
let update_list = KVStoreSync::list(
1263+
&*persister_0.kv_store,
1264+
CHANNEL_MONITOR_UPDATE_PERSISTENCE_PRIMARY_NAMESPACE,
1265+
&monitor_name.to_string(),
12691266
);
1267+
assert_eq!(update_list.unwrap().len() as u64, expected_updates, "persister 0");
12701268
}
12711269
persisted_chan_data_1 =
12721270
persister_1.read_all_channel_monitors_with_updates().unwrap();
12731271
assert_eq!(persisted_chan_data_1.len(), 1);
12741272
for (_, mon) in persisted_chan_data_1.iter() {
12751273
assert_eq!(mon.get_latest_update_id(), $expected_update_id);
12761274
let monitor_name = mon.persistence_key();
1277-
assert_eq!(
1278-
KVStoreSync::list(
1279-
&*persister_1.kv_store,
1280-
CHANNEL_MONITOR_UPDATE_PERSISTENCE_PRIMARY_NAMESPACE,
1281-
&monitor_name.to_string()
1282-
)
1283-
.unwrap()
1284-
.len() as u64,
1285-
mon.get_latest_update_id() % persister_1_max_pending_updates,
1286-
"Wrong number of updates stored in persister 1",
1275+
let expected_updates = if persister_1_max_pending_updates == 0 {
1276+
0
1277+
} else {
1278+
mon.get_latest_update_id() % persister_1_max_pending_updates
1279+
};
1280+
let update_list = KVStoreSync::list(
1281+
&*persister_1.kv_store,
1282+
CHANNEL_MONITOR_UPDATE_PERSISTENCE_PRIMARY_NAMESPACE,
1283+
&monitor_name.to_string(),
12871284
);
1285+
assert_eq!(update_list.unwrap().len() as u64, expected_updates, "persister 1");
12881286
}
12891287
};
12901288
}
@@ -1345,11 +1343,20 @@ mod tests {
13451343
check_added_monitors!(nodes[1], 1);
13461344

13471345
// Make sure everything is persisted as expected after close.
1346+
// We always send at least two payments, and loop up to persister_0_max_pending_updates *
1347+
// 2.
13481348
check_persisted_data!(
1349-
persister_0_max_pending_updates * 2 * EXPECTED_UPDATES_PER_PAYMENT + 1
1349+
cmp::max(2, persister_0_max_pending_updates * 2) * EXPECTED_UPDATES_PER_PAYMENT + 1
13501350
);
13511351
}
13521352

1353+
#[test]
1354+
fn persister_with_real_monitors() {
1355+
do_persister_with_real_monitors((7, 3));
1356+
do_persister_with_real_monitors((0, 1));
1357+
do_persister_with_real_monitors((4, 2));
1358+
}
1359+
13531360
// Test that if the `MonitorUpdatingPersister`'s can't actually write, trying to persist a
13541361
// monitor or update with it results in the persister returning an UnrecoverableError status.
13551362
#[test]

0 commit comments

Comments
 (0)