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

Node http endpoints and handlers #168

Open
wants to merge 29 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 28 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
ed1b563
Implement sendopen http interface for node. Requires updated hs-clien…
wi-ski May 4, 2019
8a9fe95
Implement grindname, getproof, and getresource
wi-ski May 5, 2019
3fcd483
Use paired hs-client
wi-ski May 5, 2019
ad782fe
Undo need for update hs-client
wi-ski May 14, 2019
1d20d2a
Remaining getInfo calls
wi-ski May 14, 2019
b9b9396
this correction
wi-ski May 14, 2019
0f98759
Update test/node-http-test.js
wi-ski May 14, 2019
5db3f58
Update test/node-http-test.js
wi-ski May 14, 2019
14a5c31
Surface level test tweaks, next try sleep again
wi-ski May 15, 2019
dc5e518
Test implementation using sleep attempt
wi-ski May 15, 2019
0722801
Kill before each
wi-ski May 15, 2019
ffb2fc1
Extend timeout?
wi-ski May 15, 2019
0fdce37
Extend timeout again.
wi-ski May 15, 2019
6baaa6d
Event listening
wi-ski May 15, 2019
b035103
First pass at mining fewer blocks
wi-ski May 15, 2019
721f8fd
Remove sleep
wi-ski May 15, 2019
593a151
Mine even fewer blocks
wi-ski May 15, 2019
5fd20d5
Bring test timeout back down to reflect time seen in CI
wi-ski May 15, 2019
bccab33
PR tweaks for Tynes. Use getJSON util method, variable renaming
wi-ski Jun 7, 2019
32d8557
Try CI again
wi-ski Jun 8, 2019
c0bec63
Fix missing ref
wi-ski Jun 27, 2019
9050228
Fix for var shadowing
wi-ski Jun 27, 2019
3b8a8a4
Update routes, use more determnistic mining setup, implement and test…
wi-ski Jun 30, 2019
4dbf7f7
Update test descriptions for TYPE_DEADEND
wi-ski Jun 30, 2019
41f5088
Tighten test line spacing up
wi-ski Jun 30, 2019
d4015a5
Lint fix
wi-ski Jul 1, 2019
4c6afb4
Return null when DB doesnt know about name
wi-ski Jul 1, 2019
231e30e
Restore empty line
wi-ski Jul 1, 2019
177a78e
Dedup proof response vals, shorten line length
wi-ski Jul 24, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
129 changes: 129 additions & 0 deletions lib/node/http.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ const Claim = require('../primitives/claim');
const Address = require('../primitives/address');
const Network = require('../protocol/network');
const pkg = require('../pkg');
const rules = require('../covenants/rules');
const Resource = require('../dns/resource');

