Refactor some transaction calculations (#6150)

Refactor some transaction calculations, including moving upfront
overflow checks from the constructor to the validator and optimizing
some RLP calculations. Also fix all lint errors.

Signed-off-by: Danno Ferrin <danno.ferrin@swirldslabs.com>

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

@@ -99,7 +99,7 @@ public class Transaction
   private final Optional<BigInteger> chainId;
 
   // Caches a "hash" of a portion of the transaction used for sender recovery.
-  // Note that this hash does not include the transaction signature so it does not
+  // Note that this hash does not include the transaction signature, so it does not
   // fully identify the transaction (use the result of the {@code hash()} for that).
   // It is only used to compute said signature and recover the sender from it.
   private volatile Bytes32 hashNoSignature;
@@ -226,10 +226,6 @@ public class Transaction
     this.chainId = chainId;
     this.versionedHashes = versionedHashes;
     this.blobsWithCommitments = blobsWithCommitments;
-
-    if (!forCopy && isUpfrontGasCostTooHigh()) {
-      throw new IllegalArgumentException("Upfront gas cost exceeds UInt256");
-    }
   }
 
   /**
@@ -566,15 +562,6 @@ public class Transaction
         getMaxGasPrice(), getMaxFeePerBlobGas().orElse(Wei.ZERO), blobGasPerBlock);
   }
 
-  /**
-   * Check if the upfront gas cost is over the max allowed
-   *
-   * @return true is upfront data cost overflow uint256 max value
-   */
-  private boolean isUpfrontGasCostTooHigh() {
-    return calculateUpfrontGasCost(getMaxGasPrice(), Wei.ZERO, 0L).bitLength() > 256;
-  }
-
   /**
    * Calculates the up-front cost for the gas and blob gas the transaction can use.
    *
@@ -597,7 +584,7 @@ public class Transaction
     }
   }
 
-  private BigInteger calculateUpfrontGasCost(
+  public BigInteger calculateUpfrontGasCost(
       final Wei gasPrice, final Wei blobGasPrice, final long totalBlobGas) {
     var cost =
         new BigInteger(1, Longs.toByteArray(getGasLimit())).multiply(gasPrice.getAsBigInteger());
@@ -619,7 +606,9 @@ public class Transaction
    * @return the up-front gas cost for the transaction
    */
   public Wei getUpfrontCost(final long totalBlobGas) {
-    return getMaxUpfrontGasCost(totalBlobGas).addExact(getValue());
+    Wei maxUpfrontGasCost = getMaxUpfrontGasCost(totalBlobGas);
+    Wei result = maxUpfrontGasCost.add(getValue());
+    return (maxUpfrontGasCost.compareTo(result) > 0) ? Wei.MAX_WEI : result;
   }
 
   /**

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/encoding/AccessListTransactionDecoder.java

@@ -32,6 +32,10 @@ class AccessListTransactionDecoder {
   private static final Supplier<SignatureAlgorithm> SIGNATURE_ALGORITHM =
       Suppliers.memoize(SignatureAlgorithmFactory::getInstance);
 
+  private AccessListTransactionDecoder() {
+    // private constructor
+  }
+
   public static Transaction decode(final RLPInput rlpInput) {
     rlpInput.enterList();
     final Transaction.Builder preSignatureTransactionBuilder =
@@ -43,7 +47,7 @@ class AccessListTransactionDecoder {
             .gasLimit(rlpInput.readLongScalar())
             .to(
                 rlpInput.readBytes(
-                    addressBytes -> addressBytes.size() == 0 ? null : Address.wrap(addressBytes)))
+                    addressBytes -> addressBytes.isEmpty() ? null : Address.wrap(addressBytes)))
             .value(Wei.of(rlpInput.readUInt256Scalar()))
             .payload(rlpInput.readBytes())
             .accessList(
@@ -57,7 +61,7 @@ class AccessListTransactionDecoder {
                       accessListEntryRLPInput.leaveList();
                       return accessListEntry;
                     }));
-    final byte recId = (byte) rlpInput.readIntScalar();
+    final byte recId = (byte) rlpInput.readUnsignedByteScalar();
     final Transaction transaction =
         preSignatureTransactionBuilder
             .signature(

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/encoding/BlobTransactionDecoder.java

@@ -32,6 +32,10 @@ public class BlobTransactionDecoder {
   private static final Supplier<SignatureAlgorithm> SIGNATURE_ALGORITHM =
       Suppliers.memoize(SignatureAlgorithmFactory::getInstance);
 
+  private BlobTransactionDecoder() {
+    // private constructor
+  }
+
   /**
    * Decodes a blob transaction from the provided RLP input.
    *
@@ -68,7 +72,7 @@ public class BlobTransactionDecoder {
         .maxPriorityFeePerGas(Wei.of(input.readUInt256Scalar()))
         .maxFeePerGas(Wei.of(input.readUInt256Scalar()))
         .gasLimit(input.readLongScalar())
-        .to(input.readBytes(v -> v.size() == 0 ? null : Address.wrap(v)))
+        .to(input.readBytes(v -> v.isEmpty() ? null : Address.wrap(v)))
         .value(Wei.of(input.readUInt256Scalar()))
         .payload(input.readBytes())
         .accessList(
@@ -86,7 +90,7 @@ public class BlobTransactionDecoder {
         .versionedHashes(
             input.readList(versionedHashes -> new VersionedHash(versionedHashes.readBytes32())));
 
-    final byte recId = (byte) input.readIntScalar();
+    final byte recId = (byte) input.readUnsignedByteScalar();
     builder.signature(
         SIGNATURE_ALGORITHM
             .get()

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/encoding/EIP1559TransactionDecoder.java

@@ -32,6 +32,10 @@ public class EIP1559TransactionDecoder {
   private static final Supplier<SignatureAlgorithm> SIGNATURE_ALGORITHM =
       Suppliers.memoize(SignatureAlgorithmFactory::getInstance);
 
+  private EIP1559TransactionDecoder() {
+    // private constructor
+  }
+
   public static Transaction decode(final RLPInput input) {
     input.enterList();
     final BigInteger chainId = input.readBigIntegerScalar();
@@ -43,7 +47,7 @@ public class EIP1559TransactionDecoder {
             .maxPriorityFeePerGas(Wei.of(input.readUInt256Scalar()))
             .maxFeePerGas(Wei.of(input.readUInt256Scalar()))
             .gasLimit(input.readLongScalar())
-            .to(input.readBytes(v -> v.size() == 0 ? null : Address.wrap(v)))
+            .to(input.readBytes(v -> v.isEmpty() ? null : Address.wrap(v)))
             .value(Wei.of(input.readUInt256Scalar()))
             .payload(input.readBytes())
             .accessList(
@@ -57,7 +61,7 @@ public class EIP1559TransactionDecoder {
                       accessListEntryRLPInput.leaveList();
                       return accessListEntry;
                     }));
-    final byte recId = (byte) input.readIntScalar();
+    final byte recId = (byte) input.readUnsignedByteScalar();
     final Transaction transaction =
         builder
             .signature(

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/BlockHeaderValidator.java

@@ -41,31 +41,24 @@ public class BlockHeaderValidator {
       final BlockHeader parent,
       final ProtocolContext protocolContext,
       final HeaderValidationMode mode) {
-    switch (mode) {
-      case NONE:
-        return true;
-      case LIGHT_DETACHED_ONLY:
-        return applyRules(
-            header,
-            parent,
-            protocolContext,
-            rule -> rule.includeInLightValidation() && rule.isDetachedSupported());
-      case LIGHT_SKIP_DETACHED:
-        return applyRules(
-            header,
-            parent,
-            protocolContext,
-            rule -> rule.includeInLightValidation() && !rule.isDetachedSupported());
-      case LIGHT:
-        return applyRules(header, parent, protocolContext, Rule::includeInLightValidation);
-      case DETACHED_ONLY:
-        return applyRules(header, parent, protocolContext, Rule::isDetachedSupported);
-      case SKIP_DETACHED:
-        return applyRules(header, parent, protocolContext, rule -> !rule.isDetachedSupported());
-      case FULL:
-        return applyRules(header, parent, protocolContext, rule -> true);
-    }
-    throw new IllegalArgumentException("Unknown HeaderValidationMode: " + mode);
+    return switch (mode) {
+      case NONE -> true;
+      case LIGHT_DETACHED_ONLY -> applyRules(
+          header,
+          parent,
+          protocolContext,
+          rule -> rule.includeInLightValidation() && rule.isDetachedSupported());
+      case LIGHT_SKIP_DETACHED -> applyRules(
+          header,
+          parent,
+          protocolContext,
+          rule -> rule.includeInLightValidation() && !rule.isDetachedSupported());
+      case LIGHT -> applyRules(header, parent, protocolContext, Rule::includeInLightValidation);
+      case DETACHED_ONLY -> applyRules(header, parent, protocolContext, Rule::isDetachedSupported);
+      case SKIP_DETACHED -> applyRules(
+          header, parent, protocolContext, rule -> !rule.isDetachedSupported());
+      case FULL -> applyRules(header, parent, protocolContext, rule -> true);
+    };
   }
 
   public boolean validateHeader(
@@ -90,7 +83,9 @@ public class BlockHeaderValidator {
         .allMatch(
             rule -> {
               boolean worked = rule.validate(header, parent, protocolContext);
-              if (!worked) LOG.debug("{} rule failed", rule.innerRuleClass().getCanonicalName());
+              if (!worked) {
+                LOG.debug("{} rule failed", rule.innerRuleClass().getCanonicalName());
+              }
               return worked;
             });
   }

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/MainnetTransactionValidator.java

@@ -182,6 +182,13 @@ public class MainnetTransactionValidator implements TransactionValidator {
               intrinsicGasCost, transaction.getGasLimit()));
     }
 
+    if (transaction.calculateUpfrontGasCost(transaction.getMaxGasPrice(), Wei.ZERO, 0).bitLength()
+        > 256) {
+      return ValidationResult.invalid(
+          TransactionInvalidReason.UPFRONT_COST_EXCEEDS_UINT256,
+          "Upfront gas cost cannot exceed 2^256 Wei");
+    }
+
     return ValidationResult.valid();
   }
 

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/transaction/TransactionInvalidReason.java

@@ -23,6 +23,7 @@ public enum TransactionInvalidReason {
   REPLAY_PROTECTED_SIGNATURE_REQUIRED,
   INVALID_SIGNATURE,
   UPFRONT_COST_EXCEEDS_BALANCE,
+  UPFRONT_COST_EXCEEDS_UINT256,
   NONCE_TOO_LOW,
   NONCE_TOO_HIGH,
   NONCE_OVERFLOW,

ethereum/core/src/test/java/org/hyperledger/besu/ethereum/core/encoding/TransactionRLPDecoderTest.java

@@ -86,8 +86,18 @@ class TransactionRLPDecoderTest {
           {EIP1559_TX_RLP, "EIP1559_TX_RLP", true},
           {NONCE_64_BIT_MAX_MINUS_2_TX_RLP, "NONCE_64_BIT_MAX_MINUS_2_TX_RLP", true},
           {
-            "01f89a0130308263309430303030303030303030303030303030303030303030f838f7943030303030303030303030303030303030303030e0a0303030303030303030303030303030303030303030303030303030303030303001a03130303130303031313031313031303130303030323030323030323030313030a03030303030303030303030303030303030303030303030303030303030303030",
-            "EIP1559 list too small",
+            "b89d01f89a0130308263309430303030303030303030303030303030303030303030f838f7943030303030303030303030303030303030303030e0a0303030303030303030303030303030303030303030303030303030303030303001a03130303130303031313031313031303130303030323030323030323030313030a03030303030303030303030303030303030303030303030303030303030303030",
+            "too large for enclosing list",
+            false
+          },
+          {
+            "b84401f8410130308330303080308430303030d6d5943030303030303030303030303030303030303030c0808230309630303030303030303030303030303030303030303030",
+            "list ends outside of enclosing list",
+            false
+          },
+          {
+            "9602d4013030308430303030803080c084303030013030",
+            "Cannot read a unsigned byte scalar, expecting a maximum of 1 bytes but current element is 4 bytes long",
             false
           }
         });
@@ -119,13 +129,14 @@ class TransactionRLPDecoderTest {
 
   @ParameterizedTest(name = "[{index}] {1}")
   @MethodSource("dataTransactionSize")
-  void shouldDecodeRLP(final String txRlp, final String ignoredName, final boolean valid) {
+  void shouldDecodeRLP(final String txRlp, final String name, final boolean valid) {
     if (valid) {
       // thrown exceptions will break test
       decodeRLP(RLP.input(Bytes.fromHexString(txRlp)));
     } else {
       assertThatThrownBy(() -> decodeRLP(RLP.input(Bytes.fromHexString(txRlp))))
-          .isInstanceOf(RLPException.class);
+          .isInstanceOf(RLPException.class)
+          .hasMessageContaining(name);
     }
   }
 

ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/NewPooledTransactionHashesMessageProcessorTest.java

@@ -18,9 +18,9 @@ import static java.time.Duration.ofMillis;
 import static java.time.Duration.ofMinutes;
 import static java.time.Instant.now;
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
 import static org.hyperledger.besu.ethereum.eth.encoding.TransactionAnnouncementDecoder.getDecoder;
 import static org.hyperledger.besu.ethereum.eth.encoding.TransactionAnnouncementEncoder.getEncoder;
-import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.mockito.Answers.RETURNS_DEEP_STUBS;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.mock;
@@ -64,7 +64,7 @@ import org.mockito.quality.Strictness;
 
 @ExtendWith(MockitoExtension.class)
 @MockitoSettings(strictness = Strictness.LENIENT)
-public class NewPooledTransactionHashesMessageProcessorTest {
+class NewPooledTransactionHashesMessageProcessorTest {
 
   @Mock private TransactionPool transactionPool;
 
@@ -105,7 +105,7 @@ public class NewPooledTransactionHashesMessageProcessorTest {
   }
 
   @Test
-  public void shouldAddInitiatedRequestingTransactions() {
+  void shouldAddInitiatedRequestingTransactions() {
 
     messageHandler.processNewPooledTransactionHashesMessage(
         peer1,
@@ -120,7 +120,7 @@ public class NewPooledTransactionHashesMessageProcessorTest {
   }
 
   @Test
-  public void shouldNotAddAlreadyPresentTransactions() {
+  void shouldNotAddAlreadyPresentTransactions() {
 
     when(transactionPool.getTransactionByHash(hash1)).thenReturn(Optional.of(transaction1));
     when(transactionPool.getTransactionByHash(hash2)).thenReturn(Optional.of(transaction2));
@@ -138,7 +138,7 @@ public class NewPooledTransactionHashesMessageProcessorTest {
   }
 
   @Test
-  public void shouldAddInitiatedRequestingTransactionsWhenOutOfSync() {
+  void shouldAddInitiatedRequestingTransactionsWhenOutOfSync() {
 
     messageHandler.processNewPooledTransactionHashesMessage(
         peer1,
@@ -149,7 +149,7 @@ public class NewPooledTransactionHashesMessageProcessorTest {
   }
 
   @Test
-  public void shouldNotMarkReceivedExpiredTransactionsAsSeen() {
+  void shouldNotMarkReceivedExpiredTransactionsAsSeen() {
     messageHandler.processNewPooledTransactionHashesMessage(
         peer1,
         NewPooledTransactionHashesMessage.create(transactionList, EthProtocol.ETH66),
@@ -164,7 +164,7 @@ public class NewPooledTransactionHashesMessageProcessorTest {
   }
 
   @Test
-  public void shouldNotAddReceivedTransactionsToTransactionPoolIfExpired() {
+  void shouldNotAddReceivedTransactionsToTransactionPoolIfExpired() {
     messageHandler.processNewPooledTransactionHashesMessage(
         peer1,
         NewPooledTransactionHashesMessage.create(transactionList, EthProtocol.ETH66),
@@ -179,8 +179,7 @@ public class NewPooledTransactionHashesMessageProcessorTest {
   }
 
   @Test
-  public void
-      shouldScheduleGetPooledTransactionsTaskWhenNewTransactionAddedFromPeerForTheFirstTime() {
+  void shouldScheduleGetPooledTransactionsTaskWhenNewTransactionAddedFromPeerForTheFirstTime() {
 
     final EthScheduler ethScheduler = mock(EthScheduler.class);
     when(ethContext.getScheduler()).thenReturn(ethScheduler);
@@ -198,7 +197,7 @@ public class NewPooledTransactionHashesMessageProcessorTest {
   }
 
   @Test
-  public void shouldNotScheduleGetPooledTransactionsTaskTwice() {
+  void shouldNotScheduleGetPooledTransactionsTaskTwice() {
 
     messageHandler.processNewPooledTransactionHashesMessage(
         peer1,
@@ -220,7 +219,7 @@ public class NewPooledTransactionHashesMessageProcessorTest {
   }
 
   @Test
-  public void shouldCreateAndDecodeForEth66() {
+  void shouldCreateAndDecodeForEth66() {
 
     final List<TransactionAnnouncement> expectedAnnouncementList =
         transactionList.stream().map(TransactionAnnouncement::new).toList();
@@ -233,8 +232,8 @@ public class NewPooledTransactionHashesMessageProcessorTest {
         .pendingTransactions()
         .forEach(
             t -> {
-              assertThat(t.getSize().isPresent()).isFalse();
-              assertThat(t.getType().isPresent()).isFalse();
+              assertThat(t.getSize()).isEmpty();
+              assertThat(t.getType()).isEmpty();
             });
 
     // assert all transaction hashes are the same as announcement message
@@ -246,7 +245,7 @@ public class NewPooledTransactionHashesMessageProcessorTest {
   }
 
   @Test
-  public void shouldCreateAndDecodeForEth68() {
+  void shouldCreateAndDecodeForEth68() {
     final List<TransactionAnnouncement> expectedTransactions =
         transactionList.stream().map(TransactionAnnouncement::new).collect(Collectors.toList());
 
@@ -258,25 +257,25 @@ public class NewPooledTransactionHashesMessageProcessorTest {
   }
 
   @Test
-  public void shouldThrowRLPExceptionIfIncorrectVersion() {
+  void shouldThrowRLPExceptionIfIncorrectVersion() {
 
     // message for Eth/68 with 66 data should throw RLPException
     final NewPooledTransactionHashesMessage message66 =
         new NewPooledTransactionHashesMessage(
             getEncoder(EthProtocol.ETH68).encode(transactionList), EthProtocol.ETH66);
     // assert RLPException
-    assertThrows(RLPException.class, message66::pendingTransactions);
+    assertThatThrownBy(message66::pendingTransactions).isInstanceOf(RLPException.class);
 
     // message for Eth/66 with 68 data should throw RLPException
     final NewPooledTransactionHashesMessage message68 =
         new NewPooledTransactionHashesMessage(
             getEncoder(EthProtocol.ETH68).encode(transactionList), EthProtocol.ETH66);
     // assert RLPException
-    assertThrows(RLPException.class, message68::pendingTransactions);
+    assertThatThrownBy(message68::pendingTransactions).isInstanceOf(RLPException.class);
   }
 
   @Test
-  public void shouldEncodeTransactionsCorrectly_Eth68() {
+  void shouldEncodeTransactionsCorrectly_Eth68() {
 
     final String expected =
         "0xf86d83000102c3010203f863a00000000000000000000000000000000000000000000000000000000000000001a00000000000000000000000000000000000000000000000000000000000000002a00000000000000000000000000000000000000000000000000000000000000003";
@@ -297,7 +296,7 @@ public class NewPooledTransactionHashesMessageProcessorTest {
   }
 
   @Test
-  public void shouldDecodeBytesCorrectly_Eth68() {
+  void shouldDecodeBytesCorrectly_Eth68() {
     /*
      * [
      * "0x0000102"]
@@ -341,7 +340,7 @@ public class NewPooledTransactionHashesMessageProcessorTest {
   }
 
   @Test
-  public void shouldDecodeBytesCorrectly_PreviousImplementations_Eth68() {
+  void shouldDecodeBytesCorrectly_PreviousImplementations_Eth68() {
     /*
      * [
      * "0x0000102"]
@@ -385,7 +384,7 @@ public class NewPooledTransactionHashesMessageProcessorTest {
   }
 
   @Test
-  public void shouldEncodeAndDecodeTransactionAnnouncement_Eth66() {
+  void shouldEncodeAndDecodeTransactionAnnouncement_Eth66() {
     final Transaction t1 = generator.transaction(TransactionType.FRONTIER);
     final Transaction t2 = generator.transaction(TransactionType.ACCESS_LIST);
     final Transaction t3 = generator.transaction(TransactionType.EIP1559);
@@ -404,7 +403,7 @@ public class NewPooledTransactionHashesMessageProcessorTest {
   }
 
   @Test
-  public void shouldEncodeAndDecodeTransactionAnnouncement_Eth68() {
+  void shouldEncodeAndDecodeTransactionAnnouncement_Eth68() {
     final Transaction t1 = generator.transaction(TransactionType.FRONTIER);
     final Transaction t2 = generator.transaction(TransactionType.ACCESS_LIST);
     final Transaction t3 = generator.transaction(TransactionType.EIP1559);
@@ -415,7 +414,7 @@ public class NewPooledTransactionHashesMessageProcessorTest {
     final List<TransactionAnnouncement> announcementList =
         getDecoder(EthProtocol.ETH68).decode(RLP.input(bytes));
 
-    assertThat(announcementList.size()).isEqualTo(list.size());
+    assertThat(announcementList).hasSameSizeAs(list);
 
     for (final Transaction transaction : list) {
       final TransactionAnnouncement announcement = announcementList.get(list.indexOf(transaction));
@@ -426,124 +425,91 @@ public class NewPooledTransactionHashesMessageProcessorTest {
   }
 
   @Test
-  public void shouldThrowInvalidArgumentExceptionWhenCreatingListsWithDifferentSizes() {
-    final Exception exception =
-        assertThrows(
-            IllegalArgumentException.class,
-            () ->
-                TransactionAnnouncement.create(new ArrayList<>(), List.of(1L), new ArrayList<>()));
-    final String expectedMessage = "Hashes, sizes and types must have the same number of elements";
-    final String actualMessage = exception.getMessage();
-    assertThat(actualMessage).isEqualTo(expectedMessage);
+  void shouldThrowInvalidArgumentExceptionWhenCreatingListsWithDifferentSizes() {
+    assertThatThrownBy(
+            () -> TransactionAnnouncement.create(new ArrayList<>(), List.of(1L), new ArrayList<>()))
+        .isInstanceOf(IllegalArgumentException.class)
+        .hasMessage("Hashes, sizes and types must have the same number of elements");
   }
 
   @Test
-  public void shouldThrowInvalidArgumentExceptionWhenEncodingListsWithDifferentSizes() {
-    final Exception exception =
-        assertThrows(
-            IllegalArgumentException.class,
+  void shouldThrowInvalidArgumentExceptionWhenEncodingListsWithDifferentSizes() {
+    assertThatThrownBy(
             () ->
                 TransactionAnnouncementEncoder.encodeForEth68(
-                    new ArrayList<>(), List.of(1), new ArrayList<>()));
-    final String expectedMessage = "Hashes, sizes and types must have the same number of elements";
-    final String actualMessage = exception.getMessage();
-    assertThat(actualMessage).isEqualTo(expectedMessage);
+                    new ArrayList<>(), List.of(1), new ArrayList<>()))
+        .isInstanceOf(IllegalArgumentException.class)
+        .hasMessage("Hashes, sizes and types must have the same number of elements");
   }
 
   @Test
   @SuppressWarnings("UnusedVariable")
-  public void shouldThrowRLPExceptionWhenDecodingListsWithDifferentSizes() {
+  void shouldThrowRLPExceptionWhenDecodingListsWithDifferentSizes() {
 
     // ["0x000102",[],["0x881699519a25b0e32db9b1ba9981f3fbec93fbc0726c3e096af89e5ada2b1351"]]
     final Bytes invalidMessageBytes =
         Bytes.fromHexString(
             "0xe783000102c0e1a0881699519a25b0e32db9b1ba9981f3fbec93fbc0726c3e096af89e5ada2b1351");
 
-    final Exception exception =
-        assertThrows(
-            RLPException.class,
+    assertThatThrownBy(
             () ->
                 TransactionAnnouncementDecoder.getDecoder(EthProtocol.ETH68)
-                    .decode(RLP.input(invalidMessageBytes)));
-
-    final String expectedMessage = "Hashes, sizes and types must have the same number of elements";
-    final String actualMessage = exception.getMessage();
-    assertThat(actualMessage).isEqualTo(expectedMessage);
+                    .decode(RLP.input(invalidMessageBytes)))
+        .isInstanceOf(RLPException.class)
+        .hasMessage("Hashes, sizes and types must have the same number of elements");
   }
 
   @Test
-  public void shouldThrowRLPExceptionWhenTypeIsInvalid() {
+  void shouldThrowRLPExceptionWhenTypeIsInvalid() {
     final Bytes invalidMessageBytes =
         Bytes.fromHexString(
             // ["0x07",["0x00000002"],["0x881699519a25b0e32db9b1ba9981f3fbec93fbc0726c3e096af89e5ada2b1351"]]
             "0xe907c58400000002e1a0881699519a25b0e32db9b1ba9981f3fbec93fbc0726c3e096af89e5ada2b1351");
 
-    final Exception exception =
-        assertThrows(
-            IllegalArgumentException.class,
+    assertThatThrownBy(
             () ->
                 TransactionAnnouncementDecoder.getDecoder(EthProtocol.ETH68)
-                    .decode(RLP.input(invalidMessageBytes)));
-
-    final String expectedMessage = "Unsupported transaction type";
-    final String actualMessage = exception.getMessage();
-    assertThat(actualMessage).contains(expectedMessage);
+                    .decode(RLP.input(invalidMessageBytes)))
+        .isInstanceOf(IllegalArgumentException.class)
+        .hasMessageContaining("Unsupported transaction type");
   }
 
   @Test
-  public void shouldThrowRLPExceptionWhenSizeSizeGreaterThanFourBytes() {
+  void shouldThrowRLPExceptionWhenSizeSizeGreaterThanFourBytes() {
     final Bytes invalidMessageBytes =
         Bytes.fromHexString(
             // ["0x02",["0xffffffff01"],["0x881699519a25b0e32db9b1ba9981f3fbec93fbc0726c3e096af89e5ada2b1351"]]
             "0xea02c685ffffffff00e1a0881699519a25b0e32db9b1ba9981f3fbec93fbc0726c3e096af89e5ada2b1351");
 
-    final Exception exception =
-        assertThrows(
-            RLPException.class,
+    assertThatThrownBy(
             () ->
                 TransactionAnnouncementDecoder.getDecoder(EthProtocol.ETH68)
-                    .decode(RLP.input(invalidMessageBytes)));
-
-    final String expectedMessage = "Expected max 4 bytes for unsigned int, but got 5 bytes";
-    assertThat(exception)
-        .hasCauseInstanceOf(RLPException.class)
-        .cause()
-        .hasMessage(expectedMessage);
+                    .decode(RLP.input(invalidMessageBytes)))
+        .isInstanceOf(RLPException.class)
+        .hasMessageContaining("Expected max 4 bytes for unsigned int, but got 5 bytes");
   }
 
   @Test
-  public void shouldThrowNullPointerIfArgumentsAreNull() {
+  void shouldThrowNullPointerIfArgumentsAreNull() {
     final Hash hash = Hash.hash(Bytes.random(32));
-    assertThat(
-            assertThrows(NullPointerException.class, () -> new TransactionAnnouncement((Hash) null))
-                .getMessage())
-        .isEqualTo("Hash cannot be null");
+    assertThatThrownBy(() -> new TransactionAnnouncement((Hash) null))
+        .isInstanceOf(NullPointerException.class)
+        .hasMessage("Hash cannot be null");
 
-    assertThat(
-            assertThrows(
-                    NullPointerException.class,
-                    () -> new TransactionAnnouncement(null, TransactionType.EIP1559, 0L))
-                .getMessage())
-        .isEqualTo("Hash cannot be null");
+    assertThatThrownBy(() -> new TransactionAnnouncement(null, TransactionType.EIP1559, 0L))
+        .isInstanceOf(NullPointerException.class)
+        .hasMessage("Hash cannot be null");
 
-    assertThat(
-            assertThrows(
-                    NullPointerException.class, () -> new TransactionAnnouncement(hash, null, 0L))
-                .getMessage())
-        .isEqualTo("Type cannot be null");
+    assertThatThrownBy(() -> new TransactionAnnouncement(hash, null, 0L))
+        .isInstanceOf(NullPointerException.class)
+        .hasMessage("Type cannot be null");
 
-    assertThat(
-            assertThrows(
-                    NullPointerException.class,
-                    () -> new TransactionAnnouncement(hash, TransactionType.EIP1559, null))
-                .getMessage())
-        .isEqualTo("Size cannot be null");
+    assertThatThrownBy(() -> new TransactionAnnouncement(hash, TransactionType.EIP1559, null))
+        .isInstanceOf(NullPointerException.class)
+        .hasMessage("Size cannot be null");
 
-    assertThat(
-            assertThrows(
-                    NullPointerException.class,
-                    () -> new TransactionAnnouncement((Transaction) null))
-                .getMessage())
-        .isEqualTo("Transaction cannot be null");
+    assertThatThrownBy(() -> new TransactionAnnouncement((Transaction) null))
+        .isInstanceOf(NullPointerException.class)
+        .hasMessage("Transaction cannot be null");
   }
 }

ethereum/evmtool/src/test/java/org/hyperledger/besu/evmtool/StateTestSubCommandTest.java

@@ -27,10 +27,10 @@ import java.io.PrintWriter;
 import org.junit.jupiter.api.Test;
 import picocli.CommandLine;
 
-public class StateTestSubCommandTest {
+class StateTestSubCommandTest {
 
   @Test
-  public void shouldDetectUnsupportedFork() {
+  void shouldDetectUnsupportedFork() {
     final ByteArrayOutputStream baos = new ByteArrayOutputStream();
     EvmToolCommand parentCommand =
         new EvmToolCommand(System.in, new PrintWriter(baos, true, UTF_8));
@@ -44,7 +44,7 @@ public class StateTestSubCommandTest {
   }
 
   @Test
-  public void shouldWorkWithValidStateTest() {
+  void shouldWorkWithValidStateTest() {
     final ByteArrayOutputStream baos = new ByteArrayOutputStream();
     EvmToolCommand parentCommand =
         new EvmToolCommand(System.in, new PrintWriter(baos, true, UTF_8));
@@ -55,7 +55,7 @@ public class StateTestSubCommandTest {
   }
 
   @Test
-  public void shouldWorkWithValidAccessListStateTest() {
+  void shouldWorkWithValidAccessListStateTest() {
     final ByteArrayOutputStream baos = new ByteArrayOutputStream();
     EvmToolCommand parentCommand =
         new EvmToolCommand(System.in, new PrintWriter(baos, true, UTF_8));
@@ -66,7 +66,7 @@ public class StateTestSubCommandTest {
   }
 
   @Test
-  public void noJsonTracer() {
+  void noJsonTracer() {
     final ByteArrayOutputStream baos = new ByteArrayOutputStream();
     EvmToolCommand parentCommand =
         new EvmToolCommand(System.in, new PrintWriter(baos, true, UTF_8));
@@ -80,7 +80,7 @@ public class StateTestSubCommandTest {
   }
 
   @Test
-  public void testsInvalidTransactions() {
+  void testsInvalidTransactions() {
     final ByteArrayOutputStream baos = new ByteArrayOutputStream();
     final ByteArrayInputStream bais =
         new ByteArrayInputStream(
@@ -91,11 +91,11 @@ public class StateTestSubCommandTest {
     final StateTestSubCommand stateTestSubCommand =
         new StateTestSubCommand(new EvmToolCommand(bais, new PrintWriter(baos, true, UTF_8)));
     stateTestSubCommand.run();
-    assertThat(baos.toString(UTF_8)).contains("Transaction had out-of-bounds parameters");
+    assertThat(baos.toString(UTF_8)).contains("Upfront gas cost cannot exceed 2^256 Wei");
   }
 
   @Test
-  public void shouldStreamTests() {
+  void shouldStreamTests() {
     final ByteArrayOutputStream baos = new ByteArrayOutputStream();
     final ByteArrayInputStream bais =
         new ByteArrayInputStream(
@@ -110,7 +110,7 @@ public class StateTestSubCommandTest {
   }
 
   @Test
-  public void failStreamMissingFile() {
+  void failStreamMissingFile() {
     final ByteArrayOutputStream baos = new ByteArrayOutputStream();
     final ByteArrayInputStream bais =
         new ByteArrayInputStream("./file-dose-not-exist.json".getBytes(UTF_8));
@@ -121,7 +121,7 @@ public class StateTestSubCommandTest {
   }
 
   @Test
-  public void failStreamBadFile() {
+  void failStreamBadFile() {
     final ByteArrayOutputStream baos = new ByteArrayOutputStream();
     final ByteArrayInputStream bais =
         new ByteArrayInputStream(

ethereum/rlp/src/main/java/org/hyperledger/besu/ethereum/rlp/AbstractRLPInput.java

@@ -32,6 +32,8 @@ import org.apache.tuweni.units.bigints.UInt64;
 
 abstract class AbstractRLPInput implements RLPInput {
 
+  private static final String errorMessageSuffix = " (at bytes %d-%d: %s%s[%s]%s%s)";
+
   private final boolean lenient;
 
   protected long size; // The number of bytes in this rlp-encoded byte string
@@ -73,7 +75,7 @@ abstract class AbstractRLPInput implements RLPInput {
     // input is corrupted.
     if (size > inputSize) {
       // Our error message include a snippet of the input and that code assume size is not set
-      // outside of the input, and that's exactly the case we're testing, so resetting the size
+      // outside the input, and that's exactly the case we're testing, so resetting the size
       // simply for the sake of the error being properly generated.
       final long itemEnd = size;
       size = inputSize;
@@ -140,14 +142,13 @@ abstract class AbstractRLPInput implements RLPInput {
       currentPayloadSize = elementMetadata.payloadSize;
     } catch (final RLPException exception) {
       final String message =
-          String.format(
-              exception.getMessage() + getErrorMessageSuffix(), getErrorMessageSuffixParams());
+          String.format(exception.getMessage() + errorMessageSuffix, getErrorMessageSuffixParams());
       throw new RLPException(message, exception);
     }
   }
 
   private void validateCurrentItem() {
-    // Validate that a single byte SHORT_ELEMENT payload is not <= 0x7F. If it is, is should have
+    // Validate that a single byte SHORT_ELEMENT payload is not <= 0x7F. If it is, it should have
     // been written as a BYTE_ELEMENT.
     if (currentKind == RLPDecodingHelpers.Kind.SHORT_ELEMENT
         && currentPayloadSize == 1
@@ -211,11 +212,7 @@ abstract class AbstractRLPInput implements RLPInput {
 
   private String errorMsg(final String message, final Object... params) {
     return String.format(
-        message + getErrorMessageSuffix(), concatParams(params, getErrorMessageSuffixParams()));
-  }
-
-  private String getErrorMessageSuffix() {
-    return " (at bytes %d-%d: %s%s[%s]%s%s)";
+        message + errorMessageSuffix, concatParams(params, getErrorMessageSuffixParams()));
   }
 
   private Object[] getErrorMessageSuffixParams() {
@@ -328,6 +325,14 @@ abstract class AbstractRLPInput implements RLPInput {
     return readLongScalar();
   }
 
+  @Override
+  public int readUnsignedByteScalar() {
+    checkScalar("unsigned byte scalar", 1);
+    int result = (currentPayloadSize == 0) ? 0 : payloadByte(0) & 0xff;
+    setTo(nextItem());
+    return result;
+  }
+
   @Override
   public BigInteger readBigIntegerScalar() {
     checkScalar("arbitrary precision scalar");
@@ -410,7 +415,7 @@ abstract class AbstractRLPInput implements RLPInput {
       return InetAddress.getByAddress(address);
     } catch (final UnknownHostException e) {
       // InetAddress.getByAddress() only throws for an address of illegal length, and we have
-      // validated that length already, this this genuinely shouldn't throw.
+      // validated that length already, this genuinely shouldn't throw.
       throw new AssertionError(e);
     }
   }
@@ -485,7 +490,7 @@ abstract class AbstractRLPInput implements RLPInput {
     if (depth > endOfListOffset.length) {
       endOfListOffset = Arrays.copyOf(endOfListOffset, (endOfListOffset.length * 3) / 2);
     }
-    // The first list element is the beginning of the payload. It's end is the end of this item.
+    // The first list element is the beginning of the payload. Its end is the end of this item.
     final long listStart = currentPayloadOffset;
     final long listEnd = nextItem();
 
@@ -495,6 +500,12 @@ abstract class AbstractRLPInput implements RLPInput {
           listEnd, size);
     }
 
+    if (depth > 1 && (listEnd > endOfListOffset[depth - 2])) {
+      throw corrupted(
+          "Invalid RLP item: list ends outside of enclosing list (inner: %d, outer: %d)",
+          listEnd, endOfListOffset[depth - 2]);
+    }
+
     endOfListOffset[depth - 1] = listEnd;
     int count = -1;
 

ethereum/rlp/src/main/java/org/hyperledger/besu/ethereum/rlp/RLPInput.java

@@ -41,9 +41,9 @@ import org.apache.tuweni.units.bigints.UInt64;
  *
  * <p>A {@link RLPInput} thus provides methods to decode both lists and binary values. A list in the
  * input is "entered" by calling {@link #enterList()} and left by calling {@link #leaveList()}.
- * Binary values can be read directly with {@link #readBytes()} ()}, but the {@link RLPInput}
- * interface provides a wealth of convenience methods to read specific types of data that are in
- * specific encoding.
+ * Binary values can be read directly with {@link #readBytes()}, but the {@link RLPInput} interface
+ * provides a wealth of convenience methods to read specific types of data that are in specific
+ * encoding.
  *
  * <p>Amongst the methods to read binary data, some methods are provided to read "scalar". A scalar
  * should simply be understood as a positive integer that is encoded with no leading zeros. In other
@@ -121,8 +121,8 @@ public interface RLPInput {
    * Exits the current list after all its items have been consumed.
    *
    * <p>Note that this method technically doesn't consume any input but must be called after having
-   * read the last element of a list. This allow to ensure the structure of the input is indeed the
-   * one expected.
+   * read the last element of a list. This allows it to ensure the structure of the input is indeed
+   * the one expected.
    *
    * @throws RLPException if the current list is not finished (it has more items).
    */
