From f9c71074df49c4bdd1b01d8ac434719fda1a1dbf Mon Sep 17 00:00:00 2001 From: tuyennhv Date: Thu, 17 Aug 2023 08:19:21 +0700 Subject: [PATCH] feat: forkchoice to track head vote by proto index (#5882) * feat: forkchoice to track head vote by proto index * fix: pass proto array length to computeDeltas() * fix: set vote index to null for out of date vote * fix: handle no vote in prune() * chore: fastest way to create deltas array --- .../beacon-node/src/chain/archiver/index.ts | 3 +- .../fork-choice/src/forkChoice/forkChoice.ts | 47 +++++- .../src/protoArray/computeDeltas.ts | 76 ++++------ .../fork-choice/src/protoArray/interface.ts | 6 +- .../perf/protoArray/computeDeltas.test.ts | 16 +- .../unit/protoArray/computeDeltas.test.ts | 142 +++++++++--------- 6 files changed, 152 insertions(+), 138 deletions(-) diff --git a/packages/beacon-node/src/chain/archiver/index.ts b/packages/beacon-node/src/chain/archiver/index.ts index 5e9dc314a351..9c0290bfd8c4 100644 --- a/packages/beacon-node/src/chain/archiver/index.ts +++ b/packages/beacon-node/src/chain/archiver/index.ts @@ -106,12 +106,13 @@ export class Archiver { this.chain.regen.pruneOnFinalized(finalizedEpoch); // tasks rely on extended fork choice - this.chain.forkChoice.prune(finalized.rootHex); + const prunedBlocks = this.chain.forkChoice.prune(finalized.rootHex); await this.updateBackfillRange(finalized); this.logger.verbose("Finish processing finalized checkpoint", { epoch: finalizedEpoch, rootHex: finalized.rootHex, + prunedBlocks: prunedBlocks.length, }); } catch (e) { this.logger.error("Error processing finalized checkpoint", {epoch: finalized.epoch}, e as Error); diff --git a/packages/fork-choice/src/forkChoice/forkChoice.ts b/packages/fork-choice/src/forkChoice/forkChoice.ts index cba427b22c57..6dda2c639df2 100644 --- a/packages/fork-choice/src/forkChoice/forkChoice.ts +++ b/packages/fork-choice/src/forkChoice/forkChoice.ts @@ -186,7 +186,7 @@ export class ForkChoice implements IForkChoice { const oldBalances = this.balances; const newBalances = this.fcStore.justified.balances; const deltas = computeDeltas( - this.protoArray.indices, + this.protoArray.nodes.length, this.votes, oldBalances, newBalances, @@ -555,7 +555,7 @@ export class ForkChoice implements IForkChoice { } return { epoch: vote.nextEpoch, - root: vote.nextRoot, + root: vote.nextIndex === null ? HEX_ZERO_HASH : this.protoArray.nodes[vote.nextIndex].blockRoot, }; } @@ -665,8 +665,37 @@ export class ForkChoice implements IForkChoice { return this.protoArray.isDescendant(ancestorRoot, descendantRoot); } + /** + * All indices in votes are relative to proto array so always keep it up to date + */ prune(finalizedRoot: RootHex): ProtoBlock[] { - return this.protoArray.maybePrune(finalizedRoot); + const prunedNodes = this.protoArray.maybePrune(finalizedRoot); + const prunedCount = prunedNodes.length; + for (const vote of this.votes) { + // validator has never voted + if (vote === undefined) { + continue; + } + + if (vote.currentIndex !== null) { + if (vote.currentIndex >= prunedCount) { + vote.currentIndex -= prunedCount; + } else { + // the vote was for a pruned proto node + vote.currentIndex = null; + } + } + + if (vote.nextIndex !== null) { + if (vote.nextIndex >= prunedCount) { + vote.nextIndex -= prunedCount; + } else { + // the vote was for a pruned proto node + vote.nextIndex = null; + } + } + } + return prunedNodes; } setPruneThreshold(threshold: number): void { @@ -1091,14 +1120,20 @@ export class ForkChoice implements IForkChoice { */ private addLatestMessage(validatorIndex: ValidatorIndex, nextEpoch: Epoch, nextRoot: RootHex): void { const vote = this.votes[validatorIndex]; + // should not happen, attestation is validated before this step + const nextIndex = this.protoArray.indices.get(nextRoot); + if (nextIndex === undefined) { + throw new Error(`Could not find proto index for nextRoot ${nextRoot}`); + } + if (vote === undefined) { this.votes[validatorIndex] = { - currentRoot: HEX_ZERO_HASH, - nextRoot, + currentIndex: null, + nextIndex, nextEpoch, }; } else if (nextEpoch > vote.nextEpoch) { - vote.nextRoot = nextRoot; + vote.nextIndex = nextIndex; vote.nextEpoch = nextEpoch; } // else its an old vote, don't count it diff --git a/packages/fork-choice/src/protoArray/computeDeltas.ts b/packages/fork-choice/src/protoArray/computeDeltas.ts index b90219b680ef..8301c1e77a75 100644 --- a/packages/fork-choice/src/protoArray/computeDeltas.ts +++ b/packages/fork-choice/src/protoArray/computeDeltas.ts @@ -1,6 +1,6 @@ import {ValidatorIndex} from "@lodestar/types"; import {EffectiveBalanceIncrements} from "@lodestar/state-transition"; -import {VoteTracker, HEX_ZERO_HASH} from "./interface.js"; +import {VoteTracker} from "./interface.js"; import {ProtoArrayError, ProtoArrayErrorCode} from "./errors.js"; /** @@ -13,31 +13,20 @@ import {ProtoArrayError, ProtoArrayErrorCode} from "./errors.js"; * - If a value in `indices` is greater to or equal to `indices.length`. */ export function computeDeltas( - indices: Map, + numProtoNodes: number, votes: VoteTracker[], oldBalances: EffectiveBalanceIncrements, newBalances: EffectiveBalanceIncrements, equivocatingIndices: Set ): number[] { - const deltas = Array(indices.size).fill(0); - const zeroHash = HEX_ZERO_HASH; + const deltas = new Array(numProtoNodes); + for (let i = 0; i < numProtoNodes; i++) { + deltas[i] = 0; + } + // avoid creating new variables in the loop to potentially reduce GC pressure let oldBalance, newBalance: number; - let currentRoot, nextRoot: string; - let currentDeltaIndex, nextDeltaIndex: number | undefined; - // this function tends to get some very few roots from `indices` so create a small cache to improve performance - const cachedIndices = new Map(); - - const getIndex = (root: string): number | undefined => { - let index = cachedIndices.get(root); - if (index === undefined) { - index = indices.get(root); - if (index !== undefined) { - cachedIndices.set(root, index); - } - } - return index; - }; + let currentIndex, nextIndex: number | null; for (let vIndex = 0; vIndex < votes.length; vIndex++) { const vote = votes[vIndex]; @@ -46,11 +35,8 @@ export function computeDeltas( if (vote === undefined) { continue; } - currentRoot = vote.currentRoot; - nextRoot = vote.nextRoot; - if (currentRoot === zeroHash && nextRoot === zeroHash) { - continue; - } + currentIndex = vote.currentIndex; + nextIndex = vote.nextIndex; // IF the validator was not included in the _old_ balances (i.e. it did not exist yet) // then say its balance was 0 @@ -65,49 +51,45 @@ export function computeDeltas( if (equivocatingIndices.size > 0 && equivocatingIndices.has(vIndex)) { // this function could be called multiple times but we only want to process slashing validator for 1 time - if (currentRoot !== zeroHash) { - currentDeltaIndex = getIndex(currentRoot); - if (currentDeltaIndex !== undefined) { - if (currentDeltaIndex >= deltas.length) { - throw new ProtoArrayError({ - code: ProtoArrayErrorCode.INVALID_NODE_DELTA, - index: currentDeltaIndex, - }); - } - deltas[currentDeltaIndex] -= oldBalance; + if (currentIndex !== null) { + if (currentIndex >= numProtoNodes) { + throw new ProtoArrayError({ + code: ProtoArrayErrorCode.INVALID_NODE_DELTA, + index: currentIndex, + }); } + deltas[currentIndex] -= oldBalance; } - vote.currentRoot = zeroHash; + vote.currentIndex = null; continue; } - if (currentRoot !== nextRoot || oldBalance !== newBalance) { + if (currentIndex !== nextIndex || oldBalance !== newBalance) { // We ignore the vote if it is not known in `indices . // We assume that it is outside of our tree (ie: pre-finalization) and therefore not interesting - currentDeltaIndex = getIndex(currentRoot); - if (currentDeltaIndex !== undefined) { - if (currentDeltaIndex >= deltas.length) { + if (currentIndex !== null) { + if (currentIndex >= numProtoNodes) { throw new ProtoArrayError({ code: ProtoArrayErrorCode.INVALID_NODE_DELTA, - index: currentDeltaIndex, + index: currentIndex, }); } - deltas[currentDeltaIndex] -= oldBalance; + deltas[currentIndex] -= oldBalance; } + // We ignore the vote if it is not known in `indices . // We assume that it is outside of our tree (ie: pre-finalization) and therefore not interesting - nextDeltaIndex = getIndex(nextRoot); - if (nextDeltaIndex !== undefined) { - if (nextDeltaIndex >= deltas.length) { + if (nextIndex !== null) { + if (nextIndex >= numProtoNodes) { throw new ProtoArrayError({ code: ProtoArrayErrorCode.INVALID_NODE_DELTA, - index: nextDeltaIndex, + index: nextIndex, }); } - deltas[nextDeltaIndex] += newBalance; + deltas[nextIndex] += newBalance; } } - vote.currentRoot = nextRoot; + vote.currentIndex = nextIndex; } return deltas; diff --git a/packages/fork-choice/src/protoArray/interface.ts b/packages/fork-choice/src/protoArray/interface.ts index 87a37a7b6ff7..1910ad2b4206 100644 --- a/packages/fork-choice/src/protoArray/interface.ts +++ b/packages/fork-choice/src/protoArray/interface.ts @@ -6,10 +6,12 @@ export const HEX_ZERO_HASH = "0x000000000000000000000000000000000000000000000000 /** * Simplified 'latest message' with previous message + * The index is relative to ProtoArray indices */ export type VoteTracker = { - currentRoot: RootHex; - nextRoot: RootHex; + currentIndex: number | null; + // if a vode is out of date (the voted index was in the past while proto array is pruned), it will be set to null + nextIndex: number | null; nextEpoch: Epoch; }; diff --git a/packages/fork-choice/test/perf/protoArray/computeDeltas.test.ts b/packages/fork-choice/test/perf/protoArray/computeDeltas.test.ts index 44df7e481b9f..1843ef91c22a 100644 --- a/packages/fork-choice/test/perf/protoArray/computeDeltas.test.ts +++ b/packages/fork-choice/test/perf/protoArray/computeDeltas.test.ts @@ -1,6 +1,4 @@ -import crypto from "node:crypto"; import {itBench, setBenchOpts} from "@dapplion/benchmark"; -import {toHexString} from "@chainsafe/ssz"; import {EffectiveBalanceIncrements, getEffectiveBalanceIncrementsZeroed} from "@lodestar/state-transition"; import {VoteTracker} from "../../../src/protoArray/interface.js"; import {computeDeltas} from "../../../src/protoArray/computeDeltas.js"; @@ -10,8 +8,6 @@ describe("computeDeltas", () => { let oldBalances: EffectiveBalanceIncrements; let newBalances: EffectiveBalanceIncrements; - const oldRoot = "0x32dec344944029ba183ac387a7aa1f2068591c00e9bfadcfb238e50fbe9ea38e"; - const newRoot = "0xb59f3a209f639dd6b5645ea9fad8d441df44c3be93bd1bbf50ef90bf124d1238"; const oneHourProtoNodes = (60 * 60) / 12; const fourHourProtoNodes = 4 * oneHourProtoNodes; const oneDayProtoNodes = 24 * oneHourProtoNodes; @@ -35,12 +31,6 @@ describe("computeDeltas", () => { }); for (const numProtoNode of [oneHourProtoNodes, fourHourProtoNodes, oneDayProtoNodes]) { - const indices: Map = new Map(); - for (let i = 0; i < numProtoNode; i++) { - indices.set(toHexString(crypto.randomBytes(32)), i); - } - indices.set(oldRoot, Math.floor(numProtoNode / 2)); - indices.set(newRoot, Math.floor(numProtoNode / 2) + 1); itBench({ id: `computeDeltas ${numValidator} validators ${numProtoNode} proto nodes`, beforeEach: () => { @@ -48,15 +38,15 @@ describe("computeDeltas", () => { const epoch = 100_000; for (let i = 0; i < numValidator; i++) { votes.push({ - currentRoot: oldRoot, - nextRoot: newRoot, + currentIndex: Math.floor(numProtoNode / 2), + nextIndex: Math.floor(numProtoNode / 2) + 1, nextEpoch: epoch, }); } return votes; }, fn: (votes) => { - computeDeltas(indices, votes, oldBalances, newBalances, new Set()); + computeDeltas(numProtoNode, votes, oldBalances, newBalances, new Set()); }, }); } diff --git a/packages/fork-choice/test/unit/protoArray/computeDeltas.test.ts b/packages/fork-choice/test/unit/protoArray/computeDeltas.test.ts index 3bf0e13176f6..3981ef84ff4c 100644 --- a/packages/fork-choice/test/unit/protoArray/computeDeltas.test.ts +++ b/packages/fork-choice/test/unit/protoArray/computeDeltas.test.ts @@ -15,21 +15,21 @@ describe("computeDeltas", () => { for (const i of Array.from({length: validatorCount}, (_, i) => i)) { indices.set(i.toString(), i); votes.push({ - currentRoot: "0", - nextRoot: "0", + currentIndex: 0, + nextIndex: 0, nextEpoch: 0, }); oldBalances[i] = 0; newBalances[i] = 0; } - const deltas = computeDeltas(indices, votes, oldBalances, newBalances, new Set()); + const deltas = computeDeltas(indices.size, votes, oldBalances, newBalances, new Set()); expect(deltas.length).to.eql(validatorCount); expect(deltas).to.deep.equal(Array.from({length: validatorCount}, () => 0)); for (const vote of votes) { - expect(vote.currentRoot).to.eql(vote.nextRoot); + expect(vote.currentIndex).to.eql(vote.nextIndex); } }); @@ -45,15 +45,15 @@ describe("computeDeltas", () => { for (const i of Array.from({length: validatorCount}, (_, i) => i)) { indices.set((i + 1).toString(), i); votes.push({ - currentRoot: "0", - nextRoot: "1", + currentIndex: null, + nextIndex: 0, nextEpoch: 0, }); oldBalances[i] = balance; newBalances[i] = balance; } - const deltas = computeDeltas(indices, votes, oldBalances, newBalances, new Set()); + const deltas = computeDeltas(indices.size, votes, oldBalances, newBalances, new Set()); expect(deltas.length).to.eql(validatorCount); @@ -78,15 +78,15 @@ describe("computeDeltas", () => { for (const i of Array.from({length: validatorCount}, (_, i) => i)) { indices.set((i + 1).toString(), i); votes.push({ - currentRoot: "0", - nextRoot: (i + 1).toString(), + currentIndex: null, + nextIndex: i, nextEpoch: 0, }); oldBalances[i] = balance; newBalances[i] = balance; } - const deltas = computeDeltas(indices, votes, oldBalances, newBalances, new Set()); + const deltas = computeDeltas(indices.size, votes, oldBalances, newBalances, new Set()); expect(deltas.length).to.eql(validatorCount); @@ -107,15 +107,15 @@ describe("computeDeltas", () => { for (const i of Array.from({length: validatorCount}, (_, i) => i)) { indices.set((i + 1).toString(), i); votes.push({ - currentRoot: "1", - nextRoot: "2", + currentIndex: 0, + nextIndex: 1, nextEpoch: 0, }); oldBalances[i] = balance; newBalances[i] = balance; } - const deltas = computeDeltas(indices, votes, oldBalances, newBalances, new Set()); + const deltas = computeDeltas(indices.size, votes, oldBalances, newBalances, new Set()); expect(deltas.length).to.eql(validatorCount); @@ -132,47 +132,51 @@ describe("computeDeltas", () => { } }); - it("move out of tree", () => { - const balance = 42; - - const indices = new Map(); - // there is only one block - indices.set("2", 0); - - // There are two validators - const votes = [ - // one validator moves their vote from the block to the zero hash - { - currentRoot: "2", - nextRoot: "0", - nextEpoch: 0, - }, - // one validator moves their vote from the block to something outside the tree - { - currentRoot: "2", - nextRoot: "1337", - nextEpoch: 0, - }, - ]; - - const oldBalances = getEffectiveBalanceIncrementsZeroed(votes.length); - const newBalances = getEffectiveBalanceIncrementsZeroed(votes.length); - for (const balances of [oldBalances, newBalances]) { - for (let i = 0; i < votes.length; i++) { - balances[i] = balance; - } - } - - const deltas = computeDeltas(indices, votes, oldBalances, newBalances, new Set()); - - expect(deltas.length).to.eql(1); - - expect(deltas[0].toString()).to.eql((0 - balance * 2).toString()); - - for (const vote of votes) { - expect(vote.currentRoot).to.equal(vote.nextRoot); - } - }); + /** + * Starting Aug 2023, this test case is not valid because when an attestation is added + * to forkchoice, the block should come first, i.e. nextIndex should be a number + */ + // it("move out of tree", () => { + // const balance = 42; + + // const indices = new Map(); + // // there is only one block + // indices.set("2", 0); + + // // There are two validators + // const votes = [ + // // one validator moves their vote from the block to the zero hash + // { + // currentRoot: "2", + // nextRoot: "0", + // nextEpoch: 0, + // }, + // // one validator moves their vote from the block to something outside the tree + // { + // currentRoot: "2", + // nextRoot: "1337", + // nextEpoch: 0, + // }, + // ]; + + // const oldBalances = getEffectiveBalanceIncrementsZeroed(votes.length); + // const newBalances = getEffectiveBalanceIncrementsZeroed(votes.length); + // for (const balances of [oldBalances, newBalances]) { + // for (let i = 0; i < votes.length; i++) { + // balances[i] = balance; + // } + // } + + // const deltas = computeDeltas(indices, votes, oldBalances, newBalances, new Set()); + + // expect(deltas.length).to.eql(1); + + // expect(deltas[0].toString()).to.eql((0 - balance * 2).toString()); + + // for (const vote of votes) { + // expect(vote.currentRoot).to.equal(vote.nextRoot); + // } + // }); it("changing balances", () => { const oldBalance = 42; @@ -187,15 +191,15 @@ describe("computeDeltas", () => { for (const i of Array.from({length: validatorCount}, (_, i) => i)) { indices.set((i + 1).toString(), i); votes.push({ - currentRoot: "1", - nextRoot: "2", + currentIndex: 0, + nextIndex: 1, nextEpoch: 0, }); oldBalances[i] = oldBalance; newBalances[i] = newBalance; } - const deltas = computeDeltas(indices, votes, oldBalances, newBalances, new Set()); + const deltas = computeDeltas(indices.size, votes, oldBalances, newBalances, new Set()); expect(deltas.length).to.eql(validatorCount); @@ -220,8 +224,8 @@ describe("computeDeltas", () => { // Both validators move votes from block1 to block2 const votes = Array.from({length: 2}, () => ({ - currentRoot: "2", - nextRoot: "3", + currentIndex: 0, + nextIndex: 1, nextEpoch: 0, })); @@ -233,7 +237,7 @@ describe("computeDeltas", () => { newBalances[0] = balance; newBalances[1] = balance; - const deltas = computeDeltas(indices, votes, oldBalances, newBalances, new Set()); + const deltas = computeDeltas(indices.size, votes, oldBalances, newBalances, new Set()); expect(deltas.length).to.eql(2); @@ -241,7 +245,7 @@ describe("computeDeltas", () => { expect(deltas[1].toString()).to.eql((balance * 2).toString()); for (const vote of votes) { - expect(vote.currentRoot).to.equal(vote.nextRoot); + expect(vote.currentIndex).to.equal(vote.nextIndex); } }); @@ -255,8 +259,8 @@ describe("computeDeltas", () => { // Both validators move votes from block1 to block2 const votes = Array.from({length: 2}, () => ({ - currentRoot: "2", - nextRoot: "3", + currentIndex: 0, + nextIndex: 1, nextEpoch: 0, })); // There are two validators in the old balances. @@ -267,7 +271,7 @@ describe("computeDeltas", () => { const newBalances = getEffectiveBalanceIncrementsZeroed(1); newBalances[0] = balance; - const deltas = computeDeltas(indices, votes, oldBalances, newBalances, new Set()); + const deltas = computeDeltas(indices.size, votes, oldBalances, newBalances, new Set()); expect(deltas.length).to.eql(2); @@ -275,7 +279,7 @@ describe("computeDeltas", () => { expect(deltas[1].toString()).to.eql(balance.toString()); for (const vote of votes) { - expect(vote.currentRoot).to.equal(vote.nextRoot); + expect(vote.currentIndex).to.equal(vote.nextIndex); } }); @@ -290,21 +294,21 @@ describe("computeDeltas", () => { // Both validators move votes from block1 to block2 const votes = Array.from({length: 2}, () => ({ - currentRoot: "2", - nextRoot: "3", + currentIndex: 0, + nextIndex: 1, nextEpoch: 0, })); const balances = new Uint8Array([firstBalance, secondBalance]); // 1st validator is part of an attester slashing const equivocatingIndices = new Set([0]); - let deltas = computeDeltas(indices, votes, balances, balances, equivocatingIndices); + let deltas = computeDeltas(indices.size, votes, balances, balances, equivocatingIndices); expect(deltas[0]).to.be.equals( -1 * (firstBalance + secondBalance), "should disregard the 1st validator due to attester slashing" ); expect(deltas[1]).to.be.equals(secondBalance, "should move 2nd balance from 1st root to 2nd root"); - deltas = computeDeltas(indices, votes, balances, balances, equivocatingIndices); + deltas = computeDeltas(indices.size, votes, balances, balances, equivocatingIndices); expect(deltas).to.be.deep.equals([0, 0], "calling computeDeltas again should not have any affect on the weight"); }); });