From 99bc10f144ea35fe824a24cd962a3736cb6e0c7f Mon Sep 17 00:00:00 2001 From: harkamal Date: Mon, 7 Feb 2022 15:32:03 +0530 Subject: [PATCH 1/2] Check, validate and skip if we got any deposit events already present in DB --- packages/lodestar/src/eth1/errors.ts | 7 ++-- .../lodestar/src/eth1/eth1DepositsCache.ts | 32 ++++++++++++++----- 2 files changed, 29 insertions(+), 10 deletions(-) diff --git a/packages/lodestar/src/eth1/errors.ts b/packages/lodestar/src/eth1/errors.ts index 0d5712eb5a67..d939c479d7ce 100644 --- a/packages/lodestar/src/eth1/errors.ts +++ b/packages/lodestar/src/eth1/errors.ts @@ -21,6 +21,8 @@ export enum Eth1ErrorCode { DUPLICATE_DISTINCT_LOG = "ETH1_ERROR_DUPLICATE_DISTINCT_LOG", /** Attempted to insert a log with index != prev + 1 into the Eth1DepositsCache */ NON_CONSECUTIVE_LOGS = "ETH1_ERROR_NON_CONSECUTIVE_LOGS", + /** Expected a deposit log in the db for the index, missing log implies a corrupted db */ + MISSING_DEPOSIT_LOG = "ETH1_ERROR_MISSING_DEPOSIT_LOG", } export type Eth1ErrorType = @@ -31,7 +33,8 @@ export type Eth1ErrorType = | {code: Eth1ErrorCode.NO_DEPOSITS_FOR_BLOCK_RANGE; fromBlock: number; toBlock: number} | {code: Eth1ErrorCode.NO_DEPOSIT_ROOT; depositCount: number} | {code: Eth1ErrorCode.NOT_ENOUGH_DEPOSIT_ROOTS; index: number; treeLength: number} - | {code: Eth1ErrorCode.DUPLICATE_DISTINCT_LOG; newIndex: number; prevIndex: number} - | {code: Eth1ErrorCode.NON_CONSECUTIVE_LOGS; newIndex: number; prevIndex: number}; + | {code: Eth1ErrorCode.DUPLICATE_DISTINCT_LOG; index: number; lastLogIndex: number} + | {code: Eth1ErrorCode.NON_CONSECUTIVE_LOGS; newIndex: number; lastLogIndex: number} + | {code: Eth1ErrorCode.MISSING_DEPOSIT_LOG; index: number; lastLogIndex: number}; export class Eth1Error extends LodestarError {} diff --git a/packages/lodestar/src/eth1/eth1DepositsCache.ts b/packages/lodestar/src/eth1/eth1DepositsCache.ts index 2557c35332dc..e78bcade6cf6 100644 --- a/packages/lodestar/src/eth1/eth1DepositsCache.ts +++ b/packages/lodestar/src/eth1/eth1DepositsCache.ts @@ -1,6 +1,8 @@ import {phase0, ssz} from "@chainsafe/lodestar-types"; +import {byteArrayEquals} from "@chainsafe/ssz"; import {IFilterOptions} from "@chainsafe/lodestar-db"; import {IChainForkConfig} from "@chainsafe/lodestar-config"; + import {IBeaconDb} from "../db"; import {getEth1DataForBlocks} from "./utils/eth1Data"; import {assertConsecutiveDeposits} from "./utils/eth1DepositEvent"; @@ -37,15 +39,29 @@ export class Eth1DepositsCache { assertConsecutiveDeposits(depositEvents); const lastLog = await this.db.depositEvent.lastValue(); - const firstEvent = depositEvents[0]; - if (lastLog !== null && firstEvent !== undefined) { - const newIndex = firstEvent.index; - const prevIndex = lastLog.index; - if (newIndex <= prevIndex) { - throw new Eth1Error({code: Eth1ErrorCode.DUPLICATE_DISTINCT_LOG, newIndex, prevIndex}); + // Check, validate and skip if we got any deposit events already present in DB + if (lastLog !== null) { + const lastLogIndex = lastLog.index; + let skipEvents = 0; + for (; skipEvents < depositEvents.length && depositEvents[skipEvents].index <= lastLogIndex; skipEvents++) { + const depositEvent = depositEvents[skipEvents]; + const prevDBSerializedEvent = await this.db.depositEvent.getBinary(depositEvent.index); + if (!prevDBSerializedEvent) { + throw new Eth1Error({code: Eth1ErrorCode.MISSING_DEPOSIT_LOG, index: depositEvent.index, lastLogIndex}); + } + const serializedEvent = ssz.phase0.DepositEvent.serialize(depositEvent); + if (!byteArrayEquals(prevDBSerializedEvent, serializedEvent)) + throw new Eth1Error({code: Eth1ErrorCode.DUPLICATE_DISTINCT_LOG, index: depositEvent.index, lastLogIndex}); + skipEvents++; } - if (newIndex > prevIndex + 1) { - throw new Eth1Error({code: Eth1ErrorCode.NON_CONSECUTIVE_LOGS, newIndex, prevIndex}); + + if (skipEvents > 0) depositEvents.splice(0, skipEvents); + const firstEvent = depositEvents[0]; + if (firstEvent !== undefined) { + const newIndex = firstEvent.index; + if (newIndex > lastLogIndex + 1) { + throw new Eth1Error({code: Eth1ErrorCode.NON_CONSECUTIVE_LOGS, newIndex, lastLogIndex}); + } } } From 8f1ded354e1324fa19f65533eb36245fa6494368 Mon Sep 17 00:00:00 2001 From: harkamal Date: Mon, 7 Feb 2022 21:26:26 +0530 Subject: [PATCH 2/2] Refactor the logic, provide for a flag to overwrite previous data --- .../cli/src/options/beaconNodeOptions/eth1.ts | 11 +++ .../unit/options/beaconNodeOptions.test.ts | 2 + packages/lodestar/src/eth1/errors.ts | 4 +- .../src/eth1/eth1DepositDataTracker.ts | 2 +- .../lodestar/src/eth1/eth1DepositsCache.ts | 68 +++++++++++++------ packages/lodestar/src/eth1/options.ts | 2 + .../e2e/eth1/eth1ForBlockProduction.test.ts | 1 + .../e2e/eth1/eth1MergeBlockTracker.test.ts | 1 + .../test/e2e/eth1/eth1Provider.test.ts | 1 + .../lodestar/test/e2e/eth1/stream.test.ts | 1 + 10 files changed, 70 insertions(+), 23 deletions(-) diff --git a/packages/cli/src/options/beaconNodeOptions/eth1.ts b/packages/cli/src/options/beaconNodeOptions/eth1.ts index 202a507cd01c..4b74f1b92695 100644 --- a/packages/cli/src/options/beaconNodeOptions/eth1.ts +++ b/packages/cli/src/options/beaconNodeOptions/eth1.ts @@ -7,6 +7,7 @@ export interface IEth1Args { "eth1.providerUrls": string[]; "eth1.depositContractDeployBlock": number; "eth1.disableEth1DepositDataTracker": boolean; + "eth1.unsafeAllowDepositDataOverwrite": boolean; } export function parseArgs(args: IEth1Args): IBeaconNodeOptions["eth1"] { @@ -22,6 +23,7 @@ export function parseArgs(args: IEth1Args): IBeaconNodeOptions["eth1"] { providerUrls: providerUrls, depositContractDeployBlock: args["eth1.depositContractDeployBlock"], disableEth1DepositDataTracker: args["eth1.disableEth1DepositDataTracker"], + unsafeAllowDepositDataOverwrite: args["eth1.unsafeAllowDepositDataOverwrite"], }; } @@ -61,4 +63,13 @@ export const options: ICliCommandOptions = { defaultDescription: String(defaultOptions.eth1.disableEth1DepositDataTracker), group: "eth1", }, + + "eth1.unsafeAllowDepositDataOverwrite": { + hidden: true, + description: + "Allow the deposit tracker to overwrite previously fetched and saved deposit event data. Warning!!! This is an unsafe operation, so enable this flag only if you know what you are doing.", + type: "boolean", + defaultDescription: String(defaultOptions.eth1.unsafeAllowDepositDataOverwrite), + group: "eth1", + }, }; diff --git a/packages/cli/test/unit/options/beaconNodeOptions.test.ts b/packages/cli/test/unit/options/beaconNodeOptions.test.ts index e24cc439e883..0b577c29ac9a 100644 --- a/packages/cli/test/unit/options/beaconNodeOptions.test.ts +++ b/packages/cli/test/unit/options/beaconNodeOptions.test.ts @@ -24,6 +24,7 @@ describe("options / beaconNodeOptions", () => { "eth1.providerUrls": ["http://my.node:8545"], "eth1.depositContractDeployBlock": 1625314, "eth1.disableEth1DepositDataTracker": true, + "eth1.unsafeAllowDepositDataOverwrite": false, "execution.urls": ["http://localhost:8550"], "execution.timeout": 12000, @@ -78,6 +79,7 @@ describe("options / beaconNodeOptions", () => { providerUrls: ["http://my.node:8545"], depositContractDeployBlock: 1625314, disableEth1DepositDataTracker: true, + unsafeAllowDepositDataOverwrite: false, }, executionEngine: { urls: ["http://localhost:8550"], diff --git a/packages/lodestar/src/eth1/errors.ts b/packages/lodestar/src/eth1/errors.ts index d939c479d7ce..8585b8f227b8 100644 --- a/packages/lodestar/src/eth1/errors.ts +++ b/packages/lodestar/src/eth1/errors.ts @@ -33,8 +33,8 @@ export type Eth1ErrorType = | {code: Eth1ErrorCode.NO_DEPOSITS_FOR_BLOCK_RANGE; fromBlock: number; toBlock: number} | {code: Eth1ErrorCode.NO_DEPOSIT_ROOT; depositCount: number} | {code: Eth1ErrorCode.NOT_ENOUGH_DEPOSIT_ROOTS; index: number; treeLength: number} - | {code: Eth1ErrorCode.DUPLICATE_DISTINCT_LOG; index: number; lastLogIndex: number} + | {code: Eth1ErrorCode.DUPLICATE_DISTINCT_LOG; newIndex: number; lastLogIndex: number} | {code: Eth1ErrorCode.NON_CONSECUTIVE_LOGS; newIndex: number; lastLogIndex: number} - | {code: Eth1ErrorCode.MISSING_DEPOSIT_LOG; index: number; lastLogIndex: number}; + | {code: Eth1ErrorCode.MISSING_DEPOSIT_LOG; newIndex: number; lastLogIndex: number}; export class Eth1Error extends LodestarError {} diff --git a/packages/lodestar/src/eth1/eth1DepositDataTracker.ts b/packages/lodestar/src/eth1/eth1DepositDataTracker.ts index f7776bf355c6..27b179160f9f 100644 --- a/packages/lodestar/src/eth1/eth1DepositDataTracker.ts +++ b/packages/lodestar/src/eth1/eth1DepositDataTracker.ts @@ -54,7 +54,7 @@ export class Eth1DepositDataTracker { this.signal = signal; this.logger = logger; this.eth1Provider = eth1Provider; - this.depositsCache = new Eth1DepositsCache(config, db); + this.depositsCache = new Eth1DepositsCache(opts, config, db); this.eth1DataCache = new Eth1DataCache(config, db); this.lastProcessedDepositBlockNumber = null; diff --git a/packages/lodestar/src/eth1/eth1DepositsCache.ts b/packages/lodestar/src/eth1/eth1DepositsCache.ts index e78bcade6cf6..0a55ccb7e61f 100644 --- a/packages/lodestar/src/eth1/eth1DepositsCache.ts +++ b/packages/lodestar/src/eth1/eth1DepositsCache.ts @@ -10,12 +10,14 @@ import {getDepositsWithProofs} from "./utils/deposits"; import {Eth1Error, Eth1ErrorCode} from "./errors"; export class Eth1DepositsCache { + unsafeAllowDepositDataOverwrite: boolean; db: IBeaconDb; config: IChainForkConfig; - constructor(config: IChainForkConfig, db: IBeaconDb) { + constructor(opts: {unsafeAllowDepositDataOverwrite: boolean}, config: IChainForkConfig, db: IBeaconDb) { this.config = config; this.db = db; + this.unsafeAllowDepositDataOverwrite = opts.unsafeAllowDepositDataOverwrite; } /** @@ -39,29 +41,55 @@ export class Eth1DepositsCache { assertConsecutiveDeposits(depositEvents); const lastLog = await this.db.depositEvent.lastValue(); + const firstEvent = depositEvents[0]; + // Check, validate and skip if we got any deposit events already present in DB - if (lastLog !== null) { + // This can happen if the remote eth1/EL resets its head in these four scenarios: + // 1. Remote eth1/EL resynced/restarted from head behind its previous head pre-merge + // 2. In a post merge scenario, Lodestar restarted from finalized state from DB which + // generally is a few epochs behind the last synced head. This causes eth1 tracker to reset + // and refetch the deposits as the lodestar syncs further along (Post merge there is 1-1 + // correspondence between EL and CL blocks) + // 3. The EL reorged beyond the eth1 follow distance. + // + // While 1. & 2. are benign and we handle them below by checking if the duplicate log fetched + // is same as one written in DB. Refer to this issue for some data dump of how this happens + // https://github.com/ChainSafe/lodestar/issues/3674 + // + // If the duplicate log fetched is not same as written in DB then its probablu scenario 3. + // which would be a catastrophic event for the network (or we messed up real bad!!!). + // + // So we provide for a way to overwrite this log without deleting full db via + // --unsafeAllowDepositDataOverwrite cli flag which will just overwrite the previous tracker data + // if any. This option as indicated by its name is unsafe and to be only used if you know what + // you are doing. + if (lastLog !== null && firstEvent !== undefined) { + const newIndex = firstEvent.index; const lastLogIndex = lastLog.index; - let skipEvents = 0; - for (; skipEvents < depositEvents.length && depositEvents[skipEvents].index <= lastLogIndex; skipEvents++) { - const depositEvent = depositEvents[skipEvents]; - const prevDBSerializedEvent = await this.db.depositEvent.getBinary(depositEvent.index); - if (!prevDBSerializedEvent) { - throw new Eth1Error({code: Eth1ErrorCode.MISSING_DEPOSIT_LOG, index: depositEvent.index, lastLogIndex}); - } - const serializedEvent = ssz.phase0.DepositEvent.serialize(depositEvent); - if (!byteArrayEquals(prevDBSerializedEvent, serializedEvent)) - throw new Eth1Error({code: Eth1ErrorCode.DUPLICATE_DISTINCT_LOG, index: depositEvent.index, lastLogIndex}); - skipEvents++; - } - if (skipEvents > 0) depositEvents.splice(0, skipEvents); - const firstEvent = depositEvents[0]; - if (firstEvent !== undefined) { - const newIndex = firstEvent.index; - if (newIndex > lastLogIndex + 1) { - throw new Eth1Error({code: Eth1ErrorCode.NON_CONSECUTIVE_LOGS, newIndex, lastLogIndex}); + if (!this.unsafeAllowDepositDataOverwrite && firstEvent.index <= lastLog.index) { + // lastLogIndex - newIndex + 1 events are duplicate since this is a consecutive log + // as asserted by assertConsecutiveDeposits. Splice those events out from depositEvents. + const skipEvents = depositEvents.splice(0, lastLogIndex - newIndex + 1); + // After splicing skipEvents will contain duplicate events to be checked and validated + // and rest of the remaining events in depositEvents could be safely written to DB and + // move the tracker along. + for (const depositEvent of skipEvents) { + const prevDBSerializedEvent = await this.db.depositEvent.getBinary(depositEvent.index); + if (!prevDBSerializedEvent) { + throw new Eth1Error({code: Eth1ErrorCode.MISSING_DEPOSIT_LOG, newIndex, lastLogIndex}); + } + const serializedEvent = ssz.phase0.DepositEvent.serialize(depositEvent); + if (!byteArrayEquals(prevDBSerializedEvent, serializedEvent)) { + throw new Eth1Error({code: Eth1ErrorCode.DUPLICATE_DISTINCT_LOG, newIndex, lastLogIndex}); + } } + } else if (newIndex > lastLogIndex + 1) { + // deposit events need to be consective, the way we fetch our tracker. If the deposit event + // is not consecutive it means either our tracker, or the corresponding eth1/EL + // node or the database has messed up. All these failures are critical and the tracker + // shouldn't proceed without the resolution of this error. + throw new Eth1Error({code: Eth1ErrorCode.NON_CONSECUTIVE_LOGS, newIndex, lastLogIndex}); } } diff --git a/packages/lodestar/src/eth1/options.ts b/packages/lodestar/src/eth1/options.ts index 0a7466ea0518..580c89fe53f6 100644 --- a/packages/lodestar/src/eth1/options.ts +++ b/packages/lodestar/src/eth1/options.ts @@ -3,10 +3,12 @@ export type Eth1Options = { disableEth1DepositDataTracker?: boolean; providerUrls: string[]; depositContractDeployBlock?: number; + unsafeAllowDepositDataOverwrite: boolean; }; export const defaultEth1Options: Eth1Options = { enabled: true, providerUrls: ["http://localhost:8545"], depositContractDeployBlock: 0, + unsafeAllowDepositDataOverwrite: false, }; diff --git a/packages/lodestar/test/e2e/eth1/eth1ForBlockProduction.test.ts b/packages/lodestar/test/e2e/eth1/eth1ForBlockProduction.test.ts index 732e32efa986..4025e1c164ad 100644 --- a/packages/lodestar/test/e2e/eth1/eth1ForBlockProduction.test.ts +++ b/packages/lodestar/test/e2e/eth1/eth1ForBlockProduction.test.ts @@ -66,6 +66,7 @@ describe("eth1 / Eth1Provider", function () { enabled: true, providerUrls: [getGoerliRpcUrl()], depositContractDeployBlock: medallaTestnetConfig.depositBlock, + unsafeAllowDepositDataOverwrite: false, }; const eth1Provider = new Eth1Provider(config, eth1Options, controller.signal); diff --git a/packages/lodestar/test/e2e/eth1/eth1MergeBlockTracker.test.ts b/packages/lodestar/test/e2e/eth1/eth1MergeBlockTracker.test.ts index 4f8367789f2f..ec5ecb9c84ce 100644 --- a/packages/lodestar/test/e2e/eth1/eth1MergeBlockTracker.test.ts +++ b/packages/lodestar/test/e2e/eth1/eth1MergeBlockTracker.test.ts @@ -37,6 +37,7 @@ describe("eth1 / Eth1MergeBlockTracker", function () { enabled: true, providerUrls: [getGoerliRpcUrl()], depositContractDeployBlock: 0, + unsafeAllowDepositDataOverwrite: false, }; }); diff --git a/packages/lodestar/test/e2e/eth1/eth1Provider.test.ts b/packages/lodestar/test/e2e/eth1/eth1Provider.test.ts index e64b839f62ca..b05029944dce 100644 --- a/packages/lodestar/test/e2e/eth1/eth1Provider.test.ts +++ b/packages/lodestar/test/e2e/eth1/eth1Provider.test.ts @@ -24,6 +24,7 @@ describe("eth1 / Eth1Provider", function () { enabled: true, providerUrls: [getGoerliRpcUrl()], depositContractDeployBlock: 0, + unsafeAllowDepositDataOverwrite: false, }; return new Eth1Provider(config, eth1Options, controller.signal); } diff --git a/packages/lodestar/test/e2e/eth1/stream.test.ts b/packages/lodestar/test/e2e/eth1/stream.test.ts index df81d4effa65..d1372076e09f 100644 --- a/packages/lodestar/test/e2e/eth1/stream.test.ts +++ b/packages/lodestar/test/e2e/eth1/stream.test.ts @@ -22,6 +22,7 @@ describe("Eth1 streams", function () { enabled: true, providerUrls: [getGoerliRpcUrl()], depositContractDeployBlock: 0, + unsafeAllowDepositDataOverwrite: false, }; return new Eth1Provider(config, eth1Options, controller.signal); }