Merge HTLC-update events, remove FailHTLC ErrorAction

UpdateFailHTLC isn't really an error anymore now that its handled
async after channel commitment (as required by BOLT 2), and since
its unused this is free. To resolve the TODO which intended to use
it for HTLC failure when trying to route forwards, we instead opt
to merge all the HTLC update events into one UpdateHTLCs event
which just contains a CommitmentUpdate object.
This commit is contained in:
Matt Corallo 2018-08-22 12:09:11 -04:00
parent 6e50a84f14
commit ab00e4ccff
6 changed files with 64 additions and 93 deletions

View file

@ -256,7 +256,6 @@ pub fn do_test(data: &[u8]) {
Ok(r) => Some(r),
Err(e) => match e.action {
None => return,
Some(ErrorAction::UpdateFailHTLC {..}) => None,
Some(ErrorAction::DisconnectPeer {..}) => return,
Some(ErrorAction::IgnoreError) => None,
Some(ErrorAction::SendErrorMessage {..}) => None,

View file

@ -703,8 +703,7 @@ mod tests {
assert_eq!(log_entries.get(&("lightning::ln::peer_handler".to_string(), "Handling SendFundingLocked event in peer_handler for node 030200000000000000000000000000000000000000000000000000000000000000 for channel 3f00000000000000000000000000000000000000000000000000000000000000".to_string())), Some(&1)); // 4
assert_eq!(log_entries.get(&("lightning::ln::peer_handler".to_string(), "Encoding and sending message of type 133 to 030000000000000000000000000000000000000000000000000000000000000000".to_string())), Some(&2)); // 5
assert_eq!(log_entries.get(&("lightning::ln::peer_handler".to_string(), "Encoding and sending message of type 132 to 030000000000000000000000000000000000000000000000000000000000000000".to_string())), Some(&2)); // 6
assert_eq!(log_entries.get(&("lightning::ln::peer_handler".to_string(), "Handling SendHTLCs event in peer_handler for node 030200000000000000000000000000000000000000000000000000000000000000 with 1 HTLCs for channel 3f00000000000000000000000000000000000000000000000000000000000000".to_string())), Some(&2)); // 7
assert_eq!(log_entries.get(&("lightning::ln::peer_handler".to_string(), "Handling SendFulfillHTLCs event in peer_handler for node 030000000000000000000000000000000000000000000000000000000000000000 with payment_preimage ff00888888888888888888888888888888888888888888888888888888888888 for channel 3d00000000000000000000000000000000000000000000000000000000000000".to_string())), Some(&1)); // 8
assert_eq!(log_entries.get(&("lightning::ln::peer_handler".to_string(), "Handling UpdateHTLCs event in peer_handler for node 030200000000000000000000000000000000000000000000000000000000000000 with 1 adds, 0 fulfills, 0 fails for channel 3f00000000000000000000000000000000000000000000000000000000000000".to_string())), Some(&2)); // 7
assert_eq!(log_entries.get(&("lightning::ln::peer_handler".to_string(), "Handling UpdateHTLCs event in peer_handler for node 030000000000000000000000000000000000000000000000000000000000000000 with 0 adds, 1 fulfills, 0 fails for channel 3d00000000000000000000000000000000000000000000000000000000000000".to_string())), Some(&1)); // 8
}
}

View file

@ -952,10 +952,14 @@ impl ChannelManager {
}
let mut events = self.pending_events.lock().unwrap();
events.push(events::Event::SendHTLCs {
events.push(events::Event::UpdateHTLCs {
node_id: first_hop_node_id,
msgs: vec![update_add],
commitment_msg: commitment_signed,
updates: msgs::CommitmentUpdate {
update_add_htlcs: vec![update_add],
update_fulfill_htlcs: Vec::new(),
update_fail_htlcs: Vec::new(),
commitment_signed,
},
});
Ok(())
}
@ -1092,10 +1096,14 @@ impl ChannelManager {
continue;
},
};
new_events.push((Some(monitor), events::Event::SendHTLCs {
new_events.push((Some(monitor), events::Event::UpdateHTLCs {
node_id: forward_chan.get_their_node_id(),
msgs: add_htlc_msgs,
commitment_msg: commitment_msg,
updates: msgs::CommitmentUpdate {
update_add_htlcs: add_htlc_msgs,
update_fulfill_htlcs: Vec::new(),
update_fail_htlcs: Vec::new(),
commitment_signed: commitment_msg,
},
}));
}
} else {
@ -1211,11 +1219,14 @@ impl ChannelManager {
}
let mut pending_events = self.pending_events.lock().unwrap();
//TODO: replace by HandleError ? UpdateFailHTLC in handle_update_add_htlc need also to build a CommitmentSigned
pending_events.push(events::Event::SendFailHTLC {
pending_events.push(events::Event::UpdateHTLCs {
node_id,
msg: msg,
commitment_msg: commitment_msg,
updates: msgs::CommitmentUpdate {
update_add_htlcs: Vec::new(),
update_fulfill_htlcs: Vec::new(),
update_fail_htlcs: vec![msg],
commitment_signed: commitment_msg,
},
});
},
None => {},
@ -1307,10 +1318,14 @@ impl ChannelManager {
if let Some((msg, commitment_msg)) = fulfill_msgs.0 {
let mut pending_events = self.pending_events.lock().unwrap();
pending_events.push(events::Event::SendFulfillHTLC {
pending_events.push(events::Event::UpdateHTLCs {
node_id: node_id,
msg,
commitment_msg,
updates: msgs::CommitmentUpdate {
update_add_htlcs: Vec::new(),
update_fulfill_htlcs: vec![msg],
update_fail_htlcs: Vec::new(),
commitment_signed: commitment_msg,
}
});
}
true
@ -2461,8 +2476,10 @@ mod tests {
impl SendEvent {
fn from_event(event: Event) -> SendEvent {
match event {
Event::SendHTLCs { node_id, msgs, commitment_msg } => {
SendEvent { node_id: node_id, msgs: msgs, commitment_msg: commitment_msg }
Event::UpdateHTLCs { node_id, updates: msgs::CommitmentUpdate { update_add_htlcs, update_fulfill_htlcs, update_fail_htlcs, commitment_signed } } => {
assert!(update_fulfill_htlcs.is_empty());
assert!(update_fail_htlcs.is_empty());
SendEvent { node_id: node_id, msgs: update_add_htlcs, commitment_msg: commitment_signed }
},
_ => panic!("Unexpected event type!"),
}
@ -2618,9 +2635,12 @@ mod tests {
let events = node.node.get_and_clear_pending_events();
assert_eq!(events.len(), 1);
match events[0] {
Event::SendFulfillHTLC { ref node_id, ref msg, ref commitment_msg } => {
Event::UpdateHTLCs { ref node_id, updates: msgs::CommitmentUpdate { ref update_add_htlcs, ref update_fulfill_htlcs, ref update_fail_htlcs, ref commitment_signed } } => {
assert!(update_add_htlcs.is_empty());
assert_eq!(update_fulfill_htlcs.len(), 1);
assert!(update_fail_htlcs.is_empty());
expected_next_node = node_id.clone();
next_msgs = Some((msg.clone(), commitment_msg.clone()));
next_msgs = Some((update_fulfill_htlcs[0].clone(), commitment_signed.clone()));
},
_ => panic!("Unexpected event"),
};
@ -2739,9 +2759,12 @@ mod tests {
let events = node.node.get_and_clear_pending_events();
assert_eq!(events.len(), 1);
match events[0] {
Event::SendFailHTLC { ref node_id, ref msg, ref commitment_msg } => {
Event::UpdateHTLCs { ref node_id, updates: msgs::CommitmentUpdate { ref update_add_htlcs, ref update_fulfill_htlcs, ref update_fail_htlcs, ref commitment_signed } } => {
assert!(update_add_htlcs.is_empty());
assert!(update_fulfill_htlcs.is_empty());
assert_eq!(update_fail_htlcs.len(), 1);
expected_next_node = node_id.clone();
next_msgs = Some((msg.clone(), commitment_msg.clone()));
next_msgs = Some((update_fail_htlcs[0].clone(), commitment_signed.clone()));
},
_ => panic!("Unexpected event"),
};
@ -3057,7 +3080,9 @@ mod tests {
let events = $node.node.get_and_clear_pending_events();
assert_eq!(events.len(), 1);
match events[0] {
Event::SendFulfillHTLC { ref node_id, .. } => {
Event::UpdateHTLCs { ref node_id, updates: msgs::CommitmentUpdate { ref update_add_htlcs, ref update_fail_htlcs, .. } } => {
assert!(update_add_htlcs.is_empty());
assert!(update_fail_htlcs.is_empty());
assert_eq!(*node_id, $prev_node.node.get_our_node_id());
},
_ => panic!("Unexpected event"),

View file

@ -376,11 +376,6 @@ pub struct ChannelUpdate {
/// Used to put an error message in a HandleError
pub enum ErrorAction {
/// Indicates an inbound HTLC add resulted in a failure, and the UpdateFailHTLC provided in msg
/// should be sent back to the sender.
UpdateFailHTLC {
msg: UpdateFailHTLC
},
/// The peer took some action which made us think they were useless. Disconnect them.
DisconnectPeer {
msg: Option<ErrorMessage>

View file

@ -303,10 +303,6 @@ impl<Descriptor: SocketDescriptor> PeerManager<Descriptor> {
Err(e) => {
if let Some(action) = e.action {
match action {
msgs::ErrorAction::UpdateFailHTLC { msg } => {
encode_and_send_msg!(msg, 131);
continue;
},
msgs::ErrorAction::DisconnectPeer { msg: _ } => {
//TODO: Try to push msg
log_trace!(self, "Got Err handling message, disconnecting peer because {}", e.err);
@ -680,44 +676,26 @@ impl<Descriptor: SocketDescriptor> PeerManager<Descriptor> {
Self::do_attempt_write_data(&mut descriptor, peer);
continue;
},
Event::SendHTLCs { ref node_id, ref msgs, ref commitment_msg } => {
log_trace!(self, "Handling SendHTLCs event in peer_handler for node {} with {} HTLCs for channel {}",
Event::UpdateHTLCs { ref node_id, updates: msgs::CommitmentUpdate { ref update_add_htlcs, ref update_fulfill_htlcs, ref update_fail_htlcs, ref commitment_signed } } => {
log_trace!(self, "Handling UpdateHTLCs event in peer_handler for node {} with {} adds, {} fulfills, {} fails for channel {}",
log_pubkey!(node_id),
msgs.len(),
log_bytes!(commitment_msg.channel_id));
update_add_htlcs.len(),
update_fulfill_htlcs.len(),
update_fail_htlcs.len(),
log_bytes!(commitment_signed.channel_id));
let (mut descriptor, peer) = get_peer_for_forwarding!(node_id, {
//TODO: Do whatever we're gonna do for handling dropped messages
});
for msg in msgs {
for msg in update_add_htlcs {
peer.pending_outbound_buffer.push_back(peer.channel_encryptor.encrypt_message(&encode_msg!(msg, 128)));
}
peer.pending_outbound_buffer.push_back(peer.channel_encryptor.encrypt_message(&encode_msg!(commitment_msg, 132)));
Self::do_attempt_write_data(&mut descriptor, peer);
continue;
},
Event::SendFulfillHTLC { ref node_id, ref msg, ref commitment_msg } => {
log_trace!(self, "Handling SendFulfillHTLCs event in peer_handler for node {} with payment_preimage {} for channel {}",
log_pubkey!(node_id),
log_bytes!(msg.payment_preimage),
log_bytes!(msg.channel_id));
let (mut descriptor, peer) = get_peer_for_forwarding!(node_id, {
//TODO: Do whatever we're gonna do for handling dropped messages
});
peer.pending_outbound_buffer.push_back(peer.channel_encryptor.encrypt_message(&encode_msg!(msg, 130)));
peer.pending_outbound_buffer.push_back(peer.channel_encryptor.encrypt_message(&encode_msg!(commitment_msg, 132)));
Self::do_attempt_write_data(&mut descriptor, peer);
continue;
},
Event::SendFailHTLC { ref node_id, ref msg, ref commitment_msg } => {
log_trace!(self, "Handling SendFailHTLCs event in peer_handler for node {} for HTLC ID {} for channel {}",
log_pubkey!(node_id),
msg.htlc_id,
log_bytes!(msg.channel_id));
let (mut descriptor, peer) = get_peer_for_forwarding!(node_id, {
//TODO: Do whatever we're gonna do for handling dropped messages
});
peer.pending_outbound_buffer.push_back(peer.channel_encryptor.encrypt_message(&encode_msg!(msg, 131)));
peer.pending_outbound_buffer.push_back(peer.channel_encryptor.encrypt_message(&encode_msg!(commitment_msg, 132)));
for msg in update_fulfill_htlcs {
peer.pending_outbound_buffer.push_back(peer.channel_encryptor.encrypt_message(&encode_msg!(msg, 130)));
}
for msg in update_fail_htlcs {
peer.pending_outbound_buffer.push_back(peer.channel_encryptor.encrypt_message(&encode_msg!(msg, 131)));
}
peer.pending_outbound_buffer.push_back(peer.channel_encryptor.encrypt_message(&encode_msg!(commitment_signed, 132)));
Self::do_attempt_write_data(&mut descriptor, peer);
continue;
},
@ -775,18 +753,6 @@ impl<Descriptor: SocketDescriptor> PeerManager<Descriptor> {
Event::HandleError { ref node_id, ref action } => {
if let Some(ref action) = *action {
match *action {
msgs::ErrorAction::UpdateFailHTLC { ref msg } => {
log_trace!(self, "Handling UpdateFailHTLC HandleError event in peer_handler for node {} for HTLC ID {} for channel {}",
log_pubkey!(node_id),
msg.htlc_id,
log_bytes!(msg.channel_id));
let (mut descriptor, peer) = get_peer_for_forwarding!(node_id, {
//TODO: Do whatever we're gonna do for handling dropped messages
});
peer.pending_outbound_buffer.push_back(peer.channel_encryptor.encrypt_message(&encode_msg!(msg, 131)));
Self::do_attempt_write_data(&mut descriptor, peer);
},
msgs::ErrorAction::DisconnectPeer { ref msg } => {
if let Some(mut descriptor) = peers.node_id_to_descriptor.remove(node_id) {
if let Some(mut peer) = peers.peers.remove(&descriptor) {

View file

@ -70,24 +70,11 @@ pub enum Event {
msg: msgs::FundingLocked,
announcement_sigs: Option<msgs::AnnouncementSignatures>,
},
/// Used to indicate that a series of update_add_htlc messages, as well as a commitment_signed
/// Used to indicate that a series of HTLC update messages, as well as a commitment_signed
/// message should be sent to the peer with the given node_id.
SendHTLCs {
UpdateHTLCs {
node_id: PublicKey,
msgs: Vec<msgs::UpdateAddHTLC>,
commitment_msg: msgs::CommitmentSigned,
},
/// Used to indicate that we're ready to fulfill an htlc from the peer with the given node_id.
SendFulfillHTLC {
node_id: PublicKey,
msg: msgs::UpdateFulfillHTLC,
commitment_msg: msgs::CommitmentSigned,
},
/// Used to indicate that we need to fail an htlc from the peer with the given node_id.
SendFailHTLC {
node_id: PublicKey,
msg: msgs::UpdateFailHTLC,
commitment_msg: msgs::CommitmentSigned,
updates: msgs::CommitmentUpdate,
},
/// Used to indicate that a shutdown message should be sent to the peer with the given node_id.
SendShutdown {