Skip to content

Commit

Permalink
Common, VM: Remove unused HF options from Common, move supportedHardf…
Browse files Browse the repository at this point in the history
…orks to VM (#1649)

* common: remove onlySupported and onlyActive hf options

* vm: move supportedHardforks option from common

* common: remove supportedHardforks option

* common: update README for supportedHardforks removal

* vm: update README for supportedHardforks addition

* common: cleanup in src/index.ts

* vm: refactor supportedHardforks per PR feedback

* common: remove _chooseHardfork method per PR feedback

* vm: updated README per PR feedback

* vm: supportedHardforks local variable, switch to enum
  • Loading branch information
emersonmacro authored and holgerd77 committed Feb 7, 2022
1 parent be87bea commit 6b031c0
Show file tree
Hide file tree
Showing 7 changed files with 70 additions and 226 deletions.
12 changes: 0 additions & 12 deletions packages/common/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -48,18 +48,6 @@ c.genesis().hash // 0x41941023680923e0fe4d74a34bdac8141f2540e3ae90623718e47d66d1
c.bootstrapNodes() // Array with current nodes
```

If the initializing library only supports a certain range of `hardforks` you can use the `supportedHardforks` option to restrict hardfork access on the `Common` instance:

```typescript
const c = new Common({
chain: 'ropsten',
supportedHardforks: ['byzantium', 'constantinople', 'petersburg'],
})
```

This will e.g. throw an error when a param is requested for an unsupported hardfork and
like this prevents unpredicted behaviour.

For an improved developer experience, there are `Chain` and `Hardfork` enums available:

```typescript
Expand Down
122 changes: 21 additions & 101 deletions packages/common/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -102,10 +102,6 @@ interface BaseOpts {
* Default: Hardfork.Istanbul
*/
hardfork?: string | Hardfork
/**
* Limit parameter returns to the given hardforks
*/
supportedHardforks?: Array<string | Hardfork>
/**
* Selected EIPs which can be activated, please use an array for instantiation
* (e.g. `eips: [ 2537, ]`)
Expand Down Expand Up @@ -162,13 +158,6 @@ export interface CustomCommonOpts extends BaseOpts {
baseChain?: string | number | Chain | BN
}

interface hardforkOptions {
/** optional, only allow supported HFs (default: false) */
onlySupported?: boolean
/** optional, only active HFs (default: false) */
onlyActive?: boolean
}

/**
* Common class to access chain and hardfork parameters and to provide
* a unified and shared view on the network and hardfork state.
Expand All @@ -182,7 +171,6 @@ export default class Common extends EventEmitter {

private _chainParams: IChain
private _hardfork: string | Hardfork
private _supportedHardforks: Array<string | Hardfork> = []
private _eips: number[] = []
private _customChains: IChain[] | [IChain, GenesisState][]

Expand Down Expand Up @@ -293,13 +281,11 @@ export default class Common extends EventEmitter {
* chain params on.
* @param customChainParams The custom parameters of the chain.
* @param hardfork String identifier ('byzantium') for hardfork (optional)
* @param supportedHardforks Limit parameter returns to the given hardforks (optional)
*/
static forCustomChain(
baseChain: string | number | Chain,
customChainParams: Partial<IChain>,
hardfork?: string | Hardfork,
supportedHardforks?: Array<string | Hardfork>
hardfork?: string | Hardfork
): Common {
const standardChainParams = Common._getChainParams(baseChain)

Expand All @@ -309,7 +295,6 @@ export default class Common extends EventEmitter {
...customChainParams,
},
hardfork: hardfork,
supportedHardforks: supportedHardforks,
})
}

