Skip to content

Commit

Permalink
[VM] add homestead support; fix very big KECCAK operations
Browse files Browse the repository at this point in the history
[VM] Pre-TangerineWhistle: make too much gas forwarded in calls go OOG

[VM] lint

[VM] update package scripts

[VM] fix workflow

[VM] include Homestead in PR test runner
  • Loading branch information
jochem-brouwer committed Aug 6, 2020
1 parent f4df8a2 commit 17e0b70
Show file tree
Hide file tree
Showing 11 changed files with 83 additions and 29 deletions.
6 changes: 4 additions & 2 deletions .github/workflows/vm-pr.yml
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ jobs:
runs-on: ubuntu-latest
strategy:
matrix:
fork: ['Petersburg', 'Istanbul', 'MuirGlacier']
fork: ['Petersburg', 'Istanbul', 'MuirGlacier', 'Homestead', 'SpuriousDragon', 'TangerineWhistle', 'Byzantium']
fail-fast: false
steps:
- uses: actions/setup-node@v1
Expand Down Expand Up @@ -84,6 +84,8 @@ jobs:
matrix:
# This is the most fair division among 1 directory vs. everything else
args: ['--excludeDir=stTimeConsuming', '--dir=GeneralStateTests/stTimeConsuming']
# Run specific fork tests
fork: ['Homestead', 'Istanbul']
fail-fast: false
steps:
- uses: actions/setup-node@v1
Expand All @@ -108,4 +110,4 @@ jobs:
- run: npm run build:vm
working-directory: ${{github.workspace}}

