7582: Finish applying naive RLP caching to transaction encoder/decode…

…r classes Signed-off-by: Matilda Clerke <[email protected]>
hyperledger · Jan 21, 2025 · cb03e27 · cb03e27
1 parent 3e68934
commit cb03e27
Show file tree

Hide file tree

Showing 10 changed files with 95 additions and 29 deletions.
diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/Transaction.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/Transaction.java
@@ -489,9 +489,7 @@ private Bytes32 getOrComputeSenderRecoveryHash() {
    * @param out the output to write the transaction to
    */
   public void writeTo(final RLPOutput out) {
-    rawRlp.ifPresentOrElse(
-        out::writeRLPBytes,
-        () -> TransactionEncoder.encodeRLP(this, out, EncodingContext.BLOCK_BODY));
+    TransactionEncoder.encodeRLP(this, out, EncodingContext.BLOCK_BODY);
   }
 
   @Override

diff --git a/...c/main/java/org/hyperledger/besu/ethereum/core/encoding/AccessListTransactionEncoder.java b/...c/main/java/org/hyperledger/besu/ethereum/core/encoding/AccessListTransactionEncoder.java
@@ -34,7 +34,9 @@ public static void encode(final Transaction transaction, final RLPOutput rlpOutp
     transaction
         .getRawRlp()
         .ifPresentOrElse(
-            rlpOutput::writeRLPBytes,
+            (rawRlp) ->
+                rlpOutput.writeRLPBytes(
+                    Bytes.concatenate(Bytes.of(transaction.getType().getSerializedType()), rawRlp)),
             () -> {
               rlpOutput.startList();
               encodeAccessListInner(
@@ -106,8 +108,9 @@ public static void writeAccessList(
             out.writeBytes(accessListEntry.address());
             out.writeList(
                 accessListEntry.storageKeys(),
-                (storageKeyBytes, storageKeyBytesRLPOutput) ->
-                    storageKeyBytesRLPOutput.writeBytes(storageKeyBytes));
+                (storageKeyBytes, storageKeyBytesRLPOutput) -> {
+                  storageKeyBytesRLPOutput.writeBytes(storageKeyBytes);
+                });
             accessListEntryRLPOutput.endList();
           });
     }

diff --git a/...in/java/org/hyperledger/besu/ethereum/core/encoding/CodeDelegationTransactionEncoder.java b/...in/java/org/hyperledger/besu/ethereum/core/encoding/CodeDelegationTransactionEncoder.java
@@ -66,7 +66,9 @@ public static void encode(final Transaction transaction, final RLPOutput out) {
     transaction
         .getRawRlp()
         .ifPresentOrElse(
-            out::writeRLPBytes,
+            (rawRlp) ->
+                out.writeRLPBytes(
+                    Bytes.concatenate(Bytes.of(transaction.getType().getSerializedType()), rawRlp)),
             () -> {
               out.startList();
               out.writeBigIntegerScalar(transaction.getChainId().orElseThrow());

diff --git a/.../src/main/java/org/hyperledger/besu/ethereum/core/encoding/EIP1559TransactionEncoder.java b/.../src/main/java/org/hyperledger/besu/ethereum/core/encoding/EIP1559TransactionEncoder.java
@@ -28,7 +28,9 @@ public static void encode(final Transaction transaction, final RLPOutput out) {
     transaction
         .getRawRlp()
         .ifPresentOrElse(
-            out::writeRLPBytes,
+            (rawRlp) ->
+                out.writeRLPBytes(
+                    Bytes.concatenate(Bytes.of(transaction.getType().getSerializedType()), rawRlp)),
             () -> {
               out.startList();
               out.writeBigIntegerScalar(transaction.getChainId().orElseThrow());

diff --git a/...um/core/src/main/java/org/hyperledger/besu/ethereum/core/encoding/TransactionDecoder.java b/...um/core/src/main/java/org/hyperledger/besu/ethereum/core/encoding/TransactionDecoder.java
@@ -79,12 +79,7 @@ private static Transaction decodeTypedTransaction(
     TransactionType transactionType =
         getTransactionType(typedTransactionBytes)
             .orElseThrow((() -> new IllegalArgumentException("Unsupported transaction type")));
-    Transaction typedTransaction =
-        decodeTypedTransaction(typedTransactionBytes, transactionType, context);
-    return Transaction.builder()
-        .copiedFrom(typedTransaction)
-        .rawRlp(Optional.of(typedTransactionBytes))
-        .build();
+    return decodeTypedTransaction(typedTransactionBytes, transactionType, context);
   }
 
   /**

diff --git a/...um/core/src/main/java/org/hyperledger/besu/ethereum/core/encoding/TransactionEncoder.java b/...um/core/src/main/java/org/hyperledger/besu/ethereum/core/encoding/TransactionEncoder.java
@@ -57,15 +57,10 @@ public static void encodeRLP(
       final Transaction transaction,
       final RLPOutput rlpOutput,
       final EncodingContext encodingContext) {
-    transaction
-        .getRawRlp()
-        .ifPresentOrElse(
-            rlpOutput::writeRLPBytes,
-            () -> {
-              final TransactionType transactionType = getTransactionType(transaction);
-              Bytes opaqueBytes = encodeOpaqueBytes(transaction, encodingContext);
-              encodeRLP(transactionType, opaqueBytes, rlpOutput);
-            });
+    final TransactionType transactionType = getTransactionType(transaction);
+
+    Bytes opaqueBytes = encodeOpaqueBytes(transaction, encodingContext);
+    encodeRLP(transactionType, opaqueBytes, rlpOutput);
   }
 
   /**
@@ -103,7 +98,9 @@ public static Bytes encodeOpaqueBytes(
       transaction
           .getRawRlp()
           .ifPresentOrElse(
-              out::writeRLPBytes,
+              (rawRlp) ->
+                  out.writeRLPBytes(
+                      Bytes.concatenate(Bytes.of(transactionType.getSerializedType()), rawRlp)),
               () -> {
                 out.writeByte(transaction.getType().getSerializedType());
                 encoder.encode(transaction, out);

diff --git a/.../src/test/java/org/hyperledger/besu/ethereum/core/encoding/TransactionRLPDecoderTest.java b/.../src/test/java/org/hyperledger/besu/ethereum/core/encoding/TransactionRLPDecoderTest.java
@@ -19,7 +19,9 @@
 import static org.hyperledger.besu.evm.account.Account.MAX_NONCE;
 import static org.junit.jupiter.api.Assumptions.assumeTrue;
 
+import org.hyperledger.besu.datatypes.TransactionType;
 import org.hyperledger.besu.datatypes.Wei;
+import org.hyperledger.besu.ethereum.core.BlockDataGenerator;
 import org.hyperledger.besu.ethereum.core.Transaction;
 import org.hyperledger.besu.ethereum.rlp.RLP;
 import org.hyperledger.besu.ethereum.rlp.RLPException;
@@ -33,8 +35,11 @@
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.MethodSource;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 class TransactionRLPDecoderTest {
+  private static final Logger LOG = LoggerFactory.getLogger(TransactionRLPDecoderTest.class);
 
   private static final String FRONTIER_TX_RLP =
       "0xf901fc8032830138808080b901ae60056013565b6101918061001d6000396000f35b3360008190555056006001600060e060020a6000350480630a874df61461003a57806341c0e1b514610058578063a02b161e14610066578063dbbdf0831461007757005b610045600435610149565b80600160a060020a031660005260206000f35b610060610161565b60006000f35b6100716004356100d4565b60006000f35b61008560043560243561008b565b60006000f35b600054600160a060020a031632600160a060020a031614156100ac576100b1565b6100d0565b8060018360005260205260406000208190555081600060005260206000a15b5050565b600054600160a060020a031633600160a060020a031614158015610118575033600160a060020a0316600182600052602052604060002054600160a060020a031614155b61012157610126565b610146565b600060018260005260205260406000208190555080600060005260206000a15b50565b60006001826000526020526040600020549050919050565b600054600160a060020a031633600160a060020a0316146101815761018f565b600054600160a060020a0316ff5b561ca0c5689ed1ad124753d54576dfb4b571465a41900a1dff4058d8adf16f752013d0a01221cbd70ec28c94a3b55ec771bcbc70778d6ee0b51ca7ea9514594c861b1884";
@@ -79,6 +84,20 @@ void shouldDecodeWithHighNonce() {
     assertThat(transaction.getNonce()).isEqualTo(MAX_NONCE - 1);
   }
 
+  @Test
+  void testForAccessListTransaction() {
+    BlockDataGenerator gen = new BlockDataGenerator();
+    Transaction accessListTransaction = gen.transaction(TransactionType.ACCESS_LIST);
+    Bytes encodedBytes =
+        TransactionEncoder.encodeOpaqueBytes(accessListTransaction, EncodingContext.BLOCK_BODY);
+    Transaction decodedTransaction =
+        TransactionDecoder.decodeOpaqueBytes(encodedBytes, EncodingContext.BLOCK_BODY);
+    assertThat(accessListTransaction).isEqualTo(decodedTransaction);
+    Bytes reencodedBytes =
+        TransactionEncoder.encodeOpaqueBytes(decodedTransaction, EncodingContext.BLOCK_BODY);
+    assertThat(encodedBytes).isEqualTo(reencodedBytes);
+  }
+
   private static Collection<Object[]> dataTransactionSize() {
     return Arrays.asList(
         new Object[][] {

diff --git a/ethereum/rlp/build.gradle b/ethereum/rlp/build.gradle
@@ -30,13 +30,14 @@ jar {
 }
 
 dependencies {
+  api 'org.slf4j:slf4j-api'
+
   annotationProcessor 'org.openjdk.jmh:jmh-generator-annprocess'
 
   implementation 'io.tmio:tuweni-bytes'
   implementation 'io.tmio:tuweni-units'
   implementation 'com.google.guava:guava'
 
-
   jmh project(':util')
 
   testImplementation project(':testutil')

diff --git a/ethereum/rlp/src/main/java/org/hyperledger/besu/ethereum/rlp/AbstractRLPInput.java b/ethereum/rlp/src/main/java/org/hyperledger/besu/ethereum/rlp/AbstractRLPInput.java
@@ -494,7 +494,11 @@ public int enterList() {
    * @return -1 if skipCount==true, otherwise, the number of item of the entered list.
    */
   public int enterList(final boolean skipCount) {
-    LOG.trace("Entering list (depth = {}, endOfListOffset = {}, currentItem = {})", depth, endOfListOffset, currentItem);
+    LOG.trace(
+        "Entering list (depth = {}, endOfListOffset = {}, currentItem = {})",
+        depth,
+        endOfListOffset,
+        currentItem);
     if (currentItem >= size) {
       throw error("Cannot enter a lists, input is fully consumed");
     }
@@ -537,7 +541,13 @@ public int enterList(final boolean skipCount) {
 
     // And lastly reset on the list first element before returning
     setTo(listStart);
-    LOG.trace("Entered list (depth = {}, endOfListOffset = {}, currentItem = {}, listEnd = {}, elements = {})", depth, endOfListOffset, currentItem, listEnd, count);
+    LOG.trace(
+        "Entered list (depth = {}, endOfListOffset = {}, currentItem = {}, listEnd = {}, elements = {})",
+        depth,
+        endOfListOffset,
+        currentItem,
+        listEnd,
+        count);
     return count;
   }
 
@@ -552,7 +562,12 @@ public void leaveListLenient() {
   }
 
   private void leaveList(final boolean ignoreRest) {
-    LOG.trace("Leaving list (depth = {}, endOfListOffset = {}, ignoreRest = {}, currentItem = {}", depth, endOfListOffset, ignoreRest, currentItem);
+    LOG.trace(
+        "Leaving list (depth = {}, endOfListOffset = {}, ignoreRest = {}, currentItem = {}",
+        depth,
+        endOfListOffset,
+        ignoreRest,
+        currentItem);
     checkState(depth > 0, "Not within an RLP list");
 
     if (!ignoreRest) {
@@ -561,7 +576,12 @@ private void leaveList(final boolean ignoreRest) {
     }
 
     --depth;
-    LOG.trace("Left list (depth = {}, endOfListOffset = {}, ignoreRes = {}, currentItem = {}", depth, endOfListOffset, ignoreRest, currentItem);
+    LOG.trace(
+        "Left list (depth = {}, endOfListOffset = {}, ignoreRes = {}, currentItem = {}",
+        depth,
+        endOfListOffset,
+        ignoreRest,
+        currentItem);
   }
 
   @Override

diff --git a/ethereum/rlp/src/main/java/org/hyperledger/besu/ethereum/rlp/AbstractRLPOutput.java b/ethereum/rlp/src/main/java/org/hyperledger/besu/ethereum/rlp/AbstractRLPOutput.java
@@ -23,8 +23,11 @@
 
 import org.apache.tuweni.bytes.Bytes;
 import org.apache.tuweni.bytes.MutableBytes;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 abstract class AbstractRLPOutput implements RLPOutput {
+  private static final Logger LOG = LoggerFactory.getLogger(AbstractRLPOutput.class);
   /*
    * The algorithm implemented works as follows:
    *
@@ -79,10 +82,12 @@ private int currentList() {
 
   @Override
   public void writeBytes(final Bytes v) {
+    LOG.trace("Writing bytes {}", v);
     checkState(
         stackSize > 1 || values.isEmpty(), "Terminated RLP output, cannot add more elements");
     values.add(v);
     payloadSizes[currentList()] += RLPEncodingHelpers.elementSize(v);
+    LOG.trace("Wrote bytes {}", v);
   }
 
   @Override
@@ -97,6 +102,12 @@ public void writeRaw(final Bytes v) {
 
   @Override
   public void startList() {
+    LOG.trace(
+        "Starting list (stackSize = {}, listsCount = {}, payloadSizes = {}, parentListStack = {})",
+        stackSize,
+        listsCount,
+        payloadSizes,
+        parentListStack);
     values.add(LIST_MARKER);
     ++listsCount; // we'll add a new element to payloadSizes
     ++stackSize; // and to the list stack.
@@ -112,10 +123,22 @@ public void startList() {
     // The new current list size is store in the slot we just made room for by incrementing
     // listsCount
     parentListStack[stackSize - 1] = listsCount - 1;
+    LOG.trace(
+        "Started list (stackSize = {}, listsCount = {}, payloadSizes = {}, parentListStack = {})",
+        stackSize,
+        listsCount,
+        payloadSizes,
+        parentListStack);
   }
 
   @Override
   public void endList() {
+    LOG.trace(
+        "Ending list (stackSize = {}, listsCount = {}, payloadSizes = {}, parentListStack = {})",
+        stackSize,
+        listsCount,
+        payloadSizes,
+        parentListStack);
     checkState(stackSize > 1, "LeaveList() called with no prior matching startList()");
 
     final int current = currentList();
@@ -125,6 +148,12 @@ public void endList() {
     // We just finished an item of our parent list, add it to that parent list size now.
     final int newCurrent = currentList();
     payloadSizes[newCurrent] += finishedListSize;
+    LOG.trace(
+        "Ended list (stackSize = {}, listsCount = {}, payloadSizes = {}, parentListStack = {})",
+        stackSize,
+        listsCount,
+        payloadSizes,
+        parentListStack);
   }
 
   /**