Skip to content

Commit 41ebfba

Browse files
holgerd77holgerd77
authored
Merge pull request ethereumjs#607 from ethereumjs/istanbul-tests
[WIP] Add state tests for Istanbul
2 parents 00d8fe3 + 970ddbf commit 41ebfba

File tree

14 files changed

+105
-66
lines changed

14 files changed

+105
-66
lines changed

‎.circleci/config.yml

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,15 @@ jobs:
8282
- run:
8383
name: testStatePetersburg
8484
command: npm run testStatePetersburg
85+
test_state_istanbul:
86+
<<: *defaults
87+
steps:
88+
- attach_workspace:
89+
at: ~/project
90+
- *restore_node_modules
91+
- run:
92+
name: testStateIstanbul
93+
command: npm run testStateIstanbul
8594
test_blockchain:
8695
<<: *defaults
8796
steps:
@@ -91,6 +100,15 @@ jobs:
91100
- run:
92101
name: testBlockchain
93102
command: npm run testBlockchain
103+
test_blockchain_petersburg:
104+
<<: *defaults
105+
steps:
106+
- attach_workspace:
107+
at: ~/project
108+
- *restore_node_modules
109+
- run:
110+
name: testBlockchainPetersburg
111+
command: npm run testBlockchainPetersburg
94112
coveralls:
95113
<<: *defaults
96114
steps:
@@ -123,9 +141,15 @@ workflows:
123141
- test_state_petersburg:
124142
requires:
125143
- install
144+
- test_state_istanbul:
145+
requires:
146+
- install
126147
- test_blockchain:
127148
requires:
128149
- install
150+
- test_blockchain_petersburg:
151+
requires:
152+
- install
129153
- coveralls:
130154
requires:
131155
- install

‎lib/evm/eei.ts

