Skip to content

Commit ddbe53f

Browse files
author
Antoine Riard
committed
Make field error of LightingError mandatory
We also fulfilled last empty ErrorAction: - Router secp fail : IgnoreError - processing error in Router : IgnoreError - get_channel_update too early : IgnoreError
1 parent 7608483 commit ddbe53f

File tree

10 files changed

+160
-177
lines changed

10 files changed

+160
-177
lines changed

‎fuzz/fuzz_targets/chanmon_fail_consistency.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -392,7 +392,7 @@ pub fn do_test(data: &[u8]) {
392392
($res: expr) => {
393393
match $res {
394394
Ok(()) => {},
395-
Err(LightningError { action: Some(ErrorAction::IgnoreError), .. }) => { },
395+
Err(LightningError { action: ErrorAction::IgnoreError, .. }) => { },
396396
_ => { $res.unwrap() },
397397
}
398398
}

‎src/ln/chanmon_update_fail_tests.rs

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -190,7 +190,7 @@ fn do_test_monitor_temporary_update_fail(disconnect_count: usize) {
190190
_ => panic!("Unexpected event"),
191191
}
192192

193-
if let Err(msgs::LightningError{err, action: Some(msgs::ErrorAction::IgnoreError) }) = nodes[0].node.handle_commitment_signed(&nodes[1].node.get_our_node_id(), commitment_signed) {
193+
if let Err(msgs::LightningError{err, action: msgs::ErrorAction::IgnoreError }) = nodes[0].node.handle_commitment_signed(&nodes[1].node.get_our_node_id(), commitment_signed) {
194194
assert_eq!(err, "Previous monitor update failure prevented generation of RAA");
195195
} else { panic!(); }
196196
}
@@ -485,7 +485,7 @@ fn test_monitor_update_fail_cs() {
485485
nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &send_event.msgs[0]).unwrap();
486486

487487
*nodes[1].chan_monitor.update_ret.lock().unwrap() = Err(ChannelMonitorUpdateErr::TemporaryFailure);
488-
if let msgs::LightningError { err, action: Some(msgs::ErrorAction::IgnoreError) } = nodes[1].node.handle_commitment_signed(&nodes[0].node.get_our_node_id(), &send_event.commitment_msg).unwrap_err() {
488+
if let msgs::LightningError { err, action: msgs::ErrorAction::IgnoreError } = nodes[1].node.handle_commitment_signed(&nodes[0].node.get_our_node_id(), &send_event.commitment_msg).unwrap_err() {
489489
assert_eq!(err, "Failed to update ChannelMonitor");
490490
} else { panic!(); }
491491
check_added_monitors!(nodes[1], 1);
@@ -515,7 +515,7 @@ fn test_monitor_update_fail_cs() {
515515
assert_eq!(*node_id, nodes[0].node.get_our_node_id());
516516

517517
*nodes[0].chan_monitor.update_ret.lock().unwrap() = Err(ChannelMonitorUpdateErr::TemporaryFailure);
518-
if let msgs::LightningError { err, action: Some(msgs::ErrorAction::IgnoreError) } = nodes[0].node.handle_commitment_signed(&nodes[1].node.get_our_node_id(), &updates.commitment_signed).unwrap_err() {
518+
if let msgs::LightningError { err, action: msgs::ErrorAction::IgnoreError } = nodes[0].node.handle_commitment_signed(&nodes[1].node.get_our_node_id(), &updates.commitment_signed).unwrap_err() {
519519
assert_eq!(err, "Failed to update ChannelMonitor");
520520
} else { panic!(); }
521521
check_added_monitors!(nodes[0], 1);
@@ -565,7 +565,7 @@ fn test_monitor_update_fail_no_rebroadcast() {
565565
let bs_raa = commitment_signed_dance!(nodes[1], nodes[0], send_event.commitment_msg, false, true, false, true);
566566

567567
*nodes[1].chan_monitor.update_ret.lock().unwrap() = Err(ChannelMonitorUpdateErr::TemporaryFailure);
568-
if let msgs::LightningError { err, action: Some(msgs::ErrorAction::IgnoreError) } = nodes[1].node.handle_revoke_and_ack(&nodes[0].node.get_our_node_id(), &bs_raa).unwrap_err() {
568+
if let msgs::LightningError { err, action: msgs::ErrorAction::IgnoreError } = nodes[1].node.handle_revoke_and_ack(&nodes[0].node.get_our_node_id(), &bs_raa).unwrap_err() {
569569
assert_eq!(err, "Failed to update ChannelMonitor");
570570
} else { panic!(); }
571571
assert!(nodes[1].node.get_and_clear_pending_msg_events().is_empty());
@@ -618,12 +618,12 @@ fn test_monitor_update_raa_while_paused() {
618618

619619
*nodes[0].chan_monitor.update_ret.lock().unwrap() = Err(ChannelMonitorUpdateErr::TemporaryFailure);
620620
nodes[0].node.handle_update_add_htlc(&nodes[1].node.get_our_node_id(), &send_event_2.msgs[0]).unwrap();
621-
if let msgs::LightningError { err, action: Some(msgs::ErrorAction::IgnoreError) } = nodes[0].node.handle_commitment_signed(&nodes[1].node.get_our_node_id(), &send_event_2.commitment_msg).unwrap_err() {
621+
if let msgs::LightningError { err, action: msgs::ErrorAction::IgnoreError } = nodes[0].node.handle_commitment_signed(&nodes[1].node.get_our_node_id(), &send_event_2.commitment_msg).unwrap_err() {
622622
assert_eq!(err, "Failed to update ChannelMonitor");
623623
} else { panic!(); }
624624
check_added_monitors!(nodes[0], 1);
625625

626-
if let msgs::LightningError { err, action: Some(msgs::ErrorAction::IgnoreError) } = nodes[0].node.handle_revoke_and_ack(&nodes[1].node.get_our_node_id(), &bs_raa).unwrap_err() {
626+
if let msgs::LightningError { err, action: msgs::ErrorAction::IgnoreError } = nodes[0].node.handle_revoke_and_ack(&nodes[1].node.get_our_node_id(), &bs_raa).unwrap_err() {
627627
assert_eq!(err, "Previous monitor update failure prevented responses to RAA");
628628
} else { panic!(); }
629629
check_added_monitors!(nodes[0], 1);
@@ -704,7 +704,7 @@ fn do_test_monitor_update_fail_raa(test_ignore_second_cs: bool) {
704704

705705
// Now fail monitor updating.
706706
*nodes[1].chan_monitor.update_ret.lock().unwrap() = Err(ChannelMonitorUpdateErr::TemporaryFailure);
707-
if let msgs::LightningError { err, action: Some(msgs::ErrorAction::IgnoreError) } = nodes[1].node.handle_revoke_and_ack(&nodes[2].node.get_our_node_id(), &bs_revoke_and_ack).unwrap_err() {
707+
if let msgs::LightningError { err, action: msgs::ErrorAction::IgnoreError } = nodes[1].node.handle_revoke_and_ack(&nodes[2].node.get_our_node_id(), &bs_revoke_and_ack).unwrap_err() {
708708
assert_eq!(err, "Failed to update ChannelMonitor");
709709
} else { panic!(); }
710710
assert!(nodes[1].node.get_and_clear_pending_events().is_empty());
@@ -768,7 +768,7 @@ fn do_test_monitor_update_fail_raa(test_ignore_second_cs: bool) {
768768

769769
send_event = SendEvent::from_event(nodes[2].node.get_and_clear_pending_msg_events().remove(0));
770770
nodes[1].node.handle_update_add_htlc(&nodes[2].node.get_our_node_id(), &send_event.msgs[0]).unwrap();
771-
if let Err(msgs::LightningError{err, action: Some(msgs::ErrorAction::IgnoreError) }) = nodes[1].node.handle_commitment_signed(&nodes[2].node.get_our_node_id(), &send_event.commitment_msg) {
771+
if let Err(msgs::LightningError{err, action: msgs::ErrorAction::IgnoreError }) = nodes[1].node.handle_commitment_signed(&nodes[2].node.get_our_node_id(), &send_event.commitment_msg) {
772772
assert_eq!(err, "Previous monitor update failure prevented generation of RAA");
773773
} else { panic!(); }
774774
assert!(nodes[1].node.get_and_clear_pending_msg_events().is_empty());
@@ -945,7 +945,7 @@ fn test_monitor_update_fail_reestablish() {
945945

946946
nodes[0].node.handle_channel_reestablish(&nodes[1].node.get_our_node_id(), &bs_reestablish).unwrap();
947947

948-
if let msgs::LightningError { err, action: Some(msgs::ErrorAction::IgnoreError) } = nodes[1].node.handle_channel_reestablish(&nodes[0].node.get_our_node_id(), &as_reestablish).unwrap_err() {
948+
if let msgs::LightningError { err, action: msgs::ErrorAction::IgnoreError } = nodes[1].node.handle_channel_reestablish(&nodes[0].node.get_our_node_id(), &as_reestablish).unwrap_err() {
949949
assert_eq!(err, "Failed to update ChannelMonitor");
950950
} else { panic!(); }
951951
check_added_monitors!(nodes[1], 1);
@@ -1033,12 +1033,12 @@ fn raa_no_response_awaiting_raa_state() {
10331033
// then restore channel monitor updates.
10341034
*nodes[1].chan_monitor.update_ret.lock().unwrap() = Err(ChannelMonitorUpdateErr::TemporaryFailure);
10351035
nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &payment_event.msgs[0]).unwrap();
1036-
if let msgs::LightningError { err, action: Some(msgs::ErrorAction::IgnoreError) } = nodes[1].node.handle_commitment_signed(&nodes[0].node.get_our_node_id(), &payment_event.commitment_msg).unwrap_err() {
1036+
if let msgs::LightningError { err, action: msgs::ErrorAction::IgnoreError } = nodes[1].node.handle_commitment_signed(&nodes[0].node.get_our_node_id(), &payment_event.commitment_msg).unwrap_err() {
10371037
assert_eq!(err, "Failed to update ChannelMonitor");
10381038
} else { panic!(); }
10391039
check_added_monitors!(nodes[1], 1);
10401040

1041-
if let msgs::LightningError { err, action: Some(msgs::ErrorAction::IgnoreError) } = nodes[1].node.handle_revoke_and_ack(&nodes[0].node.get_our_node_id(), &as_raa).unwrap_err() {
1041+
if let msgs::LightningError { err, action: msgs::ErrorAction::IgnoreError } = nodes[1].node.handle_revoke_and_ack(&nodes[0].node.get_our_node_id(), &as_raa).unwrap_err() {
10421042
assert_eq!(err, "Previous monitor update failure prevented responses to RAA");
10431043
} else { panic!(); }
10441044
check_added_monitors!(nodes[1], 1);
@@ -1130,7 +1130,7 @@ fn claim_while_disconnected_monitor_update_fail() {
11301130
// update.
11311131
*nodes[1].chan_monitor.update_ret.lock().unwrap() = Err(ChannelMonitorUpdateErr::TemporaryFailure);
11321132

1133-
if let msgs::LightningError { err, action: Some(msgs::ErrorAction::IgnoreError) } = nodes[1].node.handle_channel_reestablish(&nodes[0].node.get_our_node_id(), &as_reconnect).unwrap_err() {
1133+
if let msgs::LightningError { err, action: msgs::ErrorAction::IgnoreError } = nodes[1].node.handle_channel_reestablish(&nodes[0].node.get_our_node_id(), &as_reconnect).unwrap_err() {
11341134
assert_eq!(err, "Failed to update ChannelMonitor");
11351135
} else { panic!(); }
11361136
check_added_monitors!(nodes[1], 1);
@@ -1145,7 +1145,7 @@ fn claim_while_disconnected_monitor_update_fail() {
11451145

11461146
let as_updates = get_htlc_update_msgs!(nodes[0], nodes[1].node.get_our_node_id());
11471147
nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &as_updates.update_add_htlcs[0]).unwrap();
1148-
if let msgs::LightningError { err, action: Some(msgs::ErrorAction::IgnoreError) } = nodes[1].node.handle_commitment_signed(&nodes[0].node.get_our_node_id(), &as_updates.commitment_signed).unwrap_err() {
1148+
if let msgs::LightningError { err, action: msgs::ErrorAction::IgnoreError } = nodes[1].node.handle_commitment_signed(&nodes[0].node.get_our_node_id(), &as_updates.commitment_signed).unwrap_err() {
11491149
assert_eq!(err, "Previous monitor update failure prevented generation of RAA");
11501150
} else { panic!(); }
11511151
// Note that nodes[1] not updating monitor here is OK - it wont take action on the new HTLC
@@ -1235,7 +1235,7 @@ fn monitor_failed_no_reestablish_response() {
12351235
assert_eq!(events.len(), 1);
12361236
let payment_event = SendEvent::from_event(events.pop().unwrap());
12371237
nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &payment_event.msgs[0]).unwrap();
1238-
if let msgs::LightningError { err, action: Some(msgs::ErrorAction::IgnoreError) } = nodes[1].node.handle_commitment_signed(&nodes[0].node.get_our_node_id(), &payment_event.commitment_msg).unwrap_err() {
1238+
if let msgs::LightningError { err, action: msgs::ErrorAction::IgnoreError } = nodes[1].node.handle_commitment_signed(&nodes[0].node.get_our_node_id(), &payment_event.commitment_msg).unwrap_err() {
12391239
assert_eq!(err, "Failed to update ChannelMonitor");
12401240
} else { panic!(); }
12411241
check_added_monitors!(nodes[1], 1);
@@ -1327,7 +1327,7 @@ fn first_message_on_recv_ordering() {
13271327
// Deliver the final RAA for the first payment, which does not require a response. RAAs
13281328
// generally require a commitment_signed, so the fact that we're expecting an opposite response
13291329
// to the next message also tests resetting the delivery order.
1330-
if let msgs::LightningError { err, action: Some(msgs::ErrorAction::IgnoreError) } = nodes[1].node.handle_revoke_and_ack(&nodes[0].node.get_our_node_id(), &as_raa).unwrap_err() {
1330+
if let msgs::LightningError { err, action: msgs::ErrorAction::IgnoreError } = nodes[1].node.handle_revoke_and_ack(&nodes[0].node.get_our_node_id(), &as_raa).unwrap_err() {
13311331
assert_eq!(err, "Failed to update ChannelMonitor");
13321332
} else { panic!(); }
13331333
check_added_monitors!(nodes[1], 1);
@@ -1336,7 +1336,7 @@ fn first_message_on_recv_ordering() {
13361336
// RAA/CS response, which should be generated when we call test_restore_channel_monitor (with
13371337
// the appropriate HTLC acceptance).
13381338
nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &payment_event.msgs[0]).unwrap();
1339-
if let msgs::LightningError { err, action: Some(msgs::ErrorAction::IgnoreError) } = nodes[1].node.handle_commitment_signed(&nodes[0].node.get_our_node_id(), &payment_event.commitment_msg).unwrap_err() {
1339+
if let msgs::LightningError { err, action: msgs::ErrorAction::IgnoreError } = nodes[1].node.handle_commitment_signed(&nodes[0].node.get_our_node_id(), &payment_event.commitment_msg).unwrap_err() {
13401340
assert_eq!(err, "Previous monitor update failure prevented generation of RAA");
13411341
} else { panic!(); }
13421342

@@ -1597,7 +1597,7 @@ fn do_during_funding_monitor_fail(fail_on_generate: bool, restore_between_fails:
15971597
}
15981598
let funding_signed_res = nodes[0].node.handle_funding_signed(&nodes[1].node.get_our_node_id(), &get_event_msg!(nodes[1], MessageSendEvent::SendFundingSigned, nodes[0].node.get_our_node_id()));
15991599
if fail_on_signed || !restore_between_fails {
1600-
if let msgs::LightningError { err, action: Some(msgs::ErrorAction::IgnoreError) } = funding_signed_res.unwrap_err() {
1600+
if let msgs::LightningError { err, action: msgs::ErrorAction::IgnoreError } = funding_signed_res.unwrap_err() {
16011601
if fail_on_generate && !restore_between_fails {
16021602
assert_eq!(err, "Previous monitor update failure prevented funding_signed from allowing funding broadcast");
16031603
check_added_monitors!(nodes[0], 0);

‎src/ln/channelmanager.rs

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -152,12 +152,12 @@ impl MsgHandleErrInternal {
152152
Self {
153153
err: LightningError {
154154
err,
155-
action: Some(msgs::ErrorAction::SendErrorMessage {
155+
action: msgs::ErrorAction::SendErrorMessage {
156156
msg: msgs::ErrorMessage {
157157
channel_id,
158158
data: err.to_string()
159159
},
160-
}),
160+
},
161161
},
162162
shutdown_finish: None,
163163
}
@@ -167,7 +167,7 @@ impl MsgHandleErrInternal {
167167
Self {
168168
err: LightningError {
169169
err,
170-
action: Some(msgs::ErrorAction::IgnoreError),
170+
action: msgs::ErrorAction::IgnoreError,
171171
},
172172
shutdown_finish: None,
173173
}
@@ -181,12 +181,12 @@ impl MsgHandleErrInternal {
181181
Self {
182182
err: LightningError {
183183
err,
184-
action: Some(msgs::ErrorAction::SendErrorMessage {
184+
action: msgs::ErrorAction::SendErrorMessage {
185185
msg: msgs::ErrorMessage {
186186
channel_id,
187187
data: err.to_string()
188188
},
189-
}),
189+
},
190190
},
191191
shutdown_finish: Some((shutdown_res, channel_update)),
192192
}
@@ -197,25 +197,25 @@ impl MsgHandleErrInternal {
197197
err: match err {
198198
ChannelError::Ignore(msg) => LightningError {
199199
err: msg,
200-
action: Some(msgs::ErrorAction::IgnoreError),
200+
action: msgs::ErrorAction::IgnoreError,
201201
},
202202
ChannelError::Close(msg) => LightningError {
203203
err: msg,
204-
action: Some(msgs::ErrorAction::SendErrorMessage {
204+
action: msgs::ErrorAction::SendErrorMessage {
205205
msg: msgs::ErrorMessage {
206206
channel_id,
207207
data: msg.to_string()
208208
},
209-
}),
209+
},
210210
},
211211
ChannelError::CloseDelayBroadcast { msg, .. } => LightningError {
212212
err: msg,
213-
action: Some(msgs::ErrorAction::SendErrorMessage {
213+
action: msgs::ErrorAction::SendErrorMessage {
214214
msg: msgs::ErrorMessage {
215215
channel_id,
216216
data: msg.to_string()
217217
},
218-
}),
218+
},
219219
},
220220
},
221221
shutdown_finish: None,
@@ -1011,7 +1011,7 @@ impl ChannelManager {
10111011
/// May be called with channel_state already locked!
10121012
fn get_channel_update(&self, chan: &Channel) -> Result<msgs::ChannelUpdate, LightningError> {
10131013
let short_channel_id = match chan.get_short_channel_id() {
1014-
None => return Err(LightningError{err: "Channel not yet established", action: None}),
1014+
None => return Err(LightningError{err: "Channel not yet established", action: msgs::ErrorAction::IgnoreError}),
10151015
Some(id) => id,
10161016
};
10171017

@@ -1140,7 +1140,7 @@ impl ChannelManager {
11401140
match handle_error!(self, err) {
11411141
Ok(_) => unreachable!(),
11421142
Err(e) => {
1143-
if let Some(msgs::ErrorAction::IgnoreError) = e.action {
1143+
if let msgs::ErrorAction::IgnoreError = e.action {
11441144
} else {
11451145
log_error!(self, "Got bad keys: {}!", e.err);
11461146
let mut channel_state = self.channel_state.lock().unwrap();
@@ -1434,7 +1434,7 @@ impl ChannelManager {
14341434
match handle_error!(self, err) {
14351435
Ok(_) => {},
14361436
Err(e) => {
1437-
if let Some(msgs::ErrorAction::IgnoreError) = e.action {
1437+
if let msgs::ErrorAction::IgnoreError = e.action {
14381438
} else {
14391439
let mut channel_state = self.channel_state.lock().unwrap();
14401440
channel_state.pending_msg_events.push(events::MessageSendEvent::HandleError {
@@ -1660,7 +1660,7 @@ impl ChannelManager {
16601660
match handle_error!(self, err) {
16611661
Ok(_) => {},
16621662
Err(e) => {
1663-
if let Some(msgs::ErrorAction::IgnoreError) = e.action {
1663+
if let msgs::ErrorAction::IgnoreError = e.action {
16641664
} else {
16651665
let mut channel_state = self.channel_state.lock().unwrap();
16661666
channel_state.pending_msg_events.push(events::MessageSendEvent::HandleError {
@@ -2292,7 +2292,7 @@ impl ChannelManager {
22922292
return Err(MsgHandleErrInternal::send_err_msg_no_close("Got a message for a channel from the wrong node!", msg.channel_id));
22932293
}
22942294
if !chan.get().is_usable() {
2295-
return Err(MsgHandleErrInternal::from_no_close(LightningError{err: "Got an announcement_signatures before we were ready for it", action: Some(msgs::ErrorAction::IgnoreError)}));
2295+
return Err(MsgHandleErrInternal::from_no_close(LightningError{err: "Got an announcement_signatures before we were ready for it", action: msgs::ErrorAction::IgnoreError}));
22962296
}
22972297

22982298
let our_node_id = self.get_our_node_id();
@@ -2445,7 +2445,7 @@ impl ChannelManager {
24452445
match handle_error!(self, err) {
24462446
Ok(_) => unreachable!(),
24472447
Err(e) => {
2448-
if let Some(msgs::ErrorAction::IgnoreError) = e.action {
2448+
if let msgs::ErrorAction::IgnoreError = e.action {
24492449
} else {
24502450
log_error!(self, "Got bad keys: {}!", e.err);
24512451
let mut channel_state = self.channel_state.lock().unwrap();
@@ -2538,7 +2538,7 @@ impl ChainListener for ChannelManager {
25382538
} else if let Err(e) = chan_res {
25392539
pending_msg_events.push(events::MessageSendEvent::HandleError {
25402540
node_id: channel.get_their_node_id(),
2541-
action: Some(msgs::ErrorAction::SendErrorMessage { msg: e }),
2541+
action: msgs::ErrorAction::SendErrorMessage { msg: e },
25422542
});
25432543
return false;
25442544
}

0 commit comments

Comments
 (0)