Refactor sync hashBlocks (#4343)

* Use Buffer.concat for bytes utils

* Simplify Buffer.concat()

* Remove need for byteArrayConcat

Co-authored-by: dapplion <[email protected]>

packages/beacon-node/src/sync/range/batch.ts

@@ -1,5 +1,5 @@
 import PeerId from "peer-id";
-import {allForks, Epoch, phase0} from "@lodestar/types";
+import {allForks, Epoch, phase0, RootHex} from "@lodestar/types";
 import {IChainForkConfig} from "@lodestar/config";
 import {LodestarError} from "@lodestar/utils";
 import {MAX_BATCH_DOWNLOAD_ATTEMPTS, MAX_BATCH_PROCESSING_ATTEMPTS} from "../constants.js";
@@ -32,7 +32,7 @@ export type Attempt = {
   /** The peer that made the attempt */
   peer: PeerId;
   /** The hash of the blocks of the attempt */
-  hash: Uint8Array;
+  hash: RootHex;
 };
 
 export type BatchState =

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

@@ -5,7 +5,6 @@ import {IChainForkConfig} from "@lodestar/config";
 import {toHexString} from "@chainsafe/ssz";
 import {PeerAction} from "../../network/index.js";
 import {ItTrigger} from "../../util/itTrigger.js";
-import {byteArrayEquals} from "../../util/bytes.js";
 import {PeerMap} from "../../util/peerMap.js";
 import {wrapError} from "../../util/wrapError.js";
 import {RangeSyncType} from "../utils/remoteSyncType.js";
@@ -483,7 +482,7 @@ export class SyncChain {
         // The last batch attempt is right, all others are wrong. Penalize other peers
         const attemptOk = batch.validationSuccess();
         for (const attempt of batch.failedProcessingAttempts) {
-          if (!byteArrayEquals(attempt.hash, attemptOk.hash)) {
+          if (attempt.hash !== attemptOk.hash) {
             if (attemptOk.peer.toB58String() === attempt.peer.toB58String()) {
               // The same peer corrected its previous attempt
               this.reportPeer(attempt.peer, PeerAction.MidToleranceError, "SyncChainInvalidBatchSelf");

packages/beacon-node/src/sync/range/utils/hashBlocks.ts

@@ -1,14 +1,23 @@
-import {allForks} from "@lodestar/types";
+import {allForks, RootHex} from "@lodestar/types";
 import {IChainForkConfig} from "@lodestar/config";
-import {byteArrayConcat} from "../../../util/bytes.js";
+import {toHex} from "@lodestar/utils";
 
 /**
- * Hash SignedBeaconBlock in a byte form easy to compare only
- * @param blocks
- * @param config
+ * String to uniquely identify block segments. Used for peer scoring and to compare if batches are equivalent.
  */
-export function hashBlocks(blocks: allForks.SignedBeaconBlock[], config: IChainForkConfig): Uint8Array {
-  return byteArrayConcat(
-    blocks.map((block) => config.getForkTypes(block.message.slot).SignedBeaconBlock.hashTreeRoot(block))
-  );
+export function hashBlocks(blocks: allForks.SignedBeaconBlock[], config: IChainForkConfig): RootHex {
+  switch (blocks.length) {
+    case 0:
+      return "0x";
+    case 1:
+      return toHex(config.getForkTypes(blocks[0].message.slot).SignedBeaconBlock.hashTreeRoot(blocks[0]));
+    default: {
+      const block0 = blocks[0];
+      const blockN = blocks[blocks.length - 1];
+      return (
+        toHex(config.getForkTypes(block0.message.slot).SignedBeaconBlock.hashTreeRoot(block0)) +
+        toHex(config.getForkTypes(blockN.message.slot).SignedBeaconBlock.hashTreeRoot(blockN))
+      );
+    }
+  }
 }

packages/beacon-node/src/util/bytes.ts

@@ -1,18 +1,5 @@
 import {Root} from "@lodestar/types";
 
-export function byteArrayConcat(bytesArr: Uint8Array[]): Uint8Array {
-  const totalBytes = bytesArr.reduce((total, bytes) => total + bytes.length, 0);
-  const mergedBytes = new Uint8Array(totalBytes);
-
-  let offset = 0;
-  for (const bytes of bytesArr) {
-    mergedBytes.set(bytes, offset);
-    offset += bytes.length;
-  }
-
-  return mergedBytes;
-}
-
 export function byteArrayEquals(a: Uint8Array | Root, b: Uint8Array | Root): boolean {
   if (a.length !== b.length) {
     return false;

packages/beacon-node/test/perf/util/bytes.test.ts

@@ -0,0 +1,22 @@
+import {itBench} from "@dapplion/benchmark";
+
+describe("bytes utils", function () {
+  const roots: Uint8Array[] = [];
+  let buffers: Buffer[] = [];
+  const count = 32;
+  before(function () {
+    this.timeout(60 * 1000);
+    for (let i = 0; i < count; i++) {
+      roots.push(new Uint8Array(Array.from({length: 32}, () => i)));
+    }
+    buffers = roots.map((root) => Buffer.from(root.buffer));
+  });
+
+  itBench({
+    id: `Buffer.concat ${count} items`,
+    fn: () => {
+      Buffer.concat(buffers);
+    },
+    runsFactor: 1000,
+  });
+});

packages/beacon-node/test/unit/util/bytes.test.ts

@@ -1,7 +1,12 @@
 import {expect} from "chai";
 import {fromHexString, toHexString} from "@chainsafe/ssz";
 
-import {byteArrayConcat, byteArrayEquals} from "../../../src/util/bytes.js";
+import {byteArrayEquals} from "../../../src/util/bytes.js";
+
+/** Reference implementation of byteArrayConcat */
+function byteArrayConcat(bytesArr: Uint8Array[]): Uint8Array {
+  return Buffer.concat(bytesArr);
+}
 
 describe("util / bytes", () => {
   describe("byteArrayConcat", () => {