Skip to content

Commit 8d6ed64

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 8d6ed64

File tree

1 file changed

+34
-30
lines changed

1 file changed

+34
-30
lines changed

lightning/src/util/persist.rs

Lines changed: 34 additions & 30 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,17 +1189,12 @@ 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(max_pending_updates_0: u64, max_pending_updates_1: u64) {
11971193
let chanmon_cfgs = create_chanmon_cfgs(4);
11981194
let persister_0 = MonitorUpdatingPersister {
11991195
kv_store: &TestStore::new(false),
12001196
logger: &TestLogger::new(),
1201-
maximum_pending_updates: persister_0_max_pending_updates,
1197+
maximum_pending_updates: max_pending_updates_0,
12021198
entropy_source: &chanmon_cfgs[0].keys_manager,
12031199
signer_provider: &chanmon_cfgs[0].keys_manager,
12041200
broadcaster: &chanmon_cfgs[0].tx_broadcaster,
@@ -1207,7 +1203,7 @@ mod tests {
12071203
let persister_1 = MonitorUpdatingPersister {
12081204
kv_store: &TestStore::new(false),
12091205
logger: &TestLogger::new(),
1210-
maximum_pending_updates: persister_1_max_pending_updates,
1206+
maximum_pending_updates: max_pending_updates_1,
12111207
entropy_source: &chanmon_cfgs[1].keys_manager,
12121208
signer_provider: &chanmon_cfgs[1].keys_manager,
12131209
broadcaster: &chanmon_cfgs[1].tx_broadcaster,
@@ -1256,35 +1252,35 @@ mod tests {
12561252
assert_eq!(mon.get_latest_update_id(), $expected_update_id);
12571253

12581254
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",
1255+
let expected_updates = if max_pending_updates_0 == 0 {
1256+
0
1257+
} else {
1258+
mon.get_latest_update_id() % max_pending_updates_0
1259+
};
1260+
let update_list = KVStoreSync::list(
1261+
&*persister_0.kv_store,
1262+
CHANNEL_MONITOR_UPDATE_PERSISTENCE_PRIMARY_NAMESPACE,
1263+
&monitor_name.to_string(),
12691264
);
1265+
assert_eq!(update_list.unwrap().len() as u64, expected_updates, "persister 0");
12701266
}
12711267
persisted_chan_data_1 =
12721268
persister_1.read_all_channel_monitors_with_updates().unwrap();
12731269
assert_eq!(persisted_chan_data_1.len(), 1);
12741270
for (_, mon) in persisted_chan_data_1.iter() {
12751271
assert_eq!(mon.get_latest_update_id(), $expected_update_id);
12761272
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",
1273+
let expected_updates = if max_pending_updates_1 == 0 {
1274+
0
1275+
} else {
1276+
mon.get_latest_update_id() % max_pending_updates_1
1277+
};
1278+
let update_list = KVStoreSync::list(
1279+
&*persister_1.kv_store,
1280+
CHANNEL_MONITOR_UPDATE_PERSISTENCE_PRIMARY_NAMESPACE,
1281+
&monitor_name.to_string(),
12871282
);
1283+
assert_eq!(update_list.unwrap().len() as u64, expected_updates, "persister 1");
12881284
}
12891285
};
12901286
}
@@ -1302,7 +1298,7 @@ mod tests {
13021298
// Send a few more payments to try all the alignments of max pending updates with
13031299
// updates for a payment sent and received.
13041300
let mut sender = 0;
1305-
for i in 3..=persister_0_max_pending_updates * 2 {
1301+
for i in 3..=max_pending_updates_0 * 2 {
13061302
let receiver;
13071303
if sender == 0 {
13081304
sender = 1;
@@ -1345,11 +1341,19 @@ mod tests {
13451341
check_added_monitors!(nodes[1], 1);
13461342

13471343
// Make sure everything is persisted as expected after close.
1344+
// We always send at least two payments, and loop up to max_pending_updates_0 * 2.
13481345
check_persisted_data!(
1349-
persister_0_max_pending_updates * 2 * EXPECTED_UPDATES_PER_PAYMENT + 1
1346+
cmp::max(2, max_pending_updates_0 * 2) * EXPECTED_UPDATES_PER_PAYMENT + 1
13501347
);
13511348
}
13521349

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

0 commit comments

Comments
 (0)