From a7662201f221bccee5a272a1d90739c4357bdee3 Mon Sep 17 00:00:00 2001 From: Vasyl Ivanchuk Date: Mon, 20 Jan 2025 18:48:56 +0200 Subject: [PATCH 1/6] fix: health checks for worker and fetcher (#374) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # What ❔ Improve health checks for worker and data fetcher services to check RPC availability and DB (for worker). ## Why ❔ Since RPC availability is not checked if the connection from the service to the RPC goes down, the service keeps receiving requests. The problem is described in [this](https://github.com/matter-labs/block-explorer/issues/368) issue. ## Checklist - [X] PR title corresponds to the body of PR (we generate changelog entries from PRs). - [X] Tests for the changes have been added / updated. --- package-lock.json | 26 +++-- packages/data-fetcher/.env.example | 4 +- packages/data-fetcher/package.json | 2 + packages/data-fetcher/src/config.spec.ts | 3 + packages/data-fetcher/src/config.ts | 4 + .../data-fetcher/src/health/health.module.ts | 3 +- .../src/health/jsonRpcProvider.health.spec.ts | 105 +++++++++++++----- .../src/health/jsonRpcProvider.health.ts | 54 ++++++++- packages/worker/.env.example | 3 + packages/worker/src/config.spec.ts | 8 ++ packages/worker/src/config.ts | 6 + .../src/health/health.controller.spec.ts | 14 ++- .../worker/src/health/health.controller.ts | 8 +- packages/worker/src/health/health.module.ts | 3 +- .../src/health/jsonRpcProvider.health.spec.ts | 105 +++++++++++++----- .../src/health/jsonRpcProvider.health.ts | 54 ++++++++- 16 files changed, 323 insertions(+), 79 deletions(-) diff --git a/package-lock.json b/package-lock.json index bf83baeca7..c4b8ffb039 100644 --- a/package-lock.json +++ b/package-lock.json @@ -5503,13 +5503,13 @@ "dev": true }, "node_modules/@nestjs/axios": { - "version": "3.0.0", - "resolved": "https://registry.npmjs.org/@nestjs/axios/-/axios-3.0.0.tgz", - "integrity": "sha512-ULdH03jDWkS5dy9X69XbUVbhC+0pVnrRcj7bIK/ytTZ76w7CgvTZDJqsIyisg3kNOiljRW/4NIjSf3j6YGvl+g==", + "version": "3.1.3", + "resolved": "https://registry.npmjs.org/@nestjs/axios/-/axios-3.1.3.tgz", + "integrity": "sha512-RZ/63c1tMxGLqyG3iOCVt7A72oy4x1eM6QEhd4KzCYpaVWW0igq0WSREeRoEZhIxRcZfDfIIkvsOMiM7yfVGZQ==", + "license": "MIT", "peerDependencies": { "@nestjs/common": "^7.0.0 || ^8.0.0 || ^9.0.0 || ^10.0.0", "axios": "^1.3.1", - "reflect-metadata": "^0.1.12", "rxjs": "^6.0.0 || ^7.0.0" } }, @@ -19048,11 +19048,12 @@ } }, "node_modules/axios": { - "version": "1.5.1", - "resolved": "https://registry.npmjs.org/axios/-/axios-1.5.1.tgz", - "integrity": "sha512-Q28iYCWzNHjAm+yEAot5QaAMxhMghWLFVf7rRdwhUI+c2jix2DUXjAHXVi+s1ibs3mjPO/cCgbA++3BjD0vP/A==", + "version": "1.7.9", + "resolved": "https://registry.npmjs.org/axios/-/axios-1.7.9.tgz", + "integrity": "sha512-LhLcE7Hbiryz8oMDdDptSrWowmB4Bl6RCt6sIJKpRB4XtVf0iEgewX3au/pJqm+Py1kCASkb/FFKjxQaLtxJvw==", + "license": "MIT", "dependencies": { - "follow-redirects": "^1.15.0", + "follow-redirects": "^1.15.6", "form-data": "^4.0.0", "proxy-from-env": "^1.1.0" } @@ -26015,15 +26016,16 @@ "integrity": "sha512-GRnmB5gPyJpAhTQdSZTSp9uaPSvl09KoYcMQtsB9rQoOmzs9dH6ffeccH+Z+cv6P68Hu5bC6JjRh4Ah/mHSNRw==" }, "node_modules/follow-redirects": { - "version": "1.15.3", - "resolved": "https://registry.npmjs.org/follow-redirects/-/follow-redirects-1.15.3.tgz", - "integrity": "sha512-1VzOtuEM8pC9SFU1E+8KfTjZyMztRsgEfwQl44z8A25uy13jSzTj6dyK2Df52iV0vgHCfBwLhDWevLn95w5v6Q==", + "version": "1.15.9", + "resolved": "https://registry.npmjs.org/follow-redirects/-/follow-redirects-1.15.9.tgz", + "integrity": "sha512-gew4GsXizNgdoRyqmyfMHyAmXsZDk6mHkSxZFCzW9gwlbtOW44CDtYavM+y+72qD/Vq2l550kMF52DT8fOLJqQ==", "funding": [ { "type": "individual", "url": "https://github.com/sponsors/RubenVerborgh" } ], + "license": "MIT", "engines": { "node": ">=4.0" }, @@ -54387,12 +54389,14 @@ "version": "0.0.0", "license": "MIT", "dependencies": { + "@nestjs/axios": "^3.1.3", "@nestjs/common": "^9.0.0", "@nestjs/config": "^2.2.0", "@nestjs/core": "^9.0.0", "@nestjs/platform-express": "^9.0.0", "@nestjs/terminus": "^9.1.2", "@willsoto/nestjs-prometheus": "^4.7.0", + "axios": "^1.7.9", "ethers": "6.13.4", "nest-winston": "^1.7.0", "prom-client": "^14.1.0", diff --git a/packages/data-fetcher/.env.example b/packages/data-fetcher/.env.example index cddefc6c7b..7371b2d632 100644 --- a/packages/data-fetcher/.env.example +++ b/packages/data-fetcher/.env.example @@ -16,4 +16,6 @@ RPC_BATCH_MAX_COUNT=10 RPC_BATCH_MAX_SIZE_BYTES=1048576 RPC_BATCH_STALL_TIME_MS=0 -MAX_BLOCKS_BATCH_SIZE=20 \ No newline at end of file +MAX_BLOCKS_BATCH_SIZE=20 + +RPC_HEALTH_CHECK_TIMEOUT_MS=20_000 \ No newline at end of file diff --git a/packages/data-fetcher/package.json b/packages/data-fetcher/package.json index 6c0ce6cb5a..8a0afc8981 100644 --- a/packages/data-fetcher/package.json +++ b/packages/data-fetcher/package.json @@ -24,12 +24,14 @@ "test:e2e": "jest --config ./test/jest-e2e.json" }, "dependencies": { + "@nestjs/axios": "^3.1.3", "@nestjs/common": "^9.0.0", "@nestjs/config": "^2.2.0", "@nestjs/core": "^9.0.0", "@nestjs/platform-express": "^9.0.0", "@nestjs/terminus": "^9.1.2", "@willsoto/nestjs-prometheus": "^4.7.0", + "axios": "^1.7.9", "ethers": "6.13.4", "nest-winston": "^1.7.0", "prom-client": "^14.1.0", diff --git a/packages/data-fetcher/src/config.spec.ts b/packages/data-fetcher/src/config.spec.ts index aae8bc6c74..23b33571ca 100644 --- a/packages/data-fetcher/src/config.spec.ts +++ b/packages/data-fetcher/src/config.spec.ts @@ -22,6 +22,9 @@ describe("config", () => { }, maxBlocksBatchSize: 20, gracefulShutdownTimeoutMs: 0, + healthChecks: { + rpcHealthCheckTimeoutMs: 20_000, + }, }; }); diff --git a/packages/data-fetcher/src/config.ts b/packages/data-fetcher/src/config.ts index c5d1ef86ce..62dd4962f5 100644 --- a/packages/data-fetcher/src/config.ts +++ b/packages/data-fetcher/src/config.ts @@ -15,6 +15,7 @@ export default () => { RPC_BATCH_STALL_TIME_MS, MAX_BLOCKS_BATCH_SIZE, GRACEFUL_SHUTDOWN_TIMEOUT_MS, + RPC_HEALTH_CHECK_TIMEOUT_MS, } = process.env; return { @@ -42,5 +43,8 @@ export default () => { }, maxBlocksBatchSize: parseInt(MAX_BLOCKS_BATCH_SIZE, 10) || 20, gracefulShutdownTimeoutMs: parseInt(GRACEFUL_SHUTDOWN_TIMEOUT_MS, 10) || 0, + healthChecks: { + rpcHealthCheckTimeoutMs: parseInt(RPC_HEALTH_CHECK_TIMEOUT_MS, 10) || 20_000, + }, }; }; diff --git a/packages/data-fetcher/src/health/health.module.ts b/packages/data-fetcher/src/health/health.module.ts index dae128825d..1c4b98ad8c 100644 --- a/packages/data-fetcher/src/health/health.module.ts +++ b/packages/data-fetcher/src/health/health.module.ts @@ -1,11 +1,12 @@ import { Module } from "@nestjs/common"; import { TerminusModule } from "@nestjs/terminus"; +import { HttpModule } from "@nestjs/axios"; import { HealthController } from "./health.controller"; import { JsonRpcHealthIndicator } from "./jsonRpcProvider.health"; @Module({ controllers: [HealthController], - imports: [TerminusModule], + imports: [TerminusModule, HttpModule], providers: [JsonRpcHealthIndicator], }) export class HealthModule {} diff --git a/packages/data-fetcher/src/health/jsonRpcProvider.health.spec.ts b/packages/data-fetcher/src/health/jsonRpcProvider.health.spec.ts index 2cfaa28919..e495025bbb 100644 --- a/packages/data-fetcher/src/health/jsonRpcProvider.health.spec.ts +++ b/packages/data-fetcher/src/health/jsonRpcProvider.health.spec.ts @@ -1,17 +1,20 @@ import { Test, TestingModule } from "@nestjs/testing"; +import { Logger } from "@nestjs/common"; import { mock } from "jest-mock-extended"; -import { HealthCheckError } from "@nestjs/terminus"; import { JsonRpcProviderBase } from "../rpcProvider"; import { JsonRpcHealthIndicator } from "./jsonRpcProvider.health"; +import { ConfigService } from "@nestjs/config"; +import { HttpService } from "@nestjs/axios"; +import { of, throwError } from "rxjs"; +import { AxiosError } from "axios"; describe("JsonRpcHealthIndicator", () => { - const healthIndicatorKey = "rpcProvider"; let jsonRpcProviderMock: JsonRpcProviderBase; let jsonRpcHealthIndicator: JsonRpcHealthIndicator; + let httpService: HttpService; + let configService: ConfigService; - beforeEach(async () => { - jsonRpcProviderMock = mock(); - + const getHealthIndicator = async () => { const app: TestingModule = await Test.createTestingModule({ providers: [ JsonRpcHealthIndicator, @@ -19,38 +22,90 @@ describe("JsonRpcHealthIndicator", () => { provide: JsonRpcProviderBase, useValue: jsonRpcProviderMock, }, + { + provide: HttpService, + useValue: httpService, + }, + { + provide: ConfigService, + useValue: configService, + }, ], }).compile(); - jsonRpcHealthIndicator = app.get(JsonRpcHealthIndicator); + app.useLogger(mock()); + return app.get(JsonRpcHealthIndicator); + }; + + beforeEach(async () => { + jsonRpcProviderMock = mock(); + + httpService = mock({ + post: jest.fn(), + }); + + configService = mock({ + get: jest.fn().mockImplementation((key: string) => { + if (key === "blockchain.rpcUrl") return "http://localhost:3050"; + if (key === "healthChecks.rpcHealthCheckTimeoutMs") return 5000; + return null; + }), + }); + + jsonRpcHealthIndicator = await getHealthIndicator(); }); describe("isHealthy", () => { - describe("when rpcProvider is open", () => { - beforeEach(() => { - jest.spyOn(jsonRpcProviderMock, "getState").mockReturnValueOnce("open"); - }); + const rpcRequest = { + id: 1, + jsonrpc: "2.0", + method: "eth_chainId", + params: [], + }; - it("returns OK health indicator result", async () => { - const result = await jsonRpcHealthIndicator.isHealthy(healthIndicatorKey); - expect(result).toEqual({ [healthIndicatorKey]: { rpcProviderState: "open", status: "up" } }); + it("returns healthy status when RPC responds successfully", async () => { + (httpService.post as jest.Mock).mockReturnValueOnce(of({ data: { result: "0x1" } })); + const result = await jsonRpcHealthIndicator.isHealthy("jsonRpcProvider"); + expect(result).toEqual({ + jsonRpcProvider: { + status: "up", + }, }); + expect(httpService.post).toHaveBeenCalledWith("http://localhost:3050", rpcRequest, { timeout: 5000 }); }); - describe("when rpcProvider is closed", () => { - beforeEach(() => { - jest.spyOn(jsonRpcProviderMock, "getState").mockReturnValueOnce("closed"); - }); + it("throws HealthCheckError when RPC request fails", async () => { + const error = new AxiosError(); + error.response = { + status: 503, + data: "Service Unavailable", + } as any; - it("throws HealthCheckError error", async () => { - expect.assertions(2); - try { - await jsonRpcHealthIndicator.isHealthy(healthIndicatorKey); - } catch (error) { - expect(error).toBeInstanceOf(HealthCheckError); - expect(error.message).toBe("JSON RPC provider is not in open state"); - } + (httpService.post as jest.Mock).mockReturnValueOnce(throwError(() => error)); + await expect(jsonRpcHealthIndicator.isHealthy("jsonRpcProvider")).rejects.toThrow(); + expect(httpService.post).toHaveBeenCalledWith("http://localhost:3050", rpcRequest, { timeout: 5000 }); + }); + + it("throws HealthCheckError when RPC request times out", async () => { + const error = new AxiosError(); + error.code = "ECONNABORTED"; + + (httpService.post as jest.Mock).mockReturnValueOnce(throwError(() => error)); + await expect(jsonRpcHealthIndicator.isHealthy("jsonRpcProvider")).rejects.toThrow(); + expect(httpService.post).toHaveBeenCalledWith("http://localhost:3050", rpcRequest, { timeout: 5000 }); + }); + + it("uses configured timeout from config service", async () => { + (configService.get as jest.Mock).mockImplementation((key: string) => { + if (key === "blockchain.rpcUrl") return "http://localhost:3050"; + if (key === "healthChecks.rpcHealthCheckTimeoutMs") return 10000; + return null; }); + jsonRpcHealthIndicator = await getHealthIndicator(); + + (httpService.post as jest.Mock).mockReturnValueOnce(of({ data: { result: "0x1" } })); + await jsonRpcHealthIndicator.isHealthy("jsonRpcProvider"); + expect(httpService.post).toHaveBeenCalledWith("http://localhost:3050", rpcRequest, { timeout: 10000 }); }); }); }); diff --git a/packages/data-fetcher/src/health/jsonRpcProvider.health.ts b/packages/data-fetcher/src/health/jsonRpcProvider.health.ts index 60406bfb4c..e5434382b2 100644 --- a/packages/data-fetcher/src/health/jsonRpcProvider.health.ts +++ b/packages/data-fetcher/src/health/jsonRpcProvider.health.ts @@ -1,22 +1,64 @@ import { Injectable } from "@nestjs/common"; import { HealthIndicator, HealthIndicatorResult, HealthCheckError } from "@nestjs/terminus"; -import { JsonRpcProviderBase } from "../rpcProvider"; +import { ConfigService } from "@nestjs/config"; +import { Logger } from "@nestjs/common"; +import { HttpService } from "@nestjs/axios"; +import { catchError, firstValueFrom } from "rxjs"; +import { AxiosError } from "axios"; @Injectable() export class JsonRpcHealthIndicator extends HealthIndicator { - constructor(private readonly provider: JsonRpcProviderBase) { + private readonly rpcUrl: string; + private readonly healthCheckTimeoutMs: number; + private readonly logger: Logger; + + constructor(configService: ConfigService, private readonly httpService: HttpService) { super(); + this.logger = new Logger(JsonRpcHealthIndicator.name); + this.rpcUrl = configService.get("blockchain.rpcUrl"); + this.healthCheckTimeoutMs = configService.get("healthChecks.rpcHealthCheckTimeoutMs"); } async isHealthy(key: string): Promise { - const rpcProviderState = this.provider.getState(); - const isHealthy = rpcProviderState === "open"; - const result = this.getStatus(key, isHealthy, { rpcProviderState }); + let isHealthy = true; + try { + // Check RPC health with a pure HTTP request to remove SDK out of the picture + // and avoid any SDK-specific issues. + // Use eth_chainId call as it is the lightest one and return a static value from the memory. + await firstValueFrom( + this.httpService + .post( + this.rpcUrl, + { + id: 1, + jsonrpc: "2.0", + method: "eth_chainId", + params: [], + }, + { timeout: this.healthCheckTimeoutMs } + ) + .pipe( + catchError((error: AxiosError) => { + this.logger.error({ + message: `Failed to ping RPC`, + stack: error.stack, + status: error.response?.status, + response: error.response?.data, + }); + throw error; + }) + ) + ); + } catch { + isHealthy = false; + } + + const result = this.getStatus(key, isHealthy, { status: isHealthy ? "up" : "down" }); if (isHealthy) { return result; } - throw new HealthCheckError("JSON RPC provider is not in open state", result); + throw new HealthCheckError("JSON RPC provider is down or not reachable", result); } } diff --git a/packages/worker/.env.example b/packages/worker/.env.example index d9773b9d8b..9a551c5b21 100644 --- a/packages/worker/.env.example +++ b/packages/worker/.env.example @@ -30,6 +30,9 @@ RPC_BATCH_STALL_TIME_MS=0 COLLECT_DB_CONNECTION_POOL_METRICS_INTERVAL=10000 COLLECT_BLOCKS_TO_PROCESS_METRIC_INTERVAL=10000 +RPC_HEALTH_CHECK_TIMEOUT_MS=20000 +DB_HEALTH_CHECK_TIMEOUT_MS=20000 + DISABLE_MISSING_BLOCKS_METRIC=false CHECK_MISSING_BLOCKS_METRIC_INTERVAL=86400000 diff --git a/packages/worker/src/config.spec.ts b/packages/worker/src/config.spec.ts index e6d561388e..7c678217c7 100644 --- a/packages/worker/src/config.spec.ts +++ b/packages/worker/src/config.spec.ts @@ -61,6 +61,10 @@ describe("config", () => { interval: 86_400_000, }, }, + healthChecks: { + rpcHealthCheckTimeoutMs: 20_000, + dbHealthCheckTimeoutMs: 20_000, + }, }; }); @@ -123,6 +127,10 @@ describe("config", () => { interval: 86_400_000, }, }, + healthChecks: { + rpcHealthCheckTimeoutMs: 20_000, + dbHealthCheckTimeoutMs: 20_000, + }, }); }); diff --git a/packages/worker/src/config.ts b/packages/worker/src/config.ts index ba43f72596..b6e83aefd9 100644 --- a/packages/worker/src/config.ts +++ b/packages/worker/src/config.ts @@ -33,6 +33,8 @@ export default () => { COINGECKO_API_KEY, DISABLE_MISSING_BLOCKS_METRIC, CHECK_MISSING_BLOCKS_METRIC_INTERVAL, + RPC_HEALTH_CHECK_TIMEOUT_MS, + DB_HEALTH_CHECK_TIMEOUT_MS, } = process.env; return { @@ -97,5 +99,9 @@ export default () => { interval: parseInt(CHECK_MISSING_BLOCKS_METRIC_INTERVAL, 10) || 86_400_000, // 1 day }, }, + healthChecks: { + rpcHealthCheckTimeoutMs: parseInt(RPC_HEALTH_CHECK_TIMEOUT_MS, 10) || 20_000, + dbHealthCheckTimeoutMs: parseInt(DB_HEALTH_CHECK_TIMEOUT_MS, 10) || 20_000, + }, }; }; diff --git a/packages/worker/src/health/health.controller.spec.ts b/packages/worker/src/health/health.controller.spec.ts index 6dc59e2200..a7b47551ea 100644 --- a/packages/worker/src/health/health.controller.spec.ts +++ b/packages/worker/src/health/health.controller.spec.ts @@ -4,12 +4,14 @@ import { HealthCheckService, TypeOrmHealthIndicator, HealthCheckResult } from "@ import { mock } from "jest-mock-extended"; import { JsonRpcHealthIndicator } from "./jsonRpcProvider.health"; import { HealthController } from "./health.controller"; +import { ConfigService } from "@nestjs/config"; describe("HealthController", () => { let healthCheckServiceMock: HealthCheckService; let dbHealthCheckerMock: TypeOrmHealthIndicator; let jsonRpcHealthIndicatorMock: JsonRpcHealthIndicator; let healthController: HealthController; + let configServiceMock: ConfigService; beforeEach(async () => { healthCheckServiceMock = mock({ @@ -20,6 +22,12 @@ describe("HealthController", () => { }), }); + configServiceMock = mock({ + get: jest.fn().mockImplementation((key: string) => { + if (key === "healthChecks.dbHealthCheckTimeoutMs") return 5000; + return null; + }), + }); dbHealthCheckerMock = mock(); jsonRpcHealthIndicatorMock = mock(); @@ -38,6 +46,10 @@ describe("HealthController", () => { provide: JsonRpcHealthIndicator, useValue: jsonRpcHealthIndicatorMock, }, + { + provide: ConfigService, + useValue: configServiceMock, + }, ], }).compile(); @@ -50,7 +62,7 @@ describe("HealthController", () => { it("checks health of the DB", async () => { await healthController.check(); expect(dbHealthCheckerMock.pingCheck).toHaveBeenCalledTimes(1); - expect(dbHealthCheckerMock.pingCheck).toHaveBeenCalledWith("database"); + expect(dbHealthCheckerMock.pingCheck).toHaveBeenCalledWith("database", { timeout: 5000 }); }); it("checks health of the JSON RPC provider", async () => { diff --git a/packages/worker/src/health/health.controller.ts b/packages/worker/src/health/health.controller.ts index ae5678e812..15c34bb98d 100644 --- a/packages/worker/src/health/health.controller.ts +++ b/packages/worker/src/health/health.controller.ts @@ -1,17 +1,21 @@ import { Logger, Controller, Get } from "@nestjs/common"; +import { ConfigService } from "@nestjs/config"; import { HealthCheckService, TypeOrmHealthIndicator, HealthCheck, HealthCheckResult } from "@nestjs/terminus"; import { JsonRpcHealthIndicator } from "./jsonRpcProvider.health"; @Controller(["health", "ready"]) export class HealthController { private readonly logger: Logger; + private readonly dbHealthCheckTimeoutMs: number; constructor( private readonly healthCheckService: HealthCheckService, private readonly dbHealthChecker: TypeOrmHealthIndicator, - private readonly jsonRpcHealthIndicator: JsonRpcHealthIndicator + private readonly jsonRpcHealthIndicator: JsonRpcHealthIndicator, + configService: ConfigService ) { this.logger = new Logger(HealthController.name); + this.dbHealthCheckTimeoutMs = configService.get("healthChecks.dbHealthCheckTimeoutMs"); } @Get() @@ -19,7 +23,7 @@ export class HealthController { public async check(): Promise { try { return await this.healthCheckService.check([ - () => this.dbHealthChecker.pingCheck("database"), + () => this.dbHealthChecker.pingCheck("database", { timeout: this.dbHealthCheckTimeoutMs }), () => this.jsonRpcHealthIndicator.isHealthy("jsonRpcProvider"), ]); } catch (error) { diff --git a/packages/worker/src/health/health.module.ts b/packages/worker/src/health/health.module.ts index dae128825d..37f46aac46 100644 --- a/packages/worker/src/health/health.module.ts +++ b/packages/worker/src/health/health.module.ts @@ -2,10 +2,11 @@ import { Module } from "@nestjs/common"; import { TerminusModule } from "@nestjs/terminus"; import { HealthController } from "./health.controller"; import { JsonRpcHealthIndicator } from "./jsonRpcProvider.health"; +import { HttpModule } from "@nestjs/axios"; @Module({ controllers: [HealthController], - imports: [TerminusModule], + imports: [TerminusModule, HttpModule], providers: [JsonRpcHealthIndicator], }) export class HealthModule {} diff --git a/packages/worker/src/health/jsonRpcProvider.health.spec.ts b/packages/worker/src/health/jsonRpcProvider.health.spec.ts index 2cfaa28919..e495025bbb 100644 --- a/packages/worker/src/health/jsonRpcProvider.health.spec.ts +++ b/packages/worker/src/health/jsonRpcProvider.health.spec.ts @@ -1,17 +1,20 @@ import { Test, TestingModule } from "@nestjs/testing"; +import { Logger } from "@nestjs/common"; import { mock } from "jest-mock-extended"; -import { HealthCheckError } from "@nestjs/terminus"; import { JsonRpcProviderBase } from "../rpcProvider"; import { JsonRpcHealthIndicator } from "./jsonRpcProvider.health"; +import { ConfigService } from "@nestjs/config"; +import { HttpService } from "@nestjs/axios"; +import { of, throwError } from "rxjs"; +import { AxiosError } from "axios"; describe("JsonRpcHealthIndicator", () => { - const healthIndicatorKey = "rpcProvider"; let jsonRpcProviderMock: JsonRpcProviderBase; let jsonRpcHealthIndicator: JsonRpcHealthIndicator; + let httpService: HttpService; + let configService: ConfigService; - beforeEach(async () => { - jsonRpcProviderMock = mock(); - + const getHealthIndicator = async () => { const app: TestingModule = await Test.createTestingModule({ providers: [ JsonRpcHealthIndicator, @@ -19,38 +22,90 @@ describe("JsonRpcHealthIndicator", () => { provide: JsonRpcProviderBase, useValue: jsonRpcProviderMock, }, + { + provide: HttpService, + useValue: httpService, + }, + { + provide: ConfigService, + useValue: configService, + }, ], }).compile(); - jsonRpcHealthIndicator = app.get(JsonRpcHealthIndicator); + app.useLogger(mock()); + return app.get(JsonRpcHealthIndicator); + }; + + beforeEach(async () => { + jsonRpcProviderMock = mock(); + + httpService = mock({ + post: jest.fn(), + }); + + configService = mock({ + get: jest.fn().mockImplementation((key: string) => { + if (key === "blockchain.rpcUrl") return "http://localhost:3050"; + if (key === "healthChecks.rpcHealthCheckTimeoutMs") return 5000; + return null; + }), + }); + + jsonRpcHealthIndicator = await getHealthIndicator(); }); describe("isHealthy", () => { - describe("when rpcProvider is open", () => { - beforeEach(() => { - jest.spyOn(jsonRpcProviderMock, "getState").mockReturnValueOnce("open"); - }); + const rpcRequest = { + id: 1, + jsonrpc: "2.0", + method: "eth_chainId", + params: [], + }; - it("returns OK health indicator result", async () => { - const result = await jsonRpcHealthIndicator.isHealthy(healthIndicatorKey); - expect(result).toEqual({ [healthIndicatorKey]: { rpcProviderState: "open", status: "up" } }); + it("returns healthy status when RPC responds successfully", async () => { + (httpService.post as jest.Mock).mockReturnValueOnce(of({ data: { result: "0x1" } })); + const result = await jsonRpcHealthIndicator.isHealthy("jsonRpcProvider"); + expect(result).toEqual({ + jsonRpcProvider: { + status: "up", + }, }); + expect(httpService.post).toHaveBeenCalledWith("http://localhost:3050", rpcRequest, { timeout: 5000 }); }); - describe("when rpcProvider is closed", () => { - beforeEach(() => { - jest.spyOn(jsonRpcProviderMock, "getState").mockReturnValueOnce("closed"); - }); + it("throws HealthCheckError when RPC request fails", async () => { + const error = new AxiosError(); + error.response = { + status: 503, + data: "Service Unavailable", + } as any; - it("throws HealthCheckError error", async () => { - expect.assertions(2); - try { - await jsonRpcHealthIndicator.isHealthy(healthIndicatorKey); - } catch (error) { - expect(error).toBeInstanceOf(HealthCheckError); - expect(error.message).toBe("JSON RPC provider is not in open state"); - } + (httpService.post as jest.Mock).mockReturnValueOnce(throwError(() => error)); + await expect(jsonRpcHealthIndicator.isHealthy("jsonRpcProvider")).rejects.toThrow(); + expect(httpService.post).toHaveBeenCalledWith("http://localhost:3050", rpcRequest, { timeout: 5000 }); + }); + + it("throws HealthCheckError when RPC request times out", async () => { + const error = new AxiosError(); + error.code = "ECONNABORTED"; + + (httpService.post as jest.Mock).mockReturnValueOnce(throwError(() => error)); + await expect(jsonRpcHealthIndicator.isHealthy("jsonRpcProvider")).rejects.toThrow(); + expect(httpService.post).toHaveBeenCalledWith("http://localhost:3050", rpcRequest, { timeout: 5000 }); + }); + + it("uses configured timeout from config service", async () => { + (configService.get as jest.Mock).mockImplementation((key: string) => { + if (key === "blockchain.rpcUrl") return "http://localhost:3050"; + if (key === "healthChecks.rpcHealthCheckTimeoutMs") return 10000; + return null; }); + jsonRpcHealthIndicator = await getHealthIndicator(); + + (httpService.post as jest.Mock).mockReturnValueOnce(of({ data: { result: "0x1" } })); + await jsonRpcHealthIndicator.isHealthy("jsonRpcProvider"); + expect(httpService.post).toHaveBeenCalledWith("http://localhost:3050", rpcRequest, { timeout: 10000 }); }); }); }); diff --git a/packages/worker/src/health/jsonRpcProvider.health.ts b/packages/worker/src/health/jsonRpcProvider.health.ts index 60406bfb4c..e5434382b2 100644 --- a/packages/worker/src/health/jsonRpcProvider.health.ts +++ b/packages/worker/src/health/jsonRpcProvider.health.ts @@ -1,22 +1,64 @@ import { Injectable } from "@nestjs/common"; import { HealthIndicator, HealthIndicatorResult, HealthCheckError } from "@nestjs/terminus"; -import { JsonRpcProviderBase } from "../rpcProvider"; +import { ConfigService } from "@nestjs/config"; +import { Logger } from "@nestjs/common"; +import { HttpService } from "@nestjs/axios"; +import { catchError, firstValueFrom } from "rxjs"; +import { AxiosError } from "axios"; @Injectable() export class JsonRpcHealthIndicator extends HealthIndicator { - constructor(private readonly provider: JsonRpcProviderBase) { + private readonly rpcUrl: string; + private readonly healthCheckTimeoutMs: number; + private readonly logger: Logger; + + constructor(configService: ConfigService, private readonly httpService: HttpService) { super(); + this.logger = new Logger(JsonRpcHealthIndicator.name); + this.rpcUrl = configService.get("blockchain.rpcUrl"); + this.healthCheckTimeoutMs = configService.get("healthChecks.rpcHealthCheckTimeoutMs"); } async isHealthy(key: string): Promise { - const rpcProviderState = this.provider.getState(); - const isHealthy = rpcProviderState === "open"; - const result = this.getStatus(key, isHealthy, { rpcProviderState }); + let isHealthy = true; + try { + // Check RPC health with a pure HTTP request to remove SDK out of the picture + // and avoid any SDK-specific issues. + // Use eth_chainId call as it is the lightest one and return a static value from the memory. + await firstValueFrom( + this.httpService + .post( + this.rpcUrl, + { + id: 1, + jsonrpc: "2.0", + method: "eth_chainId", + params: [], + }, + { timeout: this.healthCheckTimeoutMs } + ) + .pipe( + catchError((error: AxiosError) => { + this.logger.error({ + message: `Failed to ping RPC`, + stack: error.stack, + status: error.response?.status, + response: error.response?.data, + }); + throw error; + }) + ) + ); + } catch { + isHealthy = false; + } + + const result = this.getStatus(key, isHealthy, { status: isHealthy ? "up" : "down" }); if (isHealthy) { return result; } - throw new HealthCheckError("JSON RPC provider is not in open state", result); + throw new HealthCheckError("JSON RPC provider is down or not reachable", result); } } From 1bd495dde35161eeae7349773d408f17ed2dba93 Mon Sep 17 00:00:00 2001 From: Roman Petriv Date: Tue, 21 Jan 2025 16:44:05 +0200 Subject: [PATCH 2/6] fix: handle null blocks in array of blocks to process (#376) --- packages/worker/src/block/block.processor.ts | 4 ++-- packages/worker/src/blocksRevert/blocksRevert.service.ts | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/worker/src/block/block.processor.ts b/packages/worker/src/block/block.processor.ts index 3fa438135b..7418ebe8f4 100644 --- a/packages/worker/src/block/block.processor.ts +++ b/packages/worker/src/block/block.processor.ts @@ -78,12 +78,12 @@ export class BlockProcessor { return false; } - if (lastDbBlock && lastDbBlock.hash !== blocksToProcess[0].block?.parentHash) { + if (lastDbBlock && lastDbBlock.hash !== blocksToProcess[0]?.block?.parentHash) { this.triggerBlocksRevertEvent(lastDbBlockNumber); return false; } - const allBlocksExist = !blocksToProcess.find((blockInfo) => !blockInfo.block || !blockInfo.blockDetails); + const allBlocksExist = !blocksToProcess.find((blockInfo) => !blockInfo?.block || !blockInfo?.blockDetails); if (!allBlocksExist) { // We don't need to handle this potential revert as these blocks are not in DB yet, // try again later once these blocks are present in blockchain again. diff --git a/packages/worker/src/blocksRevert/blocksRevert.service.ts b/packages/worker/src/blocksRevert/blocksRevert.service.ts index 497cd4fb5d..8dd75a1323 100644 --- a/packages/worker/src/blocksRevert/blocksRevert.service.ts +++ b/packages/worker/src/blocksRevert/blocksRevert.service.ts @@ -81,7 +81,7 @@ export class BlocksRevertService { lastExecutedBlockNumber: number, detectedIncorrectBlockNumber: number ) => { - // binary search the last block with matching hash between latest executed block from DB and incorrect clock detected + // binary search the last block with matching hash between latest executed block from DB and incorrect block detected let start = lastExecutedBlockNumber; let end = detectedIncorrectBlockNumber; From 55768112bd6647fcba0f8e4e992534fe1a389a3a Mon Sep 17 00:00:00 2001 From: Marko Arambasic <131957563+kiriyaga-txfusion@users.noreply.github.com> Date: Tue, 21 Jan 2025 16:26:32 +0100 Subject: [PATCH 3/6] feat: add system contracts on the first run (#372) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # What ❔ This PR addresses an issue where system contracts were not displayed as contracts in the UI until they were updated. The implemented solution ensures that system contracts are loaded and saved in the database immediately after work begins. This guarantees their proper display in the UI, providing a consistent and accurate reflection of the system's state. ## Why ❔ System contracts are not displayed as contracts in the UI until they have been updated. ## Checklist This PR fixes: https://github.com/matter-labs/block-explorer/issues/358 - [+] PR title corresponds to the body of PR (we generate changelog entries from PRs). - [+] Tests for the changes have been added / updated. - [ ] Documentation comments have been added / updated. Co-authored-by: Vasyl Ivanchuk --- .../app/src/components/contract/InfoTable.vue | 2 +- packages/worker/src/app.module.ts | 2 + packages/worker/src/app.service.spec.ts | 16 ++ packages/worker/src/app.service.ts | 5 +- .../contract/systemContract.service.spec.ts | 107 +++++++++++++ .../src/contract/systemContract.service.ts | 143 ++++++++++++++++++ 6 files changed, 273 insertions(+), 2 deletions(-) create mode 100644 packages/worker/src/contract/systemContract.service.spec.ts create mode 100644 packages/worker/src/contract/systemContract.service.ts diff --git a/packages/app/src/components/contract/InfoTable.vue b/packages/app/src/components/contract/InfoTable.vue index 27405d72f1..a9cab847f0 100644 --- a/packages/app/src/components/contract/InfoTable.vue +++ b/packages/app/src/components/contract/InfoTable.vue @@ -9,7 +9,7 @@ - + {{ t("contract.table.creator") }} diff --git a/packages/worker/src/app.module.ts b/packages/worker/src/app.module.ts index bec09a377a..da46beea4e 100644 --- a/packages/worker/src/app.module.ts +++ b/packages/worker/src/app.module.ts @@ -52,6 +52,7 @@ import { MetricsModule } from "./metrics"; import { DbMetricsService } from "./dbMetrics.service"; import { UnitOfWorkModule } from "./unitOfWork"; import { DataFetcherService } from "./dataFetcher/dataFetcher.service"; +import { SystemContractService } from "./contract/systemContract.service"; @Module({ imports: [ @@ -130,6 +131,7 @@ import { DataFetcherService } from "./dataFetcher/dataFetcher.service"; Logger, RetryDelayProvider, DbMetricsService, + SystemContractService, ], }) export class AppModule {} diff --git a/packages/worker/src/app.service.spec.ts b/packages/worker/src/app.service.spec.ts index 3d4afb8363..60d5a8b295 100644 --- a/packages/worker/src/app.service.spec.ts +++ b/packages/worker/src/app.service.spec.ts @@ -13,6 +13,7 @@ import { BlocksRevertService } from "./blocksRevert"; import { TokenOffChainDataSaverService } from "./token/tokenOffChainData/tokenOffChainDataSaver.service"; import runMigrations from "./utils/runMigrations"; import { BLOCKS_REVERT_DETECTED_EVENT } from "./constants"; +import { SystemContractService } from "./contract/systemContract.service"; jest.mock("./utils/runMigrations"); @@ -39,6 +40,7 @@ describe("AppService", () => { let tokenOffChainDataSaverService: TokenOffChainDataSaverService; let dataSourceMock: DataSource; let configServiceMock: ConfigService; + let systemContractService: SystemContractService; beforeEach(async () => { balancesCleanerService = mock({ @@ -68,6 +70,9 @@ describe("AppService", () => { configServiceMock = mock({ get: jest.fn().mockReturnValue(false), }); + systemContractService = mock({ + addSystemContracts: jest.fn().mockResolvedValue(null), + }); const module = await Test.createTestingModule({ imports: [EventEmitterModule.forRoot()], @@ -106,6 +111,10 @@ describe("AppService", () => { provide: ConfigService, useValue: configServiceMock, }, + { + provide: SystemContractService, + useValue: systemContractService, + }, ], }).compile(); @@ -205,6 +214,13 @@ describe("AppService", () => { appService.onModuleDestroy(); expect(tokenOffChainDataSaverService.stop).toBeCalledTimes(1); }); + + it("adds system contracts", async () => { + appService.onModuleInit(); + await migrationsRunFinished; + expect(systemContractService.addSystemContracts).toBeCalledTimes(1); + appService.onModuleDestroy(); + }); }); describe("onModuleDestroy", () => { diff --git a/packages/worker/src/app.service.ts b/packages/worker/src/app.service.ts index d6ec852c73..71d82079eb 100644 --- a/packages/worker/src/app.service.ts +++ b/packages/worker/src/app.service.ts @@ -10,6 +10,7 @@ import { CounterService } from "./counter"; import { BalancesCleanerService } from "./balance"; import { TokenOffChainDataSaverService } from "./token/tokenOffChainData/tokenOffChainDataSaver.service"; import runMigrations from "./utils/runMigrations"; +import { SystemContractService } from "./contract/systemContract.service"; @Injectable() export class AppService implements OnModuleInit, OnModuleDestroy { @@ -23,13 +24,15 @@ export class AppService implements OnModuleInit, OnModuleDestroy { private readonly balancesCleanerService: BalancesCleanerService, private readonly tokenOffChainDataSaverService: TokenOffChainDataSaverService, private readonly dataSource: DataSource, - private readonly configService: ConfigService + private readonly configService: ConfigService, + private readonly systemContractService: SystemContractService ) { this.logger = new Logger(AppService.name); } public onModuleInit() { runMigrations(this.dataSource, this.logger).then(() => { + this.systemContractService.addSystemContracts(); this.startWorkers(); }); } diff --git a/packages/worker/src/contract/systemContract.service.spec.ts b/packages/worker/src/contract/systemContract.service.spec.ts new file mode 100644 index 0000000000..8e07ebaa9f --- /dev/null +++ b/packages/worker/src/contract/systemContract.service.spec.ts @@ -0,0 +1,107 @@ +import { mock } from "jest-mock-extended"; +import { Test, TestingModule } from "@nestjs/testing"; +import { Logger } from "@nestjs/common"; +import { BlockchainService } from "../blockchain/blockchain.service"; +import { AddressRepository } from "../repositories/address.repository"; +import { SystemContractService } from "./systemContract.service"; +import { Address } from "../entities"; + +describe("SystemContractService", () => { + let systemContractService: SystemContractService; + let blockchainServiceMock: BlockchainService; + let addressRepositoryMock: AddressRepository; + const systemContracts = SystemContractService.getSystemContracts(); + + beforeEach(async () => { + blockchainServiceMock = mock({ + getCode: jest.fn().mockImplementation((address: string) => Promise.resolve(`${address}-code`)), + }); + + addressRepositoryMock = mock({ + find: jest.fn(), + }); + + const app: TestingModule = await Test.createTestingModule({ + providers: [ + SystemContractService, + { + provide: BlockchainService, + useValue: blockchainServiceMock, + }, + { + provide: AddressRepository, + useValue: addressRepositoryMock, + }, + ], + }).compile(); + + app.useLogger(mock()); + systemContractService = app.get(SystemContractService); + }); + + describe("addSystemContracts", () => { + it("doesn't add any system contracts if they already exist in DB", async () => { + (addressRepositoryMock.find as jest.Mock).mockResolvedValue( + SystemContractService.getSystemContracts().map((contract) => mock
({ address: contract.address })) + ); + await systemContractService.addSystemContracts(); + expect(addressRepositoryMock.add).toBeCalledTimes(0); + }); + + it("adds all system contracts if none of them exist in the DB", async () => { + (addressRepositoryMock.find as jest.Mock).mockResolvedValue([]); + await systemContractService.addSystemContracts(); + expect(addressRepositoryMock.add).toBeCalledTimes(systemContracts.length); + for (const systemContract of systemContracts) { + expect(addressRepositoryMock.add).toBeCalledWith({ + address: systemContract.address, + bytecode: `${systemContract.address}-code`, + }); + } + }); + + it("adds only missing system contracts", async () => { + const existingContractAddresses = [ + "0x000000000000000000000000000000000000800d", + "0x0000000000000000000000000000000000008006", + ]; + (addressRepositoryMock.find as jest.Mock).mockResolvedValue( + existingContractAddresses.map((existingContractAddress) => mock
({ address: existingContractAddress })) + ); + await systemContractService.addSystemContracts(); + expect(addressRepositoryMock.add).toBeCalledTimes(systemContracts.length - existingContractAddresses.length); + for (const systemContract of systemContracts) { + if (!existingContractAddresses.includes(systemContract.address)) { + expect(addressRepositoryMock.add).toBeCalledWith({ + address: systemContract.address, + bytecode: `${systemContract.address}-code`, + }); + } + } + }); + + it("adds contracts only if they are deployed to the network", async () => { + const notDeployedSystemContracts = [ + "0x000000000000000000000000000000000000800d", + "0x0000000000000000000000000000000000008006", + ]; + (addressRepositoryMock.find as jest.Mock).mockResolvedValue([]); + (blockchainServiceMock.getCode as jest.Mock).mockImplementation(async (address: string) => { + if (notDeployedSystemContracts.includes(address)) { + return "0x"; + } + return `${address}-code`; + }); + await systemContractService.addSystemContracts(); + expect(addressRepositoryMock.add).toBeCalledTimes(systemContracts.length - notDeployedSystemContracts.length); + for (const systemContract of systemContracts) { + if (!notDeployedSystemContracts.includes(systemContract.address)) { + expect(addressRepositoryMock.add).toBeCalledWith({ + address: systemContract.address, + bytecode: `${systemContract.address}-code`, + }); + } + } + }); + }); +}); diff --git a/packages/worker/src/contract/systemContract.service.ts b/packages/worker/src/contract/systemContract.service.ts new file mode 100644 index 0000000000..09a8b93041 --- /dev/null +++ b/packages/worker/src/contract/systemContract.service.ts @@ -0,0 +1,143 @@ +import { Injectable } from "@nestjs/common"; +import { BlockchainService } from "../blockchain/blockchain.service"; +import { AddressRepository } from "../repositories"; +import { In } from "typeorm"; + +@Injectable() +export class SystemContractService { + constructor( + private readonly addressRepository: AddressRepository, + private readonly blockchainService: BlockchainService + ) {} + + public async addSystemContracts(): Promise { + const systemContracts = SystemContractService.getSystemContracts(); + const existingContracts = await this.addressRepository.find({ + where: { + address: In(systemContracts.map((contract) => contract.address)), + }, + select: { + address: true, + }, + }); + + for (const contract of systemContracts) { + if (!existingContracts.find((existingContract) => existingContract.address === contract.address)) { + const bytecode = await this.blockchainService.getCode(contract.address); + // some contract might not exist on the environment yet + if (bytecode !== "0x") { + await this.addressRepository.add({ + address: contract.address, + bytecode, + }); + } + } + } + } + + public static getSystemContracts() { + // name field is never used, it's just for better readability & understanding + return [ + { + address: "0x0000000000000000000000000000000000000000", + name: "EmptyContract", + }, + { + address: "0x0000000000000000000000000000000000000001", + name: "Ecrecover", + }, + { + address: "0x0000000000000000000000000000000000000002", + name: "SHA256", + }, + { + address: "0x0000000000000000000000000000000000000006", + name: "EcAdd", + }, + { + address: "0x0000000000000000000000000000000000000007", + name: "EcMul", + }, + { + address: "0x0000000000000000000000000000000000000008", + name: "EcPairing", + }, + { + address: "0x0000000000000000000000000000000000008001", + name: "EmptyContract", + }, + { + address: "0x0000000000000000000000000000000000008002", + name: "AccountCodeStorage", + }, + { + address: "0x0000000000000000000000000000000000008003", + name: "NonceHolder", + }, + { + address: "0x0000000000000000000000000000000000008004", + name: "KnownCodesStorage", + }, + { + address: "0x0000000000000000000000000000000000008005", + name: "ImmutableSimulator", + }, + { + address: "0x0000000000000000000000000000000000008006", + name: "ContractDeployer", + }, + { + address: "0x0000000000000000000000000000000000008008", + name: "L1Messenger", + }, + { + address: "0x0000000000000000000000000000000000008009", + name: "MsgValueSimulator", + }, + { + address: "0x000000000000000000000000000000000000800a", + name: "L2BaseToken", + }, + { + address: "0x000000000000000000000000000000000000800b", + name: "SystemContext", + }, + { + address: "0x000000000000000000000000000000000000800c", + name: "BootloaderUtilities", + }, + { + address: "0x000000000000000000000000000000000000800d", + name: "EventWriter", + }, + { + address: "0x000000000000000000000000000000000000800e", + name: "Compressor", + }, + { + address: "0x000000000000000000000000000000000000800f", + name: "ComplexUpgrader", + }, + { + address: "0x0000000000000000000000000000000000008010", + name: "Keccak256", + }, + { + address: "0x0000000000000000000000000000000000008012", + name: "CodeOracle", + }, + { + address: "0x0000000000000000000000000000000000000100", + name: "P256Verify", + }, + { + address: "0x0000000000000000000000000000000000008011", + name: "PubdataChunkPublisher", + }, + { + address: "0x0000000000000000000000000000000000010000", + name: "Create2Factory", + }, + ]; + } +} From 237bdf159b21c3a6587897fb1d4fdd2fd87d0cf7 Mon Sep 17 00:00:00 2001 From: Marko Arambasic <131957563+kiriyaga-txfusion@users.noreply.github.com> Date: Tue, 21 Jan 2025 19:00:12 +0100 Subject: [PATCH 4/6] fix: calldata encoding fails for tuple params (#333) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # What ❔ This PR implements support for encoding non-primitive input types. With this update, all input data, including tuples, arrays, and combinations of various types, will be correctly encoded. ## Why ❔ In the current version of the block explorer, users encounter an error when input data includes complex objects or arrays. For show we will use: https://explorer.zksync.io/tx/0xe39dbc98b41bb22620b78e3a3848ab34982310f9564f27d1e809cfdd461fa54e Example without fix: ![image](https://github.com/user-attachments/assets/ece3ffcc-ff03-4eb1-9dd2-d380a4655695) Example with fix: ![image](https://github.com/user-attachments/assets/549059ae-a304-4fb7-9a69-bd3753cc9972) fixes https://github.com/matter-labs/block-explorer/issues/321 ## Checklist - [+ ] PR title corresponds to the body of PR (we generate changelog entries from PRs). - [+ ] Tests for the changes have been added / updated. - [ ] Documentation comments have been added / updated. Co-authored-by: Vasyl Ivanchuk --- .../app/src/composables/useTransactionData.ts | 34 ++-- packages/app/src/utils/helpers.ts | 56 +++++- .../composables/useTransactionData.spec.ts | 4 + packages/app/tests/utils/helpers.spec.ts | 163 ++++++++++++++++++ 4 files changed, 238 insertions(+), 19 deletions(-) diff --git a/packages/app/src/composables/useTransactionData.ts b/packages/app/src/composables/useTransactionData.ts index 341eb482e2..8cbf4121da 100644 --- a/packages/app/src/composables/useTransactionData.ts +++ b/packages/app/src/composables/useTransactionData.ts @@ -1,6 +1,6 @@ import { ref } from "vue"; -import { AbiCoder, Interface } from "ethers"; +import { Interface } from "ethers"; import useAddress from "./useAddress"; import useContext from "./useContext"; @@ -10,22 +10,26 @@ import type { AbiFragment } from "./useAddress"; import type { InputType } from "./useEventLog"; import type { Address } from "@/types"; -const defaultAbiCoder: AbiCoder = AbiCoder.defaultAbiCoder(); +import { decodeInputData } from "@/utils/helpers"; +export type MethodData = { + name: string; + inputs: InputData[]; +}; + +export type InputData = { + name: string; + type: InputType | string; + value: string; + inputs: InputData[]; + encodedValue: string; +}; export type TransactionData = { calldata: string; contractAddress: Address | null; value: string; sighash: string; - method?: { - name: string; - inputs: { - name: string; - type: InputType; - value: string; - encodedValue: string; - }[]; - }; + method?: MethodData; }; export function decodeDataWithABI( @@ -33,7 +37,6 @@ export function decodeDataWithABI( abi: AbiFragment[] ): TransactionData["method"] | undefined { const contractInterface = new Interface(abi); - try { const decodedData = contractInterface.parseTransaction({ data: transactionData.calldata, @@ -41,12 +44,7 @@ export function decodeDataWithABI( })!; return { name: decodedData.name, - inputs: decodedData.fragment.inputs.map((input) => ({ - name: input.name, - type: input.type as InputType, - value: decodedData.args[input.name]?.toString(), - encodedValue: defaultAbiCoder.encode([input.type], [decodedData.args[input.name]]).split("0x")[1], - })), + inputs: decodedData.fragment.inputs.flatMap((input) => decodeInputData(input, decodedData.args[input.name])), }; } catch { return undefined; diff --git a/packages/app/src/utils/helpers.ts b/packages/app/src/utils/helpers.ts index fe8bc94e0d..b3a9c6a282 100644 --- a/packages/app/src/utils/helpers.ts +++ b/packages/app/src/utils/helpers.ts @@ -1,5 +1,5 @@ import { format } from "date-fns"; -import { Interface } from "ethers"; +import { AbiCoder, Interface } from "ethers"; import { utils } from "zksync-ethers"; import { DEPLOYER_CONTRACT_ADDRESS } from "./constants"; @@ -8,9 +8,13 @@ import type { DecodingType } from "@/components/transactions/infoTable/HashViewe import type { AbiFragment } from "@/composables/useAddress"; import type { InputType, TransactionEvent, TransactionLogEntry } from "@/composables/useEventLog"; import type { TokenTransfer } from "@/composables/useTransaction"; +import type { InputData } from "@/composables/useTransactionData"; +import type { ParamType, Result } from "ethers"; const { BOOTLOADER_FORMAL_ADDRESS } = utils; +export const DefaultAbiCoder: AbiCoder = AbiCoder.defaultAbiCoder(); + export function utcStringFromUnixTimestamp(timestamp: number) { const isoDate = new Date(+`${timestamp}000`).toISOString(); return format(new Date(isoDate.slice(0, -1)), "yyyy-MM-dd HH:mm 'UTC'"); @@ -116,6 +120,56 @@ export function decodeLogWithABI(log: TransactionLogEntry, abi: AbiFragment[]): } } +export function decodeInputData(input: ParamType, args: Result): InputData[] { + if (input.isArray()) { + return decodeArrayInputData(input, args); + } + + if (input.isTuple()) { + return decodeTupleInputData(input, args); + } + + return [ + { + name: input.name, + type: input.type as InputType, + value: args.toString(), + encodedValue: DefaultAbiCoder.encode([input.type], [args]).split("0x")[1], + inputs: [], + }, + ]; +} + +function decodeArrayInputData(input: ParamType, args: Result): InputData[] { + const inputs = args.flatMap((arg) => decodeInputData(input.arrayChildren!, arg)); + + return [ + { + name: input.name, + type: `${inputs[0]?.type ? `${inputs[0]?.type}[]` : input.type}`, + value: `[${inputs.map((input) => input.value).join(",")}]`, + inputs: inputs, + encodedValue: `[${inputs.map((input) => input.encodedValue).join(",")}]`, + }, + ]; +} + +function decodeTupleInputData(input: ParamType, args: Result): InputData[] { + const inputs = input.components!.flatMap((component: ParamType, index: number) => + decodeInputData(component, args[index]) + ); + + return [ + { + name: input.name, + type: `tuple(${inputs.map((input) => input.type).join(",")})`, + value: `(${inputs.map((input) => input.value).join(",")})`, + inputs, + encodedValue: `(${inputs.map((input) => input.encodedValue).join(",")})`, + }, + ]; +} + export function sortTokenTransfers(transfers: TokenTransfer[]): TokenTransfer[] { return [...transfers].sort((_, t) => { if (t.to === BOOTLOADER_FORMAL_ADDRESS || t.from === BOOTLOADER_FORMAL_ADDRESS) { diff --git a/packages/app/tests/composables/useTransactionData.spec.ts b/packages/app/tests/composables/useTransactionData.spec.ts index 0d6b921bb1..cfc625a85a 100644 --- a/packages/app/tests/composables/useTransactionData.spec.ts +++ b/packages/app/tests/composables/useTransactionData.spec.ts @@ -53,12 +53,14 @@ const transactionDataDecodedMethod = { inputs: [ { name: "recipient", + inputs: [], type: "address", value: "0xa1cf087DB965Ab02Fb3CFaCe1f5c63935815f044", encodedValue: "000000000000000000000000a1cf087db965ab02fb3cface1f5c63935815f044", }, { name: "amount", + inputs: [], type: "uint256", value: "1", encodedValue: "0000000000000000000000000000000000000000000000000000000000000001", @@ -175,12 +177,14 @@ describe("useTransactionData:", () => { inputs: [ { encodedValue: "000000000000000000000000a1cf087db965ab02fb3cface1f5c63935815f044", + inputs: [], name: "recipient", type: "address", value: "0xa1cf087DB965Ab02Fb3CFaCe1f5c63935815f044", }, { encodedValue: "0000000000000000000000000000000000000000000000000000000000000001", + inputs: [], name: "amount", type: "uint256", value: "1", diff --git a/packages/app/tests/utils/helpers.spec.ts b/packages/app/tests/utils/helpers.spec.ts index 6ad49b6614..63d5919ef3 100644 --- a/packages/app/tests/utils/helpers.spec.ts +++ b/packages/app/tests/utils/helpers.spec.ts @@ -1,17 +1,20 @@ import { describe, expect, it } from "vitest"; import { format } from "date-fns"; +import { ParamType } from "ethers"; import { utils } from "zksync-ethers"; import ExecuteTx from "@/../mock/transactions/Execute.json"; import type { InputType } from "@/composables/useEventLog"; import type { TokenTransfer } from "@/composables/useTransaction"; +import type { Result } from "ethers"; import { arrayHalfDivider, camelCaseFromSnakeCase, contractInputTypeToHumanType, + decodeInputData, getRawFunctionType, getRequiredArrayLength, getTypeFromEvent, @@ -178,4 +181,164 @@ describe("helpers:", () => { expect(truncateNumber("0.02", 5)).toEqual("0.02"); }); }); + describe("decodeInputData:", () => { + it("decodes a simple input type", () => { + const input = ParamType.from({ + name: "value", + type: "uint256", + baseType: "scalar", + isArray: () => true, + isTuple: () => false, + }); + const args = 42; + + const result = decodeInputData(input, args as unknown as Result); + + expect(result).toEqual([ + { + name: "value", + type: "uint256", + value: "42", + encodedValue: expect.any(String), + inputs: [], + }, + ]); + }); + + it("decodes an array input type", () => { + const input = ParamType.from({ + name: "values", + baseType: "array", + type: "uint256[]", + arrayChildren: { name: "value", type: "uint256" }, + }); + const args = [42, 43]; + + const result = decodeInputData(input, args as unknown as Result); + + expect(result).toEqual([ + { + name: "values", + type: "uint256[]", + value: "[42,43]", + inputs: [ + { + name: "", + type: "uint256", + value: "42", + encodedValue: "000000000000000000000000000000000000000000000000000000000000002a", + inputs: [], + }, + { + name: "", + type: "uint256", + value: "43", + encodedValue: "000000000000000000000000000000000000000000000000000000000000002b", + inputs: [], + }, + ], + encodedValue: + "[000000000000000000000000000000000000000000000000000000000000002a,000000000000000000000000000000000000000000000000000000000000002b]", + }, + ]); + }); + + it("decodes a tuple input type", () => { + const input = ParamType.from({ + name: "tupleValue", + type: "tuple", + baseType: "tuple", + components: [ + { name: "value1", type: "uint256", baseType: "scalar" }, + { name: "value2", type: "string", baseType: "scalar" }, + ], + }); + const args = ["42", "test"]; + + const result = decodeInputData(input, args as unknown as Result); + expect(result).toEqual([ + { + name: "tupleValue", + type: "tuple(uint256,string)", + value: "(42,test)", + inputs: [ + { + name: "value1", + type: "uint256", + value: "42", + encodedValue: "000000000000000000000000000000000000000000000000000000000000002a", + inputs: [], + }, + { + name: "value2", + type: "string", + value: "test", + encodedValue: + "000000000000000000000000000000000000000000000000000000000000002000000000000000000000000000000000000000000000000000000000000000047465737400000000000000000000000000000000000000000000000000000000", + inputs: [], + }, + ], + encodedValue: + "(000000000000000000000000000000000000000000000000000000000000002a,000000000000000000000000000000000000000000000000000000000000002000000000000000000000000000000000000000000000000000000000000000047465737400000000000000000000000000000000000000000000000000000000)", + }, + ]); + }); + + it("decodes a tuple with an array", () => { + const input = ParamType.from({ + name: "tupleValue", + type: "tuple", + baseType: "tuple", + components: [ + { name: "value1", type: "uint256", baseType: "scalar" }, + { name: "value2", type: "uint256[]", baseType: "array", arrayChildren: { name: "value", type: "uint256" } }, + ], + }); + const args = [42, [43, 44]]; + + const result = decodeInputData(input, args as unknown as Result); + + expect(result).toEqual([ + { + name: "tupleValue", + type: "tuple(uint256,uint256[])", + value: "(42,[43,44])", + inputs: [ + { + name: "value1", + type: "uint256", + value: "42", + encodedValue: "000000000000000000000000000000000000000000000000000000000000002a", + inputs: [], + }, + { + name: "value2", + type: "uint256[]", + value: "[43,44]", + inputs: [ + { + name: "", + type: "uint256", + value: "43", + encodedValue: "000000000000000000000000000000000000000000000000000000000000002b", + inputs: [], + }, + { + name: "", + type: "uint256", + value: "44", + encodedValue: "000000000000000000000000000000000000000000000000000000000000002c", + inputs: [], + }, + ], + encodedValue: + "[000000000000000000000000000000000000000000000000000000000000002b,000000000000000000000000000000000000000000000000000000000000002c]", + }, + ], + encodedValue: + "(000000000000000000000000000000000000000000000000000000000000002a,[000000000000000000000000000000000000000000000000000000000000002b,000000000000000000000000000000000000000000000000000000000000002c])", + }, + ]); + }); + }); }); From 2f55299a25b0df7a2f056594c0d42f5a159f782f Mon Sep 17 00:00:00 2001 From: Roman Petriv Date: Wed, 22 Jan 2025 12:44:39 +0200 Subject: [PATCH 5/6] fix: prevent concurrent block reverts, reset block number after revert (#377) --- packages/worker/src/app.service.ts | 10 +++++++++- packages/worker/src/block/block.watcher.ts | 2 +- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/packages/worker/src/app.service.ts b/packages/worker/src/app.service.ts index 71d82079eb..f0fbaa8301 100644 --- a/packages/worker/src/app.service.ts +++ b/packages/worker/src/app.service.ts @@ -15,6 +15,7 @@ import { SystemContractService } from "./contract/systemContract.service"; @Injectable() export class AppService implements OnModuleInit, OnModuleDestroy { private readonly logger: Logger; + private isHandlingBlocksRevert = false; public constructor( private readonly counterService: CounterService, @@ -43,6 +44,11 @@ export class AppService implements OnModuleInit, OnModuleDestroy { @OnEvent(BLOCKS_REVERT_DETECTED_EVENT) protected async handleBlocksRevert({ detectedIncorrectBlockNumber }: { detectedIncorrectBlockNumber: number }) { + if (this.isHandlingBlocksRevert) { + return; + } + this.isHandlingBlocksRevert = true; + this.logger.log("Stopping workers before blocks revert"); await this.stopWorkers(); @@ -50,7 +56,9 @@ export class AppService implements OnModuleInit, OnModuleDestroy { await this.blocksRevertService.handleRevert(detectedIncorrectBlockNumber); this.logger.log("Starting workers after blocks revert"); - await this.startWorkers(); + this.startWorkers(); + + this.isHandlingBlocksRevert = false; } private startWorkers() { diff --git a/packages/worker/src/block/block.watcher.ts b/packages/worker/src/block/block.watcher.ts index f491c6554a..0ae3f5978a 100644 --- a/packages/worker/src/block/block.watcher.ts +++ b/packages/worker/src/block/block.watcher.ts @@ -119,7 +119,7 @@ export class BlockWatcher implements OnModuleInit, OnModuleDestroy { this.logger.debug(`Last block number is set to: ${this.lastBlockchainBlockNumber}`); this.blockchainService.on("block", (blockNumber) => { - this.lastBlockchainBlockNumber = Math.max(this.lastBlockchainBlockNumber, blockNumber); + this.lastBlockchainBlockNumber = blockNumber || this.lastBlockchainBlockNumber; this.blockchainBlocksMetric.set(this.lastBlockchainBlockNumber); this.logger.debug(`Last block number is updated to: ${this.lastBlockchainBlockNumber}`); }); From 24aecc3ebef5e7b7f83c672974f1335961afaf32 Mon Sep 17 00:00:00 2001 From: Roman Petriv Date: Wed, 22 Jan 2025 13:20:25 +0200 Subject: [PATCH 6/6] fix: handle potential concurrent system contracts insertion (#378) If indexer finds a contract deployment event for a system contract at the same time as system contracts service inserts it - there will be a collision. It can very likely happen especially after these events are added on the blockchain side for initial deployments of system contracts. --- .../src/contract/systemContract.service.spec.ts | 14 +++++++------- .../worker/src/contract/systemContract.service.ts | 2 +- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/packages/worker/src/contract/systemContract.service.spec.ts b/packages/worker/src/contract/systemContract.service.spec.ts index 8e07ebaa9f..6359fddd72 100644 --- a/packages/worker/src/contract/systemContract.service.spec.ts +++ b/packages/worker/src/contract/systemContract.service.spec.ts @@ -45,15 +45,15 @@ describe("SystemContractService", () => { SystemContractService.getSystemContracts().map((contract) => mock
({ address: contract.address })) ); await systemContractService.addSystemContracts(); - expect(addressRepositoryMock.add).toBeCalledTimes(0); + expect(addressRepositoryMock.upsert).toBeCalledTimes(0); }); it("adds all system contracts if none of them exist in the DB", async () => { (addressRepositoryMock.find as jest.Mock).mockResolvedValue([]); await systemContractService.addSystemContracts(); - expect(addressRepositoryMock.add).toBeCalledTimes(systemContracts.length); + expect(addressRepositoryMock.upsert).toBeCalledTimes(systemContracts.length); for (const systemContract of systemContracts) { - expect(addressRepositoryMock.add).toBeCalledWith({ + expect(addressRepositoryMock.upsert).toBeCalledWith({ address: systemContract.address, bytecode: `${systemContract.address}-code`, }); @@ -69,10 +69,10 @@ describe("SystemContractService", () => { existingContractAddresses.map((existingContractAddress) => mock
({ address: existingContractAddress })) ); await systemContractService.addSystemContracts(); - expect(addressRepositoryMock.add).toBeCalledTimes(systemContracts.length - existingContractAddresses.length); + expect(addressRepositoryMock.upsert).toBeCalledTimes(systemContracts.length - existingContractAddresses.length); for (const systemContract of systemContracts) { if (!existingContractAddresses.includes(systemContract.address)) { - expect(addressRepositoryMock.add).toBeCalledWith({ + expect(addressRepositoryMock.upsert).toBeCalledWith({ address: systemContract.address, bytecode: `${systemContract.address}-code`, }); @@ -93,10 +93,10 @@ describe("SystemContractService", () => { return `${address}-code`; }); await systemContractService.addSystemContracts(); - expect(addressRepositoryMock.add).toBeCalledTimes(systemContracts.length - notDeployedSystemContracts.length); + expect(addressRepositoryMock.upsert).toBeCalledTimes(systemContracts.length - notDeployedSystemContracts.length); for (const systemContract of systemContracts) { if (!notDeployedSystemContracts.includes(systemContract.address)) { - expect(addressRepositoryMock.add).toBeCalledWith({ + expect(addressRepositoryMock.upsert).toBeCalledWith({ address: systemContract.address, bytecode: `${systemContract.address}-code`, }); diff --git a/packages/worker/src/contract/systemContract.service.ts b/packages/worker/src/contract/systemContract.service.ts index 09a8b93041..d6b10b1c85 100644 --- a/packages/worker/src/contract/systemContract.service.ts +++ b/packages/worker/src/contract/systemContract.service.ts @@ -26,7 +26,7 @@ export class SystemContractService { const bytecode = await this.blockchainService.getCode(contract.address); // some contract might not exist on the environment yet if (bytecode !== "0x") { - await this.addressRepository.add({ + await this.addressRepository.upsert({ address: contract.address, bytecode, });