From 2e785fa79d0cca07ebf4654965eec09149561cdc Mon Sep 17 00:00:00 2001 From: Jochem Brouwer Date: Thu, 22 Jun 2023 19:19:02 +0200 Subject: [PATCH] VM/EVM: move selfdestruct type to set (#2806) * vm/evm: move selfdestruct type to set * evm: selfdestruct updates * vm: fix test runner * vm: fix setting hardfork correctly --- packages/evm/src/evm.ts | 14 ++++---- packages/evm/src/interpreter.ts | 34 ++++++++++++++----- packages/evm/src/message.ts | 9 +++-- packages/evm/src/types.ts | 8 ++--- packages/vm/src/runTx.ts | 5 ++- .../tester/runners/BlockchainTestsRunner.ts | 2 +- 6 files changed, 43 insertions(+), 29 deletions(-) diff --git a/packages/evm/src/evm.ts b/packages/evm/src/evm.ts index d3207a6112..872fc3b3ad 100644 --- a/packages/evm/src/evm.ts +++ b/packages/evm/src/evm.ts @@ -716,7 +716,7 @@ export class EVM implements EVMInterface { this.journal ) if (message.selfdestruct) { - interpreter._result.selfdestruct = message.selfdestruct as { [key: string]: Uint8Array } + interpreter._result.selfdestruct = message.selfdestruct } if (message.createdAddresses) { interpreter._result.createdAddresses = message.createdAddresses @@ -738,7 +738,7 @@ export class EVM implements EVMInterface { result = { ...result, logs: [], - selfdestruct: {}, + selfdestruct: new Set(), createdAddresses: new Set(), } } @@ -798,7 +798,7 @@ export class EVM implements EVMInterface { isCompiled: opts.isCompiled, isStatic: opts.isStatic, salt: opts.salt, - selfdestruct: opts.selfdestruct ?? {}, + selfdestruct: opts.selfdestruct ?? new Set(), createdAddresses: opts.createdAddresses ?? new Set(), delegatecall: opts.delegatecall, versionedHashes: opts.versionedHashes, @@ -868,7 +868,7 @@ export class EVM implements EVMInterface { // (this only happens the Frontier/Chainstart fork) // then the error is dismissed if (err && err.error !== ERROR.CODESTORE_OUT_OF_GAS) { - result.execResult.selfdestruct = {} + result.execResult.selfdestruct = new Set() result.execResult.createdAddresses = new Set() result.execResult.gasRefund = BigInt(0) } @@ -914,7 +914,7 @@ export class EVM implements EVMInterface { caller: opts.caller, value: opts.value, depth: opts.depth, - selfdestruct: opts.selfdestruct ?? {}, + selfdestruct: opts.selfdestruct ?? new Set(), isStatic: opts.isStatic, versionedHashes: opts.versionedHashes, }) @@ -1073,9 +1073,9 @@ export interface ExecResult { */ logs?: Log[] /** - * A map from the accounts that have self-destructed to the addresses to send their funds to + * A set of accounts to selfdestruct */ - selfdestruct?: { [k: string]: Uint8Array } + selfdestruct?: Set /** * Map of addresses which were created (used in EIP 6780) */ diff --git a/packages/evm/src/interpreter.ts b/packages/evm/src/interpreter.ts index 8ebd053769..07f93a7b1b 100644 --- a/packages/evm/src/interpreter.ts +++ b/packages/evm/src/interpreter.ts @@ -29,9 +29,9 @@ export interface RunResult { logs: Log[] returnValue?: Uint8Array /** - * A map from the accounts that have self-destructed to the addresses to send their funds to + * A set of accounts to selfdestruct */ - selfdestruct: { [k: string]: Uint8Array } + selfdestruct: Set /** * A map which tracks which addresses were created (used in EIP 6780) @@ -160,7 +160,7 @@ export class Interpreter { this._result = { logs: [], returnValue: undefined, - selfdestruct: {}, + selfdestruct: new Set(), } } @@ -840,7 +840,7 @@ export class Interpreter { } async _baseCall(msg: Message): Promise { - const selfdestruct = { ...this._result.selfdestruct } + const selfdestruct = new Set(this._result.selfdestruct) msg.selfdestruct = selfdestruct msg.gasRefund = this._runState.gasRefund @@ -882,7 +882,9 @@ export class Interpreter { } if (!results.execResult.exceptionError) { - Object.assign(this._result.selfdestruct, selfdestruct) + for (const addressToSelfdestructHex of selfdestruct) { + this._result.selfdestruct.add(addressToSelfdestructHex) + } if (this._common.isActivatedEIP(6780)) { // copy over the items to result via iterator for (const item of createdAddresses!) { @@ -910,7 +912,7 @@ export class Interpreter { data: Uint8Array, salt?: Uint8Array ): Promise { - const selfdestruct = { ...this._result.selfdestruct } + const selfdestruct = new Set(this._result.selfdestruct) const caller = this._env.address const depth = this._env.depth + 1 @@ -954,6 +956,12 @@ export class Interpreter { versionedHashes: this._env.versionedHashes, }) + let createdAddresses: Set + if (this._common.isActivatedEIP(6780)) { + createdAddresses = new Set(this._result.createdAddresses) + message.createdAddresses = createdAddresses + } + const results = await this._evm.runCall({ message }) if (results.execResult.logs) { @@ -975,7 +983,15 @@ export class Interpreter { !results.execResult.exceptionError || results.execResult.exceptionError.error === ERROR.CODESTORE_OUT_OF_GAS ) { - Object.assign(this._result.selfdestruct, selfdestruct) + for (const addressToSelfdestructHex of selfdestruct) { + this._result.selfdestruct.add(addressToSelfdestructHex) + } + if (this._common.isActivatedEIP(6780)) { + // copy over the items to result via iterator + for (const item of createdAddresses!) { + this._result.createdAddresses!.add(item) + } + } // update stateRoot on current contract const account = await this._stateManager.getAccount(this._env.address) if (!account) { @@ -1017,11 +1033,11 @@ export class Interpreter { async _selfDestruct(toAddress: Address): Promise { // only add to refund if this is the first selfdestruct for the address - if (this._result.selfdestruct[bytesToHex(this._env.address.bytes)] === undefined) { + if (!this._result.selfdestruct.has(bytesToHex(this._env.address.bytes))) { this.refundGas(this._common.param('gasPrices', 'selfdestructRefund')) } - this._result.selfdestruct[bytesToHex(this._env.address.bytes)] = toAddress.bytes + this._result.selfdestruct.add(bytesToHex(this._env.address.bytes)) // Add to beneficiary balance let toAccount = await this._stateManager.getAccount(toAddress) diff --git a/packages/evm/src/message.ts b/packages/evm/src/message.ts index 209c159b4c..20ac31092e 100644 --- a/packages/evm/src/message.ts +++ b/packages/evm/src/message.ts @@ -26,9 +26,9 @@ interface MessageOpts { isCompiled?: boolean salt?: Uint8Array /** - * A map of addresses to selfdestruct, see {@link Message.selfdestruct} + * A set of addresses to selfdestruct, see {@link Message.selfdestruct} */ - selfdestruct?: { [key: string]: boolean } | { [key: string]: Uint8Array } + selfdestruct?: Set /** * Map of addresses which were created (used in EIP 6780) */ @@ -53,10 +53,9 @@ export class Message { salt?: Uint8Array containerCode?: Uint8Array /** container code for EOF1 contracts - used by CODECOPY/CODESIZE */ /** - * Map of addresses to selfdestruct. Key is the unprefixed address. - * Value is a boolean when marked for destruction and replaced with a Uint8Array containing the address where the remaining funds are sent. + * Set of addresses to selfdestruct. Key is the unprefixed address. */ - selfdestruct?: { [key: string]: boolean } | { [key: string]: Uint8Array } + selfdestruct?: Set /** * Map of addresses which were created (used in EIP 6780) */ diff --git a/packages/evm/src/types.ts b/packages/evm/src/types.ts index 4cd3670ea4..3a3d4f9642 100644 --- a/packages/evm/src/types.ts +++ b/packages/evm/src/types.ts @@ -92,9 +92,9 @@ export interface EVMRunCallOpts { */ salt?: Uint8Array /** - * Addresses to selfdestruct. Defaults to none. + * Addresses to selfdestruct. Defaults to the empty set. */ - selfdestruct?: { [k: string]: boolean } + selfdestruct?: Set /** * Created addresses in current context. Used in EIP 6780 */ @@ -171,9 +171,9 @@ export interface EVMRunCodeOpts { */ isStatic?: boolean /** - * Addresses to selfdestruct. Defaults to none. + * Addresses to selfdestruct. Defaults to the empty set. */ - selfdestruct?: { [k: string]: boolean } + selfdestruct?: Set /** * The address of the account that is executing this code (`address(this)`). Defaults to the zero address. */ diff --git a/packages/vm/src/runTx.ts b/packages/vm/src/runTx.ts index 9da66cf91a..6723c2264d 100644 --- a/packages/vm/src/runTx.ts +++ b/packages/vm/src/runTx.ts @@ -503,9 +503,8 @@ async function _runTx(this: VM, opts: RunTxOpts): Promise { * Cleanup accounts */ if (results.execResult.selfdestruct !== undefined) { - const keys = Object.keys(results.execResult.selfdestruct) - for (const k of keys) { - const address = new Address(hexToBytes(k)) + for (const addressToSelfdestructHex of results.execResult.selfdestruct) { + const address = new Address(hexToBytes(addressToSelfdestructHex)) if (this._common.isActivatedEIP(6780)) { // skip cleanup of addresses not in createdAddresses if (!results.execResult.createdAddresses!.has(address.toString())) { diff --git a/packages/vm/test/tester/runners/BlockchainTestsRunner.ts b/packages/vm/test/tester/runners/BlockchainTestsRunner.ts index 017a1f0658..eb863b6b28 100644 --- a/packages/vm/test/tester/runners/BlockchainTestsRunner.ts +++ b/packages/vm/test/tester/runners/BlockchainTestsRunner.ts @@ -181,7 +181,7 @@ export async function runBlockchainTest(options: any, testData: any, t: tape.Tes await blockBuilder.revert() // will only revert if checkpointed } - const block = Block.fromRLPSerializedBlock(blockRlp, { common }) + const block = Block.fromRLPSerializedBlock(blockRlp, { common, setHardfork: TD }) await blockchain.putBlock(block) // This is a trick to avoid generating the canonical genesis