From 0dd0236ead7d573f40614a2718c20b025c55701d Mon Sep 17 00:00:00 2001 From: Liu <57480598+liu-zhipeng@users.noreply.github.com> Date: Tue, 10 Oct 2023 10:51:12 +0800 Subject: [PATCH 1/5] fix: check hub chain id early --- .../src/tasks/propose/operations/propose.ts | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/packages/agents/lighthouse/src/tasks/propose/operations/propose.ts b/packages/agents/lighthouse/src/tasks/propose/operations/propose.ts index c4f2da3e3c..e8835182b9 100644 --- a/packages/agents/lighthouse/src/tasks/propose/operations/propose.ts +++ b/packages/agents/lighthouse/src/tasks/propose/operations/propose.ts @@ -6,6 +6,7 @@ import { SnapshotRoot, SparseMerkleTree, jsonifyError, + domainToChainId, } from "@connext/nxtp-utils"; import { BigNumber } from "ethers"; @@ -26,11 +27,17 @@ export const propose = async () => { const { logger, config, + chainData, adapters: { database }, } = getContext(); const { requestContext, methodContext } = createLoggingContext(propose.name); logger.info("Starting propose operation", requestContext, methodContext); + const hubChainId = chainData.get(config.hubDomain)?.chainId; + if (!hubChainId) { + throw new NoChainIdForHubDomain(config.hubDomain, requestContext, methodContext); + } + // Get the latest pending snapshots // Generate aggreagate root given latest snapshot // Encode params data for contract call @@ -84,18 +91,13 @@ export const proposeSnapshot = async (snapshotId: string, snapshotRoots: string[ logger, adapters: { contracts, relayers, database, chainreader, subgraph }, config, - chainData, } = getContext(); const { requestContext, methodContext } = createLoggingContext("proposeSnapshot", _requestContext); const rootManagerMeta: RootManagerMeta = await subgraph.getRootManagerMeta(config.hubDomain); const domains = rootManagerMeta.domains; - const hubChainId = chainData.get(config.hubDomain)?.chainId; - if (!hubChainId) { - throw new NoChainIdForHubDomain(config.hubDomain, requestContext, methodContext); - } - const relayerProxyHubAddress = config.chains[config.hubDomain].deployments.relayerProxy; + const hubChainId = domainToChainId(+config.hubDomain); // const _totalFee = constants.Zero; const baseAggregateRoot = await database.getBaseAggregateRoot(); From a718acff7dfc325d170958131184593bbe0fbe6f Mon Sep 17 00:00:00 2001 From: Liu <57480598+liu-zhipeng@users.noreply.github.com> Date: Tue, 10 Oct 2023 21:51:06 +0800 Subject: [PATCH 2/5] fix: propose unit tests draft --- packages/adapters/database/test/mock.ts | 1 + packages/agents/lighthouse/.nycrc.json | 6 ++++ .../src/tasks/propose/operations/propose.ts | 2 +- .../agents/lighthouse/test/globalTestHook.ts | 6 ++++ packages/agents/lighthouse/test/mock.ts | 20 +++++++++++++ .../tasks/propose/operations/propose.spec.ts | 29 +++++++++++++++++++ 6 files changed, 63 insertions(+), 1 deletion(-) create mode 100644 packages/agents/lighthouse/test/tasks/propose/operations/propose.spec.ts diff --git a/packages/adapters/database/test/mock.ts b/packages/adapters/database/test/mock.ts index 66522f72b1..529ed5ce72 100644 --- a/packages/adapters/database/test/mock.ts +++ b/packages/adapters/database/test/mock.ts @@ -77,5 +77,6 @@ export const mockDatabase = (): Database => { saveFinalizedRoots: stub().resolves(), savePropagatedOptimisticRoots: stub().resolves(), saveSnapshotRoots: stub().resolves(), + getPendingSnapshots: stub().resolves([mock.entity.snapshotRoot()]), }; }; diff --git a/packages/agents/lighthouse/.nycrc.json b/packages/agents/lighthouse/.nycrc.json index 3ab751ac02..cb0861e9a9 100644 --- a/packages/agents/lighthouse/.nycrc.json +++ b/packages/agents/lighthouse/.nycrc.json @@ -38,6 +38,12 @@ "src/tasks/sendOutboundRoot/sendOutboundRoot.ts", "src/tasks/sendOutboundRoot/operations/index.ts", "src/tasks/sendOutboundRoot/helpers/index.ts", + "src/tasks/propose/errors.ts", + "src/tasks/propose/context.ts", + "src/tasks/propose/index.ts", + "src/tasks/propose/propose.ts", + "src/tasks/propose/operations/index.ts", + "src/tasks/propose/adapters/index.ts", "test/**/*.ts", "./globalTestHook.ts" ] diff --git a/packages/agents/lighthouse/src/tasks/propose/operations/propose.ts b/packages/agents/lighthouse/src/tasks/propose/operations/propose.ts index e8835182b9..4d3db6452d 100644 --- a/packages/agents/lighthouse/src/tasks/propose/operations/propose.ts +++ b/packages/agents/lighthouse/src/tasks/propose/operations/propose.ts @@ -76,7 +76,7 @@ export const propose = async () => { orderedSnapshotRoots.push(...snapshotRoots!.filter((s) => s.spokeDomain === Number(domain))); }); - proposeSnapshot( + await proposeSnapshot( latestSnapshotId, orderedSnapshotRoots.map((s) => s.root), requestContext, diff --git a/packages/agents/lighthouse/test/globalTestHook.ts b/packages/agents/lighthouse/test/globalTestHook.ts index 91c7e49457..d63feff693 100644 --- a/packages/agents/lighthouse/test/globalTestHook.ts +++ b/packages/agents/lighthouse/test/globalTestHook.ts @@ -6,11 +6,13 @@ import { Relayer } from "@connext/nxtp-adapters-relayer"; import { ProverContext } from "../src/tasks/prover/context"; import { ProcessFromRootContext } from "../src/tasks/processFromRoot/context"; +import { ProposeContext } from "../src/tasks/propose/context"; import { mock, mockTaskId } from "./mock"; import * as ProverFns from "../src/tasks/prover/prover"; import * as ProcessFromRootFns from "../src/tasks/processFromRoot/processFromRoot"; import * as PropagateFns from "../src/tasks/propagate/propagate"; import * as SendOutboundRootFns from "../src/tasks/sendOutboundRoot/sendOutboundRoot"; +import * as ProposeFns from "../src/tasks/propose/propose"; import * as Mockable from "../src/mockable"; import { PropagateContext } from "../src/tasks/propagate/context"; import { SendOutboundRootContext } from "../src/tasks/sendOutboundRoot/context"; @@ -20,6 +22,7 @@ export let proverCtxMock: ProverContext; export let processFromRootCtxMock: ProcessFromRootContext; export let propagateCtxMock: PropagateContext; export let sendOutboundRootCtxMock: SendOutboundRootContext; +export let proposeCtxMock: ProposeContext; export let chainReaderMock: SinonStubbedInstance; export let existsSyncStub: SinonStub; @@ -76,6 +79,9 @@ export const mochaHooks = { } as any); getBestProviderMock = stub(Mockable, "getBestProvider").resolves("http://example.com"); + + proposeCtxMock = mock.proposeCtx(); + stub(ProposeFns, "getContext").returns(proposeCtxMock); }, afterEach() { restore(); diff --git a/packages/agents/lighthouse/test/mock.ts b/packages/agents/lighthouse/test/mock.ts index 6bd63acefd..4577c5955e 100644 --- a/packages/agents/lighthouse/test/mock.ts +++ b/packages/agents/lighthouse/test/mock.ts @@ -23,6 +23,7 @@ import { ProcessFromRootContext } from "../src/tasks/processFromRoot/context"; import { PropagateContext } from "../src/tasks/propagate/context"; import { mockSubgraph } from "@connext/nxtp-adapters-subgraph/test/mock"; import { SendOutboundRootContext } from "../src/tasks/sendOutboundRoot/context"; +import { ProposeContext } from "../src/tasks/propose/context"; export const mockTaskId = mkBytes32("0xabcdef123"); export const mockRelayerAddress = mkAddress("0xabcdef123"); @@ -93,6 +94,7 @@ export const mock = { contracts: mock.adapters.contracts(), relayers: mock.adapters.relayers(), database: mock.adapters.database(), + subgraph: mock.adapters.subgraph(), databaseWriter: { database: mock.adapters.database(), pool: mockDatabasePool() }, cache: mockCache() as any, mqClient: mockMqClient() as any, @@ -109,6 +111,7 @@ export const mock = { contracts: mock.adapters.deployments(), relayers: mock.adapters.relayers(), database: mock.adapters.database(), + subgraph: mock.adapters.subgraph(), }, config: mock.config(), chainData: mock.chainData(), @@ -122,6 +125,7 @@ export const mock = { deployments: mock.adapters.deployments(), contracts: mock.adapters.contracts(), relayers: mock.adapters.relayers(), + database: mock.adapters.database(), subgraph: mock.adapters.subgraph(), ambs: mock.adapters.ambs(), }, @@ -144,6 +148,22 @@ export const mock = { chainData: mock.chainData(), }; }, + proposeCtx: (): ProposeContext => { + return { + logger: new Logger({ name: "mock", level: process.env.LOG_LEVEL || "silent" }), + adapters: { + chainreader: mock.adapters.chainreader() as unknown as ChainReader, + deployments: mock.adapters.deployments(), + contracts: mock.adapters.contracts(), + relayers: mock.adapters.relayers(), + database: mock.adapters.database(), + subgraph: mock.adapters.subgraph(), + ambs: mock.adapters.ambs(), + }, + config: mock.config(), + chainData: mock.chainData(), + }; + }, config: (): NxtpLighthouseConfig => ({ chains: { [mock.domain.A]: { diff --git a/packages/agents/lighthouse/test/tasks/propose/operations/propose.spec.ts b/packages/agents/lighthouse/test/tasks/propose/operations/propose.spec.ts new file mode 100644 index 0000000000..7d5030ff07 --- /dev/null +++ b/packages/agents/lighthouse/test/tasks/propose/operations/propose.spec.ts @@ -0,0 +1,29 @@ +import { expect, getNtpTimeSeconds, getRandomBytes32 } from "@connext/nxtp-utils"; +import { SinonStub, stub } from "sinon"; + +import { NoChainIdForHubDomain } from "../../../../src/tasks/propose/errors"; +import { propose } from "../../../../src/tasks/propose/operations"; +import * as ProposeFns from "../../../../src/tasks/propose/operations/propose"; +import { proposeCtxMock, sendWithRelayerWithBackupStub } from "../../../globalTestHook"; +import { mock } from "../../../mock"; +import * as Mockable from "../../../../src/mockable"; + +describe("Operations: Propose", () => { + describe("#propose", () => { + beforeEach(() => {}); + + it("should throw an error if no hub domain id", async () => { + proposeCtxMock.chainData = new Map(); + await expect(propose()).to.eventually.be.rejectedWith(NoChainIdForHubDomain); + }); + + it("should call propose snapshot succesfully", async () => { + let proposeSnapshotStub = stub(ProposeFns, "proposeSnapshot").resolves(); + + (proposeCtxMock.adapters.database.getPendingSnapshots as SinonStub).resolves([mock.entity.snapshotRoot()]); + + await propose(); + expect(proposeSnapshotStub).callCount(1); + }); + }); +}); From 43a2bbcc39df0f5c59026c86c6d1b33776538ede Mon Sep 17 00:00:00 2001 From: Liu <57480598+liu-zhipeng@users.noreply.github.com> Date: Tue, 10 Oct 2023 21:58:23 +0800 Subject: [PATCH 3/5] fix: bypass LH unit tests --- packages/agents/lighthouse/.nycrc.json | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/packages/agents/lighthouse/.nycrc.json b/packages/agents/lighthouse/.nycrc.json index cb0861e9a9..75211df0c9 100644 --- a/packages/agents/lighthouse/.nycrc.json +++ b/packages/agents/lighthouse/.nycrc.json @@ -38,12 +38,9 @@ "src/tasks/sendOutboundRoot/sendOutboundRoot.ts", "src/tasks/sendOutboundRoot/operations/index.ts", "src/tasks/sendOutboundRoot/helpers/index.ts", - "src/tasks/propose/errors.ts", - "src/tasks/propose/context.ts", - "src/tasks/propose/index.ts", - "src/tasks/propose/propose.ts", - "src/tasks/propose/operations/index.ts", - "src/tasks/propose/adapters/index.ts", + "src/tasks/propose/**/*.ts", + "src/tasks/propagate/**/*.ts", + "src/tasks/prover/**/*.ts", "test/**/*.ts", "./globalTestHook.ts" ] From 6b71476d7c66e10b2fc263300a3949306d313f78 Mon Sep 17 00:00:00 2001 From: Liu <57480598+liu-zhipeng@users.noreply.github.com> Date: Tue, 10 Oct 2023 23:24:09 +0800 Subject: [PATCH 4/5] test: watcher unit tests --- packages/agents/watcher/test/mock.ts | 9 ++++- .../test/operations/validateAndSwitch.spec.ts | 38 +++++++++++++++++++ 2 files changed, 45 insertions(+), 2 deletions(-) create mode 100644 packages/agents/watcher/test/operations/validateAndSwitch.spec.ts diff --git a/packages/agents/watcher/test/mock.ts b/packages/agents/watcher/test/mock.ts index ab1ecf869c..5ba7940af2 100644 --- a/packages/agents/watcher/test/mock.ts +++ b/packages/agents/watcher/test/mock.ts @@ -1,6 +1,6 @@ import { Wallet } from "ethers"; import { createStubInstance } from "sinon"; -import { WatcherAdapter } from "@connext/nxtp-adapters-watcher"; +import { OpModeMonitor, WatcherAdapter } from "@connext/nxtp-adapters-watcher"; import { Logger, mkHash, mock as _mock, mockSequencer } from "@connext/nxtp-utils"; import { mockSubgraph } from "@connext/nxtp-adapters-subgraph/test/mock"; @@ -15,13 +15,18 @@ export const mock = { subgraph: mockSubgraph(), wallet: createStubInstance(Wallet, { getAddress: Promise.resolve(mockSequencer) }), watcher: createStubInstance(WatcherAdapter, { - checkInvariants: Promise.resolve(true), + checkInvariants: Promise.resolve({ needsPause: true }), pause: Promise.resolve([ { domain: mock.domain.A, error: "foo", paused: true, relevantTransaction: mkHash("0x1") }, { domain: mock.domain.B, error: "bar", paused: true, relevantTransaction: mkHash("0x1") }, ]), alert: Promise.resolve(), }), + monitor: createStubInstance(OpModeMonitor, { + validateProposal: Promise.resolve({ needsSwitch: false, reason: "" }), + switch: Promise.resolve(), + alert: Promise.resolve(), + }), }, config: mock.config(), chainData: mock.chainData(), diff --git a/packages/agents/watcher/test/operations/validateAndSwitch.spec.ts b/packages/agents/watcher/test/operations/validateAndSwitch.spec.ts new file mode 100644 index 0000000000..70213faf2d --- /dev/null +++ b/packages/agents/watcher/test/operations/validateAndSwitch.spec.ts @@ -0,0 +1,38 @@ +import { BaseRequestContext, createRequestContext, expect, mkHash } from "@connext/nxtp-utils"; +import { PauseResponse, ReportEventType } from "@connext/nxtp-adapters-watcher"; +import { stub, SinonStub } from "sinon"; +import * as validateAndSwitchFns from "../../src/operations/validateAndSwitch"; +import { ctxMock } from "../globalTestHook"; + +const requestContext = createRequestContext("test"); + +describe("Operations:validateAndSwitch", () => { + describe("validateAndSwitch", () => { + let validateAndSwitchStub: SinonStub< + [requestContext: BaseRequestContext, reason: string], + Promise + >; + beforeEach(() => { + validateAndSwitchStub = stub(validateAndSwitchFns, "switchAndAlert").resolves(); + }); + + it("should not call switchAndAlert if it doesnt need pause", async () => { + (ctxMock.adapters.monitor.validateProposal as SinonStub).resolves({ needsSwitch: false }); + await validateAndSwitchFns.validateAndSwitch(); + expect(validateAndSwitchStub.callCount).to.be.eq(0); + }); + + it("should call switchAndAlert if needs pause", async () => { + (ctxMock.adapters.monitor.validateProposal as SinonStub).resolves({ needsSwitch: true }); + await validateAndSwitchFns.validateAndSwitch(); + expect(validateAndSwitchStub).to.have.been.calledOnce; + }); + }); + + describe("switchAndAlert", () => { + it("should switch and alert", async () => { + await validateAndSwitchFns.switchAndAlert(requestContext, "reason"); + expect(ctxMock.adapters.watcher.alert).to.be.calledOnce; + }); + }); +}); From cf1d14a7728d6d6af6e511e2467a27786ee1b74f Mon Sep 17 00:00:00 2001 From: Liu <57480598+liu-zhipeng@users.noreply.github.com> Date: Wed, 11 Oct 2023 00:34:51 +0800 Subject: [PATCH 5/5] test: fix watcher unit test --- .../agents/watcher/test/operations/validateAndSwitch.spec.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/agents/watcher/test/operations/validateAndSwitch.spec.ts b/packages/agents/watcher/test/operations/validateAndSwitch.spec.ts index 70213faf2d..5ea85761bd 100644 --- a/packages/agents/watcher/test/operations/validateAndSwitch.spec.ts +++ b/packages/agents/watcher/test/operations/validateAndSwitch.spec.ts @@ -31,8 +31,9 @@ describe("Operations:validateAndSwitch", () => { describe("switchAndAlert", () => { it("should switch and alert", async () => { + (ctxMock.adapters.monitor.switch as SinonStub).resolves(); await validateAndSwitchFns.switchAndAlert(requestContext, "reason"); - expect(ctxMock.adapters.watcher.alert).to.be.calledOnce; + expect(ctxMock.adapters.monitor.alert).to.be.calledOnce; }); }); });