Refactor handle_open_channel as a wrapper error handling function #140
Conversation
|
Side-note: I took the opportunity to review the BOLT 2 open_channel message part, I think we're fine on the imperative part, maybe if we to follow the optional otems we can add a msg.funding_satoshis < min_chan_size { return Err }? |
src/ln/channelmanager.rs
Outdated
| Some(msgs::ErrorAction::IgnoreError) => { | ||
| return Err(false); | ||
| }, | ||
| Some(msgs::ErrorAction::SendErrorMessage { msg: error_msg }) => { |
There was a problem hiding this comment.
Wait, why change the return type to Result<.., bool>? Should still be able to return the same Err object, just possibly handling the channel closure on Disconnect/SendErrorMessage first?
There was a problem hiding this comment.
Was my inquiry, do we want to give back exactly the same Err object even if action is no more accurate? At first I've written return { HandleError: ...action : msgs::ErrorAction::IgnoreError } for DisconnectPeer and SendErrorMessage.
In the wrapper we just manage channel closure, not the others errors actions given back?
There was a problem hiding this comment.
Yea, I assume just return the same Err, since the ErrorMessage still needs to be relayed to the peer.
ariard
force-pushed
the
error_handling_refactor
branch
from
567afe5 to
1653afb
Compare
| }, | ||
| Some(msgs::ErrorAction::IgnoreError) => {}, | ||
| Some(msgs::ErrorAction::SendErrorMessage { msg: _ }) => { | ||
| $self.force_close_channel(&$channel_id); |
There was a problem hiding this comment.
Do we want a force_close i.e an unilateral close on every error case ? maybe a close_channel here
There was a problem hiding this comment.
No, I think any case where we hit an ErrorMessage we'll want a full unilateral close, sadly.
That said, we do need to extend this a bit as an ErrorMessage with a channel_id of [0u8; 32] indicates we want to force-close all channels from a given node_id, not just the one.
|
Taken as #147. |
Okay, start the refactor of handle_MSG to handle our own Error message and closing the channel when needed. First try redesign, do we really want to dry up handle_MSG of HandleError so ChannelMessageHandler client doesn't have to do it by itself ?
No sure about the macro stuff, it's really moving it from peer_handler to channel_manager in some way..