diff --git a/packages/block/karma.conf.js b/packages/block/karma.conf.js index 24677b6234..5ecd0e9d62 100644 --- a/packages/block/karma.conf.js +++ b/packages/block/karma.conf.js @@ -6,8 +6,9 @@ module.exports = function(config) { preprocessors: { './test-build/**/*.js': ['browserify'], }, + concurrency: 1, reporters: ['dots'], - browsers: ['ChromeHeadless'], + browsers: ['FirefoxHeadless', 'ChromeHeadless'], singleRun: true, }) } diff --git a/packages/vm/lib/evm/opFns.ts b/packages/vm/lib/evm/opFns.ts index d6b5df7b32..e460b0dc44 100644 --- a/packages/vm/lib/evm/opFns.ts +++ b/packages/vm/lib/evm/opFns.ts @@ -1,5 +1,12 @@ import BN = require('bn.js') -import { keccak256, setLengthRight, TWO_POW256, MAX_INTEGER, KECCAK256_NULL } from 'ethereumjs-util' +import { + keccak256, + setLengthRight, + setLengthLeft, + TWO_POW256, + MAX_INTEGER, + KECCAK256_NULL, +} from 'ethereumjs-util' import { ERROR, VmError } from '../exceptions' import { RunState } from './interpreter' @@ -730,7 +737,7 @@ export const handlers: Map = new Map([ // TODO: Replace getContractStorage with EEI method const found = await getContractStorage(runState, runState.eei.getAddress(), keyBuf) - updateSstoreGas(runState, found, value) + updateSstoreGas(runState, found, setLengthLeftStorage(value)) await runState.eei.storageStore(keyBuf, value) }, ], @@ -1287,12 +1294,14 @@ function jumpSubIsValid(runState: RunState, dest: number): boolean { } async function getContractStorage(runState: RunState, address: Buffer, key: Buffer) { - const current = await runState.stateManager.getContractStorage(address, key) + const current = setLengthLeftStorage(await runState.stateManager.getContractStorage(address, key)) if ( runState._common.hardfork() === 'constantinople' || runState._common.gteHardfork('istanbul') ) { - const original = await runState.stateManager.getOriginalContractStorage(address, key) + const original = setLengthLeftStorage( + await runState.stateManager.getOriginalContractStorage(address, key), + ) return { current, original } } else { return current @@ -1436,3 +1445,18 @@ function writeCallOutput(runState: RunState, outOffset: BN, outLength: BN) { runState.memory.write(memOffset, dataLength, data) } } + +/** + setLengthLeftStorage + @param value: Buffer which we want to pad + This is just a proxy function to ethereumjs-util's setLengthLeft, except it returns a zero length buffer in case the buffer is full of zeros. +*/ + +function setLengthLeftStorage(value: Buffer) { + if (value.equals(Buffer.alloc(value.length, 0))) { + // return the empty buffer (the value is zero) + return Buffer.alloc(0) + } else { + return setLengthLeft(value, 32) + } +} diff --git a/packages/vm/lib/runTx.ts b/packages/vm/lib/runTx.ts index 95de533742..009205770b 100644 --- a/packages/vm/lib/runTx.ts +++ b/packages/vm/lib/runTx.ts @@ -190,6 +190,7 @@ async function _runTx(this: VM, opts: RunTxOpts): Promise { } } await state.cleanupTouchedAccounts() + await state.clearOriginalStorageCache() /** * The `afterTx` event diff --git a/packages/vm/lib/state/interface.ts b/packages/vm/lib/state/interface.ts index 0a3bb694e6..0c423c63a8 100644 --- a/packages/vm/lib/state/interface.ts +++ b/packages/vm/lib/state/interface.ts @@ -31,4 +31,5 @@ export interface StateManager { accountIsEmpty(address: Buffer): Promise accountExists(address: Buffer): Promise cleanupTouchedAccounts(): Promise + clearOriginalStorageCache(): void } diff --git a/packages/vm/lib/state/stateManager.ts b/packages/vm/lib/state/stateManager.ts index a7e340b8df..3e5d5db7b9 100644 --- a/packages/vm/lib/state/stateManager.ts +++ b/packages/vm/lib/state/stateManager.ts @@ -235,6 +235,14 @@ export default class DefaultStateManager implements StateManager { this._originalStorageCache = new Map() } + /** + * Clears the original storage cache. Refer to [[getOriginalContractStorage]] + * for more explanation. Alias of the internal _clearOriginalStorageCache + */ + clearOriginalStorageCache(): void { + this._clearOriginalStorageCache() + } + /** * Modifies the storage trie of an account. * @private diff --git a/packages/vm/tests/api/runCall.js b/packages/vm/tests/api/runCall.js index 436c8a425f..f9ad20e814 100644 --- a/packages/vm/tests/api/runCall.js +++ b/packages/vm/tests/api/runCall.js @@ -156,5 +156,52 @@ tape('Ensure that precompile activation creates non-empty accounts', async (t) = t.assert(diff.eq(new BN(expected)), "precompiles are activated") + t.end() +}) + +tape('Ensure that Istanbul sstoreCleanRefundEIP2200 gas is applied correctly', async (t) => { + // setup the accounts for this test + const caller = Buffer.from('00000000000000000000000000000000000000ee', 'hex') // caller addres + const address = Buffer.from('00000000000000000000000000000000000000ff', 'hex') + // setup the vm + const common = new Common({ chain: 'mainnet', hardfork: 'istanbul' }) + const vm = new VM({ common: common}) + const code = "61000260005561000160005500" + /* + idea: store the original value in the storage slot, except it is now a 1-length buffer instead of a 32-length buffer + code: + PUSH2 0x0002 + PUSH1 0x00 + SSTORE -> make storage slot 0 "dirty" + PUSH2 0x0001 + PUSH1 0x00 + SSTORE -> -> restore it to the original storage value (refund sstoreCleanRefundEIP2200) + STOP + gas cost: + 4x PUSH 12 + 2x SSTORE (slot is nonzero, so charge 5000): 10000 + net 10012 + gas refund + sstoreCleanRefundEIP2200 4200 + gas used + 10012 - 4200 = 5812 + + */ + + await vm.stateManager.putContractCode(address, Buffer.from(code, 'hex')) + await vm.stateManager.putContractStorage(address, Buffer.alloc(32, 0), Buffer.from(("00").repeat(31) + "01", 'hex')) + + // setup the call arguments + let runCallArgs = { + caller: caller, // call address + to: address, + gasLimit: new BN(0xffffffffff), // ensure we pass a lot of gas, so we do not run out of gas + } + + const result = await vm.runCall(runCallArgs) + console.log(result.gasUsed, result.execResult.gasRefund) + t.equal(result.gasUsed.toNumber(), 5812, "gas used incorrect") + t.equal(result.execResult.gasRefund.toNumber(), 4200, "gas refund incorrect") + t.end() }) \ No newline at end of file diff --git a/packages/vm/tests/api/runTx.js b/packages/vm/tests/api/runTx.js index 420820a0fc..c6db294b8c 100644 --- a/packages/vm/tests/api/runTx.js +++ b/packages/vm/tests/api/runTx.js @@ -5,6 +5,7 @@ const runTx = require('../../dist/runTx').default const { DefaultStateManager } = require('../../dist/state') const VM = require('../../dist/index').default const { createAccount } = require('./utils') +const Common = require('@ethereumjs/common').default function setup(vm = null) { if (vm === null) { @@ -111,6 +112,40 @@ tape('should fail when account balance overflows (create)', async (t) => { t.end() }) +tape('should clear storage cache after every transaction', async(t) => { + const vm = new VM({common: new Common({ chain: 'mainnet', hardfork: "istanbul" })}) + const suite = setup(vm) + const privateKey = Buffer.from( + 'e331b6d69882b4cb4ea581d88e0b604039a3de5967688d3dcffdd2270c0fd109', + 'hex', + ) + /* Code which is deployed here: + PUSH1 01 + PUSH1 00 + SSTORE + INVALID + */ + const code = "6001600055FE" + const address = Buffer.from("00000000000000000000000000000000000000ff", 'hex') + await vm.stateManager.putContractCode(Buffer.from(address, 'hex'), Buffer.from(code, 'hex')) + await vm.stateManager.putContractStorage(address, Buffer.from(("00").repeat(32), 'hex'), Buffer.from(("00").repeat(31) + "01", 'hex')) + const tx = new Transaction ({ + nonce: '0x00', + gasPrice: 1, + gasLimit: 100000, + to: address, + chainId: 3, + }) + tx.sign(privateKey) + + await vm.stateManager.putAccount(tx.from, createAccount()) + + await vm.runTx({tx}) // this tx will fail, but we have to ensure that the cache is cleared + + t.equal(vm.stateManager._originalStorageCache.size, 0, 'storage cache should be cleared') + t.end() +}) + // The following test tries to verify that running a tx // would work, even when stateManager is not using a cache. // It fails at the moment, and has been therefore commented.