Transaction detachedCopy to optimize txpool memory usage (hyperledger…

…#5985)

Signed-off-by: Fabio Di Fabio <[email protected]>
Co-authored-by: Justin Florentine <[email protected]>

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

@@ -134,6 +134,8 @@ public static Transaction readFrom(final RLPInput rlpInput) {
   /**
    * Instantiates a transaction instance.
    *
+   * @param forCopy true when using to create a copy of an already validated transaction avoid to
+   *     redo the validation
    * @param transactionType the transaction type
    * @param nonce the nonce
    * @param gasPrice the gas price
@@ -154,7 +156,8 @@ public static Transaction readFrom(final RLPInput rlpInput) {
    *     <p>The {@code chainId} must be greater than 0 to be applied to a specific chain; otherwise
    *     it will default to any chain.
    */
-  public Transaction(
+  private Transaction(
+      final boolean forCopy,
       final TransactionType transactionType,
       final long nonce,
       final Optional<Wei> gasPrice,
@@ -172,36 +175,40 @@ public Transaction(
       final Optional<List<VersionedHash>> versionedHashes,
       final Optional<BlobsWithCommitments> blobsWithCommitments) {
 
-    if (transactionType.requiresChainId()) {
-      checkArgument(
-          chainId.isPresent(), "Chain id must be present for transaction type %s", transactionType);
-    }
+    if (!forCopy) {
+      if (transactionType.requiresChainId()) {
+        checkArgument(
+            chainId.isPresent(),
+            "Chain id must be present for transaction type %s",
+            transactionType);
+      }
 
-    if (maybeAccessList.isPresent()) {
-      checkArgument(
-          transactionType.supportsAccessList(),
-          "Must not specify access list for transaction not supporting it");
-    }
+      if (maybeAccessList.isPresent()) {
+        checkArgument(
+            transactionType.supportsAccessList(),
+            "Must not specify access list for transaction not supporting it");
+      }
 
-    if (Objects.equals(transactionType, TransactionType.ACCESS_LIST)) {
-      checkArgument(
-          maybeAccessList.isPresent(), "Must specify access list for access list transaction");
-    }
+      if (Objects.equals(transactionType, TransactionType.ACCESS_LIST)) {
+        checkArgument(
+            maybeAccessList.isPresent(), "Must specify access list for access list transaction");
+      }
 
-    if (versionedHashes.isPresent() || maxFeePerBlobGas.isPresent()) {
-      checkArgument(
-          transactionType.supportsBlob(),
-          "Must not specify blob versioned hashes or max fee per blob gas for transaction not supporting it");
-    }
+      if (versionedHashes.isPresent() || maxFeePerBlobGas.isPresent()) {
+        checkArgument(
+            transactionType.supportsBlob(),
+            "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 versioned hash");
-      checkArgument(
-          maxFeePerBlobGas.isPresent(), "Must specify max fee per blob gas for blob 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 versioned hash");
+        checkArgument(
+            maxFeePerBlobGas.isPresent(), "Must specify max fee per blob gas for blob transaction");
+      }
     }
 
     this.transactionType = transactionType;
@@ -221,7 +228,7 @@ public Transaction(
     this.versionedHashes = versionedHashes;
     this.blobsWithCommitments = blobsWithCommitments;
 
-    if (isUpfrontGasCostTooHigh()) {
+    if (!forCopy && isUpfrontGasCostTooHigh()) {
       throw new IllegalArgumentException("Upfront gas cost exceeds UInt256");
     }
   }
@@ -998,6 +1005,84 @@ public Optional<Address> contractAddress() {
     return Optional.empty();
   }
 
+  /**
+   * Creates a copy of this transaction that does not share any underlying byte array.
+   *
+   * <p>This is useful in case the transaction is built from a block body and fields, like to or
+   * payload, are wrapping (and so keeping references) sections of the large RPL encoded block body,
+   * and we plan to keep the transaction around for some time, like in the txpool in case of a
+   * reorg, and do not want to keep all the block body in memory for a long time, but only the
+   * actual transaction.
+   *
+   * @return a copy of the transaction
+   */
+  public Transaction detachedCopy() {
+    final Optional<Address> detachedTo =
+        to.isEmpty() ? to : Optional.of(Address.wrap(to.get().copy()));
+    final Optional<List<AccessListEntry>> detachedAccessList =
+        maybeAccessList.isEmpty()
+            ? maybeAccessList
+            : Optional.of(
+                maybeAccessList.get().stream().map(this::accessListDetachedCopy).toList());
+    final Optional<List<VersionedHash>> detachedVersionedHashes =
+        versionedHashes.isEmpty()
+            ? versionedHashes
+            : Optional.of(
+                versionedHashes.get().stream()
+                    .map(vh -> new VersionedHash(vh.toBytes().copy()))
+                    .toList());
+    final Optional<BlobsWithCommitments> detachedBlobsWithCommitments =
+        blobsWithCommitments.isEmpty()
+            ? blobsWithCommitments
+            : Optional.of(
+                blobsWithCommitmentsDetachedCopy(
+                    blobsWithCommitments.get(), detachedVersionedHashes.get()));
+
+    return new Transaction(
+        true,
+        transactionType,
+        nonce,
+        gasPrice,
+        maxPriorityFeePerGas,
+        maxFeePerGas,
+        maxFeePerBlobGas,
+        gasLimit,
+        detachedTo,
+        value,
+        signature,
+        payload.copy(),
+        detachedAccessList,
+        sender,
+        chainId,
+        detachedVersionedHashes,
+        detachedBlobsWithCommitments);
+  }
+
+  private AccessListEntry accessListDetachedCopy(final AccessListEntry accessListEntry) {
+    final Address detachedAddress = Address.wrap(accessListEntry.address().copy());
+    final var detachedStorage = accessListEntry.storageKeys().stream().map(Bytes32::copy).toList();
+    return new AccessListEntry(detachedAddress, detachedStorage);
+  }
+
+  private BlobsWithCommitments blobsWithCommitmentsDetachedCopy(
+      final BlobsWithCommitments blobsWithCommitments, final List<VersionedHash> versionedHashes) {
+    final var detachedCommitments =
+        blobsWithCommitments.getKzgCommitments().stream()
+            .map(kc -> new KZGCommitment(kc.getData().copy()))
+            .toList();
+    final var detachedBlobs =
+        blobsWithCommitments.getBlobs().stream()
+            .map(blob -> new Blob(blob.getData().copy()))
+            .toList();
+    final var detachedProofs =
+        blobsWithCommitments.getKzgProofs().stream()
+            .map(proof -> new KZGProof(proof.getData().copy()))
+            .toList();
+
+    return new BlobsWithCommitments(
+        detachedCommitments, detachedBlobs, detachedProofs, versionedHashes);
+  }
+
   public static class Builder {
     private static final Optional<List<AccessListEntry>> EMPTY_ACCESS_LIST = Optional.of(List.of());
 
@@ -1134,6 +1219,7 @@ public TransactionType getTransactionType() {
     public Transaction build() {
       if (transactionType == null) guessType();
       return new Transaction(
+          false,
           transactionType,
           nonce,
           Optional.ofNullable(gasPrice),

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

@@ -24,9 +24,9 @@
 
 /**
  * Class responsible for decoding blob transactions from the transaction pool. Blob transactions
- * have two network representations. During transaction gossip responses (PooledTransactions), the
- * EIP-2718 TransactionPayload of the blob transaction is wrapped to become: rlp([tx_payload_body,
- * blobs, commitments, proofs]).
+ * have two representations. The network representation is used during transaction gossip responses
+ * (PooledTransactions), the EIP-2718 TransactionPayload of the blob transaction is wrapped to
+ * become: rlp([tx_payload_body, blobs, commitments, proofs]).
  */
 public class BlobPooledTransactionDecoder {
 

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

@@ -31,14 +31,20 @@
 public abstract class PendingTransaction
     implements org.hyperledger.besu.datatypes.PendingTransaction {
   static final int NOT_INITIALIZED = -1;
-  static final int FRONTIER_BASE_MEMORY_SIZE = 944;
-  static final int ACCESS_LIST_BASE_MEMORY_SIZE = 944;
-  static final int EIP1559_BASE_MEMORY_SIZE = 1056;
-  static final int OPTIONAL_TO_MEMORY_SIZE = 92;
+  static final int FRONTIER_AND_ACCESS_LIST_BASE_MEMORY_SIZE = 872;
+  static final int EIP1559_AND_EIP4844_BASE_MEMORY_SIZE = 984;
+  static final int OPTIONAL_TO_MEMORY_SIZE = 112;
+  static final int OPTIONAL_CHAIN_ID_MEMORY_SIZE = 80;
   static final int PAYLOAD_BASE_MEMORY_SIZE = 32;
   static final int ACCESS_LIST_STORAGE_KEY_MEMORY_SIZE = 32;
-  static final int ACCESS_LIST_ENTRY_BASE_MEMORY_SIZE = 128;
+  static final int ACCESS_LIST_ENTRY_BASE_MEMORY_SIZE = 248;
   static final int OPTIONAL_ACCESS_LIST_MEMORY_SIZE = 24;
+  static final int VERSIONED_HASH_SIZE = 96;
+  static final int BASE_LIST_SIZE = 48;
+  static final int BASE_OPTIONAL_SIZE = 16;
+  static final int KZG_COMMITMENT_OR_PROOF_SIZE = 112;
+  static final int BLOB_SIZE = 131136;
+  static final int BLOBS_WITH_COMMITMENTS_SIZE = 32;
   static final int PENDING_TRANSACTION_MEMORY_SIZE = 40;
   private static final AtomicLong TRANSACTIONS_ADDED = new AtomicLong();
   private final Transaction transaction;
@@ -47,10 +53,15 @@ public abstract class PendingTransaction
 
   private int memorySize = NOT_INITIALIZED;
 
-  protected PendingTransaction(final Transaction transaction, final long addedAt) {
+  private PendingTransaction(
+      final Transaction transaction, final long addedAt, final long sequence) {
     this.transaction = transaction;
     this.addedAt = addedAt;
-    this.sequence = TRANSACTIONS_ADDED.getAndIncrement();
+    this.sequence = sequence;
+  }
+
+  private PendingTransaction(final Transaction transaction, final long addedAt) {
+    this(transaction, addedAt, TRANSACTIONS_ADDED.getAndIncrement());
   }
 
   @Override
@@ -90,6 +101,8 @@ public int memorySize() {
     return memorySize;
   }
 
+  public abstract PendingTransaction detachedCopy();
+
   private int computeMemorySize() {
     return switch (transaction.getType()) {
           case FRONTIER -> computeFrontierMemorySize();
@@ -101,30 +114,49 @@ private int computeMemorySize() {
   }
 
   private int computeFrontierMemorySize() {
-    return FRONTIER_BASE_MEMORY_SIZE + computePayloadMemorySize() + computeToMemorySize();
+    return FRONTIER_AND_ACCESS_LIST_BASE_MEMORY_SIZE
+        + computePayloadMemorySize()
+        + computeToMemorySize()
+        + computeChainIdMemorySize();
   }
 
   private int computeAccessListMemorySize() {
-    return ACCESS_LIST_BASE_MEMORY_SIZE
+    return FRONTIER_AND_ACCESS_LIST_BASE_MEMORY_SIZE
         + computePayloadMemorySize()
         + computeToMemorySize()
+        + computeChainIdMemorySize()
         + computeAccessListEntriesMemorySize();
   }
 
   private int computeEIP1559MemorySize() {
-    return EIP1559_BASE_MEMORY_SIZE
+    return EIP1559_AND_EIP4844_BASE_MEMORY_SIZE
         + computePayloadMemorySize()
         + computeToMemorySize()
+        + computeChainIdMemorySize()
         + computeAccessListEntriesMemorySize();
   }
 
   private int computeBlobMemorySize() {
-    // ToDo 4844: adapt for blobs
-    return computeEIP1559MemorySize();
+    return computeEIP1559MemorySize()
+        + BASE_OPTIONAL_SIZE // for the versionedHashes field
+        + computeBlobWithCommitmentsMemorySize();
+  }
+
+  private int computeBlobWithCommitmentsMemorySize() {
+    final int blobCount = transaction.getBlobCount();
+
+    return BASE_OPTIONAL_SIZE
+        + BLOBS_WITH_COMMITMENTS_SIZE
+        + (BASE_LIST_SIZE * 4)
+        + (KZG_COMMITMENT_OR_PROOF_SIZE * blobCount * 2)
+        + (VERSIONED_HASH_SIZE * blobCount)
+        + (BLOB_SIZE * blobCount);
   }
 
   private int computePayloadMemorySize() {
-    return PAYLOAD_BASE_MEMORY_SIZE + transaction.getPayload().size();
+    return transaction.getPayload().size() > 0
+        ? PAYLOAD_BASE_MEMORY_SIZE + transaction.getPayload().size()
+        : 0;
   }
 
   private int computeToMemorySize() {
@@ -134,6 +166,13 @@ private int computeToMemorySize() {
     return 0;
   }
 
+  private int computeChainIdMemorySize() {
+    if (transaction.getChainId().isPresent()) {
+      return OPTIONAL_CHAIN_ID_MEMORY_SIZE;
+    }
+    return 0;
+  }
+
   private int computeAccessListEntriesMemorySize() {
     return transaction
         .getAccessList()
@@ -212,6 +251,15 @@ public Local(final Transaction transaction) {
       this(transaction, System.currentTimeMillis());
     }
 
+    private Local(final long sequence, final Transaction transaction) {
+      super(transaction, System.currentTimeMillis(), sequence);
+    }
+
+    @Override
+    public PendingTransaction detachedCopy() {
+      return new Local(getSequence(), getTransaction().detachedCopy());
+    }
+
     @Override
     public boolean isReceivedFromLocalSource() {
       return true;
@@ -228,6 +276,15 @@ public Remote(final Transaction transaction) {
       this(transaction, System.currentTimeMillis());
     }
 
+    private Remote(final long sequence, final Transaction transaction) {
+      super(transaction, System.currentTimeMillis(), sequence);
+    }
+
+    @Override
+    public PendingTransaction detachedCopy() {
+      return new Remote(getSequence(), getTransaction().detachedCopy());
+    }
+
     @Override
     public boolean isReceivedFromLocalSource() {
       return false;

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

@@ -150,7 +150,7 @@ public TransactionAddedResult add(final PendingTransaction pendingTransaction, f
     }
 
     if (addStatus.isSuccess()) {
-      processAdded(pendingTransaction);
+      processAdded(pendingTransaction.detachedCopy());
       addStatus.maybeReplacedTransaction().ifPresent(this::replaced);
 
       nextLayer.notifyAdded(pendingTransaction);