- 
                Notifications
    You must be signed in to change notification settings 
- Fork 209
Gossipsub Extensions #630
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
Gossipsub Extensions #630
Conversation
996ce03    to
    97ebfb9      
    Compare
  
    4f69cbd    to
    222428a      
    Compare
  
    | } | ||
|  | ||
| func (e *testExtension) AddPeer(id peer.ID) { | ||
| e.sendRPC(id, &RPC{ | 
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.
Since you send an RPC here, should you read it from your peer as well?
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. Might be easier to test as well. thanks!
222428a    to
    5299023      
    Compare
  
    | Just a friendly reminder that you can view the diffs between versions of these commits at: or (preferred) by locally running: The latter is preferred as it also tells you which commit introduced the change. | 
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.
Looking good to me
18833cc    to
    7402459      
    Compare
  
    Useful to let protocols add to the first message
939defb    to
    4e4faae      
    Compare
  
    4e4faae    to
    857c8cc      
    Compare
  
    
Before Merge:
An alternative would be to rely on peers ignoring protobuf messages they don't understand. It seems better to explicitly signal support for the Extensions control Message and only send it if you know your peer supports it. I'll create a new specs issue/PR to discuss this.
This implements the Gossipsub Extensions Control Message from libp2p/specs#684. It also implements the Test Extension libp2p/specs#686 as an interop test.
There is a rust-libp2p implementation, and using the gossip-interop tester, I've tested interop between these implementations for the Test Extension.
Don't let the diff stats scare you, they are mostly generated protobuf code.