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 2 commits
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
25 changes: 15 additions & 10 deletions whisper/whisperv6/peer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,21 +149,26 @@ func (peer *Peer) expire() {
// broadcast iterates over the collection of envelopes and transmits yet unknown
// ones over the network.
func (p *Peer) broadcast() error {
var cnt int
envelopes := p.host.Envelopes()
bundle := make([]*Envelope, 0, len(envelopes))
for _, envelope := range envelopes {
if !p.marked(envelope) {
err := p2p.Send(p.ws, messagesCode, envelope)
if err != nil {
return err
} else {
p.mark(envelope)
cnt++
}
bundle = append(bundle, envelope)
}
}
if cnt > 0 {
log.Trace("broadcast", "num. messages", cnt)

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)
}

log.Trace("broadcast", "num. messages", len(bundle))
}
return nil
}
Expand Down
29 changes: 19 additions & 10 deletions whisper/whisperv6/whisper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -515,18 +515,27 @@ func (wh *Whisper) runMessageLoop(p *Peer, rw p2p.MsgReadWriter) error {
log.Warn("unxepected status message received", "peer", p.peer.ID())
case messagesCode:
// decode the contained envelopes
var envelope Envelope
if err := packet.Decode(&envelope); err != nil {
log.Warn("failed to decode envelope, peer will be disconnected", "peer", p.peer.ID(), "err", err)
return errors.New("invalid envelope")
var envelopes []*Envelope
if err := packet.Decode(&envelopes); err != nil {
log.Warn("failed to decode envelopes, peer will be disconnected", "peer", p.peer.ID(), "err", err)
return errors.New("invalid envelopes")
}
cached, err := wh.add(&envelope)
if err != nil {
log.Warn("bad envelope received, peer will be disconnected", "peer", p.peer.ID(), "err", err)
return errors.New("invalid envelope")

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 && trouble == nil {
// only report the first occurring error
trouble = err
}
if cached {
p.mark(env)
}
}
if cached {
p.mark(&envelope)

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.
Expand Down