Ban zero blob transactions (#5425)

* Ban zero blob transactions

Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>

* Update CHANGELOG

Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>

* Fix tests

Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>

* Update ethereum/core/src/test/java/org/hyperledger/besu/ethereum/mainnet/MainnetTransactionValidatorTest.java

Co-authored-by: Sally MacFarlane <macfarla.github@gmail.com>
Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>

* Update ethereum/core/src/test/java/org/hyperledger/besu/ethereum/mainnet/MainnetTransactionValidatorTest.java

Co-authored-by: Sally MacFarlane <macfarla.github@gmail.com>
Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>

---------

Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
Co-authored-by: Stefan Pingel <16143240+pinges@users.noreply.github.com>
Co-authored-by: Sally MacFarlane <macfarla.github@gmail.com>

CHANGELOG.md

@@ -6,6 +6,7 @@
 
 ### Additions and Improvements
 - "Big-EOF" (the EOF version initially slotted for Shanghai) has been moved from Cancun to FutureEIPs [#5429](https://github.com/hyperledger/besu/pull/5429)
+- EIP-4844: Zero blob transactions are invalid [#5425](https://github.com/hyperledger/besu/pull/5425)
 
 ### Bug Fixes
 - Fix eth_feeHistory response for the case in which blockCount is higher than highestBlock requested. [#5397](https://github.com/hyperledger/besu/pull/5397)

ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/results/TransactionCompleteResultTest.java

@@ -41,6 +41,7 @@ public class TransactionCompleteResultTest {
         new TransactionCompleteResult(
             new TransactionWithMetadata(
                 new TransactionTestFixture()
+                    .type(TransactionType.EIP1559)
                     .maxFeePerGas(Optional.of(Wei.ONE))
                     .maxPriorityFeePerGas(Optional.of(Wei.ZERO))
                     .createTransaction(gen.generateKeyPair()),

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/Transaction.java

@@ -186,6 +186,8 @@ public class Transaction
     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");
       checkArgument(
           maxFeePerDataGas.isPresent(), "Must specify max fee per data gas for blob transaction");
     }

ethereum/core/src/test-support/java/org/hyperledger/besu/ethereum/core/TransactionTestFixture.java

@@ -16,21 +16,29 @@ package org.hyperledger.besu.ethereum.core;
 
 import org.hyperledger.besu.crypto.KeyPair;
 import org.hyperledger.besu.datatypes.Address;
+import org.hyperledger.besu.datatypes.Hash;
 import org.hyperledger.besu.datatypes.Wei;
+import org.hyperledger.besu.evm.AccessListEntry;
 import org.hyperledger.besu.plugin.data.TransactionType;
 
 import java.math.BigInteger;
+import java.util.List;
 import java.util.Optional;
 
 import org.apache.tuweni.bytes.Bytes;
+import org.apache.tuweni.bytes.Bytes32;
 
 public class TransactionTestFixture {
+  private static final Hash DEFAULT_VERSIONED_HASH =
+      Hash.wrap(
+          Bytes32.wrap(
+              Bytes.concatenate(Bytes.fromHexString("0x01"), Bytes.repeat((byte) 42, 31))));
 
   private TransactionType transactionType = TransactionType.FRONTIER;
 
   private long nonce = 0;
 
-  private Wei gasPrice = Wei.of(5000);
+  private Optional<Wei> gasPrice = Optional.empty();
 
   private long gasLimit = 5000;
 
@@ -45,6 +53,10 @@ public class TransactionTestFixture {
 
   private Optional<Wei> maxPriorityFeePerGas = Optional.empty();
   private Optional<Wei> maxFeePerGas = Optional.empty();
+  private Optional<Wei> maxFeePerDataGas = Optional.empty();
+
+  private Optional<List<AccessListEntry>> accessListEntries = Optional.empty();
+  private Optional<List<Hash>> versionedHashes = Optional.empty();
 
   private Optional<BigInteger> v = Optional.empty();
 
@@ -53,18 +65,35 @@ public class TransactionTestFixture {
     builder
         .type(transactionType)
         .gasLimit(gasLimit)
-        .gasPrice(gasPrice)
         .nonce(nonce)
         .payload(payload)
         .value(value)
         .sender(sender);
 
+    switch (transactionType) {
+      case FRONTIER:
+        builder.gasPrice(gasPrice.orElse(Wei.of(5000)));
+        break;
+      case ACCESS_LIST:
+        builder.gasPrice(gasPrice.orElse(Wei.of(5000)));
+        builder.accessList(accessListEntries.orElse(List.of()));
+        break;
+      case EIP1559:
+        builder.maxPriorityFeePerGas(maxPriorityFeePerGas.orElse(Wei.of(500)));
+        builder.maxFeePerGas(maxFeePerGas.orElse(Wei.of(5000)));
+        builder.accessList(accessListEntries.orElse(List.of()));
+        break;
+      case BLOB:
+        builder.maxPriorityFeePerGas(maxPriorityFeePerGas.orElse(Wei.of(500)));
+        builder.maxFeePerGas(maxFeePerGas.orElse(Wei.of(5000)));
+        builder.accessList(accessListEntries.orElse(List.of()));
+        builder.maxFeePerDataGas(maxFeePerDataGas.orElse(Wei.ONE));
+        builder.versionedHashes(versionedHashes.orElse(List.of(DEFAULT_VERSIONED_HASH)));
+        break;
+    }
+
     to.ifPresent(builder::to);
     chainId.ifPresent(builder::chainId);
-
-    maxPriorityFeePerGas.ifPresent(builder::maxPriorityFeePerGas);
-    maxFeePerGas.ifPresent(builder::maxFeePerGas);
-
     v.ifPresent(builder::v);
 
     return builder.signAndBuild(keys);
@@ -81,7 +110,7 @@ public class TransactionTestFixture {
   }
 
   public TransactionTestFixture gasPrice(final Wei gasPrice) {
-    this.gasPrice = gasPrice;
+    this.gasPrice = Optional.ofNullable(gasPrice);
     return this;
   }
 
@@ -125,6 +154,21 @@ public class TransactionTestFixture {
     return this;
   }
 
+  public TransactionTestFixture maxFeePerDataGas(final Optional<Wei> maxFeePerDataGas) {
+    this.maxFeePerDataGas = maxFeePerDataGas;
+    return this;
+  }
+
+  public TransactionTestFixture accessList(final List<AccessListEntry> accessListEntries) {
+    this.accessListEntries = Optional.ofNullable(accessListEntries);
+    return this;
+  }
+
+  public TransactionTestFixture versionedHashes(final List<Hash> versionedHashes) {
+    this.versionedHashes = Optional.ofNullable(versionedHashes);
+    return this;
+  }
+
   public TransactionTestFixture v(final Optional<BigInteger> v) {
     this.v = v;
     return this;

ethereum/core/src/test/java/org/hyperledger/besu/ethereum/core/TransactionBuilderTest.java

@@ -17,18 +17,29 @@ package org.hyperledger.besu.ethereum.core;
 
 import static java.util.stream.Collectors.toUnmodifiableSet;
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.jupiter.api.Assertions.fail;
 
+import org.hyperledger.besu.crypto.KeyPair;
+import org.hyperledger.besu.crypto.SignatureAlgorithm;
+import org.hyperledger.besu.crypto.SignatureAlgorithmFactory;
 import org.hyperledger.besu.datatypes.Wei;
 import org.hyperledger.besu.evm.AccessListEntry;
 import org.hyperledger.besu.plugin.data.TransactionType;
 
+import java.math.BigInteger;
 import java.util.List;
+import java.util.Optional;
 import java.util.Set;
+import java.util.function.Supplier;
 import java.util.stream.Stream;
 
+import com.google.common.base.Suppliers;
 import org.junit.Test;
 
 public class TransactionBuilderTest {
+  private static final Supplier<SignatureAlgorithm> SIGNATURE_ALGORITHM =
+      Suppliers.memoize(SignatureAlgorithmFactory::getInstance);
+  private static final KeyPair senderKeys = SIGNATURE_ALGORITHM.get().generateKeyPair();
 
   @Test
   public void guessTypeCanGuessAllTypes() {
@@ -50,4 +61,18 @@ public class TransactionBuilderTest {
               TransactionType.FRONTIER, TransactionType.ACCESS_LIST, TransactionType.EIP1559
             });
   }
+
+  @Test
+  public void zeroBlobTransactionIsInvalid() {
+    try {
+      new TransactionTestFixture()
+          .type(TransactionType.BLOB)
+          .chainId(Optional.of(BigInteger.ONE))
+          .versionedHashes(List.of())
+          .createTransaction(senderKeys);
+      fail();
+    } catch (IllegalArgumentException iea) {
+      assertThat(iea).hasMessage("Blob transaction must have at least one blob");
+    }
+  }
 }

ethereum/core/src/test/java/org/hyperledger/besu/ethereum/mainnet/MainnetTransactionValidatorTest.java

@@ -504,6 +504,29 @@ public class MainnetTransactionValidatorTest {
         .isEqualTo("Initcode size of 49153 exceeds maximum size of 49152");
   }
 
+  @Test
+  public void shouldAcceptTransactionWithAtLeastOneBlob() {
+    final MainnetTransactionValidator validator =
+        new MainnetTransactionValidator(
+            gasCalculator,
+            GasLimitCalculator.constant(),
+            FeeMarket.london(0L),
+            false,
+            Optional.of(BigInteger.ONE),
+            Set.of(TransactionType.FRONTIER, TransactionType.EIP1559, TransactionType.BLOB),
+            0xc000);
+
+    var blobTx =
+        new TransactionTestFixture()
+            .type(TransactionType.BLOB)
+            .chainId(Optional.of(BigInteger.ONE))
+            .createTransaction(senderKeys);
+    var validationResult =
+        validator.validate(blobTx, Optional.empty(), transactionValidationParams);
+
+    assertThat(validationResult.isValid()).isTrue();
+  }
+
   private Account accountWithNonce(final long nonce) {
     return account(basicTransaction.getUpfrontCost(0L), nonce);
   }