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

add msgs to msg pool #797

Merged
merged 2 commits into from
Nov 2, 2020
Merged

add msgs to msg pool #797

merged 2 commits into from
Nov 2, 2020

Conversation

dutterbutter
Copy link
Contributor

Summary of changes
Changes introduced in this pull request:

  • Adds msgpool to libp2p service
  • Introduces logic to add msgs to mpool

Reference issue to close (if applicable)

Closes #763

Other information and links

@@ -95,9 +95,10 @@ pub enum NetworkMessage {
},
}
/// The Libp2pService listens to events from the Libp2p swarm.
pub struct Libp2pService<DB: BlockStore> {
pub struct Libp2pService<DB: BlockStore, T: 'static> {
Copy link
Contributor

Choose a reason for hiding this comment

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

There isn't really a reason to constrain the generics on the struct, should just be constrained on the impl. You can remove the 'static constraint on the MessagePool struct, there was no reason for that to be there.

Suggested change
pub struct Libp2pService<DB: BlockStore, T: 'static> {
pub struct Libp2pService<DB, T> {

Comment on lines 210 to +217
emit_event(&self.network_sender_out, NetworkEvent::PubsubMessage{
source,
message: PubsubMessage::Message(m),
message: PubsubMessage::Message(m.clone()),
}).await;
// add message to message pool
if let Err(e) = self.mpool.add(m).await {
warn!("Gossip Message failed to be added to Message pool");
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Just noting here that it's not ideal to clone all messages because it's only needed inside mempool and not used from the emitted event, but can come in a following PR

Copy link
Member

Choose a reason for hiding this comment

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

That's true. I don think anywhere else uses will listen on the channel for a message. I can make a PR for that

Comment on lines 210 to +217
emit_event(&self.network_sender_out, NetworkEvent::PubsubMessage{
source,
message: PubsubMessage::Message(m),
message: PubsubMessage::Message(m.clone()),
}).await;
// add message to message pool
if let Err(e) = self.mpool.add(m).await {
warn!("Gossip Message failed to be added to Message pool");
}
Copy link
Member

Choose a reason for hiding this comment

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

That's true. I don think anywhere else uses will listen on the channel for a message. I can make a PR for that

@dutterbutter dutterbutter merged commit 7da5234 into main Nov 2, 2020
@dutterbutter dutterbutter deleted the db/pub-msg branch November 2, 2020 16:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Network Libp2p and PubSub stuff Status: Needs Review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add messages received over gossipsub to the message pool
3 participants