Skip to content

Commit

Permalink
refactor(trie): rename persistRoot to useRootPersistence (#2223)
Browse files Browse the repository at this point in the history
  • Loading branch information
faustbrian authored Aug 24, 2022
1 parent 5f67f7c commit 74994e1
Show file tree
Hide file tree
Showing 6 changed files with 68 additions and 36 deletions.
9 changes: 6 additions & 3 deletions packages/trie/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,18 @@ Beta 3 release for the upcoming breaking release round on the [EthereumJS monore

### Root Hash Persistance

The trie library now comes with a new constructor option `persistRoot` which is disabled by default but allows to persist state root updates along write operations directly in the DB and therefore omits the need to manually set to a new state root, see PR [#2071](https://github.com/ethereumjs/ethereumjs-monorepo/pull/2071) and PR [#2123](https://github.com/ethereumjs/ethereumjs-monorepo/pull/2123), thanks to @faustbrian for the contribution! ❤️
The trie library now comes with a new constructor option `useRootPersistence` which is disabled by default but allows to persist state root updates along write operations directly in the DB and therefore omits the need to manually set to a new state root, see PR [#2071](https://github.com/ethereumjs/ethereumjs-monorepo/pull/2071) and PR [#2123](https://github.com/ethereumjs/ethereumjs-monorepo/pull/2123), thanks to @faustbrian for the contribution! ❤️

To activate root hash persistance you can set the `persistRoot` option on instantiation:
To activate root hash persistance you can set the `useRootPersistence` option on instantiation:

```typescript
import { Trie, LevelDB } from '@ethereumjs/trie'
import { Level } from 'level'

const trie = new Trie({ db: new LevelDB(new Level('MY_TRIE_DB_LOCATION')), persistRoot: true })
const trie = new Trie({
db: new LevelDB(new Level('MY_TRIE_DB_LOCATION')),
useRootPersistence: true,
})
```

### Other Changes
Expand Down
4 changes: 2 additions & 2 deletions packages/trie/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -67,15 +67,15 @@ By default, the deletion of trie nodes from the underlying database does not occ

#### Persistence

You can enable persistence by setting the `persistRoot` option to `true` when constructing a trie through the `Trie.create` function. As such, this value is preserved when creating copies of the trie and is incapable of being modified once a trie is instantiated.
You can enable persistence by setting the `useRootPersistence` option to `true` when constructing a trie through the `Trie.create` function. As such, this value is preserved when creating copies of the trie and is incapable of being modified once a trie is instantiated.

```typescript
import { Trie, LevelDB } from '@ethereumjs/trie'
import { Level } from 'level'

const trie = await Trie.create({
db: new LevelDB(new Level('MY_TRIE_DB_LOCATION')),
persistRoot: true,
useRootPersistence: true,
})
```

Expand Down
12 changes: 6 additions & 6 deletions packages/trie/src/trie/trie.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ export class Trie {
protected _useKeyHashing: boolean
protected _useKeyHashingFunction: HashKeysFunction
protected _hashLen: number
protected _persistRoot: boolean
protected _useRootPersistence: boolean

/**
* Create a new trie
Expand All @@ -66,7 +66,7 @@ export class Trie {
this._hashLen = this.EMPTY_TRIE_ROOT.length
this._root = this.EMPTY_TRIE_ROOT
this._deleteFromDB = opts?.deleteFromDB ?? false
this._persistRoot = opts?.persistRoot ?? false
this._useRootPersistence = opts?.useRootPersistence ?? false

if (opts?.root) {
this.root = opts.root
Expand All @@ -82,7 +82,7 @@ export class Trie {

key = Buffer.from(key)

if (opts?.db !== undefined && opts?.persistRoot === true) {
if (opts?.db !== undefined && opts?.useRootPersistence === true) {
if (opts?.root === undefined) {
opts.root = (await opts?.db.get(key)) ?? undefined
} else {
Expand Down Expand Up @@ -151,7 +151,7 @@ export class Trie {
* @returns A Promise that resolves once value is stored.
*/
async put(key: Buffer, value: Buffer): Promise<void> {
if (this._persistRoot && key.equals(ROOT_DB_KEY)) {
if (this._useRootPersistence && key.equals(ROOT_DB_KEY)) {
throw new Error(`Attempted to set '${ROOT_DB_KEY.toString()}' key but it is not allowed.`)
}

Expand Down Expand Up @@ -745,7 +745,7 @@ export class Trie {
const trie = new Trie({
db: this.db.db.copy(),
deleteFromDB: this._deleteFromDB,
persistRoot: this._persistRoot,
useRootPersistence: this._useRootPersistence,
root: this.root,
useKeyHashing: this._useKeyHashing,
useKeyHashingFunction: this._useKeyHashingFunction,
Expand All @@ -760,7 +760,7 @@ export class Trie {
* Persists the root hash in the underlying database
*/
async persistRoot() {
if (this._persistRoot === true) {
if (this._useRootPersistence === true) {
await this.db.put(this.appliedKey(ROOT_DB_KEY), this.root)
}
}
Expand Down
2 changes: 1 addition & 1 deletion packages/trie/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ export interface TrieOpts {
/**
* Store the root inside the database after every `write` operation
*/
persistRoot?: boolean
useRootPersistence?: boolean
}

export type BatchDBOp = PutBatch | DelBatch
Expand Down
2 changes: 1 addition & 1 deletion packages/trie/test/trie/secure.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ tape('SecureTrie', function (t) {
})

it.test('put fails if the key is the ROOT_DB_KEY', async function (st) {
const trie = new Trie({ useKeyHashing: true, db: new MapDB(), persistRoot: true })
const trie = new Trie({ useKeyHashing: true, db: new MapDB(), useRootPersistence: true })

try {
await trie.put(ROOT_DB_KEY, Buffer.from('bar'))
Expand Down
75 changes: 52 additions & 23 deletions packages/trie/test/trie/trie.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,30 +41,42 @@ for (const { constructor, defaults, title } of [
t.test(
'creates an instance via the static constructor `create` function and defaults to `false` with a database',
async function (st) {
st.false(((await constructor.create({ ...defaults, db: new MapDB() })) as any)._persistRoot)
st.false(
((await constructor.create({ ...defaults, db: new MapDB() })) as any)._useRootPersistence
)

st.end()
}
)

t.test(
'creates an instance via the static constructor `create` function and respects the `persistRoot` option with a database',
'creates an instance via the static constructor `create` function and respects the `useRootPersistence` option with a database',
async function (st) {
st.false(
((await constructor.create({ ...defaults, db: new MapDB(), persistRoot: false })) as any)
._persistRoot
(
(await constructor.create({
...defaults,
db: new MapDB(),
useRootPersistence: false,
})) as any
)._useRootPersistence
)

st.end()
}
)

t.test(
'creates an instance via the static constructor `create` function and respects the `persistRoot` option with a database',
'creates an instance via the static constructor `create` function and respects the `useRootPersistence` option with a database',
async function (st) {
st.false(
((await constructor.create({ ...defaults, db: new MapDB(), persistRoot: false })) as any)
._persistRoot
(
(await constructor.create({
...defaults,
db: new MapDB(),
useRootPersistence: false,
})) as any
)._useRootPersistence
)

st.end()
Expand All @@ -74,14 +86,20 @@ for (const { constructor, defaults, title } of [
t.test(
'creates an instance via the static constructor `create` function and defaults to `false` without a database',
async function (st) {
st.false(((await constructor.create({ ...defaults, db: new MapDB() })) as any)._persistRoot)
st.false(
((await constructor.create({ ...defaults, db: new MapDB() })) as any)._useRootPersistence
)

st.end()
}
)

t.test('persist the root if the `persistRoot` option is `true`', async function (st) {
const trie = await constructor.create({ ...defaults, db: new MapDB(), persistRoot: true })
t.test('persist the root if the `useRootPersistence` option is `true`', async function (st) {
const trie = await constructor.create({
...defaults,
db: new MapDB(),
useRootPersistence: true,
})

st.equal(await trie.db.get(ROOT_DB_KEY), null)

Expand All @@ -97,7 +115,7 @@ for (const { constructor, defaults, title } of [
...defaults,
db: new MapDB(),
root: KECCAK256_RLP,
persistRoot: true,
useRootPersistence: true,
})

st.true((await trie.db.get(ROOT_DB_KEY))?.equals(KECCAK256_RLP))
Expand All @@ -109,20 +127,27 @@ for (const { constructor, defaults, title } of [
st.end()
})

t.test('does not persist the root if the `persistRoot` option is `false`', async function (st) {
const trie = await constructor.create({ ...defaults, db: new MapDB(), persistRoot: false })
t.test(
'does not persist the root if the `useRootPersistence` option is `false`',
async function (st) {
const trie = await constructor.create({
...defaults,
db: new MapDB(),
useRootPersistence: false,
})

st.equal(await trie.db.get(ROOT_DB_KEY), null)
st.equal(await trie.db.get(ROOT_DB_KEY), null)

await trie.put(Buffer.from('do_not_persist_with_db'), Buffer.from('bar'))
await trie.put(Buffer.from('do_not_persist_with_db'), Buffer.from('bar'))

st.equal(await trie.db.get(ROOT_DB_KEY), null)
st.equal(await trie.db.get(ROOT_DB_KEY), null)

st.end()
})
st.end()
}
)

t.test('persists the root if the `db` option is not provided', async function (st) {
const trie = await constructor.create({ ...defaults, persistRoot: true })
const trie = await constructor.create({ ...defaults, useRootPersistence: true })

st.equal(await trie.db.get(ROOT_DB_KEY), null)

Expand All @@ -136,24 +161,28 @@ for (const { constructor, defaults, title } of [
t.test('persist and restore the root', async function (st) {
const db = new MapDB()

const trie = await constructor.create({ ...defaults, db, persistRoot: true })
const trie = await constructor.create({ ...defaults, db, useRootPersistence: true })
st.equal(await trie.db.get(ROOT_DB_KEY), null)
await trie.put(Buffer.from('foo'), Buffer.from('bar'))
st.equal(bytesToHex(await trie.db.get(ROOT_DB_KEY)), EXPECTED_ROOTS)

// Using the same database as `trie` so we should have restored the root
const copy = await constructor.create({ ...defaults, db, persistRoot: true })
const copy = await constructor.create({ ...defaults, db, useRootPersistence: true })
st.equal(bytesToHex(await copy.db.get(ROOT_DB_KEY)), EXPECTED_ROOTS)

// New trie with a new database so we shouldn't find a root to restore
const empty = await constructor.create({ ...defaults, db: new MapDB(), persistRoot: true })
const empty = await constructor.create({
...defaults,
db: new MapDB(),
useRootPersistence: true,
})
st.equal(await empty.db.get(ROOT_DB_KEY), null)

st.end()
})

t.test('put fails if the key is the ROOT_DB_KEY', async function (st) {
const trie = new constructor({ ...defaults, db: new MapDB(), persistRoot: true })
const trie = new constructor({ ...defaults, db: new MapDB(), useRootPersistence: true })

try {
await trie.put(BASE_DB_KEY, Buffer.from('bar'))
Expand Down

0 comments on commit 74994e1

Please sign in to comment.