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

Cannot create a node with an initialized repo #1245

Closed
realharry opened this issue Mar 4, 2018 · 8 comments
Closed

Cannot create a node with an initialized repo #1245

realharry opened this issue Mar 4, 2018 · 8 comments
Labels
exp/expert Having worked on the specific codebase is important help wanted Seeking public contribution on this issue kind/bug A bug in existing code (including security flaws) P1 High: Likely tackled by core team if no one steps up status/ready Ready to be worked

Comments

@realharry
Copy link

  • Version: 0.28.0
  • Platform: Ubuntu Desktop 17.10 (Linux 4.13.0-36-generic update roadmap #40-Ubuntu SMP Fri Feb 16 20:07:48 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux)
  • Subsystem: Node: 6.11.5

Type: Bug

Severity: Medium

Description:

Attempting to create an IPFS node using an initialized Repo object throws "Callback was already called." error.

Not sure if I'm using the API correctly (practically, it's my first day of using js-ipfs), but if I try to pass the repo argument with the initialized Repo object in the IPFS constructor, I get the following exception from Async:

/home/harry/ipfsjs/ipfs-repo/node_modules/async/internal/onlyOnce.js:9
        if (fn === null) throw new Error("Callback was already called.");
                     ^
Error: Callback was already called.
    at /home/harry/ipfsjs/ipfs-repo/node_modules/async/internal/onlyOnce.js:9:32
    at /home/harry/ipfsjs/ipfs-repo/node_modules/async/internal/parallel.js:36:13
    at /home/harry/ipfsjs/ipfs-repo/node_modules/write-file-atomic/index.js:142:7
    at FSReqWrap.oncomplete (fs.js:123:15)

Steps to reproduce the error:

Here' a sample code snippet:

const IPFS = require('ipfs');
const IPFSRepo = require('ipfs-repo');
const repo1 = new IPFSRepo('/tmp/ipfs-repo1');

repo1.init({}, (err) => {
    if (err) { console.log(err); return; }
    console.log('IPFS repo 1 initialized.');

    console.log('Creating IPFS node 1.');
    let node1 = new IPFS({
      repo: repo1,
      init: false,
      start: true,
      EXPERIMENTAL: {
      },
    });
    console.log('IPFS node 1 created.');
    node1.on('start', () => {
      console.log('IPFS node 1 started.');
    });
  })
@daviddias daviddias added kind/bug A bug in existing code (including security flaws) exp/expert Having worked on the specific codebase is important help wanted Seeking public contribution on this issue status/ready Ready to be worked P1 High: Likely tackled by core team if no one steps up P2 Medium: Good to have, but can wait until someone steps up and removed P2 Medium: Good to have, but can wait until someone steps up labels Mar 12, 2018
@JonKrone
Copy link
Contributor

Hey @realharry I spent a little time looking at this problem and was able narrow the problem but I'm not sure of expected behavior here so I wanted to share what I've found.

IPFS takes additional steps when initializing a new repo, adding a Keychain and Identity config property for local storage of a node's identity and private keys:

(exists, cb) => {
self.log('repo exists?', exists)
if (exists === true) {
return cb(new Error('repo already exists'))
}
// Generate peer identity keypair + transform to desired format + add to config.
opts.log(`generating ${opts.bits}-bit RSA keypair...`, false)
self.log('generating peer id: %s bits', opts.bits)
peerId.create({ bits: opts.bits }, cb)
},
(keys, cb) => {
self.log('identity generated')
config.Identity = {
PeerID: keys.toB58String(),
PrivKey: keys.privKey.bytes.toString('base64')
}
if (opts.pass) {
privateKey = keys.privKey
config.Keychain = Keychain.generateOptions()
}
opts.log('done')
opts.log('peer identity: ' + config.Identity.PeerID)
self._repo.init(config, cb)
},

Because of this, IPFS is happy to use a repo previously created by itself but a fresh repo created without these config options fails. I'm not sure if this is all that it does but I was able to get a node started with this code:

code spoiler
const IPFS = require('./')
const IPFSRepo = require('ipfs-repo')
const path = require('path')
const Keychain = require('libp2p-keychain')
const peerId = require('peer-id')

const repo1 = new IPFSRepo(path.join(process.cwd(), 'ipfs-repo1'))
const config = {}

peerId.create({ bits: 2048 }, (err, keys) => {
  config.Identity = {
    PeerID: keys.toB58String(),
    PrivKey: keys.privKey.bytes.toString('base64')
  }
  privateKey = keys.privKey
  config.Keychain = Keychain.generateOptions()

  repo1.init(config, (err) => {
    if (err) {
      console.log('error initializing repo:', err)
      return
    }
    console.log('IPFS repo 1 initialized.')

    console.log('Creating IPFS node 1.')
    let node1 = new IPFS({
      repo: repo1,
      init: false,
      start: true,
      EXPERIMENTAL: {
      },
    })

    console.log('IPFS node 1 created.')
    node1.on('start', () => {
      console.log('IPFS node 1 started.')
    })
  })
})

I'm not sure of the expected behavior here so I wanted others' thoughts. We could:

  • Check that these properties exist in a repo config and if not, create them. However, this might go against the init: false option passed to new IPFS().
  • Error on missing required config options
  • ..?

