-
Notifications
You must be signed in to change notification settings - Fork 159
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
fix: fetch tipsets in a single request #5071
Conversation
I will do a patch release once this is reviewed and merged. |
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.
I'd love if our leading P2P expert could pronounce if we're not missing anything here. @hanabi1224
src/chain_sync/chain_muxer.rs
Outdated
let tipset = Self::get_full_tipset( | ||
network.clone(), | ||
chain_store.clone(), | ||
source, |
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.
In the previous logic, bitswap reqeusts are sent to all connected peers, should we fallback to non-source peers if it fails here?
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.
No, if a peer tells us about a new block without having it themselves, we should ignore it.
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.
Oh wait, as I understand, in gossipsub source could be a proxy peer that does not actually own the block
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.
https://docs.rs/libp2p-gossipsub/latest/libp2p_gossipsub/enum.Event.html#variants
propagation_source: PeerId
The peer that forwarded us this message.
if let gossipsub::Event::Message {
propagation_source: source,
message,
message_id: _,
} = e
{
let topic = message.topic.as_str();
let message = message.data;
trace!("Got a Gossip Message from {:?}", source);
if topic == pubsub_block_str {
match from_slice_with_fallback::<GossipBlock>(&message) {
Ok(b) => {
emit_event(
network_sender_out,
NetworkEvent::PubsubMessage {
source,
message: PubsubMessage::Block(b),
},
)
.await;
}
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.
"Fixed" by broadcasting the chain_exchange request. Eventually, all of this code needs to be reworked.
Summary of changes
Changes introduced in this pull request:
Reference issue to close (if applicable)
Closes #5032
Other information and links
Change checklist