@@ -132,8 +132,8 @@ public interface RLPInput {
    * Exits the current list, ignoring any remaining unconsumed elements.
    *
    * <p>Note that this method technically doesn't consume any input but must be called after having
-   * read the last element of a list. This allow to ensure the structure of the input is indeed the
-   * one expected.
+   * read the last element of a list. This allows it to ensure the structure of the input is indeed
+   * the one expected.
    */
   void leaveListLenient();
 
@@ -272,6 +272,17 @@ public interface RLPInput {
    *     fit an unsigned int or has leading zeros.
    */
   long readUnsignedIntScalar();
+
+  /**
+   * Reads a scalar from the input and return is as an unsigned byte contained in an int
+   *
+   * @return The next scalar item of this input as an unsigned byte value as int
+   * @throws RLPException if the next item to read is a list, the input is at the end of its current
+   *     list (and {@link #leaveList()} hasn't been called) or if the next item is either too big to
+   *     fit an unsigned byte or has leading zeros.
+   */
+  int readUnsignedByteScalar();
+
   /**
    * Reads an inet address from this input.
    *
@@ -374,10 +385,11 @@ public interface RLPInput {
     for (int i = 0; i < size; i++) {
       try {
         res.add(valueReader.apply(this));
+      } catch (final RLPException e) {
+        throw e;
       } catch (final Exception e) {
         throw new RLPException(
-            String.format(
-                "Error applying element decoding function on " + "element %d of the list", i),
+            String.format("Error applying element decoding function on element %d of the list", i),
             e);
       }
     }