Skip to content
This repository has been archived by the owner on Jan 19, 2021. It is now read-only.

Support for proofs of null/absence. Dried up prove/verify. #82

Merged
merged 1 commit into from
Jun 22, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
38 changes: 33 additions & 5 deletions src/baseTrie.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,39 @@ module.exports = class Trie {
this.root = root
}

static fromProof (proofNodes, cb, proofTrie) {
let opStack = proofNodes.map((nodeValue) => {
return {type: 'put', key: ethUtil.keccak(nodeValue), value: ethUtil.toBuffer(nodeValue)}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

its kindof like the verification is done in this step because when you traverse the tree you already know each key is the hash of the node it's storing

})

if (!proofTrie) {
proofTrie = new Trie()
if (opStack[0]) { proofTrie.root = opStack[0].key }
}

proofTrie.db.batch(opStack, (e) => {
cb(e, proofTrie)
})
}

static prove (trie, key, cb) {
trie.findPath(key, function (err, node, remaining, stack) {
if (err) return cb(err)
let p = stack.map((stackElem) => { return stackElem.serialize() })
Copy link
Contributor

Choose a reason for hiding this comment

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

You're including embedded nodes (specifically those with length < 32) in the proof in comparison with the previous prove method. Is that necessary for the non-existence proofs?

cb(null, p)
})
}

static verifyProof (rootHash, key, proofNodes, cb) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't root of proofTrie be checked against rootHash here?

Copy link
Contributor Author

@zmitton zmitton May 3, 2019

Choose a reason for hiding this comment

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

Actually yeah that's true. It's hard to say who's job this is. But if it's going to take the argument rootHash it should verify it. somehting like this will do it

  static verifyProof (rootHash, key, proofNodes, cb) {
    let proofTrie = new Trie(null, rootHash)
    Trie.fromProof(proofNodes, (error, proofTrie) => {
      if (error) cb(new Error('Invalid proof nodes given'), null)
      proofTrie.get(key, (e, r) => {
        return cb(e, r)
      })
    }, proofTrie)

let proofTrie = new Trie(null, rootHash)
Trie.fromProof(proofNodes, (error, proofTrie) => {
if (error) cb(new Error('Invalid proof nodes given'), null)
proofTrie.get(key, (e, r) => {
return cb(e, r)
})
}, proofTrie)
}

/**
* Gets a value given a `key`
* @method get
Expand Down Expand Up @@ -135,16 +168,11 @@ module.exports = class Trie {
cb(null, new TrieNode(node))
} else {
this.db.get(node, (err, value) => {
if (err) {
throw err
Copy link
Contributor

Choose a reason for hiding this comment

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

db.get doesn't return an error if value is not found (just null for value). I wouldn't remove this if there's no special reason for this, as I can imagine in future db.get could potentially return errors for other reasons (e.g. validation of input).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

db.get doesn't return an error if value is not found
There is a special reason for it. The way in which this tree is traversed and the property that is is a merkle tree, the child node must always exist in a valid tree.

I can imagine in future db.get could potentially return errors for other reasons
Im still returning that error below. Im just not blowing up as before (throwing an error during async function was just unhelpful blow up)

}

if (value) {
value = new TrieNode(rlp.decode(value))
} else {
err = new Error('Missing node in DB')
}

cb(err, value)
})
}
Expand Down
9 changes: 0 additions & 9 deletions src/checkpointTrie.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
const async = require('async')
const WriteStream = require('level-ws')
const BaseTrie = require('./baseTrie')
const proof = require('./proof.js')
const ScratchReadStream = require('./scratchReadStream')
const ScratchDB = require('./scratch')
const { callTogether } = require('./util/async')
Expand All @@ -17,14 +16,6 @@ module.exports = class CheckpointTrie extends BaseTrie {
this._checkpoints = []
}

static prove (...args) {
return proof.prove(...args)
}

static verifyProof (...args) {
return proof.verifyProof(...args)
}

/**
* Is the trie during a checkpoint phase?
*/
Expand Down
100 changes: 0 additions & 100 deletions src/proof.js

This file was deleted.

3 changes: 1 addition & 2 deletions test/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,18 +6,17 @@ const ethUtil = require('ethereumjs-util')

tape('simple save and retrive', function (tester) {
var it = tester.test

it('should not crash if given a non-existant root', function (t) {
var root = new Buffer('3f4399b08efe68945c1cf90ffe85bbe3ce978959da753f9e649f034015b8817d', 'hex')
var trie = new Trie(null, root)

trie.get('test', function (err, value) {
t.equal(value, null)

t.notEqual(err, null)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

note The behavior here has changed slightly, but reading the objective of the test I think it's ok

t.end()
})
})

var trie = new Trie()

it('save a value', function (t) {
Expand Down
43 changes: 34 additions & 9 deletions test/proof.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,28 +39,53 @@ tape('simple merkle proofs generation and verification', function (tester) {
},
function (cb) {
Trie.prove(trie, 'key2bb', function (err, prove) {
t.equal(err, null, 'Path to key2 should create valid proof of absence')
if (err) return cb(err)
Trie.verifyProof(trie.root, 'randomkey', prove, function (err, val) {
t.notEqual(err, null, 'Expected error: ' + err.message)
Trie.verifyProof(trie.root, 'key2', prove, function (err, val) {
// In this case, the proof _happens_ to contain enough nodes to prove `key2` because
// traversing into `key22` would touch all the same nodes as traversing into `key2`
t.equal(val, null, 'Expected value at a random key to be null')
t.equal(err, null, 'Path to key2 should show a null value')
cb()
})
})
},
function (cb) {
Trie.prove(trie, 'key2bb', function (err, prove) {
let myKey = 'anyrandomkey'
Trie.prove(trie, myKey, function (err, prove) {
if (err) return cb(err)
Trie.verifyProof(trie.root, 'key2b', prove, function (err, val) {
t.notEqual(err, null, 'Expected error: ' + err.message)
Trie.verifyProof(trie.root, myKey, prove, function (err, val) {
t.equal(val, null, 'Expected value to be null')
t.equal(err, null, err)
cb()
})
})
},
function (cb) {
Trie.prove(trie, 'key2bb', function (err, prove) {
let myKey = 'anothergarbagekey' // should generate a valid proof of null
Trie.prove(trie, myKey, function (err, prove) {
if (err) return cb(err)
prove.push(Buffer.from('123456'))// extra nodes are just ignored
Trie.verifyProof(trie.root, myKey, prove, function (err, val) {
t.equal(val, null, 'Expected value to be null')
t.equal(err, null, err)
cb()
})
})
},
function (cb) {
trie.put('another', '3498h4riuhgwe', cb)
},
function (cb) {
// to throw an error we need to request proof for one key
Trie.prove(trie, 'another', function (err, prove) {
if (err) return cb(err)
prove.push(Buffer.from('123456'))
Trie.verifyProof(trie.root, 'key2b', prove, function (err, val) {
t.notEqual(err, null, 'Expected error: ' + err.message)
// and try to use that proof on another key
Trie.verifyProof(trie.root, 'key1aa', prove, function (err, val) {
t.equal(val, null, 'Expected value: to be null ')
// this proof would be insignificant to prove `key1aa`
t.notEqual(err, null, 'Expected error: Missing node in DB')
t.notEqual(err, undefined, 'Expected error: Missing node in DB')
cb()
})
})
Expand Down