Merge branch 'unstable' into nflaig/review-configs

.eslintrc.js

@@ -4,6 +4,7 @@ module.exports = {
     browser: true,
     es6: true,
     node: true,
+    // Performance tests still use mocha
     mocha: true,
   },
   globals: {
@@ -110,7 +111,11 @@ module.exports = {
       "error",
       {
         groups: ["builtin", "external", "internal", "parent", "sibling", "index"],
-        pathGroups: [{pattern: "@lodestar/**", group: "internal"}],
+        pathGroups: [
+          {pattern: "@lodestar/**", group: "internal"},
+          // We want mocks to be imported before any internal code
+          {pattern: "**/mocks/**", group: "internal"},
+        ],
         pathGroupsExcludedImportTypes: ["builtin"],
       },
     ],
@@ -201,24 +206,18 @@ module.exports = {
     },
     {
       files: ["**/test/**/*.test.ts"],
-      plugins: ["mocha", "chai-expect"],
-      extends: ["plugin:mocha/recommended", "plugin:chai-expect/recommended"],
+      plugins: ["vitest"],
+      extends: ["plugin:vitest/recommended"],
       rules: {
-        // We observed that having multiple top level "describe" save valuable indentation
-        // https://github.com/lo1tuma/eslint-plugin-mocha/blob/master/docs/rules/max-top-level-suites.md
-        "mocha/max-top-level-suites": "off",
-        // We need to disable because we disabled "mocha/no-setup-in-describe" rule
-        // TODO: Move all setup code to before/beforeEach and then disable async describe
-        // https://github.com/lo1tuma/eslint-plugin-mocha/blob/master/docs/rules/no-async-describe.md
-        "mocha/no-async-describe": "off",
-        // Use of arrow functions are very common
-        "mocha/no-mocha-arrows": "off",
-        // It's common to call function inside describe block
-        // https://github.com/lo1tuma/eslint-plugin-mocha/blob/master/docs/rules/no-setup-in-describe.md
-        "mocha/no-setup-in-describe": "off",
-        // We use to split before in small isolated tasks
-        // https://github.com/lo1tuma/eslint-plugin-mocha/blob/master/docs/rules/no-sibling-hooks.md
-        "mocha/no-sibling-hooks": "off",
+        "vitest/consistent-test-it": ["error", {fn: "it", withinDescribe: "it"}],
+        // We use a lot dynamic assertions so tests may not have usage of expect
+        "vitest/expect-expect": "off",
+        "vitest/no-disabled-tests": "warn",
+        "vitest/no-focused-tests": "error",
+        "vitest/prefer-called-with": "error",
+        "vitest/prefer-spy-on": "error",
+        // Our usage contains dynamic test title, this rule enforce static string value
+        "vitest/valid-title": "off",
       },
     },
     {

.wordlist.txt

@@ -84,6 +84,7 @@ UPnP
 UTF
 VM
 Vitalik
+Vitest
 Wagyu
 api
 async
@@ -174,6 +175,7 @@ scalability
 secp
 sepolia
 sharding
+src
 ssz
 stakers
 subnet
@@ -188,6 +190,7 @@ util
 utils
 validator
 validators
+vitest
 wip
 xcode
 yaml

CONTRIBUTING.md

@@ -39,15 +39,25 @@ Contributing to tests:
   - Do not pull unpinned versions from DockerHub (use deterministic tag) or Github (checkout commit not branch).
   - Carefully design tests that depend on timing sensitive events like p2p network e2e tests. Consider that Github runners are significantly less powerful than your development environment.
 
+### Common Issues
+
+**Error: [vitest] Cannot mock "../../src/db/repositories/index.js" because it is already loaded by "src/db/beacon.ts"**
+
+If you observe any error in tests with matching to above error message, that implies you are loading the mocks in the wrong order. The correct order is to import the mocks first and then the actual module. We suggest to import the mocks on very top before any local modules. 
+
+**✖ Error: Cannot find package 'async_hooks' imported from**
+
+If you observe following error running any of the test files that means you are running a file which itself or any dependency of that file imports `vitest`, but you are not running that file with `vitest` runner. Try running it with `yarn vitest` command, not with `node` command.
+
 ### Debugging Spec Tests
 
 - To fix errors always focus on passing all minimal tests first without running mainnet tests.
-- Spec tests often compare full expected vs actual states in JSON format. To better understand the diff it's convenient to use mocha's option `--inline-diffs`.
-- A single logical error can cause many spec tests to fail. To focus on a single test at a time you can use mocha's option `--bail` to stop at the first failed test
-- To then run only that failed test you can run against a specific file as use mocha's option `--grep` to run only one case
+- Spec tests often compare full expected vs actual states in JSON format.
+- A single logical error can cause many spec tests to fail. To focus on a single test at a time you can use vitest's option `--bail` to stop at the first failed test
+- To then run only that failed test you can run against a specific file as use vitest's filters to run only one case
 
 ```sh
-LODESTAR_PRESET=minimal ../../node_modules/.bin/mocha --config .mocharc.spec.yml test/spec/phase0/sanity.test.ts --inline-diffs --bail --grep "attestation"
+LODESTAR_PRESET=minimal yarn vitest --run --config vitest.config.spec.ts test/spec/phase0/sanity.test.ts
 ```
 
 ## Docker

docs/pages/contribution/testing/integration-tests.md

@@ -13,7 +13,7 @@ There are two ENV variables that are required to run this test:
 
 The command to run this test is:
 
-`EL_BINARY_DIR=g11tech/geth:withdrawals EL_SCRIPT_DIR=gethdocker yarn mocha test/sim/withdrawal-interop.test.ts`
+`EL_BINARY_DIR=g11tech/geth:withdrawals EL_SCRIPT_DIR=gethdocker yarn vitest --run test/sim/withdrawal-interop.test.ts`
 
 The images used by this test during CI are:
 

docs/pages/validator-management/vc-configuration.md

@@ -103,7 +103,7 @@ With Lodestar's `--builder.selection` validator options, you can select:
 
 #### Calculating builder boost factor with examples
 
-To calculate the builder boost factor setting, you need to know what percentage you will accept a builder block for against a local execution block using the following formula: `100*100/(100+percentage)`.
+To calculate the builder boost factor setting, you need to know what percentage you will accept a builder block for against a local execution block using the following formula: `100*100/(100+percentage)`. The value passed to `--builder.boostFactor` must be a valid number without decimals.
 
 Example 1: I will only accept a builder block with 25% more value than the local execution block.
 ```

package.json

@@ -47,69 +47,45 @@
   "devDependencies": {
     "@chainsafe/eslint-plugin-node": "^11.2.3",
     "@dapplion/benchmark": "^0.2.4",
-    "@types/chai": "^4.3.6",
-    "@types/chai-as-promised": "^7.1.6",
-    "@types/mocha": "^10.0.1",
+    "@types/mocha": "^10.0.6",
     "@types/node": "^20.6.5",
-    "@types/sinon": "^10.0.16",
-    "@types/sinon-chai": "^3.2.9",
     "@typescript-eslint/eslint-plugin": "6.7.2",
     "@typescript-eslint/parser": "6.7.2",
-    "@vitest/coverage-v8": "^1.1.0",
-    "@vitest/browser": "^1.1.0",
-    "c8": "^8.0.1",
-    "chai": "^4.3.8",
-    "chai-as-promised": "^7.1.1",
+    "@vitest/coverage-v8": "^1.2.1",
+    "@vitest/browser": "^1.2.1",
     "codecov": "^3.8.3",
     "crypto-browserify": "^3.12.0",
     "electron": "^26.2.2",
     "eslint": "^8.50.0",
     "eslint-import-resolver-typescript": "^3.6.1",
-    "eslint-plugin-chai-expect": "^3.0.0",
     "eslint-plugin-import": "^2.28.1",
-    "eslint-plugin-mocha": "^10.2.0",
     "eslint-plugin-prettier": "^5.0.0",
+    "eslint-plugin-vitest": "^0.3.20",
     "https-browserify": "^1.0.0",
     "jsdom": "^23.0.1",
-    "karma": "^6.4.2",
-    "karma-chai": "^0.1.0",
-    "karma-chrome-launcher": "^3.2.0",
-    "karma-cli": "^2.0.0",
-    "karma-electron": "^7.3.0",
-    "karma-firefox-launcher": "^2.1.2",
-    "karma-mocha": "^2.0.1",
-    "karma-spec-reporter": "^0.0.36",
-    "karma-webpack": "^5.0.0",
     "lerna": "^7.3.0",
     "libp2p": "1.1.1",
     "mocha": "^10.2.0",
     "node-gyp": "^9.4.0",
     "npm-run-all": "^4.1.5",
-    "nyc": "^15.1.0",
     "path-browserify": "^1.0.1",
     "prettier": "^3.0.3",
     "process": "^0.11.10",
-    "resolve-typescript-plugin": "^2.0.1",
-    "sinon": "^16.0.0",
-    "sinon-chai": "^3.7.0",
     "stream-browserify": "^3.0.0",
     "stream-http": "^3.2.0",
     "supertest": "^6.3.3",
-    "ts-loader": "^9.4.4",
     "ts-node": "^10.9.1",
     "typescript": "^5.2.2",
     "typescript-docs-verifier": "^2.5.0",
-    "vite-plugin-node-polyfills": "^0.18.0",
+    "vite-plugin-node-polyfills": "^0.19.0",
     "vite-plugin-top-level-await": "^1.4.1",
-    "vitest": "^1.1.0",
-    "vitest-when": "^0.3.0",
+    "vitest": "^1.2.1",
+    "vitest-when": "^0.3.1",
     "wait-port": "^1.1.0",
-    "webdriverio": "^8.27.0",
-    "webpack": "^5.88.2"
+    "webdriverio": "^8.28.0"
   },
   "resolutions": {
     "dns-over-http-resolver": "^2.1.1",
-    "chai": "^4.3.10",
     "loupe": "^2.3.6",
     "vite": "^5.0.0"
   }

packages/api/package.json

@@ -65,7 +65,7 @@
     "lint:fix": "yarn run lint --fix",
     "pretest": "yarn run check-types",
     "test": "yarn test:unit && yarn test:e2e",
-    "test:unit": "vitest --run --dir test/unit/ --coverage",
+    "test:unit": "vitest --run --dir test/unit/",
     "check-readme": "typescript-docs-verifier"
   },
   "dependencies": {

packages/api/test/utils/utils.ts

@@ -38,7 +38,6 @@ export function getTestServer(): {baseUrl: string; server: FastifyInstance} {
   return {baseUrl, server};
 }
 
-/** Type helper to get a Sinon mock object type with Api */
 export function getMockApi<Api extends Record<string, any>>(
   routeIds: Record<string, any>
 ): MockedObject<ServerApi<Api>> & ServerApi<Api> {

packages/api/vitest.config.ts

@@ -1,5 +1,5 @@
 import {defineConfig, mergeConfig} from "vitest/config";
-import vitestConfig from "../../vitest.base.config";
+import vitestConfig from "../../vitest.base.unit.config";
 
 export default mergeConfig(
   vitestConfig,

packages/beacon-node/package.json

@@ -77,15 +77,15 @@
     "lint:fix": "yarn run lint --fix",
     "pretest": "yarn run check-types",
     "test": "yarn test:unit && yarn test:e2e",
-    "test:unit:minimal": "vitest --run --segfaultRetry 3 --dir test/unit/ --coverage",
-    "test:unit:mainnet": "LODESTAR_PRESET=mainnet nyc --cache-dir .nyc_output/.cache -e .ts mocha 'test/unit-mainnet/**/*.test.ts'",
+    "test:unit:minimal": "vitest --run --segfaultRetry 3 --dir test/unit/",
+    "test:unit:mainnet": "LODESTAR_PRESET=mainnet vitest --run --dir test/unit-mainnet",
     "test:unit": "wrapper() { yarn test:unit:minimal $@ && yarn test:unit:mainnet $@; }; wrapper",
-    "test:e2e": "LODESTAR_PRESET=minimal vitest --run --segfaultRetry 3 --poolOptions.threads.singleThread true --dir test/e2e",
-    "test:sim": "mocha 'test/sim/**/*.test.ts'",
-    "test:sim:merge-interop": "mocha 'test/sim/merge-interop.test.ts'",
-    "test:sim:mergemock": "mocha 'test/sim/mergemock.test.ts'",
-    "test:sim:withdrawals": "mocha 'test/sim/withdrawal-interop.test.ts'",
-    "test:sim:blobs": "mocha 'test/sim/4844-interop.test.ts'",
+    "test:e2e": "LODESTAR_PRESET=minimal vitest --run --segfaultRetry 3 --config vitest.config.e2e.ts --dir test/e2e",
+    "test:sim": "vitest --run test/sim/**/*.test.ts",
+    "test:sim:merge-interop": "vitest --run test/sim/merge-interop.test.ts",
+    "test:sim:mergemock": "vitest --run test/sim/mergemock.test.ts",
+    "test:sim:withdrawals": "vitest --run test/sim/withdrawal-interop.test.ts",
+    "test:sim:blobs": "vitest --run test/sim/4844-interop.test.ts",
     "download-spec-tests": "node --loader=ts-node/esm test/spec/downloadTests.ts",
     "test:spec:bls": "vitest --run --config vitest.config.spec.ts --dir test/spec/bls/",
     "test:spec:general": "vitest --run --config vitest.config.spec.ts --dir test/spec/general/",

packages/beacon-node/src/network/peers/client.ts

@@ -7,7 +7,13 @@ export enum ClientKind {
   Unknown = "Unknown",
 }
 
-export function clientFromAgentVersion(agentVersion: string): ClientKind {
+/**
+ * Get known client from agent version.
+ * If client is not known, don't return ClientKind.Unknown here.
+ * For metrics it'll have fallback logic to use ClientKind.Unknown
+ * For logs, we want to print out agentVersion instead for debugging purposes.
+ */
+export function getKnownClientFromAgentVersion(agentVersion: string): ClientKind | null {
   const slashIndex = agentVersion.indexOf("/");
   const agent = slashIndex >= 0 ? agentVersion.slice(0, slashIndex) : agentVersion;
   const agentLC = agent.toLowerCase();
@@ -16,5 +22,6 @@ export function clientFromAgentVersion(agentVersion: string): ClientKind {
   if (agentLC === "prysm") return ClientKind.Prysm;
   if (agentLC === "nimbus") return ClientKind.Nimbus;
   if (agentLC === "lodestar" || agentLC === "js-libp2p") return ClientKind.Lodestar;
-  return ClientKind.Unknown;
+
+  return null;
 }

packages/beacon-node/src/network/peers/peerManager.ts

@@ -19,7 +19,7 @@ import {NetworkCoreMetrics} from "../core/metrics.js";
 import {LodestarDiscv5Opts} from "../discv5/types.js";
 import {PeerDiscovery, SubnetDiscvQueryMs} from "./discover.js";
 import {PeersData, PeerData} from "./peersData.js";
-import {clientFromAgentVersion, ClientKind} from "./client.js";
+import {getKnownClientFromAgentVersion, ClientKind} from "./client.js";
 import {
   getConnectedPeerIds,
   hasSomeConnectedPeer,
@@ -615,7 +615,7 @@ export class PeerManager {
         if (agentVersionBytes) {
           const agentVersion = new TextDecoder().decode(agentVersionBytes) || "N/A";
           peerData.agentVersion = agentVersion;
-          peerData.agentClient = clientFromAgentVersion(agentVersion);
+          peerData.agentClient = getKnownClientFromAgentVersion(agentVersion);
         }
       },
       {retries: 3, retryDelay: 1000}

packages/beacon-node/src/network/peers/peersData.ts

@@ -38,8 +38,8 @@ export class PeersData {
     return this.connectedPeers.get(peerIdStr)?.agentVersion ?? "NA";
   }
 
-  getPeerKind(peerIdStr: string): ClientKind {
-    return this.connectedPeers.get(peerIdStr)?.agentClient ?? ClientKind.Unknown;
+  getPeerKind(peerIdStr: string): ClientKind | null {
+    return this.connectedPeers.get(peerIdStr)?.agentClient ?? null;
   }
 
   getEncodingPreference(peerIdStr: string): Encoding | null {

packages/beacon-node/src/network/reqresp/ReqRespBeaconNode.ts

@@ -88,7 +88,8 @@ export class ReqRespBeaconNode extends ReqResp {
           metrics?.reqResp.rateLimitErrors.inc({method});
         },
         getPeerLogMetadata(peerId) {
-          return peersData.getPeerKind(peerId);
+          // this logs the whole agent version for unknown client which is good for debugging
+          return peersData.getPeerKind(peerId) ?? peersData.getAgentVersion(peerId);
         },
       }
     );