Skip to content

Commit

Permalink
Common, Block: remove and rename ambiguous active methods (#1753)
Browse files Browse the repository at this point in the history
* common: rename hardforkIsActiveOnChain to isIncludedHardfork

* common: remove activeHardfork and activeHardforks

* common: adapt _getHardfork, remove isIncludedHardfork

* common: type fixes in forkHash
  • Loading branch information
emersonmacro authored and holgerd77 committed May 4, 2022
1 parent 275dfc8 commit e6c5692
Show file tree
Hide file tree
Showing 7 changed files with 41 additions and 134 deletions.
12 changes: 4 additions & 8 deletions packages/block/src/header.ts
Original file line number Diff line number Diff line change
Expand Up @@ -447,7 +447,7 @@ export class BlockHeader {
)
throw new Error(msg)
}
const hardfork = this._getHardfork()
const hardfork = this._common.hardfork()
const blockTs = this.timestamp
const { timestamp: parentTs, difficulty: parentDif } = parentBlockHeader
const minimumDifficulty = new BN(
Expand Down Expand Up @@ -570,7 +570,7 @@ export class BlockHeader {
parentGasLimit = parentGasLimit.mul(elasticity)
}
const gasLimit = this.gasLimit
const hardfork = this._getHardfork()
const hardfork = this._common.hardfork()

const a = parentGasLimit.div(
new BN(this._common.paramByHardfork('gasConfig', 'gasLimitBoundDivisor', hardfork))
Expand Down Expand Up @@ -607,7 +607,7 @@ export class BlockHeader {
if (this.isGenesis()) {
return
}
const hardfork = this._getHardfork()
const hardfork = this._common.hardfork()
// Consensus type dependent checks
if (this._common.consensusAlgorithm() === ConsensusAlgorithm.Ethash) {
// PoW/Ethash
Expand Down Expand Up @@ -980,10 +980,6 @@ export class BlockHeader {
return jsonDict
}

private _getHardfork(): string {
return this._common.hardfork() || this._common.activeHardfork(this.number.toNumber())
}

private async _getHeaderByHash(
blockchain: Blockchain,
hash: Buffer
Expand All @@ -1005,7 +1001,7 @@ export class BlockHeader {
* activation block (see: https://blog.slock.it/hard-fork-specification-24b889e70703)
*/
private _validateDAOExtraData() {
if (!this._common.hardforkIsActiveOnChain(Hardfork.Dao)) {
if (!this._common.hardforkIsActiveOnBlock(Hardfork.Dao, this.number)) {
return
}
const DAOActivationBlock = this._common.hardforkBlock(Hardfork.Dao)
Expand Down
2 changes: 1 addition & 1 deletion packages/block/test/block.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import util from 'util'

tape('[Block]: block functions', function (t) {
t.test('should test block initialization', function (st) {
const common = new Common({ chain: Chain.Ropsten, hardfork: Hardfork.Chainstart })
const common = new Common({ chain: Chain.Mainnet, hardfork: Hardfork.Chainstart })
const genesis = Block.genesis({}, { common })
st.ok(genesis.hash().toString('hex'), 'block should initialize')

Expand Down
69 changes: 13 additions & 56 deletions packages/common/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -505,14 +505,14 @@ export default class Common extends EventEmitter {
/**
* Internal helper function, returns the params for the given hardfork for the chain set
* @param hardfork Hardfork name
* @returns Dictionary with hardfork params
* @returns Dictionary with hardfork params or null if hardfork not on chain
*/
_getHardfork(hardfork: string | Hardfork): any {
_getHardfork(hardfork: string | Hardfork): HardforkParams | null {
const hfs = this.hardforks()
for (const hf of hfs) {
if (hf['name'] === hardfork) return hf
}
throw new Error(`Hardfork ${hardfork} not defined for chain ${this.chainName()}`)
return null
}

/**
Expand Down Expand Up @@ -620,14 +620,15 @@ export default class Common extends EventEmitter {
}

/**
* Returns a parameter for the hardfork active on block number
* Returns a parameter for the hardfork active on block number or
* optional provided total difficulty (Merge HF)
* @param topic Parameter topic
* @param name Parameter name
* @param blockNumber Block number
* @param td Total difficulty
*/
paramByBlock(topic: string, name: string, blockNumber: BNLike): any {
const activeHfs = this.activeHardforks(blockNumber)
const hardfork = activeHfs[activeHfs.length - 1]['name']
paramByBlock(topic: string, name: string, blockNumber: BNLike, td?: BNLike): any {
const hardfork = this.getHardforkByBlockNumber(blockNumber, td)
return this.paramByHardfork(topic, name, hardfork)
}

Expand Down Expand Up @@ -711,58 +712,14 @@ export default class Common extends EventEmitter {
return this.hardforkGteHardfork(null, hardfork)
}

/**
* Checks if given or set hardfork is active on the chain
* @param hardfork Hardfork name, optional if HF set
* @returns True if hardfork is active on the chain
*/
hardforkIsActiveOnChain(hardfork?: string | Hardfork | null): boolean {
hardfork = hardfork ?? this._hardfork
for (const hf of this.hardforks()) {
if (hf['name'] === hardfork && hf['block'] !== null) return true
}
return false
}

/**
* Returns the active hardfork switches for the current chain
* @param blockNumber up to block if provided, otherwise for the whole chain
* @return Array with hardfork arrays
*/
activeHardforks(blockNumber?: BNLike | null): HardforkParams[] {
const activeHardforks: HardforkParams[] = []
const hfs = this.hardforks()
for (const hf of hfs) {
if (hf['block'] === null) continue
if (blockNumber !== undefined && blockNumber !== null && blockNumber < hf['block']) break

activeHardforks.push(hf)
}
return activeHardforks
}

/**
* Returns the latest active hardfork name for chain or block or throws if unavailable
* @param blockNumber up to block if provided, otherwise for the whole chain
* @return Hardfork name
*/
activeHardfork(blockNumber?: BNLike | null): string {
const activeHardforks = this.activeHardforks(blockNumber)
if (activeHardforks.length > 0) {
return activeHardforks[activeHardforks.length - 1]['name']
} else {
throw new Error(`No (supported) active hardfork found`)
}
}

/**
* Returns the hardfork change block for hardfork provided or set
* @param hardfork Hardfork name, optional if HF set
* @returns Block number or null if unscheduled
*/
hardforkBlock(hardfork?: string | Hardfork): BN | null {
hardfork = hardfork ?? this._hardfork
const block = this._getHardfork(hardfork)['block']
const block = this._getHardfork(hardfork)?.['block']
if (block === undefined || block === null) {
return null
}
Expand All @@ -776,7 +733,7 @@ export default class Common extends EventEmitter {
*/
hardforkTD(hardfork?: string | Hardfork): BN | null {
hardfork = hardfork ?? this._hardfork
const td = this._getHardfork(hardfork)['td']
const td = this._getHardfork(hardfork)?.['td']
if (td === undefined || td === null) {
return null
}
Expand Down Expand Up @@ -869,14 +826,14 @@ export default class Common extends EventEmitter {
* Returns an eth/64 compliant fork hash (EIP-2124)
* @param hardfork Hardfork name, optional if HF set
*/
forkHash(hardfork?: string | Hardfork) {
forkHash(hardfork?: string | Hardfork): string {
hardfork = hardfork ?? this._hardfork
const data = this._getHardfork(hardfork)
if (data['block'] === null && data['td'] === undefined) {
if (data === null || (data?.['block'] === null && data?.['td'] === undefined)) {
const msg = 'No fork hash calculation possible for future hardfork'
throw new Error(msg)
}
if (data['forkHash'] !== undefined) {
if (data?.['forkHash'] !== null && data?.['forkHash'] !== undefined) {
return data['forkHash']
}
return this._calcForkHash(hardfork)
Expand Down
76 changes: 10 additions & 66 deletions packages/common/tests/hardforks.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,9 @@ tape('[Common]: Hardfork logic', function (t: tape.Test) {
let msg = 'should return the correct HF change block for byzantium (provided)'
st.ok(c.hardforkBlock(Hardfork.Byzantium)!.eqn(1700000), msg)

msg = 'should return null if HF does not exist on chain'
st.equal(c.hardforkBlock('thisHardforkDoesNotExist'), null, msg)

c = new Common({ chain: Chain.Ropsten, hardfork: Hardfork.Byzantium })
msg = 'should return the correct HF change block for byzantium (set)'
st.ok(c.hardforkBlock()!.eqn(1700000), msg)
Expand Down Expand Up @@ -150,50 +153,6 @@ tape('[Common]: Hardfork logic', function (t: tape.Test) {
st.end()
})

t.test('activeHardforks()', function (st: tape.Test) {
let c = new Common({ chain: Chain.Ropsten })
let msg = 'should return correct number of active hardforks for Ropsten'
st.equal(c.activeHardforks().length, 11, msg)

msg = 'should return the correct HF data for Ropsten'
st.equal(c.activeHardforks()[3]['name'], Hardfork.SpuriousDragon, msg)

msg = 'should return 3 active hardforks for Ropsten up to block 9'
st.equal(c.activeHardforks(9).length, 3, msg)

msg = 'should return 4 active hardforks for Ropsten up to block 10'
st.equal(c.activeHardforks(10).length, 4, msg)

c = new Common({ chain: Chain.Mainnet })
msg = 'should return correct number of active HFs for mainnet'
st.equal(c.activeHardforks().length, 13, msg)

c = new Common({ chain: Chain.Rinkeby })
msg = 'should return correct number of active HFs for rinkeby'
st.equal(c.activeHardforks().length, 10, msg)

c = new Common({ chain: Chain.Goerli })
msg = 'should return correct number of active HFs for goerli'
st.equal(c.activeHardforks().length, 10, msg)

st.end()
})

t.test('activeHardfork()', function (st: tape.Test) {
let c = new Common({ chain: Chain.Ropsten })
let msg = 'should return correct latest active HF for Ropsten'
st.equal(c.activeHardfork(), Hardfork.London, msg)

msg = 'should return spuriousDragon as latest active HF for Ropsten for block 10'
st.equal(c.activeHardfork(10), Hardfork.SpuriousDragon, msg)

c = new Common({ chain: Chain.Rinkeby })
msg = 'should return correct latest active HF for Rinkeby'
st.equal(c.activeHardfork(), Hardfork.London, msg)

st.end()
})

t.test('hardforkIsActiveOnBlock() / activeOnBlock()', function (st: tape.Test) {
let c = new Common({ chain: Chain.Ropsten })
let msg = 'Ropsten, byzantium (provided), 1700000 -> true'
Expand Down Expand Up @@ -276,27 +235,6 @@ tape('[Common]: Hardfork logic', function (t: tape.Test) {
st.end()
})

t.test('hardforkIsActiveOnChain()', function (st: tape.Test) {
let c = new Common({ chain: Chain.Ropsten })
let msg = 'should return true for byzantium (provided) on Ropsten'
st.equal(c.hardforkIsActiveOnChain(Hardfork.Byzantium), true, msg)

msg = 'should return false for dao (provided) on Ropsten'
st.equal(c.hardforkIsActiveOnChain(Hardfork.Dao), false, msg)

msg = 'should return true for petersburg (provided) on Ropsten'
st.equal(c.hardforkIsActiveOnChain(Hardfork.Petersburg), true, msg)

msg = 'should return false for a non-existing HF (provided) on Ropsten'
st.equal(c.hardforkIsActiveOnChain('notexistinghardfork'), false, msg)

c = new Common({ chain: Chain.Ropsten, hardfork: Hardfork.Byzantium })
msg = 'should return true for byzantium (set) on Ropsten'
st.equal(c.hardforkIsActiveOnChain(), true, msg)

st.end()
})

t.test('_calcForkHash()', function (st: tape.Test) {
let c = new Common({ chain: Chain.Mainnet })
let msg = 'should calc correctly for chainstart (only genesis)'
Expand Down Expand Up @@ -333,12 +271,18 @@ tape('[Common]: Hardfork logic', function (t: tape.Test) {
st.equal(c.forkHash(Hardfork.SpuriousDragon), '0x3edd5b10', msg)

c = new Common({ chain: Chain.Kovan })
const f = () => {
let f = () => {
c.forkHash(Hardfork.Merge)
}
msg = 'should throw when called on non-applied or future HF'
st.throws(f, /No fork hash calculation possible/, msg)

f = () => {
c.forkHash('thisHardforkDoesNotExist')
}
msg = 'should throw when called with a HF that does not exist on chain'
st.throws(f, /No fork hash calculation possible/, msg)

st.end()
})

Expand Down
5 changes: 5 additions & 0 deletions packages/common/tests/mergePOS.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,11 @@ tape('[Common]: Merge/POS specific logic', function (t: tape.Test) {
const customChains = [testnetMerge]
const c = new Common({ chain: 'testnetMerge', hardfork: Hardfork.Istanbul, customChains })
st.ok(c.hardforkTD(Hardfork.Merge)?.eqn(5000), 'should get the HF total difficulty')
st.equal(
c.hardforkTD('thisHardforkDoesNotExist'),
null,
'should return null if HF does not exist on chain'
)

st.end()
})
Expand Down
5 changes: 5 additions & 0 deletions packages/common/tests/params.spec.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import tape from 'tape'
import Common, { Chain, Hardfork } from '../src/'
import { BN } from 'ethereumjs-util'

tape('[Common]: Parameter access for param(), paramByHardfork()', function (t: tape.Test) {
t.test('Basic usage', function (st: tape.Test) {
Expand Down Expand Up @@ -72,6 +73,10 @@ tape('[Common]: Parameter access for param(), paramByHardfork()', function (t: t
msg = 'Should correctly translate block numbers into HF states (original value)'
st.equal(c.paramByBlock('pow', 'minerReward', 4369999), '5000000000000000000', msg)

msg = 'Should correctly translate total difficulty into HF states'
const td = new BN('1196768507891266117779')
st.equal(c.paramByBlock('pow', 'minerReward', 4370000, td), '3000000000000000000', msg)

st.comment('-----------------------------------------------------------------')
st.end()
})
Expand Down
6 changes: 3 additions & 3 deletions packages/vm/src/runBlock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { debug as createDebugLogger } from 'debug'
import { BaseTrie as Trie } from 'merkle-patricia-tree'
import { Account, Address, bigIntToBN, BN, bnToBigInt, intToBuffer, rlp } from 'ethereumjs-util'
import { Block } from '@ethereumjs/block'
import { ConsensusType } from '@ethereumjs/common'
import { ConsensusType, Hardfork } from '@ethereumjs/common'
import VM from './index'
import Bloom from './bloom'
import { StateManager } from './state'
Expand Down Expand Up @@ -143,8 +143,8 @@ export default async function runBlock(this: VM, opts: RunBlockOpts): Promise<Ru

// check for DAO support and if we should apply the DAO fork
if (
this._common.hardforkIsActiveOnChain('dao') &&
block.header.number.eq(this._common.hardforkBlock('dao')!)
this._common.hardforkIsActiveOnBlock(Hardfork.Dao, block.header.number) &&
block.header.number.eq(this._common.hardforkBlock(Hardfork.Dao)!)
) {
if (this.DEBUG) {
debug(`Apply DAO hardfork`)
Expand Down

0 comments on commit e6c5692

Please sign in to comment.