Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(trie): rename persistRoot to useRootPersistence #2223

Merged
merged 37 commits into from
Aug 24, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
fe64bd1
trie: removed deprecated prove() function (renamed to createProof())
holgerd77 Aug 23, 2022
ddadf76
trie: added secure option to base trie, integrated secure trie functi…
holgerd77 Aug 23, 2022
aa731fb
trie: adopted secureTrie.spec.ts tests (5 failing)
holgerd77 Aug 23, 2022
3a511d8
trie: fixed copy function
holgerd77 Aug 23, 2022
ce77025
trie: fixed secureTrie.spec.ts branching test
holgerd77 Aug 23, 2022
5cb020a
trie: additional test fixes
holgerd77 Aug 23, 2022
80156ec
trie: removed secure.ts SecureTrie class definition file
holgerd77 Aug 23, 2022
31f89cb
trie: updated README
holgerd77 Aug 23, 2022
f090a80
trie: removed secure export, unified prepareTrieOpts and expand to Ch…
holgerd77 Aug 23, 2022
93dc08e
chore: wip
faustbrian Aug 23, 2022
19f06dd
chore: wip
faustbrian Aug 23, 2022
3042ff7
trie: renamed _secure -> _hashKeys and _hash() -> _hashKeysFunction
holgerd77 Aug 23, 2022
789607e
refactor(trie): rename `secure` to `useHashedKeys`
faustbrian Aug 23, 2022
7add314
Merge branch 'refactore-secure-trie-as-an-option' into trie-checkpoin…
faustbrian Aug 23, 2022
f8e2572
chore: wip
faustbrian Aug 23, 2022
a2ccd7c
chore: wip
faustbrian Aug 23, 2022
132fa1f
chore: wip
faustbrian Aug 23, 2022
a793952
Merge branch 'refactore-secure-trie-as-an-option' into trie-checkpoin…
faustbrian Aug 23, 2022
1dfc5b4
chore: wip
faustbrian Aug 23, 2022
94021ed
chore: wip
faustbrian Aug 23, 2022
925469b
chore: wip
faustbrian Aug 23, 2022
4ec3364
chore: wip
faustbrian Aug 23, 2022
b014f34
Merge branch 'refactore-secure-trie-as-an-option' into trie-checkpoin…
faustbrian Aug 23, 2022
a50c89c
refactor(trie): rename `secure` to `useHashedKeys`
faustbrian Aug 23, 2022
2388783
Merge branch 'refactore-secure-trie-as-an-option' into trie-checkpoin…
faustbrian Aug 23, 2022
906f600
chore: wip
faustbrian Aug 23, 2022
29a5445
refactor(trie): always use `CheckpointDB` as internal database
faustbrian Aug 23, 2022
f45b020
chore: wip
faustbrian Aug 23, 2022
7ceff28
chore: wip
faustbrian Aug 23, 2022
953b819
Merge branch 'unify-db' into trie-checkpoint-option
faustbrian Aug 24, 2022
1e1178e
Merge branch 'master' into trie-checkpoint-option
faustbrian Aug 24, 2022
913f7f1
refactor: always support checkpointing
faustbrian Aug 24, 2022
a0ab555
chore: wip
faustbrian Aug 24, 2022
cb3809a
refactor(trie): rename `persistRoot` to `useRootPersistence`
faustbrian Aug 24, 2022
1c5fe9f
Merge branch 'master' into rename-useRootPersistence
faustbrian Aug 24, 2022
4d14f36
Merge branch 'master' into rename-useRootPersistence
holgerd77 Aug 24, 2022
d86c649
Merge remote-tracking branch 'origin/big-messy-bunch' into rename-use…
faustbrian Aug 24, 2022
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
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