[WIP] experimental routers to make ipns faster #2201
Conversation
resolves #2190
Printing raw buffers can end up outputting characters into the terminal that mess up the encoding for all subsequent lines, which is a pain when debugging things. This change just encodes the buffer before printing to stop that from happening.
fixes: #1918 `ipns name resolve` dns tests moved to interface-core resolve call now returns a string as per documention.
There was a problem hiding this comment.
Just a note for the future - I'm finding this really difficult to review. I don't have any context at all (no description in the PR) and there's no changes or additions to docs in the commits.
Are there some docs where I can read about what this is and how it works?
Minor, but there are lots of console.log statements in this PR that need to be replaced with called to debug.
| } | ||
|
|
||
| log(`ipns record for ${key.toString()} was stored in the routing`) | ||
| log(`ipns record for /ipns/${peerId.toB58String()} was stored in the routing`) |
There was a problem hiding this comment.
Why did this change from key to peerId?
| ipnsStores.push(new DnsDatastore(ipfs._options.ipns)) | ||
| ipnsStores.push(new MDnsDatastore(ipfs._options.ipns)) | ||
| return new ExperimentalTieredDatastore(ipnsStores) | ||
| } |
There was a problem hiding this comment.
No DHT or pubsub datastore if ipnsDNS is enabled?
src/core/ipns/routing/config.js
Outdated
| ipnsStores.push(offlineDatastore) | ||
| } else { | ||
| // Add DHT if we are online | ||
| if (ipfs.isOnline()) { |
There was a problem hiding this comment.
Not sure I understand the reason behind this change?
| const keyStr = keyToBase32(key) | ||
| const data = await ky | ||
| .put( | ||
| 'https://ipns.dev', |
| // https://dns.google.com/experimental | ||
| // https://cloudflare-dns.com/dns-query | ||
| // https://mozilla.cloudflare-dns.com/dns-query | ||
| return dohBinary('https://cloudflare-dns.com/dns-query', 'dns.ipns.dev', key) |
| .catch(err => { | ||
| log.error(err) | ||
| setImmediate(() => callback(Errors.notFoundError())) | ||
| }) |
There was a problem hiding this comment.
Promises force then and catch onto a future tick so setImmediate should not be necessary here unless there's a different reason I'm missing?
|
|
||
| // DNS datastore aims to mimic the same encoding as routing when storing records | ||
| // to the local datastore | ||
| class MDNSDataStore { |
There was a problem hiding this comment.
I don't understand how this is MDNS?
|
|
||
| // Workers datastore aims to mimic the same encoding as routing when storing records | ||
| // to the local datastore | ||
| class WorkersDataStore { |
There was a problem hiding this comment.
How does this differ from the MDNS datastore?
| ipnsPubsub: argv.enableNamesysPubsub, | ||
| dht: argv.enableDhtExperiment, | ||
| sharding: argv.enableShardingExperiment | ||
| }, | ||
| ipns: { | ||
| alias: argv.experimentalIpnsAlias |
There was a problem hiding this comment.
These new constructor options need to be documented on the README.
| const log = debug('ipfs:ipns:doh') | ||
| log.error = debug('ipfs:ipns:doh:error') | ||
|
|
||
| async function dohBinary (url, domain, key) { |
`ipns name resolve` dns tests moved to interface-core resolve call now return a string as per documention
hugomrdias
force-pushed
the
feat/fast-ipns
branch
from
c01dc2c
to
1405e1f
Compare
|
@hugomrdias do you want to merge this and send the fixups as a seperate PR? I could get an RC released? |
|
@hugomrdias what's the status on this? |
alanshaw
changed the title
No description provided.