/**
* HTTP
Expand Down Expand Up @@ -432,6 +434,133 @@ class HTTP extends Server {

res.json(200, { success: true });
});

Copy link
Contributor

Choose a reason for hiding this comment

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

Each of these endpoints could use a comment describing their functionality

this.get('/name/:name', async (req, res) => {
const valid = Validator.fromRequest(req);
const name = valid.str('name');

if (!name || !rules.verifyName(name))
throw new Error('Invalid parameter.');
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: 'Invalid name.'


const network = this.network;
const height = this.chain.height;
const nameHash = rules.hashName(name);
const reserved = rules.isReserved(nameHash, height + 1, network);
const [start, week] = rules.getRollout(nameHash, network);
const ns = await this.chain.db.getNameState(nameHash);

let info = null;

if (ns) {
if (!ns.isExpired(height, network))
info = ns.getJSON(height, network);
}

return res.json(200, {
start: {
reserved: reserved,
week: week,
start: start
},
info
});
});

this.get('/resource/hash/:hash', async (req, res) => {
const valid = Validator.fromRequest(req);
const hash = valid.bhash('hash');

if (!hash)
throw new Error('Invalid hash.');

const ns = await this.chain.db.getNameState(hash);
const height = this.chain.tip.height;
const network = this.network;

if (!ns)
Copy link
Contributor

Choose a reason for hiding this comment

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

Also need to check if ns.data.length === 0

return res.json(404);

return res.json(200, ns.getJSON(height,network));
});

this.get('/resource/name/:name', async (req, res) => {
const valid = Validator.fromRequest(req);
const name = valid.str('name');

if (!name || !rules.verifyName(name))
throw new Error('Invalid name.');

const nameHash = rules.hashName(name);
const ns = await this.chain.db.getNameState(nameHash);

Copy link
Contributor

Choose a reason for hiding this comment

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

We don't want to return null here, we want to return a res.json(404)

if (!ns || ns.data.length === 0)
return null;

const resource = Resource.decode(ns.data);

return res.json(200, resource.getJSON(name));
});

this.get('/proof/name/:name', async (req, res) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Now that I think about it, there are different kinds of proofs. There is an Urkle Tree Proof, which authenticates the namestate and then there is a Merkle Proof for transactions in a block. Ideally we want to be able to support both of them and if we use /proof/name/:name then we can use /proof/tx/:txid, but I think that would only work if transaction indexing is turned on. So using /proof/name/:name works.

const valid = Validator.fromRequest(req);
const name = valid.str('name');

if (!name || !rules.verifyName(name))
throw new Error('Invalid name.');

const tip = this.chain.tip.hash;
const height = this.chain.tip.height;
const root = this.chain.tip.treeRoot;
const nameHash = rules.hashName(name);
const proof = await this.chain.db.prove(root, nameHash);

return res.json(200, {
hash: tip.toString('hex'),
height: height,
root: root.toString('hex'),
name: name,
key: nameHash.toString('hex'),
proof: proof.toJSON()
Copy link
Contributor

Choose a reason for hiding this comment

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

The shape of proof.toJSON looks like:

  toJSON() {
    return {
      type: typesByVal[this.type],
      depth: this.depth,
      nodes: this.nodes.map(node => node.toJSON()),
      prefix: this.prefix ? this.prefix.toString() : undefined,
      left: this.left ? this.left.toString('hex') : undefined,
      right: this.right ? this.right.toString('hex') : undefined,
      key: this.key ? this.key.toString('hex') : undefined,
      hash: this.hash ? this.hash.toString('hex') : undefined,
      value: this.value ? this.value.toString('hex') : undefined
    };
  }

We should try to deduplicate any fields that correspond between the two. It feels a little bit cleaner to me to follow the style in other endpoints where the only object returned is a primitive.toJSON.

We want to be 100% sure that we can easily verify the proof client side.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

🤔 I dont entirely follow.

This shape mirrors the response of the RPC method:
https://github.com/handshake-org/hsd/blob/master/lib/node/rpc.js#L2375

Would you prefer something like:

    return {
      hash: hash.toString('hex'),
      height: height,
      root: root.toString('hex'),
      name: name,
      key: key.toString('hex'),
      ...proof.toJSON() // <- This would achieve your de-duping.
    };

It is definitely cleaner to return something.toJSON() - but to confirm, we have other handlers in this file that construct their response "in the handler" like: https://github.com/handshake-org/hsd/blob/master/lib/node/http.js#L354

Whichever we decide - Illl do the same for: https://github.com/handshake-org/hsd/pull/168/files/231e30eef2c442d4914c57cddd840a03624abac9#diff-bbe506ab5582252a30d6872d35fd8ce6R540

Copy link
Contributor Author

@wi-ski wi-ski Jul 3, 2019

Choose a reason for hiding this comment

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

Current question(s) I believe Im trying to answer:
a. Do we want to return the shape of proof.toJSON() with height & root & name included?
or
b. Something else that results in something akin to:

      return res.json(200, primitive.toJSON()); // Like proof.toJSON?

Copy link
Contributor

Choose a reason for hiding this comment

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

but to confirm, we have other handlers in this file that construct their response "in the handler"

This one was done that way because there was no filter.JSON and the filter object lives in a different repo and its hard to coordinate PRs in multiple repos

Copy link
Contributor

Choose a reason for hiding this comment

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

Between a and b, I think something in between is good, it would be nice to have the height and the root along with the rest of proof.toJSON. What we really need to be 100% sure of is that we have everything that we need client side to verify the proof, which I believe proof.toJSON gives.

Copy link
Contributor Author

@wi-ski wi-ski Jul 24, 2019

Choose a reason for hiding this comment

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

I added the:

    return {
      hash: hash.toString('hex'),
      height: height,
      root: root.toString('hex'),
      name: name,
      key: key.toString('hex'),
      ...proof.toJSON()
    };

☝️ This makes sense to me? Let me know if you feel otherwise!

});
});

this.get('/proof/hash/:nameHash', async (req, res) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: nameHash -> hash

const valid = Validator.fromRequest(req);
const nameHash = valid.bhash('nameHash');

if (!nameHash)
throw new Error('Invalid hash.');

const tip = this.chain.tip.hash;
const height = this.chain.tip.height;
const root = this.chain.tip.treeRoot;
const proof = await this.chain.db.prove(root, nameHash);
const ns = await this.chain.db.getNameState(nameHash);

return res.json(200, {
hash: tip.toString('hex'),
height: height,
root: root.toString('hex'),
key: nameHash.toString('hex'),
proof: proof.toJSON(),
name: ns ? ns.name.toString() : null
});
});

this.get('/grind', async (req, res) => {
const valid = Validator.fromRequest(req);
const size = valid.u32('size');

if (size < 1 || size > 63)
throw new Error('Invalid length.');

const network = this.network;
const height = this.chain.height;

const name = await rules.grindName(size, height + 1, network);

res.json(200, { name });
});
}

/**
Expand Down
Loading