Skip to content

Commit

Permalink
CI: ensure all hardforks and transitions run when there is a "test al…
Browse files Browse the repository at this point in the history 
…l hardforks" label on PR (#1901)

* vm/tests: update CI
* block: semi-address Berlin->London transition bug
* vm: add test for transitionTests
* ci: update runnerrs
* common: fix test
* vm: update test count
* ci: add london
  • Loading branch information
@jochem-brouwer
jochem-brouwer authored Jul 21, 2022
1 parent 0bb7e21 commit d276fcc
Show file tree
Hide file tree
Showing 7 changed files with 91 additions and 41 deletions.
29 changes: 18 additions & 11 deletions .github/workflows/vm-pr.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -134,19 +134,26 @@ jobs:
runs-on: ubuntu-latest
strategy:
matrix:
# Args to pass to the tester. Note that some have split the slow tests and only
# run on forks where applicable (see PR #489 for numbers on these)

# Tests were split with --dir and --excludeDir to balance execution times below the 9min mark.
# Args to pass to the tester.
args:
[
'--fork=Berlin --expected-test-amount=33',
'--fork=Constantinople --dir=GeneralStateTests/stTimeConsuming --expected-test-amount=15561',
'--fork=Constantinople --excludeDir=stTimeConsuming --expected-test-amount=16836',
'--fork=Petersburg --dir=GeneralStateTests/stTimeConsuming --expected-test-amount=15561',
'--fork=Petersburg --excludeDir=stTimeConsuming --expected-test-amount=16821',
'--fork=Homestead --expected-test-amount=6623',
'--fork=Chainstart --expected-test-amount=4044',
'--fork=Chainstart --verify-test-amount-alltests',
'--fork=Homestead --verify-test-amount-alltests',
'--fork=TangerineWhistle --verify-test-amount-alltests',
'--fork=SpuriousDragon --verify-test-amount-alltests',
'--fork=Byzantium --verify-test-amount-alltests',
'--fork=Constantinople --verify-test-amount-alltests',
'--fork=Petersburg --verify-test-amount-alltests',
'--fork=Istanbul --verify-test-amount-alltests',
'--fork=MuirGlacier --verify-test-amount-alltests',
'--fork=Berlin --verify-test-amount-alltests',
'--fork=London --verify-test-amount-alltests',
'--fork=ByzantiumToConstantinopleFixAt5 --verify-test-amount-alltests',
'--fork=EIP158ToByzantiumAt5 --verify-test-amount-alltests',
'--fork=FrontierToHomesteadAt5 --verify-test-amount-alltests',
'--fork=HomesteadToDaoAt5 --verify-test-amount-alltests',
'--fork=HomesteadToEIP150At5 --verify-test-amount-alltests',
'--fork=BerlinToLondonAt5 --verify-test-amount-alltests',
]
fail-fast: false
steps:
Expand Down
9 changes: 9 additions & 0 deletions packages/block/src/header.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,15 @@ export class BlockHeader {
throw new Error('invalid header. Less values than expected were received')
}

// eslint-disable-next-line @typescript-eslint/strict-boolean-expressions
if (opts.common?.isActivatedEIP(1559) && baseFeePerGas === undefined) {
const eip1559ActivationBlock = bigIntToBuffer(opts.common?.eipBlock(1559)!)
// eslint-disable-next-line @typescript-eslint/strict-boolean-expressions
if (eip1559ActivationBlock && eip1559ActivationBlock.equals(number)) {
throw new Error('invalid header. baseFeePerGas should be provided')
}
}

