From d78f9031002b90130191119891c70cf96476eab6 Mon Sep 17 00:00:00 2001 From: Gabriel Rocheleau Date: Mon, 29 Aug 2022 23:54:52 -0400 Subject: [PATCH 1/2] trie: remove isTruthy and isFalsy --- packages/trie/benchmarks/engines/level.ts | 5 ++--- packages/trie/recipes/level-legacy.ts | 5 ++--- packages/trie/recipes/level.ts | 5 ++--- packages/trie/src/trie/trie.ts | 26 +++++++++++------------ packages/trie/test/official.spec.ts | 7 +++--- packages/trie/test/trie/secure.spec.ts | 13 ++++++++---- 6 files changed, 31 insertions(+), 30 deletions(-) diff --git a/packages/trie/benchmarks/engines/level.ts b/packages/trie/benchmarks/engines/level.ts index fb4a8fcacc..4f5a66a1a0 100644 --- a/packages/trie/benchmarks/engines/level.ts +++ b/packages/trie/benchmarks/engines/level.ts @@ -1,5 +1,4 @@ // eslint-disable-next-line implicit-dependencies/no-implicit -import { isTruthy } from '@ethereumjs/util' import { MemoryLevel } from 'memory-level' import type { BatchDBOp, DB } from '../../src/types' @@ -29,11 +28,11 @@ export class LevelDB implements DB { * @inheritDoc */ async get(key: Buffer): Promise { - let value = null + let value: Buffer | null = null try { value = await this._leveldb.get(key, ENCODING_OPTS) } catch (error: any) { - if (isTruthy(error.notFound)) { + if (error.notFound !== undefined) { // not found, returning null } else { throw error diff --git a/packages/trie/recipes/level-legacy.ts b/packages/trie/recipes/level-legacy.ts index 515b315301..bcd2d22e24 100644 --- a/packages/trie/recipes/level-legacy.ts +++ b/packages/trie/recipes/level-legacy.ts @@ -1,4 +1,3 @@ -import { isTruthy } from '@ethereumjs/util' import level from 'level-mem' import type { BatchDBOp, DB } from '@ethereumjs/trie' @@ -14,11 +13,11 @@ export class LevelDB implements DB { } async get(key: Buffer): Promise { - let value = null + let value: Buffer | null = null try { value = await this._leveldb.get(key, ENCODING_OPTS) } catch (error: any) { - if (isTruthy(error.notFound)) { + if (error.notFound !== undefined) { // not found, returning null } else { throw error diff --git a/packages/trie/recipes/level.ts b/packages/trie/recipes/level.ts index 7b26011365..e0b04f7bd5 100644 --- a/packages/trie/recipes/level.ts +++ b/packages/trie/recipes/level.ts @@ -1,4 +1,3 @@ -import { isTruthy } from '@ethereumjs/util' import { MemoryLevel } from 'memory-level' import type { BatchDBOp, DB } from '@ethereumjs/trie' @@ -16,11 +15,11 @@ export class LevelDB implements DB { } async get(key: Buffer): Promise { - let value = null + let value: Buffer | null = null try { value = await this._leveldb.get(key, ENCODING_OPTS) } catch (error: any) { - if (isTruthy(error.notFound)) { + if (error.notFound !== undefined) { // not found, returning null } else { throw error diff --git a/packages/trie/src/trie/trie.ts b/packages/trie/src/trie/trie.ts index 06eb71c8a4..3e457e7de8 100644 --- a/packages/trie/src/trie/trie.ts +++ b/packages/trie/src/trie/trie.ts @@ -1,4 +1,4 @@ -import { RLP_EMPTY_STRING, isFalsy, isTruthy } from '@ethereumjs/util' +import { RLP_EMPTY_STRING } from '@ethereumjs/util' import { keccak256 } from 'ethereum-cryptography/keccak' import { CheckpointDB, MapDB } from '../db' @@ -97,9 +97,9 @@ export class Trie { /** * Gets and/or Sets the current root of the `trie` */ - root(value?: Buffer): Buffer { + root(value?: Buffer | null): Buffer { if (value !== undefined) { - if (isFalsy(value)) { + if (value === null) { value = this.EMPTY_TRIE_ROOT } @@ -137,7 +137,7 @@ export class Trie { */ async get(key: Buffer, throwIfMissing = false): Promise { const { node, remaining } = await this.findPath(this.appliedKey(key), throwIfMissing) - let value = null + let value: Buffer | null = null if (node && remaining.length === 0) { value = node.value() } @@ -157,7 +157,7 @@ export class Trie { } // If value is empty, delete - if (isFalsy(value) || value.toString() === '') { + if (value === null || value.length === 0) { return await this.del(key) } @@ -426,9 +426,9 @@ export class Trie { stack: TrieNode[] ) => { // branchNode is the node ON the branch node not THE branch node - if (isFalsy(parentNode) || parentNode instanceof BranchNode) { + if (parentNode === null || parentNode === undefined || parentNode instanceof BranchNode) { // branch->? - if (isTruthy(parentNode)) { + if (parentNode !== null && parentNode !== undefined) { stack.push(parentNode) } @@ -460,9 +460,9 @@ export class Trie { stack.push(parentNode) } else { const branchNodeKey = branchNode.key() - // branch node is an leaf or extension and parent node is an exstention + // branch node is an leaf or extension and parent node is an extension // add two keys together - // dont push the parent node + // don't push the parent node branchNodeKey.unshift(branchKey) key = key.concat(branchNodeKey) parentKey = parentKey.concat(branchNodeKey) @@ -475,8 +475,8 @@ export class Trie { return key } - let lastNode = stack.pop() as TrieNode - if (isFalsy(lastNode)) throw new Error('missing last node') + let lastNode = stack.pop() + if (lastNode === undefined) throw new Error('missing last node') let parentNode = stack.pop() const opStack: BatchDBOp[] = [] @@ -633,7 +633,7 @@ export class Trie { async batch(ops: BatchDBOp[]): Promise { for (const op of ops) { if (op.type === 'put') { - if (isFalsy(op.value)) { + if (op.value === null || op.value === undefined) { throw new Error('Invalid batch db operation') } await this.put(op.key, op.value) @@ -657,7 +657,7 @@ export class Trie { } as PutBatch }) - if (this.root() === this.EMPTY_TRIE_ROOT && isTruthy(opStack[0])) { + if (this.root() === this.EMPTY_TRIE_ROOT && opStack[0] !== undefined && opStack[0] !== null) { this.root(opStack[0].key) } diff --git a/packages/trie/test/official.spec.ts b/packages/trie/test/official.spec.ts index 1ad27b7ba7..5d93fc4c57 100644 --- a/packages/trie/test/official.spec.ts +++ b/packages/trie/test/official.spec.ts @@ -1,4 +1,3 @@ -import { isTruthy } from '@ethereumjs/util' import * as tape from 'tape' import { Trie } from '../src' @@ -13,9 +12,9 @@ tape('official tests', async function (t) { const expect = jsonTests[testName].root for (const input of inputs) { for (let i = 0; i < 2; i++) { - if (isTruthy(input[i]) && input[i].slice(0, 2) === '0x') { + if (input[i] !== undefined && input[i] !== null && input[i].slice(0, 2) === '0x') { input[i] = Buffer.from(input[i].slice(2), 'hex') - } else if (isTruthy(input[i]) && typeof input[i] === 'string') { + } else if (typeof input[i] === 'string') { input[i] = Buffer.from(input[i]) } await trie.put(Buffer.from(input[0]), input[1]) @@ -42,7 +41,7 @@ tape('official tests any order', async function (t) { key = Buffer.from(key.slice(2), 'hex') } - if (isTruthy(val) && val.slice(0, 2) === '0x') { + if (val !== undefined && val !== null && val.slice(0, 2) === '0x') { val = Buffer.from(val.slice(2), 'hex') } diff --git a/packages/trie/test/trie/secure.spec.ts b/packages/trie/test/trie/secure.spec.ts index 66b8e70d7c..1bd5ba52a5 100644 --- a/packages/trie/test/trie/secure.spec.ts +++ b/packages/trie/test/trie/secure.spec.ts @@ -1,4 +1,3 @@ -import { isTruthy } from '@ethereumjs/util' import { createHash } from 'crypto' import * as tape from 'tape' @@ -41,7 +40,10 @@ tape('SecureTrie', function (t) { it.test('empty values', async function (t) { for (const row of jsonTests.emptyValues.in) { - const val = isTruthy(row[1]) ? Buffer.from(row[1]) : (null as unknown as Buffer) + const val = + row[1] !== undefined && row[1] !== null + ? Buffer.from(row[1]) + : (null as unknown as Buffer) await trie.put(Buffer.from(row[0]), val) } t.equal('0x' + trie.root().toString('hex'), jsonTests.emptyValues.root) @@ -51,7 +53,10 @@ tape('SecureTrie', function (t) { it.test('branchingTests', async function (t) { trie = new Trie({ useKeyHashing: true, db: new MapDB() }) for (const row of jsonTests.branchingTests.in) { - const val = isTruthy(row[1]) ? Buffer.from(row[1]) : (null as unknown as Buffer) + const val = + row[1] !== undefined && row[1] !== null + ? Buffer.from(row[1]) + : (null as unknown as Buffer) await trie.put(Buffer.from(row[0]), val) } t.equal('0x' + trie.root().toString('hex'), jsonTests.branchingTests.root) @@ -61,7 +66,7 @@ tape('SecureTrie', function (t) { it.test('jeff', async function (t) { for (const row of jsonTests.jeff.in) { let val = row[1] - if (isTruthy(val)) { + if (val !== undefined && val !== null) { val = Buffer.from(row[1].slice(2), 'hex') } await trie.put(Buffer.from(row[0].slice(2), 'hex'), val) From 089295e57f726d8ae4aec703215ce245cd712030 Mon Sep 17 00:00:00 2001 From: Gabriel Rocheleau Date: Tue, 30 Aug 2022 09:26:33 -0400 Subject: [PATCH 2/2] trie: simplify get method logic --- packages/trie/benchmarks/engines/level.ts | 9 +++++---- packages/trie/recipes/level-legacy.ts | 9 +++++---- packages/trie/recipes/level.ts | 9 +++++---- 3 files changed, 15 insertions(+), 12 deletions(-) diff --git a/packages/trie/benchmarks/engines/level.ts b/packages/trie/benchmarks/engines/level.ts index 4f5a66a1a0..73623019d6 100644 --- a/packages/trie/benchmarks/engines/level.ts +++ b/packages/trie/benchmarks/engines/level.ts @@ -32,13 +32,14 @@ export class LevelDB implements DB { try { value = await this._leveldb.get(key, ENCODING_OPTS) } catch (error: any) { - if (error.notFound !== undefined) { - // not found, returning null - } else { + // https://github.com/Level/abstract-level/blob/915ad1317694d0ce8c580b5ab85d81e1e78a3137/abstract-level.js#L309 + // This should be `true` if the error came from LevelDB + // so we can check for `NOT true` to identify any non-404 errors + if (error.notFound !== true) { throw error } } - return value as Buffer + return value } /** diff --git a/packages/trie/recipes/level-legacy.ts b/packages/trie/recipes/level-legacy.ts index bcd2d22e24..c06c008e0a 100644 --- a/packages/trie/recipes/level-legacy.ts +++ b/packages/trie/recipes/level-legacy.ts @@ -17,13 +17,14 @@ export class LevelDB implements DB { try { value = await this._leveldb.get(key, ENCODING_OPTS) } catch (error: any) { - if (error.notFound !== undefined) { - // not found, returning null - } else { + // https://github.com/Level/abstract-level/blob/915ad1317694d0ce8c580b5ab85d81e1e78a3137/abstract-level.js#L309 + // This should be `true` if the error came from LevelDB + // so we can check for `NOT true` to identify any non-404 errors + if (error.notFound !== true) { throw error } } - return value as Buffer + return value } async put(key: Buffer, val: Buffer): Promise { diff --git a/packages/trie/recipes/level.ts b/packages/trie/recipes/level.ts index e0b04f7bd5..62ac888cdb 100644 --- a/packages/trie/recipes/level.ts +++ b/packages/trie/recipes/level.ts @@ -19,13 +19,14 @@ export class LevelDB implements DB { try { value = await this._leveldb.get(key, ENCODING_OPTS) } catch (error: any) { - if (error.notFound !== undefined) { - // not found, returning null - } else { + // https://github.com/Level/abstract-level/blob/915ad1317694d0ce8c580b5ab85d81e1e78a3137/abstract-level.js#L309 + // This should be `true` if the error came from LevelDB + // so we can check for `NOT true` to identify any non-404 errors + if (error.notFound !== true) { throw error } } - return value as Buffer + return value } async put(key: Buffer, val: Buffer): Promise {