Common, Block: remove and rename ambiguous active methods (#1753)

* common: rename hardforkIsActiveOnChain to isIncludedHardfork

* common: remove activeHardfork and activeHardforks

* common: adapt _getHardfork, remove isIncludedHardfork

* common: type fixes in forkHash

Author: emersonmacro

packages/block/src/header.ts

@@ -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(
@@ -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))
@@ -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
@@ -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
@@ -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)

packages/block/test/block.spec.ts

@@ -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')
 

packages/common/src/index.ts

@@ -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
   }
 
   /**
@@ -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)
   }
 
@@ -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
     }
@@ -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
     }
@@ -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)

packages/common/tests/hardforks.spec.ts

@@ -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)
@@ -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'
@@ -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)'
@@ -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()
   })
 

packages/common/tests/mergePOS.spec.ts

@@ -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()
   })

packages/common/tests/params.spec.ts

@@ -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) {
@@ -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()
   })

packages/vm/src/runBlock.ts

@@ -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'
@@ -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`)