Lines changed: 5 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,6 @@ export interface Env {
3333
export interface RunResult {
3434
logs: any // TODO: define type for Log (each log: [Buffer(address), [Buffer(topic0), ...]])
3535
returnValue?: Buffer
36-
gasRefund: BN
3736
/**
3837
* A map from the accounts that have self-destructed to the addresses to send their funds to
3938
*/
@@ -67,7 +66,6 @@ export default class EEI {
6766
this._result = {
6867
logs: [],
6968
returnValue: undefined,
70-
gasRefund: new BN(0),
7169
selfdestruct: {},
7270
}
7371
}
@@ -90,17 +88,17 @@ export default class EEI {
9088
* @param amount - Amount of gas refunded
9189
*/
9290
refundGas(amount: BN): void {
93-
this._result.gasRefund.iadd(amount)
91+
this._evm._refund.iadd(amount)
9492
}
9593

9694
/**
9795
* Reduces amount of gas to be refunded by a positive value.
9896
* @param amount - Amount to subtract from gas refunds
9997
*/
10098
subRefund(amount: BN): void {
101-
this._result.gasRefund.isub(amount)
102-
if (this._result.gasRefund.ltn(0)) {
103-
this._result.gasRefund = new BN(0)
99+
this._evm._refund.isub(amount)
100+
if (this._evm._refund.ltn(0)) {
101+
this._evm._refund = new BN(0)
104102
trap(ERROR.REFUND_EXHAUSTED)
105103
}
106104
}
@@ -163,10 +161,6 @@ export default class EEI {
163161
* input data passed with the message call instruction or transaction.
164162
*/
165163
getCallDataSize(): BN {
166-
if (this._env.callData.length === 1 && this._env.callData[0] === 0) {
167-
return new BN(0)
168-
}
169-
170164
return new BN(this._env.callData.length)
171165
}
172166

@@ -354,9 +348,7 @@ export default class EEI {
354348
async _selfDestruct(toAddress: Buffer): Promise<void> {
355349
// only add to refund if this is the first selfdestruct for the address
356350
if (!this._result.selfdestruct[this._env.address.toString('hex')]) {
357-
this._result.gasRefund = this._result.gasRefund.addn(
358-
this._common.param('gasPrices', 'selfdestructRefund'),
359-
)
351+
this.refundGas(new BN(this._common.param('gasPrices', 'selfdestructRefund')))
360352
}
361353

362354
this._result.selfdestruct[this._env.address.toString('hex')] = toAddress
@@ -491,11 +483,6 @@ export default class EEI {
491483
this._result.logs = this._result.logs.concat(results.execResult.logs)
492484
}
493485

494-
// add gasRefund
495-
if (results.execResult.gasRefund) {
496-
this._result.gasRefund = this._result.gasRefund.add(results.execResult.gasRefund)
497-
}
498-
499486
// this should always be safe
500487
this.useGas(results.gasUsed)
501488

@@ -553,11 +540,6 @@ export default class EEI {
553540
this._result.logs = this._result.logs.concat(results.execResult.logs)
554541
}
555542

556-
// add gasRefund
557-
if (results.execResult.gasRefund) {
558-
this._result.gasRefund = this._result.gasRefund.add(results.execResult.gasRefund)
559-
}
560-
561543
// this should always be safe
562544
this.useGas(results.gasUsed)
563545

‎lib/evm/evm.ts

Lines changed: 16 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import {
1010
import Account from 'ethereumjs-account'
1111
import { ERROR, VmError } from '../exceptions'
1212
import PStateManager from '../state/promisified'
13-
import { getPrecompile, PrecompileFunc } from './precompiles'
13+
import { getPrecompile, PrecompileFunc, ripemdPrecompileAddress } from './precompiles'
1414
import TxContext from './txContext'
1515
import Message from './message'
1616
import EEI from './eei'
@@ -61,14 +61,14 @@ export interface ExecResult {
6161
* Array of logs that the contract emitted
6262
*/
6363
logs?: any[]
64-
/**
65-
* Amount of gas to refund from deleting storage values
66-
*/
67-
gasRefund?: BN
6864
/**
6965
* A map from the accounts that have self-destructed to the addresses to send their funds to
7066
*/
7167
selfdestruct?: { [k: string]: Buffer }
68+
/**
69+
* Total amount of gas to be refunded from all nested calls.
70+
*/
71+
gasRefund?: BN
7272
}
7373

7474
export interface NewContractEvent {
@@ -96,12 +96,17 @@ export default class EVM {
9696
_state: PStateManager
9797
_tx: TxContext
9898
_block: any
99+
/**
100+
* Amount of gas to refund from deleting storage values
101+
*/
102+
_refund: BN
99103

100104
constructor(vm: any, txContext: TxContext, block: any) {
101105
this._vm = vm
102106
this._state = this._vm.pStateManager
103107
this._tx = txContext
104108
this._block = block
109+
this._refund = new BN(0)
105110
}
106111

107112
/**
@@ -120,20 +125,14 @@ export default class EVM {
120125
} else {
121126
result = await this._executeCreate(message)
122127
}
128+
// TODO: Move `gasRefund` to a tx-level result object
129+
// instead of `ExecResult`.
130+
result.execResult.gasRefund = this._refund.clone()
123131

124132
const err = result.execResult.exceptionError
125133
if (err) {
126134
result.execResult.logs = []
127135
await this._state.revert()
128-
if (message.isCompiled) {
129-
// Empty precompiled contracts need to be deleted even in case of OOG
130-
// because the bug in both Geth and Parity led to deleting RIPEMD precompiled in this case
131-
// see https://github.com/ethereum/go-ethereum/pull/3341/files#diff-2433aa143ee4772026454b8abd76b9dd
132-
// We mark the account as touched here, so that is can be removed among other touched empty accounts (after tx finalization)
133-
if (err.error === ERROR.OUT_OF_GAS) {
134-
await this._touchAccount(message.to)
135-
}
136-
}
137136
} else {
138137
await this._state.commit()
139138
}
@@ -289,6 +288,7 @@ export default class EVM {
289288
eei._result.selfdestruct = message.selfdestruct
290289
}
291290

291+
const oldRefund = this._refund.clone()
292292
const interpreter = new Interpreter(this._vm, eei)
293293
const interpreterRes = await interpreter.run(message.code as Buffer, opts)
294294

@@ -303,9 +303,10 @@ export default class EVM {
303303
result = {
304304
...result,
305305
logs: [],
306-
gasRefund: new BN(0),
307306
selfdestruct: {},
308307
}
308+
// Revert gas refund if message failed
309+
this._refund = oldRefund
309310
}
310311

311312
return {

‎lib/evm/opFns.ts

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -959,9 +959,7 @@ function updateSstoreGas(runState: RunState, found: any, value: Buffer) {
959959
// If original value is not 0
960960
if (current.length === 0) {
961961
// If current value is 0 (also means that new value is not 0), remove 15000 gas from refund counter. We can prove that refund counter will never go below 0.
962-
runState.eei._result.gasRefund.isub(
963-
new BN(runState._common.param('gasPrices', 'netSstoreClearRefund')),
964-
)
962+
runState.eei.subRefund(new BN(runState._common.param('gasPrices', 'netSstoreClearRefund')))
965963
} else if (value.length === 0) {
966964
// If new value is 0 (also means that current value is not 0), add 15000 gas to refund counter.
967965
runState.eei.refundGas(new BN(runState._common.param('gasPrices', 'netSstoreClearRefund')))

‎lib/evm/precompiles/09-blake2f.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -404,7 +404,7 @@ export default function(opts: PrecompileInput): ExecResult {
404404
const f = lastByte === 1
405405

406406
const gasUsed = new BN(opts._common.param('gasPrices', 'blake2Round'))
407-
gasUsed.imuln(rounds)
407+
gasUsed.imul(new BN(rounds))
408408
if (opts.gasLimit.lt(gasUsed)) {
409409
return OOGResult(opts.gasLimit)
410410
}

‎lib/evm/precompiles/index.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,11 @@ interface Precompiles {
1313
[key: string]: PrecompileFunc
1414
}
1515

16+
const ripemdPrecompileAddress = '0000000000000000000000000000000000000003'
1617
const precompiles: Precompiles = {
1718
'0000000000000000000000000000000000000001': p1,
1819
'0000000000000000000000000000000000000002': p2,
19-
'0000000000000000000000000000000000000003': p3,
20+
[ripemdPrecompileAddress]: p3,
2021
'0000000000000000000000000000000000000004': p4,
2122
'0000000000000000000000000000000000000005': p5,
2223
'0000000000000000000000000000000000000006': p6,
@@ -29,4 +30,4 @@ function getPrecompile(address: string): PrecompileFunc {
2930
return precompiles[address]
3031
}
3132

32-
export { precompiles, getPrecompile, PrecompileFunc, PrecompileInput }
33+
export { precompiles, getPrecompile, PrecompileFunc, PrecompileInput, ripemdPrecompileAddress }

‎lib/runTx.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -154,10 +154,10 @@ async function _runTx(this: VM, opts: RunTxOpts): Promise<RunTxResult> {
154154
// Caculate the total gas used
155155
results.gasUsed = results.gasUsed.add(basefee)
156156
// Process any gas refund
157-
results.gasRefund = results.execResult.gasRefund
158-
if (results.gasRefund) {
159-
if (results.gasRefund.lt(results.gasUsed.divn(2))) {
160-
results.gasUsed.isub(results.gasRefund)
157+
const gasRefund = evm._refund
158+
if (gasRefund) {
159+
if (gasRefund.lt(results.gasUsed.divn(2))) {
160+
results.gasUsed.isub(gasRefund)
161161
} else {
162162
results.gasUsed.isub(results.gasUsed.divn(2))
163163
}

‎lib/state/stateManager.ts

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import Common from 'ethereumjs-common'
88
import { genesisStateByName } from 'ethereumjs-common/dist/genesisStates'
99
import Account from 'ethereumjs-account'
1010
import Cache from './cache'
11+
import { ripemdPrecompileAddress } from '../evm/precompiles'
1112

1213
/**
1314
* Storage values of an account
@@ -102,11 +103,22 @@ export default class StateManager {
102103
// if (toAccount.balance.toString('hex') === '00') {
103104
// if they have money or a non-zero nonce or code, then write to tree
104105
this._cache.put(address, account)
105-
this._touched.add(address.toString('hex'))
106+
this.touchAccount(address)
106107
// self._trie.put(addressHex, account.serialize(), cb)
107108
cb()
108109
}
109110

111+
/**
112+
* Marks an account as touched, according to the definition
113+
* in [EIP-158](https://github.com/ethereum/EIPs/issues/158).
114+
* This happens when the account is triggered for a state-changing
115+
* event. Touched accounts that are empty will be cleared
116+
* at the end of the tx.
117+
*/
118+
touchAccount(address: Buffer): void {
119+
this._touched.add(address.toString('hex'))
120+
}
121+
110122
/**
111123
* Adds `value` to the state trie as code, and sets `codeHash` on the account
112124
* corresponding to `address` to reference this.
@@ -275,7 +287,7 @@ export default class StateManager {
275287
const contract = this._cache.get(address)
276288
contract.stateRoot = storageTrie.root
277289
this.putAccount(address, contract, cb)
278-
this._touched.add(address.toString('hex'))
290+
this.touchAccount(address)
279291
})
280292
})
281293
}
@@ -372,6 +384,13 @@ export default class StateManager {
372384
if (!touched) {
373385
throw new Error('Reverting to invalid state checkpoint failed')
374386
}
387+
// Exceptional case due to consensus issue in Geth and Parity.
388+
// See [EIP-716](https://github.com/ethereum/EIPs/issues/716) for context.
389+
// The RIPEMD precompile has to remain *touched* even when the call reverts,
390+
// and be considered for deletion.
391+
if (this._touched.has(ripemdPrecompileAddress)) {
392+
touched.add(ripemdPrecompileAddress)
393+
}
375394
this._touched = touched
376395
this._checkpointCount--
377396

‎package.json

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,10 @@
1818
"testStateByzantium": "npm run build:dist && node ./tests/tester -s --fork='Byzantium' --dist",
1919
"testStateConstantinople": "npm run build:dist && node ./tests/tester -s --fork='Constantinople' --dist",
2020
"testStatePetersburg": "npm run build:dist && node ./tests/tester -s --fork='Petersburg' --dist",
21+
"testStateIstanbul": "npm run build:dist && node ./tests/tester -s --fork='Istanbul' --dist",
2122
"testBuildIntegrity": "npm run build:dist && node ./tests/tester -s --dist --test='stackOverflow'",
22-
"testBlockchain": "npm run build:dist && node --stack-size=1500 ./tests/tester -b --fork='Petersburg' --dist --excludeDir='GeneralStateTests'",
23+
"testBlockchain": "npm run build:dist && node --stack-size=1500 ./tests/tester -b --fork='Istanbul' --dist --excludeDir='GeneralStateTests'",
24+
"testBlockchainPetersburg": "npm run build:dist && node --stack-size=1500 ./tests/tester -b --fork='Petersburg' --dist --excludeDir='GeneralStateTests'",
2325
"testBlockchainGeneralStateTests": "npm run build:dist && node --stack-size=1500 ./tests/tester -b --dist --dir='GeneralStateTests'",
2426
"testAPI": "npm run build:dist && tape './tests/api/**/*.js'",
2527
"testAPI:browser": "npm run build:dist && karma start karma.conf.js",
@@ -51,11 +53,11 @@
5153
"async-eventemitter": "^0.2.2",
5254
"core-js-pure": "^3.0.1",
5355
"ethereumjs-account": "^3.0.0",
54-
"ethereumjs-block": "~2.2.0",
55-
"ethereumjs-blockchain": "^4.0.1",
56+
"ethereumjs-block": "^2.2.1",
57+
"ethereumjs-blockchain": "^4.0.2",
5658
"ethereumjs-common": "^1.3.2",
5759
"ethereumjs-tx": "^2.1.1",
58-
"ethereumjs-util": "^6.1.0",
60+
"ethereumjs-util": "~6.1.0",
5961
"fake-merkle-patricia-tree": "^1.0.1",
6062
"functional-red-black-tree": "^1.0.1",
6163
"merkle-patricia-tree": "^2.3.2",
@@ -74,7 +76,7 @@
7476
"@types/node": "^11.13.4",
7577
"browserify": "^16.2.3",
7678
"coveralls": "^3.0.0",
77-
"ethereumjs-testing": "git+https://github.com/ethereumjs/ethereumjs-testing.git#v1.2.7",
79+
"ethereumjs-testing": "git+https://github.com/ethereumjs/ethereumjs-testing.git#v1.3.0",
7880
"husky": "^2.1.0",
7981
"karma": "^4.0.1",
8082
"karma-browserify": "^6.0.0",

‎tests/GeneralStateTestsRunner.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ function runTestCase (options, testData, t, cb) {
6262
testUtil.setupPreConditions(state, testData, done)
6363
},
6464
function (done) {
65-
var tx = testUtil.makeTx(testData.transaction)
65+
var tx = testUtil.makeTx(testData.transaction, options.forkConfig.toLowerCase())
6666
block = testUtil.makeBlockFromEnv(testData.env)
6767
tx._homestead = true
6868
tx.enableHomestead = true

0 commit comments

Comments
 (0)