Skip to content

Commit

Permalink
Review eip-4844 sync code (#5053)
Browse files Browse the repository at this point in the history
* Compute emptyKzgAggregatedProof once lazily

* Fetch blocks and blobs in concurrently

* Remove BlockInput case postEIP4844OldBlobs

* Review beaconBlocksMaybeBlobsByRoot

* Rename beaconBlocksMaybeBlobsByRange

* Ensure beaconBlocksMaybeBlobsByRange is same epoch

* rebase fixes

* type fix

* blobs by range edge condition handling

* fix test

* fix breaking spec

---------

Co-authored-by: harkamal <[email protected]>
  • Loading branch information
dapplion and g11tech authored Feb 19, 2023
1 parent 3857acc commit f75d109
Show file tree
Hide file tree
Showing 10 changed files with 169 additions and 110 deletions.
1 change: 1 addition & 0 deletions packages/beacon-node/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@
"@multiformats/multiaddr": "^11.0.0",
"@types/datastore-level": "^3.0.0",
"buffer-xor": "^2.0.2",
"c-kzg": "^1.0.9",
"cross-fetch": "^3.1.4",
"datastore-core": "^8.0.1",
"datastore-level": "^9.0.1",
Expand Down
14 changes: 1 addition & 13 deletions packages/beacon-node/src/chain/blocks/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,11 @@ import {IChainForkConfig} from "@lodestar/config";
export enum BlockInputType {
preDeneb = "preDeneb",
postDeneb = "postDeneb",
postDenebOldBlobs = "postDenebOldBlobs",
}

export type BlockInput =
| {type: BlockInputType.preDeneb; block: allForks.SignedBeaconBlock}
| {type: BlockInputType.postDeneb; block: allForks.SignedBeaconBlock; blobs: deneb.BlobsSidecar}
| {type: BlockInputType.postDenebOldBlobs; block: allForks.SignedBeaconBlock};
| {type: BlockInputType.postDeneb; block: allForks.SignedBeaconBlock; blobs: deneb.BlobsSidecar};

export function blockRequiresBlobs(config: IChainForkConfig, blockSlot: Slot, clockSlot: Slot): boolean {
return (
Expand Down Expand Up @@ -44,16 +42,6 @@ export const getBlockInput = {
blobs,
};
},

postDenebOldBlobs(config: IChainForkConfig, block: allForks.SignedBeaconBlock): BlockInput {
if (config.getForkSeq(block.message.slot) < ForkSeq.deneb) {
throw Error(`Pre Deneb block slot ${block.message.slot}`);
}
return {
type: BlockInputType.postDenebOldBlobs,
block,
};
},
};

export enum AttestationImportOpt {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -128,9 +128,5 @@ function maybeValidateBlobs(

case BlockInputType.preDeneb:
return DataAvailableStatus.preDeneb;

// TODO: Ok to assume old data available?
case BlockInputType.postDenebOldBlobs:
return DataAvailableStatus.available;
}
}
79 changes: 12 additions & 67 deletions packages/beacon-node/src/network/network.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,12 @@ import {deneb, Epoch, phase0, allForks} from "@lodestar/types";
import {routes} from "@lodestar/api";
import {IMetrics} from "../metrics/index.js";
import {ChainEvent, IBeaconChain, IBeaconClock} from "../chain/index.js";
import {BlockInput, BlockInputType, getBlockInput} from "../chain/blocks/types.js";
import {BlockInput, BlockInputType} from "../chain/blocks/types.js";
import {isValidBlsToExecutionChangeForBlockInclusion} from "../chain/opPools/utils.js";
import {INetworkOptions} from "./options.js";
import {INetwork, Libp2p} from "./interface.js";
import {ReqRespBeaconNode, ReqRespHandlers, doBeaconBlocksMaybeBlobsByRange} from "./reqresp/index.js";
import {ReqRespBeaconNode, ReqRespHandlers, beaconBlocksMaybeBlobsByRange} from "./reqresp/index.js";
import {beaconBlocksMaybeBlobsByRoot} from "./reqresp/beaconBlocksMaybeBlobsByRoot.js";
import {
Eth2Gossipsub,
getGossipHandlers,
Expand Down Expand Up @@ -328,81 +329,25 @@ export class Network implements INetwork {
beaconBlock: blockInput.block as deneb.SignedBeaconBlock,
blobsSidecar: blockInput.blobs,
});

case BlockInputType.postDenebOldBlobs:
throw Error(`Attempting to broadcast old BlockInput slot ${blockInput.block.message.slot}`);
}
}

