From 39c18b133b57469c157d97f5cfefafa52b89bd12 Mon Sep 17 00:00:00 2001 From: emersonmacro <77563348+emersonmacro@users.noreply.github.com> Date: Tue, 27 Jul 2021 12:55:40 -0700 Subject: [PATCH] VM: stateManager -> add modifyAccountFields method (#1369) --- packages/vm/CHANGELOG.md | 2 + packages/vm/src/evm/eei.ts | 6 +- packages/vm/src/state/interface.ts | 3 + packages/vm/src/state/stateManager.ts | 23 ++++- .../tests/api/EIPs/eip-1559-FeeMarket.spec.ts | 24 ++--- .../tests/api/EIPs/eip-3198-BaseFee.spec.ts | 4 +- .../vm/tests/api/state/stateManager.spec.ts | 87 +++++++++++++++++++ 7 files changed, 122 insertions(+), 27 deletions(-) diff --git a/packages/vm/CHANGELOG.md b/packages/vm/CHANGELOG.md index 77efb6eea3..7ce23d5362 100644 --- a/packages/vm/CHANGELOG.md +++ b/packages/vm/CHANGELOG.md @@ -10,6 +10,8 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/) **New Features** +- StateManager: Added `modifyAccountFields` method to simplify the `getAccount` -> modify fields -> `putAccount` pattern, PR [#1369](https://github.com/ethereumjs/ethereumjs-monorepo/pull/1369) + **Bug Fixes** - Fix EIP1559 bug to include tx value in balance check, fix nonce check, PR [#1372](https://github.com/ethereumjs/ethereumjs-monorepo/pull/1372) diff --git a/packages/vm/src/evm/eei.ts b/packages/vm/src/evm/eei.ts index 0027b32bed..bf6afdd3a7 100644 --- a/packages/vm/src/evm/eei.ts +++ b/packages/vm/src/evm/eei.ts @@ -423,9 +423,9 @@ export default class EEI { await this._state.putAccount(toAddress, toAccount) // Subtract from contract balance - const account = await this._state.getAccount(this._env.address) - account.balance = new BN(0) - await this._state.putAccount(this._env.address, account) + await this._state.modifyAccountFields(this._env.address, { + balance: new BN(0), + }) trap(ERROR.STOP) } diff --git a/packages/vm/src/state/interface.ts b/packages/vm/src/state/interface.ts index 6cff8e10a9..e6e740e195 100644 --- a/packages/vm/src/state/interface.ts +++ b/packages/vm/src/state/interface.ts @@ -8,12 +8,15 @@ export interface StorageDump { [key: string]: string } +export type AccountFields = Partial> + export interface StateManager { copy(): StateManager getAccount(address: Address): Promise putAccount(address: Address, account: Account): Promise deleteAccount(address: Address): Promise touchAccount(address: Address): void + modifyAccountFields(address: Address, accountFields: AccountFields): Promise putContractCode(address: Address, value: Buffer): Promise getContractCode(address: Address): Promise getContractStorage(address: Address, key: Buffer): Promise diff --git a/packages/vm/src/state/stateManager.ts b/packages/vm/src/state/stateManager.ts index 8b0242d36f..4dbdbd4688 100644 --- a/packages/vm/src/state/stateManager.ts +++ b/packages/vm/src/state/stateManager.ts @@ -13,7 +13,7 @@ import { import { encode, decode } from 'rlp' import Common, { Chain, Hardfork } from '@ethereumjs/common' import { genesisStateByName } from '@ethereumjs/common/dist/genesisStates' -import { StateManager, StorageDump } from './interface' +import { AccountFields, StateManager, StorageDump } from './interface' import Cache from './cache' import { getActivePrecompiles, ripemdPrecompileAddress } from '../evm/precompiles' import { short } from '../evm/opcodes' @@ -157,11 +157,28 @@ export default class DefaultStateManager implements StateManager { * This happens when the account is triggered for a state-changing * event. Touched accounts that are empty will be cleared * at the end of the tx. + * @param address - Address of the account to touch */ touchAccount(address: Address): void { this._touched.add(address.buf.toString('hex')) } + /** + * Gets the account associated with `address`, modifies the given account + * fields, then saves the account into state. Account fields can include + * `nonce`, `balance`, `stateRoot`, and `codeHash`. + * @param address - Address of the account to modify + * @param accountFields - Object containing account fields and values to modify + */ + async modifyAccountFields(address: Address, accountFields: AccountFields): Promise { + const account = await this.getAccount(address) + account.nonce = accountFields.nonce ?? account.nonce + account.balance = accountFields.balance ?? account.balance + account.stateRoot = accountFields.stateRoot ?? account.stateRoot + account.codeHash = accountFields.codeHash ?? account.codeHash + await this.putAccount(address, account) + } + /** * Adds `value` to the state trie as code, and sets `codeHash` on the account * corresponding to `address` to reference this. @@ -177,12 +194,10 @@ export default class DefaultStateManager implements StateManager { await this._trie.db.put(codeHash, value) - const account = await this.getAccount(address) if (this.DEBUG) { debug(`Update codeHash (-> ${short(codeHash)}) for account ${address}`) } - account.codeHash = codeHash - await this.putAccount(address, account) + await this.modifyAccountFields(address, { codeHash }) } /** diff --git a/packages/vm/tests/api/EIPs/eip-1559-FeeMarket.spec.ts b/packages/vm/tests/api/EIPs/eip-1559-FeeMarket.spec.ts index 96d9236c97..ce8374cbb5 100644 --- a/packages/vm/tests/api/EIPs/eip-1559-FeeMarket.spec.ts +++ b/packages/vm/tests/api/EIPs/eip-1559-FeeMarket.spec.ts @@ -74,10 +74,8 @@ tape('EIP1559 tests', (t) => { ) const block = makeBlock(GWEI, tx, 2) const vm = new VM({ common }) - let account = await vm.stateManager.getAccount(sender) const balance = GWEI.muln(21000).muln(10) - account.balance = balance - await vm.stateManager.putAccount(sender, account) + await vm.stateManager.modifyAccountFields(sender, { balance }) const results = await vm.runTx({ tx: block.transactions[0], block, @@ -96,7 +94,7 @@ tape('EIP1559 tests', (t) => { let miner = await vm.stateManager.getAccount(coinbase) st.ok(miner.balance.eq(expectedMinerBalance), 'miner balance correct') - account = await vm.stateManager.getAccount(sender) + let account = await vm.stateManager.getAccount(sender) st.ok(account.balance.eq(expectedAccountBalance), 'account balance correct') st.ok(results.amountSpent.eq(expectedCost), 'reported cost correct') @@ -109,11 +107,8 @@ tape('EIP1559 tests', (t) => { { common } ) const block2 = makeBlock(GWEI, tx2, 1) - account = await vm.stateManager.getAccount(sender) - account.balance = balance - await vm.stateManager.putAccount(sender, account) - miner.balance = new BN(0) - await vm.stateManager.putAccount(coinbase, miner) + await vm.stateManager.modifyAccountFields(sender, { balance }) + await vm.stateManager.modifyAccountFields(coinbase, { balance: new BN(0) }) const results2 = await vm.runTx({ tx: block2.transactions[0], block: block2, @@ -140,11 +135,8 @@ tape('EIP1559 tests', (t) => { { common } ) const block3 = makeBlock(GWEI, tx3, 0) - account = await vm.stateManager.getAccount(sender) - account.balance = balance - await vm.stateManager.putAccount(sender, account) - miner.balance = new BN(0) - await vm.stateManager.putAccount(coinbase, miner) + await vm.stateManager.modifyAccountFields(sender, { balance }) + await vm.stateManager.modifyAccountFields(coinbase, { balance: new BN(0) }) const results3 = await vm.runTx({ tx: block3.transactions[0], block: block3, @@ -180,10 +172,8 @@ tape('EIP1559 tests', (t) => { ) const block = makeBlock(GWEI, tx, 2) const vm = new VM({ common }) - const account = await vm.stateManager.getAccount(sender) const balance = GWEI.muln(210000).muln(10) - account.balance = balance - await vm.stateManager.putAccount(sender, account) + await vm.stateManager.modifyAccountFields(sender, { balance }) /** * GASPRICE diff --git a/packages/vm/tests/api/EIPs/eip-3198-BaseFee.spec.ts b/packages/vm/tests/api/EIPs/eip-3198-BaseFee.spec.ts index d6c1e883a3..a0242e98ba 100644 --- a/packages/vm/tests/api/EIPs/eip-3198-BaseFee.spec.ts +++ b/packages/vm/tests/api/EIPs/eip-3198-BaseFee.spec.ts @@ -74,9 +74,7 @@ tape('EIP3198 tests', (t) => { ) const block = makeBlock(fee, tx, 2) const vm = new VM({ common }) - const account = await vm.stateManager.getAccount(sender) - account.balance = ETHER - await vm.stateManager.putAccount(sender, account) + await vm.stateManager.modifyAccountFields(sender, { balance: ETHER }) // Track stack diff --git a/packages/vm/tests/api/state/stateManager.spec.ts b/packages/vm/tests/api/state/stateManager.spec.ts index e4611f0417..dfd6d8d8e3 100644 --- a/packages/vm/tests/api/state/stateManager.spec.ts +++ b/packages/vm/tests/api/state/stateManager.spec.ts @@ -168,6 +168,93 @@ tape('StateManager', (t) => { st.end() } ) + + t.test('should modify account fields correctly', async (st) => { + const stateManager = new DefaultStateManager() + const account = createAccount() + const address = new Address(Buffer.from('a94f5374fce5edbc8e2a8697c15331677e6ebf0b', 'hex')) + await stateManager.putAccount(address, account) + + await stateManager.modifyAccountFields(address, { balance: new BN(1234) }) + + const res1 = await stateManager.getAccount(address) + + st.equal(res1.balance.toString('hex'), '4d2') + + await stateManager.modifyAccountFields(address, { nonce: new BN(1) }) + + const res2 = await stateManager.getAccount(address) + + st.equal(res2.nonce.toNumber(), 1) + + await stateManager.modifyAccountFields(address, { + codeHash: Buffer.from( + 'd748bf26ab37599c944babfdbeecf6690801bd61bf2670efb0a34adfc6dca10b', + 'hex' + ), + stateRoot: Buffer.from( + 'cafd881ab193703b83816c49ff6c2bf6ba6f464a1be560c42106128c8dbc35e7', + 'hex' + ), + }) + + const res3 = await stateManager.getAccount(address) + + st.equal( + res3.codeHash.toString('hex'), + 'd748bf26ab37599c944babfdbeecf6690801bd61bf2670efb0a34adfc6dca10b' + ) + st.equal( + res3.stateRoot.toString('hex'), + 'cafd881ab193703b83816c49ff6c2bf6ba6f464a1be560c42106128c8dbc35e7' + ) + + st.end() + }) + + t.test( + 'should modify account fields correctly on previously non-existent account', + async (st) => { + const stateManager = new DefaultStateManager() + const address = new Address(Buffer.from('a94f5374fce5edbc8e2a8697c15331677e6ebf0b', 'hex')) + + await stateManager.modifyAccountFields(address, { balance: new BN(1234) }) + + const res1 = await stateManager.getAccount(address) + + st.equal(res1.balance.toString('hex'), '4d2') + + await stateManager.modifyAccountFields(address, { nonce: new BN(1) }) + + const res2 = await stateManager.getAccount(address) + + st.equal(res2.nonce.toNumber(), 1) + + await stateManager.modifyAccountFields(address, { + codeHash: Buffer.from( + 'd748bf26ab37599c944babfdbeecf6690801bd61bf2670efb0a34adfc6dca10b', + 'hex' + ), + stateRoot: Buffer.from( + 'cafd881ab193703b83816c49ff6c2bf6ba6f464a1be560c42106128c8dbc35e7', + 'hex' + ), + }) + + const res3 = await stateManager.getAccount(address) + + st.equal( + res3.codeHash.toString('hex'), + 'd748bf26ab37599c944babfdbeecf6690801bd61bf2670efb0a34adfc6dca10b' + ) + st.equal( + res3.stateRoot.toString('hex'), + 'cafd881ab193703b83816c49ff6c2bf6ba6f464a1be560c42106128c8dbc35e7' + ) + + st.end() + } + ) t.test( 'should generate the genesis state root correctly for mainnet from ethereum/tests data', async (st) => {