blob restoral in both legacy and layered implementations and test cov…

…erage of tx copy in builder

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

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

@@ -1096,6 +1096,26 @@ public static class Builder {
     protected List<VersionedHash> versionedHashes = null;
     private BlobsWithCommitments blobsWithCommitments;
 
+    public Builder copiedFrom(final Transaction toCopy) {
+        this.transactionType = toCopy.transactionType;
+        this.nonce = toCopy.nonce;
+        this.gasPrice = toCopy.gasPrice.orElse(null);
+        this.maxPriorityFeePerGas = toCopy.maxPriorityFeePerGas.orElse(null);
+        this.maxFeePerGas = toCopy.maxFeePerGas.orElse(null);
+        this.maxFeePerBlobGas = toCopy.maxFeePerBlobGas.orElse(null);
+        this.gasLimit = toCopy.gasLimit;
+        this.to = toCopy.to;
+        this.value = toCopy.value;
+        this.signature = toCopy.signature;
+        this.payload = toCopy.payload;
+        this.accessList = toCopy.maybeAccessList;
+        this.sender = toCopy.sender;
+        this.chainId = toCopy.chainId;
+        this.versionedHashes = toCopy.versionedHashes.orElse(null);
+        this.blobsWithCommitments = toCopy.blobsWithCommitments.orElse(null);
+        return this;
+    }
+
     public Builder type(final TransactionType transactionType) {
       this.transactionType = transactionType;
       return this;
@@ -1260,5 +1280,10 @@ public Builder kzgBlobs(
           new BlobsWithCommitments(kzgCommitments, blobs, kzgProofs, versionedHashes);
       return this;
     }
+
+    public Builder blobsWithCommitments(final BlobsWithCommitments blobsWithCommitments) {
+      this.blobsWithCommitments = blobsWithCommitments;
+      return this;
+    }
   }
 }

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

@@ -17,8 +17,12 @@
 
 import static java.util.stream.Collectors.toUnmodifiableSet;
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.fail;
 
+import org.apache.tuweni.bytes.Bytes;
+import org.apache.tuweni.bytes.DelegatingBytes;
+import org.apache.tuweni.bytes.MutableBytes;
 import org.hyperledger.besu.crypto.KeyPair;
 import org.hyperledger.besu.crypto.SignatureAlgorithm;
 import org.hyperledger.besu.crypto.SignatureAlgorithmFactory;
@@ -34,6 +38,8 @@
 import java.util.stream.Stream;
 
 import com.google.common.base.Suppliers;
+import org.hyperledger.besu.ethereum.rlp.BytesValueRLPOutput;
+import org.hyperledger.besu.ethereum.rlp.RLPOutput;
 import org.junit.jupiter.api.Test;
 
 public class TransactionBuilderTest {
@@ -76,4 +82,20 @@ public void zeroBlobTransactionIsInvalid() {
       assertThat(iea).hasMessage("Blob transaction must have at least one versioned hash");
     }
   }
+
+  @Test
+  @SuppressWarnings("ReferenceEquality")
+  public 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();
+    BytesValueRLPOutput sourceRLP = new BytesValueRLPOutput();
+    transaction.writeTo(sourceRLP);
+    BytesValueRLPOutput copyRLP = new BytesValueRLPOutput();
+    copy.writeTo(copyRLP);
+    assertEquals(sourceRLP.encoded(), copyRLP.encoded());
+  }
 }

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

@@ -116,4 +116,9 @@ public String toTraceLog() {
   public String logStats() {
     return "Disabled";
   }
+
+  @Override
+  public Optional<Transaction> restoreBlob(final Transaction transaction) {
+    return Optional.empty();
+  }
 }

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

@@ -83,7 +83,7 @@ default void signalInvalidAndRemoveDependentTransactions(final Transaction trans
     // no-op
   }
 
