Make field error of ErrorPacket (former HandleError) mandatory #390
Conversation
|
Fuck yes! rust-lightning finally growing up :). Not sure about the name ErrorPacket, though, seems to imply sending a message on the wire or so. Maybe something even more generic like LightningError or just crate::Error? |
ariard
force-pushed
the
2019-11-consistent-errors
branch
2 times, most recently
from
130c82f to
6e78406
Compare
|
LightningError won by default of better name. |
src/ln/router.rs
Outdated
| @@ -404,13 +404,13 @@ macro_rules! secp_verify_sig { | |||
| ( $secp_ctx: expr, $msg: expr, $sig: expr, $pubkey: expr ) => { | |||
| match $secp_ctx.verify($msg, $sig, $pubkey) { | |||
| Ok(_) => {}, | |||
| Err(_) => return Err(HandleError{err: "Invalid signature from remote node", action: None}), | |||
| Err(_) => return Err(LightningError{err: "Invalid signature from remote node", action: ErrorAction::SendErrorMessage { msg: ErrorMessage { channel_id: [0;32], data: "Invalid signature".to_string() }}}), | |||
There was a problem hiding this comment.
Hmmmmmmm I really dont know how I feel about this. Does it really make sense to force-close all our channels with this peer because they sent us an invalid gossip message (also, do we properly handle this - dont we need to do force-closing on our end to send an ErrorMessage with a null channel_id, too?)
There was a problem hiding this comment.
If we follow specs that the behavior to do:
if bitcoin_signature_1, bitcoin_signature_2, node_signature_1 OR node_signature_2 are invalid OR NOT correct: SHOULD fail the connection.
But we may have peer on the network being more liberal and not doing their sanitize of third-party messages cleanly so we can be liberal too for now and have better error handling latter (with maybe new error actions for Router, like ban peer only if send more than X wrong-sigs channel announcements)
There was a problem hiding this comment.
Right, and Error like that implies maybe closing channels with that peer, which is confusing, and, honestly, probably not what we want to do.
ariard
force-pushed
the
2019-11-consistent-errors
branch
from
6e78406 to
f3c4758
Compare
We also fulfilled last empty ErrorAction: - Router secp fail : IgnoreError - processing error in Router : IgnoreError - get_channel_update too early : IgnoreError
ariard
force-pushed
the
2019-11-consistent-errors
branch
from
f3c4758 to
ddbe53f
Compare
First step in a set of PRs to simplify our errors handling.