Skip to content

Commit c7fa29c

Browse files
wpaulinowpaulino
committed
Use channel ID over funding outpoint to track monitors in ChainMonitor
Once dual funding/splicing is supported, channels will no longer maintain their original funding outpoint. Ideally, we identify `ChannelMonitor`s with a stable identifier, such as the channel ID, which is fixed throughout the channel's lifetime. This commit replaces the use of funding outpoints as the key to the monitor index with the channel ID. Note that this is strictly done to the in-memory state; it does not change how we track monitors within their persisted state, they are still keyed by their funding outpoint there. Addressing that is left for follow-up work.
1 parent 3d2b4de commit c7fa29c

14 files changed

+373
-377
lines changed

‎fuzz/src/chanmon_consistency.rs

Lines changed: 43 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -198,7 +198,7 @@ struct TestChainMonitor {
198198
Arc<TestPersister>,
199199
>,
200200
>,
201-
pub latest_monitors: Mutex<HashMap<OutPoint, LatestMonitorState>>,
201+
pub latest_monitors: Mutex<HashMap<ChannelId, LatestMonitorState>>,
202202
}
203203
impl TestChainMonitor {
204204
pub fn new(
@@ -222,12 +222,12 @@ impl TestChainMonitor {
222222
}
223223
impl chain::Watch<TestChannelSigner> for TestChainMonitor {
224224
fn watch_channel(
225-
&self, funding_txo: OutPoint, monitor: channelmonitor::ChannelMonitor<TestChannelSigner>,
225+
&self, channel_id: ChannelId, monitor: channelmonitor::ChannelMonitor<TestChannelSigner>,
226226
) -> Result<chain::ChannelMonitorUpdateStatus, ()> {
227227
let mut ser = VecWriter(Vec::new());
228228
monitor.write(&mut ser).unwrap();
229229
let monitor_id = monitor.get_latest_update_id();
230-
let res = self.chain_monitor.watch_channel(funding_txo, monitor);
230+
let res = self.chain_monitor.watch_channel(channel_id, monitor);
231231
let state = match res {
232232
Ok(chain::ChannelMonitorUpdateStatus::Completed) => LatestMonitorState {
233233
persisted_monitor_id: monitor_id,
@@ -240,17 +240,17 @@ impl chain::Watch<TestChannelSigner> for TestChainMonitor {
240240
Ok(chain::ChannelMonitorUpdateStatus::UnrecoverableError) => panic!(),
241241
Err(()) => panic!(),
242242
};
243-
if self.latest_monitors.lock().unwrap().insert(funding_txo, state).is_some() {
243+
if self.latest_monitors.lock().unwrap().insert(channel_id, state).is_some() {
244244
panic!("Already had monitor pre-watch_channel");
245245
}
246246
res
247247
}
248248

249249
fn update_channel(
250-
&self, funding_txo: OutPoint, update: &channelmonitor::ChannelMonitorUpdate,
250+
&self, channel_id: ChannelId, update: &channelmonitor::ChannelMonitorUpdate,
251251
) -> chain::ChannelMonitorUpdateStatus {
252252
let mut map_lock = self.latest_monitors.lock().unwrap();
253-
let map_entry = map_lock.get_mut(&funding_txo).expect("Didn't have monitor on update call");
253+
let map_entry = map_lock.get_mut(&channel_id).expect("Didn't have monitor on update call");
254254
let latest_monitor_data = map_entry
255255
.pending_monitors
256256
.last()
@@ -274,7 +274,7 @@ impl chain::Watch<TestChannelSigner> for TestChainMonitor {
274274
.unwrap();
275275
let mut ser = VecWriter(Vec::new());
276276
deserialized_monitor.write(&mut ser).unwrap();
277-
let res = self.chain_monitor.update_channel(funding_txo, update);
277+
let res = self.chain_monitor.update_channel(channel_id, update);
278278
match res {
279279
chain::ChannelMonitorUpdateStatus::Completed => {
280280
map_entry.persisted_monitor_id = update.update_id;
@@ -763,9 +763,9 @@ pub fn do_test<Out: Output>(data: &[u8], underlying_out: Out, anchors: bool) {
763763
.1,
764764
chain_monitor.clone(),
765765
);
766-
for (funding_txo, mon) in monitors.drain() {
766+
for (channel_id, mon) in monitors.drain() {
767767
assert_eq!(
768-
chain_monitor.chain_monitor.watch_channel(funding_txo, mon),
768+
chain_monitor.chain_monitor.watch_channel(channel_id, mon),
769769
Ok(ChannelMonitorUpdateStatus::Completed)
770770
);
771771
}
@@ -836,7 +836,6 @@ pub fn do_test<Out: Output>(data: &[u8], underlying_out: Out, anchors: bool) {
836836
};
837837

838838
$source.handle_accept_channel($dest.get_our_node_id(), &accept_channel);
839-
let funding_output;
840839
{
841840
let mut events = $source.get_and_clear_pending_events();
842841
assert_eq!(events.len(), 1);
@@ -856,7 +855,6 @@ pub fn do_test<Out: Output>(data: &[u8], underlying_out: Out, anchors: bool) {
856855
script_pubkey: output_script,
857856
}],
858857
};
859-
funding_output = OutPoint { txid: tx.compute_txid(), index: 0 };
860858
$source
861859
.funding_transaction_generated(
862860
temporary_channel_id,
@@ -901,13 +899,19 @@ pub fn do_test<Out: Output>(data: &[u8], underlying_out: Out, anchors: bool) {
901899
$source.handle_funding_signed($dest.get_our_node_id(), &funding_signed);
902900
let events = $source.get_and_clear_pending_events();
903901
assert_eq!(events.len(), 1);
904-
if let events::Event::ChannelPending { ref counterparty_node_id, .. } = events[0] {
902+
let channel_id = if let events::Event::ChannelPending {
903+
ref counterparty_node_id,
904+
ref channel_id,
905+
..
906+
} = events[0]
907+
{
905908
assert_eq!(counterparty_node_id, &$dest.get_our_node_id());
909+
channel_id.clone()
906910
} else {
907911
panic!("Wrong event type");
908-
}
912+
};
909913

910-
funding_output
914+
channel_id
911915
}};
912916
}
913917

@@ -974,8 +978,8 @@ pub fn do_test<Out: Output>(data: &[u8], underlying_out: Out, anchors: bool) {
974978

975979
let mut nodes = [node_a, node_b, node_c];
976980

977-
let chan_1_funding = make_channel!(nodes[0], nodes[1], keys_manager_b, 0);
978-
let chan_2_funding = make_channel!(nodes[1], nodes[2], keys_manager_c, 1);
981+
let chan_1_id = make_channel!(nodes[0], nodes[1], keys_manager_b, 0);
982+
let chan_2_id = make_channel!(nodes[1], nodes[2], keys_manager_c, 1);
979983

980984
for node in nodes.iter() {
981985
confirm_txn!(node);
@@ -1374,14 +1378,14 @@ pub fn do_test<Out: Output>(data: &[u8], underlying_out: Out, anchors: bool) {
13741378
}
13751379
};
13761380

1377-
let complete_all_monitor_updates = |monitor: &Arc<TestChainMonitor>, chan_funding| {
1378-
if let Some(state) = monitor.latest_monitors.lock().unwrap().get_mut(chan_funding) {
1381+
let complete_all_monitor_updates = |monitor: &Arc<TestChainMonitor>, chan_id| {
1382+
if let Some(state) = monitor.latest_monitors.lock().unwrap().get_mut(chan_id) {
13791383
assert!(
13801384
state.pending_monitors.windows(2).all(|pair| pair[0].0 < pair[1].0),
13811385
"updates should be sorted by id"
13821386
);
13831387
for (id, data) in state.pending_monitors.drain(..) {
1384-
monitor.chain_monitor.channel_monitor_updated(*chan_funding, id).unwrap();
1388+
monitor.chain_monitor.channel_monitor_updated(*chan_id, id).unwrap();
13851389
if id > state.persisted_monitor_id {
13861390
state.persisted_monitor_id = id;
13871391
state.persisted_monitor = data;
@@ -1421,10 +1425,10 @@ pub fn do_test<Out: Output>(data: &[u8], underlying_out: Out, anchors: bool) {
14211425
ChannelMonitorUpdateStatus::Completed
14221426
},
14231427

1424-
0x08 => complete_all_monitor_updates(&monitor_a, &chan_1_funding),
1425-
0x09 => complete_all_monitor_updates(&monitor_b, &chan_1_funding),
1426-
0x0a => complete_all_monitor_updates(&monitor_b, &chan_2_funding),
1427-
0x0b => complete_all_monitor_updates(&monitor_c, &chan_2_funding),
1428+
0x08 => complete_all_monitor_updates(&monitor_a, &chan_1_id),
1429+
0x09 => complete_all_monitor_updates(&monitor_b, &chan_1_id),
1430+
0x0a => complete_all_monitor_updates(&monitor_b, &chan_2_id),
1431+
0x0b => complete_all_monitor_updates(&monitor_c, &chan_2_id),
14281432

14291433
0x0c => {
14301434
if !chan_a_disconnected {
@@ -1694,21 +1698,21 @@ pub fn do_test<Out: Output>(data: &[u8], underlying_out: Out, anchors: bool) {
16941698
nodes[2].maybe_update_chan_fees();
16951699
},
16961700

1697-
0xf0 => complete_monitor_update(&monitor_a, &chan_1_funding, &complete_first),
1698-
0xf1 => complete_monitor_update(&monitor_a, &chan_1_funding, &complete_second),
1699-
0xf2 => complete_monitor_update(&monitor_a, &chan_1_funding, &Vec::pop),
1701+
0xf0 => complete_monitor_update(&monitor_a, &chan_1_id, &complete_first),
1702+
0xf1 => complete_monitor_update(&monitor_a, &chan_1_id, &complete_second),
1703+
0xf2 => complete_monitor_update(&monitor_a, &chan_1_id, &Vec::pop),
17001704

1701-
0xf4 => complete_monitor_update(&monitor_b, &chan_1_funding, &complete_first),
1702-
0xf5 => complete_monitor_update(&monitor_b, &chan_1_funding, &complete_second),
1703-
0xf6 => complete_monitor_update(&monitor_b, &chan_1_funding, &Vec::pop),
1705+
0xf4 => complete_monitor_update(&monitor_b, &chan_1_id, &complete_first),
1706+
0xf5 => complete_monitor_update(&monitor_b, &chan_1_id, &complete_second),
1707+
0xf6 => complete_monitor_update(&monitor_b, &chan_1_id, &Vec::pop),
17041708

1705-
0xf8 => complete_monitor_update(&monitor_b, &chan_2_funding, &complete_first),
1706-
0xf9 => complete_monitor_update(&monitor_b, &chan_2_funding, &complete_second),
1707-
0xfa => complete_monitor_update(&monitor_b, &chan_2_funding, &Vec::pop),
1709+
0xf8 => complete_monitor_update(&monitor_b, &chan_2_id, &complete_first),
1710+
0xf9 => complete_monitor_update(&monitor_b, &chan_2_id, &complete_second),
1711+
0xfa => complete_monitor_update(&monitor_b, &chan_2_id, &Vec::pop),
17081712

1709-
0xfc => complete_monitor_update(&monitor_c, &chan_2_funding, &complete_first),
1710-
0xfd => complete_monitor_update(&monitor_c, &chan_2_funding, &complete_second),
1711-
0xfe => complete_monitor_update(&monitor_c, &chan_2_funding, &Vec::pop),
1713+
0xfc => complete_monitor_update(&monitor_c, &chan_2_id, &complete_first),
1714+
0xfd => complete_monitor_update(&monitor_c, &chan_2_id, &complete_second),
1715+
0xfe => complete_monitor_update(&monitor_c, &chan_2_id, &Vec::pop),
17121716

17131717
0xff => {
17141718
// Test that no channel is in a stuck state where neither party can send funds even
@@ -1722,10 +1726,10 @@ pub fn do_test<Out: Output>(data: &[u8], underlying_out: Out, anchors: bool) {
17221726
*monitor_c.persister.update_ret.lock().unwrap() =
17231727
ChannelMonitorUpdateStatus::Completed;
17241728

1725-
complete_all_monitor_updates(&monitor_a, &chan_1_funding);
1726-
complete_all_monitor_updates(&monitor_b, &chan_1_funding);
1727-
complete_all_monitor_updates(&monitor_b, &chan_2_funding);
1728-
complete_all_monitor_updates(&monitor_c, &chan_2_funding);
1729+
complete_all_monitor_updates(&monitor_a, &chan_1_id);
1730+
complete_all_monitor_updates(&monitor_b, &chan_1_id);
1731+
complete_all_monitor_updates(&monitor_b, &chan_2_id);
1732+
complete_all_monitor_updates(&monitor_c, &chan_2_id);
17291733

17301734
// Next, make sure peers are all connected to each other
17311735
if chan_a_disconnected {

‎lightning-block-sync/src/init.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@ where
125125
///
126126
/// // Allow the chain monitor to watch any channels.
127127
/// let monitor = monitor_listener.0;
128-
/// chain_monitor.watch_channel(monitor.get_funding_txo().0, monitor);
128+
/// chain_monitor.watch_channel(monitor.channel_id(), monitor);
129129
///
130130
/// // Create an SPV client to notify the chain monitor and channel manager of block events.
131131
/// let chain_poller = poll::ChainPoller::new(block_source, Network::Bitcoin);

0 commit comments

Comments
 (0)