Fix client integration tests (ethereumjs#1956)

* Re-enable integration tests

* Remove hardcoded difficulty in mockchain blocks

* Check for subchain before examining bounds

* Copy common to avoid max listener warnings

* make destroyServer async

* Proper fix for setting difficulty in mockchain

* reqId fix

* remove the redundant while loop

* wait for setTineout to clear out

Co-authored-by: harkamal <[email protected]>

Author: acolytec3

.github/workflows/client-build.yml

@@ -45,6 +45,8 @@ jobs:
           flags: client
         if: ${{ matrix.node-version == 16 }}
 
+      - run: npm run test:integration
+      
       - run: npm run lint
   test-client-cli:
     runs-on: ubuntu-latest

packages/client/lib/net/protocol/ethprotocol.ts

@@ -75,8 +75,6 @@ export interface EthProtocolMethods {
   getReceipts: (opts: GetReceiptsOpts) => Promise<[bigint, TxReceipt[]]>
 }
 
-let id = BigInt(0)
-
 /**
  * Implements eth/66 protocol
  * @memberof module:net/protocol
@@ -119,7 +117,7 @@ export class EthProtocol extends Protocol {
       code: 0x03,
       response: 0x04,
       encode: ({ reqId, block, max, skip = 0, reverse = false }: GetBlockHeadersOpts) => [
-        bigIntToBuffer(reqId ?? ++id),
+        bigIntToBuffer(reqId ?? ++this.nextReqId),
         [
           typeof block === 'bigint' ? bigIntToBuffer(block) : block,
           max === 0 ? Buffer.from([]) : intToBuffer(max),
@@ -160,7 +158,10 @@ export class EthProtocol extends Protocol {
       name: 'GetBlockBodies',
       code: 0x05,
       response: 0x06,
-      encode: ({ reqId, hashes }: GetBlockBodiesOpts) => [bigIntToBuffer(reqId ?? ++id), hashes],
+      encode: ({ reqId, hashes }: GetBlockBodiesOpts) => [
+        bigIntToBuffer(reqId ?? ++this.nextReqId),
+        hashes,
+      ],
       decode: ([reqId, hashes]: [Buffer, Buffer[]]) => ({
         reqId: bufferToBigInt(reqId),
         hashes,
@@ -196,7 +197,7 @@ export class EthProtocol extends Protocol {
       code: 0x09,
       response: 0x0a,
       encode: ({ reqId, hashes }: GetPooledTransactionsOpts) => [
-        bigIntToBuffer(reqId ?? ++id),
+        bigIntToBuffer(reqId ?? ++this.nextReqId),
         hashes,
       ],
       decode: ([reqId, hashes]: [Buffer, Buffer[]]) => ({
@@ -231,7 +232,7 @@ export class EthProtocol extends Protocol {
       code: 0x0f,
       response: 0x10,
       encode: ({ reqId, hashes }: { reqId: bigint; hashes: Buffer[] }) => [
-        bigIntToBuffer(reqId ?? ++id),
+        bigIntToBuffer(reqId ?? ++this.nextReqId),
         hashes,
       ],
       decode: ([reqId, hashes]: [Buffer, Buffer[]]) => ({

packages/client/lib/net/protocol/lesprotocol.ts

@@ -34,8 +34,6 @@ export interface LesProtocolMethods {
   ) => Promise<{ reqId: bigint; bv: bigint; headers: BlockHeader[] }>
 }
 
-let id = BigInt(0)
-
 /**
  * Implements les/1 and les/2 protocols
  * @memberof module:net/protocol
@@ -44,7 +42,9 @@ export class LesProtocol extends Protocol {
   private chain: Chain
   private flow: FlowControl | undefined
   private isServer: boolean
+  private nextReqId = BigInt(0)
 
+  /* eslint-disable no-invalid-this */
   private protocolMessages: Message[] = [
     {
       name: 'Announce',
@@ -69,7 +69,7 @@ export class LesProtocol extends Protocol {
       code: 0x02,
       response: 0x03,
       encode: ({ reqId, block, max, skip = 0, reverse = false }: GetBlockHeadersOpts) => [
-        bigIntToBuffer(reqId ?? ++id),
+        bigIntToBuffer(reqId ?? ++this.nextReqId),
         [typeof block === 'bigint' ? bigIntToBuffer(block) : block, max, skip, !reverse ? 0 : 1],
       ],
       decode: ([reqId, [block, max, skip, reverse]]: any) => ({

packages/client/lib/sync/beaconsync.ts

@@ -249,7 +249,8 @@ export class BeaconSynchronizer extends Synchronizer {
     // Execute single block when within 50 blocks of head,
     // otherwise run execution in batch of 50 blocks when filling canonical chain.
     if (
-      this.chain.blocks.height > this.skeleton.bounds().head - BigInt(50) ||
+      (this.skeleton.bounds() &&
+        this.chain.blocks.height > this.skeleton.bounds().head - BigInt(50)) ||
       this.chain.blocks.height % BigInt(50) === BigInt(0)
     ) {
       void this.execution.run(false)

packages/client/test/integration/mocks/mockchain.ts

@@ -1,4 +1,5 @@
 import { Block } from '@ethereumjs/block'
+import { Hardfork } from '@ethereumjs/common'
 import { Chain, ChainOptions } from '../../../lib/blockchain'
 
 interface MockChainOptions extends ChainOptions {
@@ -29,7 +30,7 @@ export default class MockChain extends Chain {
         {
           header: {
             number: number + 1,
-            difficulty: 1,
+            difficulty: common.gteHardfork(Hardfork.Merge) ? 0 : 1,
             parentHash: number ? blocks[number - 1].hash() : this.genesis.hash(),
           },
         },

packages/client/test/integration/mocks/mockserver.ts

@@ -1,7 +1,7 @@
 import { Server, ServerOptions } from '../../../lib/net/server'
 import { Event } from '../../../lib/types'
 import MockPeer from './mockpeer'
-import { RemoteStream, createServer, destroyServer } from './network'
+import { RemoteStream, createServer, destroyServer, servers } from './network'
 
 interface MockServerOptions extends ServerOptions {
   location?: string
@@ -42,8 +42,12 @@ export default class MockServer extends Server {
   }
 
   async stop(): Promise<boolean> {
+    // This wait is essential to clear out the pending setTimeout in the
+    // createStream in ./network.ts
     await this.wait(20)
-    destroyServer(this.location)
+    while (servers[this.location]) {
+      await destroyServer(this.location)
+    }
     await super.stop()
     return this.started
   }

packages/client/test/integration/mocks/network.ts

@@ -45,13 +45,14 @@ export function destroyStream(id: string, location: string) {
   }
 }
 
-export function destroyServer(location: string) {
-  if (servers[location]) {
+export async function destroyServer(location: string) {
+  return new Promise<void>((resolve) => {
     for (const id of Object.keys(servers[location].streams)) {
       destroyStream(id, location)
     }
-  }
-  delete servers[location]
+    delete servers[location]
+    resolve()
+  })
 }
 
 export function createStream(id: string, location: string, protocols: string[]) {

packages/client/test/integration/util.ts

@@ -23,7 +23,7 @@ export async function setup(
   const minPeers = options.minPeers ?? 1
 
   const lightserv = syncmode === 'full'
-  const common = options.common
+  const common = options.common?.copy()
   const config = new Config({ syncmode, lightserv, minPeers, common, safeReorgDistance: 0 })
 
   const server = new MockServer({ config, location })