return new BlockHeader(
{
parentHash,
Expand Down
4 changes: 2 additions & 2 deletions packages/blockchain/src/blockchain.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -586,13 +586,13 @@ export class Blockchain implements BlockchainInterface {
}

// check blockchain dependent EIP1559 values
if (this._common.isActivatedEIP(1559) === true) {
if (header._common.isActivatedEIP(1559) === true) {
// check if the base fee is correct
let expectedBaseFee
const londonHfBlock = this._common.hardforkBlock(Hardfork.London)
const isInitialEIP1559Block = number === londonHfBlock
if (isInitialEIP1559Block) {
expectedBaseFee = this._common.param('gasConfig', 'initialBaseFee')
expectedBaseFee = header._common.param('gasConfig', 'initialBaseFee')
} else {
expectedBaseFee = parentHeader.calcNextBaseFee()
}
Expand Down
18 changes: 18 additions & 0 deletions packages/common/src/common.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -547,6 +547,24 @@ export class Common extends EventEmitter {
return BigInt(block)
}

/**
* Returns the hardfork change block for eip
* @param eip EIP number
* @returns Block number or null if unscheduled
*/
eipBlock(eip: number): bigint | null {
for (const hfChanges of HARDFORK_CHANGES) {
const hf = hfChanges[1]
if ('eips' in hf) {
// eslint-disable-next-line @typescript-eslint/strict-boolean-expressions
if (hf['eips'].includes(eip)) {
return this.hardforkBlock(hfChanges[0])
}
}
}
return null
}

/**
* Returns the hardfork change total difficulty (Merge HF) for hardfork provided or set
* @param hardfork Hardfork name, optional if HF set
Expand Down
12 changes: 12 additions & 0 deletions packages/common/tests/eips.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,4 +58,16 @@ tape('[Common/EIPs]: Initialization / Chain params', function (t: tape.Test) {

st.end()
})

t.test('eipBlock', function (st: tape.Test) {
const c = new Common({ chain: Chain.Mainnet })

let msg = 'should return correct value'
st.ok(c.eipBlock(1559)! === 12965000n, msg)

msg = 'should return null for unscheduled eip'
st.equal(c.eipBlock(0), null, msg)

st.end()
})
})
5 changes: 3 additions & 2 deletions packages/vm/package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,13 @@
"tape": "tape -r ts-node/register --stack-size=1500",
"tester": "ts-node ./tests/tester --stack-size=1500",
"test:state": "npm run tester -- --state",
"test:state:allForks": "echo 'Chainstart Homestead dao TangerineWhistle SpuriousDragon Byzantium Constantinople Petersburg Istanbul MuirGlacier Berlin London ByzantiumToConstantinopleFixAt5 EIP158ToByzantiumAt5 FrontierToHomesteadAt5 HomesteadToDaoAt5 HomesteadToEIP150At5' | xargs -n1 | xargs -I v1 npm run test:state -- --fork=v1 --verify-test-amount-alltests",
"test:state:allForks": "echo 'Chainstart Homestead dao TangerineWhistle SpuriousDragon Byzantium Constantinople Petersburg Istanbul MuirGlacier Berlin London ByzantiumToConstantinopleFixAt5 EIP158ToByzantiumAt5 FrontierToHomesteadAt5 HomesteadToDaoAt5 HomesteadToEIP150At5 BerlinToLondonAt5' | xargs -n1 | xargs -I v1 npm run test:state -- --fork=v1 --verify-test-amount-alltests",
"test:state:selectedForks": "echo 'Homestead TangerineWhistle SpuriousDragon Petersburg Berlin London' | xargs -n1 | xargs -I v1 npm run test:state -- --fork=v1 --verify-test-amount-alltests",
"test:state:slow": "npm run test:state -- --runSkipped=slow",
"test:buildIntegrity": "npm run test:state -- --test='stackOverflow'",
"test:blockchain": "npm run tester -- --blockchain",
"test:blockchain:allForks": "echo 'Chainstart Homestead dao TangerineWhistle SpuriousDragon Byzantium Constantinople Petersburg Istanbul MuirGlacier Berlin London ByzantiumToConstantinopleFixAt5 EIP158ToByzantiumAt5 FrontierToHomesteadAt5 HomesteadToDaoAt5 HomesteadToEIP150At5' | xargs -n1 | xargs -I v1 npm run tester -- --blockchain --fork=v1 --verify-test-amount-alltests",
"test:blockchain:allForks": "echo 'Chainstart Homestead dao TangerineWhistle SpuriousDragon Byzantium Constantinople Petersburg Istanbul MuirGlacier Berlin London ByzantiumToConstantinopleFixAt5 EIP158ToByzantiumAt5 FrontierToHomesteadAt5 HomesteadToDaoAt5 HomesteadToEIP150At5 BerlinToLondonAt5' | xargs -n1 | xargs -I v1 npm run tester -- --blockchain --fork=v1 --verify-test-amount-alltests",
"test:blockchain:transitionForks": "echo 'ByzantiumToConstantinopleFixAt5 EIP158ToByzantiumAt5 FrontierToHomesteadAt5 HomesteadToDaoAt5 HomesteadToEIP150At5 BerlinToLondonAt5' | xargs -n1 | xargs -I v1 npm run tester -- --blockchain --fork=v1 --verify-test-amount-alltests",
"test:API": "npm run tape -- './tests/api/**/*.spec.ts'",
"test:API:browser": "karma start karma.conf.js",
"test": "echo \"[INFO] Generic test cmd not used. See package.json for more specific test run cmds.\"",
Expand Down
55 changes: 29 additions & 26 deletions packages/vm/tests/tester/config.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -263,16 +263,17 @@ export function getTestDirs(network: string, testType: string) {
* @returns {Common} the Common which should be used
*/
export function getCommon(targetNetwork: string) {
let network = targetNetwork.toLowerCase()
let network = targetNetwork
const networkLowercase = network.toLowerCase()
if (network.includes('+')) {
const index = network.indexOf('+')
network = network.slice(0, index)
}
if (normalHardforks.map((str) => str.toLowerCase()).includes(network)) {
if (normalHardforks.map((str) => str.toLowerCase()).includes(networkLowercase)) {
// normal hard fork, return the common with this hard fork
// find the right upper/lowercased version
const hfName = normalHardforks.reduce((previousValue, currentValue) =>
currentValue.toLowerCase() === network ? currentValue : previousValue
currentValue.toLowerCase() === networkLowercase ? currentValue : previousValue
)
const mainnetCommon = new Common({ chain: Chain.Mainnet, hardfork: hfName })
const hardforks = mainnetCommon.hardforks()
Expand Down Expand Up @@ -355,36 +356,38 @@ export function getCommon(targetNetwork: string) {

const expectedTestsFull: any = {
BlockchainTests: {
Chainstart: 4385,
Homestead: 7001,
Chainstart: 4496,
Homestead: 7321,
Dao: 0,
TangerineWhistle: 4255,
SpuriousDragon: 4305,
Byzantium: 15379,
Constantinople: 32750,
Petersburg: 32735,
Istanbul: 35378,
MuirGlacier: 35378,
Berlin: 33,
TangerineWhistle: 4609,
SpuriousDragon: 4632,
Byzantium: 15703,
Constantinople: 33146,
Petersburg: 33128,
Istanbul: 38773,
MuirGlacier: 38773,
Berlin: 41872,
London: 61547,
ByzantiumToConstantinopleFixAt5: 3,
EIP158ToByzantiumAt5: 3,
FrontierToHomesteadAt5: 12,
HomesteadToDaoAt5: 18,
FrontierToHomesteadAt5: 13,
HomesteadToDaoAt5: 32,
HomesteadToEIP150At5: 3,
BerlinToLondonAt5: 0,
BerlinToLondonAt5: 24,
},
GeneralStateTests: {
Chainstart: 896,
Homestead: 1847,
Chainstart: 1045,
Homestead: 2078,
Dao: 0,
TangerineWhistle: 969,
SpuriousDragon: 1094,
Byzantium: 4626,
Constantinople: 10402,
Petersburg: 10397,
Istanbul: 10715,
MuirGlacier: 10715,
Berlin: 13065,
TangerineWhistle: 1200,
SpuriousDragon: 1325,
Byzantium: 4857,
Constantinople: 10648,
Petersburg: 10642,
Istanbul: 12439,
MuirGlacier: 12439,
Berlin: 13214,
London: 19449,
ByzantiumToConstantinopleFixAt5: 0,
EIP158ToByzantiumAt5: 0,
FrontierToHomesteadAt5: 0,
Expand Down

1 comment on commit d276fcc

@github-actions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 2.

Benchmark suite Current: d276fcc Previous: 0bb7e21 Ratio
Block 9422905 10624 ops/sec (±3.51%) 22116 ops/sec (±3.09%) 2.08
Block 9422906 10380 ops/sec (±5.59%) 21489 ops/sec (±4.23%) 2.07
Block 9422907 10795 ops/sec (±2.66%) 21737 ops/sec (±1.67%) 2.01
Block 9422910 10320 ops/sec (±2.75%) 22099 ops/sec (±1.48%) 2.14

This comment was automatically generated by workflow using github-action-benchmark.

Please sign in to comment.