async beaconBlocksMaybeBlobsByRange(
peerId: PeerId,
request: phase0.BeaconBlocksByRangeRequest
): Promise<BlockInput[]> {
return doBeaconBlocksMaybeBlobsByRange(this.config, this.reqResp, peerId, request, this.clock.currentEpoch);
return beaconBlocksMaybeBlobsByRange(this.config, this.reqResp, peerId, request, this.clock.currentEpoch);
}

async beaconBlocksMaybeBlobsByRoot(peerId: PeerId, request: phase0.BeaconBlocksByRootRequest): Promise<BlockInput[]> {
// Assume all requests are post Deneb
if (this.config.getForkSeq(this.chain.forkChoice.getFinalizedBlock().slot) >= ForkSeq.deneb) {
const blocksAndBlobs = await this.reqResp.beaconBlockAndBlobsSidecarByRoot(peerId, request);
return blocksAndBlobs.map(({beaconBlock, blobsSidecar}) =>
getBlockInput.postDeneb(this.config, beaconBlock, blobsSidecar)
);
}

// Assume all request are pre Deneb
else if (this.config.getForkSeq(this.clock.currentSlot) < ForkSeq.deneb) {
const blocks = await this.reqResp.beaconBlocksByRoot(peerId, request);
return blocks.map((block) => getBlockInput.preDeneb(this.config, block));
}

// NOTE: Consider blocks may be post or pre Deneb
// TODO Deneb: Request either blocks, or blocks+blobs
else {
const results = await Promise.all(
request.map(
async (beaconBlockRoot): Promise<BlockInput | null> => {
const [resultBlockBlobs, resultBlocks] = await Promise.allSettled([
this.reqResp.beaconBlockAndBlobsSidecarByRoot(peerId, [beaconBlockRoot]),
this.reqResp.beaconBlocksByRoot(peerId, [beaconBlockRoot]),
]);

if (resultBlockBlobs.status === "fulfilled" && resultBlockBlobs.value.length === 1) {
const {beaconBlock, blobsSidecar} = resultBlockBlobs.value[0];
return getBlockInput.postDeneb(this.config, beaconBlock, blobsSidecar);
}

if (resultBlocks.status === "rejected") {
return Promise.reject(resultBlocks.reason);
}

// Promise fullfilled + no result = block not found
if (resultBlocks.value.length < 1) {
return null;
}

const block = resultBlocks.value[0];

if (this.config.getForkSeq(block.message.slot) >= ForkSeq.deneb) {
// beaconBlockAndBlobsSidecarByRoot should have succeeded
if (resultBlockBlobs.status === "rejected") {
// Recycle existing error for beaconBlockAndBlobsSidecarByRoot if any
return Promise.reject(resultBlockBlobs.reason);
} else {
throw Error(
`Received post Deneb ${beaconBlockRoot} over beaconBlocksByRoot not beaconBlockAndBlobsSidecarByRoot`
);
}
}

// Block is pre Deneb
return getBlockInput.preDeneb(this.config, block);
}
)
);

return results.filter((blockOrNull): blockOrNull is BlockInput => blockOrNull !== null);
}
return beaconBlocksMaybeBlobsByRoot(
this.config,
this.reqResp,
peerId,
request,
this.clock.currentSlot,
this.chain.forkChoice.getFinalizedBlock().slot
);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,35 +5,49 @@ import {ForkSeq} from "@lodestar/params";
import {computeEpochAtSlot} from "@lodestar/state-transition";

import {BlockInput, getBlockInput} from "../../chain/blocks/types.js";
import {ckzg} from "../../util/kzg.js";
import {getEmptyBlobsSidecar} from "../../util/blobs.js";
import {IReqRespBeaconNode} from "./interface.js";

export async function doBeaconBlocksMaybeBlobsByRange(
export async function beaconBlocksMaybeBlobsByRange(
config: IBeaconConfig,
reqResp: IReqRespBeaconNode,
peerId: PeerId,
request: phase0.BeaconBlocksByRangeRequest,
currentEpoch: Epoch
): Promise<BlockInput[]> {
// TODO Deneb: Assumes all blocks in the same epoch
// TODO Deneb: Ensure all blocks are in the same epoch
if (config.getForkSeq(request.startSlot) < ForkSeq.deneb) {
// Code below assumes the request is in the same epoch
// Range sync satisfies this condition, but double check here for sanity
const {startSlot, count} = request;
if (count < 1) {
return [];
}
const endSlot = startSlot + count - 1;

const startEpoch = computeEpochAtSlot(startSlot);
const endEpoch = computeEpochAtSlot(endSlot);
if (startEpoch !== endEpoch) {
throw Error(
`BeaconBlocksByRangeRequest must be in the same epoch startEpoch=${startEpoch} != endEpoch=${endEpoch}`
);
}

// Note: Assumes all blocks in the same epoch
if (config.getForkSeq(startSlot) < ForkSeq.deneb) {
const blocks = await reqResp.beaconBlocksByRange(peerId, request);
return blocks.map((block) => getBlockInput.preDeneb(config, block));
}

// Only request blobs if they are recent enough
else if (computeEpochAtSlot(request.startSlot) >= currentEpoch - config.MIN_EPOCHS_FOR_BLOBS_SIDECARS_REQUESTS) {
// TODO Deneb: Do two requests at once for blocks and blobs
const blocks = await reqResp.beaconBlocksByRange(peerId, request);
const blobsSidecars = await reqResp.blobsSidecarsByRange(peerId, request);
else if (computeEpochAtSlot(startSlot) >= currentEpoch - config.MIN_EPOCHS_FOR_BLOBS_SIDECARS_REQUESTS) {
const [blocks, blobsSidecars] = await Promise.all([
reqResp.beaconBlocksByRange(peerId, request),
reqResp.blobsSidecarsByRange(peerId, request),
]);

const blockInputs: BlockInput[] = [];
let blobSideCarIndex = 0;
let lastMatchedSlot = -1;

const emptyKzgAggregatedProof = ckzg.computeAggregateKzgProof([]);

// Match blobSideCar with the block as some blocks would have no blobs and hence
// would be omitted from the response. If there are any inconsitencies in the
// response, the validations during import will reject the block and hence this
Expand All @@ -56,19 +70,18 @@ export async function doBeaconBlocksMaybeBlobsByRange(
`Missing blobsSidecar for blockSlot=${block.message.slot} with blobKzgCommitmentsLen=${blobKzgCommitmentsLen}`
);
}
blobsSidecar = {
beaconBlockRoot: config.getForkTypes(block.message.slot).BeaconBlock.hashTreeRoot(block.message),
beaconBlockSlot: block.message.slot,
blobs: [],
kzgAggregatedProof: emptyKzgAggregatedProof,
};
blobsSidecar = getEmptyBlobsSidecar(config, block as deneb.SignedBeaconBlock);
}
blockInputs.push(getBlockInput.postDeneb(config, block, blobsSidecar));
}

// If there are still unconsumed blobs this means that the response was inconsistent
// and matching was wrong and hence we should throw error
if (blobsSidecars[blobSideCarIndex] !== undefined) {
if (
blobsSidecars[blobSideCarIndex] !== undefined &&
// If there are no blobs, the blobs request can give 1 block outside the requested range
blobsSidecars[blobSideCarIndex].beaconBlockSlot <= endSlot
) {
throw Error(
`Unmatched blobsSidecars, blocks=${blocks.length}, blobs=${
blobsSidecars.length
Expand All @@ -82,7 +95,6 @@ export async function doBeaconBlocksMaybeBlobsByRange(

// Post Deneb but old blobs
else {
const blocks = await reqResp.beaconBlocksByRange(peerId, request);
return blocks.map((block) => getBlockInput.postDenebOldBlobs(config, block));
throw Error("Cannot sync blobs outside of blobs prune window");
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
import {PeerId} from "@libp2p/interface-peer-id";
import {IBeaconConfig} from "@lodestar/config";
import {RequestError, RequestErrorCode} from "@lodestar/reqresp";
import {Epoch, phase0, Root, Slot} from "@lodestar/types";
import {toHex} from "@lodestar/utils";
import {ForkSeq} from "@lodestar/params";
import {BlockInput, getBlockInput} from "../../chain/blocks/types.js";
import {wrapError} from "../../util/wrapError.js";
import {IReqRespBeaconNode} from "./interface.js";

export async function beaconBlocksMaybeBlobsByRoot(
config: IBeaconConfig,
reqResp: IReqRespBeaconNode,
peerId: PeerId,
request: phase0.BeaconBlocksByRootRequest,
currentSlot: Epoch,
finalizedSlot: Slot
): Promise<BlockInput[]> {
// Assume all requests are post Deneb
if (config.getForkSeq(finalizedSlot) >= ForkSeq.deneb) {
const blocksAndBlobs = await reqResp.beaconBlockAndBlobsSidecarByRoot(peerId, request);
return blocksAndBlobs.map(({beaconBlock, blobsSidecar}) =>
getBlockInput.postDeneb(config, beaconBlock, blobsSidecar)
);
}

// Assume all request are pre EIP-4844
else if (config.getForkSeq(currentSlot) < ForkSeq.deneb) {
const blocks = await reqResp.beaconBlocksByRoot(peerId, request);
return blocks.map((block) => getBlockInput.preDeneb(config, block));
}

// We don't know if a requested root is after the deneb fork or not.
// Thus some sort of retry is necessary while deneb is not finalized
else {
return Promise.all(
request.map(async (beaconBlockRoot) =>
beaconBlockAndBlobsSidecarByRootFallback(config, reqResp, peerId, beaconBlockRoot)
)
);
}
}

async function beaconBlockAndBlobsSidecarByRootFallback(
config: IBeaconConfig,
reqResp: IReqRespBeaconNode,
peerId: PeerId,
beaconBlockRoot: Root
): Promise<BlockInput> {
const resBlockBlobs = await wrapError(reqResp.beaconBlockAndBlobsSidecarByRoot(peerId, [beaconBlockRoot]));

if (resBlockBlobs.err) {
// From the spec, if the block is from before the fork, errors with 3: ResourceUnavailable
// > Clients MUST support requesting blocks and sidecars since minimum_request_epoch, where
// minimum_request_epoch = max(finalized_epoch, current_epoch - MIN_EPOCHS_FOR_BLOBS_SIDECARS_REQUESTS, EIP4844_FORK_EPOCH).
// If any root in the request content references a block earlier than minimum_request_epoch,
// peers SHOULD respond with error code 3: ResourceUnavailable.
// Ref: https://github.com/ethereum/consensus-specs/blob/aede132f4999ed54b98d35e27aca9451042a1ee9/specs/eip4844/p2p-interface.md#beaconblockandblobssidecarbyroot-v1
if (
resBlockBlobs.err instanceof RequestError &&
resBlockBlobs.err.type.code === RequestErrorCode.RESOURCE_UNAVAILABLE
) {
// retry with blocks
} else {
// Unexpected error, throw
throw resBlockBlobs.err;
}
} else {
if (resBlockBlobs.result.length < 1) {
throw Error(`beaconBlockAndBlobsSidecarByRoot return empty for block root ${toHex(beaconBlockRoot)}`);
}

const {beaconBlock, blobsSidecar} = resBlockBlobs.result[0];
return getBlockInput.postDeneb(config, beaconBlock, blobsSidecar);
}

const resBlocks = await reqResp.beaconBlocksByRoot(peerId, [beaconBlockRoot]);
if (resBlocks.length < 1) {
throw Error(`beaconBlocksByRoot return empty for block root ${toHex(beaconBlockRoot)}`);
}

return getBlockInput.preDeneb(config, resBlocks[0]);
}
2 changes: 1 addition & 1 deletion packages/beacon-node/src/network/reqresp/index.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
export * from "./ReqRespBeaconNode.js";
export * from "./interface.js";
export * from "./doBeaconBlocksMaybeBlobsByRange.js";
export * from "./beaconBlocksMaybeBlobsByRange.js";
24 changes: 24 additions & 0 deletions packages/beacon-node/src/util/blobs.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import {IChainForkConfig} from "@lodestar/config";
import {deneb} from "@lodestar/types";
import {ckzg} from "./kzg.js";

// Cache empty KZG proof, compute once lazily if needed
let emptyKzgAggregatedProof: Uint8Array | null = null;
function getEmptyKzgAggregatedProof(): Uint8Array {
if (!emptyKzgAggregatedProof) {
emptyKzgAggregatedProof = ckzg.computeAggregateKzgProof([]);
}
return emptyKzgAggregatedProof;
}

/**
* Construct a valid BlobsSidecar for a SignedBeaconBlock that references 0 commitments
*/
export function getEmptyBlobsSidecar(config: IChainForkConfig, block: deneb.SignedBeaconBlock): deneb.BlobsSidecar {
return {
beaconBlockRoot: config.getForkTypes(block.message.slot).BeaconBlock.hashTreeRoot(block.message),
beaconBlockSlot: block.message.slot,
blobs: [],
kzgAggregatedProof: getEmptyKzgAggregatedProof(),
};
}
Loading

0 comments on commit f75d109

Please sign in to comment.