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.

Sign up for GitHub

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

Jump to bottom

feat: send and use validators fee recipient for engine block production #5831

Merged
merged 6 commits into from
Aug 14, 2023
Merged

feat: send and use validators fee recipient for engine block production #5831

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
303a036
feat: send and use validators fee recipient for block production
g11tech Aug 1, 2023
6d4c693
fix tests
g11tech Aug 3, 2023
b5011cf
mock tests that fee recipients are correctly passed
g11tech Aug 8, 2023
0940b6f
improve logging
g11tech Aug 8, 2023
3208300
add more info
g11tech Aug 8, 2023
3fbbc16
add more info
g11tech Aug 8, 2023
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
26 changes: 17 additions & 9 deletions packages/api/src/beacon/routes/validator.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,8 @@ export type Api = {
produceBlock(
slot: Slot,
randaoReveal: BLSSignature,
graffiti: string
graffiti: string,
feeRecipient?: string
): Promise<
ApiClientResponse<
{[HttpStatusCode.OK]: {data: allForks.BeaconBlock; blockValue: Wei}},
Expand All @@ -222,7 +223,8 @@ export type Api = {
produceBlockV2(
slot: Slot,
randaoReveal: BLSSignature,
graffiti: string
graffiti: string,
feeRecipient?: string
): Promise<
ApiClientResponse<
{[HttpStatusCode.OK]: {data: allForks.BeaconBlock | BlockContents; version: ForkName; blockValue: Wei}},
Expand All @@ -233,7 +235,8 @@ export type Api = {
produceBlindedBlock(
slot: Slot,
randaoReveal: BLSSignature,
graffiti: string
graffiti: string,
feeRecipient?: string
): Promise<
ApiClientResponse<
{
Expand Down Expand Up @@ -428,7 +431,7 @@ export type ReqTypes = {
getProposerDuties: {params: {epoch: Epoch}};
getSyncCommitteeDuties: {params: {epoch: Epoch}; body: U64Str[]};
produceBlock: {params: {slot: number}; query: {randao_reveal: string; graffiti: string}};
produceBlockV2: {params: {slot: number}; query: {randao_reveal: string; graffiti: string}};
produceBlockV2: {params: {slot: number}; query: {randao_reveal: string; graffiti: string; fee_recipient?: string}};
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not part of the spec right? https://github.com/ethereum/beacon-APIs/blob/ea5668708f2c6717fd2934c9a0b82b6705818a2e/apis/validator/block.v2.yaml#L13 adding this here will cause our API client to break with spec compliant impls

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes you are correct 🙂 its not in spec, plan to PR there as well (for produce block v3),

but its spec compatible, in the sense that if the fee recipient is not passed, it will behave in the old spec complaint way

produceBlindedBlock: {params: {slot: number}; query: {randao_reveal: string; graffiti: string}};
produceAttestationData: {query: {slot: number; committee_index: number}};
produceSyncCommitteeContribution: {query: {slot: number; subcommittee_index: number; beacon_block_root: string}};
Expand Down Expand Up @@ -484,15 +487,20 @@ export function getReqSerializers(): ReqSerializers<Api, ReqTypes> {
{jsonCase: "eth2"}
);

const produceBlock: ReqSerializers<Api, ReqTypes>["produceBlock"] = {
writeReq: (slot, randaoReveal, graffiti) => ({
const produceBlock: ReqSerializers<Api, ReqTypes>["produceBlockV2"] = {
writeReq: (slot, randaoReveal, graffiti, feeRecipient) => ({
params: {slot},
query: {randao_reveal: toHexString(randaoReveal), graffiti: toGraffitiHex(graffiti)},
query: {randao_reveal: toHexString(randaoReveal), graffiti: toGraffitiHex(graffiti), fee_recipient: feeRecipient},
}),
parseReq: ({params, query}) => [params.slot, fromHexString(query.randao_reveal), fromGraffitiHex(query.graffiti)],
parseReq: ({params, query}) => [
params.slot,
fromHexString(query.randao_reveal),
fromGraffitiHex(query.graffiti),
query.fee_recipient,
],
schema: {
params: {slot: Schema.UintRequired},
query: {randao_reveal: Schema.StringRequired, graffiti: Schema.String},
query: {randao_reveal: Schema.StringRequired, graffiti: Schema.String, fee_recipient: Schema.String},
},
};

Expand Down
7 changes: 4 additions & 3 deletions packages/api/test/unit/beacon/testData/validator.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ const ZERO_HASH_HEX = "0x" + ZERO_HASH.toString("hex");
const randaoReveal = Buffer.alloc(96, 1);
const selectionProof = Buffer.alloc(96, 1);
const graffiti = "a".repeat(32);
const feeRecipient = "0xbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb";

export const testData: GenericServerTestCases<Api> = {
getAttesterDuties: {
Expand Down Expand Up @@ -44,15 +45,15 @@ export const testData: GenericServerTestCases<Api> = {
},
},
produceBlock: {
args: [32000, randaoReveal, graffiti],
args: [32000, randaoReveal, graffiti, feeRecipient],
res: {data: ssz.phase0.BeaconBlock.defaultValue(), blockValue: ssz.Wei.defaultValue()},
},
produceBlockV2: {
args: [32000, randaoReveal, graffiti],
args: [32000, randaoReveal, graffiti, feeRecipient],
res: {data: ssz.altair.BeaconBlock.defaultValue(), version: ForkName.altair, blockValue: ssz.Wei.defaultValue()},
},
produceBlindedBlock: {
args: [32000, randaoReveal, graffiti],
args: [32000, randaoReveal, graffiti, feeRecipient],
res: {
data: ssz.bellatrix.BlindedBeaconBlock.defaultValue(),
version: ForkName.bellatrix,
Expand Down
6 changes: 4 additions & 2 deletions packages/beacon-node/src/api/impl/validator/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,8 @@ export function getValidatorApi({
const produceBlockV2: ServerApi<routes.validator.Api>["produceBlockV2"] = async function produceBlockV2(
slot,
randaoReveal,
graffiti
graffiti,
feeRecipient
) {
const source = ProducedBlockSource.engine;
let timer;
Expand All @@ -297,6 +298,7 @@ export function getValidatorApi({
slot,
randaoReveal,
graffiti: toGraffitiBuffer(graffiti || ""),
feeRecipient,
});
metrics?.blockProductionSuccess.inc({source});
metrics?.blockProductionNumAggregated.observe({source}, block.body.attestations.length);
Expand Down Expand Up @@ -326,7 +328,7 @@ export function getValidatorApi({
randaoReveal,
graffiti
) {
const {data, version, blockValue} = await produceBlockV2(slot, randaoReveal, graffiti);
const {data, version, blockValue} = await produceBlockV2(slot, randaoReveal, graffiti, undefined);
if ((data as BlockContents).block !== undefined) {
throw Error(`Invalid block contents for produceBlock at fork=${version}`);
} else {
Expand Down
3 changes: 2 additions & 1 deletion packages/beacon-node/src/chain/chain.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -468,7 +468,7 @@ export class BeaconChain implements IBeaconChain {

async produceBlockWrapper<T extends BlockType>(
blockType: T,
{randaoReveal, graffiti, slot}: BlockAttributes
{randaoReveal, graffiti, slot, feeRecipient}: BlockAttributes
): Promise<{block: AssembledBlockType<T>; blockValue: Wei}> {
const head = this.forkChoice.getHead();
const state = await this.regen.getBlockSlotState(
Expand All @@ -485,6 +485,7 @@ export class BeaconChain implements IBeaconChain {
randaoReveal,
graffiti,
slot,
feeRecipient,
parentSlot: slot - 1,
parentBlockRoot,
proposerIndex,
Expand Down
47 changes: 41 additions & 6 deletions packages/beacon-node/src/chain/produceBlock/produceBlockBody.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,11 +49,12 @@ export type BlockAttributes = {
randaoReveal: BLSSignature;
graffiti: Bytes32;
slot: Slot;
feeRecipient?: string;
};

export enum BlockType {
Full,
Blinded,
Full = "Full",
Blinded = "Blinded",
}
export type AssembledBodyType<T extends BlockType> = T extends BlockType.Full
? allForks.BeaconBlockBody
Expand All @@ -80,6 +81,7 @@ export async function produceBlockBody<T extends BlockType>(
randaoReveal,
graffiti,
slot: blockSlot,
feeRecipient: requestedFeeRecipient,
parentSlot,
parentBlockRoot,
proposerIndex,
Expand All @@ -96,6 +98,14 @@ export async function produceBlockBody<T extends BlockType>(
// TODO: Does not guarantee that preDeneb enum goes with a preDeneb block
let blobsResult: BlobsResult;
let blockValue: Wei;
const fork = currentState.config.getForkName(blockSlot);

const logMeta: Record<string, string | number | bigint> = {
fork,
blockType,
slot: blockSlot,
};
this.logger.verbose("Producing beacon block body", logMeta);

// TODO:
// Iterate through the naive aggregation pool and ensure all the attestations from there
Expand Down Expand Up @@ -125,8 +135,6 @@ export async function produceBlockBody<T extends BlockType>(
voluntaryExits,
};

this.logger.verbose("Produced phase0 beacon block body", {slot: blockSlot, numAttestations: attestations.length});

const blockEpoch = computeEpochAtSlot(blockSlot);

if (blockEpoch >= this.config.ALTAIR_FORK_EPOCH) {
Expand All @@ -137,12 +145,25 @@ export async function produceBlockBody<T extends BlockType>(
(blockBody as altair.BeaconBlockBody).syncAggregate = syncAggregate;
}

const fork = currentState.config.getForkName(blockSlot);
Object.assign(logMeta, {
attestations: attestations.length,
deposits: deposits.length,
voluntaryExits: voluntaryExits.length,
attesterSlashings: attesterSlashings.length,
proposerSlashings: proposerSlashings.length,
});

if (isForkExecution(fork)) {
const safeBlockHash = this.forkChoice.getJustifiedBlock().executionPayloadBlockHash ?? ZERO_HASH_HEX;
const finalizedBlockHash = this.forkChoice.getFinalizedBlock().executionPayloadBlockHash ?? ZERO_HASH_HEX;
const feeRecipient = this.beaconProposerCache.getOrDefault(proposerIndex);
const feeRecipient = requestedFeeRecipient ?? this.beaconProposerCache.getOrDefault(proposerIndex);
const feeRecipientType = requestedFeeRecipient
? "requested"
: this.beaconProposerCache.get(proposerIndex)
? "cached"
: "default";

Object.assign(logMeta, {feeRecipientType, feeRecipient});

if (blockType === BlockType.Blinded) {
if (!this.executionBuilder) throw Error("Execution Builder not available");
Expand Down Expand Up @@ -182,6 +203,8 @@ export async function produceBlockBody<T extends BlockType>(
}
(blockBody as deneb.BlindedBeaconBlockBody).blobKzgCommitments = blobKzgCommitments;
blobsResult = {type: BlobsResultType.blinded};

Object.assign(logMeta, {blobs: blobKzgCommitments.length});
} else {
blobsResult = {type: BlobsResultType.preDeneb};
}
Expand Down Expand Up @@ -212,6 +235,8 @@ export async function produceBlockBody<T extends BlockType>(
blockValue = BigInt(0);
} else {
const {prepType, payloadId} = prepareRes;
Object.assign(logMeta, {executionPayloadPrepType: prepType});

if (prepType !== PayloadPreparationType.Cached) {
// Wait for 500ms to allow EL to add some txs to the payload
// the pitfalls of this have been put forward here, but 500ms delay for block proposal
Expand All @@ -225,6 +250,7 @@ export async function produceBlockBody<T extends BlockType>(
const {executionPayload, blobsBundle} = engineRes;
(blockBody as allForks.ExecutionBlockBody).executionPayload = executionPayload;
blockValue = engineRes.blockValue;
Object.assign(logMeta, {transactions: executionPayload.transactions.length});

const fetchedTime = Date.now() / 1000 - computeTimeAtSlot(this.config, blockSlot, this.genesisTime);
this.metrics?.blockPayload.payloadFetchedTime.observe({prepType}, fetchedTime);
Expand Down Expand Up @@ -265,6 +291,8 @@ export async function produceBlockBody<T extends BlockType>(
return blobSidecar;
}) as deneb.BlobSidecars;
blobsResult = {type: BlobsResultType.produced, blobSidecars, blockHash};

Object.assign(logMeta, {blobs: blobSidecars.length});
} else {
blobsResult = {type: BlobsResultType.preDeneb};
}
Expand Down Expand Up @@ -299,8 +327,15 @@ export async function produceBlockBody<T extends BlockType>(
if (ForkSeq[fork] >= ForkSeq.capella) {
// TODO: blsToExecutionChanges should be passed in the produceBlock call
(blockBody as capella.BeaconBlockBody).blsToExecutionChanges = blsToExecutionChanges;
Object.assign(logMeta, {
blsToExecutionChanges: blsToExecutionChanges.length,
withdrawals: (blockBody as capella.BeaconBlockBody).executionPayload.withdrawals.length,
});
}

Object.assign(logMeta, {blockValue});
this.logger.verbose("Produced beacon block body", logMeta);

return {body: blockBody as AssembledBodyType<T>, blobs: blobsResult, blockValue};
}

Expand Down
Loading
Loading