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.

Sign up for GitHub

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

trie: remove isTruthy and isFalsy #2249

Merged
merged 2 commits into from
Aug 30, 2022
Merged

trie: remove isTruthy and isFalsy #2249

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d78f903
trie: remove isTruthy and isFalsy
gabrocheleau Aug 30, 2022
089295e
trie: simplify get method logic
gabrocheleau Aug 30, 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
12 changes: 6 additions & 6 deletions packages/trie/benchmarks/engines/level.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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'
Expand Down Expand Up @@ -29,17 +28,18 @@ export class LevelDB implements DB {
* @inheritDoc
*/
async get(key: Buffer): Promise<Buffer | null> {
let value = null
let value: Buffer | null = null
try {
value = await this._leveldb.get(key, ENCODING_OPTS)
} catch (error: any) {
if (isTruthy(error.notFound)) {
// 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
}

/**
Expand Down
12 changes: 6 additions & 6 deletions packages/trie/recipes/level-legacy.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import { isTruthy } from '@ethereumjs/util'
import level from 'level-mem'

import type { BatchDBOp, DB } from '@ethereumjs/trie'
Expand All @@ -14,17 +13,18 @@ export class LevelDB implements DB {
}

async get(key: Buffer): Promise<Buffer | null> {
let value = null
let value: Buffer | null = null
try {
value = await this._leveldb.get(key, ENCODING_OPTS)
} catch (error: any) {
if (isTruthy(error.notFound)) {
// 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<void> {
Expand Down
12 changes: 6 additions & 6 deletions packages/trie/recipes/level.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import { isTruthy } from '@ethereumjs/util'
import { MemoryLevel } from 'memory-level'

import type { BatchDBOp, DB } from '@ethereumjs/trie'
Expand All @@ -16,17 +15,18 @@ export class LevelDB implements DB {
}

async get(key: Buffer): Promise<Buffer | null> {
let value = null
let value: Buffer | null = null
try {
value = await this._leveldb.get(key, ENCODING_OPTS)
} catch (error: any) {
if (isTruthy(error.notFound)) {
// 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<void> {
Expand Down
26 changes: 13 additions & 13 deletions packages/trie/src/trie/trie.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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'
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -137,7 +137,7 @@ export class Trie {
*/
async get(key: Buffer, throwIfMissing = false): Promise<Buffer | null> {
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()
}
Expand All @@ -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)
}

Expand Down Expand Up @@ -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)
}

Expand Down Expand Up @@ -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)
Expand All @@ -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[] = []

Expand Down Expand Up @@ -633,7 +633,7 @@ export class Trie {
async batch(ops: BatchDBOp[]): Promise<void> {
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)
Expand All @@ -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) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, but totally love all this (have taken a random line to comment 😋)!!!

This is structurally such an improvement for the whole library suite! 🎉

this.root(opStack[0].key)
}

Expand Down
7 changes: 3 additions & 4 deletions packages/trie/test/official.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import { isTruthy } from '@ethereumjs/util'
import * as tape from 'tape'

import { Trie } from '../src'
Expand All @@ -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])
Expand All @@ -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')
}

Expand Down
13 changes: 9 additions & 4 deletions packages/trie/test/trie/secure.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import { isTruthy } from '@ethereumjs/util'
import { createHash } from 'crypto'
import * as tape from 'tape'

Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand Down