- run: npm run test:blockchain -- ${{ matrix.args }}
- run: npm run test:blockchain -- ${{ matrix.args }} --fork=${{ matrix.fork }}
2 changes: 1 addition & 1 deletion packages/account/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
},
"homepage": "https://github.com/ethereumjs/ethereumjs-vm/tree/master/packages/account#synopsis",
"dependencies": {
"ethereumjs-util": "^7.0.2",
"ethereumjs-util": "^7.0.4",
"rlp": "^2.2.3",
"safe-buffer": "^5.1.1"
},
Expand Down
2 changes: 1 addition & 1 deletion packages/block/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@
"@ethereumjs/common": "^1.5.1",
"@ethereumjs/tx": "^2.1.2",
"@types/bn.js": "^4.11.6",
"ethereumjs-util": "^7.0.2",
"ethereumjs-util": "^7.0.4",
"merkle-patricia-tree": "^4.0.0"
},
"devDependencies": {
Expand Down
2 changes: 1 addition & 1 deletion packages/blockchain/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@
"@ethereumjs/common": "^1.5.1",
"@ethereumjs/ethash": "^1.0.0",
"async": "^2.6.1",
"ethereumjs-util": "^7.0.2",
"ethereumjs-util": "^7.0.4",
"flow-stoplight": "^1.0.0",
"level-mem": "^3.0.1",
"lru-cache": "^5.1.1",
Expand Down
2 changes: 1 addition & 1 deletion packages/ethash/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
"@types/levelup": "^4.3.0",
"async": "^2.1.2",
"buffer-xor": "^2.0.1",
"ethereumjs-util": "^7.0.2",
"ethereumjs-util": "^7.0.4",
"miller-rabin": "^4.0.0"
},
"devDependencies": {
Expand Down
2 changes: 1 addition & 1 deletion packages/tx/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
"license": "MPL-2.0",
"dependencies": {
"@ethereumjs/common": "^1.5.1",
"ethereumjs-util": "^7.0.2"
"ethereumjs-util": "^7.0.4"
},
"devDependencies": {
"@ethereumjs/config-nyc": "^1.1.1",
Expand Down
16 changes: 11 additions & 5 deletions packages/vm/lib/evm/memory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,12 +54,18 @@ export default class Memory {
* @param size - How many bytes to read
*/
read(offset: number, size: number): Buffer {
const loaded = this._store.slice(offset, offset + size)
// Fill the remaining length with zeros
for (let i = loaded.length; i < size; i++) {
loaded[i] = 0
const returnBuffer = Buffer.allocUnsafe(size)
// Copy the stored "buffer" from memory into the return Buffer

const loaded = Buffer.from(this._store.slice(offset, offset + size))
returnBuffer.fill(loaded, 0, loaded.length)

if (loaded.length < size) {
// fill the remaining part of the Buffer with zeros
returnBuffer.fill(0, loaded.length, size)
}
return Buffer.from(loaded)

return returnBuffer
}
}

Expand Down
66 changes: 55 additions & 11 deletions packages/vm/lib/evm/opFns.ts
Original file line number Diff line number Diff line change
Expand Up @@ -607,7 +607,11 @@ export const handlers: { [k: string]: OpHandler } = {

subMemUsage(runState, offset, length)
let gasLimit = new BN(runState.eei.getGasLeft())
gasLimit = maxCallGas(gasLimit, runState.eei.getGasLeft())
gasLimit = maxCallGas(
gasLimit,
runState.eei.getGasLeft(),
runState._common.gteHardfork('tangerineWhistle'),
)

let data = Buffer.alloc(0)
if (!length.isZero()) {
Expand All @@ -630,7 +634,7 @@ export const handlers: { [k: string]: OpHandler } = {
new BN(runState._common.param('gasPrices', 'sha3Word')).imul(divCeil(length, new BN(32))),
)
let gasLimit = new BN(runState.eei.getGasLeft())
gasLimit = maxCallGas(gasLimit, runState.eei.getGasLeft())
gasLimit = maxCallGas(gasLimit, runState.eei.getGasLeft(), true) // CREATE2 is only available after TW (Constantinople introduced this opcode)

let data = Buffer.alloc(0)
if (!length.isZero()) {
Expand Down Expand Up @@ -666,7 +670,6 @@ export const handlers: { [k: string]: OpHandler } = {
if (!value.isZero()) {
runState.eei.useGas(new BN(runState._common.param('gasPrices', 'callValueTransfer')))
}
gasLimit = maxCallGas(gasLimit, runState.eei.getGasLeft())

let data = Buffer.alloc(0)
if (!inLength.isZero()) {
Expand All @@ -685,6 +688,16 @@ export const handlers: { [k: string]: OpHandler } = {
runState.eei.useGas(new BN(runState._common.param('gasPrices', 'callNewAccount')))
}

gasLimit = maxCallGas(
gasLimit,
runState.eei.getGasLeft(),
runState._common.gteHardfork('tangerineWhistle'),
)
// note that TW or later this cannot happen (it could have ran out of gas prior to getting here though)
if (gasLimit.gt(runState.eei.getGasLeft())) {
trap(ERROR.OUT_OF_GAS)
}

if (!value.isZero()) {
// TODO: Don't use private attr directly
runState.eei._gasLeft.iaddn(runState._common.param('gasPrices', 'callStipend'))
Expand Down Expand Up @@ -713,7 +726,15 @@ export const handlers: { [k: string]: OpHandler } = {
if (!value.isZero()) {
runState.eei.useGas(new BN(runState._common.param('gasPrices', 'callValueTransfer')))
}
gasLimit = maxCallGas(gasLimit, runState.eei.getGasLeft())
gasLimit = maxCallGas(
gasLimit,
runState.eei.getGasLeft(),
runState._common.gteHardfork('tangerineWhistle'),
)
// note that TW or later this cannot happen (it could have ran out of gas prior to getting here though)
if (gasLimit.gt(runState.eei.getGasLeft())) {
trap(ERROR.OUT_OF_GAS)
}
if (!value.isZero()) {
// TODO: Don't use private attr directly
runState.eei._gasLeft.iaddn(runState._common.param('gasPrices', 'callStipend'))
Expand All @@ -737,7 +758,15 @@ export const handlers: { [k: string]: OpHandler } = {

subMemUsage(runState, inOffset, inLength)
subMemUsage(runState, outOffset, outLength)
gasLimit = maxCallGas(gasLimit, runState.eei.getGasLeft())
gasLimit = maxCallGas(
gasLimit,
runState.eei.getGasLeft(),
runState._common.gteHardfork('tangerineWhistle'),
)
// note that TW or later this cannot happen (it could have ran out of gas prior to getting here though)
if (gasLimit.gt(runState.eei.getGasLeft())) {
trap(ERROR.OUT_OF_GAS)
}

let data = Buffer.alloc(0)
if (!inLength.isZero()) {
Expand All @@ -756,7 +785,7 @@ export const handlers: { [k: string]: OpHandler } = {

subMemUsage(runState, inOffset, inLength)
subMemUsage(runState, outOffset, outLength)
gasLimit = maxCallGas(gasLimit, runState.eei.getGasLeft())
gasLimit = maxCallGas(gasLimit, runState.eei.getGasLeft(), true) // we set TW or later to true here, as STATICCALL was available from Byzantium (which is after TW)

let data = Buffer.alloc(0)
if (!inLength.isZero()) {
Expand Down Expand Up @@ -807,8 +836,8 @@ export const handlers: { [k: string]: OpHandler } = {
deductGas = true
}
}
} else {
// Pre EIP-161 (Spurious Dragon) gas semantics
} else if (runState._common.gteHardfork('tangerineWhistle')) {
// Pre EIP-150 (Tangerine Whistle) gas semantics
const exists = await runState.stateManager.accountExists(selfdestructToAddressBuf)
if (!exists) {
deductGas = true
Expand Down Expand Up @@ -893,9 +922,24 @@ function jumpIsValid(runState: RunState, dest: number): boolean {
return runState.validJumps.indexOf(dest) !== -1
}

function maxCallGas(gasLimit: BN, gasLeft: BN): BN {
const gasAllowed = gasLeft.sub(gasLeft.divn(64))
return gasLimit.gt(gasAllowed) ? gasAllowed : gasLimit
// returns the max call gas which we want to use for the gas limit
// note that pre-TW there was no hard limit. if you tried sending more gas, your CALLs (or equivalent) went OOG
//

/**
* Returns an overflow-safe slice of an array. It right-pads
* the data with zeros to `length`.
* @param {BN} gasLimit - requested gas Limit
* @param {BN} gasLeft - current gas left
* @param {boolean} isTangerineWhistleOrLater - set to true if we are on TW or later (this converts this to the identify function if it is false)
*/
function maxCallGas(gasLimit: BN, gasLeft: BN, isTangerineWhistleOrLater: boolean): BN {
if (isTangerineWhistleOrLater) {
const gasAllowed = gasLeft.sub(gasLeft.divn(64))
return gasLimit.gt(gasAllowed) ? gasAllowed : gasLimit
} else {
return gasLimit
}
}

async function getContractStorage(runState: RunState, address: Buffer, key: Buffer) {
Expand Down
1 change: 1 addition & 0 deletions packages/vm/lib/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 = [
'homestead',
'tangerineWhistle',
'spuriousDragon',
'byzantium',
Expand Down
8 changes: 4 additions & 4 deletions packages/vm/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,12 @@
"coverage:test": "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=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:allForks": "npm run test:state -- --fork=Homestead && 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=Homestead && npm run test:state -- --fork=TangerineWhistle && npm run test:state -- --fork=SpuriousDragon",
"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",
"test:blockchain:allForks": "node -r ts-node/register --stack-size=1500 ./tests/tester --blockchain --fork=TangerineWhistle && node -r ts-node/register --stack-size=1500 ./tests/tester --blockchain --fork=SpuriousDragon && node -r ts-node/register --stack-size=1500 ./tests/tester --blockchain --fork=Byzantium && node -r ts-node/register --stack-size=1500 ./tests/tester --blockchain --fork=Constantinople && node -r ts-node/register --stack-size=1500 ./tests/tester --blockchain --fork=Petersburg && node -r ts-node/register --stack-size=1500 ./tests/tester --blockchain --fork=Istanbul && node -r ts-node/register --stack-size=1500 ./tests/tester --blockchain --fork=MuirGlacier",
"test:blockchain:allForks": "node -r ts-node/register --stack-size=1500 ./tests/tester --blockchain --fork=Homestead && node -r ts-node/register --stack-size=1500 ./tests/tester --blockchain --fork=TangerineWhistle && node -r ts-node/register --stack-size=1500 ./tests/tester --blockchain --fork=SpuriousDragon && node -r ts-node/register --stack-size=1500 ./tests/tester --blockchain --fork=Byzantium && node -r ts-node/register --stack-size=1500 ./tests/tester --blockchain --fork=Constantinople && node -r ts-node/register --stack-size=1500 ./tests/tester --blockchain --fork=Petersburg && node -r ts-node/register --stack-size=1500 ./tests/tester --blockchain --fork=Istanbul && node -r ts-node/register --stack-size=1500 ./tests/tester --blockchain --fork=MuirGlacier",
"test:API": "tape -r ts-node/register --stack-size=1500 ./tests/api/**/*.js",
"test:API:browser": "npm run build && karma start karma.conf.js",
"test": "echo \"[INFO] Generic test cmd not used. See package.json for more specific test run cmds.\"",
Expand Down Expand Up @@ -49,7 +49,7 @@
"@ethereumjs/blockchain": "^4.0.3",
"@ethereumjs/common": "^1.5.1",
"@ethereumjs/tx": "^2.1.2",
"ethereumjs-util": "^7.0.2",
"ethereumjs-util": "^7.0.4",
"functional-red-black-tree": "^1.0.1",
"merkle-patricia-tree": "^4.0.0",
"rustbn.js": "~0.2.0",
Expand Down
5 changes: 3 additions & 2 deletions packages/vm/tests/tester.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ const argv = require('minimist')(process.argv.slice(2))
const tape = require('tape')
const config = require('./config')
const testLoader = require('./testLoader')
const Common = require('@ethereumjs/common').default

function runTests() {
let name
Expand Down Expand Up @@ -96,8 +97,8 @@ function runTests() {
// Tests for HFs before Istanbul have been moved under `LegacyTests/Constantinople`:
// https://github.com/ethereum/tests/releases/tag/v7.0.0-beta.1

// TODO: Replace with Common.lteHardfork('Istanbul')
if (testGetterArgs.forkConfig !== 'Istanbul') {
const common = new Common('mainnet', FORK_CONFIG_VM)
if (!common.gteHardfork('istanbul')) {
name = 'LegacyTests/Constantinople/'.concat(name)
}
testLoader.getTestsFromArgs(
Expand Down

0 comments on commit 17e0b70

Please sign in to comment.