Merge branch 'main' into validate-null-parameters-v3

Gabriel-Trintinalia · Aug 31, 2023 · bed05f4 · bed05f4
2 parents 7832d21 + e3b0112
commit bed05f4
Show file tree

Hide file tree

Showing 6 changed files with 71 additions and 15 deletions.
diff --git a/datatypes/src/main/java/org/hyperledger/besu/datatypes/BlobsWithCommitments.java b/datatypes/src/main/java/org/hyperledger/besu/datatypes/BlobsWithCommitments.java
@@ -38,11 +38,15 @@ public BlobsWithCommitments(
       final List<Blob> blobs,
       final List<KZGProof> kzgProofs,
       final List<VersionedHash> versionedHashes) {
+    if (blobs.size() == 0) {
+      throw new InvalidParameterException(
+          "There needs to be a minimum of one blob in a blob transaction with commitments");
+    }
     if (blobs.size() != kzgCommitments.size()
         || blobs.size() != kzgProofs.size()
-        || kzgCommitments.size() != versionedHashes.size()) {
+        || blobs.size() != versionedHashes.size()) {
       throw new InvalidParameterException(
-          "There must be an equal number of blobs, commitments and proofs");
+          "There must be an equal number of blobs, commitments, proofs, and versioned hashes");
     }
     this.kzgCommitments = kzgCommitments;
     this.blobs = blobs;

diff --git a/datatypes/src/test/java/org/hyperledger/besu/datatypes/BlobsWithCommitmentsTest.java b/datatypes/src/test/java/org/hyperledger/besu/datatypes/BlobsWithCommitmentsTest.java
@@ -22,19 +22,70 @@
 import java.util.List;
 
 import org.apache.tuweni.bytes.Bytes;
+import org.apache.tuweni.bytes.Bytes32;
 import org.junit.jupiter.api.Test;
 
 public class BlobsWithCommitmentsTest {
   @Test
-  public void blobsWithCommitmentsMustHaveSameNumberOfElements() {
+  public void blobsWithCommitmentsMustHaveAtLeastOneBlob() {
+    String actualMessage =
+        assertThrows(
+                InvalidParameterException.class,
+                () -> new BlobsWithCommitments(List.of(), List.of(), List.of(), List.of()))
+            .getMessage();
+    final String expectedMessage =
+        "There needs to be a minimum of one blob in a blob transaction with commitments";
+    assertThat(actualMessage).isEqualTo(expectedMessage);
+  }
+
+  @Test
+  public void blobsWithCommitmentsMustHaveSameNumberOfElementsVersionedHashes() {
+    String actualMessage =
+        assertThrows(
+                InvalidParameterException.class,
+                () ->
+                    new BlobsWithCommitments(
+                        List.of(new KZGCommitment(Bytes.of(1))),
+                        List.of(new Blob(Bytes.EMPTY)),
+                        List.of(new KZGProof(Bytes.EMPTY)),
+                        List.of()))
+            .getMessage();
+    final String expectedMessage =
+        "There must be an equal number of blobs, commitments, proofs, and versioned hashes";
+    assertThat(actualMessage).isEqualTo(expectedMessage);
+  }
+
+  @Test
+  public void blobsWithCommitmentsMustHaveSameNumberOfElementsKZGCommitment() {
+    String actualMessage =
+        assertThrows(
+                InvalidParameterException.class,
+                () ->
+                    new BlobsWithCommitments(
+                        List.of(),
+                        List.of(new Blob(Bytes.EMPTY)),
+                        List.of(new KZGProof(Bytes.EMPTY)),
+                        List.of(new VersionedHash(Bytes32.rightPad(Bytes.fromHexString("0x01"))))))
+            .getMessage();
+    final String expectedMessage =
+        "There must be an equal number of blobs, commitments, proofs, and versioned hashes";
+    assertThat(actualMessage).isEqualTo(expectedMessage);
+  }
+
+  @Test
+  public void blobsWithCommitmentsMustHaveSameNumberOfElementsKZGProof() {
     String actualMessage =
         assertThrows(
                 InvalidParameterException.class,
                 () ->
                     new BlobsWithCommitments(
-                        List.of(new KZGCommitment(Bytes.of(1))), List.of(), List.of(), List.of()))
+                        List.of(new KZGCommitment(Bytes.of(1))),
+                        List.of(new Blob(Bytes.EMPTY)),
+                        List.of(),
+                        List.of(new VersionedHash(Bytes32.rightPad(Bytes.fromHexString("0x01"))))))
             .getMessage();
-    final String expectedMessage = "There must be an equal number of blobs, commitments and proofs";
+    final String expectedMessage =
+        "There must be an equal number of blobs, commitments, proofs, and versioned hashes";
     assertThat(actualMessage).isEqualTo(expectedMessage);
   }
 }
