Fix for block import withdrawals (#5442)

* Refactor to remove duplicated code that was handling RLP read/write. This logic doesn't change that ofter having the read and writes spread accross the code base has tripped some devs. We are now handling all the Body readfrom rlp and write to rlp using methods of the BlockBody class. Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com> --------- Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>
hyperledger · May 9, 2023 · 7fff05a · 7fff05a
1 parent 2571353
commit 7fff05a
Show file tree

Hide file tree

Showing 19 changed files with 147 additions and 86 deletions.
diff --git a/besu/src/main/java/org/hyperledger/besu/chainimport/RlpBlockImporter.java b/besu/src/main/java/org/hyperledger/besu/chainimport/RlpBlockImporter.java
@@ -21,6 +21,7 @@
 import org.hyperledger.besu.ethereum.chain.MutableBlockchain;
 import org.hyperledger.besu.ethereum.core.Block;
 import org.hyperledger.besu.ethereum.core.BlockHeader;
+import org.hyperledger.besu.ethereum.core.BlockHeaderFunctions;
 import org.hyperledger.besu.ethereum.core.BlockImporter;
 import org.hyperledger.besu.ethereum.core.Difficulty;
 import org.hyperledger.besu.ethereum.core.Transaction;
@@ -102,13 +103,9 @@ public RlpBlockImporter.ImportResult importBlockchain(
     final ProtocolContext context = besuController.getProtocolContext();
     final MutableBlockchain blockchain = context.getBlockchain();
     int count = 0;
-
-    try (final RawBlockIterator iterator =
-        new RawBlockIterator(
-            blocks,
-            rlp ->
-                BlockHeader.readFrom(
-                    rlp, ScheduleBasedBlockHeaderFunctions.create(protocolSchedule)))) {
+    final BlockHeaderFunctions blockHeaderFunctions =
+        ScheduleBasedBlockHeaderFunctions.create(protocolSchedule);
+    try (final RawBlockIterator iterator = new RawBlockIterator(blocks, blockHeaderFunctions)) {
       BlockHeader previousHeader = null;
       CompletableFuture<Void> previousBlockFuture = null;
       final AtomicReference<Throwable> threadedException = new AtomicReference<>();

diff --git a/besu/src/main/java/org/hyperledger/besu/cli/subcommands/operator/RestoreState.java b/besu/src/main/java/org/hyperledger/besu/cli/subcommands/operator/RestoreState.java
@@ -145,7 +145,7 @@ private void restoreBlocks() throws IOException {
             BlockHeader.readFrom(
                 new BytesValueRLPInput(Bytes.wrap(headerEntry), false, true), functions);
         final BlockBody body =
-            BlockBody.readFrom(
+            BlockBody.readWrappedBodyFrom(
                 new BytesValueRLPInput(Bytes.wrap(bodyEntry), false, true), functions);
         final RLPInput receiptsRlp = new BytesValueRLPInput(Bytes.wrap(receiptEntry), false, true);
         final int receiptsCount = receiptsRlp.enterList();

diff --git a/besu/src/test/java/org/hyperledger/besu/chainexport/RlpBlockExporterTest.java b/besu/src/test/java/org/hyperledger/besu/chainexport/RlpBlockExporterTest.java
@@ -26,6 +26,7 @@
 import org.hyperledger.besu.ethereum.core.Block;
 import org.hyperledger.besu.ethereum.core.BlockBody;
 import org.hyperledger.besu.ethereum.core.BlockHeader;
+import org.hyperledger.besu.ethereum.core.BlockHeaderFunctions;
 import org.hyperledger.besu.ethereum.core.InMemoryKeyValueStorageProvider;
 import org.hyperledger.besu.ethereum.core.MiningParameters;
 import org.hyperledger.besu.ethereum.core.PrivacyParameters;
@@ -244,10 +245,9 @@ public void exportBlocks_outOfOrderBounds() throws IOException {
   }
 
   private RawBlockIterator getBlockIterator(final Path blocks) throws IOException {
-    return new RawBlockIterator(
-        blocks,
-        rlp ->
-            BlockHeader.readFrom(rlp, ScheduleBasedBlockHeaderFunctions.create(protocolSchedule)));
+    final BlockHeaderFunctions blockHeaderFunctions =
+        ScheduleBasedBlockHeaderFunctions.create(protocolSchedule);
+    return new RawBlockIterator(blocks, blockHeaderFunctions);
   }
 
   private Block getBlock(final Blockchain blockchain, final long blockNumber) {

diff --git a/...c/integration-test/java/org/hyperledger/besu/ethereum/api/jsonrpc/BlockchainImporter.java b/...c/integration-test/java/org/hyperledger/besu/ethereum/api/jsonrpc/BlockchainImporter.java
@@ -17,7 +17,7 @@
 import org.hyperledger.besu.config.GenesisConfigFile;
 import org.hyperledger.besu.ethereum.chain.GenesisState;
 import org.hyperledger.besu.ethereum.core.Block;
-import org.hyperledger.besu.ethereum.core.BlockHeader;
+import org.hyperledger.besu.ethereum.core.BlockHeaderFunctions;
 import org.hyperledger.besu.ethereum.mainnet.MainnetProtocolSchedule;
 import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule;
 import org.hyperledger.besu.ethereum.mainnet.ScheduleBasedBlockHeaderFunctions;
@@ -43,14 +43,11 @@ public BlockchainImporter(final URL blocksUrl, final String genesisJson) throws
     protocolSchedule =
         MainnetProtocolSchedule.fromConfig(
             GenesisConfigFile.fromConfig(genesisJson).getConfigOptions());
-
+    final BlockHeaderFunctions blockHeaderFunctions =
+        ScheduleBasedBlockHeaderFunctions.create(protocolSchedule);
     blocks = new ArrayList<>();
     try (final RawBlockIterator iterator =
-        new RawBlockIterator(
-            Paths.get(blocksUrl.toURI()),
-            rlp ->
-                BlockHeader.readFrom(
-                    rlp, ScheduleBasedBlockHeaderFunctions.create(protocolSchedule)))) {
+        new RawBlockIterator(Paths.get(blocksUrl.toURI()), blockHeaderFunctions)) {
       while (iterator.hasNext()) {
         blocks.add(iterator.next());
       }

diff --git a/...java/org/hyperledger/besu/ethereum/api/graphql/internal/pojoadapter/BlockAdapterBase.java b/...java/org/hyperledger/besu/ethereum/api/graphql/internal/pojoadapter/BlockAdapterBase.java
@@ -273,7 +273,7 @@ Optional<Bytes> getRaw(final DataFetchingEnvironment environment) {
         .map(
             blockBody -> {
               final BytesValueRLPOutput rlpOutput = new BytesValueRLPOutput();
-              blockBody.writeTo(rlpOutput);
+              blockBody.writeWrappedBodyTo(rlpOutput);
               return rlpOutput.encoded();
             });
   }

diff --git a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/query/StateBackupService.java b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/query/StateBackupService.java
@@ -307,7 +307,7 @@ private void backupChainData() throws IOException {
         headerWriter.writeBytes(headerOutput.encoded().toArrayUnsafe());
 
         final BytesValueRLPOutput bodyOutput = new BytesValueRLPOutput();
-        block.get().getBody().writeTo(bodyOutput);
+        block.get().getBody().writeWrappedBodyTo(bodyOutput);
         bodyWriter.writeBytes(bodyOutput.encoded().toArrayUnsafe());
 
         final BytesValueRLPOutput receiptsOutput = new BytesValueRLPOutput();

diff --git a/...est/java/org/hyperledger/besu/ethereum/api/graphql/AbstractEthGraphQLHttpServiceTest.java b/...est/java/org/hyperledger/besu/ethereum/api/graphql/AbstractEthGraphQLHttpServiceTest.java
@@ -24,7 +24,7 @@
 import org.hyperledger.besu.ethereum.chain.GenesisState;
 import org.hyperledger.besu.ethereum.chain.MutableBlockchain;
 import org.hyperledger.besu.ethereum.core.Block;
-import org.hyperledger.besu.ethereum.core.BlockHeader;
+import org.hyperledger.besu.ethereum.core.BlockHeaderFunctions;
 import org.hyperledger.besu.ethereum.core.BlockImporter;
 import org.hyperledger.besu.ethereum.core.DefaultSyncStatus;
 import org.hyperledger.besu.ethereum.core.InMemoryKeyValueStorageProvider;
@@ -106,11 +106,10 @@ public static void setupConstants() throws Exception {
 
     final URL genesisJsonUrl = BlockTestUtil.getTestGenesisUrl();
 
+    final BlockHeaderFunctions blockHeaderFunctions = new MainnetBlockHeaderFunctions();
     BLOCKS = new ArrayList<>();
     try (final RawBlockIterator iterator =
-        new RawBlockIterator(
-            Paths.get(blocksUrl.toURI()),
-            rlp -> BlockHeader.readFrom(rlp, new MainnetBlockHeaderFunctions()))) {
+        new RawBlockIterator(Paths.get(blocksUrl.toURI()), blockHeaderFunctions)) {
       while (iterator.hasNext()) {
         BLOCKS.add(iterator.next());
       }

diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/Block.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/Block.java
@@ -19,9 +19,7 @@
 import org.hyperledger.besu.ethereum.rlp.RLPInput;
 import org.hyperledger.besu.ethereum.rlp.RLPOutput;
 
-import java.util.List;
 import java.util.Objects;
-import java.util.Optional;
 
 import org.apache.tuweni.bytes.Bytes;
 
@@ -59,26 +57,18 @@ public void writeTo(final RLPOutput out) {
     out.startList();
 
     header.writeTo(out);
-    out.writeList(body.getTransactions(), Transaction::writeTo);
-    out.writeList(body.getOmmers(), BlockHeader::writeTo);
-    body.getWithdrawals().ifPresent(withdrawals -> out.writeList(withdrawals, Withdrawal::writeTo));
-    body.getDeposits().ifPresent(deposits -> out.writeList(deposits, Deposit::writeTo));
+    body.writeTo(out);
 
     out.endList();
   }
 
   public static Block readFrom(final RLPInput in, final BlockHeaderFunctions hashFunction) {
     in.enterList();
     final BlockHeader header = BlockHeader.readFrom(in, hashFunction);
-    final List<Transaction> transactions = in.readList(Transaction::readFrom);
-    final List<BlockHeader> ommers = in.readList(rlp -> BlockHeader.readFrom(rlp, hashFunction));
-    final Optional<List<Withdrawal>> withdrawals =
-        in.isEndOfCurrentList() ? Optional.empty() : Optional.of(in.readList(Withdrawal::readFrom));
-    final Optional<List<Deposit>> deposits =
-        in.isEndOfCurrentList() ? Optional.empty() : Optional.of(in.readList(Deposit::readFrom));
+    final BlockBody body = BlockBody.readFrom(in, hashFunction);
     in.leaveList();
 
-    return new Block(header, new BlockBody(transactions, ommers, withdrawals, deposits));
+    return new Block(header, body);
   }
 
   @Override

diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/BlockBody.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/BlockBody.java
@@ -28,7 +28,9 @@ public class BlockBody implements org.hyperledger.besu.plugin.data.BlockBody {
       new BlockBody(Collections.emptyList(), Collections.emptyList());
   /**
    * Adding a new field with a corresponding root hash in the block header will require a change in
-   * {@link org.hyperledger.besu.ethereum.eth.manager.task.GetBodiesFromPeerTask.BodyIdentifier }
+   * {@link org.hyperledger.besu.ethereum.eth.manager.task.GetBodiesFromPeerTask.BodyIdentifier}
+   * Also requires adding the new field to the constructor used in the {@link
+   * org.hyperledger.besu.ethereum.util.RawBlockIterator }
    */
   private final List<Transaction> transactions;
 
@@ -99,23 +101,35 @@ public Optional<List<Deposit>> getDeposits() {
    *
    * @param output Output to write to
    */
-  public void writeTo(final RLPOutput output) {
+  public void writeWrappedBodyTo(final RLPOutput output) {
     output.startList();
+    writeTo(output);
+    output.endList();
+  }
 
+  public void writeTo(final RLPOutput output) {
     output.writeList(getTransactions(), Transaction::writeTo);
     output.writeList(getOmmers(), BlockHeader::writeTo);
     withdrawals.ifPresent(withdrawals -> output.writeList(withdrawals, Withdrawal::writeTo));
     deposits.ifPresent(deposits -> output.writeList(deposits, Deposit::writeTo));
-
-    output.endList();
   }
 
-  public static BlockBody readFrom(
+  public static BlockBody readWrappedBodyFrom(
       final RLPInput input, final BlockHeaderFunctions blockHeaderFunctions) {
-    return readFrom(input, blockHeaderFunctions, false);
+    return readWrappedBodyFrom(input, blockHeaderFunctions, false);
   }
 
-  public static BlockBody readFrom(
+  /**
+   * Read all fields from the block body expecting a list wrapping them An example of valid body
+   * structure that this method would be able to read is: [[txs],[ommers],[withdrawals]] This is
+   * used for decoding list of bodies
+   *
+   * @param input The RLP-encoded input
+   * @param blockHeaderFunctions The block header functions used for parsing block headers
+   * @param allowEmptyBody A flag indicating whether an empty body is allowed
+   * @return the decoded BlockBody from the RLP
+   */
+  public static BlockBody readWrappedBodyFrom(
       final RLPInput input,
       final BlockHeaderFunctions blockHeaderFunctions,
       final boolean allowEmptyBody) {
@@ -125,21 +139,33 @@ public static BlockBody readFrom(
       input.leaveList();
       return empty();
     }
-    // TODO: Support multiple hard fork transaction formats.
-    final BlockBody body =
-        new BlockBody(
-            input.readList(Transaction::readFrom),
-            input.readList(rlp -> BlockHeader.readFrom(rlp, blockHeaderFunctions)),
-            input.isEndOfCurrentList()
-                ? Optional.empty()
-                : Optional.of(input.readList(Withdrawal::readFrom)),
-            input.isEndOfCurrentList()
-                ? Optional.empty()
-                : Optional.of(input.readList(Deposit::readFrom)));
+    final BlockBody body = readFrom(input, blockHeaderFunctions);
     input.leaveList();
     return body;
   }
 
+  /**
+   * Read all fields from the block body expecting no list wrapping them. An example of a valid body
+   * would be: [txs],[ommers],[withdrawals],[deposits] this method is called directly when importing
+   * a single block
+   *
+   * @param input The RLP-encoded input
+   * @param blockHeaderFunctions The block header functions used for parsing block headers
+   * @return the BlockBody decoded from the RLP
+   */
+  public static BlockBody readFrom(
+      final RLPInput input, final BlockHeaderFunctions blockHeaderFunctions) {
+    return new BlockBody(
+        input.readList(Transaction::readFrom),
+        input.readList(rlp -> BlockHeader.readFrom(rlp, blockHeaderFunctions)),
+        input.isEndOfCurrentList()
+            ? Optional.empty()
+            : Optional.of(input.readList(Withdrawal::readFrom)),
+        input.isEndOfCurrentList()
+            ? Optional.empty()
+            : Optional.of(input.readList(Deposit::readFrom)));
+  }
+
   @Override
   public boolean equals(final Object o) {
     if (this == o) return true;

diff --git a/...perledger/besu/ethereum/storage/keyvalue/KeyValueStoragePrefixedKeyBlockchainStorage.java b/...perledger/besu/ethereum/storage/keyvalue/KeyValueStoragePrefixedKeyBlockchainStorage.java
@@ -95,7 +95,7 @@ public Optional<BlockHeader> getBlockHeader(final Hash blockHash) {
   @Override
   public Optional<BlockBody> getBlockBody(final Hash blockHash) {
     return get(BLOCK_BODY_PREFIX, blockHash)
-        .map(bytes -> BlockBody.readFrom(RLP.input(bytes), blockHeaderFunctions));
+        .map(bytes -> BlockBody.readWrappedBodyFrom(RLP.input(bytes), blockHeaderFunctions));
   }
 
   @Override
@@ -151,7 +151,7 @@ public void putBlockHeader(final Hash blockHash, final BlockHeader blockHeader)
 
     @Override
     public void putBlockBody(final Hash blockHash, final BlockBody blockBody) {
-      set(BLOCK_BODY_PREFIX, blockHash, RLP.encode(blockBody::writeTo));
+      set(BLOCK_BODY_PREFIX, blockHash, RLP.encode(blockBody::writeWrappedBodyTo));
     }
 
     @Override

diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/util/RawBlockIterator.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/util/RawBlockIterator.java
@@ -17,7 +17,7 @@
 import org.hyperledger.besu.ethereum.core.Block;
 import org.hyperledger.besu.ethereum.core.BlockBody;
 import org.hyperledger.besu.ethereum.core.BlockHeader;
-import org.hyperledger.besu.ethereum.core.Transaction;
+import org.hyperledger.besu.ethereum.core.BlockHeaderFunctions;
 import org.hyperledger.besu.ethereum.rlp.BytesValueRLPInput;
 import org.hyperledger.besu.ethereum.rlp.RLP;
 import org.hyperledger.besu.ethereum.rlp.RLPInput;
@@ -29,33 +29,30 @@
 import java.nio.file.Path;
 import java.util.Iterator;
 import java.util.NoSuchElementException;
-import java.util.function.Function;
 
 import org.apache.tuweni.bytes.Bytes;
 
 public final class RawBlockIterator implements Iterator<Block>, Closeable {
   private static final int DEFAULT_INIT_BUFFER_CAPACITY = 1 << 16;
 
   private final FileChannel fileChannel;
-  private final Function<RLPInput, BlockHeader> headerReader;
   private ByteBuffer readBuffer;
+  private final BlockHeaderFunctions blockHeaderFunctions;
 
   private Block next;
 
   RawBlockIterator(
-      final Path file,
-      final Function<RLPInput, BlockHeader> headerReader,
-      final int initialCapacity)
+      final Path file, final BlockHeaderFunctions blockHeaderFunctions, final int initialCapacity)
       throws IOException {
+    this.blockHeaderFunctions = blockHeaderFunctions;
     fileChannel = FileChannel.open(file);
-    this.headerReader = headerReader;
     readBuffer = ByteBuffer.allocate(initialCapacity);
     nextBlock();
   }
 
-  public RawBlockIterator(final Path file, final Function<RLPInput, BlockHeader> headerReader)
+  public RawBlockIterator(final Path file, final BlockHeaderFunctions blockHeaderFunctions)
       throws IOException {
-    this(file, headerReader, DEFAULT_INIT_BUFFER_CAPACITY);
+    this(file, blockHeaderFunctions, DEFAULT_INIT_BUFFER_CAPACITY);
   }
 
   @Override
@@ -99,9 +96,8 @@ private void nextBlock() throws IOException {
       final Bytes rlpBytes = Bytes.wrap(Bytes.wrapByteBuffer(readBuffer, 0, length).toArray());
       final RLPInput rlp = new BytesValueRLPInput(rlpBytes, false);
       rlp.enterList();
-      final BlockHeader header = headerReader.apply(rlp);
-      final BlockBody body =
-          new BlockBody(rlp.readList(Transaction::readFrom), rlp.readList(headerReader));
+      final BlockHeader header = BlockHeader.readFrom(rlp, blockHeaderFunctions);
+      final BlockBody body = BlockBody.readFrom(rlp, blockHeaderFunctions);
       next = new Block(header, body);
       readBuffer.position(length);
       readBuffer.compact();

diff --git a/...eum/core/src/test-support/java/org/hyperledger/besu/ethereum/core/BlockSyncTestUtils.java b/...eum/core/src/test-support/java/org/hyperledger/besu/ethereum/core/BlockSyncTestUtils.java
@@ -37,10 +37,9 @@ public static List<Block> firstBlocks(final int count) {
     try {
       temp.create();
       final Path blocks = temp.newFile().toPath();
+      final BlockHeaderFunctions blockHeaderFunctions = new MainnetBlockHeaderFunctions();
       BlockTestUtil.write1000Blocks(blocks);
-      try (final RawBlockIterator iterator =
-          new RawBlockIterator(
-              blocks, rlp -> BlockHeader.readFrom(rlp, new MainnetBlockHeaderFunctions()))) {
+      try (final RawBlockIterator iterator = new RawBlockIterator(blocks, blockHeaderFunctions)) {
         for (int i = 0; i < count; ++i) {
           result.add(iterator.next());
         }

diff --git a/...um/core/src/test-support/java/org/hyperledger/besu/ethereum/core/BlockchainSetupUtil.java b/...um/core/src/test-support/java/org/hyperledger/besu/ethereum/core/BlockchainSetupUtil.java
@@ -184,8 +184,7 @@ private static BlockchainSetupUtil create(
       final BlockHeaderFunctions blockHeaderFunctions =
           ScheduleBasedBlockHeaderFunctions.create(protocolSchedule);
       try (final RawBlockIterator iterator =
-          new RawBlockIterator(
-              blocksPath, rlp -> BlockHeader.readFrom(rlp, blockHeaderFunctions))) {
+          new RawBlockIterator(blocksPath, blockHeaderFunctions)) {
         while (iterator.hasNext()) {
           blocks.add(iterator.next());
         }

diff --git a/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/util/RawBlockIteratorTest.java b/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/util/RawBlockIteratorTest.java
@@ -19,6 +19,7 @@
 import org.hyperledger.besu.ethereum.core.Block;
 import org.hyperledger.besu.ethereum.core.BlockDataGenerator;
 import org.hyperledger.besu.ethereum.core.BlockHeader;
+import org.hyperledger.besu.ethereum.core.BlockHeaderFunctions;
 import org.hyperledger.besu.ethereum.core.Transaction;
 import org.hyperledger.besu.ethereum.mainnet.MainnetBlockHeaderFunctions;
 import org.hyperledger.besu.ethereum.rlp.BytesValueRLPOutput;
@@ -77,14 +78,11 @@ public void readsBlocksWithInitialCapacity(
       }
     }
     writer.close();
-
+    final BlockHeaderFunctions blockHeaderFunctions = new MainnetBlockHeaderFunctions();
     // Read blocks
     final int initialCapacity = initialCapacityFromBlockSize.apply(firstSerializedBlock.length);
     final RawBlockIterator iterator =
-        new RawBlockIterator(
-            blocksFile.toPath(),
-            rlp -> BlockHeader.readFrom(rlp, new MainnetBlockHeaderFunctions()),
-            initialCapacity);
+        new RawBlockIterator(blocksFile.toPath(), blockHeaderFunctions, initialCapacity);
 
     // Read blocks and check that they match
     for (int i = 0; i < blockCount; i++) {

diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthServer.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthServer.java
@@ -197,7 +197,7 @@ static MessageData constructGetBodiesResponse(
 
       final BlockBody body = maybeBody.get();
       final BytesValueRLPOutput bodyOutput = new BytesValueRLPOutput();
-      body.writeTo(bodyOutput);
+      body.writeWrappedBodyTo(bodyOutput);
       final int encodedSize = bodyOutput.encodedSize();
       if (responseSizeEstimate + encodedSize > maxMessageSize) {
         break;