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

fix(identify): make agentversion dynamic and add it to the peerstore #724

Merged
merged 1 commit into from
Aug 4, 2020

Conversation

jacobheun
Copy link
Contributor

The AgentVersion in identify shouldn't be static. In go-libp2p the AgentVersion is generated by the application name and version, so for go-ipfs this looks like go-ipfs/<version>. Ideally we would replicate this, however this is a trickier to handle in JS as we need to account for browser usage. At the very least we need to default to the js-libp2p version, so this adds that basic level of support.

This is extremely helpful for understanding what versions are being used on the network, which helps us when determining upgrade strategies and change rollouts on the network.

go-libp2p also tracks the AgentVersion of peers in the metadata book, so I added that here as well, which is necessary for actually checking this data (we were previously ignoring this).

Copy link
Member

@vasco-santos vasco-santos left a comment

Choose a reason for hiding this comment

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

LGTM! Just left a detail that we should probably think for a future iteration

@@ -175,6 +175,7 @@ class IdentifyService {
// Update peers data in PeerStore
this.peerStore.addressBook.set(id, listenAddrs.map((addr) => multiaddr(addr)))
this.peerStore.protoBook.set(id, protocols)
this.peerStore.metadataBook.set(id, 'AgentVersion', Buffer.from(message.agentVersion))
Copy link
Member

Choose a reason for hiding this comment

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

I was thinking that metadataBook would only be used by the application layer.
I think it is useful to store the agentVersion, but I wonder if we should have a way of identifying who stored data in the metadataBook

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If there is a need to separate them in the future we could make an internal book for libp2p but I'd prefer to avoid that unless a problem arises. Worst case scenario users/apps can override values, but if we document what libp2p stores there I think it should be fine.

@jacobheun jacobheun merged commit 726a746 into master Aug 4, 2020
@jacobheun jacobheun deleted the fix/agent-version branch August 4, 2020 16:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants