Skip to content

Commit

Permalink
common, vm, tx: centralize default hardfork setting in Common (defaul…
Browse files Browse the repository at this point in the history
…t if none provided, before: null), remove static default HF references from VM, tx
  • Loading branch information
holgerd77 committed Sep 7, 2020
1 parent c1bf90e commit 2ba5701
Show file tree
Hide file tree
Showing 6 changed files with 18 additions and 34 deletions.
22 changes: 11 additions & 11 deletions packages/common/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import { hardforks as hardforkChanges } from './hardforks'
import { EIPs } from './eips'
import { Chain } from './types'

const DEFAULT_HARDFORK = 'petersburg'

/**
* Options for instantiating a [[Common]] instance.
*/
Expand All @@ -14,8 +16,10 @@ export interface CommonOpts {
chain: string | number | object
/**
* String identifier ('byzantium') for hardfork
*
* Default: `petersburg`
*/
hardfork?: string | null
hardfork?: string
/**
* Limit parameter returns to the given hardforks
*/
Expand All @@ -33,7 +37,7 @@ interface hardforkOptions {
* Common class to access chain and hardfork parameters
*/
export default class Common {
private _hardfork: string | null
private _hardfork: string
private _supportedHardforks: Array<string>
private _chainParams: Chain

Expand All @@ -50,7 +54,7 @@ export default class Common {
static forCustomChain(
baseChain: string | number,
customChainParams: Partial<Chain>,
hardfork?: string | null,
hardfork?: string,
supportedHardforks?: Array<string>,
): Common {
const standardChainParams = Common._getChainParams(baseChain)
Expand Down Expand Up @@ -86,7 +90,7 @@ export default class Common {
*/
constructor(opts: CommonOpts) {
this._chainParams = this.setChain(opts.chain)
this._hardfork = null
this._hardfork = DEFAULT_HARDFORK
this._supportedHardforks = opts.supportedHardforks === undefined ? [] : opts.supportedHardforks
if (opts.hardfork) {
this.setHardfork(opts.hardfork)
Expand Down Expand Up @@ -118,9 +122,9 @@ export default class Common {

/**
* Sets the hardfork to get params for
* @param hardfork String identifier ('byzantium')
* @param hardfork String identifier (e.g. 'byzantium')
*/
setHardfork(hardfork: string | null): void {
setHardfork(hardfork: string): void {
if (!this._isSupportedHardfork(hardfork)) {
throw new Error(`Hardfork ${hardfork} not set as supported in supportedHardforks`)
}
Expand All @@ -143,11 +147,7 @@ export default class Common {
*/
_chooseHardfork(hardfork?: string | null, onlySupported: boolean = true): string {
if (!hardfork) {
if (!this._hardfork) {
throw new Error('Method called with neither a hardfork set nor provided by param')
} else {
hardfork = this._hardfork
}
hardfork = this._hardfork
} else if (onlySupported && !this._isSupportedHardfork(hardfork)) {
throw new Error(`Hardfork ${hardfork} not set as supported in supportedHardforks`)
}
Expand Down
2 changes: 1 addition & 1 deletion packages/common/tests/chains.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ tape('[Common]: Initialization / Chain params', function (t: tape.Test) {
st.equal(c.chainName(), 'mainnet', 'should initialize with chain name')
st.equal(c.chainId(), 1, 'should return correct chain Id')
st.equal(c.networkId(), 1, 'should return correct network Id')
st.equal(c.hardfork(), null, 'should set hardfork to null')
st.equal(c.hardfork(), 'petersburg', 'should set hardfork to the default hardfork')
st.equal(c._isSupportedHardfork('constantinople'), true, 'should not restrict supported HFs')

c = new Common({ chain: 1 })
Expand Down
13 changes: 3 additions & 10 deletions packages/common/tests/hardforks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -300,22 +300,15 @@ tape('[Common]: Hardfork logic', function (t: tape.Test) {
})

t.test('forkHash()', function (st: tape.Test) {
let c = new Common({ chain: 'mainnet' })
let f = () => {
c.forkHash()
}
let msg = 'should throw when no hardfork set or provided'
st.throws(f, /neither a hardfork set nor provided by param$/, msg)

c = new Common({ chain: 'mainnet', hardfork: 'byzantium' })
msg = 'should provide correct forkHash for HF set'
let c = new Common({ chain: 'mainnet', hardfork: 'byzantium' })
let msg = 'should provide correct forkHash for HF set'
st.equal(c.forkHash(), '0xa00bc324', msg)

msg = 'should provide correct forkHash for HF provided'
st.equal(c.forkHash('spuriousDragon'), '0x3edd5b10', msg)

c = new Common({ chain: 'ropsten' })
f = () => {
let f = () => {
c.forkHash('dao')
}
msg = 'should throw when called on non-applied or future HF'
Expand Down
9 changes: 2 additions & 7 deletions packages/common/tests/params.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,16 +16,11 @@ tape('[Common]: Parameter access', function (t: tape.Test) {

t.test('Error cases', function (st: tape.Test) {
let c = new Common({ chain: 'mainnet' })
let f = function () {
c.param('gasPrices', 'ecAdd')
}
let msg = 'Should throw when no hardfork set or provided'
st.throws(f, /neither a hardfork set nor provided by param$/, msg)

f = function () {
let f = function () {
c.param('gasPrizes', 'ecAdd', 'byzantium')
}
msg = 'Should throw when called with non-existing topic'
let msg = 'Should throw when called with non-existing topic'
st.throws(f, /Topic gasPrizes not defined$/, msg)

f = function () {
Expand Down
4 changes: 1 addition & 3 deletions packages/tx/src/transaction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,9 +80,7 @@ export default class Transaction {
this._common = opts.common
} else {
const chain = opts.chain ? opts.chain : 'mainnet'
const hardfork = opts.hardfork ? opts.hardfork : 'petersburg'

this._common = new Common({ chain, hardfork })
this._common = new Common({ chain })
}

// Define Properties
Expand Down
2 changes: 0 additions & 2 deletions packages/vm/lib/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,6 @@ export default class VM extends AsyncEventEmitter {
this._common = opts.common
} else {
const DEFAULT_CHAIN = 'mainnet'
const DEFAULT_HARDFORK = 'petersburg'
const supportedHardforks = [
'chainstart',
'homestead',
Expand All @@ -134,7 +133,6 @@ export default class VM extends AsyncEventEmitter {

this._common = new Common({
chain: DEFAULT_CHAIN,
hardfork: DEFAULT_HARDFORK,
supportedHardforks,
})
}
Expand Down

0 comments on commit 2ba5701

Please sign in to comment.