From c64f9483bfa99c4b92fe2f5aefa42eacafc6b1cd Mon Sep 17 00:00:00 2001 From: Holger Drewes Date: Mon, 26 Aug 2024 19:49:22 +0200 Subject: [PATCH] Common: Deep-Copy Fix (#3613) * Add test for deep copy * Fix deep copies in Common * Fix and clean-up test * Fix spelling error * Client test fix * Fix difficulty tests --------- Co-authored-by: Amir --- packages/block/test/difficulty.spec.ts | 15 +++++------ .../test/rpc/eth/sendRawTransaction.spec.ts | 6 +++++ packages/common/src/common.ts | 10 ++++---- packages/common/test/chains.spec.ts | 25 +++++++++++++++++++ 4 files changed, 44 insertions(+), 12 deletions(-) diff --git a/packages/block/test/difficulty.spec.ts b/packages/block/test/difficulty.spec.ts index 3bb4afe830..c92ee6eb68 100644 --- a/packages/block/test/difficulty.spec.ts +++ b/packages/block/test/difficulty.spec.ts @@ -39,6 +39,13 @@ const chainTestData: TestData = { } describe('[Header]: difficulty tests', () => { + // Unschedule any timestamp since tests are not configured for timestamps + Mainnet.hardforks + .filter((hf) => hf.timestamp !== undefined) + .map((hf) => { + hf.timestamp = undefined + }) + it('by hardfork', () => { /* eslint-disable no-restricted-syntax */ for (const hardfork in hardforkTestData) { @@ -46,13 +53,7 @@ describe('[Header]: difficulty tests', () => { for (const testName in testData) { const test = testData[testName] const common = new Common({ chain: Mainnet, hardfork }) - // Unschedule any timestamp since tests are not configured for timestamps - common - .hardforks() - .filter((hf) => hf.timestamp !== undefined) - .map((hf) => { - hf.timestamp = undefined - }) + const blockOpts = { common } const uncleHash = test.parentUncles === '0x00' ? undefined : test.parentUncles const parentBlock = createBlock( diff --git a/packages/client/test/rpc/eth/sendRawTransaction.spec.ts b/packages/client/test/rpc/eth/sendRawTransaction.spec.ts index 8d1748049d..9a9247b199 100644 --- a/packages/client/test/rpc/eth/sendRawTransaction.spec.ts +++ b/packages/client/test/rpc/eth/sendRawTransaction.spec.ts @@ -30,6 +30,12 @@ describe(method, () => { DefaultStateManager.prototype.shallowCopy = function () { return this } + // Unschedule any timestamp since tests are not configured for timestamps + Mainnet.hardforks + .filter((hf) => hf.timestamp !== undefined) + .map((hf) => { + hf.timestamp = undefined + }) const common = new Common({ chain: Mainnet }) common .hardforks() diff --git a/packages/common/src/common.ts b/packages/common/src/common.ts index 7b598ae1ba..0b07ac5d84 100644 --- a/packages/common/src/common.ts +++ b/packages/common/src/common.ts @@ -60,7 +60,7 @@ export class Common { constructor(opts: CommonOpts) { this.events = new EventEmitter() - this._chainParams = opts.chain + this._chainParams = JSON.parse(JSON.stringify(opts.chain)) // copy this.DEFAULT_HARDFORK = this._chainParams.defaultHardfork ?? Hardfork.Cancun // Assign hardfork changes in the sequence of the applied hardforks this.HARDFORK_CHANGES = this.hardforks().map((hf) => [ @@ -69,7 +69,7 @@ export class Common { (this._chainParams.customHardforks && this._chainParams.customHardforks[hf.name]), ]) this._hardfork = this.DEFAULT_HARDFORK - this._params = { ...(opts.params ?? {}) } // copy + this._params = opts.params ? JSON.parse(JSON.stringify(opts.params)) : {} // copy if (opts.hardfork !== undefined) { this.setHardfork(opts.hardfork) @@ -105,9 +105,9 @@ export class Common { updateParams(params: ParamsDict) { for (const [eip, paramsConfig] of Object.entries(params)) { if (!(eip in this._params)) { - this._params[eip] = { ...paramsConfig } // copy + this._params[eip] = JSON.parse(JSON.stringify(paramsConfig)) // copy } else { - this._params[eip] = { ...this._params[eip], ...params[eip] } + this._params[eip] = JSON.parse(JSON.stringify({ ...this._params[eip], ...params[eip] })) // copy } } @@ -130,7 +130,7 @@ export class Common { * @param params */ resetParams(params: ParamsDict) { - this._params = { ...params } // copy + this._params = JSON.parse(JSON.stringify(params)) // copy this._buildParamsCache() } diff --git a/packages/common/test/chains.spec.ts b/packages/common/test/chains.spec.ts index d7ad6e6b98..f55f9e7ba6 100644 --- a/packages/common/test/chains.spec.ts +++ b/packages/common/test/chains.spec.ts @@ -10,6 +10,8 @@ import { getPresetChainConfig, } from '../src/index.js' +import type { ChainConfig } from '../src/index.js' + describe('[Common/Chains]: Initialization / Chain params', () => { it('Should initialize with chain provided', () => { const c = new Common({ chain: Mainnet }) @@ -23,6 +25,29 @@ describe('[Common/Chains]: Initialization / Chain params', () => { ) }) + it('Deep copied common object should have parameters that are independent of the original copy', async () => { + let chainConfig: ChainConfig + let c: Common + const setCommon = async () => { + chainConfig = JSON.parse(JSON.stringify(Mainnet)) + c = new Common({ chain: chainConfig }) + assert.equal(c.chainName(), 'mainnet', 'should initialize with chain name') + assert.equal(c.chainId(), BigInt(1), 'should return correct chain Id') + } + + const resetCommon = async () => { + // modify chain config + const cCopy = c.copy() + chainConfig.chainId = 2 + chainConfig.name = 'testnet' + assert.equal(cCopy.chainName(), 'mainnet', 'should return original chain name') + assert.equal(cCopy.chainId(), BigInt(1), 'should return original chain Id') + } + + await setCommon() + await resetCommon() + }) + it('Should initialize with chain provided by chain name or network Id', () => { let chain = getPresetChainConfig('mainnet') let c = new Common({ chain })