From e6c5692976219ab3d0c1cfc90206ee403d5b196e Mon Sep 17 00:00:00 2001 From: emersonmacro <77563348+emersonmacro@users.noreply.github.com> Date: Tue, 1 Mar 2022 01:16:59 -0800 Subject: [PATCH] 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 --- packages/block/src/header.ts | 12 ++-- packages/block/test/block.spec.ts | 2 +- packages/common/src/index.ts | 69 +++++----------------- packages/common/tests/hardforks.spec.ts | 76 ++++--------------------- packages/common/tests/mergePOS.spec.ts | 5 ++ packages/common/tests/params.spec.ts | 5 ++ packages/vm/src/runBlock.ts | 6 +- 7 files changed, 41 insertions(+), 134 deletions(-) diff --git a/packages/block/src/header.ts b/packages/block/src/header.ts index d94170f28c..ca2d627831 100644 --- a/packages/block/src/header.ts +++ b/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) diff --git a/packages/block/test/block.spec.ts b/packages/block/test/block.spec.ts index 9f1250d8fb..bdc5eed413 100644 --- a/packages/block/test/block.spec.ts +++ b/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') diff --git a/packages/common/src/index.ts b/packages/common/src/index.ts index 004171170f..017857579d 100644 --- a/packages/common/src/index.ts +++ b/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,50 +712,6 @@ 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 @@ -762,7 +719,7 @@ export default class Common extends EventEmitter { */ 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) diff --git a/packages/common/tests/hardforks.spec.ts b/packages/common/tests/hardforks.spec.ts index a7af1e906b..9894a5fb33 100644 --- a/packages/common/tests/hardforks.spec.ts +++ b/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() }) diff --git a/packages/common/tests/mergePOS.spec.ts b/packages/common/tests/mergePOS.spec.ts index 9e092aa28c..ac42cd774c 100644 --- a/packages/common/tests/mergePOS.spec.ts +++ b/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() }) diff --git a/packages/common/tests/params.spec.ts b/packages/common/tests/params.spec.ts index 8aa81eaefc..f060ea052d 100644 --- a/packages/common/tests/params.spec.ts +++ b/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() }) diff --git a/packages/vm/src/runBlock.ts b/packages/vm/src/runBlock.ts index 2e12057fb0..ef9c6d3a87 100644 --- a/packages/vm/src/runBlock.ts +++ b/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