feat: reject builder blocks if engine indicates censorship (ChainSafe…

…#6258)

* feat: reject builder blocks if engine indicates censorship

* apply feeback

* fix tests

* small refac and test fixes and selection test extenstion for override combinations

* make typing tighter for blindedblock

* apply feedbac

* Fix typo

---------

Co-authored-by: Nico Flaig <nflaig@protonmail.com>

packages/beacon-node/src/api/impl/validator/index.ts

@@ -353,7 +353,7 @@ export function getValidatorApi({
       strictFeeRecipientCheck,
       skipHeadChecksAndUpdate,
     }: Omit<routes.validator.ExtraProduceBlockOps, "builderSelection"> & {skipHeadChecksAndUpdate?: boolean} = {}
-  ): Promise<routes.validator.ProduceBlockOrContentsRes> {
+  ): Promise<routes.validator.ProduceBlockOrContentsRes & {shouldOverrideBuilder?: boolean}> {
     const source = ProducedBlockSource.engine;
     metrics?.blockProductionRequests.inc({source});
 
@@ -371,7 +371,7 @@ export function getValidatorApi({
     let timer;
     try {
       timer = metrics?.blockProductionTime.startTimer();
-      const {block, executionPayloadValue, consensusBlockValue} = await chain.produceBlock({
+      const {block, executionPayloadValue, consensusBlockValue, shouldOverrideBuilder} = await chain.produceBlock({
         slot,
         randaoReveal,
         graffiti: toGraffitiBuffer(graffiti || ""),
@@ -408,9 +408,10 @@ export function getValidatorApi({
           version,
           executionPayloadValue,
           consensusBlockValue,
+          shouldOverrideBuilder,
         };
       } else {
-        return {data: block, version, executionPayloadValue, consensusBlockValue};
+        return {data: block, version, executionPayloadValue, consensusBlockValue, shouldOverrideBuilder};
       }
     } finally {
       if (timer) timer({source});
@@ -504,7 +505,10 @@ export function getValidatorApi({
         // reference index of promises in the race
         const promisesOrder = [ProducedBlockSource.builder, ProducedBlockSource.engine];
         [blindedBlock, fullBlock] = await racePromisesWithCutoff<
-          routes.validator.ProduceBlockOrContentsRes | routes.validator.ProduceBlindedBlockRes | null
+          | ((routes.validator.ProduceBlockOrContentsRes | routes.validator.ProduceBlindedBlockRes) & {
+              shouldOverrideBuilder?: boolean;
+            })
+          | null
         >(
           [blindedBlockPromise, fullBlockPromise],
           BLOCK_PRODUCTION_RACE_CUTOFF_MS,
@@ -552,8 +556,20 @@ export function getValidatorApi({
       const blockValueEngine = enginePayloadValue + gweiToWei(consensusBlockValueEngine); // Total block value is in wei
 
       let executionPayloadSource: ProducedBlockSource | null = null;
+      const shouldOverrideBuilder = fullBlock?.shouldOverrideBuilder ?? false;
 
-      if (fullBlock && blindedBlock) {
+      // handle the builder override case separately
+      if (shouldOverrideBuilder === true) {
+        executionPayloadSource = ProducedBlockSource.engine;
+        logger.info("Selected engine block as censorship suspected in builder blocks", {
+          // winston logger doesn't like bigint
+          enginePayloadValue: `${enginePayloadValue}`,
+          consensusBlockValueEngine: `${consensusBlockValueEngine}`,
+          blockValueEngine: `${blockValueEngine}`,
+          shouldOverrideBuilder,
+          slot,
+        });
+      } else if (fullBlock && blindedBlock) {
         switch (builderSelection) {
           case routes.validator.BuilderSelection.MaxProfit: {
             if (
@@ -579,7 +595,7 @@ export function getValidatorApi({
             executionPayloadSource = ProducedBlockSource.builder;
           }
         }
-        logger.verbose(`Selected executionPayloadSource=${executionPayloadSource} block`, {
+        logger.info(`Selected executionPayloadSource=${executionPayloadSource} block`, {
           builderSelection,
           // winston logger doesn't like bigint
           builderBoostFactor: `${builderBoostFactor}`,
@@ -589,24 +605,27 @@ export function getValidatorApi({
           consensusBlockValueBuilder: `${consensusBlockValueBuilder}`,
           blockValueEngine: `${blockValueEngine}`,
           blockValueBuilder: `${blockValueBuilder}`,
+          shouldOverrideBuilder,
           slot,
         });
       } else if (fullBlock && !blindedBlock) {
         executionPayloadSource = ProducedBlockSource.engine;
-        logger.verbose("Selected engine block: no builder block produced", {
+        logger.info("Selected engine block: no builder block produced", {
           // winston logger doesn't like bigint
           enginePayloadValue: `${enginePayloadValue}`,
           consensusBlockValueEngine: `${consensusBlockValueEngine}`,
           blockValueEngine: `${blockValueEngine}`,
+          shouldOverrideBuilder,
           slot,
         });
       } else if (blindedBlock && !fullBlock) {
         executionPayloadSource = ProducedBlockSource.builder;
-        logger.verbose("Selected builder block: no engine block produced", {
+        logger.info("Selected builder block: no engine block produced", {
           // winston logger doesn't like bigint
           builderPayloadValue: `${builderPayloadValue}`,
           consensusBlockValueBuilder: `${consensusBlockValueBuilder}`,
           blockValueBuilder: `${blockValueBuilder}`,
+          shouldOverrideBuilder,
           slot,
         });
       }

packages/beacon-node/src/chain/chain.ts

@@ -472,22 +472,32 @@ export class BeaconChain implements IBeaconChain {
     return data && {block: data, executionOptimistic: false};
   }
 
-  produceBlock(
-    blockAttributes: BlockAttributes
-  ): Promise<{block: allForks.BeaconBlock; executionPayloadValue: Wei; consensusBlockValue: Gwei}> {
+  produceBlock(blockAttributes: BlockAttributes): Promise<{
+    block: allForks.BeaconBlock;
+    executionPayloadValue: Wei;
+    consensusBlockValue: Gwei;
+    shouldOverrideBuilder?: boolean;
+  }> {
     return this.produceBlockWrapper<BlockType.Full>(BlockType.Full, blockAttributes);
   }
 
-  produceBlindedBlock(
-    blockAttributes: BlockAttributes
-  ): Promise<{block: allForks.BlindedBeaconBlock; executionPayloadValue: Wei; consensusBlockValue: Gwei}> {
+  produceBlindedBlock(blockAttributes: BlockAttributes): Promise<{
+    block: allForks.BlindedBeaconBlock;
+    executionPayloadValue: Wei;
+    consensusBlockValue: Gwei;
+  }> {
     return this.produceBlockWrapper<BlockType.Blinded>(BlockType.Blinded, blockAttributes);
   }
 
   async produceBlockWrapper<T extends BlockType>(
     blockType: T,
     {randaoReveal, graffiti, slot, feeRecipient}: BlockAttributes
-  ): Promise<{block: AssembledBlockType<T>; executionPayloadValue: Wei; consensusBlockValue: Gwei}> {
+  ): Promise<{
+    block: AssembledBlockType<T>;
+    executionPayloadValue: Wei;
+    consensusBlockValue: Gwei;
+    shouldOverrideBuilder?: boolean;
+  }> {
     const head = this.forkChoice.getHead();
     const state = await this.regen.getBlockSlotState(
       head.blockRoot,
@@ -499,16 +509,21 @@ export class BeaconChain implements IBeaconChain {
     const proposerIndex = state.epochCtx.getBeaconProposer(slot);
     const proposerPubKey = state.epochCtx.index2pubkey[proposerIndex].toBytes();
 
-    const {body, blobs, executionPayloadValue} = await produceBlockBody.call(this, blockType, state, {
-      randaoReveal,
-      graffiti,
-      slot,
-      feeRecipient,
-      parentSlot: slot - 1,
-      parentBlockRoot,
-      proposerIndex,
-      proposerPubKey,
-    });
+    const {body, blobs, executionPayloadValue, shouldOverrideBuilder} = await produceBlockBody.call(
+      this,
+      blockType,
+      state,
+      {
+        randaoReveal,
+        graffiti,
+        slot,
+        feeRecipient,
+        parentSlot: slot - 1,
+        parentBlockRoot,
+        proposerIndex,
+        proposerPubKey,
+      }
+    );
 
     // The hashtree root computed here for debug log will get cached and hence won't introduce additional delays
     const bodyRoot =
@@ -559,7 +574,7 @@ export class BeaconChain implements IBeaconChain {
       this.metrics?.blockProductionCaches.producedContentsCache.set(this.producedContentsCache.size);
     }
 
-    return {block, executionPayloadValue, consensusBlockValue: proposerReward};
+    return {block, executionPayloadValue, consensusBlockValue: proposerReward, shouldOverrideBuilder};
   }
 
   /**

packages/beacon-node/src/chain/interface.ts

@@ -155,12 +155,17 @@ export interface IBeaconChain {
 
   getContents(beaconBlock: deneb.BeaconBlock): deneb.Contents;
 
-  produceBlock(
-    blockAttributes: BlockAttributes
-  ): Promise<{block: allForks.BeaconBlock; executionPayloadValue: Wei; consensusBlockValue: Gwei}>;
-  produceBlindedBlock(
-    blockAttributes: BlockAttributes
-  ): Promise<{block: allForks.BlindedBeaconBlock; executionPayloadValue: Wei; consensusBlockValue: Gwei}>;
+  produceBlock(blockAttributes: BlockAttributes): Promise<{
+    block: allForks.BeaconBlock;
+    executionPayloadValue: Wei;
+    consensusBlockValue: Gwei;
+    shouldOverrideBuilder?: boolean;
+  }>;
+  produceBlindedBlock(blockAttributes: BlockAttributes): Promise<{
+    block: allForks.BlindedBeaconBlock;
+    executionPayloadValue: Wei;
+    consensusBlockValue: Gwei;
+  }>;
 
   /** Process a block until complete */
   processBlock(block: BlockInput, opts?: ImportBlockOpts): Promise<void>;

packages/beacon-node/src/chain/produceBlock/produceBlockBody.ts

@@ -109,12 +109,20 @@ export async function produceBlockBody<T extends BlockType>(
     proposerIndex: ValidatorIndex;
     proposerPubKey: BLSPubkey;
   }
-): Promise<{body: AssembledBodyType<T>; blobs: BlobsResult; executionPayloadValue: Wei}> {
+): Promise<{
+  body: AssembledBodyType<T>;
+  blobs: BlobsResult;
+  executionPayloadValue: Wei;
+  shouldOverrideBuilder?: boolean;
+}> {
   // Type-safe for blobs variable. Translate 'null' value into 'preDeneb' enum
   // TODO: Not ideal, but better than just using null.
   // TODO: Does not guarantee that preDeneb enum goes with a preDeneb block
   let blobsResult: BlobsResult;
   let executionPayloadValue: Wei;
+  // even though shouldOverrideBuilder is relevant for the engine response, for simplicity of typing
+  // we just return it undefined for the builder which anyway doesn't get consumed downstream
+  let shouldOverrideBuilder: boolean | undefined;
   const fork = currentState.config.getForkName(blockSlot);
 
   const logMeta: Record<string, string | number | bigint> = {
@@ -295,9 +303,11 @@ export async function produceBlockBody<T extends BlockType>(
 
           const engineRes = await this.executionEngine.getPayload(fork, payloadId);
           const {executionPayload, blobsBundle} = engineRes;
+          shouldOverrideBuilder = engineRes.shouldOverrideBuilder;
+
           (blockBody as allForks.ExecutionBlockBody).executionPayload = executionPayload;
           executionPayloadValue = engineRes.executionPayloadValue;
-          Object.assign(logMeta, {transactions: executionPayload.transactions.length});
+          Object.assign(logMeta, {transactions: executionPayload.transactions.length, shouldOverrideBuilder});
 
           const fetchedTime = Date.now() / 1000 - computeTimeAtSlot(this.config, blockSlot, this.genesisTime);
           this.metrics?.blockPayload.payloadFetchedTime.observe({prepType}, fetchedTime);
@@ -380,7 +390,7 @@ export async function produceBlockBody<T extends BlockType>(
   Object.assign(logMeta, {executionPayloadValue});
   this.logger.verbose("Produced beacon block body", logMeta);
 
-  return {body: blockBody as AssembledBodyType<T>, blobs: blobsResult, executionPayloadValue};
+  return {body: blockBody as AssembledBodyType<T>, blobs: blobsResult, executionPayloadValue, shouldOverrideBuilder};
 }
 
 /**

packages/beacon-node/src/execution/engine/http.ts

@@ -363,7 +363,12 @@ export class ExecutionEngineHttp implements IExecutionEngine {
   async getPayload(
     fork: ForkName,
     payloadId: PayloadId
-  ): Promise<{executionPayload: allForks.ExecutionPayload; executionPayloadValue: Wei; blobsBundle?: BlobsBundle}> {
+  ): Promise<{
+    executionPayload: allForks.ExecutionPayload;
+    executionPayloadValue: Wei;
+    blobsBundle?: BlobsBundle;
+    shouldOverrideBuilder?: boolean;
+  }> {
     const method =
       ForkSeq[fork] >= ForkSeq.deneb
         ? "engine_getPayloadV3"

packages/beacon-node/src/execution/engine/interface.ts

@@ -136,7 +136,12 @@ export interface IExecutionEngine {
   getPayload(
     fork: ForkName,
     payloadId: PayloadId
-  ): Promise<{executionPayload: allForks.ExecutionPayload; executionPayloadValue: Wei; blobsBundle?: BlobsBundle}>;
+  ): Promise<{
+    executionPayload: allForks.ExecutionPayload;
+    executionPayloadValue: Wei;
+    blobsBundle?: BlobsBundle;
+    shouldOverrideBuilder?: boolean;
+  }>;
 
   getPayloadBodiesByHash(blockHash: DATA[]): Promise<(ExecutionPayloadBody | null)[]>;
 

packages/beacon-node/src/execution/engine/types.ts

@@ -107,6 +107,7 @@ type ExecutionPayloadRpcWithValue = {
   // even though CL tracks this as executionPayloadValue, EL returns this as blockValue
   blockValue: QUANTITY;
   blobsBundle?: BlobsBundleRpc;
+  shouldOverrideBuilder?: boolean;
 };
 type ExecutionPayloadResponse = ExecutionPayloadRpc | ExecutionPayloadRpcWithValue;
 
@@ -207,19 +208,28 @@ export function hasPayloadValue(response: ExecutionPayloadResponse): response is
 export function parseExecutionPayload(
   fork: ForkName,
   response: ExecutionPayloadResponse
-): {executionPayload: allForks.ExecutionPayload; executionPayloadValue: Wei; blobsBundle?: BlobsBundle} {
+): {
+  executionPayload: allForks.ExecutionPayload;
+  executionPayloadValue: Wei;
+  blobsBundle?: BlobsBundle;
+  shouldOverrideBuilder?: boolean;
+} {
   let data: ExecutionPayloadRpc;
   let executionPayloadValue: Wei;
   let blobsBundle: BlobsBundle | undefined;
+  let shouldOverrideBuilder: boolean;
+
   if (hasPayloadValue(response)) {
     executionPayloadValue = quantityToBigint(response.blockValue);
     data = response.executionPayload;
     blobsBundle = response.blobsBundle ? parseBlobsBundle(response.blobsBundle) : undefined;
+    shouldOverrideBuilder = response.shouldOverrideBuilder ?? false;
   } else {
     data = response;
     // Just set it to zero as default
     executionPayloadValue = BigInt(0);
     blobsBundle = undefined;
+    shouldOverrideBuilder = false;
   }
 
   const executionPayload = {
@@ -269,7 +279,7 @@ export function parseExecutionPayload(
     (executionPayload as deneb.ExecutionPayload).excessBlobGas = quantityToBigint(excessBlobGas);
   }
 
-  return {executionPayload, executionPayloadValue, blobsBundle};
+  return {executionPayload, executionPayloadValue, blobsBundle, shouldOverrideBuilder};
 }
 
 export function serializePayloadAttributes(data: PayloadAttributes): PayloadAttributesRpc {

packages/beacon-node/test/unit/api/impl/validator/produceBlockV2.test.ts

@@ -85,7 +85,11 @@ describe("api/validator - produceBlockV2", function () {
     const feeRecipient = "0xcccccccccccccccccccccccccccccccccccccccc";
 
     const api = getValidatorApi(modules);
-    server.chainStub.produceBlock.mockResolvedValue({block: fullBlock, executionPayloadValue, consensusBlockValue});
+    server.chainStub.produceBlock.mockResolvedValue({
+      block: fullBlock,
+      executionPayloadValue,
+      consensusBlockValue,
+    });
 
     // check if expectedFeeRecipient is passed to produceBlock
     await api.produceBlockV2(slot, randaoReveal, graffiti, {feeRecipient});