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

Add ability to persist and load nodes #21

Closed
make-github-pseudonymous-again opened this issue May 1, 2020 · 13 comments · Fixed by #22
Closed

Add ability to persist and load nodes #21

make-github-pseudonymous-again opened this issue May 1, 2020 · 13 comments · Fixed by #22

Comments

@make-github-pseudonymous-again
Copy link
Contributor

Continuing holepunchto/hyperdht#4.

@make-github-pseudonymous-again
Copy link
Contributor Author

Something like the following:

class DHT extends EventEmitter {

  ...

  toJSON () {
    return {
      nodes: this.nodes.toArray(),
    }
  }

  addNodes (nodes) {
    for (const node of nodes) {
      const { id , token , to } = node
      const peer = {
        host: node.host,
        port: node.port,
      }
      this._addNode(id, peer, token, to)
    }
  }

}

@mafintosh
Copy link
Contributor

Yep! I wouldn't call it toJSON though as the ids are binary. Just toArray() or toNodesArray() is fine

@make-github-pseudonymous-again
Copy link
Contributor Author

What about the next and prev properties leaking from the tos should we throw them away?
Same for tick.
We could still have a convenience function to output some JSON. Stringify the ids?

@mafintosh
Copy link
Contributor

Good idea stripping the private info. A map of that array to { id, port, ip, referrer: { ip, port, id } } should be enough.

I'm soft -1 on stringify'ing the ids. I think that is the responsibility of whoever is serialising. Alternatively addNodes should deserialise them from hex if they are strings.

@make-github-pseudonymous-again
Copy link
Contributor Author

What should be in the referrer ?

class DHT extends EventEmitter {

  ...

  getNodes () {
    return this.nodes.toArray().map(
      ({id, host, port, roundtripToken, to}) => ({
        id,
        host,
        port,
        referrer: { ? }
      })
    )
  }

  addNodes (nodes) {
    for (const node of nodes) {
      const { id , token , to } = node
      const peer = { host , port } = node
      this._addNode(id, peer, token, to)
    }
  }


}

@mafintosh
Copy link
Contributor

@aureooms node.referrer is set on the node. not all nodes have this

@make-github-pseudonymous-again
Copy link
Contributor Author

@aureooms node.referrer is set on the node. not all nodes have this

@mafintosh I see.

@make-github-pseudonymous-again
Copy link
Contributor Author

What should we do with referrer in addNodes? Should we even care about token and to being given?

class DHT extends EventEmitter {

  ...

  getNodes () {
    return this.nodes.toArray().map(
      ({id, host, port, referrer }) => ({
        id,
        host,
        port,
        referrer: referrer === null ? null : {
          id: referrer.id,
          host: referrer.host,
          port: referrer.port,
        }
      })
    )
  }

  addNodes (nodes) {
    for (const node of nodes) {
      const { id , token , to } = node
      const peer = { host , port } = node
      this._addNode(id, peer, token, to)
    }
  }

}

@mafintosh
Copy link
Contributor

We don't care about token/to.

Actually we don't persist the referrer in the nodes table, so disregard that for now.

@make-github-pseudonymous-again
Copy link
Contributor Author

Final iteration?

class DHT extends EventEmitter {

  ...

  getNodes () {
    return this.nodes.toArray().map(
      ({id, host, port, referrer }) => ({
        id,
        host,
        port,
        referrer: referrer === null ? null : {
          id: referrer.id,
          host: referrer.host,
          port: referrer.port,
        }
      })
    )
  }

  addNodes (nodes) {
    for (const { id , host , port } of nodes) {
      this._addNode(id, { host , port })
    }
  }

}

@mafintosh
Copy link
Contributor

just skip the referrer on export for now, other than that looks good to me. can you open a PR?

@make-github-pseudonymous-again
Copy link
Contributor Author

Yes!

@make-github-pseudonymous-again make-github-pseudonymous-again changed the title Add ability to persist and load nodes to/from JSON Add ability to persist and load nodes May 1, 2020
@make-github-pseudonymous-again
Copy link
Contributor Author

PR at #22. I added some tests. Could not get rid of the setTimeout for some reason. Had to trigger some command on a for it to add b as a node.

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 a pull request may close this issue.

2 participants