-
Notifications
You must be signed in to change notification settings - Fork 409
Support for SCID Aliases #1311
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support for SCID Aliases #1311
Changes from all commits
a9c4e70
3fe76e6
09f8aba
a274261
f54ebf7
b2629af
10be993
84fa127
e4486fe
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -693,6 +693,61 @@ pub fn create_announced_chan_between_nodes_with_value<'a, 'b, 'c, 'd>(nodes: &'a | |
(chan_announcement.1, chan_announcement.2, chan_announcement.3, chan_announcement.4) | ||
} | ||
|
||
pub fn create_unannounced_chan_between_nodes_with_value<'a, 'b, 'c, 'd>(nodes: &'a Vec<Node<'b, 'c, 'd>>, a: usize, b: usize, channel_value: u64, push_msat: u64, a_flags: InitFeatures, b_flags: InitFeatures) -> (msgs::FundingLocked, Transaction) { | ||
let mut no_announce_cfg = test_default_channel_config(); | ||
no_announce_cfg.channel_options.announced_channel = false; | ||
Comment on lines
+696
to
+698
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Getting some deja vu from another PR. Is there a way to reuse some existing utility code such that the code that follows these lines isn't needed? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm, all the existing utilities end up either calling There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't feel too strongly. I think I already asked another contributor to do the same in another PR, heh. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm, on second thought I'm kinda struggling to see how to do this - the top 7 lines could be split out into three lines and four lines, but that doesn't seem so useful. Then there's the funding generation, which almost can be combined, except that we rely on exchanging the funding_locked messages in order so that we only get one There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No problem with passing on this. My concern was more with code repetition resulting in needing to modify two places rather than reducing lines of code. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yea, I agree, its unclear how to resolve it in this case, though, without digging through a number of other tests and changing their util method calls, which is yet more diff in this PR :/ |
||
nodes[a].node.create_channel(nodes[b].node.get_our_node_id(), channel_value, push_msat, 42, Some(no_announce_cfg)).unwrap(); | ||
let open_channel = get_event_msg!(nodes[a], MessageSendEvent::SendOpenChannel, nodes[b].node.get_our_node_id()); | ||
nodes[b].node.handle_open_channel(&nodes[a].node.get_our_node_id(), a_flags, &open_channel); | ||
let accept_channel = get_event_msg!(nodes[b], MessageSendEvent::SendAcceptChannel, nodes[a].node.get_our_node_id()); | ||
nodes[a].node.handle_accept_channel(&nodes[b].node.get_our_node_id(), b_flags, &accept_channel); | ||
|
||
let (temporary_channel_id, tx, _) = create_funding_transaction(&nodes[a], channel_value, 42); | ||
nodes[a].node.funding_transaction_generated(&temporary_channel_id, tx.clone()).unwrap(); | ||
nodes[b].node.handle_funding_created(&nodes[a].node.get_our_node_id(), &get_event_msg!(nodes[a], MessageSendEvent::SendFundingCreated, nodes[b].node.get_our_node_id())); | ||
check_added_monitors!(nodes[b], 1); | ||
|
||
let cs_funding_signed = get_event_msg!(nodes[b], MessageSendEvent::SendFundingSigned, nodes[a].node.get_our_node_id()); | ||
nodes[a].node.handle_funding_signed(&nodes[b].node.get_our_node_id(), &cs_funding_signed); | ||
check_added_monitors!(nodes[a], 1); | ||
|
||
let conf_height = core::cmp::max(nodes[a].best_block_info().1 + 1, nodes[b].best_block_info().1 + 1); | ||
confirm_transaction_at(&nodes[a], &tx, conf_height); | ||
connect_blocks(&nodes[a], CHAN_CONFIRM_DEPTH - 1); | ||
confirm_transaction_at(&nodes[b], &tx, conf_height); | ||
connect_blocks(&nodes[b], CHAN_CONFIRM_DEPTH - 1); | ||
let as_funding_locked = get_event_msg!(nodes[a], MessageSendEvent::SendFundingLocked, nodes[b].node.get_our_node_id()); | ||
nodes[a].node.handle_funding_locked(&nodes[b].node.get_our_node_id(), &get_event_msg!(nodes[b], MessageSendEvent::SendFundingLocked, nodes[a].node.get_our_node_id())); | ||
let as_update = get_event_msg!(nodes[a], MessageSendEvent::SendChannelUpdate, nodes[b].node.get_our_node_id()); | ||
nodes[b].node.handle_funding_locked(&nodes[a].node.get_our_node_id(), &as_funding_locked); | ||
let bs_update = get_event_msg!(nodes[b], MessageSendEvent::SendChannelUpdate, nodes[a].node.get_our_node_id()); | ||
|
||
nodes[a].node.handle_channel_update(&nodes[b].node.get_our_node_id(), &bs_update); | ||
nodes[b].node.handle_channel_update(&nodes[a].node.get_our_node_id(), &as_update); | ||
|
||
let mut found_a = false; | ||
for chan in nodes[a].node.list_usable_channels() { | ||
if chan.channel_id == as_funding_locked.channel_id { | ||
assert!(!found_a); | ||
found_a = true; | ||
assert!(!chan.is_public); | ||
} | ||
} | ||
assert!(found_a); | ||
|
||
let mut found_b = false; | ||
for chan in nodes[b].node.list_usable_channels() { | ||
if chan.channel_id == as_funding_locked.channel_id { | ||
assert!(!found_b); | ||
found_b = true; | ||
assert!(!chan.is_public); | ||
} | ||
} | ||
assert!(found_b); | ||
|
||
(as_funding_locked, tx) | ||
} | ||
|
||
pub fn update_nodes_with_chan_announce<'a, 'b, 'c, 'd>(nodes: &'a Vec<Node<'b, 'c, 'd>>, a: usize, b: usize, ann: &msgs::ChannelAnnouncement, upd_1: &msgs::ChannelUpdate, upd_2: &msgs::ChannelUpdate) { | ||
nodes[a].node.broadcast_node_announcement([0, 0, 0], [0; 32], Vec::new()); | ||
let a_events = nodes[a].node.get_and_clear_pending_msg_events(); | ||
|
@@ -748,6 +803,11 @@ pub fn update_nodes_with_chan_announce<'a, 'b, 'c, 'd>(nodes: &'a Vec<Node<'b, ' | |
node.net_graph_msg_handler.handle_channel_update(upd_2).unwrap(); | ||
node.net_graph_msg_handler.handle_node_announcement(&a_node_announcement).unwrap(); | ||
node.net_graph_msg_handler.handle_node_announcement(&b_node_announcement).unwrap(); | ||
|
||
// Note that channel_updates are also delivered to ChannelManagers to ensure we have | ||
// forwarding info for local channels even if its not accepted in the network graph. | ||
node.node.handle_channel_update(&nodes[a].node.get_our_node_id(), &upd_1); | ||
node.node.handle_channel_update(&nodes[b].node.get_our_node_id(), &upd_2); | ||
} | ||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we do something similar when we're sending a payment, i.e. when use the alias when getting
first_hop
s for the route?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll leave that for the 0conf PR - we don't need to care right now cause first-hop SCID isn't communicated anywhere, its just an internal tracker for us, so its only needed after 0conf.