Skip to content

[HandleError] rename msg field to action #53

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Jul 20, 2018
Merged

[HandleError] rename msg field to action #53

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fd141bb
[Error] rename msg field to action
savil Jul 20, 2018
14c02c0
update TODO message for renaming "msg" field of HandleError struct
savil Jul 20, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
148 changes: 74 additions & 74 deletions src/ln/channel.rs
View file
Open in desktop

Large diffs are not rendered by default.

84 changes: 42 additions & 42 deletions src/ln/channelmanager.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ macro_rules! secp_call {
match $res {
Ok(key) => key,
//TODO: Make the err a parameter!
Err(_) => return Err(HandleError{err: "Key error", msg: None})
Err(_) => return Err(HandleError{err: "Key error", action: None})
}
};
}
Expand Down Expand Up @@ -311,7 +311,7 @@ impl ChannelManager {
(res, Some(chan_entry.remove_entry().1))
} else { (res, None) }
},
hash_map::Entry::Vacant(_) => return Err(HandleError{err: "No such channel", msg: None})
hash_map::Entry::Vacant(_) => return Err(HandleError{err: "No such channel", action: None})
}
};
for payment_hash in res.1 {
Expand Down Expand Up @@ -443,11 +443,11 @@ impl ChannelManager {
};
cur_value_msat += hop.fee_msat;
if cur_value_msat >= 21000000 * 100000000 * 1000 {
return Err(HandleError{err: "Channel fees overflowed?!", msg: None});
return Err(HandleError{err: "Channel fees overflowed?!", action: None});
}
cur_cltv += hop.cltv_expiry_delta as u32;
if cur_cltv >= 500000000 {
return Err(HandleError{err: "Channel CLTV overflowed?!", msg: None});
return Err(HandleError{err: "Channel CLTV overflowed?!", action: None});
}
last_short_channel_id = hop.short_channel_id;
}
Expand Down Expand Up @@ -576,7 +576,7 @@ impl ChannelManager {
/// only fails if the channel does not yet have an assigned short_id
fn get_channel_update(&self, chan: &Channel) -> Result<msgs::ChannelUpdate, HandleError> {
let short_channel_id = match chan.get_short_channel_id() {
None => return Err(HandleError{err: "Channel not yet established", msg: None}),
None => return Err(HandleError{err: "Channel not yet established", action: None}),
Some(id) => id,
};

Expand Down Expand Up @@ -609,12 +609,12 @@ impl ChannelManager {
/// May generate a SendHTLCs event on success, which should be relayed.
pub fn send_payment(&self, route: Route, payment_hash: [u8; 32]) -> Result<(), HandleError> {
if route.hops.len() < 1 || route.hops.len() > 20 {
return Err(HandleError{err: "Route didn't go anywhere/had bogus size", msg: None});
return Err(HandleError{err: "Route didn't go anywhere/had bogus size", action: None});
}
let our_node_id = self.get_our_node_id();
for (idx, hop) in route.hops.iter().enumerate() {
if idx != route.hops.len() - 1 && hop.pubkey == our_node_id {
return Err(HandleError{err: "Route went through us but wasn't a simple rebalance loop to us", msg: None});
return Err(HandleError{err: "Route went through us but wasn't a simple rebalance loop to us", action: None});
}
}

Expand All @@ -633,13 +633,13 @@ impl ChannelManager {
let (first_hop_node_id, (update_add, commitment_signed, chan_monitor)) = {
let mut channel_state = self.channel_state.lock().unwrap();
let id = match channel_state.short_to_id.get(&route.hops.first().unwrap().short_channel_id) {
None => return Err(HandleError{err: "No channel available with first hop!", msg: None}),
None => return Err(HandleError{err: "No channel available with first hop!", action: None}),
Some(id) => id.clone()
};
let res = {
let chan = channel_state.by_id.get_mut(&id).unwrap();
if chan.get_their_node_id() != route.hops.first().unwrap().pubkey {
return Err(HandleError{err: "Node ID mismatch on first hop!", msg: None});
return Err(HandleError{err: "Node ID mismatch on first hop!", action: None});
}
chan.send_htlc_and_commit(htlc_msat, payment_hash.clone(), htlc_cltv, onion_packet)?
};
Expand Down Expand Up @@ -1099,11 +1099,11 @@ impl ChannelMessageHandler for ChannelManager {
//TODO: Handle errors and close channel (or so)
fn handle_open_channel(&self, their_node_id: &PublicKey, msg: &msgs::OpenChannel) -> Result<msgs::AcceptChannel, HandleError> {
if msg.chain_hash != self.genesis_hash {
return Err(HandleError{err: "Unknown genesis block hash", msg: None});
return Err(HandleError{err: "Unknown genesis block hash", action: None});
}
let mut channel_state = self.channel_state.lock().unwrap();
if channel_state.by_id.contains_key(&msg.temporary_channel_id) {
return Err(HandleError{err: "temporary_channel_id collision!", msg: None});
return Err(HandleError{err: "temporary_channel_id collision!", action: None});
}

let chan_keys = if cfg!(feature = "fuzztarget") {
Expand Down Expand Up @@ -1138,12 +1138,12 @@ impl ChannelMessageHandler for ChannelManager {
match channel_state.by_id.get_mut(&msg.temporary_channel_id) {
Some(chan) => {
if chan.get_their_node_id() != *their_node_id {
return Err(HandleError{err: "Got a message for a channel from the wrong node!", msg: None})
return Err(HandleError{err: "Got a message for a channel from the wrong node!", action: None})
}
chan.accept_channel(&msg)?;
(chan.get_value_satoshis(), chan.get_funding_redeemscript().to_v0_p2wsh(), chan.get_user_id())
},
None => return Err(HandleError{err: "Failed to find corresponding channel", msg: None})
None => return Err(HandleError{err: "Failed to find corresponding channel", action: None})
}
};
let mut pending_events = self.pending_events.lock().unwrap();
Expand All @@ -1165,7 +1165,7 @@ impl ChannelMessageHandler for ChannelManager {
match channel_state.by_id.remove(&msg.temporary_channel_id) {
Some(mut chan) => {
if chan.get_their_node_id() != *their_node_id {
return Err(HandleError{err: "Got a message for a channel from the wrong node!", msg: None})
return Err(HandleError{err: "Got a message for a channel from the wrong node!", action: None})
}
match chan.funding_created(msg) {
Ok((funding_msg, monitor_update)) => {
Expand All @@ -1176,7 +1176,7 @@ impl ChannelMessageHandler for ChannelManager {
}
}
},
None => return Err(HandleError{err: "Failed to find corresponding channel", msg: None})
None => return Err(HandleError{err: "Failed to find corresponding channel", action: None})
}
}; // Release channel lock for install_watch_outpoint call,
// note that this means if the remote end is misbehaving and sends a message for the same
Expand All @@ -1196,12 +1196,12 @@ impl ChannelMessageHandler for ChannelManager {
match channel_state.by_id.get_mut(&msg.channel_id) {
Some(chan) => {
if chan.get_their_node_id() != *their_node_id {
return Err(HandleError{err: "Got a message for a channel from the wrong node!", msg: None})
return Err(HandleError{err: "Got a message for a channel from the wrong node!", action: None})
}
let chan_monitor = chan.funding_signed(&msg)?;
(chan.get_funding_txo().unwrap(), chan.get_user_id(), chan_monitor)
},
None => return Err(HandleError{err: "Failed to find corresponding channel", msg: None})
None => return Err(HandleError{err: "Failed to find corresponding channel", action: None})
}
};
if let Err(_e) = self.monitor.add_update_monitor(monitor.get_funding_txo().unwrap(), monitor) {
Expand All @@ -1220,12 +1220,12 @@ impl ChannelMessageHandler for ChannelManager {
match channel_state.by_id.get_mut(&msg.channel_id) {
Some(chan) => {
if chan.get_their_node_id() != *their_node_id {
return Err(HandleError{err: "Got a message for a channel from the wrong node!", msg: None})
return Err(HandleError{err: "Got a message for a channel from the wrong node!", action: None})
}
chan.funding_locked(&msg)?;
return Ok(self.get_announcement_sigs(chan)?);
},
None => return Err(HandleError{err: "Failed to find corresponding channel", msg: None})
None => return Err(HandleError{err: "Failed to find corresponding channel", action: None})
};
}

Expand All @@ -1237,7 +1237,7 @@ impl ChannelMessageHandler for ChannelManager {
match channel_state.by_id.entry(msg.channel_id.clone()) {
hash_map::Entry::Occupied(mut chan_entry) => {
if chan_entry.get().get_their_node_id() != *their_node_id {
return Err(HandleError{err: "Got a message for a channel from the wrong node!", msg: None})
return Err(HandleError{err: "Got a message for a channel from the wrong node!", action: None})
}
let res = chan_entry.get_mut().shutdown(&*self.fee_estimator, &msg)?;
if chan_entry.get().is_shutdown() {
Expand All @@ -1247,7 +1247,7 @@ impl ChannelMessageHandler for ChannelManager {
(res, Some(chan_entry.remove_entry().1))
} else { (res, None) }
},
hash_map::Entry::Vacant(_) => return Err(HandleError{err: "Failed to find corresponding channel", msg: None})
hash_map::Entry::Vacant(_) => return Err(HandleError{err: "Failed to find corresponding channel", action: None})
}
};
for payment_hash in res.2 {
Expand All @@ -1272,7 +1272,7 @@ impl ChannelMessageHandler for ChannelManager {
match channel_state.by_id.entry(msg.channel_id.clone()) {
hash_map::Entry::Occupied(mut chan_entry) => {
if chan_entry.get().get_their_node_id() != *their_node_id {
return Err(HandleError{err: "Got a message for a channel from the wrong node!", msg: None})
return Err(HandleError{err: "Got a message for a channel from the wrong node!", action: None})
}
let res = chan_entry.get_mut().closing_signed(&*self.fee_estimator, &msg)?;
if res.1.is_some() {
Expand All @@ -1287,7 +1287,7 @@ impl ChannelMessageHandler for ChannelManager {
(res, Some(chan_entry.remove_entry().1))
} else { (res, None) }
},
hash_map::Entry::Vacant(_) => return Err(HandleError{err: "Failed to find corresponding channel", msg: None})
hash_map::Entry::Vacant(_) => return Err(HandleError{err: "Failed to find corresponding channel", action: None})
}
};
if let Some(broadcast_tx) = res.1 {
Expand Down Expand Up @@ -1335,7 +1335,7 @@ impl ChannelMessageHandler for ChannelManager {
($msg: expr, $err_code: expr, $data: expr) => {
return Err(msgs::HandleError {
err: $msg,
msg: Some(msgs::ErrorAction::UpdateFailHTLC {
action: Some(msgs::ErrorAction::UpdateFailHTLC {
msg: msgs::UpdateFailHTLC {
channel_id: msg.channel_id,
htlc_id: msg.htlc_id,
Expand Down Expand Up @@ -1494,16 +1494,16 @@ impl ChannelMessageHandler for ChannelManager {
let (source_short_channel_id, res) = match channel_state.by_id.get_mut(&msg.channel_id) {
Some(chan) => {
if chan.get_their_node_id() != *their_node_id {
return Err(HandleError{err: "Got a message for a channel from the wrong node!", msg: None})
return Err(HandleError{err: "Got a message for a channel from the wrong node!", action: None})
}
if !chan.is_usable() {
return Err(HandleError{err: "Channel not yet available for receiving HTLCs", msg: None});
return Err(HandleError{err: "Channel not yet available for receiving HTLCs", action: None});
}
let short_channel_id = chan.get_short_channel_id().unwrap();
pending_forward_info.prev_short_channel_id = short_channel_id;
(short_channel_id, chan.update_add_htlc(&msg, pending_forward_info)?)
},
None => return Err(HandleError{err: "Failed to find corresponding channel", msg: None}), //TODO: panic?
None => return Err(HandleError{err: "Failed to find corresponding channel", action: None}), //TODO: panic?
};

match claimable_htlcs_entry {
Expand Down Expand Up @@ -1544,11 +1544,11 @@ impl ChannelMessageHandler for ChannelManager {
match channel_state.by_id.get_mut(&msg.channel_id) {
Some(chan) => {
if chan.get_their_node_id() != *their_node_id {
return Err(HandleError{err: "Got a message for a channel from the wrong node!", msg: None})
return Err(HandleError{err: "Got a message for a channel from the wrong node!", action: None})
}
chan.update_fulfill_htlc(&msg)?
},
None => return Err(HandleError{err: "Failed to find corresponding channel", msg: None})
None => return Err(HandleError{err: "Failed to find corresponding channel", action: None})
}
};
if let Err(_e) = self.monitor.add_update_monitor(monitor.get_funding_txo().unwrap(), monitor) {
Expand All @@ -1562,11 +1562,11 @@ impl ChannelMessageHandler for ChannelManager {
let payment_hash = match channel_state.by_id.get_mut(&msg.channel_id) {
Some(chan) => {
if chan.get_their_node_id() != *their_node_id {
return Err(HandleError{err: "Got a message for a channel from the wrong node!", msg: None})
return Err(HandleError{err: "Got a message for a channel from the wrong node!", action: None})
}
chan.update_fail_htlc(&msg, HTLCFailReason::ErrorPacket { err: msg.reason.clone() })
},
None => return Err(HandleError{err: "Failed to find corresponding channel", msg: None})
None => return Err(HandleError{err: "Failed to find corresponding channel", action: None})
}?;

if let Some(pending_htlc) = channel_state.claimable_htlcs.get(&payment_hash) {
Expand Down Expand Up @@ -1637,11 +1637,11 @@ impl ChannelMessageHandler for ChannelManager {
match channel_state.by_id.get_mut(&msg.channel_id) {
Some(chan) => {
if chan.get_their_node_id() != *their_node_id {
return Err(HandleError{err: "Got a message for a channel from the wrong node!", msg: None})
return Err(HandleError{err: "Got a message for a channel from the wrong node!", action: None})
}
chan.update_fail_malformed_htlc(&msg, HTLCFailReason::Reason { failure_code: msg.failure_code, data: Vec::new() })
},
None => return Err(HandleError{err: "Failed to find corresponding channel", msg: None})
None => return Err(HandleError{err: "Failed to find corresponding channel", action: None})
}
}

Expand All @@ -1651,11 +1651,11 @@ impl ChannelMessageHandler for ChannelManager {
match channel_state.by_id.get_mut(&msg.channel_id) {
Some(chan) => {
if chan.get_their_node_id() != *their_node_id {
return Err(HandleError{err: "Got a message for a channel from the wrong node!", msg: None})
return Err(HandleError{err: "Got a message for a channel from the wrong node!", action: None})
}
chan.commitment_signed(&msg)?
},
None => return Err(HandleError{err: "Failed to find corresponding channel", msg: None})
None => return Err(HandleError{err: "Failed to find corresponding channel", action: None})
}
};
if let Err(_e) = self.monitor.add_update_monitor(chan_monitor.get_funding_txo().unwrap(), chan_monitor) {
Expand All @@ -1671,11 +1671,11 @@ impl ChannelMessageHandler for ChannelManager {
match channel_state.by_id.get_mut(&msg.channel_id) {
Some(chan) => {
if chan.get_their_node_id() != *their_node_id {
return Err(HandleError{err: "Got a message for a channel from the wrong node!", msg: None})
return Err(HandleError{err: "Got a message for a channel from the wrong node!", action: None})
}
chan.revoke_and_ack(&msg)?
},
None => return Err(HandleError{err: "Failed to find corresponding channel", msg: None})
None => return Err(HandleError{err: "Failed to find corresponding channel", action: None})
}
};
if let Err(_e) = self.monitor.add_update_monitor(chan_monitor.get_funding_txo().unwrap(), chan_monitor) {
Expand Down Expand Up @@ -1721,11 +1721,11 @@ impl ChannelMessageHandler for ChannelManager {
match channel_state.by_id.get_mut(&msg.channel_id) {
Some(chan) => {
if chan.get_their_node_id() != *their_node_id {
return Err(HandleError{err: "Got a message for a channel from the wrong node!", msg: None})
return Err(HandleError{err: "Got a message for a channel from the wrong node!", action: None})
}
chan.update_fee(&*self.fee_estimator, &msg)
},
None => return Err(HandleError{err: "Failed to find corresponding channel", msg: None})
None => return Err(HandleError{err: "Failed to find corresponding channel", action: None})
}
}

Expand All @@ -1735,10 +1735,10 @@ impl ChannelMessageHandler for ChannelManager {
match channel_state.by_id.get_mut(&msg.channel_id) {
Some(chan) => {
if chan.get_their_node_id() != *their_node_id {
return Err(HandleError{err: "Got a message for a channel from the wrong node!", msg: None})
return Err(HandleError{err: "Got a message for a channel from the wrong node!", action: None})
}
if !chan.is_usable() {
return Err(HandleError{err: "Got an announcement_signatures before we were ready for it", msg: None });
return Err(HandleError{err: "Got an announcement_signatures before we were ready for it", action: None });
}

let our_node_id = self.get_our_node_id();
Expand All @@ -1759,7 +1759,7 @@ impl ChannelMessageHandler for ChannelManager {
contents: announcement,
}, self.get_channel_update(chan).unwrap()) // can only fail if we're not in a ready state
},
None => return Err(HandleError{err: "Failed to find corresponding channel", msg: None})
None => return Err(HandleError{err: "Failed to find corresponding channel", action: None})
}
};
let mut pending_events = self.pending_events.lock().unwrap();
Expand Down
6 changes: 3 additions & 3 deletions src/ln/channelmonitor.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -316,7 +316,7 @@ impl ChannelMonitor {
for i in 0..pos {
let (old_secret, old_idx) = self.old_secrets[i as usize];
if ChannelMonitor::derive_secret(secret, pos, old_idx) != old_secret {
return Err(HandleError{err: "Previous secret did not match new one", msg: None})
return Err(HandleError{err: "Previous secret did not match new one", action: None})
}
}
self.old_secrets[pos as usize] = (secret, idx);
Expand Down Expand Up @@ -421,7 +421,7 @@ impl ChannelMonitor {
pub fn insert_combine(&mut self, mut other: ChannelMonitor) -> Result<(), HandleError> {
if self.funding_txo.is_some() {
if other.funding_txo.is_some() && other.funding_txo.as_ref().unwrap() != self.funding_txo.as_ref().unwrap() {
return Err(HandleError{err: "Funding transaction outputs are not identical!", msg: None});
return Err(HandleError{err: "Funding transaction outputs are not identical!", action: None});
}
} else {
self.funding_txo = other.funding_txo.take();
Expand Down Expand Up @@ -882,7 +882,7 @@ impl ChannelMonitor {
}
}
assert!(idx < self.get_min_seen_secret());
Err(HandleError{err: "idx too low", msg: None})
Err(HandleError{err: "idx too low", action: None})
}

pub fn get_min_seen_secret(&self) -> u64 {
Expand Down
2 changes: 1 addition & 1 deletion src/ln/msgs.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -380,7 +380,7 @@ pub enum ErrorAction {

pub struct HandleError { //TODO: rename me
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@TheBlueMatt off-topic to this diff, but since we're here: what's a better name for this? Would ErrorContext be more suitable?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I think eventually we need to separate out our error types, making this more of a MessageHandleError or PeerHandleError or something like that, and use a different type where that doesn't make sense.

pub err: &'static str,
pub msg: Option<ErrorAction>, //TODO: Make this required and rename it
pub action: Option<ErrorAction>, //TODO: Make this required
}

/// Struct used to return values from revoke_and_ack messages, containing a bunch of commitment
Expand Down
Loading