trie: Better error checking for invalid proofs

packages/trie/CHANGELOG.md

@@ -12,6 +12,8 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/)
 
 **Bug Fixes**
 
+- Better error checking for invalid proofs, PR [#1373](https://github.com/ethereumjs/ethereumjs-monorepo/pull/1373)
+
 **Maintenance**
 
 - Remove use of deprecated setRoot, PR [#1376](https://github.com/ethereumjs/ethereumjs-monorepo/pull/1376)

packages/trie/README.md

@@ -20,7 +20,7 @@ The only backing store supported is LevelDB through the `levelup` module.
 
 There are 3 variants of the tree implemented in this library, namely: `BaseTrie`, `CheckpointTrie` and `SecureTrie`. `CheckpointTrie` adds checkpointing functionality to the `BaseTrie` with the methods `checkpoint`, `commit` and `revert`. `SecureTrie` extends `CheckpointTrie` and is the most suitable variant for Ethereum applications. It stores values under the `keccak256` hash of their keys.
 
-By default trie nodes are not deleted from the underlying DB to not corrupt older trie states (as of `v4.2.0`). If you are only interested in the latest state of a trie you can switch to a delete behavior (e.g. if you want to save disk space) by using the `deleteFromDB` constructor option (see related release notes in the changelog for more details).
+By default, trie nodes are not deleted from the underlying DB to not corrupt older trie states (as of `v4.2.0`). If you are only interested in the latest state of a trie, you can switch to a delete behavior (e.g. if you want to save disk space) by using the `deleteFromDB` constructor option (see related release notes in the changelog for more details).
 
 ## Initialization and Basic Usage
 
@@ -42,6 +42,12 @@ test()
 
 ## Merkle Proofs
 
+The `createProof` and `verifyProof` functions allow you to verify that a certain value does or does not exist within a Merkle-Patricia trie with a given root.
+
+### Proof of existence
+
+The below code demonstrates how to construct and then verify a proof that proves that the key `test` that corresponds to the value `one` does exist in the given trie, so a proof of existence.
+
 ```typescript
 const trie = new Trie()
 
@@ -55,6 +61,47 @@ async function test() {
 test()
 ```
 
+### Proof of non-existence
+
+The below code demonstrates how to construct and then verify a proof that proves that the key `test3` does not exist in the given trie, so a proof of non-existence.
+
+```typescript
+const trie = new Trie()
+
+async function test() {
+  await trie.put(Buffer.from('test'), Buffer.from('one'))
+  await trie.put(Buffer.from('test2'), Buffer.from('two'))
+  const proof = await Trie.createProof(trie, Buffer.from('test3'))
+  const value = await Trie.verifyProof(trie.root, Buffer.from('test3'), proof)
+  console.log(value.toString()) // null
+}
+
+test()
+```
+
+### Invalid proofs
+
+Note, if `verifyProof` detects an invalid proof, it throws an error.  While contrived, the below example demonstrates the error condition that would result if a prover tampers with the data in a merkle proof.
+
+```typescript
+const trie = new Trie()
+
+async function test() {
+  await trie.put(Buffer.from('test'), Buffer.from('one'))
+  await trie.put(Buffer.from('test2'), Buffer.from('two'))
+  const proof = await Trie.createProof(trie, Buffer.from('test2'))
+  proof[1].reverse()
+  try {
+    const value = await Trie.verifyProof(trie.root, Buffer.from('test2'), proof)
+    console.log(value.toString()) // results in error
+  } catch (err) {
+    console.log(err) // Missing node in DB
+  }
+}
+
+test()
+```
+
 ## Read stream on Geth DB
 
 ```typescript

packages/trie/package.json

@@ -29,6 +29,7 @@
     "lint": "../../config/cli/lint.sh",
     "lint:fix": "../../config/cli/lint-fix.sh",
     "tsc": "../../config/cli/ts-compile.sh",
+    "tape": "tape -r ts-node/register",
     "test": "npm run test:node && npm run test:browser",
     "test:browser": "karma start karma.conf.js",
     "test:node": "tape -r ts-node/register test/*.ts"

packages/trie/src/baseTrie.ts

@@ -102,8 +102,16 @@ export class Trie {
    * Checks if a given root exists.
    */
   async checkRoot(root: Buffer): Promise<boolean> {
-    const value = await this._lookupNode(root)
-    return !!value
+    try {
+      const value = await this._lookupNode(root)
+      return value !== null
+    } catch (error) {
+      if (error.message == 'Missing node in DB') {
+        return false
+      } else {
+        throw error
+      }
+    }
   }
 
   /**
@@ -116,10 +124,11 @@ export class Trie {
   /**
    * Gets a value given a `key`
    * @param key - the key to search for
+   * @param throwIfMissing - if true, throws if any nodes are missing. Used for verifying proofs. (default: false)
    * @returns A Promise that resolves to `Buffer` if a value was found or `null` if no value was found.
    */
-  async get(key: Buffer): Promise<Buffer | null> {
-    const { node, remaining } = await this.findPath(key)
+  async get(key: Buffer, throwIfMissing = false): Promise<Buffer | null> {
+    const { node, remaining } = await this.findPath(key, throwIfMissing)
     let value = null
     if (node && remaining.length === 0) {
       value = node.value
@@ -172,16 +181,17 @@ export class Trie {
    * Tries to find a path to the node for the given key.
    * It returns a `stack` of nodes to the closest node.
    * @param key - the search key
+   * @param throwIfMissing - if true, throws if any nodes are missing. Used for verifying proofs. (default: false)
    */
-  async findPath(key: Buffer): Promise<Path> {
+  async findPath(key: Buffer, throwIfMissing = false): Promise<Path> {
     // eslint-disable-next-line no-async-promise-executor
-    return new Promise(async (resolve) => {
+    return new Promise(async (resolve, reject) => {
       const stack: TrieNode[] = []
       const targetKey = bufferToNibbles(key)
 
       const onFound: FoundNodeFunction = async (nodeRef, node, keyProgress, walkController) => {
         if (node === null) {
-          return
+          return reject(new Error('Path not found'))
         }
         const keyRemainder = targetKey.slice(matchingNibbleLength(keyProgress, targetKey))
         stack.push(node)
@@ -223,7 +233,15 @@ export class Trie {
       }
 
       // walk trie and process nodes
-      await this.walkTrie(this.root, onFound)
+      try {
+        await this.walkTrie(this.root, onFound)
+      } catch (error) {
+        if (error.message == 'Missing node in DB' && !throwIfMissing) {
+          // pass
+        } else {
+          reject(error)
+        }
+      }
 
       // Resolve if _walkTrie finishes without finding any nodes
       resolve({ node: null, remaining: [], stack })
@@ -270,9 +288,11 @@ export class Trie {
     let value = null
     let foundNode = null
     value = await this.db.get(node as Buffer)
-
     if (value) {
       foundNode = decodeNode(value)
+    } else {
+      // Dev note: this error message text is used for error checking in `checkRoot`, `verifyProof`, and `findPath`
+      throw new Error('Missing node in DB')
     }
     return foundNode
   }
@@ -678,7 +698,7 @@ export class Trie {
    * @param key
    * @param proof
    * @throws If proof is found to be invalid.
-   * @returns The value from the key.
+   * @returns The value from the key, or null if valid proof of non-existence.
    */
   static async verifyProof(rootHash: Buffer, key: Buffer, proof: Proof): Promise<Buffer | null> {
     let proofTrie = new Trie(null, rootHash)
@@ -687,7 +707,16 @@ export class Trie {
     } catch (e) {
       throw new Error('Invalid proof nodes given')
     }
-    return proofTrie.get(key)
+    try {
+      const value = await proofTrie.get(key, true)
+      return value
+    } catch (err) {
+      if (err.message == 'Missing node in DB') {
+        throw new Error('Invalid proof provided')
+      } else {
+        throw err
+      }
+    }
   }
 
   /**

packages/trie/src/util/walkController.ts

@@ -11,6 +11,7 @@ export class WalkController {
   readonly taskExecutor: PrioritizedTaskExecutor
   readonly trie: BaseTrie
   private resolve: Function
+  private reject: Function
 
   /**
    * Creates a new WalkController
@@ -23,6 +24,7 @@ export class WalkController {
     this.taskExecutor = new PrioritizedTaskExecutor(poolSize)
     this.trie = trie
     this.resolve = () => {}
+    this.reject = () => {}
   }
 
   /**
@@ -44,9 +46,15 @@ export class WalkController {
 
   private async startWalk(root: Buffer): Promise<void> {
     // eslint-disable-next-line no-async-promise-executor
-    return await new Promise(async (resolve) => {
+    return await new Promise(async (resolve, reject) => {
       this.resolve = resolve
-      const node = await this.trie._lookupNode(root)
+      this.reject = reject
+      let node
+      try {
+        node = await this.trie._lookupNode(root)
+      } catch (error) {
+        return this.reject(error)
+      }
       this.processNode(root, node, [])
     })
   }
@@ -88,7 +96,12 @@ export class WalkController {
     this.taskExecutor.executeOrQueue(
       priority ?? key.length,
       async (taskFinishedCallback: Function) => {
-        const childNode = await this.trie._lookupNode(nodeRef)
+        let childNode
+        try {
+          childNode = await this.trie._lookupNode(nodeRef)
+        } catch (error) {
+          return this.reject(error)
+        }
         taskFinishedCallback() // this marks the current task as finished. If there are any tasks left in the queue, this will immediately execute the first task.
         this.processNode(nodeRef as Buffer, childNode as TrieNode, key)
       }

packages/trie/test/proof.spec.ts

@@ -40,9 +40,39 @@ tape('simple merkle proofs generation and verification', function (tester) {
 
     // to fail our proof we can request a proof for one key
     proof = await CheckpointTrie.createProof(trie, Buffer.from('another'))
-    // and use that proof on another key
-    const result = await CheckpointTrie.verifyProof(trie.root, Buffer.from('key1aa'), proof)
-    t.equal(result, null)
+    // and try to use that proof on another key
+    try {
+      await CheckpointTrie.verifyProof(trie.root, Buffer.from('key1aa'), proof)
+      t.fail('expected error: Invalid proof provided')
+    } catch (e) {
+      t.equal(e.message, 'Invalid proof provided')
+    }
+
+    // we can also corrupt a valid proof
+    proof = await CheckpointTrie.createProof(trie, Buffer.from('key2bb'))
+    proof[0].reverse()
+    try {
+      await CheckpointTrie.verifyProof(trie.root, Buffer.from('key2bb'), proof)
+      t.fail('expected error: Invalid proof provided')
+    } catch (e) {
+      t.equal(e.message, 'Invalid proof provided')
+    }
+
+    // test an invalid exclusion proof by creating
+    // a valid exclusion proof then making it non-null
+    myKey = Buffer.from('anyrandomkey')
+    proof = await CheckpointTrie.createProof(trie, myKey)
+    val = await CheckpointTrie.verifyProof(trie.root, myKey, proof)
+    t.equal(val, null, 'Expected value to be null')
+    // now make the key non-null so the exclusion proof becomes invalid
+    await trie.put(myKey, Buffer.from('thisisavalue'))
+    try {
+      await CheckpointTrie.verifyProof(trie.root, myKey, proof)
+      t.fail('expected error: Invalid proof provided')
+    } catch (e) {
+      t.equal(e.message, 'Invalid proof provided')
+    }
+
     t.end()
   })