From 0639af1f9cf26ae25be55cd037b084df1e0de044 Mon Sep 17 00:00:00 2001 From: Gabriel Rocheleau Date: Mon, 2 Sep 2024 08:37:04 -0400 Subject: [PATCH 1/3] trie: address type issues and type-related improvements (#3624) * trie: address type issues and improvements * trie: improve comment for empty extension node --- packages/trie/README.md | 7 ++- packages/trie/benchmarks/engines/level.ts | 10 ++-- packages/trie/examples/customLevelDB.ts | 7 ++- packages/trie/scripts/view.ts | 20 +++++--- packages/trie/src/trie.ts | 11 ++--- packages/trie/src/util/asyncWalk.ts | 8 ++- packages/trie/test/official.spec.ts | 54 +++++++++++---------- packages/trie/test/proof/range.spec.ts | 24 ++++----- packages/trie/test/trie/secure.spec.ts | 30 +++++------- packages/trie/test/trie/trie.spec.ts | 34 +++---------- packages/trie/test/util/asyncWalk.spec.ts | 59 +++++++++++++---------- packages/vm/test/tester/index.ts | 1 - 12 files changed, 129 insertions(+), 136 deletions(-) diff --git a/packages/trie/README.md b/packages/trie/README.md index d20cceb06e..f723c2b255 100644 --- a/packages/trie/README.md +++ b/packages/trie/README.md @@ -175,11 +175,10 @@ As an example, to leverage `LevelDB` for all operations then you should create a ```ts // ./examples/customLevelDB.ts#L127-L131 - const trie = new Trie({ db: new LevelDB(new Level('MY_TRIE_DB_LOCATION') as any) }) - console.log(trie.database().db) // LevelDB { ... -} -void main() +const trie = new Trie({ db: new LevelDB(new Level('MY_TRIE_DB_LOCATION')) }) +console.log(trie.database().db) // LevelDB { ... +void main() ``` #### Node Deletion (Pruning) diff --git a/packages/trie/benchmarks/engines/level.ts b/packages/trie/benchmarks/engines/level.ts index 3f80ac704b..16f3f814ab 100644 --- a/packages/trie/benchmarks/engines/level.ts +++ b/packages/trie/benchmarks/engines/level.ts @@ -41,7 +41,7 @@ const getEncodings = (opts: EncodingOpts = {}) => { */ export class LevelDB< TKey extends Uint8Array | string = Uint8Array | string, - TValue extends Uint8Array | string | DBObject = Uint8Array | string | DBObject + TValue extends Uint8Array | string | DBObject = Uint8Array | string | DBObject, > implements DB { _leveldb: AbstractLevel @@ -52,7 +52,7 @@ export class LevelDB< * @param leveldb - An abstract-leveldown compliant store */ constructor( - leveldb?: AbstractLevel + leveldb?: AbstractLevel, ) { this._leveldb = leveldb ?? new MemoryLevel() } @@ -75,7 +75,6 @@ export class LevelDB< throw error } } - // eslint-disable-next-line if (value instanceof Buffer) value = Uint8Array.from(value) return value as TValue } @@ -99,7 +98,10 @@ export class LevelDB< * @inheritDoc */ async batch(opStack: BatchDBOp[]): Promise { - const levelOps = [] + const levelOps: { + keyEncoding: string + valueEncoding: string + }[] = [] for (const op of opStack) { const encodings = getEncodings(op.opts) levelOps.push({ ...op, ...encodings }) diff --git a/packages/trie/examples/customLevelDB.ts b/packages/trie/examples/customLevelDB.ts index 0ed657cb71..dbe92d8de6 100644 --- a/packages/trie/examples/customLevelDB.ts +++ b/packages/trie/examples/customLevelDB.ts @@ -101,7 +101,10 @@ export class LevelDB< * @inheritDoc */ async batch(opStack: BatchDBOp[]): Promise { - const levelOps = [] + const levelOps: { + keyEncoding: string + valueEncoding: string + }[] = [] for (const op of opStack) { const encodings = getEncodings(op.opts) levelOps.push({ ...op, ...encodings }) @@ -124,7 +127,7 @@ export class LevelDB< } async function main() { - const trie = new Trie({ db: new LevelDB(new Level('MY_TRIE_DB_LOCATION') as any) }) + const trie = new Trie({ db: new LevelDB(new Level('MY_TRIE_DB_LOCATION')) }) console.log(trie.database().db) // LevelDB { ... } void main() diff --git a/packages/trie/scripts/view.ts b/packages/trie/scripts/view.ts index ee4e48d487..9ce644f428 100644 --- a/packages/trie/scripts/view.ts +++ b/packages/trie/scripts/view.ts @@ -1,5 +1,11 @@ import { debug as _debug } from 'debug' -import { bytesToHex, equalsBytes, hexToBytes, utf8ToBytes } from '@ethereumjs/util' +import { + bytesToHex, + equalsBytes, + hexToBytes, + PrefixedHexString, + utf8ToBytes, +} from '@ethereumjs/util' import { BranchNode, ExtensionNode, LeafNode } from '../node/index.js' import { Trie } from '../trie.js' @@ -52,15 +58,15 @@ function getNodeType(node: TrieNode): TNode { function logNode(trie: Trie, node: TrieNode, currentKey: number[]): void { delimiter(3) const type = getNodeType(node) - if (equalsBytes((trie as any).hash(node.serialize()), trie.root())) { + if (equalsBytes(trie.hash(node.serialize()), trie.root())) { debugN('rt').extend(type)( - `{ 0x${bytesToHex((trie as any).hash(node.serialize())).slice( + `{ 0x${bytesToHex(trie.hash(node.serialize())).slice( 0, 12, )}... } ---- \uD83D\uDCA5 \u211B \u2134 \u2134 \u0164 \u0147 \u2134 \u0221 \u2211 \u2737`, ) } else { - debugN(type)(`{ 0x${bytesToHex((trie as any).hash(node.serialize())).slice(0, 12)}... } ----`) + debugN(type)(`{ 0x${bytesToHex(trie.hash(node.serialize())).slice(0, 12)}... } ----`) } debugT.extend('Walking')(`from [${currentKey}]`) if ('_nibbles' in node) { @@ -106,8 +112,10 @@ export const view = async (testName: string, inputs: any[], root: string) => { testKeys.set(bytesToHex(input[0]), input[1]) testStrings.set(bytesToHex(input[0]), stringPair) for await (const [key, _val] of testKeys.entries()) { - const retrieved = await trie.get(hexToBytes(key)) - debugT.extend('get')(`[${key}]: ${retrieved && equalsBytes(retrieved, hexToBytes(key))}`) + const retrieved = await trie.get(hexToBytes(key as PrefixedHexString)) + debugT.extend('get')( + `[${key}]: ${retrieved && equalsBytes(retrieved, hexToBytes(key as PrefixedHexString))}`, + ) } } debugT(bytesToHex(trie.root()), expect) diff --git a/packages/trie/src/trie.ts b/packages/trie/src/trie.ts index 27558d341c..4ab3ef7fb9 100644 --- a/packages/trie/src/trie.ts +++ b/packages/trie/src/trie.ts @@ -403,13 +403,11 @@ export class Trie { } } const startingNode = partialPath.stack[partialPath.stack.length - 1] - const start = startingNode !== undefined ? this.hash(startingNode?.serialize()) : this.root() + const start = startingNode !== undefined ? this.hash(startingNode.serialize()) : this.root() try { this.DEBUG && this.debug( - `Walking trie from ${startingNode === undefined ? 'ROOT' : 'NODE'}: ${bytesToHex( - start as Uint8Array, - )}`, + `Walking trie from ${startingNode === undefined ? 'ROOT' : 'NODE'}: ${bytesToHex(start)}`, ['FIND_PATH'], ) await this.walkTrie(start, onFound) @@ -651,8 +649,9 @@ export class Trie { if (branchNode instanceof BranchNode) { // create an extension node // branch->extension->branch - // @ts-ignore Why does this work, and why are we doing this? See issue https://github.com/ethereumjs/ethereumjs-monorepo/issues/3620 - const extensionNode = new ExtensionNode([branchKey], null) + // We push an extension value with a temporarily empty value to the stack. + // It will be replaced later on with the correct value in saveStack + const extensionNode = new ExtensionNode([branchKey], new Uint8Array()) stack.push(extensionNode) key.push(branchKey) } else { diff --git a/packages/trie/src/util/asyncWalk.ts b/packages/trie/src/util/asyncWalk.ts index 28e57586eb..c364c5537e 100644 --- a/packages/trie/src/util/asyncWalk.ts +++ b/packages/trie/src/util/asyncWalk.ts @@ -1,7 +1,5 @@ import { RLP } from '@ethereumjs/rlp' -import { equalsBytes } from '@ethereumjs/util' -// TODO: replace with bytesToHex from @ethereumjs/util -import { toHex } from 'ethereum-cryptography/utils.js' // eslint-disable-line +import { bytesToHex, equalsBytes } from '@ethereumjs/util' import { BranchNode } from '../node/branch.js' import { ExtensionNode } from '../node/extension.js' @@ -36,10 +34,10 @@ export async function* _walkTrie( } try { const node = await this.lookupNode(nodeHash) - if (node === undefined || visited.has(toHex(this.hash(node!.serialize())))) { + if (node === undefined || visited.has(bytesToHex(this.hash(node!.serialize())))) { return } - visited.add(toHex(this.hash(node!.serialize()))) + visited.add(bytesToHex(this.hash(node!.serialize()))) await onFound(node!, currentKey) if (await filter(node!, currentKey)) { yield { node: node!, currentKey } diff --git a/packages/trie/test/official.spec.ts b/packages/trie/test/official.spec.ts index a8581d4108..5431b4ac2a 100644 --- a/packages/trie/test/official.spec.ts +++ b/packages/trie/test/official.spec.ts @@ -8,21 +8,22 @@ import trieTests from './fixtures/trietest.json' describe('official tests', () => { it('should work', async () => { - const testNames = Object.keys(trieTests.tests) + const testNames = Object.keys(trieTests.tests) as (keyof typeof trieTests.tests)[] let trie = new Trie() for (const testName of testNames) { - const inputs = (trieTests as any).tests[testName].in - const expect = (trieTests as any).tests[testName].root + const inputs = trieTests.tests[testName].in + const expect = trieTests.tests[testName].root for (const input of inputs) { - for (let i = 0; i < 2; i++) { - if (typeof input[i] === 'string' && input[i].slice(0, 2) === '0x') { - input[i] = hexToBytes(input[i]) - } else if (typeof input[i] === 'string') { - input[i] = utf8ToBytes(input[i]) + const processedInput = input.map((item) => { + if (item === null) { + return item } - await trie.put(input[0], input[1]) - } + + return isHexString(item) ? hexToBytes(item) : utf8ToBytes(item) + }) as [Uint8Array, Uint8Array | null] + + await trie.put(processedInput[0], processedInput[1]) } assert.equal(bytesToHex(trie.root()), expect) trie = new Trie() @@ -32,28 +33,31 @@ describe('official tests', () => { describe('official tests any order', async () => { it('should work', async () => { - const testNames = Object.keys(trieAnyOrderTests.tests) + const testNames = Object.keys( + trieAnyOrderTests.tests, + ) as (keyof typeof trieAnyOrderTests.tests)[] let trie = new Trie() for (const testName of testNames) { - const test = (trieAnyOrderTests.tests as any)[testName] + const test = trieAnyOrderTests.tests[testName] const keys = Object.keys(test.in) - let key: any - for (key of keys) { - let val = test.in[key] - - if (typeof key === 'string' && isHexString(key)) { - key = hexToBytes(key) - } else if (typeof key === 'string') { - key = utf8ToBytes(key) + for (const stringKey of keys) { + const stringValue: string = test.in[stringKey as keyof typeof test.in] + let key: Uint8Array + let value: Uint8Array + + if (isHexString(stringKey)) { + key = hexToBytes(stringKey) + } else { + key = utf8ToBytes(stringKey) } - if (typeof val === 'string' && isHexString(val)) { - val = hexToBytes(val) - } else if (typeof val === 'string') { - val = utf8ToBytes(val) + if (isHexString(stringValue)) { + value = hexToBytes(stringValue) + } else { + value = utf8ToBytes(stringValue) } - await trie.put(key, val) + await trie.put(key, value) } assert.equal(bytesToHex(trie.root()), test.root) trie = new Trie() diff --git a/packages/trie/test/proof/range.spec.ts b/packages/trie/test/proof/range.spec.ts index ba6fde5d9c..94883f0b10 100644 --- a/packages/trie/test/proof/range.spec.ts +++ b/packages/trie/test/proof/range.spec.ts @@ -160,16 +160,16 @@ describe('simple merkle range proofs generation and verification', () => { try { await verify(trie, entries, start + 5, end, decreasedStartKey) assert.fail() - } catch (err) { - // ignore .. + } catch { + // Ignore } assert.equal(await verify(trie, entries, start, end, startKey, increasedEndKey), true) try { await verify(trie, entries, start, end + 5, startKey, increasedEndKey) assert.fail() - } catch (err) { - // ignore .. + } catch { + // Ignore } }) @@ -269,8 +269,8 @@ describe('simple merkle range proofs generation and verification', () => { try { await cb(trie, entries) result = true - } catch (err) { - // ignore + } catch { + // Ignore } assert.isFalse(result) } @@ -352,8 +352,8 @@ describe('simple merkle range proofs generation and verification', () => { targetRange.map(([, val]) => val), ) result = true - } catch (err) { - // ignore + } catch { + // Ignore } assert.isFalse(result) }) @@ -372,8 +372,8 @@ describe('simple merkle range proofs generation and verification', () => { try { await verify(trie, entries, start, end, decreasedStartKey, decreasedEndKey) result = true - } catch (err) { - // ignore + } catch { + // Ignore } assert.isFalse(result) @@ -384,8 +384,8 @@ describe('simple merkle range proofs generation and verification', () => { try { await verify(trie, entries, start, end, increasedStartKey, increasedEndKey) result = true - } catch (err) { - // ignore + } catch { + // Ignore } assert.isFalse(result) }) diff --git a/packages/trie/test/trie/secure.spec.ts b/packages/trie/test/trie/secure.spec.ts index 228f4d7b87..56af877cca 100644 --- a/packages/trie/test/trie/secure.spec.ts +++ b/packages/trie/test/trie/secure.spec.ts @@ -34,16 +34,16 @@ describe('SecureTrie', () => { it('copy trie (new key prefix / default 0 size cache)', async () => { const keyPrefix = hexToBytes('0x1234') const t = trie.shallowCopy(true, { keyPrefix }) - assert.ok(equalsBytes(t['_opts']['keyPrefix'] as Uint8Array, keyPrefix)) - assert.equal(t['_opts']['cacheSize'] as number, 0) - assert.equal(trie['_opts']['cacheSize'] as number, 0) + assert.ok(equalsBytes(t['_opts']['keyPrefix']!, keyPrefix)) + assert.equal(t['_opts']['cacheSize'], 0) + assert.equal(trie['_opts']['cacheSize'], 0) }) it('copy trie (new cache size)', async () => { const cacheSize = 1000 const t = trie.shallowCopy(true, { cacheSize }) - assert.equal(t['_opts']['cacheSize'] as number, cacheSize) - assert.equal(trie['_opts']['cacheSize'] as number, 0) + assert.equal(t['_opts']['cacheSize'], cacheSize) + assert.equal(trie['_opts']['cacheSize'], 0) }) }) @@ -79,10 +79,7 @@ describe('secure tests', () => { it('empty values', async () => { for (const row of secureTrieTests.tests.emptyValues.in) { - const val = - row[1] !== undefined && row[1] !== null - ? utf8ToBytes(row[1]) - : (null as unknown as Uint8Array) + const val = row[1] !== undefined && row[1] !== null ? utf8ToBytes(row[1]) : null await trie.put(utf8ToBytes(row[0]!), val) } assert.equal(bytesToHex(trie.root()), secureTrieTests.tests.emptyValues.root) @@ -91,27 +88,22 @@ describe('secure tests', () => { it('branchingTests', async () => { trie = new Trie({ useKeyHashing: true, db: new MapDB() }) for (const row of secureTrieTests.tests.branchingTests.in) { - const val = - row[1] !== undefined && row[1] !== null - ? utf8ToBytes(row[1]) - : (null as unknown as Uint8Array) + const val = row[1] !== undefined && row[1] !== null ? utf8ToBytes(row[1]) : null await trie.put(utf8ToBytes(row[0]!), val) } assert.equal(bytesToHex(trie.root()), secureTrieTests.tests.branchingTests.root) }) - /** - TODO: Fix this test it('jeff', async () => { for (const row of secureTrieTests.tests.jeff.in) { - let val = row[1] + let val: string | null | Uint8Array = row[1] if (val !== undefined && val !== null) { - val = hexToBytes(row[1].slice(2)) + val = hexToBytes(`0x${row[1]!.slice(2)}`) } - await trie.put(hexToBytes(row[0].slice(2)), val) + await trie.put(hexToBytes(`0x${row[0]!.slice(2)}`), val) } assert.equal(bytesToHex(trie.root()), secureTrieTests.tests.jeff.root) - })*/ + }) it('put fails if the key is the ROOT_DB_KEY', async () => { const trie = new Trie({ useKeyHashing: true, db: new MapDB(), useRootPersistence: true }) diff --git a/packages/trie/test/trie/trie.spec.ts b/packages/trie/test/trie/trie.spec.ts index b74d13d717..f719b0195f 100644 --- a/packages/trie/test/trie/trie.spec.ts +++ b/packages/trie/test/trie/trie.spec.ts @@ -45,42 +45,24 @@ for (const { constructor, defaults, title } of [ describe(`${title} (Persistence)`, () => { it('creates an instance via createTrie and defaults to `false` with a database', async () => { - // TODO: check this test - assert.isUndefined( - ((await createTrie({ ...defaults, db: new MapDB() })) as any)._useRootPersistence, + assert.isFalse( + (await createTrie({ ...defaults, db: new MapDB() }))['_opts'].useRootPersistence, ) }) - it('creates an instance via createTrie and respects the `useRootPersistence` option with a database', async () => { - // TODO: check this test - assert.isUndefined( - ( - (await createTrie({ - ...defaults, - db: new MapDB(), - useRootPersistence: false, - })) as any - )._useRootPersistence, - ) + it('creates an instance via createTrie and defaults to `false` without a database', async () => { + assert.isFalse((await createTrie({ ...defaults }))['_opts'].useRootPersistence) }) it('creates an instance via createTrie and respects the `useRootPersistence` option with a database', async () => { - // TODO: check this test - assert.isUndefined( + assert.isFalse( ( - (await createTrie({ + await createTrie({ ...defaults, db: new MapDB(), useRootPersistence: false, - })) as any - )._useRootPersistence, - ) - }) - - it('creates an instance via createTrie and defaults to `false` without a database', async () => { - // TODO: check this test - assert.isUndefined( - ((await createTrie({ ...defaults, db: new MapDB() })) as any)._useRootPersistence, + }) + )['_opts'].useRootPersistence, ) }) diff --git a/packages/trie/test/util/asyncWalk.spec.ts b/packages/trie/test/util/asyncWalk.spec.ts index 7b875625bf..83ae68bac7 100644 --- a/packages/trie/test/util/asyncWalk.spec.ts +++ b/packages/trie/test/util/asyncWalk.spec.ts @@ -1,4 +1,4 @@ -import { bytesToHex, equalsBytes, hexToBytes, utf8ToBytes } from '@ethereumjs/util' +import { bytesToHex, equalsBytes, hexToBytes, isHexString, utf8ToBytes } from '@ethereumjs/util' import { assert, describe, it } from 'vitest' import { @@ -15,27 +15,28 @@ import trieTests from '../fixtures/trietest.json' import type { PrefixedHexString } from '@ethereumjs/util' describe('walk the tries from official tests', async () => { - const testNames = Object.keys(trieTests.tests) + const testNames = Object.keys(trieTests.tests) as (keyof typeof trieTests.tests)[] for await (const testName of testNames) { const trie = new Trie() describe(testName, async () => { - const inputs = (trieTests as any).tests[testName].in - const expect = (trieTests as any).tests[testName].root + const inputs = trieTests.tests[testName].in + const expect = trieTests.tests[testName].root const testKeys: Map = new Map() const testStrings: Map = new Map() for await (const [idx, input] of inputs.entries()) { - const stringPair: [string, string] = [inputs[idx][0], inputs[idx][1] ?? 'null'] + const stringPair: [string, string] = [inputs[idx][0]!, inputs[idx][1] ?? 'null'] describe(`put: ${stringPair}`, async () => { - for (let i = 0; i < 2; i++) { - if (typeof input[i] === 'string' && input[i].slice(0, 2) === '0x') { - input[i] = hexToBytes(input[i]) - } else if (typeof input[i] === 'string') { - input[i] = utf8ToBytes(input[i]) + const processedInput = input.map((item) => { + if (item === null) { + return null } - } + + return isHexString(item) ? hexToBytes(item) : utf8ToBytes(item) + }) as [Uint8Array, Uint8Array | null] + try { - await trie.put(input[0], input[1]) + await trie.put(processedInput[0], processedInput[1]) assert(true) } catch (e) { assert(false, (e as any).message) @@ -43,8 +44,8 @@ describe('walk the tries from official tests', async () => { trie.checkpoint() await trie.commit() trie.flushCheckpoints() - testKeys.set(bytesToHex(input[0]), input[1]) - testStrings.set(bytesToHex(input[0]), stringPair) + testKeys.set(bytesToHex(processedInput[0]), processedInput[1]) + testStrings.set(bytesToHex(processedInput[0]), stringPair) describe(`should get all keys`, async () => { for await (const [key, val] of testKeys.entries()) { const retrieved = await trie.get(hexToBytes(key)) @@ -64,26 +65,28 @@ describe('walk the tries from official tests', async () => { describe('walk a sparse trie', async () => { const trie = new Trie() - const inputs = (trieTests as any).tests.jeff.in - const expect = (trieTests as any).tests.jeff.root + const inputs = trieTests.tests.jeff.in + const expect = trieTests.tests.jeff.root // Build a Trie for await (const input of inputs) { - for (let i = 0; i < 2; i++) { - if (typeof input[i] === 'string' && input[i].slice(0, 2) === '0x') { - input[i] = hexToBytes(input[i]) - } else if (typeof input[i] === 'string') { - input[i] = utf8ToBytes(input[i]) + const processedInput = input.map((item) => { + if (item === null) { + return item } - } - await trie.put(input[0], input[1]) + + return isHexString(item) ? hexToBytes(item) : utf8ToBytes(item) + }) as [Uint8Array, Uint8Array | null] + + await trie.put(processedInput[0], processedInput[1]) } // Check the root it(`should have root ${expect}`, async () => { assert.equal(bytesToHex(trie.root()), expect) }) // Generate a proof for inputs[0] - const proofKey = inputs[0][0] + const rawProofKey = inputs[0][0] as string + const proofKey = isHexString(rawProofKey) ? hexToBytes(rawProofKey) : utf8ToBytes(rawProofKey) const proof = await createMerkleProof(trie, proofKey) assert.ok(await verifyTrieProof(proofKey, proof)) @@ -102,7 +105,11 @@ describe('walk a sparse trie', async () => { // The only leaf node should be leaf from the proof const fullKeyNibbles = [...currentKey, ...node._nibbles] assert.deepEqual(fullKeyNibbles, bytesToNibbles(proofKey)) - assert.deepEqual(node.value(), inputs[0][1]) + const rawNodeValue = inputs[0][1] as string + const nodeValue = isHexString(rawNodeValue) + ? hexToBytes(rawNodeValue) + : utf8ToBytes(rawNodeValue) + assert.deepEqual(node.value(), nodeValue) } // Count the nodes...nodes from the proof should be only nodes in the trie found++ @@ -112,7 +119,7 @@ describe('walk a sparse trie', async () => { // Walk the same sparse trie with WalkController try { - await fromProof.walkTrie(fromProof.root(), async (noderef, node, key, wc) => { + await fromProof.walkTrie(fromProof.root(), async (_, node, __, wc) => { wc.allChildren(node!) }) assert.fail('Will throw when it meets a missing node in a sparse trie') diff --git a/packages/vm/test/tester/index.ts b/packages/vm/test/tester/index.ts index 13061b2e60..20479a2fb7 100755 --- a/packages/vm/test/tester/index.ts +++ b/packages/vm/test/tester/index.ts @@ -52,7 +52,6 @@ import type { EVMBLSInterface, EVMBN254Interface } from '@ethereumjs/evm' * --profile If this flag is passed, the state/blockchain tests will profile */ -//@ts-expect-error Typescript thinks there isn't a default export on minimist but there is const argv = minimist.default(process.argv.slice(2)) async function runTests() { From db0e8b2d20937e2ece37e701f577e0787df71407 Mon Sep 17 00:00:00 2001 From: Jochem Brouwer Date: Mon, 2 Sep 2024 19:11:18 +0200 Subject: [PATCH 2/3] VM: exit early on non-existing system contracts (#3614) * VM: exit early on non-existing system contracts * correctly exit early on requests * vm: correctly exit early on system contracts, un-bumping system account nonce * Merge branch 'master' into early-fail-system --- packages/vm/src/requests.ts | 56 ++++++++++++++++--------------------- 1 file changed, 24 insertions(+), 32 deletions(-) diff --git a/packages/vm/src/requests.ts b/packages/vm/src/requests.ts index 781ac01575..54230e18bc 100644 --- a/packages/vm/src/requests.ts +++ b/packages/vm/src/requests.ts @@ -66,18 +66,15 @@ const accumulateEIP7002Requests = async ( ) const withdrawalsAddress = createAddressFromString(bytesToHex(addressBytes)) - const code = await vm.stateManager.getCode(withdrawalsAddress) - - if (code.length === 0) { - throw new Error( - 'Attempt to accumulate EIP-7002 requests failed: the contract does not exist. Ensure the deployment tx has been run, or that the required contract code is stored', - ) - } - const systemAddressBytes = bigIntToAddressBytes(vm.common.param('systemAddress')) const systemAddress = createAddressFromString(bytesToHex(systemAddressBytes)) + const systemAccount = await vm.stateManager.getAccount(systemAddress) + + const originalAccount = await vm.stateManager.getAccount(withdrawalsAddress) - const originalAccount = await vm.stateManager.getAccount(systemAddress) + if (originalAccount === undefined) { + return + } const results = await vm.evm.runCall({ caller: systemAddress, @@ -85,6 +82,12 @@ const accumulateEIP7002Requests = async ( to: withdrawalsAddress, }) + if (systemAccount === undefined) { + await vm.stateManager.deleteAccount(systemAddress) + } else { + await vm.stateManager.putAccount(systemAddress, systemAccount) + } + const resultsBytes = results.execResult.returnValue if (resultsBytes.length > 0) { // Each request is 76 bytes @@ -96,13 +99,6 @@ const accumulateEIP7002Requests = async ( requests.push(createWithdrawalRequest({ sourceAddress, validatorPubkey, amount })) } } - - if (originalAccount === undefined) { - await vm.stateManager.deleteAccount(systemAddress) - } else { - // Restore the original account (the `runCall` updates the nonce) - await vm.stateManager.putAccount(systemAddress, originalAccount) - } } const accumulateEIP7251Requests = async ( @@ -116,18 +112,15 @@ const accumulateEIP7251Requests = async ( ) const consolidationsAddress = createAddressFromString(bytesToHex(addressBytes)) - const code = await vm.stateManager.getCode(consolidationsAddress) - - if (code.length === 0) { - throw new Error( - 'Attempt to accumulate EIP-7251 requests failed: the contract does not exist. Ensure the deployment tx has been run, or that the required contract code is stored', - ) - } - const systemAddressBytes = bigIntToAddressBytes(vm.common.param('systemAddress')) const systemAddress = createAddressFromString(bytesToHex(systemAddressBytes)) + const systemAccount = await vm.stateManager.getAccount(systemAddress) + + const originalAccount = await vm.stateManager.getAccount(consolidationsAddress) - const originalAccount = await vm.stateManager.getAccount(systemAddress) + if (originalAccount === undefined) { + return + } const results = await vm.evm.runCall({ caller: systemAddress, @@ -135,6 +128,12 @@ const accumulateEIP7251Requests = async ( to: consolidationsAddress, }) + if (systemAccount === undefined) { + await vm.stateManager.deleteAccount(systemAddress) + } else { + await vm.stateManager.putAccount(systemAddress, systemAccount) + } + const resultsBytes = results.execResult.returnValue if (resultsBytes.length > 0) { // Each request is 116 bytes @@ -146,13 +145,6 @@ const accumulateEIP7251Requests = async ( requests.push(createConsolidationRequest({ sourceAddress, sourcePubkey, targetPubkey })) } } - - if (originalAccount === undefined) { - await vm.stateManager.deleteAccount(systemAddress) - } else { - // Restore the original account (the `runCall` updates the nonce) - await vm.stateManager.putAccount(systemAddress, originalAccount) - } } const accumulateDeposits = async ( From b91b545d8f672e80c2d9f7a793fba75040ad7b2d Mon Sep 17 00:00:00 2001 From: Jochem Brouwer Date: Mon, 2 Sep 2024 19:30:52 +0200 Subject: [PATCH 3/3] Ensure TransientStorage cleanups after Tx-level contract creation (#3625) * evm: fix bug not clearing transient storage on tx-level create * evm: add tests for tstore cleanup * make cspell happy --------- Co-authored-by: acolytec3 <17355484+acolytec3@users.noreply.github.com> --- packages/evm/src/evm.ts | 4 ++ packages/evm/test/transientStorage.spec.ts | 44 +++++++++++++++++++++- 2 files changed, 47 insertions(+), 1 deletion(-) diff --git a/packages/evm/src/evm.ts b/packages/evm/src/evm.ts index dd321bcd9a..067c01ff36 100644 --- a/packages/evm/src/evm.ts +++ b/packages/evm/src/evm.ts @@ -704,6 +704,10 @@ export class EVM implements EVMInterface { } } + if (message.depth === 0) { + this.postMessageCleanup() + } + return { createdAddress: message.to, execResult: result, diff --git a/packages/evm/test/transientStorage.spec.ts b/packages/evm/test/transientStorage.spec.ts index acf90275c1..b641370b1e 100644 --- a/packages/evm/test/transientStorage.spec.ts +++ b/packages/evm/test/transientStorage.spec.ts @@ -1,6 +1,14 @@ -import { createAddressFromString } from '@ethereumjs/util' +import { + createAddressFromString, + createZeroAddress, + equalsBytes, + hexToBytes, + setLengthLeft, + unpadBytes, +} from '@ethereumjs/util' import { assert, describe, it } from 'vitest' +import { createEVM } from '../src/index.js' import { TransientStorage } from '../src/transientStorage.js' describe('Transient Storage', () => { @@ -176,4 +184,38 @@ describe('Transient Storage', () => { transientStorage.revert() assert.deepEqual(transientStorage.get(address, key), value1) }) + + it('should cleanup after a message create', async () => { + const evm = await createEVM() + // PUSH 1 PUSH 1 TSTORE + const code = hexToBytes('0x600160015D') + const keyBuf = setLengthLeft(new Uint8Array([1]), 32) + const result = await evm.runCall({ + data: code, + gasLimit: BigInt(100_000), + }) + const created = result.createdAddress! + const stored = evm.transientStorage.get(created, keyBuf) + assert.ok( + equalsBytes(unpadBytes(stored), new Uint8Array()), + 'Transient storage has been cleared', + ) + }) + + it('should cleanup after a message call', async () => { + const evm = await createEVM() + const contractAddress = createZeroAddress() + // PUSH 1 PUSH 1 TSTORE + const code = hexToBytes('0x600160015D') + await evm.stateManager.putCode(contractAddress, code) + const keyBuf = setLengthLeft(new Uint8Array([1]), 32) + await evm.runCall({ + gasLimit: BigInt(100_000), + }) + const stored = evm.transientStorage.get(contractAddress, keyBuf) + assert.ok( + equalsBytes(unpadBytes(stored), new Uint8Array()), + 'Transient storage has been cleared', + ) + }) })