Skip to content

Commit

Permalink
Fix issues causing RevertPrecompiled* test failures
Browse files Browse the repository at this point in the history
  • Loading branch information
s1na committed Nov 13, 2019
1 parent 7776c06 commit c3e1ccf
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 18 deletions.
4 changes: 0 additions & 4 deletions lib/evm/eei.ts
Original file line number Diff line number Diff line change
Expand Up @@ -161,10 +161,6 @@ export default class EEI {
* input data passed with the message call instruction or transaction.
*/
getCallDataSize(): BN {
if (this._env.callData.length === 1 && this._env.callData[0] === 0) {
return new BN(0)
}

return new BN(this._env.callData.length)
}

Expand Down
11 changes: 1 addition & 10 deletions lib/evm/evm.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import {
import Account from 'ethereumjs-account'
import { ERROR, VmError } from '../exceptions'
import PStateManager from '../state/promisified'
import { getPrecompile, PrecompileFunc } from './precompiles'
import { getPrecompile, PrecompileFunc, ripemdPrecompileAddress } from './precompiles'
import TxContext from './txContext'
import Message from './message'
import EEI from './eei'
Expand Down Expand Up @@ -131,15 +131,6 @@ export default class EVM {
if (err) {
result.execResult.logs = []
await this._state.revert()
if (message.isCompiled) {
// Empty precompiled contracts need to be deleted even in case of OOG
// because the bug in both Geth and Parity led to deleting RIPEMD precompiled in this case
// see https://github.com/ethereum/go-ethereum/pull/3341/files#diff-2433aa143ee4772026454b8abd76b9dd
// We mark the account as touched here, so that is can be removed among other touched empty accounts (after tx finalization)
if (err.error === ERROR.OUT_OF_GAS) {
await this._touchAccount(message.to)
}
}
} else {
await this._state.commit()
}
Expand Down
5 changes: 3 additions & 2 deletions lib/evm/precompiles/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,11 @@ interface Precompiles {
[key: string]: PrecompileFunc
}

const ripemdPrecompileAddress = '0000000000000000000000000000000000000003'
const precompiles: Precompiles = {
'0000000000000000000000000000000000000001': p1,
'0000000000000000000000000000000000000002': p2,
'0000000000000000000000000000000000000003': p3,
[ripemdPrecompileAddress]: p3,
'0000000000000000000000000000000000000004': p4,
'0000000000000000000000000000000000000005': p5,
'0000000000000000000000000000000000000006': p6,
Expand All @@ -29,4 +30,4 @@ function getPrecompile(address: string): PrecompileFunc {
return precompiles[address]
}

export { precompiles, getPrecompile, PrecompileFunc, PrecompileInput }
export { precompiles, getPrecompile, PrecompileFunc, PrecompileInput, ripemdPrecompileAddress }
23 changes: 21 additions & 2 deletions lib/state/stateManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import Common from 'ethereumjs-common'
import { genesisStateByName } from 'ethereumjs-common/dist/genesisStates'
import Account from 'ethereumjs-account'
import Cache from './cache'
import { ripemdPrecompileAddress } from '../evm/precompiles'

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

/**
* Marks an account as touched, according to the definition
* in [EIP-158](https://github.com/ethereum/EIPs/issues/158).
* 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.
*/
touchAccount(address: Buffer): void {
this._touched.add(address.toString('hex'))
}

/**
* Adds `value` to the state trie as code, and sets `codeHash` on the account
* corresponding to `address` to reference this.
Expand Down Expand Up @@ -275,7 +287,7 @@ export default class StateManager {
const contract = this._cache.get(address)
contract.stateRoot = storageTrie.root
this.putAccount(address, contract, cb)
this._touched.add(address.toString('hex'))
this.touchAccount(address)
})
})
}
Expand Down Expand Up @@ -372,6 +384,13 @@ export default class StateManager {
if (!touched) {
throw new Error('Reverting to invalid state checkpoint failed')
}
// Exceptional case due to consensus issue in Geth and Parity.
// See [EIP-716](https://github.com/ethereum/EIPs/issues/716) for context.
// The RIPEMD precompile has to remain *touched* even when the call reverts,
// and be considered for deletion.
if (this._touched.has(ripemdPrecompileAddress)) {
touched.add(ripemdPrecompileAddress)
}
this._touched = touched
this._checkpointCount--

Expand Down

0 comments on commit c3e1ccf

Please sign in to comment.