feat: change the blob txs encoding assumption to be opaque rlp (#5569)

* feat: change the blob txs encoding assumption to be opaque rlp * add dummy blob txs * fix build
ChainSafe · Jun 3, 2023 · dd11c95 · dd11c95
1 parent 2f2bc46
commit dd11c95
Show file tree

Hide file tree

Showing 12 changed files with 95 additions and 231 deletions.
diff --git a/packages/beacon-node/src/chain/blocks/verifyBlocksExecutionPayloads.ts b/packages/beacon-node/src/chain/blocks/verifyBlocksExecutionPayloads.ts
@@ -4,8 +4,9 @@ import {
   isExecutionBlockBodyType,
   isMergeTransitionBlock as isMergeTransitionBlockFn,
   isExecutionEnabled,
+  kzgCommitmentToVersionedHash,
 } from "@lodestar/state-transition";
-import {bellatrix, allForks, Slot} from "@lodestar/types";
+import {bellatrix, allForks, Slot, deneb} from "@lodestar/types";
 import {toHexString} from "@chainsafe/ssz";
 import {
   IForkChoice,
@@ -18,6 +19,8 @@ import {
 } from "@lodestar/fork-choice";
 import {ChainForkConfig} from "@lodestar/config";
 import {ErrorAborted, Logger} from "@lodestar/utils";
+import {ForkSeq} from "@lodestar/params";
+
 import {IExecutionEngine} from "../../execution/engine/index.js";
 import {BlockError, BlockErrorCode} from "../errors/index.js";
 import {IClock} from "../../util/clock.js";
@@ -285,10 +288,12 @@ export async function verifyBlockExecutionPayload(
   }
 
   // TODO: Handle better notifyNewPayload() returning error is syncing
-  const execResult = await chain.executionEngine.notifyNewPayload(
-    chain.config.getForkName(block.message.slot),
-    executionPayloadEnabled
-  );
+  const fork = chain.config.getForkName(block.message.slot);
+  const versionedHashes =
+    ForkSeq[fork] >= ForkSeq.deneb
+      ? (block.message.body as deneb.BeaconBlockBody).blobKzgCommitments.map(kzgCommitmentToVersionedHash)
+      : undefined;
+  const execResult = await chain.executionEngine.notifyNewPayload(fork, executionPayloadEnabled, versionedHashes);
 
   chain.metrics?.engineNotifyNewPayloadResult.inc({result: execResult.status});
 

diff --git a/packages/beacon-node/src/chain/produceBlock/validateBlobsAndKzgCommitments.ts b/packages/beacon-node/src/chain/produceBlock/validateBlobsAndKzgCommitments.ts
@@ -1,28 +1,15 @@
-import {verifyKzgCommitmentsAgainstTransactions} from "@lodestar/state-transition";
-import {allForks, deneb} from "@lodestar/types";
-import {toHex} from "@lodestar/utils";
+import {allForks} from "@lodestar/types";
 import {BlobsBundle} from "../../execution/index.js";
-import {byteArrayEquals} from "../../util/bytes.js";
-import {ckzg} from "../../util/kzg.js";
 
 /**
  * Optionally sanity-check that the KZG commitments match the versioned hashes in the transactions
  * https://github.com/ethereum/consensus-specs/blob/11a037fd9227e29ee809c9397b09f8cc3383a8c0/specs/eip4844/validator.md#blob-kzg-commitments
  */
 export function validateBlobsAndKzgCommitments(payload: allForks.ExecutionPayload, blobsBundle: BlobsBundle): void {
-  verifyKzgCommitmentsAgainstTransactions(payload.transactions, blobsBundle.commitments);
-
-  // Optionally sanity-check that the KZG commitments match the blobs (as produced by the execution engine)
+  // sanity-check that the KZG commitments match the blobs (as produced by the execution engine)
   if (blobsBundle.blobs.length !== blobsBundle.commitments.length) {
     throw Error(
       `Blobs bundle blobs len ${blobsBundle.blobs.length} != commitments len ${blobsBundle.commitments.length}`
     );
   }
-
-  for (let i = 0; i < blobsBundle.blobs.length; i++) {
-    const kzg = ckzg.blobToKzgCommitment(blobsBundle.blobs[i]) as deneb.KZGCommitment;
-    if (!byteArrayEquals(kzg, blobsBundle.commitments[i])) {
-      throw Error(`Wrong KZG[${i}] ${toHex(blobsBundle.commitments[i])} expected ${toHex(kzg)}`);
-    }
-  }
 }
diff --git a/packages/beacon-node/src/chain/validation/blobsSidecar.ts b/packages/beacon-node/src/chain/validation/blobsSidecar.ts
@@ -3,7 +3,6 @@ import {CoordType} from "@chainsafe/bls/types";
 import {deneb, Root, ssz} from "@lodestar/types";
 import {bytesToBigInt, toHex} from "@lodestar/utils";
 import {BYTES_PER_FIELD_ELEMENT, FIELD_ELEMENTS_PER_BLOB} from "@lodestar/params";
-import {verifyKzgCommitmentsAgainstTransactions} from "@lodestar/state-transition";
 import {BlobsSidecarError, BlobsSidecarErrorCode} from "../errors/blobsSidecarError.js";
 import {GossipAction} from "../errors/gossipValidation.js";
 import {byteArrayEquals} from "../../util/bytes.js";
@@ -27,14 +26,6 @@ export function validateGossipBlobsSidecar(
     }
   }
 
-  // [REJECT] The KZG commitments correspond to the versioned hashes in the transactions list.
-  // -- i.e. verify_kzg_commitments_against_transactions(block.body.execution_payload.transactions, block.body.blob_kzg_commitments)
-  if (
-    !verifyKzgCommitmentsAgainstTransactions(block.body.executionPayload.transactions, block.body.blobKzgCommitments)
-  ) {
-    throw new BlobsSidecarError(GossipAction.REJECT, {code: BlobsSidecarErrorCode.INVALID_KZG_TXS});
-  }
-
   // [IGNORE] the sidecar.beacon_block_slot is for the current slot (with a MAXIMUM_GOSSIP_CLOCK_DISPARITY allowance)
   // -- i.e. sidecar.beacon_block_slot == block.slot.
   if (blobsSidecar.beaconBlockSlot !== block.slot) {

diff --git a/packages/beacon-node/src/execution/engine/http.ts b/packages/beacon-node/src/execution/engine/http.ts
@@ -15,13 +15,15 @@ import {
   PayloadAttributes,
   TransitionConfigurationV1,
   BlobsBundle,
+  VersionedHashes,
 } from "./interface.js";
 import {PayloadIdCache} from "./payloadIdCache.js";
 import {
   EngineApiRpcParamTypes,
   EngineApiRpcReturnTypes,
   parseExecutionPayload,
   serializeExecutionPayload,
+  serializeVersionedHashes,
   serializePayloadAttributes,
   ExecutionPayloadBody,
   assertReqSizeLimit,
@@ -133,20 +135,44 @@ 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 notifyNewPayload(fork: ForkName, executionPayload: allForks.ExecutionPayload): Promise<ExecutePayloadResponse> {
+  async notifyNewPayload(
+    fork: ForkName,
+    executionPayload: allForks.ExecutionPayload,
+    versionedHashes?: VersionedHashes
+  ): Promise<ExecutePayloadResponse> {
     const method =
       ForkSeq[fork] >= ForkSeq.deneb
         ? "engine_newPayloadV3"
         : ForkSeq[fork] >= ForkSeq.capella
         ? "engine_newPayloadV2"
         : "engine_newPayloadV1";
+
     const serializedExecutionPayload = serializeExecutionPayload(fork, executionPayload);
-    const {status, latestValidHash, validationError} = await (
-      this.rpcFetchQueue.push({
+    const serializedVersionedHashes =
+      versionedHashes !== undefined ? serializeVersionedHashes(versionedHashes) : undefined;
+
+    let engingRequest: EngineRequest;
+    if (ForkSeq[fork] >= ForkSeq.deneb) {
+      if (serializedVersionedHashes === undefined) {
+        throw Error(`versionedHashes required in notifyNewPayload for fork=${fork}`);
+      }
+      const method = "engine_newPayloadV3";
+      engingRequest = {
+        method,
+        params: [serializedExecutionPayload, serializedVersionedHashes],
+        methodOpts: notifyNewPayloadOpts,
+      };
+    } else {
+      const method = ForkSeq[fork] >= ForkSeq.capella ? "engine_newPayloadV2" : "engine_newPayloadV1";
+      engingRequest = {
         method,
         params: [serializedExecutionPayload],
         methodOpts: notifyNewPayloadOpts,
-      }) as Promise<EngineApiRpcReturnTypes[typeof method]>
+      };
+    }
+
+    const {status, latestValidHash, validationError} = await (
+      this.rpcFetchQueue.push(engingRequest) as Promise<EngineApiRpcReturnTypes[typeof method]>
     )
       // If there are errors by EL like connection refused, internal error, they need to be
       // treated separate from being INVALID. For now, just pass the error upstream.

diff --git a/packages/beacon-node/src/execution/engine/interface.ts b/packages/beacon-node/src/execution/engine/interface.ts
@@ -70,6 +70,8 @@ export type BlobsBundle = {
   proofs: KZGProof[];
 };
 
+export type VersionedHashes = Uint8Array[];
+
 /**
  * Execution engine represents an abstract protocol to interact with execution clients. Potential transports include:
  * - JSON RPC over network
@@ -87,7 +89,11 @@ export interface IExecutionEngine {
    *
    * Should be called in advance before, after or in parallel to block processing
    */
-  notifyNewPayload(fork: ForkName, executionPayload: allForks.ExecutionPayload): Promise<ExecutePayloadResponse>;
+  notifyNewPayload(
+    fork: ForkName,
+    executionPayload: allForks.ExecutionPayload,
+    versionedHashes?: VersionedHashes
+  ): Promise<ExecutePayloadResponse>;
 
   /**
    * Signal fork choice updates

diff --git a/packages/beacon-node/src/execution/engine/mock.ts b/packages/beacon-node/src/execution/engine/mock.ts
@@ -1,18 +1,14 @@
 import crypto from "node:crypto";
-import {
-  kzgCommitmentToVersionedHash,
-  OPAQUE_TX_BLOB_VERSIONED_HASHES_OFFSET,
-  OPAQUE_TX_MESSAGE_OFFSET,
-} from "@lodestar/state-transition";
+import {kzgCommitmentToVersionedHash} from "@lodestar/state-transition";
 import {bellatrix, deneb, RootHex, ssz} from "@lodestar/types";
 import {fromHex, toHex} from "@lodestar/utils";
 import {
   BYTES_PER_FIELD_ELEMENT,
   FIELD_ELEMENTS_PER_BLOB,
-  BLOB_TX_TYPE,
   ForkSeq,
   ForkExecution,
   ForkName,
+  BLOB_TX_TYPE,
 } from "@lodestar/params";
 import {ZERO_HASH_HEX} from "../../constants/index.js";
 import {ckzg} from "../../util/kzg.js";
@@ -139,7 +135,9 @@ export class ExecutionEngineMockBackend implements JsonRpcBackend {
    * `engine_newPayloadV1`
    */
   private notifyNewPayload(
-    executionPayloadRpc: EngineApiRpcParamTypes["engine_newPayloadV1"][0]
+    executionPayloadRpc: EngineApiRpcParamTypes["engine_newPayloadV1"][0],
+    // TODO deneb: add versionedHashes validation
+    _versionedHashes?: EngineApiRpcParamTypes["engine_newPayloadV3"][1]
   ): EngineApiRpcReturnTypes["engine_newPayloadV1"] {
     const blockHash = executionPayloadRpc.blockHash;
     const parentHash = executionPayloadRpc.parentHash;
@@ -405,26 +403,13 @@ export class ExecutionEngineMockBackend implements JsonRpcBackend {
 }
 
 function transactionForKzgCommitment(kzgCommitment: deneb.KZGCommitment): bellatrix.Transaction {
-  // Some random value that after the offset's position
-  const blobVersionedHashesOffset = OPAQUE_TX_BLOB_VERSIONED_HASHES_OFFSET + 64;
-
-  // +32 for the size of versionedHash
-  const ab = new ArrayBuffer(blobVersionedHashesOffset + 32);
-  const dv = new DataView(ab);
-  const ua = new Uint8Array(ab);
-
-  // Set tx type
-  dv.setUint8(0, BLOB_TX_TYPE);
-
-  // Set offset to hashes array
-  // const blobVersionedHashesOffset =
-  //   OPAQUE_TX_MESSAGE_OFFSET + opaqueTxDv.getUint32(OPAQUE_TX_BLOB_VERSIONED_HASHES_OFFSET, true);
-  dv.setUint32(OPAQUE_TX_BLOB_VERSIONED_HASHES_OFFSET, blobVersionedHashesOffset - OPAQUE_TX_MESSAGE_OFFSET, true);
-
+  // Just use versionedHash as the transaction encoding to mock newPayloadV3 verification
+  // prefixed with BLOB_TX_TYPE
+  const transaction = new Uint8Array(33);
   const versionedHash = kzgCommitmentToVersionedHash(kzgCommitment);
-  ua.set(versionedHash, blobVersionedHashesOffset);
-
-  return ua;
+  transaction[0] = BLOB_TX_TYPE;
+  transaction.set(versionedHash, 1);
+  return transaction;
 }
 
 /**

diff --git a/packages/beacon-node/src/execution/engine/types.ts b/packages/beacon-node/src/execution/engine/types.ts
@@ -16,7 +16,13 @@ import {
   QUANTITY,
   quantityToBigint,
 } from "../../eth1/provider/utils.js";
-import {ExecutePayloadStatus, TransitionConfigurationV1, BlobsBundle, PayloadAttributes} from "./interface.js";
+import {
+  ExecutePayloadStatus,
+  TransitionConfigurationV1,
+  BlobsBundle,
+  PayloadAttributes,
+  VersionedHashes,
+} from "./interface.js";
 import {WithdrawalV1} from "./payloadIdCache.js";
 
 /* eslint-disable @typescript-eslint/naming-convention */
@@ -27,7 +33,7 @@ export type EngineApiRpcParamTypes = {
    */
   engine_newPayloadV1: [ExecutionPayloadRpc];
   engine_newPayloadV2: [ExecutionPayloadRpc];
-  engine_newPayloadV3: [ExecutionPayloadRpc];
+  engine_newPayloadV3: [ExecutionPayloadRpc, VersionedHashesRpc];
   /**
    * 1. Object - Payload validity status with respect to the consensus rules:
    *   - blockHash: DATA, 32 Bytes - block hash value of the payload
@@ -139,6 +145,8 @@ export type WithdrawalRpc = {
   amount: QUANTITY;
 };
 
+export type VersionedHashesRpc = DATA[];
+
 export type PayloadAttributesRpc = {
   /** QUANTITY, 64 Bits - value for the timestamp field of the new payload */
   timestamp: QUANTITY;
@@ -187,6 +195,10 @@ export function serializeExecutionPayload(fork: ForkName, data: allForks.Executi
   return payload;
 }
 
+export function serializeVersionedHashes(vHashes: VersionedHashes): VersionedHashesRpc {
+  return vHashes.map(bytesToData);
+}
+
 export function hasBlockValue(response: ExecutionPayloadResponse): response is ExecutionPayloadRpcWithBlockValue {
   return (response as ExecutionPayloadRpcWithBlockValue).blockValue !== undefined;
 }

diff --git a/packages/beacon-node/test/unit/util/kzg.test.ts b/packages/beacon-node/test/unit/util/kzg.test.ts
@@ -1,11 +1,7 @@
 import {expect} from "chai";
 import {bellatrix, deneb, ssz} from "@lodestar/types";
-import {BLOB_TX_TYPE, BYTES_PER_FIELD_ELEMENT} from "@lodestar/params";
-import {
-  kzgCommitmentToVersionedHash,
-  OPAQUE_TX_BLOB_VERSIONED_HASHES_OFFSET,
-  OPAQUE_TX_MESSAGE_OFFSET,
-} from "@lodestar/state-transition";
+import {BYTES_PER_FIELD_ELEMENT, BLOB_TX_TYPE} from "@lodestar/params";
+import {kzgCommitmentToVersionedHash} from "@lodestar/state-transition";
 import {loadEthereumTrustedSetup, initCKZG, ckzg, FIELD_ELEMENTS_PER_BLOB_MAINNET} from "../../../src/util/kzg.js";
 import {validateBlobsSidecar, validateGossipBlobsSidecar} from "../../../src/chain/validation/blobsSidecar.js";
 
@@ -54,26 +50,13 @@ describe("C-KZG", () => {
 });
 
 function transactionForKzgCommitment(kzgCommitment: deneb.KZGCommitment): bellatrix.Transaction {
-  // Some random value that after the offset's position
-  const blobVersionedHashesOffset = OPAQUE_TX_BLOB_VERSIONED_HASHES_OFFSET + 64;
-
-  // +32 for the size of versionedHash
-  const ab = new ArrayBuffer(blobVersionedHashesOffset + 32);
-  const dv = new DataView(ab);
-  const ua = new Uint8Array(ab);
-
-  // Set tx type
-  dv.setUint8(0, BLOB_TX_TYPE);
-
-  // Set offset to hashes array
-  // const blobVersionedHashesOffset =
-  //   OPAQUE_TX_MESSAGE_OFFSET + opaqueTxDv.getUint32(OPAQUE_TX_BLOB_VERSIONED_HASHES_OFFSET, true);
-  dv.setUint32(OPAQUE_TX_BLOB_VERSIONED_HASHES_OFFSET, blobVersionedHashesOffset - OPAQUE_TX_MESSAGE_OFFSET, true);
-
+  // Just use versionedHash as the transaction encoding to mock newPayloadV3 verification
+  // prefixed with BLOB_TX_TYPE
+  const transaction = new Uint8Array(33);
   const versionedHash = kzgCommitmentToVersionedHash(kzgCommitment);
-  ua.set(versionedHash, blobVersionedHashesOffset);
-
-  return ua;
+  transaction[0] = BLOB_TX_TYPE;
+  transaction.set(versionedHash, 1);
+  return transaction;
 }
 
 /**

diff --git a/packages/state-transition/src/block/index.ts b/packages/state-transition/src/block/index.ts
@@ -1,5 +1,5 @@
 import {ForkSeq} from "@lodestar/params";
-import {allForks, altair, capella, deneb} from "@lodestar/types";
+import {allForks, altair, capella} from "@lodestar/types";
 import {getFullOrBlindedPayload, isExecutionEnabled} from "../util/execution.js";
 import {CachedBeaconStateAllForks, CachedBeaconStateCapella, CachedBeaconStateBellatrix} from "../types.js";
 import {processExecutionPayload} from "./processExecutionPayload.js";
@@ -62,7 +62,7 @@ export function processBlock(
   }
 
   if (fork >= ForkSeq.deneb) {
-    processBlobKzgCommitments(block.body as deneb.BeaconBlockBody);
+    processBlobKzgCommitments(externalData);
     // Only throw preDeneb so beacon can also sync/process blocks optimistically
     // and let forkChoice handle it
     if (externalData.dataAvailableStatus === DataAvailableStatus.preDeneb) {

diff --git a/packages/state-transition/src/block/processBlobKzgCommitments.ts b/packages/state-transition/src/block/processBlobKzgCommitments.ts
@@ -1,5 +1,4 @@
-import {deneb} from "@lodestar/types";
-import {verifyKzgCommitmentsAgainstTransactions} from "../util/index.js";
+import {BlockExternalData, ExecutionPayloadStatus} from "./externalData.js";
 
 /**
  * https://github.com/ethereum/consensus-specs/blob/11a037fd9227e29ee809c9397b09f8cc3383a8c0/specs/eip4844/beacon-chain.md#blob-kzg-commitments
@@ -10,8 +9,13 @@ import {verifyKzgCommitmentsAgainstTransactions} from "../util/index.js";
  *     body.blob_kzg_commitments
  *   )
  */
-export function processBlobKzgCommitments(body: deneb.BeaconBlockBody): void {
-  if (!verifyKzgCommitmentsAgainstTransactions(body.executionPayload.transactions, body.blobKzgCommitments)) {
-    throw Error("Invalid KZG commitments against transactions");
+export function processBlobKzgCommitments(externalData: BlockExternalData): void {
+  switch (externalData.executionPayloadStatus) {
+    case ExecutionPayloadStatus.preMerge:
+      throw Error("executionPayloadStatus preMerge");
+    case ExecutionPayloadStatus.invalid:
+      throw Error("Invalid execution payload");
+    case ExecutionPayloadStatus.valid:
+      break; // ok
   }
 }