From 549aa03a40e4090d752e7275d3e29046e6214b4b Mon Sep 17 00:00:00 2001 From: Nazar Hussain Date: Tue, 1 Aug 2023 19:00:12 +0200 Subject: [PATCH] fix(prover): allow payload provider to work with no finalized block (#5820) * Fix some issues in the prover * Remove unused file * Fix browser tests * Add comment for buffer --- .../prover/src/proof_provider/ordered_map.ts | 12 +- .../src/proof_provider/payload_store.ts | 41 +- packages/prover/src/utils/consensus.ts | 2 + packages/prover/src/utils/validation.ts | 4 +- .../unit/proof_provider/orderd_map.test.ts | 40 ++ .../unit/proof_provider/payload_store.test.ts | 404 ++++++++++++++++++ 6 files changed, 486 insertions(+), 17 deletions(-) create mode 100644 packages/prover/test/unit/proof_provider/orderd_map.test.ts create mode 100644 packages/prover/test/unit/proof_provider/payload_store.test.ts diff --git a/packages/prover/src/proof_provider/ordered_map.ts b/packages/prover/src/proof_provider/ordered_map.ts index 943941569df5..acf6ec2da4ef 100644 --- a/packages/prover/src/proof_provider/ordered_map.ts +++ b/packages/prover/src/proof_provider/ordered_map.ts @@ -1,21 +1,21 @@ export class OrderedMap extends Map { - private _min = 0; - private _max = 0; + private _min?: number; + private _max?: number; - get min(): number { + get min(): number | undefined { return this._min; } - get max(): number { + get max(): number | undefined { return this._max; } set(key: number, value: T): this { - if (key < this._min) { + if (this._min === undefined || key < this._min) { this._min = key; } - if (key > this._max) { + if (this._max === undefined || key > this._max) { this._max = key; } diff --git a/packages/prover/src/proof_provider/payload_store.ts b/packages/prover/src/proof_provider/payload_store.ts index bb4977ff3827..7cc5e3753f4e 100644 --- a/packages/prover/src/proof_provider/payload_store.ts +++ b/packages/prover/src/proof_provider/payload_store.ts @@ -13,10 +13,14 @@ type BlockCLRoot = string; * The in-memory store for the execution payloads to be used to verify the proofs */ export class PayloadStore { - // We store the block numbers only for finalized blocks + // We store the block root from execution for finalized blocks + // As these blocks are finalized, so not to be worried about conflicting roots private finalizedRoots = new OrderedMap(); - // Unfinalized blocks are stored by the roots of the beacon chain + // Unfinalized blocks may change over time and may have conflicting roots + // We can receive multiple light-client headers for the same block of execution + // So we why store unfinalized payloads by their CL root, which is only used + // in processing the light-client headers private unfinalizedRoots = new Map(); // Payloads store with BlockELRoot as key @@ -27,7 +31,13 @@ export class PayloadStore { constructor(private opts: {api: Api; logger: Logger}) {} get finalized(): allForks.ExecutionPayload | undefined { - const finalizedMaxRoot = this.finalizedRoots.get(this.finalizedRoots.max); + const maxBlockNumberForFinalized = this.finalizedRoots.max; + + if (maxBlockNumberForFinalized === undefined) { + return; + } + + const finalizedMaxRoot = this.finalizedRoots.get(maxBlockNumberForFinalized); if (finalizedMaxRoot) { return this.payloads.get(finalizedMaxRoot); } @@ -67,8 +77,15 @@ export class PayloadStore { return undefined; } - async getOrFetchFinalizedPayload(blockNumber: number): Promise { - if (blockNumber > this.finalizedRoots.max) { + protected async getOrFetchFinalizedPayload(blockNumber: number): Promise { + const maxBlockNumberForFinalized = this.finalizedRoots.max; + const minBlockNumberForFinalized = this.finalizedRoots.min; + + if (maxBlockNumberForFinalized === undefined || minBlockNumberForFinalized === undefined) { + return; + } + + if (blockNumber > maxBlockNumberForFinalized) { throw new Error( `Block number ${blockNumber} is higher than the latest finalized block number. We recommend to use block hash for unfinalized blocks.` ); @@ -77,7 +94,7 @@ export class PayloadStore { let blockELRoot = this.finalizedRoots.get(blockNumber); // check if we have payload cached locally else fetch from api if (!blockELRoot) { - const payloads = await getExecutionPayloadForBlockNumber(this.opts.api, this.finalizedRoots.min, blockNumber); + const payloads = await getExecutionPayloadForBlockNumber(this.opts.api, minBlockNumberForFinalized, blockNumber); for (const payload of Object.values(payloads)) { this.set(payload, true); } @@ -129,7 +146,7 @@ export class PayloadStore { // If the block is finalized and we do not have the payload // We need to fetch and set the payload - else if (finalized && !existingELRoot) { + else { this.payloads.set( bufferToHex(header.execution.blockHash), ( @@ -155,9 +172,11 @@ export class PayloadStore { // Re-org happened, we need to update the payload if (existingELRoot && existingELRoot !== blockELRoot) { this.payloads.delete(existingELRoot); - this.unfinalizedRoots.set(blockCLRoot, blockELRoot); } + // This is unfinalized header we need to store it's root related to cl root + this.unfinalizedRoots.set(blockCLRoot, blockELRoot); + // We do not have the payload for this block, we need to fetch it const payload = ( await getExecutionPayloads({ @@ -171,12 +190,14 @@ export class PayloadStore { this.prune(); } - private prune(): void { + prune(): void { if (this.finalizedRoots.size <= MAX_PAYLOAD_HISTORY) return; + // Store doe not have any finalized blocks means it's recently initialized + if (this.finalizedRoots.max === undefined || this.finalizedRoots.min === undefined) return; for ( let blockNumber = this.finalizedRoots.max - MAX_PAYLOAD_HISTORY; - blockNumber > this.finalizedRoots.min; + blockNumber >= this.finalizedRoots.min; blockNumber-- ) { const blockELRoot = this.finalizedRoots.get(blockNumber); diff --git a/packages/prover/src/utils/consensus.ts b/packages/prover/src/utils/consensus.ts index b9fd3077a9be..d008a8e42459 100644 --- a/packages/prover/src/utils/consensus.ts +++ b/packages/prover/src/utils/consensus.ts @@ -56,6 +56,7 @@ export async function getExecutionPayloads({ let slot = endSlot; let block = await fetchNearestBlock(api, slot, "down"); payloads[block.message.slot] = block.message.body.executionPayload; + slot = block.message.slot - 1; while (slot >= startSlot) { const previousBlock = await fetchNearestBlock(api, block.message.slot - 1, "down"); @@ -84,6 +85,7 @@ export async function getExecutionPayloadForBlockNumber( while (payloads[block.message.slot].blockNumber !== blockNumber) { const previousBlock = await fetchNearestBlock(api, block.message.slot - 1, "down"); block = previousBlock; + payloads[block.message.slot] = block.message.body.executionPayload; } return payloads; diff --git a/packages/prover/src/utils/validation.ts b/packages/prover/src/utils/validation.ts index 42026b0af839..3754bc0add87 100644 --- a/packages/prover/src/utils/validation.ts +++ b/packages/prover/src/utils/validation.ts @@ -78,9 +78,11 @@ export async function isValidStorageKeys({ sp.proof.map(hexToBuffer) ); + // buffer.equals is not compatible with Uint8Array for browser + // so we need to convert the output of RLP.encode to Buffer first const isStorageValid = (!expectedStorageRLP && sp.value === "0x0") || - (!!expectedStorageRLP && expectedStorageRLP.equals(RLP.encode(sp.value))); + (!!expectedStorageRLP && expectedStorageRLP.equals(Buffer.from(RLP.encode(sp.value)))); if (!isStorageValid) return false; } catch (err) { logger.error("Error verifying storage keys", undefined, err as Error); diff --git a/packages/prover/test/unit/proof_provider/orderd_map.test.ts b/packages/prover/test/unit/proof_provider/orderd_map.test.ts new file mode 100644 index 000000000000..098f4f9127d5 --- /dev/null +++ b/packages/prover/test/unit/proof_provider/orderd_map.test.ts @@ -0,0 +1,40 @@ +import {expect} from "chai"; +import {OrderedMap} from "../../../src/proof_provider/ordered_map.js"; + +describe("proof_provider/ordered_map", () => { + it("should initialize the min with undefined", () => { + const omap = new OrderedMap(); + + expect(omap.min).to.undefined; + }); + + it("should initialize the max with undefined", () => { + const omap = new OrderedMap(); + + expect(omap.max).to.undefined; + }); + + it("should set the min and max to the first value ", () => { + const omap = new OrderedMap(); + omap.set(11, "value"); + + expect(omap.min).eql(11); + expect(omap.max).eql(11); + }); + + it("should set the max value", () => { + const omap = new OrderedMap(); + omap.set(10, "value"); + omap.set(11, "value"); + + expect(omap.max).eql(11); + }); + + it("should set the min value", () => { + const omap = new OrderedMap(); + omap.set(10, "value"); + omap.set(11, "value"); + + expect(omap.min).eql(10); + }); +}); diff --git a/packages/prover/test/unit/proof_provider/payload_store.test.ts b/packages/prover/test/unit/proof_provider/payload_store.test.ts new file mode 100644 index 000000000000..dc99ab1baa22 --- /dev/null +++ b/packages/prover/test/unit/proof_provider/payload_store.test.ts @@ -0,0 +1,404 @@ +import {expect} from "chai"; +import chai from "chai"; +import sinon from "sinon"; +import sinonChai from "sinon-chai"; +import {Api} from "@lodestar/api"; +import {hash} from "@lodestar/utils"; +import {Logger} from "@lodestar/logger"; +import {allForks, capella} from "@lodestar/types"; +import {toHexString} from "@lodestar/utils"; +import {PayloadStore} from "../../../src/proof_provider/payload_store.js"; +import {MAX_PAYLOAD_HISTORY} from "../../../src/constants.js"; + +chai.use(sinonChai); + +const createHash = (input: string): Uint8Array => hash(Buffer.from(input, "utf8")); + +const buildPayload = ({blockNumber}: {blockNumber: number}): allForks.ExecutionPayload => + ({ + blockNumber, + blockHash: createHash(`"block-hash-${blockNumber}`), + parentHash: createHash(`"parent-hash-${blockNumber}`), + }) as unknown as allForks.ExecutionPayload; + +const buildLCHeader = ({slot, blockNumber}: {slot: number; blockNumber: number}): capella.LightClientHeader => + ({ + beacon: {slot, stateRoot: createHash(`"beacon-state-root-${slot}`)}, + execution: buildPayload({blockNumber}), + }) as unknown as capella.LightClientHeader; + +const buildBlock = ({slot, blockNumber}: {slot: number; blockNumber: number}): allForks.SignedBeaconBlock => + ({ + signature: createHash(`"beacon-block-signature-${slot}`), + message: { + slot, + proposerIndex: 0, + parentRoot: createHash(`"beacon-parent-root-${slot}`), + stateRoot: createHash(`"beacon-state-root-${slot}`), + body: { + executionPayload: buildPayload({blockNumber}), + }, + }, + }) as unknown as allForks.SignedBeaconBlock; + +const buildBlockResponse = ({ + slot, + blockNumber, +}: { + slot: number; + blockNumber: number; +}): {ok: boolean; response: {version: number; executionOptimistic: boolean; data: allForks.SignedBeaconBlock}} => ({ + ok: true, + response: { + version: 12, + executionOptimistic: true, + data: buildBlock({slot, blockNumber}), + }, +}); + +describe("proof_provider/payload_store", function () { + let api: Api; + let logger: Logger; + let store: PayloadStore; + + beforeEach(() => { + api = {beacon: {getBlockV2: sinon.stub()}} as unknown as Api; + logger = console as unknown as Logger; + store = new PayloadStore({api, logger}); + }); + + describe("finalized", () => { + it("should return undefined for an empty store", () => { + expect(store.finalized).to.undefined; + }); + + it("should return undefined if no finalized block", () => { + store.set(buildPayload({blockNumber: 10}), false); + + expect(store.finalized).to.undefined; + }); + + it("should return finalized payload", () => { + const payload = buildPayload({blockNumber: 10}); + store.set(payload, true); + + expect(store.finalized).to.eql(payload); + }); + + it("should return highest finalized payload", () => { + const payload1 = buildPayload({blockNumber: 10}); + const payload2 = buildPayload({blockNumber: 11}); + store.set(payload1, true); + store.set(payload2, true); + + expect(store.finalized).to.eql(payload2); + }); + }); + + describe("latest", () => { + it("should return undefined for an empty store", () => { + expect(store.latest).to.undefined; + }); + + it("should return latest payload if finalized", () => { + const payload1 = buildPayload({blockNumber: 10}); + const payload2 = buildPayload({blockNumber: 11}); + store.set(payload1, true); + store.set(payload2, true); + + expect(store.latest).to.eql(payload2); + }); + + it("should return latest payload if not finalized", () => { + const payload1 = buildPayload({blockNumber: 10}); + const payload2 = buildPayload({blockNumber: 11}); + store.set(payload1, false); + store.set(payload2, false); + + expect(store.latest).to.eql(payload2); + }); + }); + + describe("get", () => { + it("should return undefined for an empty store", async () => { + await expect(store.get(10)).to.eventually.undefined; + }); + + it("should return undefined for non existing block id", async () => { + const payload1 = buildPayload({blockNumber: 10}); + store.set(payload1, false); + + await expect(store.get(11)).to.eventually.undefined; + }); + + it("should return undefined for non existing block hash", async () => { + const payload1 = buildPayload({blockNumber: 10}); + store.set(payload1, false); + const nonExistingBlockHash = createHash("non-existing-block-hash"); + + await expect(store.get(toHexString(nonExistingBlockHash))).to.eventually.undefined; + }); + + describe("block hash as blockId", () => { + it("should return payload for a block hash", async () => { + const payload1 = buildPayload({blockNumber: 10}); + store.set(payload1, false); + + await expect(store.get(toHexString(payload1.blockHash))).to.eventually.eql(payload1); + }); + }); + + describe("block number as blockId", () => { + it("should throw error to use block hash for un-finalized blocks", async () => { + const finalizedPayload = buildPayload({blockNumber: 10}); + store.set(finalizedPayload, true); + + await expect(store.get(11)).to.rejectedWith( + "Block number 11 is higher than the latest finalized block number. We recommend to use block hash for unfinalized blocks." + ); + }); + + it("should return undefined if payload exists but not-finalized", async () => { + const payload1 = buildPayload({blockNumber: 10}); + store.set(payload1, false); + + await expect(store.get(10)).to.eventually.undefined; + }); + + it("should return payload for a block number in hex", async () => { + const payload1 = buildPayload({blockNumber: 10}); + store.set(payload1, true); + + await expect(store.get(`0x${payload1.blockNumber.toString(16)}`)).to.eventually.eql(payload1); + }); + + it("should return payload for a block number as string", async () => { + const payload1 = buildPayload({blockNumber: 10}); + store.set(payload1, true); + + await expect(store.get(payload1.blockNumber.toString())).to.eventually.eql(payload1); + }); + + it("should return payload for a block number as integer", async () => { + const payload1 = buildPayload({blockNumber: 10}); + store.set(payload1, true); + + await expect(store.get(10)).to.eventually.eql(payload1); + }); + + it("should fetch the finalized payload from API if payload root not exists", async () => { + const blockNumber = 10; + // It should be less than the finalized block to considered as finalized + const unavailableBlockNumber = 9; + const availablePayload = buildPayload({blockNumber}); + const unavailablePayload = buildPayload({blockNumber: unavailableBlockNumber}); + + (api.beacon.getBlockV2 as sinon.SinonStub) + .withArgs(blockNumber) + .resolves(buildBlockResponse({blockNumber, slot: blockNumber})); + + (api.beacon.getBlockV2 as sinon.SinonStub) + .withArgs(unavailableBlockNumber) + .resolves(buildBlockResponse({blockNumber: unavailableBlockNumber, slot: unavailableBlockNumber})); + + store.set(availablePayload, true); + + const result = await store.get(unavailablePayload.blockNumber); + + expect(api.beacon.getBlockV2 as sinon.SinonStub).calledTwice; + expect(api.beacon.getBlockV2 as sinon.SinonStub).calledWith(blockNumber); + expect(api.beacon.getBlockV2 as sinon.SinonStub).calledWith(unavailableBlockNumber); + expect(result).to.eql(unavailablePayload); + }); + }); + }); + + describe("set", () => { + it("should set the payload for non-finalized blocks", async () => { + const payload1 = buildPayload({blockNumber: 10}); + store.set(payload1, false); + + // Unfinalized blocks are not indexed by block hash + await expect(store.get(toHexString(payload1.blockHash))).to.eventually.eql(payload1); + expect(store.finalized).to.eql(undefined); + }); + + it("should set the payload for finalized blocks", async () => { + const payload1 = buildPayload({blockNumber: 10}); + store.set(payload1, true); + + await expect(store.get(payload1.blockNumber.toString())).to.eventually.eql(payload1); + expect(store.finalized).to.eql(payload1); + }); + }); + + describe("processLCHeader", () => { + describe("unfinalized header", () => { + it("should process lightclient header for un-finalized block", () => {}); + }); + + describe("finalized header", () => { + it("should process lightclient header for finalized block which does not exists in store", async () => { + const blockNumber = 10; + const slot = 20; + const header = buildLCHeader({slot, blockNumber}); + const blockResponse = buildBlockResponse({blockNumber, slot}); + const executionPayload = (blockResponse.response.data as capella.SignedBeaconBlock).message.body + .executionPayload; + (api.beacon.getBlockV2 as sinon.SinonStub).resolves(blockResponse); + + await store.processLCHeader(header, true); + + expect(api.beacon.getBlockV2).calledOnce; + expect(api.beacon.getBlockV2).calledWith(20); + expect(store.finalized).to.eql(executionPayload); + }); + + it("should process lightclient header for finalized block which exists as un-finalized in store", async () => { + const blockNumber = 10; + const slot = 20; + const header = buildLCHeader({slot, blockNumber}); + const blockResponse = buildBlockResponse({blockNumber, slot}); + const executionPayload = (blockResponse.response.data as capella.SignedBeaconBlock).message.body + .executionPayload; + (api.beacon.getBlockV2 as sinon.SinonStub).resolves(blockResponse); + + expect(store.finalized).to.undefined; + // First process as unfinalized + await store.processLCHeader(header, false); + + // Then process as finalized + await store.processLCHeader(header, true); + + // Called only once when we process unfinalized + expect(api.beacon.getBlockV2).to.be.calledOnce; + expect(store.finalized).to.eql(executionPayload); + }); + }); + + it("should fetch non-existing payload for lightclient header", async () => { + const blockNumber = 10; + const slot = 20; + const header = buildLCHeader({slot, blockNumber}); + (api.beacon.getBlockV2 as sinon.SinonStub).resolves(buildBlockResponse({blockNumber, slot})); + + await store.processLCHeader(header); + + expect(api.beacon.getBlockV2).calledOnce; + expect(api.beacon.getBlockV2).calledWith(20); + }); + + it("should not fetch existing payload for lightclient header", async () => { + const blockNumber = 10; + const slot = 20; + const header = buildLCHeader({slot, blockNumber}); + (api.beacon.getBlockV2 as sinon.SinonStub).resolves(buildBlockResponse({blockNumber, slot})); + + await store.processLCHeader(header); + + // Process same header twice + await store.processLCHeader(header); + + // The network fetch should be done once + expect(api.beacon.getBlockV2).calledOnce; + expect(api.beacon.getBlockV2).calledWith(20); + }); + + it("should prune the existing payloads", async () => { + const blockNumber = 10; + const slot = 20; + const header = buildLCHeader({slot, blockNumber}); + (api.beacon.getBlockV2 as sinon.SinonStub).resolves(buildBlockResponse({blockNumber, slot})); + + sinon.spy(store, "prune"); + + await store.processLCHeader(header); + + expect(store.prune).to.be.calledOnce; + }); + }); + + describe("prune", () => { + it("should prune without error for empty store", () => { + expect(() => store.prune()).not.to.throw; + }); + + it("should prune the existing payloads if larger than MAX_PAYLOAD_HISTORY", () => { + const numberOfPayloads = MAX_PAYLOAD_HISTORY + 2; + + for (let i = 1; i <= numberOfPayloads; i++) { + store.set(buildPayload({blockNumber: i}), true); + } + + expect(store["payloads"].size).to.equal(numberOfPayloads); + + store.prune(); + + expect(store["payloads"].size).to.equal(MAX_PAYLOAD_HISTORY); + }); + + it("should not prune the existing payloads if equal to MAX_PAYLOAD_HISTORY", () => { + const numberOfPayloads = MAX_PAYLOAD_HISTORY; + + for (let i = 1; i <= numberOfPayloads; i++) { + store.set(buildPayload({blockNumber: i}), true); + } + + expect(store["payloads"].size).to.equal(MAX_PAYLOAD_HISTORY); + + store.prune(); + + expect(store["payloads"].size).to.equal(MAX_PAYLOAD_HISTORY); + }); + + it("should not prune the existing payloads if less than MAX_PAYLOAD_HISTORY", () => { + const numberOfPayloads = MAX_PAYLOAD_HISTORY - 1; + + for (let i = 1; i <= numberOfPayloads; i++) { + store.set(buildPayload({blockNumber: i}), true); + } + + expect(store["payloads"].size).to.equal(numberOfPayloads); + + store.prune(); + + expect(store["payloads"].size).to.equal(numberOfPayloads); + }); + + it("should prune finalized roots", () => { + const numberOfPayloads = MAX_PAYLOAD_HISTORY + 2; + + for (let i = 1; i <= numberOfPayloads; i++) { + store.set(buildPayload({blockNumber: i}), true); + } + + expect(store["finalizedRoots"].size).to.equal(numberOfPayloads); + + store.prune(); + + expect(store["finalizedRoots"].size).to.equal(MAX_PAYLOAD_HISTORY); + }); + + it("should prune unfinalized roots", async () => { + const numberOfPayloads = MAX_PAYLOAD_HISTORY + 2; + + for (let i = 1; i <= numberOfPayloads; i++) { + (api.beacon.getBlockV2 as sinon.SinonStub) + .withArgs(i) + .resolves(buildBlockResponse({blockNumber: 500 + i, slot: i})); + + await store.processLCHeader(buildLCHeader({blockNumber: 500 + i, slot: i}), false); + } + + // Because all payloads are unfinalized, they are not pruned + expect(store["unfinalizedRoots"].size).to.equal(numberOfPayloads); + + // Let make some payloads finalized + await store.processLCHeader(buildLCHeader({blockNumber: 500 + 1, slot: 1}), true); + await store.processLCHeader(buildLCHeader({blockNumber: 500 + 2, slot: 2}), true); + + // store.processLCHeader will call the prune method internally and clean the unfinalized roots + expect(store["unfinalizedRoots"].size).to.equal(numberOfPayloads - 2); + }); + }); +});