diff --git a/...perledger/besu/ethereum/api/jsonrpc/internal/methods/engine/AbstractEngineNewPayload.java b/...perledger/besu/ethereum/api/jsonrpc/internal/methods/engine/AbstractEngineNewPayload.java
@@ -204,7 +204,7 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext)
             blockParam.getPrevRandao(),
             0,
             maybeWithdrawals.map(BodyValidation::withdrawalsRoot).orElse(null),
-            blockParam.getBlobGasUsed() == null ? null : blockParam.getBlobGasUsed(),
+            blockParam.getBlobGasUsed(),
             blockParam.getExcessBlobGas() == null
                 ? null
                 : BlobGas.fromHexString(blockParam.getExcessBlobGas()),

diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/Transaction.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/Transaction.java
@@ -189,14 +189,15 @@ public Transaction(
     if (versionedHashes.isPresent() || maxFeePerBlobGas.isPresent()) {
       checkArgument(
           transactionType.supportsBlob(),
-          "Must not specify blob versioned hashes of max fee per blob gas for transaction not supporting it");
+          "Must not specify blob versioned hashes or max fee per blob gas for transaction not supporting it");
     }
 
     if (transactionType.supportsBlob()) {
       checkArgument(
           versionedHashes.isPresent(), "Must specify blob versioned hashes for blob transaction");
       checkArgument(
-          !versionedHashes.get().isEmpty(), "Blob transaction must have at least one blob");
+          !versionedHashes.get().isEmpty(),
+          "Blob transaction must have at least one versioned hash");
       checkArgument(
           maxFeePerBlobGas.isPresent(), "Must specify max fee per blob gas for blob transaction");
     }
@@ -680,13 +681,13 @@ private void memoizeHashAndSize() {
 
     if (transactionType.supportsBlob()) {
       if (getBlobsWithCommitments().isPresent()) {
-        size = TransactionEncoder.encodeOpaqueBytes(this).size();
+        size = bytes.size();
+        return;
       }
-    } else {
-      final BytesValueRLPOutput rlpOutput = new BytesValueRLPOutput();
-      TransactionEncoder.encodeForWire(transactionType, bytes, rlpOutput);
-      size = rlpOutput.encodedSize();
     }
+    final BytesValueRLPOutput rlpOutput = new BytesValueRLPOutput();
+    TransactionEncoder.encodeForWire(transactionType, bytes, rlpOutput);
+    size = rlpOutput.encodedSize();
   }
 
   /**

diff --git a/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/core/TransactionBuilderTest.java b/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/core/TransactionBuilderTest.java
@@ -73,7 +73,7 @@ public void zeroBlobTransactionIsInvalid() {
           .createTransaction(senderKeys);
       fail();
     } catch (IllegalArgumentException iea) {
-      assertThat(iea).hasMessage("Blob transaction must have at least one blob");
+      assertThat(iea).hasMessage("Blob transaction must have at least one versioned hash");
     }
   }
 }
diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/RespondingEthPeer.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/RespondingEthPeer.java
@@ -307,7 +307,7 @@ public static Responder partialResponder(
       final TransactionPool transactionPool,
       final ProtocolSchedule protocolSchedule,
       final float portion) {
-    checkArgument(portion >= 0.0 && portion <= 1.0, "Portion is in the range [0.0..1.0]");
+    checkArgument(portion >= 0.0 && portion <= 1.0, "Portion is not in the range [0.0..1.0]");
 
     final Responder fullResponder =
         blockchainResponder(blockchain, worldStateArchive, transactionPool);