Skip to content
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

whisper: messge bundling #15666

Merged
merged 3 commits into from
Dec 21, 2017
Merged

whisper: messge bundling #15666

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9020bfb
whisper: messge bundling
gluk256 Dec 13, 2017
b7ecc49
whisper: process all received envelopes, do not send empty bundle
gluk256 Dec 20, 2017
b044e9a
whisper: error logging changed
gluk256 Dec 20, 2017
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
18 changes: 9 additions & 9 deletions whisper/whisperv6/peer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,17 +157,17 @@ func (p *Peer) broadcast() error {
}
}

// transmit the unknown batch (potentially empty)
if err := p2p.Send(p.ws, messagesCode, bundle); err != nil {
return err
}
if len(bundle) > 0 {
// transmit the batch of envelopes
if err := p2p.Send(p.ws, messagesCode, bundle); err != nil {
return err
}

// mark envelopes only if they were successfully sent
for _, e := range bundle {
p.mark(e)
}
// mark envelopes only if they were successfully sent
for _, e := range bundle {
p.mark(e)
}

if len(bundle) > 0 {
log.Trace("broadcast", "num. messages", len(bundle))
}
return nil
Expand Down
13 changes: 10 additions & 3 deletions whisper/whisperv6/whisper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -520,16 +520,23 @@ func (wh *Whisper) runMessageLoop(p *Peer, rw p2p.MsgReadWriter) error {
log.Warn("failed to decode envelopes, peer will be disconnected", "peer", p.peer.ID(), "err", err)
return errors.New("invalid envelopes")
}

var trouble error
Copy link
Member

Choose a reason for hiding this comment

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

maybe have it to be trouble := []error{} and store the n first errors ? More than 1 error can happen in real life.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

for _, env := range envelopes {
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")
if err != nil && trouble == nil {
// only report the first occurring error
trouble = err
}
if cached {
p.mark(env)
}
}

if trouble != nil {
log.Warn("bad envelope received, peer will be disconnected", "peer", p.peer.ID(), "err", trouble)
return errors.New("invalid envelope")
}
case p2pCode:
// peer-to-peer message, sent directly to peer bypassing PoW checks, etc.
// this message is not supposed to be forwarded to other peers, and
Expand Down