trie: convert constructor to options interface, remove deprecated setRoot (#1874)

* trie: add constructor options interface

* remove assert use, remove deprecated setRoot

Author: ryanio

packages/client/lib/execution/vmexecution.ts

@@ -36,7 +36,7 @@ export class VMExecution extends Execution {
     super(options)
 
     if (!this.config.vm) {
-      const trie = new Trie(this.stateDB)
+      const trie = new Trie({ db: this.stateDB })
 
       const stateManager = new DefaultStateManager({
         common: this.config.execCommon,

packages/client/lib/util/debug.ts

@@ -39,7 +39,7 @@ const main = async () => {
     .toString('hex')}', 'hex'), { common })
 
   const stateDB = level('${execution.config.getDataDirectory(DataDirectory.State)}')
-  const trie = new Trie(stateDB)
+  const trie = new Trie({ db: stateDB })
   const stateManager = new DefaultStateManager({ trie, common })
   // Ensure we run on the right root
   stateManager.setStateRoot(Buffer.from('${(

packages/trie/README.md

@@ -29,7 +29,7 @@ import level from 'level'
 import { BaseTrie as Trie } from 'merkle-patricia-tree'
 
 const db = level('./testdb')
-const trie = new Trie(db)
+const trie = new Trie({ db })
 
 async function test() {
   await trie.put(Buffer.from('test'), Buffer.from('one'))
@@ -120,7 +120,7 @@ const stateRoot = '0xd7f8974fb5ac78d9ac099b9ad5018bedc2ce0a72dad1827a1709da30580
 // Convert the state root to a Buffer (strip the 0x prefix)
 const stateRootBuffer = Buffer.from(stateRoot.slice(2), 'hex')
 // Initialize trie
-const trie = new Trie(db, stateRootBuffer)
+const trie = new Trie({ db, root: stateRootBuffer })
 
 trie
   .createReadStream()
@@ -140,7 +140,7 @@ import { SecureTrie as Trie } from 'merkle-patricia-tree'
 const stateRoot = 'STATE_ROOT_OF_A_BLOCK'
 
 const db = level('YOUR_PATH_TO_THE_GETH_CHAINDATA_FOLDER')
-const trie = new Trie(db, stateRoot)
+const trie = new Trie({ db, root: stateRoot })
 
 const address = 'AN_ETHEREUM_ACCOUNT_ADDRESS'
 

packages/trie/src/baseTrie.ts

@@ -18,7 +18,6 @@ import {
 import { verifyRangeProof } from './verifyRangeProof'
 // eslint-disable-next-line implicit-dependencies/no-implicit
 import type { LevelUp } from 'levelup'
-const assert = require('assert')
 
 export type Proof = Buffer[]
 
@@ -35,6 +34,24 @@ export type FoundNodeFunction = (
   walkController: WalkController
 ) => void
 
+export interface TrieOpts {
+  /**
+   * A [levelup](https://github.com/Level/levelup) instance.
+   * By default (if the db is `null` or left undefined) creates an
+   * in-memory [memdown](https://github.com/Level/memdown) instance.
+   */
+  db?: LevelUp | null
+  /**
+   * A `Buffer` for the root of a previously stored trie
+   */
+  root?: Buffer
+  /**
+   * Delete nodes from DB on delete operations (disallows switching to an older state root)
+   * Default: `false`
+   */
+  deleteFromDB?: boolean
+}
+
 /**
  * The basic trie interface, use with `import { BaseTrie as Trie } from 'merkle-patricia-tree'`.
  * In Ethereum applications stick with the {@link SecureTrie} overlay.
@@ -51,22 +68,19 @@ export class Trie {
   private _deleteFromDB: boolean
 
   /**
-   * test
-   * @param db - A [levelup](https://github.com/Level/levelup) instance. By default (if the db is `null` or
-   * left undefined) creates an in-memory [memdown](https://github.com/Level/memdown) instance.
-   * @param root - A `Buffer` for the root of a previously stored trie
-   * @param deleteFromDB - Delete nodes from DB on delete operations (disallows switching to an older state root) (default: `false`)
+   * Create a new trie
+   * @param opts Options for instantiating the trie
    */
-  constructor(db?: LevelUp | null, root?: Buffer, deleteFromDB: boolean = false) {
+  constructor(opts: TrieOpts = {}) {
     this.EMPTY_TRIE_ROOT = KECCAK256_RLP
     this.lock = new Semaphore(1)
 
-    this.db = db ? new DB(db) : new DB()
+    this.db = opts.db ? new DB(opts.db) : new DB()
     this._root = this.EMPTY_TRIE_ROOT
-    this._deleteFromDB = deleteFromDB
+    this._deleteFromDB = opts.deleteFromDB ?? false
 
-    if (root) {
-      this.root = root
+    if (opts.root) {
+      this.root = opts.root
     }
   }
 
@@ -77,7 +91,7 @@ export class Trie {
     if (!value) {
       value = this.EMPTY_TRIE_ROOT
     }
-    assert(value.length === 32, 'Invalid root length. Roots are 32 bytes')
+    if (value.length !== 32) throw new Error('Invalid root length. Roots are 32 bytes')
     this._root = value
   }
 
@@ -88,17 +102,6 @@ export class Trie {
     return this._root
   }
 
-  /**
-   * This method is deprecated.
-   * Please use {@link Trie.root} instead.
-   *
-   * @param value
-   * @deprecated
-   */
-  setRoot(value?: Buffer) {
-    this.root = value ?? this.EMPTY_TRIE_ROOT
-  }
-
   /**
    * Checks if a given root exists.
    */
@@ -458,7 +461,7 @@ export class Trie {
     }
 
     let lastNode = stack.pop() as TrieNode
-    assert(lastNode)
+    if (!lastNode) throw new Error('missing last node')
     let parentNode = stack.pop()
     const opStack: BatchDBOp[] = []
 
@@ -683,7 +686,7 @@ export class Trie {
    * @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)
+    let proofTrie = new Trie({ root: rootHash })
     try {
       proofTrie = await Trie.fromProof(proof, proofTrie)
     } catch (e: any) {
@@ -735,7 +738,7 @@ export class Trie {
    */
   copy(): Trie {
     const db = this.db.copy()
-    return new Trie(db._leveldb, this.root)
+    return new Trie({ db: db._leveldb, root: this.root, deleteFromDB: this._deleteFromDB })
   }
 
   /**

packages/trie/src/checkpointDb.ts

@@ -21,7 +21,7 @@ export class CheckpointDB extends DB {
    * defaults to an [in-memory store](https://github.com/Level/memdown).
    * @param leveldb - An abstract-leveldown compliant store
    */
-  constructor(leveldb?: LevelUp) {
+  constructor(leveldb?: LevelUp | null) {
     super(leveldb)
     // Roots of trie at the moment of checkpoint
     this.checkpoints = []

packages/trie/src/checkpointTrie.ts

@@ -1,4 +1,4 @@
-import { Trie as BaseTrie } from './baseTrie'
+import { Trie as BaseTrie, TrieOpts } from './baseTrie'
 import { CheckpointDB } from './checkpointDb'
 
 /**
@@ -7,9 +7,9 @@ import { CheckpointDB } from './checkpointDb'
 export class CheckpointTrie extends BaseTrie {
   db: CheckpointDB
 
-  constructor(...args: any) {
-    super(...args)
-    this.db = new CheckpointDB(...args)
+  constructor(opts: TrieOpts = {}) {
+    super(opts)
+    this.db = new CheckpointDB(opts.db)
   }
 
   /**
@@ -63,7 +63,11 @@ export class CheckpointTrie extends BaseTrie {
    */
   copy(includeCheckpoints = true): CheckpointTrie {
     const db = this.db.copy()
-    const trie = new CheckpointTrie(db._leveldb, this.root)
+    const trie = new CheckpointTrie({
+      db: db._leveldb,
+      root: this.root,
+      deleteFromDB: (this as any)._deleteFromDB,
+    })
     if (includeCheckpoints && this.isCheckpoint) {
       trie.db.checkpoints = [...this.db.checkpoints]
     }

packages/trie/src/db.ts

@@ -27,7 +27,7 @@ export class DB {
    * defaults to an [in-memory store](https://github.com/Level/memdown).
    * @param leveldb - An abstract-leveldown compliant store
    */
-  constructor(leveldb?: LevelUp) {
+  constructor(leveldb?: LevelUp | null) {
     this._leveldb = leveldb ?? level()
   }
 

packages/trie/src/secure.ts

@@ -11,10 +11,6 @@ import { Proof } from './baseTrie'
  * @public
  */
 export class SecureTrie extends CheckpointTrie {
-  constructor(...args: any) {
-    super(...args)
-  }
-
   /**
    * Gets a value given a `key`
    * @param key - the key to search for
@@ -110,7 +106,11 @@ export class SecureTrie extends CheckpointTrie {
    */
   copy(includeCheckpoints = true): SecureTrie {
     const db = this.db.copy()
-    const secureTrie = new SecureTrie(db._leveldb, this.root)
+    const secureTrie = new SecureTrie({
+      db: db._leveldb,
+      root: this.root,
+      deleteFromDB: (this as any)._deleteFromDB,
+    })
     if (includeCheckpoints && this.isCheckpoint) {
       secureTrie.db.checkpoints = [...this.db.checkpoints]
     }

packages/trie/src/verifyRangeProof.ts

@@ -317,7 +317,7 @@ async function verifyProof(
   key: Buffer,
   proof: Buffer[]
 ): Promise<{ value: Buffer | null; trie: Trie }> {
-  let proofTrie = new Trie(null, rootHash)
+  let proofTrie = new Trie({ root: rootHash })
   try {
     proofTrie = await Trie.fromProof(proof, proofTrie)
   } catch (e) {
@@ -483,7 +483,7 @@ export async function verifyRangeProof(
     )
   }
 
-  let trie = new Trie(null, rootHash)
+  let trie = new Trie({ root: rootHash })
   trie = await Trie.fromProof(proof, trie)
 
   // Remove all nodes between two edge proofs

packages/trie/test/index.spec.ts

@@ -14,7 +14,7 @@ tape('simple save and retrieve', function (tester) {
       '3f4399b08efe68945c1cf90ffe85bbe3ce978959da753f9e649f034015b8817d',
       'hex'
     )
-    const trie = new CheckpointTrie(null, root)
+    const trie = new CheckpointTrie({ root })
     const value = await trie.get(Buffer.from('test'))
     t.equal(value, null)
     t.end()
@@ -165,7 +165,7 @@ tape('testing deletion cases', function (tester) {
     msg: 'without DB delete',
   }
   const trieSetupWithDBDelete = {
-    trie: new CheckpointTrie(undefined, undefined, true),
+    trie: new CheckpointTrie({ deleteFromDB: true }),
     msg: 'with DB delete',
   }
   const trieSetups = [trieSetupWithoutDBDelete, trieSetupWithDBDelete]
@@ -311,12 +311,12 @@ tape('setting back state root (deleteFromDB)', async (t) => {
 
   const trieSetups = [
     {
-      trie: new BaseTrie(undefined, undefined, false),
+      trie: new BaseTrie({ deleteFromDB: false }),
       expected: v1,
       msg: 'should return v1 when setting back the state root when deleteFromDB=false',
     },
     {
-      trie: new BaseTrie(undefined, undefined, true),
+      trie: new BaseTrie({ deleteFromDB: true }),
       expected: null,
       msg: 'should return null when setting back the state root when deleteFromDB=true',
     },