@dryajov
Copy link
Member

dryajov commented Mar 13, 2018

@realharry There is an issues with your code snippet, you're trying to initialize the repo with repo1.init which causes the failure. There is no reason to do that - in fact, you don't even need to pass a repo object, you can very well use a string as the repo.

I agree that the error message is not informative tho - it should say that the repo already exists.

Here is a code snippet that uses a repo object:

const IPFS = require('ipfs');
const IPFSRepo = require('ipfs-repo');

const repo1 = new IPFSRepo('<path to repo>');
console.log('Creating IPFS node 1.');
let node1 = new IPFS({
  repo: repo1,
  init: false,
  start: true,
  EXPERIMENTAL: {}
});
node1.on('start', () => {
  console.log('IPFS node 1 started.');
});

Here is a snippet that takes a path to the repo without explicitly creating a repo object:

const IPFS = require('ipfs');

console.log('Creating IPFS node 1.');
let node1 = new IPFS({
  repo: '<path to repo>', 
  init: false,
  start: true,
  EXPERIMENTAL: {}
});
node1.on('start', () => {
  console.log('IPFS node 1 started.');
});

@JonKrone
Copy link
Contributor

@dryajov's response is the correct way to reuse a repo as long as it was previously created by ipfs, which is probably all of the time. Is there any need to look further into what I had brought up or is that an unsupported use?

@dryajov
Copy link
Member

dryajov commented Mar 13, 2018

@JonKrone there is no need to follow this steps as IPFS will perform them internally when initializing the repo. What we need to do is to handle this more intelligently in ipfs-repo.

When IPFS boots, it calls repo.open, when checks to see if the repo is already initialized, but calling repo.init directly doesn't do that - perhaps we should move that logic into ipfs-repo itself and make sure that repo.init checks if the repo exists first so that we don't get the nasty error.

@Mr0grog
Copy link
Contributor

Mr0grog commented Mar 13, 2018

Is there a way to provide clearer error messaging in this case? e.g. This repo does not have a key pair instead of failing sometime later with the relatively unhelpful Error: Callback was already called.?

It would probably be helpful to note that a repo should generally be created with new IPFS() rather than by using new IPFSRepo() directly in the “advanced options” description: https://github.com/ipfs/js-ipfs#advanced-options-when-creating-an-ipfs-node

const node = new IPFS({
  repo: repo,
  init: true, // default
  // init: false, // If you have already initialized the repo with `new IPFS()`.
  //              // The IPFS constructor initializes repos with extra options;
  //              // simply calling `init()` on an IPFSRepo instance is not enough.
  // init: {
  //   bits: 1024 // size of the RSA key generated
  // },
  start: true, // default
  ...

@Mr0grog
Copy link
Contributor

Mr0grog commented Mar 13, 2018

Actually, that error message could be even more actionable: This repo does not have a key pair (you should initialize it with new IPFS())

Mr0grog added a commit to Mr0grog/js-ipfs that referenced this issue Mar 13, 2018
Add some description to the "advanced options" documentation for the `IPFS` constructor detailing that `init: false` should really be used if a repo has already been initialized *by IPFS* and that simply calling `repoInstance.init()` doesn't init a repo with everything IPFS needs.

This partially handles ipfs#1245.
Mr0grog added a commit to Mr0grog/js-ipfs that referenced this issue Mar 13, 2018
Add some description to the "advanced options" documentation for the `IPFS` constructor detailing that `init: false` should really be used if a repo has already been initialized *by IPFS* and that simply calling `repoInstance.init()` doesn't init a repo with everything IPFS needs.

This partially handles ipfs#1245.

License: MIT
Signed-off-by: Rob Brackett <work@robbrackett.com>
@vaultec81
Copy link

Is there is any updates on this issue?

@achingbrain
Copy link
Member

Starting a node with an already initialised and open repo works for me like so:

const IPFS = require('ipfs')
const IPFSRepo = require('ipfs-repo')
const PeerId = require('peer-id')
const os = require('os')
const path = require('path')

async function main () {
  const peerId = await PeerId.create({ bits: 2048 })
  const repo = new IPFSRepo(path.join(os.tmpdir(), 'ipfs-' + Math.random()))
  await repo.init({
    // nb. you may need more config to make your node useful
    // see: https://github.com/ipfs/js-ipfs/blob/master/packages/ipfs/src/core/runtime/config-nodejs.js
    // or https://github.com/ipfs/js-ipfs/blob/master/packages/ipfs/src/core/runtime/config-browser.js
    // depending on environment
    /*
      Addresses: {
        ...
      }, ... etc
    */
    Identity: {
      PeerID: peerId.toB58String(),
      PrivKey: peerId.privKey.bytes.toString('base64')
    }
  })
  await repo.open()

  const node = await IPFS.create({
    repo
  })

  console.info('node started')

  await node.stop()

  console.info('node stopped')
}

main()

This issue is very old and the codepaths that trigger the error reported no longer exist. If the above script does not work for you please open a new issue with more recent error stack traces, etc.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
exp/expert Having worked on the specific codebase is important help wanted Seeking public contribution on this issue kind/bug A bug in existing code (including security flaws) P1 High: Likely tackled by core team if no one steps up status/ready Ready to be worked
Projects
None yet
Development

No branches or pull requests

7 participants