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

fix: revert libp2p records being signed for ipns #1570

Merged

Conversation

vasco-santos
Copy link
Member

@vasco-santos vasco-santos commented Sep 17, 2018

The js-ipfs#1543 broke the interop tests for IPNS on interop#26.

The js-ipfs#1543 was added after my analysis for the implementation of IPNS over Pubsub, as I wanted to have exactly the same behavior and interface as the DHT, in order to have an easy to plug DHT afterward. Accordingly:

DHT PUT:
js-libp2p-kad-dht/index.js#L166
js-libp2p-kad-dht/utils.js#L150

So, I wanted to make this logic available to Pubsub / local and it is all good in JS land. However,go-ipfs does not do that:

go-ipfs/namesys/publisher.go#L278
go-libp2p-kad-dht/dht.go#L149

And this causes and interop problem!

At first, different protobuf definitions:
go-libp2p-record/record.proto
js-libp2p-record/record.proto.js

So, when we publish using a GO node, followed by a JS node resolve, the resolve will try to validate the record, but it will break js-libp2p-record/record.js#L42

There is no author, and trying to get ID is 💥 Other than that, the interop is fine. I believe that, as the IPNS record is signed inside the libp2p-record, it is not bad to have it not signed. It would be better to have it signed, but we can handle this together with DHT Interop.

Other note: This can be one of many problems regarding DHT interop, and I will look deeper on that later.

@ghost ghost assigned vasco-santos Sep 17, 2018
@ghost ghost added the status/in-progress In progress label Sep 17, 2018
@vasco-santos vasco-santos mentioned this pull request Sep 17, 2018
5 tasks
@vasco-santos vasco-santos force-pushed the fix/revert-libp2p-records-for-ipns-should-be-signed branch from e344116 to ce9fb25 Compare September 20, 2018 16:24
@alanshaw alanshaw merged commit 855b3bd into master Sep 28, 2018
@alanshaw alanshaw deleted the fix/revert-libp2p-records-for-ipns-should-be-signed branch September 28, 2018 14:30
@ghost ghost removed the status/in-progress In progress label Sep 28, 2018
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.

2 participants