Skip to content

Commit 314acb5

Browse files
committed
Ignore our own gossip if it is sent to us from our counterparty
If our channel party sends us our own channel_update message, we'll erroneously use the information in that message to update our view of the forwarding parameters our counterparty requires of us, ultimately generating invoices with bogus forwarding information. This fixes that behavior by checking the channel_update's directionality before handling it.
1 parent 9c00b28 commit 314acb5

File tree

1 file changed

+32
-1
lines changed

1 file changed

+32
-1
lines changed

lightning/src/ln/channelmanager.rs

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3406,7 +3406,13 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
34063406
}
34073407
return Err(MsgHandleErrInternal::send_err_msg_no_close("Got a channel_update for a channel from the wrong node - it shouldn't know about our private channels!".to_owned(), chan_id));
34083408
}
3409-
try_chan_entry!(self, chan.get_mut().channel_update(&msg), channel_state, chan);
3409+
let were_node_one = self.get_our_node_id().serialize()[..] < chan.get().get_counterparty_node_id().serialize()[..];
3410+
let msg_from_node_one = msg.contents.flags & 1 == 0;
3411+
if were_node_one == msg_from_node_one {
3412+
return Ok(NotifyOption::SkipPersist);
3413+
} else {
3414+
try_chan_entry!(self, chan.get_mut().channel_update(&msg), channel_state, chan);
3415+
}
34103416
},
34113417
hash_map::Entry::Vacant(_) => unreachable!()
34123418
}
@@ -4978,6 +4984,31 @@ mod tests {
49784984
// At this point the channel info given by peers should still be the same.
49794985
assert_eq!(nodes[0].node.list_channels()[0], node_a_chan_info);
49804986
assert_eq!(nodes[1].node.list_channels()[0], node_b_chan_info);
4987+
4988+
// An earlier version of handle_channel_update didn't check the directionality of the
4989+
// update message and would always update the local fee info, even if our peer was
4990+
// (spuriously) forwarding us our own channel_update.
4991+
let as_node_one = nodes[0].node.get_our_node_id().serialize()[..] < nodes[1].node.get_our_node_id().serialize()[..];
4992+
let as_update = if as_node_one == (chan.0.contents.flags & 1 == 0 /* chan.0 is from node one */) { &chan.0 } else { &chan.1 };
4993+
let bs_update = if as_node_one == (chan.0.contents.flags & 1 == 0 /* chan.0 is from node one */) { &chan.1 } else { &chan.0 };
4994+
4995+
// First deliver each peers' own message, checking that the node doesn't need to be
4996+
// persisted and that its channel info remains the same.
4997+
nodes[0].node.handle_channel_update(&nodes[1].node.get_our_node_id(), &as_update);
4998+
nodes[1].node.handle_channel_update(&nodes[0].node.get_our_node_id(), &bs_update);
4999+
assert!(!nodes[0].node.await_persistable_update_timeout(Duration::from_millis(1)));
5000+
assert!(!nodes[1].node.await_persistable_update_timeout(Duration::from_millis(1)));
5001+
assert_eq!(nodes[0].node.list_channels()[0], node_a_chan_info);
5002+
assert_eq!(nodes[1].node.list_channels()[0], node_b_chan_info);
5003+
5004+
// Finally, deliver the other peers' message, ensuring each node needs to be persisted and
5005+
// the channel info has updated.
5006+
nodes[0].node.handle_channel_update(&nodes[1].node.get_our_node_id(), &bs_update);
5007+
nodes[1].node.handle_channel_update(&nodes[0].node.get_our_node_id(), &as_update);
5008+
assert!(nodes[0].node.await_persistable_update_timeout(Duration::from_millis(1)));
5009+
assert!(nodes[1].node.await_persistable_update_timeout(Duration::from_millis(1)));
5010+
assert_ne!(nodes[0].node.list_channels()[0], node_a_chan_info);
5011+
assert_ne!(nodes[1].node.list_channels()[0], node_b_chan_info);
49815012
}
49825013
}
49835014

0 commit comments

Comments
 (0)