Memoize transaction size and hash at the same time (hyperledger#4812)

Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net> Signed-off-by: Sally MacFarlane <macfarla.github@gmail.com>
macfarla · Jan 10, 2023 · e1e45a0 · e1e45a0
1 parent be8f54e
commit e1e45a0
Show file tree

Hide file tree

Showing 7 changed files with 39 additions and 16 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4,6 +4,7 @@
 ### Additions and Improvements
 - Add access list to Transaction Call Object [#4802](https://github.com/hyperledger/besu/issues/4801)
 - Add timestamp fork support, including shanghaiTime and cancunTime forks [#4743](https://github.com/hyperledger/besu/pull/4743)
+- Optimization:  Memoize transaction size and hash at the same time [#4812](https://github.com/hyperledger/besu/pull/4812)
 
 ### Breaking Changes
 

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
@@ -28,6 +28,7 @@
 import org.hyperledger.besu.datatypes.Wei;
 import org.hyperledger.besu.ethereum.core.encoding.TransactionDecoder;
 import org.hyperledger.besu.ethereum.core.encoding.TransactionEncoder;
+import org.hyperledger.besu.ethereum.rlp.BytesValueRLPOutput;
 import org.hyperledger.besu.ethereum.rlp.RLP;
 import org.hyperledger.besu.ethereum.rlp.RLPInput;
 import org.hyperledger.besu.ethereum.rlp.RLPOutput;
@@ -106,6 +107,8 @@ public class Transaction
 
   // Caches the hash used to uniquely identify the transaction.
   protected volatile Hash hash;
+  // Caches the size in bytes of the encoded transaction.
+  protected volatile int size = -1;
   private final TransactionType transactionType;
 
   private final SignatureAlgorithm signatureAlgorithm = SignatureAlgorithmFactory.getInstance();
@@ -610,11 +613,32 @@ public BigInteger getV() {
   @Override
   public Hash getHash() {
     if (hash == null) {
-      hash = Hash.hash(TransactionEncoder.encodeOpaqueBytes(this));
+      memoizeHashAndSize();
     }
     return hash;
   }
 
+  /**
+   * Returns the size in bytes of the encoded transaction.
+   *
+   * @return the size in bytes of the encoded transaction.
+   */
+  public int getSize() {
+    if (size == -1) {
+      memoizeHashAndSize();
+    }
+    return size;
+  }
+
+  private void memoizeHashAndSize() {
+    final Bytes bytes = TransactionEncoder.encodeOpaqueBytes(this);
+    hash = Hash.hash(bytes);
+
+    final BytesValueRLPOutput rlpOutput = new BytesValueRLPOutput();
+    TransactionEncoder.encodeForWire(transactionType, bytes, rlpOutput);
+    size = rlpOutput.encodedSize();
+  }
+
   /**
    * Returns whether the transaction is a contract creation
    *
@@ -926,14 +950,6 @@ public Optional<Address> contractAddress() {
     return Optional.empty();
   }
 
-  private Bytes toRlp() {
-    return RLP.encode(this::writeTo);
-  }
-
-  public int calculateSize() {
-    return toRlp().size();
-  }
-
   public static class Builder {
 
     protected TransactionType transactionType;

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
@@ -49,10 +49,16 @@ public static void encodeForWire(final Transaction transaction, final RLPOutput
     final TransactionType transactionType =
         checkNotNull(
             transaction.getType(), "Transaction type for %s was not specified.", transaction);
+    encodeForWire(transactionType, encodeOpaqueBytes(transaction), rlpOutput);
+  }
+
+  public static void encodeForWire(
+      final TransactionType transactionType, final Bytes opaqueBytes, final RLPOutput rlpOutput) {
+    checkNotNull(transactionType, "Transaction type was not specified.");
     if (TransactionType.FRONTIER.equals(transactionType)) {
-      encodeFrontier(transaction, rlpOutput);
+      rlpOutput.writeRaw(opaqueBytes);
     } else {
-      rlpOutput.writeBytes(encodeOpaqueBytes(transaction));
+      rlpOutput.writeBytes(opaqueBytes);
     }
   }
 

diff --git a/...ore/src/test/java/org/hyperledger/besu/ethereum/core/encoding/TransactionDecoderTest.java b/...ore/src/test/java/org/hyperledger/besu/ethereum/core/encoding/TransactionDecoderTest.java
@@ -39,7 +39,7 @@ class TransactionDecoderTest {
   private static final String FRONTIER_TX_RLP =
       "0xf901fc8032830138808080b901ae60056013565b6101918061001d6000396000f35b3360008190555056006001600060e060020a6000350480630a874df61461003a57806341c0e1b514610058578063a02b161e14610066578063dbbdf0831461007757005b610045600435610149565b80600160a060020a031660005260206000f35b610060610161565b60006000f35b6100716004356100d4565b60006000f35b61008560043560243561008b565b60006000f35b600054600160a060020a031632600160a060020a031614156100ac576100b1565b6100d0565b8060018360005260205260406000208190555081600060005260206000a15b5050565b600054600160a060020a031633600160a060020a031614158015610118575033600160a060020a0316600182600052602052604060002054600160a060020a031614155b61012157610126565b610146565b600060018260005260205260406000208190555080600060005260206000a15b50565b60006001826000526020526040600020549050919050565b600054600160a060020a031633600160a060020a0316146101815761018f565b600054600160a060020a0316ff5b561ca0c5689ed1ad124753d54576dfb4b571465a41900a1dff4058d8adf16f752013d0a01221cbd70ec28c94a3b55ec771bcbc70778d6ee0b51ca7ea9514594c861b1884";
   private static final String EIP1559_TX_RLP =
-      "b8a902f8a686796f6c6f7632800285012a05f20082753094000000000000000000000000000000000000aaaa8080f838f794000000000000000000000000000000000000aaaae1a0000000000000000000000000000000000000000000000000000000000000000001a00c1d69648e348fe26155b45de45004f0e4195f6352d8f0935bc93e98a3e2a862a060064e5b9765c0ac74223b0cf49635c59ae0faf82044fd17bcc68a549ade6f95";
+      "0xb8a902f8a686796f6c6f7632800285012a05f20082753094000000000000000000000000000000000000aaaa8080f838f794000000000000000000000000000000000000aaaae1a0000000000000000000000000000000000000000000000000000000000000000001a00c1d69648e348fe26155b45de45004f0e4195f6352d8f0935bc93e98a3e2a862a060064e5b9765c0ac74223b0cf49635c59ae0faf82044fd17bcc68a549ade6f95";
   private static final String GOQUORUM_PRIVATE_TX_RLP =
       "0xf88d0b808347b7608080b840290a80a37d198ff06abe189b638ff53ac8a8dc51a0aff07609d2aa75342783ae493b3e3c6b564c0eebe49284b05a0726fb33087b9e0231d349ea0c7b5661c8c526a07144db7045a395e608cda6ab051c86cc4fb42e319960b82087f3b26f0cbc3c2da00223ac129b22aec7a6c2ace3c3ef39c5eaaa54070fd82d8ee2140b0e70b1dca9";
   private static final String NONCE_64_BIT_MAX_MINUS_2_TX_RLP =
@@ -116,6 +116,6 @@ void shouldCalculateCorrectTransactionSize(final String rlp_tx, final String ign
     // Decode bytes into a transaction
     final Transaction transaction = TransactionDecoder.decodeForWire(RLP.input(bytes));
     // Bytes size should be equal to transaction size
-    assertThat(transaction.calculateSize()).isEqualTo(bytes.size());
+    assertThat(transaction.getSize()).isEqualTo(bytes.size());
   }
 }
diff --git a/.../main/java/org/hyperledger/besu/ethereum/eth/encoding/TransactionAnnouncementEncoder.java b/.../main/java/org/hyperledger/besu/ethereum/eth/encoding/TransactionAnnouncementEncoder.java
@@ -81,7 +81,7 @@ private static Bytes encodeForEth68(final List<Transaction> transactions) {
     transactions.forEach(
         transaction -> {
           types.add(transaction.getType());
-          sizes.add(transaction.calculateSize());
+          sizes.add(transaction.getSize());
           hashes.add(transaction.getHash());
         });
 

diff --git a/...src/main/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionAnnouncement.java b/...src/main/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionAnnouncement.java
@@ -40,7 +40,7 @@ public TransactionAnnouncement(final Transaction transaction) {
     this(
         checkNotNull(transaction, "Transaction cannot be null").getHash(),
         transaction.getType(),
-        (long) transaction.calculateSize());
+        (long) transaction.getSize());
   }
 
   public TransactionAnnouncement(final Hash hash, final TransactionType type, final Long size) {

diff --git a/...ledger/besu/ethereum/eth/transactions/NewPooledTransactionHashesMessageProcessorTest.java b/...ledger/besu/ethereum/eth/transactions/NewPooledTransactionHashesMessageProcessorTest.java
@@ -365,7 +365,7 @@ public void shouldEncodeAndDecodeTransactionAnnouncement_Eth68() {
       final TransactionAnnouncement announcement = announcementList.get(list.indexOf(transaction));
       assertThat(announcement.getHash()).isEqualTo(transaction.getHash());
       assertThat(announcement.getType()).hasValue(transaction.getType());
-      assertThat(announcement.getSize()).hasValue((long) transaction.calculateSize());
+      assertThat(announcement.getSize()).hasValue((long) transaction.getSize());
     }
   }