Some EIP4844 fixes (#4549)

* debug log upon sidecar validation failure

* Fill in signature catch upon SignedBeaconBlockAndBlobsSidecar deser

* Always fill blobssidecar slot and root

* Skip lastFCU when eth1monitor is nil

* fix

* Use cached root

beacon_chain/gossip_processing/block_processor.nim

@@ -168,7 +168,7 @@ proc storeBackfillBlock(
       when typeof(signedBlock).toFork() >= BeaconBlockFork.EIP4844:
         blobs.isSome() and not
           validate_blobs_sidecar(signedBlock.message.slot,
-                                 hash_tree_root(signedBlock.message),
+                                 signedBlock.root,
                                  signedBlock.message
                                  .body.blob_kzg_commitments.asSeq,
                                  blobs.get()).isOk()
@@ -403,13 +403,15 @@ proc storeBlock*(
   # Establish blob viability before calling addHeadBlock to avoid
   # writing the block in case of blob error.
   when typeof(signedBlock).toFork() >= BeaconBlockFork.EIP4844:
-    if blobs.isSome() and not
-         validate_blobs_sidecar(signedBlock.message.slot,
-                                hash_tree_root(signedBlock.message),
-                                signedBlock.message
-                                .body.blob_kzg_commitments.asSeq,
-                                blobs.get()).isOk():
-       return err((VerifierError.Invalid, ProcessingStatus.completed))
+    if blobs.isSome():
+      let res = validate_blobs_sidecar(signedBlock.message.slot,
+                                       signedBlock.root,
+                                       signedBlock.message
+                                       .body.blob_kzg_commitments.asSeq,
+                                       blobs.get())
+      if res.isErr():
+        debug "blobs sidecar validation failed", err = res.error()
+        return err((VerifierError.Invalid, ProcessingStatus.completed))
 
   type Trusted = typeof signedBlock.asTrusted()
   let blck = dag.addHeadBlock(self.verifier, signedBlock, payloadValid) do (

beacon_chain/spec/eth2_ssz_serialization.nim

@@ -59,6 +59,10 @@ template readSszBytes*(
 template readSszBytes*(
     data: openArray[byte], val: var eip4844.TrustedSignedBeaconBlock, updateRoot = true) =
   readAndUpdateRoot(data, val, updateRoot)
+template readSszBytes*(
+    data: openArray[byte], val: var eip4844.SignedBeaconBlockAndBlobsSidecar, updateRoot = true) =
+  readSszValue(data, val)
+  val.beacon_block.root = hash_tree_root(val.beacon_block.message)
 
 template readSszBytes*(
     data: openArray[byte], val: var auto, updateRoot: bool) =

beacon_chain/validators/validator_duties.nim

@@ -879,29 +879,34 @@ proc proposeBlock(node: BeaconNode,
     return head # already logged elsewhere!
 
   var forkedBlck = newBlock.get()
-  var blobs_sidecar = eip4844.BlobsSidecar()
 
   withBlck(forkedBlck):
+    var blobs_sidecar = eip4844.BlobsSidecar(
+      beacon_block_root: hash_tree_root(blck),
+      beacon_block_slot: slot,
+    )
     when blck is eip4844.BeaconBlock and const_preset != "minimal":
-      let
-        lastFcU = node.consensusManager.forkchoiceUpdatedInfo
-        payload_id = bellatrix.PayloadID(lastFcU.get.payloadId)
-        bundle = await getBlobsBundle(node, slot.epoch, validator_index, default(PayloadID))
+      # TODO when lastfcu is none, getExecutionPayload re-queries the EE.
+      # We don't do that here, which could lead us to propose invalid blocks
+      # (with a payload but no blobs).
+      if not (node.eth1Monitor.isNil) and
+         node.consensusManager.forkchoiceUpdatedInfo.isSome():
 
-        # todo: actually compute proof over blobs using nim-kzg-4844
-        kzg_aggregated_proof = default(KZGProof)
+        let
+          lastFcU = node.consensusManager.forkchoiceUpdatedInfo
+          payload_id = bellatrix.PayloadID(lastFcU.get.payloadId)
+          bundle = await getBlobsBundle(node, slot.epoch, validator_index, default(PayloadID))
 
-      blck.body.blob_kzg_commitments =
-          List[eip4844.KZGCommitment, Limit MAX_BLOBS_PER_BLOCK].init(
-            mapIt(bundle.kzgs, eip4844.KzgCommitment(it)))
+          # todo: actually compute proof over blobs using nim-kzg-4844
+          kzg_aggregated_proof = default(KZGProof)
 
-      blobs_sidecar = BlobsSidecar(
-        beacon_block_root: hash_tree_root(blck),
-        beacon_block_slot: slot,
-        blobs: List[eip4844.Blob, Limit MAX_BLOBS_PER_BLOCK].init(
-          mapIt(bundle.blobs, eip4844.Blob(it))),
-        kzg_aggregated_proof: kzg_aggregated_proof
-      )
+        blck.body.blob_kzg_commitments =
+            List[eip4844.KZGCommitment, Limit MAX_BLOBS_PER_BLOCK].init(
+              mapIt(bundle.kzgs, eip4844.KzgCommitment(it)))
+
+        blobs_sidecar.blobs = List[eip4844.Blob, Limit MAX_BLOBS_PER_BLOCK].init(
+          mapIt(bundle.blobs, eip4844.Blob(it)))
+        blobs_sidecar.kzg_aggregated_proof = kzg_aggregated_proof
 
     let
       blockRoot = hash_tree_root(blck)