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 <[email protected]>
ethereumjs · Aug 26, 2024 · c64f948 · c64f948
1 parent 01f367f
commit c64f948
Show file tree

Hide file tree

Showing 4 changed files with 44 additions and 12 deletions.
diff --git a/packages/block/test/difficulty.spec.ts b/packages/block/test/difficulty.spec.ts
@@ -39,20 +39,21 @@ 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) {
       const testData = hardforkTestData[hardfork]
       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
@@ -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
@@ -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
@@ -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 })