-  void restoreBlob(Transaction transaction);
+  Optional<Transaction> restoreBlob(Transaction transaction);
 
   @FunctionalInterface
   interface TransactionSelector {

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

@@ -232,9 +232,6 @@ private ValidationResult<TransactionInvalidReason> addTransaction(
     final boolean hasPriority = isPriorityTransaction(transaction, isLocal);
 
     if (pendingTransactions.containsTransaction(transaction)) {
-      if (transaction.getType().supportsBlob()) {
-        pendingTransactions.restoreBlob(transaction);
-      }
       LOG.atTrace()
           .setMessage("Discard already present transaction {}")
           .addArgument(transaction::toTraceLog)
@@ -243,22 +240,29 @@ private ValidationResult<TransactionInvalidReason> addTransaction(
       metrics.incrementRejected(isLocal, hasPriority, TRANSACTION_ALREADY_KNOWN, "txpool");
       return ValidationResult.invalid(TRANSACTION_ALREADY_KNOWN);
     }
+    Transaction toAdd = transaction;
     // if adding a blob tx, and it is missing its blob, is a re-org and we should restore the blob
     // from cache.
+    if (transaction.getType().supportsBlob() && transaction.getBlobsWithCommitments().isEmpty()) {
+      final Optional<Transaction> maybeCachedBlob = pendingTransactions.restoreBlob(transaction);
+      if(maybeCachedBlob.isPresent()) {
+        toAdd = maybeCachedBlob.get();
+      }
+    }
 
     final ValidationResultAndAccount validationResult =
-        validateTransaction(transaction, isLocal, hasPriority);
+        validateTransaction(toAdd, isLocal, hasPriority);
 
     if (validationResult.result.isValid()) {
       final TransactionAddedResult status =
           pendingTransactions.addTransaction(
-              PendingTransaction.newPendingTransaction(transaction, isLocal, hasPriority),
+              PendingTransaction.newPendingTransaction(toAdd, isLocal, hasPriority),
               validationResult.maybeAccount);
       if (status.isSuccess()) {
         LOG.atTrace()
             .setMessage("Added {} transaction {}")
             .addArgument(() -> isLocal ? "local" : "remote")
-            .addArgument(transaction::toTraceLog)
+            .addArgument(toAdd::toTraceLog)
             .log();
       } else {
         final var rejectReason =
@@ -271,7 +275,7 @@ private ValidationResult<TransactionInvalidReason> addTransaction(
                     });
         LOG.atTrace()
             .setMessage("Transaction {} rejected reason {}")
-            .addArgument(transaction::toTraceLog)
+            .addArgument(toAdd::toTraceLog)
             .addArgument(rejectReason)
             .log();
         metrics.incrementRejected(isLocal, hasPriority, rejectReason, "txpool");
@@ -280,15 +284,15 @@ private ValidationResult<TransactionInvalidReason> addTransaction(
     } else {
       LOG.atTrace()
           .setMessage("Discard invalid transaction {}, reason {}")
-          .addArgument(transaction::toTraceLog)
+          .addArgument(toAdd::toTraceLog)
           .addArgument(validationResult.result::getInvalidReason)
           .log();
       metrics.incrementRejected(
           isLocal, hasPriority, validationResult.result.getInvalidReason(), "txpool");
       if (!isLocal
           && !INVALID_TX_CACHE_IGNORED_ERRORS.contains(
               validationResult.result.getInvalidReason())) {
-        pendingTransactions.signalInvalidAndRemoveDependentTransactions(transaction);
+        pendingTransactions.signalInvalidAndRemoveDependentTransactions(toAdd);
       }
     }
 

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

@@ -25,6 +25,8 @@
 import static org.hyperledger.besu.ethereum.eth.transactions.layered.TransactionsLayer.RemovalReason.REPLACED;
 
 import org.hyperledger.besu.datatypes.Address;
+import org.hyperledger.besu.datatypes.Blob;
+import org.hyperledger.besu.datatypes.BlobsWithCommitments;
 import org.hyperledger.besu.datatypes.Hash;
 import org.hyperledger.besu.ethereum.core.BlockHeader;
 import org.hyperledger.besu.ethereum.core.Transaction;
@@ -76,7 +78,7 @@ public abstract class AbstractTransactionsLayer implements TransactionsLayer {
   private OptionalLong nextLayerOnDroppedListenerId = OptionalLong.empty();
   protected long spaceUsed = 0;
 
-  private final Cache blobCache;
+  private final Cache<Hash, BlobsWithCommitments> blobCache;
 
   public AbstractTransactionsLayer(
       final TransactionPoolConfiguration poolConfig,
@@ -610,4 +612,8 @@ boolean consistencyCheck(
 
   protected abstract void internalConsistencyCheck(
       final Map<Address, TreeMap<Long, PendingTransaction>> prevLayerTxsBySender);
+
+  public Cache<Hash, BlobsWithCommitments> getBlobCache() {
+    return blobCache;
+  }
 }

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

@@ -24,6 +24,7 @@
 import static org.hyperledger.besu.ethereum.eth.transactions.layered.TransactionsLayer.RemovalReason.RECONCILED;
 
 import org.hyperledger.besu.datatypes.Address;
+import org.hyperledger.besu.datatypes.BlobsWithCommitments;
 import org.hyperledger.besu.datatypes.Hash;
 import org.hyperledger.besu.ethereum.core.BlockHeader;
 import org.hyperledger.besu.ethereum.core.Transaction;
@@ -535,4 +536,17 @@ public synchronized String toTraceLog() {
   public synchronized String logStats() {
     return prioritizedTransactions.logStats();
   }
+
+  @Override
+  public Optional<Transaction> restoreBlob(final Transaction transaction) {
+    Transaction.Builder txBuilder = Transaction.builder();
+    txBuilder.copiedFrom(transaction);
+    final BlobsWithCommitments bwc = prioritizedTransactions.getBlobCache().getIfPresent(transaction.getHash());
+    if(bwc != null) {
+      txBuilder.blobsWithCommitments(bwc);
+      return Optional.of(txBuilder.build());
+    } else {
+      return Optional.empty();
+    }
+  }
 }

ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/sorter/AbstractPendingTransactionsSorter.java

@@ -547,4 +547,17 @@ public void signalInvalidAndRemoveDependentTransactions(final Transaction transa
       signalInvalidAndGetDependentTransactions(transaction).forEach(this::removeTransaction);
     }
   }
+
+  @Override
+  public Optional<Transaction> restoreBlob(final Transaction transaction) {
+    Transaction.Builder txBuilder = Transaction.builder();
+    txBuilder.copiedFrom(transaction);
+    final BlobsWithCommitments bwc = blobCache.getIfPresent(transaction.getHash());
+    if(bwc != null) {
+      txBuilder.blobsWithCommitments(bwc);
+      return Optional.of(txBuilder.build());
+    } else {
+      return Optional.empty();
+    }
+  }
 }

ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/sorter/GasPricePendingTransactionsSorter.java

@@ -17,6 +17,7 @@
 import static java.util.Comparator.comparing;
 
 import org.hyperledger.besu.ethereum.core.BlockHeader;
+import org.hyperledger.besu.ethereum.core.Transaction;
 import org.hyperledger.besu.ethereum.eth.transactions.PendingTransaction;
 import org.hyperledger.besu.ethereum.eth.transactions.TransactionPoolConfiguration;
 import org.hyperledger.besu.plugin.services.MetricsSystem;
@@ -25,6 +26,7 @@
 import java.util.Comparator;
 import java.util.Iterator;
 import java.util.NavigableSet;
+import java.util.Optional;
 import java.util.TreeSet;
 import java.util.function.Supplier;
 
@@ -57,6 +59,7 @@ public void reset() {
     prioritizedTransactions.clear();
   }
 
+
   @Override
   public void manageBlockAdded(final BlockHeader blockHeader) {
     // nothing to do
@@ -81,4 +84,5 @@ protected void removePrioritizedTransaction(final PendingTransaction removedPend
   protected PendingTransaction getLeastPriorityTransaction() {
     return prioritizedTransactions.last();
   }
+
 }

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

@@ -490,9 +490,10 @@ public void shouldReAddBlobTxsWhenReorgHappens() {
     final Block reorgFork3 = appendBlock(Difficulty.of(3000), reorgFork2.getHeader());
     verifyChainHeadIs(reorgFork3);
 
-    assertTransactionPending(transactionBlob);
     assertTransactionPending(transaction0);
     assertTransactionPending(transaction1);
+    assertTransactionPending(transactionBlob);
+
     Optional<Transaction> maybeBlob = transactions.getTransactionByHash(transactionBlob.getHash());
     assertThat(maybeBlob).isPresent();
     Transaction restoredBlob = maybeBlob.get();