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.

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
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
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)

// transmit the unknown batch (potentially empty)
Copy link
Member

Choose a reason for hiding this comment

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

transmit the (potentially empty) batch of unknown 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)
}

if len(bundle) > 0 {
log.Trace("broadcast", "num. messages", len(bundle))
}
return nil
}
Expand Down
24 changes: 13 additions & 11 deletions whisper/whisperv6/whisper.go
Original file line number Diff line number Diff line change
Expand Up @@ -515,18 +515,20 @@ 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")
}
if cached {
p.mark(&envelope)
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")
Copy link
Member

Choose a reason for hiding this comment

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

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 cached {
p.mark(env)
}
}
case p2pCode:
// peer-to-peer message, sent directly to peer bypassing PoW checks, etc.
Expand Down