diff --git a/packages/vm/lib/evm/eei.ts b/packages/vm/lib/evm/eei.ts index 7f22d09a6a..83201cd53e 100644 --- a/packages/vm/lib/evm/eei.ts +++ b/packages/vm/lib/evm/eei.ts @@ -576,7 +576,7 @@ export default class EEI { } /** - * Returns true if account is empty (according to EIP-161). + * Returns true if account is empty or non-existent (according to EIP-161). * @param address - Address of account */ async isAccountEmpty(address: Buffer): Promise { diff --git a/packages/vm/lib/evm/evm.ts b/packages/vm/lib/evm/evm.ts index 88acdae009..52df32be56 100644 --- a/packages/vm/lib/evm/evm.ts +++ b/packages/vm/lib/evm/evm.ts @@ -221,7 +221,10 @@ export default class EVM { await this._vm._emit('newContract', newContractEvent) toAccount = await this._state.getAccount(message.to) - toAccount.nonce = new BN(toAccount.nonce).addn(1).toArrayLike(Buffer) + // EIP-161 on account creation and CREATE execution + if (this._vm._common.gteHardfork('spuriousDragon')) { + toAccount.nonce = new BN(toAccount.nonce).addn(1).toArrayLike(Buffer) + } // Add tx value to the `to` account let errorMessage diff --git a/packages/vm/lib/evm/opFns.ts b/packages/vm/lib/evm/opFns.ts index 53e4f3a962..31f911fe90 100644 --- a/packages/vm/lib/evm/opFns.ts +++ b/packages/vm/lib/evm/opFns.ts @@ -660,9 +660,9 @@ export const handlers: { [k: string]: OpHandler } = { if (runState.eei.isStatic() && !value.isZero()) { trap(ERROR.STATIC_STATE_CHANGE) } - subMemUsage(runState, inOffset, inLength) subMemUsage(runState, outOffset, outLength) + if (!value.isZero()) { runState.eei.useGas(new BN(runState._common.param('gasPrices', 'callValueTransfer'))) } @@ -673,9 +673,17 @@ export const handlers: { [k: string]: OpHandler } = { data = runState.memory.read(inOffset.toNumber(), inLength.toNumber()) } - const empty = await runState.eei.isAccountEmpty(toAddressBuf) - if (empty) { - if (!value.isZero()) { + if (runState._common.gteHardfork('spuriousDragon')) { + // We are at or after Spurious Dragon + // Call new account gas: account is DEAD and we transfer nonzero value + if ((await runState.stateManager.accountIsEmpty(toAddressBuf)) && !value.isZero()) { + runState.eei.useGas(new BN(runState._common.param('gasPrices', 'callNewAccount'))) + } + } else if (!(await runState.stateManager.accountExists(toAddressBuf))) { + // We are before Spurious Dragon + // Call new account gas: account does not exist (it is not in the state trie, not even as an "empty" account) + const accountDoesNotExist = !(await runState.stateManager.accountExists(toAddressBuf)) + if (accountDoesNotExist) { runState.eei.useGas(new BN(runState._common.param('gasPrices', 'callNewAccount'))) } } @@ -789,13 +797,29 @@ export const handlers: { [k: string]: OpHandler } = { } const selfdestructToAddressBuf = addressToBuffer(selfdestructToAddress) - const balance = await runState.eei.getExternalBalance(runState.eei.getAddress()) - if (balance.gtn(0)) { - const empty = await runState.eei.isAccountEmpty(selfdestructToAddressBuf) - if (empty) { - runState.eei.useGas(new BN(runState._common.param('gasPrices', 'callNewAccount'))) + let deductGas = false + if (runState._common.gteHardfork('spuriousDragon')) { + // EIP-161: State Trie Clearing + const balance = await runState.eei.getExternalBalance(runState.eei.getAddress()) + if (balance.gtn(0)) { + // This technically checks if account is empty or non-existent + // TODO: improve on the API here (EEI and StateManager) + const empty = await runState.eei.isAccountEmpty(selfdestructToAddressBuf) + if (empty) { + const account = await runState.stateManager.getAccount(selfdestructToAddressBuf) + deductGas = true + } + } + } else { + // Pre EIP-161 (Spurious Dragon) gas semantics + const exists = await runState.stateManager.accountExists(selfdestructToAddressBuf) + if (!exists) { + deductGas = true } } + if (deductGas) { + runState.eei.useGas(new BN(runState._common.param('gasPrices', 'callNewAccount'))) + } return runState.eei.selfDestruct(selfdestructToAddressBuf) }, diff --git a/packages/vm/lib/index.ts b/packages/vm/lib/index.ts index 537926b93a..4500104625 100644 --- a/packages/vm/lib/index.ts +++ b/packages/vm/lib/index.ts @@ -110,6 +110,7 @@ export default class VM extends AsyncEventEmitter { const chain = opts.chain ? opts.chain : 'mainnet' const hardfork = opts.hardfork ? opts.hardfork : 'petersburg' const supportedHardforks = [ + 'tangerineWhistle', 'spuriousDragon', 'byzantium', 'constantinople', diff --git a/packages/vm/lib/runTx.ts b/packages/vm/lib/runTx.ts index cad65cce15..8b2822f211 100644 --- a/packages/vm/lib/runTx.ts +++ b/packages/vm/lib/runTx.ts @@ -174,9 +174,11 @@ async function _runTx(this: VM, opts: RunTxOpts): Promise { const minerAccount = await state.getAccount(block.header.coinbase) // add the amount spent on gas to the miner's account minerAccount.balance = toBuffer(new BN(minerAccount.balance).add(results.amountSpent)) - if (!new BN(minerAccount.balance).isZero()) { - await state.putAccount(block.header.coinbase, minerAccount) - } + + // Put the miner account into the state. If the balance of the miner account remains zero, note that + // the state.putAccount function puts this into the "touched" accounts. This will thus be removed when + // we clean the touched accounts below in case we are in a fork >= SpuriousDragon + await state.putAccount(block.header.coinbase, minerAccount) /* * Cleanup accounts @@ -184,7 +186,7 @@ async function _runTx(this: VM, opts: RunTxOpts): Promise { if (results.execResult.selfdestruct) { const keys = Object.keys(results.execResult.selfdestruct) for (const k of keys) { - await state.putAccount(Buffer.from(k, 'hex'), new Account()) + await state.deleteAccount(Buffer.from(k, 'hex')) } } await state.cleanupTouchedAccounts() diff --git a/packages/vm/lib/state/cache.ts b/packages/vm/lib/state/cache.ts index e126b453c5..e278e2b2da 100644 --- a/packages/vm/lib/state/cache.ts +++ b/packages/vm/lib/state/cache.ts @@ -54,24 +54,30 @@ export default class Cache { /** * Looks up address in underlying trie. * @param address - Address of account + * @param create - Create emtpy account if non-existent */ - async _lookupAccount(address: Buffer): Promise { + async _lookupAccount(address: Buffer, create: boolean = true): Promise { const raw = await this._trie.get(address) - const account = new Account(raw) - return account + if (raw || create) { + const account = new Account(raw) + return account + } } /** * Looks up address in cache, if not found, looks it up * in the underlying trie. * @param key - Address of account + * @param create - Create emtpy account if non-existent */ - async getOrLoad(key: Buffer): Promise { + async getOrLoad(key: Buffer, create: boolean = true): Promise { let account = this.lookup(key) if (!account) { - account = await this._lookupAccount(key) - this._update(key, account, false, false) + account = await this._lookupAccount(key, create) + if (account) { + this._update(key, account as Account, false, false) + } } return account @@ -87,7 +93,7 @@ export default class Cache { if (addressHex) { const address = Buffer.from(addressHex, 'hex') const account = await this._lookupAccount(address) - this._update(address, account, false, false) + this._update(address, account as Account, false, false) } } } diff --git a/packages/vm/lib/state/interface.ts b/packages/vm/lib/state/interface.ts index b6bf384ba8..87026b61bc 100644 --- a/packages/vm/lib/state/interface.ts +++ b/packages/vm/lib/state/interface.ts @@ -10,7 +10,8 @@ export interface StorageDump { export interface StateManager { copy(): StateManager getAccount(address: Buffer): Promise - putAccount(address: Buffer, account: Account): Promise + putAccount(address: Buffer, account: Account | null): Promise + deleteAccount(address: Buffer): Promise touchAccount(address: Buffer): void putContractCode(address: Buffer, value: Buffer): Promise getContractCode(address: Buffer): Promise @@ -28,5 +29,6 @@ export interface StateManager { generateCanonicalGenesis(): Promise generateGenesis(initState: any): Promise accountIsEmpty(address: Buffer): Promise + accountExists(address: Buffer): Promise cleanupTouchedAccounts(): Promise } diff --git a/packages/vm/lib/state/stateManager.ts b/packages/vm/lib/state/stateManager.ts index b9b7fd6588..933479214f 100644 --- a/packages/vm/lib/state/stateManager.ts +++ b/packages/vm/lib/state/stateManager.ts @@ -71,7 +71,7 @@ export default class DefaultStateManager implements StateManager { * @param address - Address of the `account` to get */ async getAccount(address: Buffer): Promise { - const account = await this._cache.getOrLoad(address) + const account = (await this._cache.getOrLoad(address)) as Account return account } @@ -87,7 +87,11 @@ export default class DefaultStateManager implements StateManager { // if they have money or a non-zero nonce or code, then write to tree this._cache.put(address, account) this.touchAccount(address) - // this._trie.put(addressHex, account.serialize(), cb) + } + + async deleteAccount(address: Buffer) { + this._cache.del(address) + this.touchAccount(address) } /** @@ -111,7 +115,7 @@ export default class DefaultStateManager implements StateManager { const codeHash = keccak256(value) if (codeHash.equals(KECCAK256_NULL)) { - return + //return } const account = await this.getAccount(address) @@ -468,7 +472,8 @@ export default class DefaultStateManager implements StateManager { } /** - * Checks if the `account` corresponding to `address` is empty as defined in + * Checks if the `account` corresponding to `address` + * is empty or non-existent as defined in * EIP-161 (https://eips.ethereum.org/EIPS/eip-161). * @param address - Address to check */ @@ -477,17 +482,35 @@ export default class DefaultStateManager implements StateManager { return account.isEmpty() } + /** + * Checks if the `account` corresponding to `address` + * exists + * @param address - Address of the `account` to check + */ + async accountExists(address: Buffer): Promise { + const account = await this._cache.lookup(address) + if (account) { + return true + } + if (await this._cache._trie.get(address)) { + return true + } + return false + } + /** * Removes accounts form the state trie that have been touched, * as defined in EIP-161 (https://eips.ethereum.org/EIPS/eip-161). */ async cleanupTouchedAccounts(): Promise { - const touchedArray = Array.from(this._touched) - for (const addressHex of touchedArray) { - const address = Buffer.from(addressHex, 'hex') - const empty = await this.accountIsEmpty(address) - if (empty) { - this._cache.del(address) + if (this._common.gteHardfork('spuriousDragon')) { + const touchedArray = Array.from(this._touched) + for (const addressHex of touchedArray) { + const address = Buffer.from(addressHex, 'hex') + const empty = await this.accountIsEmpty(address) + if (empty) { + this._cache.del(address) + } } } this._touched.clear() diff --git a/packages/vm/package.json b/packages/vm/package.json index 544c148bcd..84db915333 100644 --- a/packages/vm/package.json +++ b/packages/vm/package.json @@ -14,8 +14,8 @@ "coverage:test": "npm run build && tape './tests/api/**/*.js' ./tests/tester.js --state --dist", "docs:build": "typedoc --options typedoc.js", "test:state": "ts-node ./tests/tester --state", - "test:state:allForks": "npm run test:state -- --fork=SpuriousDragon && npm run test:state -- --fork=Byzantium && npm run test:state -- --fork=Constantinople && npm run test:state -- --fork=Petersburg && npm run test:state -- --fork=Istanbul && npm run test:state -- --fork=MuirGlacier", - "test:state:selectedForks": "npm run test:state -- --fork=Petersburg && npm run test:state -- --fork=SpuriousDragon", + "test:state:allForks": "npm run test:state -- --fork=TangerineWhistle && npm run test:state -- --fork=SpuriousDragon && npm run test:state -- --fork=Byzantium && npm run test:state -- --fork=Constantinople && npm run test:state -- --fork=Petersburg && npm run test:state -- --fork=Istanbul && npm run test:state -- --fork=MuirGlacier", + "test:state:selectedForks": "npm run test:state -- --fork=Petersburg && npm run test:state -- --fork=TangerineWhistle", "test:state:slow": "npm run test:state -- --runSkipped=slow", "test:buildIntegrity": "npm run test:state -- --test='stackOverflow'", "test:blockchain": "node -r ts-node/register --stack-size=1500 ./tests/tester --blockchain", diff --git a/packages/vm/tests/api/state/stateManager.js b/packages/vm/tests/api/state/stateManager.js index dbf0c7c746..90280102f5 100644 --- a/packages/vm/tests/api/state/stateManager.js +++ b/packages/vm/tests/api/state/stateManager.js @@ -113,6 +113,37 @@ tape('StateManager', (t) => { }, ) + t.test( + 'should return false for a non-existent account', + async (st) => { + const stateManager = new DefaultStateManager() + const address = Buffer.from('a94f5374fce5edbc8e2a8697c15331677e6ebf0b', 'hex') + + let res = await stateManager.accountExists(address) + + st.notOk(res) + + st.end() + }, + ) + + t.test( + 'should return true for an existent account', + async (st) => { + const stateManager = new DefaultStateManager() + const account = createAccount('0x1', '0x1') + const address = Buffer.from('a94f5374fce5edbc8e2a8697c15331677e6ebf0b', 'hex') + + await stateManager.putAccount(address, account) + + let res = await stateManager.accountExists(address) + + st.ok(res) + + st.end() + }, + ) + t.test( 'should call the callback with a false boolean representing non-emptiness when the account is not empty', async (st) => { diff --git a/packages/vm/tests/config.js b/packages/vm/tests/config.js index 78cdc89427..c752750a0e 100644 --- a/packages/vm/tests/config.js +++ b/packages/vm/tests/config.js @@ -119,6 +119,11 @@ const SKIP_VM = [ * @returns {String} Either an alias of the forkConfig param, or the forkConfig param itself */ function getRequiredForkConfigAlias(forkConfig) { + // TangerineWhistle is named EIP150 (attention: misleading name) + // in the client-independent consensus test suite + if (String(forkConfig).match(/^tangerineWhistle$/i)) { + return 'EIP150' + } // SpuriousDragon is named EIP158 (attention: misleading name) // in the client-independent consensus test suite if (String(forkConfig).match(/^spuriousDragon$/i)) {