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

P2p gossip propagate once #809

Merged
merged 5 commits into from Apr 15, 2019
Merged

P2p gossip propagate once #809

merged 5 commits into from Apr 15, 2019

Conversation

y0sher
Copy link
Contributor

@y0sher y0sher commented Apr 12, 2019

No description provided.

relayQ: relayChan,
messageQ: make(chan protocolMessage, messageQBufferSize),
propagateQ: make(chan service.MessageValidation, propagateHandleBufferSize),
//invalidMessageQ: make(map[hash]bool), // todo : remember to drain this
Copy link
Contributor Author

Choose a reason for hiding this comment

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

to remove

}()
return nil
}
return
Copy link
Contributor Author

Choose a reason for hiding this comment

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

not needed

p.Debug("sending message to peer %v, hash %d", p.pubkey, checksum)
err := p.net.SendMessage(p.pubkey, ProtocolName, msg)
if err != nil {
p.Log.Info("Gossip protocol failed to send msg (calcHash %d) to peer %v, first attempt. err=%v", checksum, p.pubkey, err)
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be an Error, not info.

Copy link
Contributor

Choose a reason for hiding this comment

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

(I know it's my code :))

p.Log.Info("Gossip protocol failed to send msg (calcHash %d) to peer %v, second attempt. err=%v", checksum, p.pubkey, err)
return
}
p.Log.Info("Gossip protocol failed to send msg (calcHash %d) to peer %v, second attempt. err=%v", checksum, p.pubkey, err)
Copy link
Contributor

Choose a reason for hiding this comment

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

also should be an error

for p := range prot.peers {
peer := prot.peers[p]
peer.send(data, h) // non blocking
for e := range exclude {
Copy link
Contributor

Choose a reason for hiding this comment

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

In which case do you expect seeing more than one sender?

Copy link
Contributor

Choose a reason for hiding this comment

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

Currently, the only call to this method passes one sender

msg := &pb.ProtocolMessage{
Metadata: header,
Payload: &pb.Payload{Data: &pb.Payload_Payload{payload}},
}

prot.processMessage(msg)
prot.processMessage(prot.localNodePubkey, msg) // todo: pass sender
Copy link
Contributor

Choose a reason for hiding this comment

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

Why the "todo"?

// message and will determine its validity, therefore we can return in such case as well
return
}
// todo : - maybe block this peer since he sends us old messages
Copy link
Contributor

Choose a reason for hiding this comment

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

add log print

…ght need to pass it to protocols and back to gossip in validation phase).

- do not propagate old messages. (ever)
- remove global invalid cache, since we'll never propagate twice even if valids
- maybe remove gossip "invalid" reporting since we'll only propagate valid and won't save old. (we can keep it for future use.)
@y0sher y0sher merged commit 0bbbd71 into develop Apr 15, 2019
@y0sher y0sher deleted the p2p_gossip_propagateonce branch June 25, 2019 07:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants