Improve the selection of the most profitable built block (#7174)

* Improve the selection of the most profitable built block

Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>

* Update CHANGELOG

Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>

* Update consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/PayloadWrapper.java

Co-authored-by: Justin Florentine <justin+github@florentine.us>
Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>

---------

Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
Co-authored-by: Justin Florentine <justin+github@florentine.us>

CHANGELOG.md

@@ -20,6 +20,7 @@
 - Promote experimental --Xbonsai-limit-trie-logs-enabled to production-ready, --bonsai-limit-trie-logs-enabled [#7192](https://github.com/hyperledger/besu/pull/7192)
 - Promote experimental --Xbonsai-trie-logs-pruning-window-size to production-ready, --bonsai-trie-logs-pruning-window-size [#7192](https://github.com/hyperledger/besu/pull/7192)
 - `admin_nodeInfo` JSON/RPC call returns the currently active EVM version [#7127](https://github.com/hyperledger/besu/pull/7127)
+- Improve the selection of the most profitable built block [#7174](https://github.com/hyperledger/besu/pull/7174)
 
 ### Bug fixes
 - Make `eth_gasPrice` aware of the base fee market [#7102](https://github.com/hyperledger/besu/pull/7102)

consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/MergeContext.java

@@ -18,7 +18,6 @@ import org.hyperledger.besu.consensus.merge.blockcreation.PayloadIdentifier;
 import org.hyperledger.besu.datatypes.Hash;
 import org.hyperledger.besu.ethereum.ConsensusContext;
 import org.hyperledger.besu.ethereum.core.BlockHeader;
-import org.hyperledger.besu.ethereum.core.BlockWithReceipts;
 import org.hyperledger.besu.ethereum.core.Difficulty;
 import org.hyperledger.besu.ethereum.eth.sync.state.SyncState;
 
@@ -167,7 +166,7 @@ public interface MergeContext extends ConsensusContext {
    * @param payloadId the payload identifier
    * @return the optional block with receipts
    */
-  Optional<BlockWithReceipts> retrieveBlockById(final PayloadIdentifier payloadId);
+  Optional<PayloadWrapper> retrievePayloadById(final PayloadIdentifier payloadId);
 
   /**
    * Is configured for a post-merge from genesis.

consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/PayloadWrapper.java

@@ -15,13 +15,53 @@
 package org.hyperledger.besu.consensus.merge;
 
 import org.hyperledger.besu.consensus.merge.blockcreation.PayloadIdentifier;
+import org.hyperledger.besu.datatypes.Wei;
+import org.hyperledger.besu.ethereum.core.BlockValueCalculator;
 import org.hyperledger.besu.ethereum.core.BlockWithReceipts;
 
+/** Wrapper for payload plus extra info. */
+public class PayloadWrapper {
+  private final PayloadIdentifier payloadIdentifier;
+  private final BlockWithReceipts blockWithReceipts;
+  private final Wei blockValue;
+
   /**
- * Wrapper for payload plus extra info.
+   * Construct a wrapper with the following fields.
    *
    * @param payloadIdentifier Payload identifier
- * @param blockWithReceipts Block With Receipts
+   * @param blockWithReceipts Block with receipts
+   */
+  public PayloadWrapper(
+      final PayloadIdentifier payloadIdentifier, final BlockWithReceipts blockWithReceipts) {
+    this.blockWithReceipts = blockWithReceipts;
+    this.payloadIdentifier = payloadIdentifier;
+    this.blockValue = BlockValueCalculator.calculateBlockValue(blockWithReceipts);
+  }
+
+  /**
+   * Get the block value
+   *
+   * @return block value in Wei
+   */
+  public Wei blockValue() {
+    return blockValue;
+  }
+
+  /**
+   * Get this payload identifier
+   *
+   * @return payload identifier
+   */
+  public PayloadIdentifier payloadIdentifier() {
+    return payloadIdentifier;
+  }
+
+  /**
+   * Get the block with receipts
+   *
+   * @return block with receipts
    */
-public record PayloadWrapper(
+  public BlockWithReceipts blockWithReceipts() {
-    PayloadIdentifier payloadIdentifier, BlockWithReceipts blockWithReceipts) {}
+    return blockWithReceipts;
+  }
+}

consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/PostMergeContext.java

@@ -16,12 +16,9 @@ package org.hyperledger.besu.consensus.merge;
 
 import org.hyperledger.besu.consensus.merge.blockcreation.PayloadIdentifier;
 import org.hyperledger.besu.datatypes.Hash;
-import org.hyperledger.besu.datatypes.Wei;
 import org.hyperledger.besu.ethereum.ConsensusContext;
 import org.hyperledger.besu.ethereum.core.Block;
 import org.hyperledger.besu.ethereum.core.BlockHeader;
-import org.hyperledger.besu.ethereum.core.BlockValueCalculator;
-import org.hyperledger.besu.ethereum.core.BlockWithReceipts;
 import org.hyperledger.besu.ethereum.core.Difficulty;
 import org.hyperledger.besu.ethereum.eth.sync.state.SyncState;
 import org.hyperledger.besu.util.Subscribers;
@@ -29,7 +26,6 @@ import org.hyperledger.besu.util.Subscribers;
 import java.util.Comparator;
 import java.util.Optional;
 import java.util.concurrent.atomic.AtomicReference;
-import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
 import com.google.common.annotations.VisibleForTesting;
@@ -45,13 +41,6 @@ public class PostMergeContext implements MergeContext {
   static final int MAX_BLOCKS_IN_PROGRESS = 12;
 
   private static final AtomicReference<PostMergeContext> singleton = new AtomicReference<>();
-
-  private static final Comparator<BlockWithReceipts> compareByGasUsedDesc =
-      Comparator.comparingLong(
-              (BlockWithReceipts blockWithReceipts) ->
-                  blockWithReceipts.getBlock().getHeader().getGasUsed())
-          .reversed();
-
   private final AtomicReference<SyncState> syncState;
   private final AtomicReference<Difficulty> terminalTotalDifficulty;
   // initial postMerge state is indeterminate until it is set:
@@ -70,7 +59,6 @@ public class PostMergeContext implements MergeContext {
   private final AtomicReference<BlockHeader> lastSafeBlock = new AtomicReference<>();
   private final AtomicReference<Optional<BlockHeader>> terminalPoWBlock =
       new AtomicReference<>(Optional.empty());
-  private final BlockValueCalculator blockValueCalculator = new BlockValueCalculator();
   private boolean isPostMergeAtGenesis;
 
   /** Instantiates a new Post merge context. */
@@ -227,66 +215,65 @@ public class PostMergeContext implements MergeContext {
   }
 
   @Override
-  public void putPayloadById(final PayloadWrapper payloadWrapper) {
+  public void putPayloadById(final PayloadWrapper newPayload) {
+    final var newBlockWithReceipts = newPayload.blockWithReceipts();
+    final var newBlockValue = newPayload.blockValue();
+
     synchronized (blocksInProgress) {
-      final Optional<BlockWithReceipts> maybeCurrBestBlock =
+      final Optional<PayloadWrapper> maybeCurrBestPayload =
-          retrieveBlockById(payloadWrapper.payloadIdentifier());
+          retrievePayloadById(newPayload.payloadIdentifier());
 
-      maybeCurrBestBlock.ifPresentOrElse(
+      maybeCurrBestPayload.ifPresent(
-          currBestBlock -> {
+          currBestPayload -> {
-            if (compareByGasUsedDesc.compare(payloadWrapper.blockWithReceipts(), currBestBlock)
+            if (newBlockValue.greaterThan(currBestPayload.blockValue())) {
-                < 0) {
               LOG.atDebug()
-                  .setMessage("New proposal for payloadId {} {} is better than the previous one {}")
+                  .setMessage(
-                  .addArgument(payloadWrapper.payloadIdentifier())
+                      "New proposal for payloadId {} {} is better than the previous one {} by {}")
+                  .addArgument(newPayload.payloadIdentifier())
+                  .addArgument(() -> logBlockProposal(newBlockWithReceipts.getBlock()))
                   .addArgument(
-                      () -> logBlockProposal(payloadWrapper.blockWithReceipts().getBlock()))
+                      () -> logBlockProposal(currBestPayload.blockWithReceipts().getBlock()))
-                  .addArgument(() -> logBlockProposal(currBestBlock.getBlock()))
+                  .addArgument(
+                      () ->
+                          newBlockValue
+                              .subtract(currBestPayload.blockValue())
+                              .toHumanReadableString())
                   .log();
+
               blocksInProgress.removeAll(
-                  retrievePayloadsById(payloadWrapper.payloadIdentifier())
+                  streamPayloadsById(newPayload.payloadIdentifier()).toList());
-                      .collect(Collectors.toUnmodifiableList()));
+
-              blocksInProgress.add(
+              logCurrentBestBlock(newPayload);
-                  new PayloadWrapper(
+            }
-                      payloadWrapper.payloadIdentifier(), payloadWrapper.blockWithReceipts()));
+          });
-              logCurrentBestBlock(payloadWrapper.blockWithReceipts());
+      blocksInProgress.add(newPayload);
-            }
-          },
-          () ->
-              blocksInProgress.add(
-                  new PayloadWrapper(
-                      payloadWrapper.payloadIdentifier(), payloadWrapper.blockWithReceipts())));
     }
   }
 
-  private void logCurrentBestBlock(final BlockWithReceipts blockWithReceipts) {
+  private void logCurrentBestBlock(final PayloadWrapper payloadWrapper) {
     if (LOG.isDebugEnabled()) {
-      final Block block = blockWithReceipts.getBlock();
+      final Block block = payloadWrapper.blockWithReceipts().getBlock();
       final float gasUsedPerc =
           100.0f * block.getHeader().getGasUsed() / block.getHeader().getGasLimit();
       final int txsNum = block.getBody().getTransactions().size();
-      final Wei reward = blockValueCalculator.calculateBlockValue(blockWithReceipts);
 
       LOG.debug(
           "Current best proposal for block {}: txs {}, gas used {}%, reward {}",
-          blockWithReceipts.getNumber(),
+          block.getHeader().getNumber(),
           txsNum,
           String.format("%1.2f", gasUsedPerc),
-          reward.toHumanReadableString());
+          payloadWrapper.blockValue().toHumanReadableString());
     }
   }
 
   @Override
-  public Optional<BlockWithReceipts> retrieveBlockById(final PayloadIdentifier payloadId) {
+  public Optional<PayloadWrapper> retrievePayloadById(final PayloadIdentifier payloadId) {
     synchronized (blocksInProgress) {
-      return retrievePayloadsById(payloadId)
+      return streamPayloadsById(payloadId).max(Comparator.comparing(PayloadWrapper::blockValue));
-          .map(payloadWrapper -> payloadWrapper.blockWithReceipts())
-          .sorted(compareByGasUsedDesc)
-          .findFirst();
     }
   }
 
-  private Stream<PayloadWrapper> retrievePayloadsById(final PayloadIdentifier payloadId) {
+  private Stream<PayloadWrapper> streamPayloadsById(final PayloadIdentifier payloadId) {
     return blocksInProgress.stream().filter(z -> z.payloadIdentifier().equals(payloadId));
   }
 

consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/TransitionContext.java

@@ -18,7 +18,6 @@ import org.hyperledger.besu.consensus.merge.blockcreation.PayloadIdentifier;
 import org.hyperledger.besu.datatypes.Hash;
 import org.hyperledger.besu.ethereum.ConsensusContext;
 import org.hyperledger.besu.ethereum.core.BlockHeader;
-import org.hyperledger.besu.ethereum.core.BlockWithReceipts;
 import org.hyperledger.besu.ethereum.core.Difficulty;
 import org.hyperledger.besu.ethereum.eth.sync.state.SyncState;
 
@@ -146,8 +145,8 @@ public class TransitionContext implements MergeContext {
   }
 
   @Override
-  public Optional<BlockWithReceipts> retrieveBlockById(final PayloadIdentifier payloadId) {
+  public Optional<PayloadWrapper> retrievePayloadById(final PayloadIdentifier payloadId) {
-    return postMergeContext.retrieveBlockById(payloadId);
+    return postMergeContext.retrievePayloadById(payloadId);
   }
 
   @Override

consensus/merge/src/test/java/org/hyperledger/besu/consensus/merge/PostMergeContextTest.java

@@ -26,6 +26,7 @@ import static org.mockito.Mockito.when;
 
 import org.hyperledger.besu.consensus.merge.blockcreation.PayloadIdentifier;
 import org.hyperledger.besu.datatypes.Hash;
+import org.hyperledger.besu.datatypes.Wei;
 import org.hyperledger.besu.ethereum.core.Block;
 import org.hyperledger.besu.ethereum.core.BlockHeader;
 import org.hyperledger.besu.ethereum.core.BlockWithReceipts;
@@ -138,9 +139,12 @@ public class PostMergeContextTest {
     BlockWithReceipts mockBlockWithReceipts = createBlockWithReceipts(1, 21000, 1);
 
     PayloadIdentifier firstPayloadId = new PayloadIdentifier(1L);
-    postMergeContext.putPayloadById(new PayloadWrapper(firstPayloadId, mockBlockWithReceipts));
+    final var payloadWrapper = createPayloadWrapper(firstPayloadId, mockBlockWithReceipts, Wei.ONE);
+    postMergeContext.putPayloadById(payloadWrapper);
 
-    assertThat(postMergeContext.retrieveBlockById(firstPayloadId)).contains(mockBlockWithReceipts);
+    assertThat(postMergeContext.retrievePayloadById(firstPayloadId))
+        .map(PayloadWrapper::blockWithReceipts)
+        .contains(mockBlockWithReceipts);
   }
 
   @Test
@@ -149,10 +153,16 @@ public class PostMergeContextTest {
     BlockWithReceipts betterBlockWithReceipts = createBlockWithReceipts(2, 11, 1);
 
     PayloadIdentifier payloadId = new PayloadIdentifier(1L);
-    postMergeContext.putPayloadById(new PayloadWrapper(payloadId, zeroTxBlockWithReceipts));
+    final var zeroTxPayloadWrapper =
-    postMergeContext.putPayloadById(new PayloadWrapper(payloadId, betterBlockWithReceipts));
+        createPayloadWrapper(payloadId, zeroTxBlockWithReceipts, Wei.ZERO);
-
+    final var betterPayloadWrapper =
-    assertThat(postMergeContext.retrieveBlockById(payloadId)).contains(betterBlockWithReceipts);
+        createPayloadWrapper(payloadId, betterBlockWithReceipts, Wei.ONE);
+    postMergeContext.putPayloadById(zeroTxPayloadWrapper);
+    postMergeContext.putPayloadById(betterPayloadWrapper);
+
+    assertThat(postMergeContext.retrievePayloadById(payloadId))
+        .map(PayloadWrapper::blockWithReceipts)
+        .contains(betterBlockWithReceipts);
   }
 
   @Test
@@ -162,25 +172,33 @@ public class PostMergeContextTest {
     BlockWithReceipts smallBlockWithReceipts = createBlockWithReceipts(3, 5, 1);
 
     PayloadIdentifier payloadId = new PayloadIdentifier(1L);
-    postMergeContext.putPayloadById(new PayloadWrapper(payloadId, zeroTxBlockWithReceipts));
+    final var zeroTxPayloadWrapper =
-    postMergeContext.putPayloadById(new PayloadWrapper(payloadId, betterBlockWithReceipts));
+        createPayloadWrapper(payloadId, zeroTxBlockWithReceipts, Wei.ZERO);
-    postMergeContext.putPayloadById(new PayloadWrapper(payloadId, smallBlockWithReceipts));
+    final var betterPayloadWrapper =
-
+        createPayloadWrapper(payloadId, betterBlockWithReceipts, Wei.of(2));
-    assertThat(postMergeContext.retrieveBlockById(payloadId)).contains(betterBlockWithReceipts);
+    final var smallPayloadWrapper =
+        createPayloadWrapper(payloadId, smallBlockWithReceipts, Wei.ONE);
+    postMergeContext.putPayloadById(zeroTxPayloadWrapper);
+    postMergeContext.putPayloadById(betterPayloadWrapper);
+    postMergeContext.putPayloadById(smallPayloadWrapper);
+
+    assertThat(postMergeContext.retrievePayloadById(payloadId))
+        .map(PayloadWrapper::blockWithReceipts)
+        .contains(betterBlockWithReceipts);
   }
 
   @Test
   public void tryingToRetrieveANotYetPutPayloadIdReturnsEmpty() {
     PayloadIdentifier payloadId = new PayloadIdentifier(1L);
 
-    assertThat(postMergeContext.retrieveBlockById(payloadId)).isEmpty();
+    assertThat(postMergeContext.retrievePayloadById(payloadId)).isEmpty();
   }
 
   @Test
   public void tryingToRetrieveABlockPutButEvictedReturnsEmpty() {
     PayloadIdentifier evictedPayloadId = new PayloadIdentifier(0L);
 
-    assertThat(postMergeContext.retrieveBlockById(evictedPayloadId)).isEmpty();
+    assertThat(postMergeContext.retrievePayloadById(evictedPayloadId)).isEmpty();
   }
 
   @Test
@@ -209,6 +227,17 @@ public class PostMergeContextTest {
     assertThat(postMergeContext.isSyncing()).isFalse();
   }
 
+  private PayloadWrapper createPayloadWrapper(
+      final PayloadIdentifier firstPayloadId,
+      final BlockWithReceipts mockBlockWithReceipts,
+      final Wei blockValue) {
+    final var payloadWrapper = mock(PayloadWrapper.class);
+    when(payloadWrapper.payloadIdentifier()).thenReturn(firstPayloadId);
+    when(payloadWrapper.blockWithReceipts()).thenReturn(mockBlockWithReceipts);
+    when(payloadWrapper.blockValue()).thenReturn(blockValue);
+    return payloadWrapper;
+  }
+
   private static BlockWithReceipts createBlockWithReceipts(
       final int number, final long gasUsed, final int txCount) {
     Block mockBlock = mock(Block.class, RETURNS_DEEP_STUBS);

ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/AbstractEngineGetPayload.java

@@ -14,9 +14,9 @@
  */
 package org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods.engine;
 
+import org.hyperledger.besu.consensus.merge.PayloadWrapper;
 import org.hyperledger.besu.consensus.merge.blockcreation.MergeMiningCoordinator;
 import org.hyperledger.besu.consensus.merge.blockcreation.PayloadIdentifier;
-import org.hyperledger.besu.datatypes.Wei;
 import org.hyperledger.besu.ethereum.ProtocolContext;
 import org.hyperledger.besu.ethereum.api.jsonrpc.internal.JsonRpcRequestContext;
 import org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods.ExecutionEngineJsonRpcMethod;
@@ -70,10 +70,9 @@ public abstract class AbstractEngineGetPayload extends ExecutionEngineJsonRpcMet
 
     final PayloadIdentifier payloadId = request.getRequiredParameter(0, PayloadIdentifier.class);
     mergeMiningCoordinator.finalizeProposalById(payloadId);
-    final Optional<BlockWithReceipts> blockWithReceipts =
+    final Optional<PayloadWrapper> maybePayload = mergeContext.get().retrievePayloadById(payloadId);
-        mergeContext.get().retrieveBlockById(payloadId);
+    if (maybePayload.isPresent()) {
-    if (blockWithReceipts.isPresent()) {
+      final BlockWithReceipts proposal = maybePayload.get().blockWithReceipts();
-      final BlockWithReceipts proposal = blockWithReceipts.get();
       LOG.atDebug()
           .setMessage("assembledBlock for payloadId {}: {}")
           .addArgument(() -> payloadId)
@@ -85,37 +84,33 @@ public abstract class AbstractEngineGetPayload extends ExecutionEngineJsonRpcMet
       if (!forkValidationResult.isValid()) {
         return new JsonRpcErrorResponse(request.getRequest().getId(), forkValidationResult);
       }
-      return createResponse(request, payloadId, proposal);
+      return createResponse(request, maybePayload.get());
     }
     return new JsonRpcErrorResponse(request.getRequest().getId(), RpcErrorType.UNKNOWN_PAYLOAD);
   }
 
-  protected void logProposal(
+  protected void logProposal(final PayloadWrapper payload) {
-      final PayloadIdentifier payloadId,
+    final BlockHeader proposalHeader = payload.blockWithReceipts().getHeader();
-      final BlockWithReceipts proposal,
-      final Optional<Wei> maybeReward) {
-    final BlockHeader proposalHeader = proposal.getHeader();
     final float gasUsedPerc = 100.0f * proposalHeader.getGasUsed() / proposalHeader.getGasLimit();
 
     final String message =
         "Fetch block proposal by identifier: {}, hash: {}, "
             + "number: {}, coinbase: {}, transaction count: {}, gas used: {}%"
-            + maybeReward.map(unused -> ", reward: {}").orElse("{}");
+            + " reward: {}";
 
     LOG.atInfo()
         .setMessage(message)
-        .addArgument(payloadId::toHexString)
+        .addArgument(payload.payloadIdentifier()::toHexString)
         .addArgument(proposalHeader::getHash)
         .addArgument(proposalHeader::getNumber)
         .addArgument(proposalHeader::getCoinbase)
-        .addArgument(() -> proposal.getBlock().getBody().getTransactions().size())
+        .addArgument(
+            () -> payload.blockWithReceipts().getBlock().getBody().getTransactions().size())
         .addArgument(() -> String.format("%1.2f", gasUsedPerc))
-        .addArgument(maybeReward.map(Wei::toHumanReadableString).orElse(""))
+        .addArgument(payload.blockValue()::toHumanReadableString)
         .log();
   }
 
   protected abstract JsonRpcResponse createResponse(
-      final JsonRpcRequestContext request,
+      final JsonRpcRequestContext request, final PayloadWrapper payload);
-      final PayloadIdentifier payloadId,
-      final BlockWithReceipts blockWithReceipts);
 }

ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineGetPayloadV1.java

@@ -14,17 +14,14 @@
  */
 package org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods.engine;
 
+import org.hyperledger.besu.consensus.merge.PayloadWrapper;
 import org.hyperledger.besu.consensus.merge.blockcreation.MergeMiningCoordinator;
-import org.hyperledger.besu.consensus.merge.blockcreation.PayloadIdentifier;
 import org.hyperledger.besu.ethereum.ProtocolContext;
 import org.hyperledger.besu.ethereum.api.jsonrpc.RpcMethod;
 import org.hyperledger.besu.ethereum.api.jsonrpc.internal.JsonRpcRequestContext;
 import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcResponse;
 import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcSuccessResponse;
 import org.hyperledger.besu.ethereum.api.jsonrpc.internal.results.BlockResultFactory;
-import org.hyperledger.besu.ethereum.core.BlockWithReceipts;
-
-import java.util.Optional;
 
 import io.vertx.core.Vertx;
 
@@ -46,12 +43,10 @@ public class EngineGetPayloadV1 extends AbstractEngineGetPayload {
 
   @Override
   protected JsonRpcResponse createResponse(
-      final JsonRpcRequestContext request,
+      final JsonRpcRequestContext request, final PayloadWrapper payload) {
-      final PayloadIdentifier payloadId,
-      final BlockWithReceipts blockWithReceipts) {
     final var result =
-        blockResultFactory.payloadTransactionCompleteV1(blockWithReceipts.getBlock());
+        blockResultFactory.payloadTransactionCompleteV1(payload.blockWithReceipts().getBlock());
-    logProposal(payloadId, blockWithReceipts, Optional.empty());
+    logProposal(payload);
     return new JsonRpcSuccessResponse(request.getRequest().getId(), result);
   }
 }

ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineGetPayloadV2.java

@@ -14,9 +14,8 @@
  */
 package org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods.engine;
 
+import org.hyperledger.besu.consensus.merge.PayloadWrapper;
 import org.hyperledger.besu.consensus.merge.blockcreation.MergeMiningCoordinator;
-import org.hyperledger.besu.consensus.merge.blockcreation.PayloadIdentifier;
-import org.hyperledger.besu.datatypes.Wei;
 import org.hyperledger.besu.ethereum.ProtocolContext;
 import org.hyperledger.besu.ethereum.api.jsonrpc.RpcMethod;
 import org.hyperledger.besu.ethereum.api.jsonrpc.internal.JsonRpcRequestContext;
@@ -24,7 +23,6 @@ import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcRespon
 import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcSuccessResponse;
 import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.RpcErrorType;
 import org.hyperledger.besu.ethereum.api.jsonrpc.internal.results.BlockResultFactory;
-import org.hyperledger.besu.ethereum.core.BlockWithReceipts;
 import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule;
 import org.hyperledger.besu.ethereum.mainnet.ScheduledProtocolSpec;
 import org.hyperledger.besu.ethereum.mainnet.ValidationResult;
@@ -61,12 +59,9 @@ public class EngineGetPayloadV2 extends AbstractEngineGetPayload {
 
   @Override
   protected JsonRpcResponse createResponse(
-      final JsonRpcRequestContext request,
+      final JsonRpcRequestContext request, final PayloadWrapper payload) {
-      final PayloadIdentifier payloadId,
+    final var result = blockResultFactory.payloadTransactionCompleteV2(payload);
-      final BlockWithReceipts blockWithReceipts) {
+    logProposal(payload);
-    final var result = blockResultFactory.payloadTransactionCompleteV2(blockWithReceipts);
-    logProposal(
-        payloadId, blockWithReceipts, Optional.of(Wei.fromHexString(result.getBlockValue())));
     return new JsonRpcSuccessResponse(request.getRequest().getId(), result);
   }
 

ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineGetPayloadV3.java

@@ -14,8 +14,8 @@
  */
 package org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods.engine;
 
+import org.hyperledger.besu.consensus.merge.PayloadWrapper;
 import org.hyperledger.besu.consensus.merge.blockcreation.MergeMiningCoordinator;
-import org.hyperledger.besu.consensus.merge.blockcreation.PayloadIdentifier;
 import org.hyperledger.besu.ethereum.ProtocolContext;
 import org.hyperledger.besu.ethereum.api.jsonrpc.RpcMethod;
 import org.hyperledger.besu.ethereum.api.jsonrpc.internal.JsonRpcRequestContext;
@@ -23,7 +23,6 @@ import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcRespon
 import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcSuccessResponse;
 import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.RpcErrorType;
 import org.hyperledger.besu.ethereum.api.jsonrpc.internal.results.BlockResultFactory;
-import org.hyperledger.besu.ethereum.core.BlockWithReceipts;
 import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule;
 import org.hyperledger.besu.ethereum.mainnet.ScheduledProtocolSpec;
 import org.hyperledger.besu.ethereum.mainnet.ValidationResult;
@@ -60,13 +59,10 @@ public class EngineGetPayloadV3 extends AbstractEngineGetPayload {
 
   @Override
   protected JsonRpcResponse createResponse(
-      final JsonRpcRequestContext request,
+      final JsonRpcRequestContext request, final PayloadWrapper payload) {
-      final PayloadIdentifier payloadId,
-      final BlockWithReceipts blockWithReceipts) {
 
     return new JsonRpcSuccessResponse(
-        request.getRequest().getId(),
+        request.getRequest().getId(), blockResultFactory.payloadTransactionCompleteV3(payload));
-        blockResultFactory.payloadTransactionCompleteV3(blockWithReceipts));
   }
 
   @Override

ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineGetPayloadV4.java

@@ -14,8 +14,8 @@
  */
 package org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods.engine;
 
+import org.hyperledger.besu.consensus.merge.PayloadWrapper;
 import org.hyperledger.besu.consensus.merge.blockcreation.MergeMiningCoordinator;
-import org.hyperledger.besu.consensus.merge.blockcreation.PayloadIdentifier;
 import org.hyperledger.besu.ethereum.ProtocolContext;
 import org.hyperledger.besu.ethereum.api.jsonrpc.RpcMethod;
 import org.hyperledger.besu.ethereum.api.jsonrpc.internal.JsonRpcRequestContext;
@@ -23,7 +23,6 @@ import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcRespon
 import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcSuccessResponse;
 import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.RpcErrorType;
 import org.hyperledger.besu.ethereum.api.jsonrpc.internal.results.BlockResultFactory;
-import org.hyperledger.besu.ethereum.core.BlockWithReceipts;
 import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule;
 import org.hyperledger.besu.ethereum.mainnet.ScheduledProtocolSpec;
 import org.hyperledger.besu.ethereum.mainnet.ValidationResult;
@@ -60,13 +59,10 @@ public class EngineGetPayloadV4 extends AbstractEngineGetPayload {
 
   @Override
   protected JsonRpcResponse createResponse(
-      final JsonRpcRequestContext request,
+      final JsonRpcRequestContext request, final PayloadWrapper payload) {
-      final PayloadIdentifier payloadId,
-      final BlockWithReceipts blockWithReceipts) {
 
     return new JsonRpcSuccessResponse(
-        request.getRequest().getId(),
+        request.getRequest().getId(), blockResultFactory.payloadTransactionCompleteV4(payload));
-        blockResultFactory.payloadTransactionCompleteV4(blockWithReceipts));
   }
 
   @Override

ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/results/BlockResultFactory.java

@@ -17,16 +17,14 @@ package org.hyperledger.besu.ethereum.api.jsonrpc.internal.results;
 import static org.hyperledger.besu.ethereum.mainnet.requests.RequestUtil.getDepositRequests;
 import static org.hyperledger.besu.ethereum.mainnet.requests.RequestUtil.getWithdrawalRequests;
 
+import org.hyperledger.besu.consensus.merge.PayloadWrapper;
 import org.hyperledger.besu.datatypes.Hash;
-import org.hyperledger.besu.datatypes.Wei;
 import org.hyperledger.besu.ethereum.api.jsonrpc.internal.results.EngineGetPayloadBodiesResultV1.PayloadBody;
 import org.hyperledger.besu.ethereum.api.query.BlockWithMetadata;
 import org.hyperledger.besu.ethereum.api.query.TransactionWithMetadata;
 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.BlockValueCalculator;
-import org.hyperledger.besu.ethereum.core.BlockWithReceipts;
 import org.hyperledger.besu.ethereum.core.encoding.EncodingContext;
 import org.hyperledger.besu.ethereum.core.encoding.TransactionEncoder;
 
@@ -108,8 +106,8 @@ public class BlockResultFactory {
     return new EngineGetPayloadResultV1(block.getHeader(), txs);
   }
 
-  public EngineGetPayloadResultV2 payloadTransactionCompleteV2(
+  public EngineGetPayloadResultV2 payloadTransactionCompleteV2(final PayloadWrapper payload) {
-      final BlockWithReceipts blockWithReceipts) {
+    final var blockWithReceipts = payload.blockWithReceipts();
     final List<String> txs =
         blockWithReceipts.getBlock().getBody().getTransactions().stream()
             .map(
@@ -118,12 +116,11 @@ public class BlockResultFactory {
             .map(Bytes::toHexString)
             .collect(Collectors.toList());
 
-    final Wei blockValue = new BlockValueCalculator().calculateBlockValue(blockWithReceipts);
     return new EngineGetPayloadResultV2(
         blockWithReceipts.getHeader(),
         txs,
         blockWithReceipts.getBlock().getBody().getWithdrawals(),
-        Quantity.create(blockValue));
+        Quantity.create(payload.blockValue()));
   }
 
   public EngineGetPayloadBodiesResultV1 payloadBodiesCompleteV1(
@@ -135,8 +132,8 @@ public class BlockResultFactory {
     return new EngineGetPayloadBodiesResultV1(payloadBodies);
   }
 
-  public EngineGetPayloadResultV3 payloadTransactionCompleteV3(
+  public EngineGetPayloadResultV3 payloadTransactionCompleteV3(final PayloadWrapper payload) {
-      final BlockWithReceipts blockWithReceipts) {
+    final var blockWithReceipts = payload.blockWithReceipts();
     final List<String> txs =
         blockWithReceipts.getBlock().getBody().getTransactions().stream()
             .map(
@@ -145,20 +142,18 @@ public class BlockResultFactory {
             .map(Bytes::toHexString)
             .collect(Collectors.toList());
 
-    final Wei blockValue = new BlockValueCalculator().calculateBlockValue(blockWithReceipts);
-
     final BlobsBundleV1 blobsBundleV1 =
         new BlobsBundleV1(blockWithReceipts.getBlock().getBody().getTransactions());
     return new EngineGetPayloadResultV3(
         blockWithReceipts.getHeader(),
         txs,
         blockWithReceipts.getBlock().getBody().getWithdrawals(),
-        Quantity.create(blockValue),
+        Quantity.create(payload.blockValue()),
         blobsBundleV1);
   }
 
-  public EngineGetPayloadResultV4 payloadTransactionCompleteV4(
+  public EngineGetPayloadResultV4 payloadTransactionCompleteV4(final PayloadWrapper payload) {
-      final BlockWithReceipts blockWithReceipts) {
+    final var blockWithReceipts = payload.blockWithReceipts();
     final List<String> txs =
         blockWithReceipts.getBlock().getBody().getTransactions().stream()
             .map(
@@ -167,8 +162,6 @@ public class BlockResultFactory {
             .map(Bytes::toHexString)
             .collect(Collectors.toList());
 
-    final Wei blockValue = new BlockValueCalculator().calculateBlockValue(blockWithReceipts);
-
     final BlobsBundleV1 blobsBundleV1 =
         new BlobsBundleV1(blockWithReceipts.getBlock().getBody().getTransactions());
     return new EngineGetPayloadResultV4(
@@ -177,7 +170,7 @@ public class BlockResultFactory {
         blockWithReceipts.getBlock().getBody().getWithdrawals(),
         getDepositRequests(blockWithReceipts.getBlock().getBody().getRequests()),
         getWithdrawalRequests(blockWithReceipts.getBlock().getBody().getRequests()),
-        Quantity.create(blockValue),
+        Quantity.create(payload.blockValue()),
         blobsBundleV1);
   }
 

ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/AbstractEngineGetPayloadTest.java

@@ -20,6 +20,7 @@ import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
 import org.hyperledger.besu.consensus.merge.MergeContext;
+import org.hyperledger.besu.consensus.merge.PayloadWrapper;
 import org.hyperledger.besu.consensus.merge.blockcreation.MergeMiningCoordinator;
 import org.hyperledger.besu.consensus.merge.blockcreation.PayloadIdentifier;
 import org.hyperledger.besu.crypto.KeyPair;
@@ -97,6 +98,8 @@ public abstract class AbstractEngineGetPayloadTest extends AbstractScheduledApiT
       new Block(mockHeader, new BlockBody(Collections.emptyList(), Collections.emptyList()));
   protected static final BlockWithReceipts mockBlockWithReceipts =
       new BlockWithReceipts(mockBlock, Collections.emptyList());
+  protected static final PayloadWrapper mockPayload =
+      new PayloadWrapper(mockPid, mockBlockWithReceipts);
   private static final Block mockBlockWithWithdrawals =
       new Block(
           mockHeader,
@@ -115,9 +118,13 @@ public abstract class AbstractEngineGetPayloadTest extends AbstractScheduledApiT
               Optional.of(Collections.emptyList())));
   protected static final BlockWithReceipts mockBlockWithReceiptsAndWithdrawals =
       new BlockWithReceipts(mockBlockWithWithdrawals, Collections.emptyList());
+  protected static final PayloadWrapper mockPayloadWithWithdrawals =
+      new PayloadWrapper(mockPid, mockBlockWithReceiptsAndWithdrawals);
 
   protected static final BlockWithReceipts mockBlockWithReceiptsAndDepositRequests =
       new BlockWithReceipts(mockBlockWithDepositRequests, Collections.emptyList());
+  protected static final PayloadWrapper mockPayloadWithDepositRequests =
+      new PayloadWrapper(mockPid, mockBlockWithReceiptsAndDepositRequests);
 
   @Mock protected ProtocolContext protocolContext;
 
@@ -130,7 +137,7 @@ public abstract class AbstractEngineGetPayloadTest extends AbstractScheduledApiT
   @Override
   public void before() {
     super.before();
-    when(mergeContext.retrieveBlockById(mockPid)).thenReturn(Optional.of(mockBlockWithReceipts));
+    when(mergeContext.retrievePayloadById(mockPid)).thenReturn(Optional.of(mockPayload));
     when(protocolContext.safeConsensusContext(Mockito.any())).thenReturn(Optional.of(mergeContext));
     if (methodFactory.isPresent()) {
       this.method =

ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineGetPayloadV2Test.java

@@ -48,9 +48,7 @@ public class EngineGetPayloadV2Test extends AbstractEngineGetPayloadTest {
   @Override
   public void before() {
     super.before();
-    lenient()
+    lenient().when(mergeContext.retrievePayloadById(mockPid)).thenReturn(Optional.of(mockPayload));
-        .when(mergeContext.retrieveBlockById(mockPid))
-        .thenReturn(Optional.of(mockBlockWithReceipts));
     when(protocolContext.safeConsensusContext(Mockito.any())).thenReturn(Optional.of(mergeContext));
     this.method =
         new EngineGetPayloadV2(
@@ -72,8 +70,8 @@ public class EngineGetPayloadV2Test extends AbstractEngineGetPayloadTest {
   @Test
   public void shouldReturnBlockForKnownPayloadId() {
     // should return withdrawals for a post-Shanghai block
-    when(mergeContext.retrieveBlockById(mockPid))
+    when(mergeContext.retrievePayloadById(mockPid))
-        .thenReturn(Optional.of(mockBlockWithReceiptsAndWithdrawals));
+        .thenReturn(Optional.of(mockPayloadWithWithdrawals));
 
     final var resp = resp(RpcMethod.ENGINE_GET_PAYLOAD_V2.getMethodName(), mockPid);
     assertThat(resp).isInstanceOf(JsonRpcSuccessResponse.class);

ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineGetPayloadV3Test.java

@@ -21,6 +21,7 @@ import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
+import org.hyperledger.besu.consensus.merge.PayloadWrapper;
 import org.hyperledger.besu.consensus.merge.blockcreation.PayloadIdentifier;
 import org.hyperledger.besu.datatypes.Address;
 import org.hyperledger.besu.datatypes.BlobGas;
@@ -67,9 +68,7 @@ public class EngineGetPayloadV3Test extends AbstractEngineGetPayloadTest {
   @Override
   public void before() {
     super.before();
-    lenient()
+    lenient().when(mergeContext.retrievePayloadById(mockPid)).thenReturn(Optional.of(mockPayload));
-        .when(mergeContext.retrieveBlockById(mockPid))
-        .thenReturn(Optional.of(mockBlockWithReceipts));
     when(protocolContext.safeConsensusContext(Mockito.any())).thenReturn(Optional.of(mergeContext));
     this.method =
         new EngineGetPayloadV3(
@@ -132,8 +131,10 @@ public class EngineGetPayloadV3Test extends AbstractEngineGetPayloadTest {
                     Optional.of(Collections.emptyList()),
                     Optional.of(Collections.emptyList()))),
             List.of(blobReceipt));
+    PayloadWrapper payloadPostCancun = new PayloadWrapper(postCancunPid, postCancunBlock);
 
-    when(mergeContext.retrieveBlockById(postCancunPid)).thenReturn(Optional.of(postCancunBlock));
+    when(mergeContext.retrievePayloadById(postCancunPid))
+        .thenReturn(Optional.of(payloadPostCancun));
 
     final var resp = resp(RpcMethod.ENGINE_GET_PAYLOAD_V3.getMethodName(), postCancunPid);
     assertThat(resp).isInstanceOf(JsonRpcSuccessResponse.class);

ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineGetPayloadV4Test.java

@@ -21,6 +21,7 @@ import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
+import org.hyperledger.besu.consensus.merge.PayloadWrapper;
 import org.hyperledger.besu.consensus.merge.blockcreation.PayloadIdentifier;
 import org.hyperledger.besu.datatypes.Address;
 import org.hyperledger.besu.datatypes.BlobGas;
@@ -70,8 +71,8 @@ public class EngineGetPayloadV4Test extends AbstractEngineGetPayloadTest {
   public void before() {
     super.before();
     lenient()
-        .when(mergeContext.retrieveBlockById(mockPid))
+        .when(mergeContext.retrievePayloadById(mockPid))
-        .thenReturn(Optional.of(mockBlockWithReceiptsAndDepositRequests));
+        .thenReturn(Optional.of(mockPayloadWithDepositRequests));
     when(protocolContext.safeConsensusContext(Mockito.any())).thenReturn(Optional.of(mergeContext));
     this.method =
         new EngineGetPayloadV4(
@@ -134,8 +135,9 @@ public class EngineGetPayloadV4Test extends AbstractEngineGetPayloadTest {
                     Optional.of(Collections.emptyList()),
                     Optional.of(Collections.emptyList()))),
             List.of(blobReceipt));
+    PayloadWrapper payload = new PayloadWrapper(payloadIdentifier, block);
 
-    when(mergeContext.retrieveBlockById(payloadIdentifier)).thenReturn(Optional.of(block));
+    when(mergeContext.retrievePayloadById(payloadIdentifier)).thenReturn(Optional.of(payload));
 
     final var resp = resp(RpcMethod.ENGINE_GET_PAYLOAD_V4.getMethodName(), payloadIdentifier);
     assertThat(resp).isInstanceOf(JsonRpcSuccessResponse.class);

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

@@ -20,7 +20,7 @@ import java.util.List;
 
 public class BlockValueCalculator {
 
-  public Wei calculateBlockValue(final BlockWithReceipts blockWithReceipts) {
+  public static Wei calculateBlockValue(final BlockWithReceipts blockWithReceipts) {
     final Block block = blockWithReceipts.getBlock();
     final List<Transaction> txs = block.getBody().getTransactions();
     final List<TransactionReceipt> receipts = blockWithReceipts.getReceipts();

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

@@ -41,8 +41,8 @@ public class BlockValueCalculatorTest {
     final Block block =
         new Block(blockHeader, new BlockBody(Collections.emptyList(), Collections.emptyList()));
     Wei blockValue =
-        new BlockValueCalculator()
+        BlockValueCalculator.calculateBlockValue(
-            .calculateBlockValue(new BlockWithReceipts(block, Collections.emptyList()));
+            new BlockWithReceipts(block, Collections.emptyList()));
     assertThat(blockValue).isEqualTo(Wei.ZERO);
   }
 
@@ -85,8 +85,7 @@ public class BlockValueCalculatorTest {
     final Block block =
         new Block(blockHeader, new BlockBody(List.of(tx1, tx2, tx3), Collections.emptyList()));
     Wei blockValue =
-        new BlockValueCalculator()
+        BlockValueCalculator.calculateBlockValue(
-            .calculateBlockValue(
             new BlockWithReceipts(block, List.of(receipt1, receipt2, receipt3)));
     // Block value = 71 * 1 + (143-71) * 2 + (214-143) * 5 = 1427
     assertThat(blockValue).isEqualTo(Wei.of(570L));
@@ -114,8 +113,7 @@ public class BlockValueCalculatorTest {
     final Block block =
         new Block(blockHeader, new BlockBody(List.of(tx1), Collections.emptyList()));
     Wei blockValue =
-        new BlockValueCalculator()
+        BlockValueCalculator.calculateBlockValue(new BlockWithReceipts(block, List.of(receipt1)));
-            .calculateBlockValue(new BlockWithReceipts(block, List.of(receipt1)));
     // Block value =~ max_long * 2
     assertThat(blockValue).isGreaterThan(Wei.of(Long.MAX_VALUE));
   }