-
Notifications
You must be signed in to change notification settings - Fork 73
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
feat(p2p): moving gossiped blocks reception from validator to p2p client #811
feat(p2p): moving gossiped blocks reception from validator to p2p client #811
Conversation
p2p/validator.go
Outdated
/*err := v.localPubsubServer.PublishWithEvents(context.Background(), gossipedBlock, map[string][]string{EventTypeKey: {EventNewGossipedBlock}}) | ||
if err != nil { | ||
v.logger.Error("publishing event", "err", err) | ||
return false | ||
} | ||
}*/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@srene supposed to be commented out?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, but i forgot to remove it. done
117f052
to
808af10
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So I understand it's a simple change to move the local gossip part out of the validator, and into the message handler?
Looks good but I requested some renames
p2p/gossip.go
Outdated
@@ -22,6 +22,8 @@ type GossipMessage struct { | |||
// GossiperOption sets optional parameters of Gossiper. | |||
type GossiperOption func(*Gossiper) error | |||
|
|||
type NewMessage func(msg *GossipMessage) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we rename this GossipMessageHandler
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
p2p/gossip.go
Outdated
ps *pubsub.PubSub | ||
topic *pubsub.Topic | ||
sub *pubsub.Subscription | ||
msgReceiver NewMessage |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we rename to msgHandler
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
p2p/gossip.go
Outdated
} | ||
|
||
// NewGossiper creates new, ready to use instance of Gossiper. | ||
// | ||
// Returned Gossiper object can be used for sending (Publishing) and receiving messages in topic identified by topicStr. | ||
func NewGossiper(host host.Host, ps *pubsub.PubSub, topicStr string, logger types.Logger, options ...GossiperOption) (*Gossiper, error) { | ||
func NewGossiper(host host.Host, ps *pubsub.PubSub, topicStr string, msgReceiver NewMessage, logger types.Logger, options ...GossiperOption) (*Gossiper, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rename msgHandler
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
Co-authored-by: Daniel T <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
great work!
…ent (#811) Co-authored-by: Daniel T <[email protected]>
PR Standards
This PR decouples p2p gossip blocks validation from blocks reception. By doing that we avoid any interference of block store application in block propagation.
Opening a pull request should be able to meet the following requirements
Close #802
<-- Briefly describe the content of this pull request -->
For Author:
godoc
commentsFor Reviewer:
After reviewer approval: