Skip to content
This repository has been archived by the owner on Oct 31, 2024. It is now read-only.

fix: remove duplicated certificate push on gossipsub #458

Merged
merged 2 commits into from
Feb 16, 2024
Merged

Conversation

hadjiszs
Copy link
Member

Description

The initial sender is supposed to be the only one to publish the Certificate on the gossipsub topic topos_gossip. The initial sender is the one who receives the Certificate from the gRPC API and not from gossipsub.

The rework on the storage by moving the management of the pending and precedence pool on the storage layer introduced the loss of the origin of the Certificate (whether it is received from gossipsub or grpc), consequently, the message was pushed by everyone on the topos_gossip topic on both cases since this storage rework.

Additions and Changes

  • Before (default config, see here)
    • Author based message id: (PeerId, nonce), so we had as many duplicate of Certificate as the number of peers
  • After
    • Content based message id: hash(payload), consequently the other peers will detect the payload being the same, so it will not be published nor downloaded multiple times

PR Checklist:

  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added or updated tests that comprehensively prove my change is effective or that my feature works

@hadjiszs hadjiszs requested a review from a team as a code owner February 16, 2024 12:56
@CLAassistant
Copy link

CLAassistant commented Feb 16, 2024

CLA assistant check
All committers have signed the CLA.

Copy link

codecov bot commented Feb 16, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (edf86ee) 0.00% compared to head (c5598c6) 70.99%.
Report is 7 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff            @@
##           main     #458       +/-   ##
=========================================
+ Coverage      0   70.99%   +70.99%     
=========================================
  Files         0      223      +223     
  Lines         0    12285    +12285     
=========================================
+ Hits          0     8722     +8722     
- Misses        0     3563     +3563     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

crates/topos-p2p/src/behaviour/gossip.rs Show resolved Hide resolved
crates/topos-p2p/src/behaviour/gossip.rs Outdated Show resolved Hide resolved
Co-authored-by: David <[email protected]>
Signed-off-by: Monir Hadji <[email protected]>
@Freyskeyd Freyskeyd merged commit b0e88dc into main Feb 16, 2024
21 checks passed
@Freyskeyd Freyskeyd deleted the feat/TP-868 branch February 16, 2024 18:40
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants