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

feat: ipns over pubsub #1559

Merged
merged 9 commits into from
Dec 4, 2018
Merged

feat: ipns over pubsub #1559

merged 9 commits into from
Dec 4, 2018

Conversation

vasco-santos
Copy link
Member

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

This PR addresses the IPNS over Pubsub feature.

It needs the following PRs:

Unblocks:

@ghost ghost assigned vasco-santos Sep 13, 2018
@ghost ghost added the status/in-progress In progress label Sep 13, 2018
@vasco-santos vasco-santos changed the title feat: ipns over pubsub [WIP] feat: ipns over pubsub Sep 13, 2018
@vasco-santos vasco-santos force-pushed the feat/ipns-over-pubsub branch 4 times, most recently from 0a2d8ca to 3c90cbf Compare September 14, 2018 09:46
@vasco-santos
Copy link
Member Author

vasco-santos commented Sep 17, 2018

@diasdavid regarding the weekly sync question:

js-datastore-pubsub is used as middleware for pubsub to be used as a datastore with TieredDatastore. This way, we can combine multiple put and get in the IPNS land transparently with a "single" operation. Perhaps the module name is not the best.

We provide to the IPNS the routing, as a TieredDatastore composed by the OfflineDatastore and the PubsubDatastore. (NOTE: It now receives the OfflineDatastore, but this will be replaced with the DHT, once it gets ready). After that, when an IPNS operation happens, such as a publish, the IPNS just puts it to the routing.

