Skip to content
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import {ChainForkConfig} from "@lodestar/config";
import {CheckpointWithHex, IForkChoice, ProtoBlock} from "@lodestar/fork-choice";
import {CheckpointWithHex, IForkChoice, PayloadStatus, ProtoBlock} from "@lodestar/fork-choice";
import {computeStartSlotAtEpoch} from "@lodestar/state-transition";
import {RootHex} from "@lodestar/types";
import {Logger} from "@lodestar/utils";
Expand Down Expand Up @@ -136,18 +136,30 @@ export class SeenPayloadEnvelopeInput {
}

pruneBelowParent(parentBlock: ProtoBlock): void {
for (const block of this.forkChoice.getAllAncestorBlocks(parentBlock.blockRoot, parentBlock.payloadStatus)) {
if (block.slot < parentBlock.slot) {
const input = this.payloadInputs.get(block.blockRoot);
if (input) {
this.evictPayloadInput(input);
this.logger?.verbose("SeenPayloadEnvelopeInput.pruneBelowParent deleted", {
slot: block.slot,
root: block.blockRoot,
});
}
let deletedCount = 0;
for (const input of this.payloadInputs.values()) {
if (
input.slot < parentBlock.slot &&
// Check if the cached FULL variant is an ancestor of the current parent block
this.forkChoice.isDescendant(
input.blockRootHex,
PayloadStatus.FULL,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe PayloadStatus.PENDING
we want to also clean the one without payload
so that the next time it loops less items

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added both FULL and PENDING conditions.

parentBlock.blockRoot,
parentBlock.payloadStatus
)
) {
this.evictPayloadInput(input);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please keep the log inside this if similar to line 144 above

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I fixed in the latest commits.

deletedCount++;
}
}

if (deletedCount > 0) {
this.logger?.debug("SeenPayloadEnvelopeInput.pruneBelowParent deleted entries", {
parentSlot: parentBlock.slot,
parentRoot: parentBlock.blockRoot,
deletedCount,
});
}
}

private evictPayloadInput(payloadInput: PayloadEnvelopeInput): void {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ describe("SeenPayloadEnvelopeInput", () => {
chainEvents = new ChainEventEmitter();
abortController = new AbortController();
forkChoice = {
getAllAncestorBlocks: vi.fn(),
isDescendant: vi.fn().mockReturnValue(false),
} as unknown as IForkChoice;
serializedCache = new SerializedCache();

Expand Down Expand Up @@ -79,7 +79,8 @@ describe("SeenPayloadEnvelopeInput", () => {
const newRootHex = addPayloadInput(2);
const parentBlock = protoBlock(newRootHex, 2);

vi.mocked(forkChoice.getAllAncestorBlocks).mockReturnValue([parentBlock, protoBlock(oldRootHex, 1)]);
// Only the older entries are ancestors
vi.mocked(forkChoice.isDescendant).mockImplementation((ancestorRoot) => ancestorRoot === oldRootHex);
cache.pruneBelowParent(parentBlock);

expect(cache.get(oldRootHex)).toBeUndefined();
Expand All @@ -90,12 +91,24 @@ describe("SeenPayloadEnvelopeInput", () => {
const rootHex = addPayloadInput(1);
const parentBlock = protoBlock(rootHex, 1);

vi.mocked(forkChoice.getAllAncestorBlocks).mockReturnValue([parentBlock]);
vi.mocked(forkChoice.isDescendant).mockReturnValue(true);
cache.pruneBelowParent(parentBlock);

expect(cache.get(rootHex)).toBeDefined();
});

it("pruneBelowParent leaves non-ancestor entries on forks alone", () => {
const forkRootHex = addPayloadInput(1);
const headRootHex = addPayloadInput(2);
const parentBlock = protoBlock(headRootHex, 2);

vi.mocked(forkChoice.isDescendant).mockReturnValue(false);
cache.pruneBelowParent(parentBlock);

expect(cache.get(forkRootHex)).toBeDefined();
expect(cache.get(headRootHex)).toBeDefined();
});

it("add returns the existing entry on duplicate root", () => {
const {block, rootHex} = generateBlock({forkName: ForkName.gloas, slot: 1});
const props = {
Expand Down