feat(trie): implement root hash persistence

[faustbrian]

Resolves #2061

Please don't update the branch via UI because I'll be squashing my commits as I keep pushing.

A few question before I continue this and add tests:

• One of the requirements is It should default to true if a DB is passed into the constructor and be hardcoded to false if not. so what should the conditional be. Something like this._persistRoot = (opts?.db ? true : opts?.persistRoot) ?? false? The requirement sounds a bit confusing because what if the user doesn't want root persistence but they still passed in a database? The boolean flag seems a bit unnecessary if we make it dependent on opts.db being defined.
• I added const ROOT_DB_KEY = Buffer.from('__root__') and wanted to add another constant for the keccak256 key but then realised that the hashing function is now modular. Should I do the hashing on the fly?
• Should I throw an exception if put methods of any trie are called with the ROOT_DB_KEY key, meaning a user manually wanted to modify that value?
• I used __root__ instead of __persistedRootHash__ since the __ prefix and suffix should already distinguish it enough from user space wording.

[faustbrian]

Something I noticed here is that hash: this.hash seems to be missing. The Trie passes it down when it calls copy but CheckpointTrie and SecureTrie don't. Is this intentional or an oversight?

  copy(): Trie {
    return new Trie({
      db: this.db.copy(),
      root: this.root,
      deleteFromDB: this._deleteFromDB,
      hash: this.hash,
      persistRoot: this._persistRoot,
    })
  }

I assume this could result in corrupted data if you copy a CheckpointTrie or SecureTrie that for example uses sha256 but then it doesn't pass down the hash function and the copy works with keccak256 hashes because const _hash = opts?.hash ?? keccak256 will fall back to the keccak256 function?

[faustbrian]

  it.test('created copy uses the correct hash function', async function (t) {
    const trie = new SecureTrie({
      db: new LevelDB(),
      hash: (value) => createHash('sha256').update(value).digest(),
    })

    await trie.put(Buffer.from('key1'), Buffer.from('value1'))
    trie.checkpoint()
    await trie.put(Buffer.from('key2'), Buffer.from('value2'))
    const trieCopy = trie.copy()
    const value = await trieCopy.get(Buffer.from('key1'))
    t.equal(value!.toString(), 'value1')
    t.end()
  })

# created copy uses the correct hash function

not ok 183 TypeError: Cannot read properties of null (reading 'toString')
  ---
    operator: error
    stack: |-
      TypeError: Cannot read properties of null (reading 'toString')
          at Test.<anonymous> (/Users/brianfaust/Work/bearmint/ethereumjs-monorepo/packages/trie/test/trie/secure.spec.ts:167:20)

I think the above reproduces the issue. If I then change to the following copy function for SecureTrie it'll pass.

  copy(includeCheckpoints = true): SecureTrie {
    const secureTrie = new SecureTrie({
      db: this.dbStorage.copy(),
      root: this.root,
      deleteFromDB: (this as any)._deleteFromDB,
      persistRoot: (this as any)._persistRoot,
      hash: this.hash,
    })
    if (includeCheckpoints && this.isCheckpoint) {
      secureTrie.db.checkpoints = [...this.db.checkpoints]
    }
    return secureTrie
  }

The above test will fail because it will store key1 hashed via sha256 and key2 hashed via keccak256 but then the copy of the trie will use keccak256 again because this.hash wasn't passed down which results in it looking up key1 hashed via keccak256 instead of the sha256 it was stored as. Does that make sense @acolytec3 and can you reproduce it?

[acolytec3]

Nice catch. It makes sense and is likely a miss from #2043. I'm AFK most of today but will try and reproduce later.

[faustbrian]

#2067 (comment)

[codecov]

Codecov Report

Merging #2067 (411f192) into master (0bb7e21) will decrease coverage by 0.05%.
The diff coverage is 69.56%.

Flag	Coverage Δ
block	83.75% <ø> (-0.27%)	⬇️
blockchain	80.10% <ø> (ø)
client	78.37% <ø> (ø)
common	95.00% <ø> (+0.09%)	⬆️
devp2p	82.60% <ø> (-0.13%)	⬇️
ethash	90.81% <ø> (ø)
evm	40.89% <ø> (ø)
statemanager	84.55% <ø> (ø)
trie	80.51% <69.56%> (-0.94%)	⬇️
tx	92.18% <ø> (ø)
util	87.22% <ø> (ø)
vm	78.51% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

[acolytec3]

this._persistRoot = (opts?.db ? true : opts?.persistRoot) ?? false

I was thinking something like this

if (opts.db) {
  this.persistRoot = opts.persistRoot ?? true
} else {
  this.persistRoot = false
}

[faustbrian]

Opened #2068 to fix an issue that needs resolving before I can write proper tests here.