javadoc and more sonar cleanup

Signed-off-by: Justin Florentine <[email protected]>

datatypes/src/main/java/org/hyperledger/besu/datatypes/BlobsWithCommitments.java

@@ -18,14 +18,11 @@
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Objects;
-import java.util.stream.Collectors;
 
 /** A class to hold the blobs, commitments, proofs and versioned hashes for a set of blobs. */
 public class BlobsWithCommitments {
 
-  /**
-   * A record to hold the blob, commitment, proof and versioned hash for a blob.
-   */
+  /** A record to hold the blob, commitment, proof and versioned hash for a blob. */
   public record BlobQuad(
       Blob blob, KZGCommitment kzgCommitment, KZGProof kzgProof, VersionedHash versionedHash) {
 
@@ -45,7 +42,6 @@ public int hashCode() {
       return Objects.hash(blob, kzgCommitment, kzgProof, versionedHash);
     }
   }
-  ;
 
   private final List<BlobQuad> blobQuads;
 
@@ -62,7 +58,7 @@ public BlobsWithCommitments(
       final List<Blob> blobs,
       final List<KZGProof> kzgProofs,
       final List<VersionedHash> versionedHashes) {
-    if (blobs.size() == 0) {
+    if (blobs.isEmpty()) {
       throw new InvalidParameterException(
           "There needs to be a minimum of one blob in a blob transaction with commitments");
     }
@@ -83,6 +79,7 @@ public BlobsWithCommitments(
 
   /**
    * Construct the class from a list of BlobQuads.
+   *
    * @param quads the list of blob quads to be attached to the transaction
    */
   public BlobsWithCommitments(final List<BlobQuad> quads) {
@@ -95,7 +92,7 @@ public BlobsWithCommitments(final List<BlobQuad> quads) {
    * @return the blobs
    */
   public List<Blob> getBlobs() {
-    return blobQuads.stream().map(BlobQuad::blob).collect(Collectors.toList());
+    return blobQuads.stream().map(BlobQuad::blob).toList();
   }
 
   /**
@@ -104,7 +101,7 @@ public List<Blob> getBlobs() {
    * @return the commitments
    */
   public List<KZGCommitment> getKzgCommitments() {
-    return blobQuads.stream().map(BlobQuad::kzgCommitment).collect(Collectors.toList());
+    return blobQuads.stream().map(BlobQuad::kzgCommitment).toList();
   }
 
   /**
@@ -113,7 +110,7 @@ public List<KZGCommitment> getKzgCommitments() {
    * @return the proofs
    */
   public List<KZGProof> getKzgProofs() {
-    return blobQuads.stream().map(BlobQuad::kzgProof).collect(Collectors.toList());
+    return blobQuads.stream().map(BlobQuad::kzgProof).toList();
   }
 
   /**
@@ -122,7 +119,7 @@ public List<KZGProof> getKzgProofs() {
    * @return the hashes
    */
   public List<VersionedHash> getVersionedHashes() {
-    return blobQuads.stream().map(BlobQuad::versionedHash).collect(Collectors.toList());
+    return blobQuads.stream().map(BlobQuad::versionedHash).toList();
   }
 
   /**

ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/EthGetTransactionByHashTest.java

@@ -181,7 +181,9 @@ void validateResultSpec() {
     assertThat(result.getRaw()).isNotNull();
     assertThat(result.getTo()).isNotNull();
     assertThat(result.getValue()).isNotNull();
-    assertThat(result.getYParity()).isNotNull();
+    if (!"0x0".equals(result.getType())) {
+      assertThat(result.getYParity()).isNotNull();
+    }
     assertThat(result.getV()).isNotNull();
     assertThat(result.getR()).isNotNull();
     assertThat(result.getS()).isNotNull();

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

@@ -38,13 +38,13 @@
 import com.google.common.base.Suppliers;
 import org.junit.jupiter.api.Test;
 
-public class TransactionBuilderTest {
+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() {
+  void guessTypeCanGuessAllTypes() {
     final BlockDataGenerator gen = new BlockDataGenerator();
     final Transaction.Builder frontierBuilder = Transaction.builder();
     final Transaction.Builder eip1559Builder = Transaction.builder().maxFeePerGas(Wei.of(5));
@@ -63,16 +63,17 @@ public void guessTypeCanGuessAllTypes() {
   }
 
   @Test
-  public void zeroBlobTransactionIsInvalid() {
+  void zeroBlobTransactionIsInvalid() {
+    TransactionTestFixture ttf =
+        new TransactionTestFixture()
+            .type(TransactionType.BLOB)
+            .chainId(Optional.of(BigInteger.ONE))
+            .versionedHashes(Optional.of(List.of()))
+            .maxFeePerGas(Optional.of(Wei.of(5)))
+            .maxPriorityFeePerGas(Optional.of(Wei.of(5)))
+            .maxFeePerBlobGas(Optional.of(Wei.of(5)));
     try {
-      new TransactionTestFixture()
-          .type(TransactionType.BLOB)
-          .chainId(Optional.of(BigInteger.ONE))
-          .versionedHashes(Optional.of(List.of()))
-          .maxFeePerGas(Optional.of(Wei.of(5)))
-          .maxPriorityFeePerGas(Optional.of(Wei.of(5)))
-          .maxFeePerBlobGas(Optional.of(Wei.of(5)))
-          .createTransaction(senderKeys);
+      ttf.createTransaction(senderKeys);
       fail();
     } catch (IllegalArgumentException iea) {
       assertThat(iea).hasMessage("Blob transaction must have at least one versioned hash");
@@ -81,13 +82,12 @@ public void zeroBlobTransactionIsInvalid() {
 
   @Test
   @SuppressWarnings("ReferenceEquality")
-  public void copyFromIsIdentical() {
+  void copyFromIsIdentical() {
     final TransactionTestFixture fixture = new TransactionTestFixture();
     final Transaction transaction = fixture.createTransaction(senderKeys);
     final Transaction.Builder builder = Transaction.builder();
     final Transaction copy = builder.copiedFrom(transaction).build();
-    assertThat(copy).isEqualTo(transaction);
-    assertThat(copy == transaction).isFalse();
+    assertThat(copy).isEqualTo(transaction).isNotSameAs(transaction);
     assertThat(copy.getHash()).isEqualTo(transaction.getHash());
     BytesValueRLPOutput sourceRLP = new BytesValueRLPOutput();
     transaction.writeTo(sourceRLP);

ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/BlobCache.java

@@ -32,10 +32,10 @@ public class BlobCache {
   private static final Logger LOG = LoggerFactory.getLogger(BlobCache.class);
 
   public BlobCache() {
-    // TODO: needs size limit, ttl policy and eviction on finalization policy - cache size should
-    // max out around 6 * blocks since final
-
-    this.cache = Caffeine.newBuilder().build();
+    this.cache =
+        Caffeine.newBuilder()
+            .maximumSize(6 * 32 * 3l)
+            .build(); // 6 blobs max per 32 slots per 3 epochs
   }
 
   public void cacheBlobs(final Transaction t) {
@@ -63,7 +63,7 @@ public Optional<Transaction> restoreBlob(final Transaction transaction) {
           if (blobQuads.stream()
               .map(BlobsWithCommitments.BlobQuad::versionedHash)
               .toList()
-              .containsAll(transaction.getVersionedHashes().get())) {
+              .containsAll(maybeHashes.get())) {
             txBuilder.blobsWithCommitments(bwc);
             return Optional.of(txBuilder.build());
           } else {
@@ -81,7 +81,7 @@ public Optional<Transaction> restoreBlob(final Transaction transaction) {
 
     } else {
       LOG.debug(
-          "can't restore blobs for non-blob transaction of type " + transaction.getType().name());
+          "can't restore blobs for non-blob transaction of type {}", transaction.getType().name());
       return Optional.empty();
     }
   }

ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/layered/AbstractTransactionsLayer.java

@@ -77,7 +77,7 @@ public abstract class AbstractTransactionsLayer implements TransactionsLayer {
 
   private final BlobCache blobCache;
 
-  public AbstractTransactionsLayer(
+  protected AbstractTransactionsLayer(
       final TransactionPoolConfiguration poolConfig,
       final TransactionsLayer nextLayer,
       final BiFunction<PendingTransaction, PendingTransaction, Boolean>
@@ -90,7 +90,6 @@ public AbstractTransactionsLayer(
     metrics.initSpaceUsed(this::getLayerSpaceUsed, name());
     metrics.initTransactionCount(pendingTransactions::size, name());
     metrics.initUniqueSenderCount(txsBySender::size, name());
-    // TODO: needs size limit, ttl policy and eviction on finalization policy
     this.blobCache = new BlobCache();
   }
 
@@ -366,11 +365,12 @@ protected PendingTransaction processRemove(
       final Transaction transaction,
       final RemovalReason removalReason) {
     final PendingTransaction removedTx = pendingTransactions.remove(transaction.getHash());
-    if (removedTx.getTransaction().getBlobsWithCommitments().isPresent()
-        && CONFIRMED.equals(removalReason)) {
-      this.blobCache.cacheBlobs(removedTx.getTransaction());
-    }
+
     if (removedTx != null) {
+      if (removedTx.getTransaction().getBlobsWithCommitments().isPresent()
+          && CONFIRMED.equals(removalReason)) {
+        this.blobCache.cacheBlobs(removedTx.getTransaction());
+      }
       decreaseSpaceUsed(removedTx);
       metrics.incrementRemoved(removedTx, removalReason.label(), name());
       internalRemove(senderTxs, removedTx, removalReason);

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

@@ -122,7 +122,6 @@
 @MockitoSettings(strictness = LENIENT)
 public abstract class AbstractTransactionPoolTest {
 
-  protected static final int MAX_TRANSACTIONS = 5;
   protected static final KeyPair KEY_PAIR1 =
       SignatureAlgorithmFactory.getInstance().generateKeyPair();
   private static final KeyPair KEY_PAIR2 =