From f14389bb8440f437e9cea6f2ba6d6386fbdd2219 Mon Sep 17 00:00:00 2001 From: Etan Kissling Date: Sat, 4 Nov 2023 20:36:12 +0100 Subject: [PATCH] avoid perpetually sending blobs to peers (#5563) Fix regression from #4808 where blobs that are already known are issued ACCEPT verdict, propagating them to peers over and over again. `validateBlobSidecar` contains the correct IGNORE logic. Moved it above the expensive checks to retain the performance of the check. --- beacon_chain/gossip_processing/eth2_processor.nim | 7 +------ beacon_chain/gossip_processing/gossip_validation.nim | 12 ++++++------ 2 files changed, 7 insertions(+), 12 deletions(-) diff --git a/beacon_chain/gossip_processing/eth2_processor.nim b/beacon_chain/gossip_processing/eth2_processor.nim index 71f22f66d..87406275b 100644 --- a/beacon_chain/gossip_processing/eth2_processor.nim +++ b/beacon_chain/gossip_processing/eth2_processor.nim @@ -287,12 +287,7 @@ proc processSignedBlobSidecar*( # Potential under/overflows are fine; would just create odd metrics and logs let delay = wallTime - signedBlobSidecar.message.slot.start_beacon_time - - if self.blobQuarantine[].hasBlob(signedBlobSidecar.message): - debug "Blob received, already in quarantine", delay - return ValidationRes.ok - else: - debug "Blob received", delay + debug "Blob received", delay let v = self.dag.validateBlobSidecar(self.quarantine, self.blobQuarantine, diff --git a/beacon_chain/gossip_processing/gossip_validation.nim b/beacon_chain/gossip_processing/gossip_validation.nim index 8ec4538f0..5874a8349 100644 --- a/beacon_chain/gossip_processing/gossip_validation.nim +++ b/beacon_chain/gossip_processing/gossip_validation.nim @@ -331,6 +331,12 @@ proc validateBlobSidecar*( if not (sbs.message.slot > dag.finalizedHead.slot): return errIgnore("SignedBlobSidecar: slot already finalized") + # [IGNORE] The sidecar is the only sidecar with valid signature + # received for the tuple (sidecar.block_root, sidecar.index). + if blobQuarantine[].hasBlob(sbs.message): + return errIgnore( + "SignedBlobSidecar: already have blob with valid signature") + # [IGNORE] The block's parent (defined by block.parent_root) has # been seen (via both gossip and non-gossip sources) (a client MAY # queue blocks for processing once the parent block is retrieved). @@ -376,12 +382,6 @@ proc validateBlobSidecar*( sbs.signature): return dag.checkedReject("SignedBlobSidecar: invalid blob signature") - # [IGNORE] The sidecar is the only sidecar with valid signature - # received for the tuple (sidecar.block_root, sidecar.index). - if blobQuarantine[].hasBlob(sbs.message): - return errIgnore( - "SignedBlobSidecar: already have blob with valid signature") - ok()