whisper: messge bundling#15666
Conversation
|
Need review from @gballet |
| cached, err := wh.add(env) | ||
| if err != nil { | ||
| log.Warn("bad envelope received, peer will be disconnected", "peer", p.peer.ID(), "err", err) | ||
| return errors.New("invalid envelope") |
There was a problem hiding this comment.
Wait, does that mean that if 1 of the envelopes has a bad format, then you will stop decoding all the envelopes? That sounds very harsh, and a potential threat to the network: all an attacker has to do to disrupt communications is to start sending invalid envelopes. They would get disconnected, sure, but if they hold a significant portion of the network they can still use this as a kill switch.
So instead, I would rather skip the bad envelope and call wh.add on correct envelopes.
| if cnt > 0 { | ||
| log.Trace("broadcast", "num. messages", cnt) | ||
|
|
||
| // transmit the unknown batch (potentially empty) |
There was a problem hiding this comment.
transmit the (potentially empty) batch of unknown envelopes
|
@gballet thanks for your review, i have fixed accordingly |
gballet
left a comment
There was a problem hiding this comment.
I'd like to have more than one error reported, if there are more than one errors.
| return errors.New("invalid envelopes") | ||
| } | ||
|
|
||
| var trouble error |
There was a problem hiding this comment.
maybe have it to be trouble := []error{} and store the n first errors ? More than 1 error can happen in real life.
There was a problem hiding this comment.
i am reluctant to complicate the processing logic. instead, i have changed the function so that it will report all the errors by logging. i have even upgraded the log level from warning to error, because this kind of error should be exceptional.
Changed the communication protocol for ordinary message, according to EIP 627. Messages will be send in bundles, i.e. array of messages will be sent instead of single message.
Changed the communication protocol for ordinary message, according to the EIP#627.
Messages will be send in bundles, i.e. array of messages will be sent instead of single message.