Fix ssz optional bounds (Consensys#8484)

zilm13 · Jul 31, 2024 · cd549d5 · cd549d5
1 parent ab99e2d
commit cd549d5
Show file tree

Hide file tree

Showing 3 changed files with 26 additions and 11 deletions.
diff --git a/...src/main/java/tech/pegasys/teku/infrastructure/ssz/schema/impl/SszOptionalSchemaImpl.java b/...src/main/java/tech/pegasys/teku/infrastructure/ssz/schema/impl/SszOptionalSchemaImpl.java
@@ -250,7 +250,7 @@ public SszLengthBounds getSszLengthBounds() {
   }
 
   private SszLengthBounds calcSszLengthBounds() {
-    return childSchema.getSszLengthBounds().addBytes(PREFIX_SIZE_BYTES).ceilToBytes();
+    return SszLengthBounds.ZERO.or(childSchema.getSszLengthBounds().addBytes(PREFIX_SIZE_BYTES));
   }
 
   @Override

diff --git a/.../ssz/src/test/java/tech/pegasys/teku/infrastructure/ssz/schema/SszOptionalSchemaTest.java b/.../ssz/src/test/java/tech/pegasys/teku/infrastructure/ssz/schema/SszOptionalSchemaTest.java
@@ -21,6 +21,7 @@
 import java.util.stream.Stream;
 import org.apache.tuweni.bytes.Bytes;
 import org.apache.tuweni.units.bigints.UInt256;
+import org.junit.jupiter.api.Test;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.Arguments;
 import org.junit.jupiter.params.provider.MethodSource;
@@ -54,6 +55,18 @@ public static Stream<SszSchema<?>> testContainerOptionalSchemas() {
     return Stream.of(ContainerWithOptionals.SSZ_SCHEMA);
   }
 
+  @Test
+  void verifySszLengthBounds() {
+    assertThat(SszOptionalSchema.create(SszPrimitiveSchemas.BIT_SCHEMA).getSszLengthBounds())
+        .matches(bound -> bound.getMinBytes() == 0)
+        .matches(bound -> bound.getMaxBytes() == 2)
+        .matches(bound -> bound.getMaxBits() == 9);
+
+    assertThat(SszOptionalSchema.create(SszPrimitiveSchemas.BYTES32_SCHEMA).getSszLengthBounds())
+        .matches(bound -> bound.getMinBytes() == 0)
+        .matches(bound -> bound.getMaxBytes() == 33);
+  }
+
   @Override
   public Stream<? extends SszSchema<?>> testSchemas() {
     return Streams.concat(testOptionalSchemas(), testContainerOptionalSchemas());

diff --git a/...ture/ssz/src/test/java/tech/pegasys/teku/infrastructure/ssz/schema/SszSchemaTestBase.java b/...ture/ssz/src/test/java/tech/pegasys/teku/infrastructure/ssz/schema/SszSchemaTestBase.java
@@ -25,6 +25,7 @@
 import tech.pegasys.teku.infrastructure.ssz.RandomSszDataGenerator;
 import tech.pegasys.teku.infrastructure.ssz.SszData;
 import tech.pegasys.teku.infrastructure.ssz.SszDataAssert;
+import tech.pegasys.teku.infrastructure.ssz.SszOptional;
 import tech.pegasys.teku.infrastructure.ssz.schema.SszSchemaHints.SszSuperNodeHint;
 import tech.pegasys.teku.infrastructure.ssz.schema.impl.AbstractSszCollectionSchema;
 import tech.pegasys.teku.infrastructure.ssz.sos.SimpleSszReader;
@@ -44,25 +45,26 @@ void getDefaultTree_shouldBeEqualToDefaultStructure(SszSchema<SszData> schema) {
 
   @MethodSource("testSchemaArguments")
   @ParameterizedTest
-  void sszDeserialize_tooLongSszShouldFailFastWithoutReadingWholeInput(SszSchema<SszData> schema) {
-    if (schema instanceof SszOptionalSchema<?, ?>) {
-      // empty SszOptional couldn't pass this test
-      return;
-    }
+  void sszDeserialize_tooLongSszShouldFailFastWithoutReadingWholeInput(
+      final SszSchema<SszData> schema) {
     long maxSszLength = schema.getSszLengthBounds().getMaxBytes();
     // ignore too large and degenerative structs
     assumeThat(maxSszLength).isLessThan(32 * 1024 * 1024).isGreaterThan(0);
     // ignore lists using SszSuperNode as many validations are skipped
-    if (schema instanceof AbstractSszCollectionSchema) {
-      assumeThat(
-              ((AbstractSszCollectionSchema<?, ?>) schema)
-                  .getHints()
-                  .getHint(SszSuperNodeHint.class))
+    if (schema instanceof AbstractSszCollectionSchema<?, ?> collectionSchema) {
+      assumeThat(collectionSchema.getHints().getHint(SszSuperNodeHint.class))
           .describedAs("uses SszSuperNode")
           .isEmpty();
     }
 
     SszData data = randomSsz.randomData(schema);
+
+    if (data instanceof SszOptional<?> optionalData) {
+      assumeThat(optionalData.getValue())
+          .describedAs("optional can't be empty to pass the test")
+          .isPresent();
+    }
+
     Bytes ssz = data.sszSerialize();
 
     Bytes sszWithExtraData = Bytes.wrap(ssz, Bytes.random((int) (maxSszLength - ssz.size() + 1)));