All in all, pubsub-datastore receives a datastore (repo's datastore in this case), as well as a pubsub implementation. It provides the same API as a general store (put, get, ...) and translates those to what they really represent in the pubsub world (publish and subscribe). After that, it also takes care of the subscriptions handling and their consequent store in the node's repo datastore.

Moreover, go-team uses a similar approach with go-libp2p-pubsub-router, but their module is specific for IPNS, while pubsub-datastore is agnostic to where it is used. In addition, they also use the same logic for the TieredDatastore here.

I hope to have been clear with this explanation. Let me know if you agree, or if you have any other direction that this sould go.

@vasco-santos vasco-santos force-pushed the feat/ipns-over-pubsub branch from 3c90cbf to 61a44fb Compare October 3, 2018 17:15
src/core/ipns/publisher.js Outdated Show resolved Hide resolved
@dirkmc
Copy link
Contributor

dirkmc commented Oct 16, 2018

This is a really nice approach 👍

@vasco-santos vasco-santos force-pushed the feat/ipns-over-pubsub branch from 61a44fb to 0627b06 Compare October 17, 2018 16:25
Copy link
Member

@alanshaw alanshaw left a comment

Choose a reason for hiding this comment

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

src/core/components/name-pubsub.js Outdated Show resolved Hide resolved
src/core/components/name-pubsub.js Outdated Show resolved Hide resolved
src/core/components/name.js Outdated Show resolved Hide resolved
src/core/ipns/path.js Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
src/core/ipns/routing/pubsub.js Outdated Show resolved Hide resolved
test/cli/name-pubsub.js Outdated Show resolved Hide resolved
test/cli/name-pubsub.js Outdated Show resolved Hide resolved
test/cli/name-pubsub.js Outdated Show resolved Hide resolved
test/cli/name-pubsub.js Show resolved Hide resolved
@vasco-santos vasco-santos force-pushed the feat/ipns-over-pubsub branch 7 times, most recently from 84d851c to ba944df Compare October 27, 2018 22:26
@vasco-santos vasco-santos changed the title [WIP] feat: ipns over pubsub feat: ipns over pubsub Oct 28, 2018
@vasco-santos vasco-santos added the status/blocked Unable to be worked further until needs are met label Oct 28, 2018
@vasco-santos vasco-santos force-pushed the feat/ipns-over-pubsub branch 3 times, most recently from aac7a52 to 83fee00 Compare November 7, 2018 10:35
@vasco-santos vasco-santos mentioned this pull request Nov 14, 2018
@vasco-santos vasco-santos force-pushed the feat/ipns-over-pubsub branch 3 times, most recently from 196dd4c to 03c24e1 Compare November 29, 2018 12:37
src/core/components/name-pubsub.js Show resolved Hide resolved
src/core/components/name-pubsub.js Outdated Show resolved Hide resolved
@@ -2,11 +2,13 @@

const series = require('async/series')
const Bitswap = require('ipfs-bitswap')
const get = require('lodash/get')
Copy link
Member

Choose a reason for hiding this comment

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

use smaller package
https://bundlephobia.com/result?p=lodash.get@4.4.2
vs
https://bundlephobia.com/result?p=dlv@1.1.2

Suggested change
const get = require('lodash/get')
const get = require('dlv')

Copy link
Member Author

Choose a reason for hiding this comment

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

Cool, thanks! Will add dlv to package.json as well.

Copy link
Member

Choose a reason for hiding this comment

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

I'm fine with using dlv over loadash.get but all other occurances of lodash.get need to be replaced as well otherwise we're just adding more to our bundle.

Copy link
Member Author

Choose a reason for hiding this comment

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

If you agree, I can create other PR with all those occurrences changed to dlv.

@@ -28,6 +28,7 @@ const schema = Joi.object().keys({
}).allow(null),
EXPERIMENTAL: Joi.object().keys({
pubsub: Joi.boolean(),
namesysPubsub: Joi.boolean(),
Copy link
Member

Choose a reason for hiding this comment

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

just pick one and use it everywhere

src/core/ipns/routing/offline-datastore.js Outdated Show resolved Hide resolved
src/core/ipns/routing/pubsub-datastore.js Outdated Show resolved Hide resolved
src/core/ipns/routing/pubsub-datastore.js Outdated Show resolved Hide resolved
src/core/ipns/routing/pubsub-datastore.js Outdated Show resolved Hide resolved
src/core/ipns/routing/pubsub-datastore.js Outdated Show resolved Hide resolved
src/core/ipns/routing/pubsub-datastore.js Outdated Show resolved Hide resolved
@vasco-santos vasco-santos force-pushed the feat/ipns-over-pubsub branch 3 times, most recently from fb48ba9 to 6f83aa1 Compare November 30, 2018 15:18
@vasco-santos vasco-santos force-pushed the feat/ipns-over-pubsub branch from 6f83aa1 to b2755a0 Compare December 1, 2018 14:34
src/core/components/start.js Outdated Show resolved Hide resolved
src/core/components/pubsub.js Outdated Show resolved Hide resolved
@@ -48,7 +49,10 @@ class OfflineDatastore {
return callback(errcode(new Error(errMsg), 'ERR_GENERATING_ROUTING_KEY'))
}

this._repo.datastore.put(routingKey, value, callback)
// Marshal to libp2p record as the DHT does
let record = new Record(key, value)
Copy link
Member

Choose a reason for hiding this comment

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

Can be const

Copy link
Member Author

Choose a reason for hiding this comment

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

Should we open an issue in aegir to target this in the lint?

src/core/ipns/routing/pubsub-datastore.js Outdated Show resolved Hide resolved
src/http/index.js Outdated Show resolved Hide resolved
if (!this._options.EXPERIMENTAL.pubsub) {
this.log('EXPERIMENTAL pubsub is enabled to use IPNS pubsub')
this._options.EXPERIMENTAL.pubsub = true
}
Copy link
Member

Choose a reason for hiding this comment

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

Does go-ipfs do IPNS over PubSub as an experimental flag? Why not just one flag that enables all of PubSub

Copy link
Member Author

@vasco-santos vasco-santos Dec 3, 2018

Choose a reason for hiding this comment

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

Yes, go-ipfs uses an experimental flag for IPNS over Pubsub: --enable-namesys-pubsub is the CLI flag

Copy link
Member

@daviddias daviddias Dec 3, 2018

Choose a reason for hiding this comment

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

"Experimental features on Alpha software", how much more bleeding edge can you go 😅

@alanshaw
Copy link
Member

alanshaw commented Dec 3, 2018

@vasco-santos released interface-ipfs-core@0.89 with ipfs-inactive/interface-js-ipfs-core#395

@vasco-santos
Copy link
Member Author

Thanks! Updated the package.json with it

@alanshaw alanshaw merged commit 8712542 into master Dec 4, 2018
@ghost ghost removed the status/in-progress In progress label Dec 4, 2018
@alanshaw alanshaw deleted the feat/ipns-over-pubsub branch December 4, 2018 08:41
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.

5 participants