Expand Down Expand Up @@ -361,9 +346,6 @@ export default class Common extends EventEmitter {
}
}
this._hardfork = this.DEFAULT_HARDFORK
if (opts.supportedHardforks) {
this._supportedHardforks = opts.supportedHardforks
}
if (opts.hardfork) {
this.setHardfork(opts.hardfork)
}
Expand Down Expand Up @@ -416,9 +398,6 @@ export default class Common extends EventEmitter {
* @param hardfork String identifier (e.g. 'byzantium') or {@link Hardfork} enum
*/
setHardfork(hardfork: string | Hardfork): void {
if (!this._isSupportedHardfork(hardfork)) {
throw new Error(`Hardfork ${hardfork} not set as supported in supportedHardforks`)
}
let existing = false
for (const hfChanges of HARDFORK_CHANGES) {
if (hfChanges[0] === hardfork) {
Expand Down Expand Up @@ -514,20 +493,6 @@ export default class Common extends EventEmitter {
return hardfork
}

/**
* Internal helper function to choose between hardfork set and hardfork provided as param
* @param hardfork Hardfork given to function as a parameter
* @returns Hardfork chosen to be used
*/
_chooseHardfork(hardfork?: string | Hardfork | null, onlySupported: boolean = true): string {
if (!hardfork) {
hardfork = this._hardfork
} else if (onlySupported && !this._isSupportedHardfork(hardfork)) {
throw new Error(`Hardfork ${hardfork} not set as supported in supportedHardforks`)
}
return hardfork
}

/**
* Internal helper function, returns the params for the given hardfork for the chain set
* @param hardfork Hardfork name
Expand All @@ -541,22 +506,6 @@ export default class Common extends EventEmitter {
throw new Error(`Hardfork ${hardfork} not defined for chain ${this.chainName()}`)
}

/**
* Internal helper function to check if a hardfork is set to be supported by the library
* @param hardfork Hardfork name
* @returns True if hardfork is supported
*/
_isSupportedHardfork(hardfork: string | Hardfork | null): boolean {
if (this._supportedHardforks.length > 0) {
for (const supportedHf of this._supportedHardforks) {
if (hardfork === supportedHf) return true
}
} else {
return true
}
return false
}

/**
* Sets the active EIPs
* @param eips
Expand Down Expand Up @@ -615,8 +564,6 @@ export default class Common extends EventEmitter {
* @returns The value requested or `null` if not found
*/
paramByHardfork(topic: string, name: string, hardfork: string | Hardfork): any {
hardfork = this._chooseHardfork(hardfork)

let value = null
for (const hfChanges of HARDFORK_CHANGES) {
// EIP-referencing HF file (e.g. berlin.json)
Expand Down Expand Up @@ -703,17 +650,11 @@ export default class Common extends EventEmitter {
* Checks if set or provided hardfork is active on block number
* @param hardfork Hardfork name or null (for HF set)
* @param blockNumber
* @param opts Hardfork options (onlyActive unused)
* @returns True if HF is active on block number
*/
hardforkIsActiveOnBlock(
hardfork: string | Hardfork | null,
blockNumber: BNLike,
opts: hardforkOptions = {}
): boolean {
hardforkIsActiveOnBlock(hardfork: string | Hardfork | null, blockNumber: BNLike): boolean {
blockNumber = toType(blockNumber, TypeOutput.BN)
const onlySupported = opts.onlySupported ?? false
hardfork = this._chooseHardfork(hardfork, onlySupported)
hardfork = hardfork ?? this._hardfork
const hfBlock = this.hardforkBlockBN(hardfork)
if (hfBlock && blockNumber.gte(hfBlock)) {
return true
Expand All @@ -724,11 +665,10 @@ export default class Common extends EventEmitter {
/**
* Alias to hardforkIsActiveOnBlock when hardfork is set
* @param blockNumber
* @param opts Hardfork options (onlyActive unused)
* @returns True if HF is active on block number
*/
activeOnBlock(blockNumber: BNLike, opts?: hardforkOptions): boolean {
return this.hardforkIsActiveOnBlock(null, blockNumber, opts)
activeOnBlock(blockNumber: BNLike): boolean {
return this.hardforkIsActiveOnBlock(null, blockNumber)
}

/**
Expand All @@ -738,20 +678,9 @@ export default class Common extends EventEmitter {
* @param opts Hardfork options
* @returns True if HF1 gte HF2
*/
hardforkGteHardfork(
hardfork1: string | Hardfork | null,
hardfork2: string | Hardfork,
opts: hardforkOptions = {}
): boolean {
const onlyActive = opts.onlyActive === undefined ? false : opts.onlyActive
hardfork1 = this._chooseHardfork(hardfork1, opts.onlySupported)

let hardforks
if (onlyActive) {
hardforks = this.activeHardforks(null, opts)
} else {
hardforks = this.hardforks()
}
hardforkGteHardfork(hardfork1: string | Hardfork | null, hardfork2: string | Hardfork): boolean {
hardfork1 = hardfork1 ?? this._hardfork
const hardforks = this.hardforks()

let posHf1 = -1,
posHf2 = -1
Expand All @@ -767,25 +696,19 @@ export default class Common extends EventEmitter {
/**
* Alias to hardforkGteHardfork when hardfork is set
* @param hardfork Hardfork name
* @param opts Hardfork options
* @returns True if hardfork set is greater than hardfork provided
*/
gteHardfork(hardfork: string | Hardfork, opts?: hardforkOptions): boolean {
return this.hardforkGteHardfork(null, hardfork, opts)
gteHardfork(hardfork: string | Hardfork): boolean {
return this.hardforkGteHardfork(null, hardfork)
}

/**
* Checks if given or set hardfork is active on the chain
* @param hardfork Hardfork name, optional if HF set
* @param opts Hardfork options (onlyActive unused)
* @returns True if hardfork is active on the chain
*/
hardforkIsActiveOnChain(
hardfork?: string | Hardfork | null,
opts: hardforkOptions = {}
): boolean {
const onlySupported = opts.onlySupported ?? false
hardfork = this._chooseHardfork(hardfork, onlySupported)
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
}
Expand All @@ -795,16 +718,14 @@ export default class Common extends EventEmitter {
/**
* Returns the active hardfork switches for the current chain
* @param blockNumber up to block if provided, otherwise for the whole chain
* @param opts Hardfork options (onlyActive unused)
* @return Array with hardfork arrays
*/
activeHardforks(blockNumber?: BNLike | null, opts: hardforkOptions = {}): HardforkParams[] {
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
if (opts.onlySupported && !this._isSupportedHardfork(hf['name'])) continue

activeHardforks.push(hf)
}
Expand All @@ -814,11 +735,10 @@ export default class Common extends EventEmitter {
/**
* 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
* @param opts Hardfork options (onlyActive unused)
* @return Hardfork name
*/
activeHardfork(blockNumber?: BNLike | null, opts: hardforkOptions = {}): string {
const activeHardforks = this.activeHardforks(blockNumber, opts)
activeHardfork(blockNumber?: BNLike | null): string {
const activeHardforks = this.activeHardforks(blockNumber)
if (activeHardforks.length > 0) {
return activeHardforks[activeHardforks.length - 1]['name']
} else {
Expand All @@ -843,7 +763,7 @@ export default class Common extends EventEmitter {
* @returns Block number or null if unscheduled
*/
hardforkBlockBN(hardfork?: string | Hardfork): BN | null {
hardfork = this._chooseHardfork(hardfork, false)
hardfork = hardfork ?? this._hardfork
const block = this._getHardfork(hardfork)['block']
if (block === undefined || block === null) {
return null
Expand All @@ -857,7 +777,7 @@ export default class Common extends EventEmitter {
* @returns Total difficulty or null if no set
*/
hardforkTD(hardfork?: string | Hardfork): BN | null {
hardfork = this._chooseHardfork(hardfork, false)
hardfork = hardfork ?? this._hardfork
const td = this._getHardfork(hardfork)['td']
if (td === undefined || td === null) {
return null
Expand All @@ -873,7 +793,7 @@ export default class Common extends EventEmitter {
*/
isHardforkBlock(blockNumber: BNLike, hardfork?: string | Hardfork): boolean {
blockNumber = toType(blockNumber, TypeOutput.BN)
hardfork = this._chooseHardfork(hardfork, false)
hardfork = hardfork ?? this._hardfork
const block = this.hardforkBlockBN(hardfork)
return block ? block.eq(blockNumber) : false
}
Expand All @@ -895,7 +815,7 @@ export default class Common extends EventEmitter {
* @returns Block number or null if not available
*/
nextHardforkBlockBN(hardfork?: string | Hardfork): BN | null {
hardfork = this._chooseHardfork(hardfork, false)
hardfork = hardfork ?? this._hardfork
const hfBlock = this.hardforkBlockBN(hardfork)
if (hfBlock === null) {
return null
Expand All @@ -919,7 +839,7 @@ export default class Common extends EventEmitter {
*/
isNextHardforkBlock(blockNumber: BNLike, hardfork?: string | Hardfork): boolean {
blockNumber = toType(blockNumber, TypeOutput.BN)
hardfork = this._chooseHardfork(hardfork, false)
hardfork = hardfork ?? this._hardfork
const nextHardforkBlock = this.nextHardforkBlockBN(hardfork)

return nextHardforkBlock === null ? false : nextHardforkBlock.eq(blockNumber)
Expand Down Expand Up @@ -963,7 +883,7 @@ export default class Common extends EventEmitter {
* @param hardfork Hardfork name, optional if HF set
*/
forkHash(hardfork?: string | Hardfork) {
hardfork = this._chooseHardfork(hardfork, false)
hardfork = hardfork ?? this._hardfork
const data = this._getHardfork(hardfork)
if (data['block'] === null && data['td'] === undefined) {
const msg = 'No fork hash calculation possible for future hardfork'
Expand Down
25 changes: 0 additions & 25 deletions packages/common/tests/chains.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ tape('[Common/Chains]: Initialization / Chain params', function (t: tape.Test) {
c.DEFAULT_HARDFORK,
'should set hardfork to hardfork set as DEFAULT_HARDFORK'
)
st.equal(c._isSupportedHardfork('constantinople'), true, 'should not restrict supported HFs')

c = new Common({ chain: 1 })
st.equal(c.chainName(), 'mainnet', 'should initialize with chain Id')
Expand All @@ -37,7 +36,6 @@ tape('[Common/Chains]: Initialization / Chain params', function (t: tape.Test) {
c.DEFAULT_HARDFORK,
'should set hardfork to hardfork set as DEFAULT_HARDFORK'
)
st.equal(c._isSupportedHardfork('constantinople'), true, 'should not restrict supported HFs')

st.end()
})
Expand All @@ -59,19 +57,6 @@ tape('[Common/Chains]: Initialization / Chain params', function (t: tape.Test) {
}
)

t.test('Should initialize with supportedHardforks provided', function (st: tape.Test) {
const c = new Common({
chain: 'mainnet',
hardfork: 'byzantium',
supportedHardforks: ['byzantium', 'constantinople'],
})
st.equal(c._isSupportedHardfork('byzantium'), true, 'should return true for supported HF')
const msg = 'should return false for unsupported HF'
st.equal(c._isSupportedHardfork('spuriousDragon'), false, msg)

st.end()
})

t.test('Should handle initialization errors', function (st: tape.Test) {
let f = function () {
new Common({ chain: 'chainnotexisting' })
Expand All @@ -85,16 +70,6 @@ tape('[Common/Chains]: Initialization / Chain params', function (t: tape.Test) {
msg = 'should throw an exception on non-existing hardfork'
st.throws(f, /not supported$/, msg) // eslint-disable-line no-new

f = function () {
new Common({
chain: 'mainnet',
hardfork: 'spuriousDragon',
supportedHardforks: ['byzantium', 'constantinople'],
})
}
msg = 'should throw an exception on conflicting active/supported HF params'
st.throws(f, /supportedHardforks$/, msg) // eslint-disable-line no-new

st.end()
})

Expand Down
Loading

0 comments on commit 6b031c0

Please sign in to comment.