From 4bc37084544891e3f824c49b2f5771564f15037b Mon Sep 17 00:00:00 2001 From: Justin Florentine Date: Thu, 9 Nov 2023 10:51:07 -0500 Subject: [PATCH] javadoc and more sonar cleanup Signed-off-by: Justin Florentine --- .../besu/datatypes/BlobsWithCommitments.java | 17 +++++------ .../methods/EthGetTransactionByHashTest.java | 4 ++- .../ethereum/core/TransactionBuilderTest.java | 28 +++++++++---------- .../ethereum/eth/transactions/BlobCache.java | 12 ++++---- .../layered/AbstractTransactionsLayer.java | 12 ++++---- .../AbstractTransactionPoolTest.java | 1 - 6 files changed, 36 insertions(+), 38 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 index 03b01f8618d..9c1f5eb8784 100644 --- a/datatypes/src/main/java/org/hyperledger/besu/datatypes/BlobsWithCommitments.java +++ b/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 blobQuads; @@ -62,7 +58,7 @@ public BlobsWithCommitments( final List blobs, final List kzgProofs, final List 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 quads) { @@ -95,7 +92,7 @@ public BlobsWithCommitments(final List quads) { * @return the blobs */ public List getBlobs() { - return blobQuads.stream().map(BlobQuad::blob).collect(Collectors.toList()); + return blobQuads.stream().map(BlobQuad::blob).toList(); } /** @@ -104,7 +101,7 @@ public List getBlobs() { * @return the commitments */ public List getKzgCommitments() { - return blobQuads.stream().map(BlobQuad::kzgCommitment).collect(Collectors.toList()); + return blobQuads.stream().map(BlobQuad::kzgCommitment).toList(); } /** @@ -113,7 +110,7 @@ public List getKzgCommitments() { * @return the proofs */ public List getKzgProofs() { - return blobQuads.stream().map(BlobQuad::kzgProof).collect(Collectors.toList()); + return blobQuads.stream().map(BlobQuad::kzgProof).toList(); } /** @@ -122,7 +119,7 @@ public List getKzgProofs() { * @return the hashes */ public List getVersionedHashes() { - return blobQuads.stream().map(BlobQuad::versionedHash).collect(Collectors.toList()); + return blobQuads.stream().map(BlobQuad::versionedHash).toList(); } /** diff --git a/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/EthGetTransactionByHashTest.java b/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/EthGetTransactionByHashTest.java index 34ab6ba93eb..3a463c2c547 100644 --- a/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/EthGetTransactionByHashTest.java +++ b/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(); 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 index 27e9ce5a7b7..91d19c28dce 100644 --- 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 @@ -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 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); diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/BlobCache.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/BlobCache.java index 9163d269f47..8c3bf79f5bf 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/BlobCache.java +++ b/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 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 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(); } } diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/layered/AbstractTransactionsLayer.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/layered/AbstractTransactionsLayer.java index 0834d526850..7a9aa51cb7e 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/layered/AbstractTransactionsLayer.java +++ b/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 @@ -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); diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/AbstractTransactionPoolTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/AbstractTransactionPoolTest.java index 1c0497ba505..a1597df1932 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/AbstractTransactionPoolTest.java +++ b/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 =