Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consistent use of RootHex in notifyForkchoiceUpdate #4308

Merged
merged 1 commit into from
Jul 17, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/beacon-node/src/chain/factory/block/body.ts
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ export async function prepareExecutionPayload(
prevRandao: toHex(prevRandao),
suggestedFeeRecipient,
}) ??
(await chain.executionEngine.notifyForkchoiceUpdate(parentHash, safeBlockHash, finalizedBlockHash, {
(await chain.executionEngine.notifyForkchoiceUpdate(toHex(parentHash), safeBlockHash, finalizedBlockHash, {
timestamp,
prevRandao,
suggestedFeeRecipient,
Expand Down
12 changes: 4 additions & 8 deletions packages/beacon-node/src/execution/engine/http.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import {bellatrix, RootHex, Root} from "@lodestar/types";
import {bellatrix, RootHex} from "@lodestar/types";
import {BYTES_PER_LOGS_BLOOM} from "@lodestar/params";
import {fromHex} from "@lodestar/utils";

Expand Down Expand Up @@ -187,13 +187,12 @@ export class ExecutionEngineHttp implements IExecutionEngine {
* If any of the above fails due to errors unrelated to the normal processing flow of the method, client software MUST respond with an error object.
*/
async notifyForkchoiceUpdate(
headBlockHash: Root | RootHex,
headBlockHash: RootHex,
safeBlockHash: RootHex,
finalizedBlockHash: RootHex,
payloadAttributes?: PayloadAttributes
): Promise<PayloadId | null> {
const method = "engine_forkchoiceUpdatedV1";
const headBlockHashData = typeof headBlockHash === "string" ? headBlockHash : bytesToData(headBlockHash);
const apiPayloadAttributes: ApiPayloadAttributes | undefined = payloadAttributes
? {
timestamp: numToQuantity(payloadAttributes.timestamp),
Expand All @@ -208,7 +207,7 @@ export class ExecutionEngineHttp implements IExecutionEngine {
payloadStatus: {status, latestValidHash: _latestValidHash, validationError},
payloadId,
} = await this.rpc.fetch<EngineApiRpcReturnTypes[typeof method], EngineApiRpcParamTypes[typeof method]>(
{method, params: [{headBlockHash: headBlockHashData, safeBlockHash, finalizedBlockHash}, apiPayloadAttributes]},
{method, params: [{headBlockHash, safeBlockHash, finalizedBlockHash}, apiPayloadAttributes]},
forkchoiceUpdatedV1Opts
);

Expand All @@ -220,10 +219,7 @@ export class ExecutionEngineHttp implements IExecutionEngine {
throw Error(`Received invalid payloadId=${payloadId}`);
}

this.payloadIdCache.add(
{headBlockHash: headBlockHashData, finalizedBlockHash, ...apiPayloadAttributes},
payloadId
);
this.payloadIdCache.add({headBlockHash, finalizedBlockHash, ...apiPayloadAttributes}, payloadId);
void this.prunePayloadIdCache();
}
return payloadId !== "0x" ? payloadId : null;
Expand Down
4 changes: 2 additions & 2 deletions packages/beacon-node/src/execution/engine/interface.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import {bellatrix, Root, RootHex} from "@lodestar/types";
import {bellatrix, RootHex} from "@lodestar/types";
import {PayloadIdCache, PayloadId, ApiPayloadAttributes} from "./payloadIdCache.js";

export {PayloadIdCache, PayloadId, ApiPayloadAttributes};
Expand Down Expand Up @@ -79,7 +79,7 @@ export interface IExecutionEngine {
* Should be called in response to fork-choice head and finalized events
*/
notifyForkchoiceUpdate(
headBlockHash: Root | RootHex,
headBlockHash: RootHex,
safeBlockHash: RootHex,
finalizedBlockHash: RootHex,
payloadAttributes?: PayloadAttributes
Expand Down
15 changes: 7 additions & 8 deletions packages/beacon-node/src/execution/engine/mock.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import crypto from "node:crypto";
import {bellatrix, RootHex, Root} from "@lodestar/types";
import {bellatrix, RootHex} from "@lodestar/types";
import {toHexString, fromHexString} from "@chainsafe/ssz";
import {BYTES_PER_LOGS_BLOOM} from "@lodestar/params";
import {ZERO_HASH, ZERO_HASH_HEX} from "../../constants/index.js";
Expand Down Expand Up @@ -81,23 +81,22 @@ export class ExecutionEngineMock implements IExecutionEngine {
* 2. Client software MUST respond with 4: Unknown block error if the payload identified by either the headBlockHash or the finalizedBlockHash is unknown.
*/
async notifyForkchoiceUpdate(
headBlockHash: Root,
headBlockHash: RootHex,
safeBlockHash: RootHex,
finalizedBlockHash: RootHex,
payloadAttributes?: PayloadAttributes
): Promise<PayloadId> {
const headBlockHashHex = toHexString(headBlockHash);
if (!this.knownBlocks.has(headBlockHashHex)) {
throw Error(`Unknown headBlockHash ${headBlockHashHex}`);
if (!this.knownBlocks.has(headBlockHash)) {
throw Error(`Unknown headBlockHash ${headBlockHash}`);
}
if (!this.knownBlocks.has(finalizedBlockHash)) {
throw Error(`Unknown finalizedBlockHash ${finalizedBlockHash}`);
}

this.headBlockRoot = headBlockHashHex;
this.headBlockRoot = headBlockHash;
this.finalizedBlockRoot = finalizedBlockHash;

const parentHashHex = headBlockHashHex;
const parentHashHex = headBlockHash;
const parentPayload = this.knownBlocks.get(parentHashHex);
if (!parentPayload) {
throw Error(`Unknown parentHash ${parentHashHex}`);
Expand All @@ -107,7 +106,7 @@ export class ExecutionEngineMock implements IExecutionEngine {

const payloadId = this.payloadId++;
const payload: bellatrix.ExecutionPayload = {
parentHash: headBlockHash,
parentHash: fromHexString(headBlockHash),
feeRecipient: fromHexString(payloadAttributes.suggestedFeeRecipient),
stateRoot: crypto.randomBytes(32),
receiptsRoot: crypto.randomBytes(32),
Expand Down