RPC blob sidecars validating refactor

StefanBratanov · Nov 2, 2023 · 1282e05 · 1282e05
1 parent ae5974d
commit 1282e05
Show file tree

Hide file tree

Showing 10 changed files with 78 additions and 64 deletions.
diff --git a/...y-test/java/tech/pegasys/teku/spec/logic/versions/deneb/helpers/CKZG4844PropertyTest.java b/...y-test/java/tech/pegasys/teku/spec/logic/versions/deneb/helpers/CKZG4844PropertyTest.java
@@ -31,6 +31,20 @@
 
 @AddLifecycleHook(KzgResolver.class)
 public class CKZG4844PropertyTest {
+
+  @Property(tries = 100)
+  void fuzzVerifyBlobKzgProof(
+      final KZG kzg,
+      @ForAll(supplier = DiverseBlobBytesSupplier.class) final Bytes blob,
+      @ForAll(supplier = KZGCommitmentSupplier.class) final KZGCommitment commitment,
+      @ForAll(supplier = KZGProofSupplier.class) final KZGProof proof) {
+    try {
+      kzg.verifyBlobKzgProof(blob, commitment, proof);
+    } catch (Exception e) {
+      assertThat(e).isInstanceOf(KZGException.class);
+    }
+  }
+
   @Property(tries = 100)
   void fuzzVerifyBlobKzgProofBatch(
       final KZG kzg,

diff --git a/networking/eth2/src/main/java/tech/pegasys/teku/networking/eth2/peers/DefaultEth2Peer.java b/networking/eth2/src/main/java/tech/pegasys/teku/networking/eth2/peers/DefaultEth2Peer.java
@@ -35,9 +35,9 @@
 import tech.pegasys.teku.infrastructure.unsigned.UInt64;
 import tech.pegasys.teku.kzg.KZG;
 import tech.pegasys.teku.networking.eth2.rpc.beaconchain.BeaconChainMethods;
-import tech.pegasys.teku.networking.eth2.rpc.beaconchain.methods.BlobSidecarByRootValidator;
 import tech.pegasys.teku.networking.eth2.rpc.beaconchain.methods.BlobSidecarsByRangeListenerValidatingProxy;
 import tech.pegasys.teku.networking.eth2.rpc.beaconchain.methods.BlobSidecarsByRootListenerValidatingProxy;
+import tech.pegasys.teku.networking.eth2.rpc.beaconchain.methods.BlobSidecarsByRootValidator;
 import tech.pegasys.teku.networking.eth2.rpc.beaconchain.methods.BlocksByRangeListenerWrapper;
 import tech.pegasys.teku.networking.eth2.rpc.beaconchain.methods.MetadataMessagesFactory;
 import tech.pegasys.teku.networking.eth2.rpc.beaconchain.methods.StatusMessageFactory;
@@ -290,10 +290,10 @@ public SafeFuture<Optional<BlobSidecar>> requestBlobSidecarByRoot(
                         maybeBlobSidecar ->
                             maybeBlobSidecar.ifPresent(
                                 blobSidecar -> {
-                                  final BlobSidecarByRootValidator blobSidecarByRootValidator =
-                                      new BlobSidecarByRootValidator(
+                                  final BlobSidecarsByRootValidator validator =
+                                      new BlobSidecarsByRootValidator(
                                           this, spec, kzg, Set.of(blobIdentifier));
-                                  blobSidecarByRootValidator.validateBlobSidecar(blobSidecar);
+                                  validator.validate(blobSidecar);
                                 })))
         .orElse(failWithUnsupportedMethodException("BlobSidecarsByRoot"));
   }

diff --git a/...h/pegasys/teku/networking/eth2/rpc/beaconchain/methods/AbstractBlobSidecarsValidator.java b/...h/pegasys/teku/networking/eth2/rpc/beaconchain/methods/AbstractBlobSidecarsValidator.java
@@ -13,31 +13,44 @@
 
 package tech.pegasys.teku.networking.eth2.rpc.beaconchain.methods;
 
+import static tech.pegasys.teku.networking.eth2.rpc.beaconchain.methods.BlobSidecarsResponseInvalidResponseException.InvalidResponseType.BLOB_SIDECAR_KZG_VERIFICATION_FAILED;
+
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 import tech.pegasys.teku.kzg.KZG;
-import tech.pegasys.teku.kzg.KZGException;
+import tech.pegasys.teku.networking.p2p.peer.Peer;
 import tech.pegasys.teku.spec.Spec;
 import tech.pegasys.teku.spec.datastructures.blobs.versions.deneb.BlobSidecar;
 
 public class AbstractBlobSidecarsValidator {
 
   private static final Logger LOG = LogManager.getLogger();
 
-  protected final Spec spec;
-  protected final KZG kzg;
+  protected final Peer peer;
+
+  private final Spec spec;
+  private final KZG kzg;
 
-  public AbstractBlobSidecarsValidator(final Spec spec, final KZG kzg) {
+  public AbstractBlobSidecarsValidator(final Peer peer, final Spec spec, final KZG kzg) {
+    this.peer = peer;
     this.spec = spec;
     this.kzg = kzg;
   }
 
-  boolean verifyBlobSidecarKzg(final BlobSidecar blobSidecar) {
+  void verifyKzg(final BlobSidecar blobSidecar) {
+    if (!verifyBlobKzgProof(blobSidecar)) {
+      throw new BlobSidecarsResponseInvalidResponseException(
+          peer, BLOB_SIDECAR_KZG_VERIFICATION_FAILED);
+    }
+  }
+
+  private boolean verifyBlobKzgProof(final BlobSidecar blobSidecar) {
     try {
       return spec.atSlot(blobSidecar.getSlot()).miscHelpers().verifyBlobKzgProof(kzg, blobSidecar);
-    } catch (final KZGException ex) {
+    } catch (final Exception ex) {
       LOG.debug("KZG verification failed for BlobSidecar {}", blobSidecar.toLogString());
-      return false;
+      throw new BlobSidecarsResponseInvalidResponseException(
+          peer, BLOB_SIDECAR_KZG_VERIFICATION_FAILED, ex);
     }
   }
 }
diff --git a/...u/networking/eth2/rpc/beaconchain/methods/BlobSidecarsByRangeListenerValidatingProxy.java b/...u/networking/eth2/rpc/beaconchain/methods/BlobSidecarsByRangeListenerValidatingProxy.java
@@ -13,7 +13,6 @@
 
 package tech.pegasys.teku.networking.eth2.rpc.beaconchain.methods;
 
-import static tech.pegasys.teku.networking.eth2.rpc.beaconchain.methods.BlobSidecarsResponseInvalidResponseException.InvalidResponseType.BLOB_SIDECAR_KZG_VERIFICATION_FAILED;
 import static tech.pegasys.teku.networking.eth2.rpc.beaconchain.methods.BlobSidecarsResponseInvalidResponseException.InvalidResponseType.BLOB_SIDECAR_SLOT_NOT_IN_RANGE;
 import static tech.pegasys.teku.networking.eth2.rpc.beaconchain.methods.BlobSidecarsResponseInvalidResponseException.InvalidResponseType.BLOB_SIDECAR_UNEXPECTED_INDEX;
 import static tech.pegasys.teku.networking.eth2.rpc.beaconchain.methods.BlobSidecarsResponseInvalidResponseException.InvalidResponseType.BLOB_SIDECAR_UNEXPECTED_SLOT;
@@ -32,7 +31,6 @@
 public class BlobSidecarsByRangeListenerValidatingProxy extends AbstractBlobSidecarsValidator
     implements RpcResponseListener<BlobSidecar> {
 
-  private final Peer peer;
   private final RpcResponseListener<BlobSidecar> blobSidecarResponseListener;
   private final Integer maxBlobsPerBlock;
   private final UInt64 startSlot;
@@ -48,8 +46,7 @@ public BlobSidecarsByRangeListenerValidatingProxy(
       final KZG kzg,
       final UInt64 startSlot,
       final UInt64 count) {
-    super(spec, kzg);
-    this.peer = peer;
+    super(peer, spec, kzg);
     this.blobSidecarResponseListener = blobSidecarResponseListener;
     this.maxBlobsPerBlock = maxBlobsPerBlock;
     this.startSlot = startSlot;
@@ -74,10 +71,7 @@ public SafeFuture<?> onResponse(final BlobSidecar blobSidecar) {
           final BlobSidecarSummary blobSidecarSummary = BlobSidecarSummary.create(blobSidecar);
           verifyBlobSidecarIsAfterLast(blobSidecarSummary);
 
-          if (!verifyBlobSidecarKzg(blobSidecar)) {
-            throw new BlobSidecarsResponseInvalidResponseException(
-                peer, BLOB_SIDECAR_KZG_VERIFICATION_FAILED);
-          }
+          verifyKzg(blobSidecar);
 
           maybeLastBlobSidecarSummary = Optional.of(blobSidecarSummary);
           return blobSidecarResponseListener.onResponse(blobSidecar);

diff --git a/...ku/networking/eth2/rpc/beaconchain/methods/BlobSidecarsByRootListenerValidatingProxy.java b/...ku/networking/eth2/rpc/beaconchain/methods/BlobSidecarsByRootListenerValidatingProxy.java
@@ -15,7 +15,6 @@
 
 import java.util.HashSet;
 import java.util.List;
-import java.util.Set;
 import tech.pegasys.teku.infrastructure.async.SafeFuture;
 import tech.pegasys.teku.kzg.KZG;
 import tech.pegasys.teku.networking.p2p.peer.Peer;
@@ -24,35 +23,27 @@
 import tech.pegasys.teku.spec.datastructures.blobs.versions.deneb.BlobSidecar;
 import tech.pegasys.teku.spec.datastructures.networking.libp2p.rpc.BlobIdentifier;
 
-public class BlobSidecarsByRootListenerValidatingProxy implements RpcResponseListener<BlobSidecar> {
+public class BlobSidecarsByRootListenerValidatingProxy extends BlobSidecarsByRootValidator
+    implements RpcResponseListener<BlobSidecar> {
 
-  private final Peer peer;
-  private final Spec spec;
-  private final RpcResponseListener<BlobSidecar> blobSidecarResponseListener;
-  private final Set<BlobIdentifier> expectedBlobIdentifiers;
-  private final KZG kzg;
+  private final RpcResponseListener<BlobSidecar> delegate;
 
   public BlobSidecarsByRootListenerValidatingProxy(
       final Peer peer,
       final Spec spec,
-      final RpcResponseListener<BlobSidecar> blobSidecarResponseListener,
+      final RpcResponseListener<BlobSidecar> delegate,
       final KZG kzg,
       final List<BlobIdentifier> expectedBlobIdentifiers) {
-    this.peer = peer;
-    this.spec = spec;
-    this.blobSidecarResponseListener = blobSidecarResponseListener;
-    this.kzg = kzg;
-    this.expectedBlobIdentifiers = new HashSet<>(expectedBlobIdentifiers);
+    super(peer, spec, kzg, new HashSet<>(expectedBlobIdentifiers));
+    this.delegate = delegate;
   }
 
   @Override
   public SafeFuture<?> onResponse(final BlobSidecar blobSidecar) {
     return SafeFuture.of(
         () -> {
-          final BlobSidecarByRootValidator blobSidecarByRootValidator =
-              new BlobSidecarByRootValidator(peer, spec, kzg, expectedBlobIdentifiers);
-          blobSidecarByRootValidator.validateBlobSidecar(blobSidecar);
-          return blobSidecarResponseListener.onResponse(blobSidecar);
+          validate(blobSidecar);
+          return delegate.onResponse(blobSidecar);
         });
   }
 }
diff --git a/...n/methods/BlobSidecarByRootValidator.java → .../methods/BlobSidecarsByRootValidator.java b/...n/methods/BlobSidecarByRootValidator.java → .../methods/BlobSidecarsByRootValidator.java
@@ -13,42 +13,35 @@
 
 package tech.pegasys.teku.networking.eth2.rpc.beaconchain.methods;
 
-import static tech.pegasys.teku.networking.eth2.rpc.beaconchain.methods.BlobSidecarsResponseInvalidResponseException.InvalidResponseType.BLOB_SIDECAR_KZG_VERIFICATION_FAILED;
-import static tech.pegasys.teku.networking.eth2.rpc.beaconchain.methods.BlobSidecarsResponseInvalidResponseException.InvalidResponseType.BLOB_SIDECAR_UNEXPECTED_IDENTIFIER;
-
 import java.util.Set;
 import tech.pegasys.teku.kzg.KZG;
+import tech.pegasys.teku.networking.eth2.rpc.beaconchain.methods.BlobSidecarsResponseInvalidResponseException.InvalidResponseType;
 import tech.pegasys.teku.networking.p2p.peer.Peer;
 import tech.pegasys.teku.spec.Spec;
 import tech.pegasys.teku.spec.datastructures.blobs.versions.deneb.BlobSidecar;
 import tech.pegasys.teku.spec.datastructures.networking.libp2p.rpc.BlobIdentifier;
 
-public class BlobSidecarByRootValidator extends AbstractBlobSidecarsValidator {
+public class BlobSidecarsByRootValidator extends AbstractBlobSidecarsValidator {
 
-  private final Peer peer;
   private final Set<BlobIdentifier> expectedBlobIdentifiers;
 
-  public BlobSidecarByRootValidator(
+  public BlobSidecarsByRootValidator(
       final Peer peer,
       final Spec spec,
       final KZG kzg,
       final Set<BlobIdentifier> expectedBlobIdentifiers) {
-    super(spec, kzg);
-    this.peer = peer;
+    super(peer, spec, kzg);
     this.expectedBlobIdentifiers = expectedBlobIdentifiers;
   }
 
-  public void validateBlobSidecar(final BlobSidecar blobSidecar) {
+  public void validate(final BlobSidecar blobSidecar) {
     final BlobIdentifier blobIdentifier =
         new BlobIdentifier(blobSidecar.getBlockRoot(), blobSidecar.getIndex());
     if (!expectedBlobIdentifiers.contains(blobIdentifier)) {
       throw new BlobSidecarsResponseInvalidResponseException(
-          peer, BLOB_SIDECAR_UNEXPECTED_IDENTIFIER);
+          peer, InvalidResponseType.BLOB_SIDECAR_UNEXPECTED_IDENTIFIER);
     }
 
-    if (!verifyBlobSidecarKzg(blobSidecar)) {
-      throw new BlobSidecarsResponseInvalidResponseException(
-          peer, BLOB_SIDECAR_KZG_VERIFICATION_FAILED);
-    }
+    verifyKzg(blobSidecar);
   }
 }
diff --git a/...networking/eth2/rpc/beaconchain/methods/BlobSidecarsResponseInvalidResponseException.java b/...networking/eth2/rpc/beaconchain/methods/BlobSidecarsResponseInvalidResponseException.java
@@ -19,18 +19,22 @@
 public class BlobSidecarsResponseInvalidResponseException extends InvalidResponseException {
 
   public BlobSidecarsResponseInvalidResponseException(
-      Peer peer, InvalidResponseType invalidResponseType) {
+      final Peer peer, final InvalidResponseType invalidResponseType) {
     super(
         String.format(
             "Received invalid response from peer %s: %s", peer, invalidResponseType.describe()));
   }
 
-  public BlobSidecarsResponseInvalidResponseException(InvalidResponseType invalidResponseType) {
-    super("Received invalid response: " + invalidResponseType.describe());
+  public BlobSidecarsResponseInvalidResponseException(
+      final Peer peer, final InvalidResponseType invalidResponseType, final Exception cause) {
+    super(
+        String.format(
+            "Received invalid response from peer %s: %s", peer, invalidResponseType.describe()),
+        cause);
   }
 
   public enum InvalidResponseType {
-    BLOB_SIDECAR_KZG_VERIFICATION_FAILED("KZG verification for BlobSidecar is failed"),
+    BLOB_SIDECAR_KZG_VERIFICATION_FAILED("KZG verification for BlobSidecar has failed"),
     BLOB_SIDECAR_SLOT_NOT_IN_RANGE("BlobSidecar slot not in requested range"),
     BLOB_SIDECAR_UNEXPECTED_INDEX("BlobSidecar with unexpected index"),
     BLOB_SIDECAR_UNKNOWN_PARENT(

diff --git a/...etworking/eth2/rpc/beaconchain/methods/BlocksByRangeResponseInvalidResponseException.java b/...etworking/eth2/rpc/beaconchain/methods/BlocksByRangeResponseInvalidResponseException.java
@@ -19,13 +19,14 @@
 public class BlocksByRangeResponseInvalidResponseException extends InvalidResponseException {
 
   public BlocksByRangeResponseInvalidResponseException(
-      Peer peer, InvalidResponseType invalidResponseType) {
+      final Peer peer, final InvalidResponseType invalidResponseType) {
     super(
         String.format(
             "Received invalid response from peer %s: %s", peer, invalidResponseType.describe()));
   }
 
-  public BlocksByRangeResponseInvalidResponseException(InvalidResponseType invalidResponseType) {
+  public BlocksByRangeResponseInvalidResponseException(
+      final InvalidResponseType invalidResponseType) {
     super("Received invalid response: " + invalidResponseType.describe());
   }
 

diff --git a/...h2/src/main/java/tech/pegasys/teku/networking/eth2/rpc/core/InvalidResponseException.java b/...h2/src/main/java/tech/pegasys/teku/networking/eth2/rpc/core/InvalidResponseException.java
@@ -18,4 +18,8 @@ public class InvalidResponseException extends RuntimeException {
   public InvalidResponseException(final String message) {
     super(message);
   }
+
+  public InvalidResponseException(final String message, final Exception cause) {
+    super(message, cause);
+  }
 }
diff --git a/...thods/BlobSidecarByRootValidatorTest.java → ...hods/BlobSidecarsByRootValidatorTest.java b/...thods/BlobSidecarByRootValidatorTest.java → ...hods/BlobSidecarsByRootValidatorTest.java
@@ -32,10 +32,10 @@
 import tech.pegasys.teku.spec.datastructures.networking.libp2p.rpc.BlobIdentifier;
 import tech.pegasys.teku.spec.util.DataStructureUtil;
 
-public class BlobSidecarByRootValidatorTest {
+public class BlobSidecarsByRootValidatorTest {
   private final Spec spec = TestSpecFactory.createMainnetDeneb();
   private final DataStructureUtil dataStructureUtil = new DataStructureUtil(spec);
-  private BlobSidecarByRootValidator validator;
+  private BlobSidecarsByRootValidator validator;
   private final Eth2Peer peer = mock(Eth2Peer.class);
   private final KZG kzg = mock(KZG.class);
 
@@ -51,8 +51,8 @@ void blobSidecarIsCorrect() {
     final BlobSidecar blobSidecar1 =
         dataStructureUtil.randomBlobSidecar(UInt64.ONE, blockRoot1, Bytes32.ZERO, UInt64.ZERO);
 
-    validator = new BlobSidecarByRootValidator(peer, spec, kzg, Set.of(blobIdentifier1));
-    assertDoesNotThrow(() -> validator.validateBlobSidecar(blobSidecar1));
+    validator = new BlobSidecarsByRootValidator(peer, spec, kzg, Set.of(blobIdentifier1));
+    assertDoesNotThrow(() -> validator.validate(blobSidecar1));
   }
 
   @Test
@@ -63,8 +63,8 @@ void blobSidecarIdentifierNotRequested() {
     final BlobSidecar blobSidecar1 =
         dataStructureUtil.randomBlobSidecar(UInt64.ONE, blockRoot1, Bytes32.ZERO, UInt64.ZERO);
 
-    validator = new BlobSidecarByRootValidator(peer, spec, kzg, Set.of(blobIdentifier2));
-    assertThatThrownBy(() -> validator.validateBlobSidecar(blobSidecar1))
+    validator = new BlobSidecarsByRootValidator(peer, spec, kzg, Set.of(blobIdentifier2));
+    assertThatThrownBy(() -> validator.validate(blobSidecar1))
         .isExactlyInstanceOf(BlobSidecarsResponseInvalidResponseException.class)
         .hasMessageContaining(
             BlobSidecarsResponseInvalidResponseException.InvalidResponseType
@@ -80,8 +80,8 @@ void blobSidecarFailsKzgVerification() {
     final BlobSidecar blobSidecar1 =
         dataStructureUtil.randomBlobSidecar(UInt64.ONE, blockRoot1, Bytes32.ZERO, UInt64.ZERO);
 
-    validator = new BlobSidecarByRootValidator(peer, spec, kzg, Set.of(blobIdentifier1));
-    assertThatThrownBy(() -> validator.validateBlobSidecar(blobSidecar1))
+    validator = new BlobSidecarsByRootValidator(peer, spec, kzg, Set.of(blobIdentifier1));
+    assertThatThrownBy(() -> validator.validate(blobSidecar1))
         .isExactlyInstanceOf(BlobSidecarsResponseInvalidResponseException.class)
         .hasMessageContaining(
             BlobSidecarsResponseInvalidResponseException.InvalidResponseType