From 34b6b1ce171f0cf2228369e5a77bd09382aabf1a Mon Sep 17 00:00:00 2001 From: Sally MacFarlane Date: Thu, 21 Sep 2023 12:36:15 +1000 Subject: [PATCH 01/11] renamed PayloadTuple and made a separate class [skip ci] Signed-off-by: Sally MacFarlane --- .../besu/consensus/merge/MergeContext.java | 7 ++++ .../consensus/merge/PayloadWithExtra.java | 33 +++++++++++++++++++ .../consensus/merge/PostMergeContext.java | 31 ++++------------- 3 files changed, 46 insertions(+), 25 deletions(-) create mode 100644 consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/PayloadWithExtra.java diff --git a/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/MergeContext.java b/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/MergeContext.java index d23fff04414..c9b58a215bf 100644 --- a/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/MergeContext.java +++ b/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/MergeContext.java @@ -154,6 +154,13 @@ void fireNewUnverifiedForkchoiceEvent( */ boolean validateCandidateHead(final BlockHeader candidateHeader); + /** + * Put payload containing Identifier. + * + * @param payload the payload + */ + default void putPayload(final PayloadWithExtra payload) {putPayloadById(payload.payloadIdentifier, payload.blockWithReceipts);}; + /** * Put payload by Identifier. * diff --git a/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/PayloadWithExtra.java b/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/PayloadWithExtra.java new file mode 100644 index 00000000000..6bd9b9e77e5 --- /dev/null +++ b/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/PayloadWithExtra.java @@ -0,0 +1,33 @@ +package org.hyperledger.besu.consensus.merge; + +import org.hyperledger.besu.consensus.merge.blockcreation.PayloadIdentifier; +import org.hyperledger.besu.ethereum.core.BlockWithReceipts; + +import java.util.Optional; + +class PayloadWithExtra { + /** + * The Payload identifier. + */ + final PayloadIdentifier payloadIdentifier; + /** + * The Block with receipts. + */ + final BlockWithReceipts blockWithReceipts; + /** + * TODO tracer info + */ + final Optional tracerOutput = Optional.empty(); + + /** + * Instantiates a new Payload. + * + * @param payloadIdentifier the payload identifier + * @param blockWithReceipts the block with receipts + */ + PayloadWithExtra( + final PayloadIdentifier payloadIdentifier, final BlockWithReceipts blockWithReceipts) { + this.payloadIdentifier = payloadIdentifier; + this.blockWithReceipts = blockWithReceipts; + } +} diff --git a/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/PostMergeContext.java b/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/PostMergeContext.java index e1cbd9e8579..ed8bac276f6 100644 --- a/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/PostMergeContext.java +++ b/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/PostMergeContext.java @@ -61,7 +61,7 @@ public class PostMergeContext implements MergeContext { private final Subscribers newUnverifiedForkchoiceCallbackSubscribers = Subscribers.create(); - private final EvictingQueue blocksInProgress = + private final EvictingQueue blocksInProgress = EvictingQueue.create(MAX_BLOCKS_IN_PROGRESS); // latest finalized block @@ -247,12 +247,12 @@ public void putPayloadById( .addArgument(() -> logBlockProposal(currBestBlock.getBlock())) .log(); blocksInProgress.removeAll( - retrieveTuplesById(payloadId).collect(Collectors.toUnmodifiableList())); - blocksInProgress.add(new PayloadTuple(payloadId, newBlockWithReceipts)); + retrievePayloadsById(payloadId).collect(Collectors.toUnmodifiableList())); + blocksInProgress.add(new PayloadWithExtra(payloadId, newBlockWithReceipts)); logCurrentBestBlock(newBlockWithReceipts); } }, - () -> blocksInProgress.add(new PayloadTuple(payloadId, newBlockWithReceipts))); + () -> blocksInProgress.add(new PayloadWithExtra(payloadId, newBlockWithReceipts))); } } @@ -276,14 +276,14 @@ private void logCurrentBestBlock(final BlockWithReceipts blockWithReceipts) { @Override public Optional retrieveBlockById(final PayloadIdentifier payloadId) { synchronized (blocksInProgress) { - return retrieveTuplesById(payloadId) + return retrievePayloadsById(payloadId) .map(tuple -> tuple.blockWithReceipts) .sorted(compareByGasUsedDesc) .findFirst(); } } - private Stream retrieveTuplesById(final PayloadIdentifier payloadId) { + private Stream retrievePayloadsById(final PayloadIdentifier payloadId) { return blocksInProgress.stream().filter(z -> z.payloadIdentifier.equals(payloadId)); } @@ -296,25 +296,6 @@ private String logBlockProposal(final Block block) { + block.getBody().getTransactions().size(); } - private static class PayloadTuple { - /** The Payload identifier. */ - final PayloadIdentifier payloadIdentifier; - /** The Block with receipts. */ - final BlockWithReceipts blockWithReceipts; - - /** - * Instantiates a new Payload tuple. - * - * @param payloadIdentifier the payload identifier - * @param blockWithReceipts the block with receipts - */ - PayloadTuple( - final PayloadIdentifier payloadIdentifier, final BlockWithReceipts blockWithReceipts) { - this.payloadIdentifier = payloadIdentifier; - this.blockWithReceipts = blockWithReceipts; - } - } - @Override public void setIsChainPruningEnabled(final boolean isChainPruningEnabled) { this.isChainPruningEnabled = isChainPruningEnabled; From 5d928d891bb2d261fdd4158b5ee7b75026e03db9 Mon Sep 17 00:00:00 2001 From: Sally MacFarlane Date: Thu, 21 Sep 2023 13:46:49 +1000 Subject: [PATCH 02/11] tidy up Signed-off-by: Sally MacFarlane --- .../besu/consensus/merge/MergeContext.java | 5 ++- .../consensus/merge/PayloadWithExtra.java | 40 +++++++------------ 2 files changed, 19 insertions(+), 26 deletions(-) diff --git a/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/MergeContext.java b/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/MergeContext.java index c9b58a215bf..d8397e3fd89 100644 --- a/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/MergeContext.java +++ b/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/MergeContext.java @@ -159,7 +159,10 @@ void fireNewUnverifiedForkchoiceEvent( * * @param payload the payload */ - default void putPayload(final PayloadWithExtra payload) {putPayloadById(payload.payloadIdentifier, payload.blockWithReceipts);}; + default void putPayload(final PayloadWithExtra payload) { + putPayloadById(payload.payloadIdentifier, payload.blockWithReceipts); + } + ; /** * Put payload by Identifier. diff --git a/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/PayloadWithExtra.java b/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/PayloadWithExtra.java index 6bd9b9e77e5..84fdf0e84a8 100644 --- a/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/PayloadWithExtra.java +++ b/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/PayloadWithExtra.java @@ -3,31 +3,21 @@ import org.hyperledger.besu.consensus.merge.blockcreation.PayloadIdentifier; import org.hyperledger.besu.ethereum.core.BlockWithReceipts; -import java.util.Optional; - class PayloadWithExtra { - /** - * The Payload identifier. - */ - final PayloadIdentifier payloadIdentifier; - /** - * The Block with receipts. - */ - final BlockWithReceipts blockWithReceipts; - /** - * TODO tracer info - */ - final Optional tracerOutput = Optional.empty(); + /** The Payload identifier. */ + final PayloadIdentifier payloadIdentifier; + /** The Block with receipts. */ + final BlockWithReceipts blockWithReceipts; - /** - * Instantiates a new Payload. - * - * @param payloadIdentifier the payload identifier - * @param blockWithReceipts the block with receipts - */ - PayloadWithExtra( - final PayloadIdentifier payloadIdentifier, final BlockWithReceipts blockWithReceipts) { - this.payloadIdentifier = payloadIdentifier; - this.blockWithReceipts = blockWithReceipts; - } + /** + * Instantiates a new Payload. + * + * @param payloadIdentifier the payload identifier + * @param blockWithReceipts the block with receipts + */ + PayloadWithExtra( + final PayloadIdentifier payloadIdentifier, final BlockWithReceipts blockWithReceipts) { + this.payloadIdentifier = payloadIdentifier; + this.blockWithReceipts = blockWithReceipts; + } } From e27fe82513c84d998c33565d9766fd5ca5872669 Mon Sep 17 00:00:00 2001 From: Sally MacFarlane Date: Thu, 21 Sep 2023 14:03:55 +1000 Subject: [PATCH 03/11] rename Signed-off-by: Sally MacFarlane --- .../hyperledger/besu/consensus/merge/MergeContext.java | 5 ++--- .../merge/{PayloadWithExtra.java => PayloadWrapper.java} | 4 ++-- .../besu/consensus/merge/PostMergeContext.java | 8 ++++---- .../ethereum/blockcreation/BlockTransactionSelector.java | 4 ++-- 4 files changed, 10 insertions(+), 11 deletions(-) rename consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/{PayloadWithExtra.java => PayloadWrapper.java} (93%) diff --git a/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/MergeContext.java b/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/MergeContext.java index d8397e3fd89..cf3e30f4fdd 100644 --- a/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/MergeContext.java +++ b/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/MergeContext.java @@ -155,14 +155,13 @@ void fireNewUnverifiedForkchoiceEvent( boolean validateCandidateHead(final BlockHeader candidateHeader); /** - * Put payload containing Identifier. + * Put payloadWrapper containing Identifier. * * @param payload the payload */ - default void putPayload(final PayloadWithExtra payload) { + default void putPayload(final PayloadWrapper payload) { putPayloadById(payload.payloadIdentifier, payload.blockWithReceipts); } - ; /** * Put payload by Identifier. diff --git a/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/PayloadWithExtra.java b/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/PayloadWrapper.java similarity index 93% rename from consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/PayloadWithExtra.java rename to consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/PayloadWrapper.java index 84fdf0e84a8..b246748ffee 100644 --- a/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/PayloadWithExtra.java +++ b/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/PayloadWrapper.java @@ -3,7 +3,7 @@ import org.hyperledger.besu.consensus.merge.blockcreation.PayloadIdentifier; import org.hyperledger.besu.ethereum.core.BlockWithReceipts; -class PayloadWithExtra { +class PayloadWrapper { /** The Payload identifier. */ final PayloadIdentifier payloadIdentifier; /** The Block with receipts. */ @@ -15,7 +15,7 @@ class PayloadWithExtra { * @param payloadIdentifier the payload identifier * @param blockWithReceipts the block with receipts */ - PayloadWithExtra( + PayloadWrapper( final PayloadIdentifier payloadIdentifier, final BlockWithReceipts blockWithReceipts) { this.payloadIdentifier = payloadIdentifier; this.blockWithReceipts = blockWithReceipts; diff --git a/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/PostMergeContext.java b/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/PostMergeContext.java index ed8bac276f6..25dcb683581 100644 --- a/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/PostMergeContext.java +++ b/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/PostMergeContext.java @@ -61,7 +61,7 @@ public class PostMergeContext implements MergeContext { private final Subscribers newUnverifiedForkchoiceCallbackSubscribers = Subscribers.create(); - private final EvictingQueue blocksInProgress = + private final EvictingQueue blocksInProgress = EvictingQueue.create(MAX_BLOCKS_IN_PROGRESS); // latest finalized block @@ -248,11 +248,11 @@ public void putPayloadById( .log(); blocksInProgress.removeAll( retrievePayloadsById(payloadId).collect(Collectors.toUnmodifiableList())); - blocksInProgress.add(new PayloadWithExtra(payloadId, newBlockWithReceipts)); + blocksInProgress.add(new PayloadWrapper(payloadId, newBlockWithReceipts)); logCurrentBestBlock(newBlockWithReceipts); } }, - () -> blocksInProgress.add(new PayloadWithExtra(payloadId, newBlockWithReceipts))); + () -> blocksInProgress.add(new PayloadWrapper(payloadId, newBlockWithReceipts))); } } @@ -283,7 +283,7 @@ public Optional retrieveBlockById(final PayloadIdentifier pay } } - private Stream retrievePayloadsById(final PayloadIdentifier payloadId) { + private Stream retrievePayloadsById(final PayloadIdentifier payloadId) { return blocksInProgress.stream().filter(z -> z.payloadIdentifier.equals(payloadId)); } diff --git a/ethereum/blockcreation/src/main/java/org/hyperledger/besu/ethereum/blockcreation/BlockTransactionSelector.java b/ethereum/blockcreation/src/main/java/org/hyperledger/besu/ethereum/blockcreation/BlockTransactionSelector.java index 7420e54e70c..1130d5021a4 100644 --- a/ethereum/blockcreation/src/main/java/org/hyperledger/besu/ethereum/blockcreation/BlockTransactionSelector.java +++ b/ethereum/blockcreation/src/main/java/org/hyperledger/besu/ethereum/blockcreation/BlockTransactionSelector.java @@ -264,7 +264,7 @@ public BlockTransactionSelector( /* This function iterates over (potentially) all transactions in the PendingTransactions, this is a long-running process. If running in a thread, it can be cancelled via the isCancelled supplier (which will result - in this throwing an CancellationException). + in this throwing a CancellationException). */ public TransactionSelectionResults buildTransactionListForBlock() { LOG.atDebug() @@ -280,7 +280,7 @@ public TransactionSelectionResults buildTransactionListForBlock() { return res; }); LOG.atTrace() - .setMessage("Transaction selection result result {}") + .setMessage("Transaction selection result {}") .addArgument(transactionSelectionResults::toTraceLog) .log(); return transactionSelectionResults; From 52bb8f72b0c504fa346872763a68b4af8b537733 Mon Sep 17 00:00:00 2001 From: Sally MacFarlane Date: Thu, 21 Sep 2023 15:21:20 +1000 Subject: [PATCH 04/11] spdx header Signed-off-by: Sally MacFarlane --- .../besu/consensus/merge/PayloadWrapper.java | 14 ++++++++++++++ .../src/reference-test/external-resources | 2 +- 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/PayloadWrapper.java b/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/PayloadWrapper.java index b246748ffee..feaa6d89149 100644 --- a/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/PayloadWrapper.java +++ b/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/PayloadWrapper.java @@ -1,3 +1,17 @@ +/* + * Copyright Hyperledger Besu Contributors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on + * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the + * specific language governing permissions and limitations under the License. + * + * SPDX-License-Identifier: Apache-2.0 + */ package org.hyperledger.besu.consensus.merge; import org.hyperledger.besu.consensus.merge.blockcreation.PayloadIdentifier; diff --git a/ethereum/referencetests/src/reference-test/external-resources b/ethereum/referencetests/src/reference-test/external-resources index 661356317ac..06e276776bc 160000 --- a/ethereum/referencetests/src/reference-test/external-resources +++ b/ethereum/referencetests/src/reference-test/external-resources @@ -1 +1 @@ -Subproject commit 661356317ac6df52208d54187e692472a25a01f8 +Subproject commit 06e276776bc87817c38f6efb492bf6f4527fa904 From 974b569a15e5771cffcb23a1ad77bf3e094e682d Mon Sep 17 00:00:00 2001 From: Sally MacFarlane Date: Thu, 21 Sep 2023 16:28:51 +1000 Subject: [PATCH 05/11] updated submodule Signed-off-by: Sally MacFarlane --- .../org/hyperledger/besu/consensus/merge/PayloadWrapper.java | 2 +- ethereum/referencetests/src/reference-test/external-resources | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/PayloadWrapper.java b/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/PayloadWrapper.java index feaa6d89149..bf102b51a47 100644 --- a/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/PayloadWrapper.java +++ b/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/PayloadWrapper.java @@ -24,7 +24,7 @@ class PayloadWrapper { final BlockWithReceipts blockWithReceipts; /** - * Instantiates a new Payload. + * Instantiates a new PayloadWrapper. * * @param payloadIdentifier the payload identifier * @param blockWithReceipts the block with receipts diff --git a/ethereum/referencetests/src/reference-test/external-resources b/ethereum/referencetests/src/reference-test/external-resources index 06e276776bc..661356317ac 160000 --- a/ethereum/referencetests/src/reference-test/external-resources +++ b/ethereum/referencetests/src/reference-test/external-resources @@ -1 +1 @@ -Subproject commit 06e276776bc87817c38f6efb492bf6f4527fa904 +Subproject commit 661356317ac6df52208d54187e692472a25a01f8 From 5b0da51b5ea70a16bf82e89738c224a07f56a56a Mon Sep 17 00:00:00 2001 From: Sally MacFarlane Date: Fri, 22 Sep 2023 11:26:16 +1000 Subject: [PATCH 06/11] public constructors Signed-off-by: Sally MacFarlane --- .../besu/consensus/merge/MergeContext.java | 2 +- .../besu/consensus/merge/PayloadWrapper.java | 19 +++++++++++++++++-- .../merge/blockcreation/MergeCoordinator.java | 9 +++++---- 3 files changed, 23 insertions(+), 7 deletions(-) diff --git a/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/MergeContext.java b/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/MergeContext.java index cf3e30f4fdd..34ce297a002 100644 --- a/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/MergeContext.java +++ b/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/MergeContext.java @@ -182,7 +182,7 @@ default void putPayload(final PayloadWrapper payload) { /** * Sets is chain pruning enabled. * - * @param isChainPruningEnabled the is chain pruning enabled + * @param isChainPruningEnabled whether chain pruning is enabled */ default void setIsChainPruningEnabled(final boolean isChainPruningEnabled) {} diff --git a/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/PayloadWrapper.java b/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/PayloadWrapper.java index bf102b51a47..fecf5b8f5b9 100644 --- a/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/PayloadWrapper.java +++ b/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/PayloadWrapper.java @@ -16,12 +16,15 @@ import org.hyperledger.besu.consensus.merge.blockcreation.PayloadIdentifier; import org.hyperledger.besu.ethereum.core.BlockWithReceipts; +import org.hyperledger.besu.evm.tracing.OperationTracer; -class PayloadWrapper { +public class PayloadWrapper { /** The Payload identifier. */ final PayloadIdentifier payloadIdentifier; /** The Block with receipts. */ final BlockWithReceipts blockWithReceipts; + /** Tracer */ + OperationTracer operationTracer = OperationTracer.NO_TRACING; /** * Instantiates a new PayloadWrapper. @@ -29,9 +32,21 @@ class PayloadWrapper { * @param payloadIdentifier the payload identifier * @param blockWithReceipts the block with receipts */ - PayloadWrapper( + public PayloadWrapper( final PayloadIdentifier payloadIdentifier, final BlockWithReceipts blockWithReceipts) { this.payloadIdentifier = payloadIdentifier; this.blockWithReceipts = blockWithReceipts; } + /** + * Instantiates a new PayloadWrapper. + * + * @param payloadIdentifier the payload identifier + * @param blockWithReceipts the block with receipts + */ + public PayloadWrapper( + final PayloadIdentifier payloadIdentifier, final BlockWithReceipts blockWithReceipts, final OperationTracer operationTracer) { + this.payloadIdentifier = payloadIdentifier; + this.blockWithReceipts = blockWithReceipts; + this.operationTracer = operationTracer; + } } diff --git a/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/blockcreation/MergeCoordinator.java b/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/blockcreation/MergeCoordinator.java index cac3edf952c..5c75e50678e 100644 --- a/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/blockcreation/MergeCoordinator.java +++ b/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/blockcreation/MergeCoordinator.java @@ -18,6 +18,7 @@ import static org.hyperledger.besu.consensus.merge.blockcreation.MergeMiningCoordinator.ForkchoiceResult.Status.INVALID; import org.hyperledger.besu.consensus.merge.MergeContext; +import org.hyperledger.besu.consensus.merge.PayloadWrapper; import org.hyperledger.besu.datatypes.Address; import org.hyperledger.besu.datatypes.Hash; import org.hyperledger.besu.datatypes.Wei; @@ -288,8 +289,8 @@ public PayloadIdentifier preparePayload( BlockProcessingResult result = validateProposedBlock(emptyBlock); if (result.isSuccessful()) { - mergeContext.putPayloadById( - payloadIdentifier, new BlockWithReceipts(emptyBlock, result.getReceipts())); + mergeContext.putPayload(new PayloadWrapper( + payloadIdentifier, new BlockWithReceipts(emptyBlock, result.getReceipts()))); LOG.info( "Start building proposals for block {} identified by {}", emptyBlock.getHeader().getNumber(), @@ -443,8 +444,8 @@ private void evaluateNewBlock( if (isBlockCreationCancelled(payloadIdentifier)) return; - mergeContext.putPayloadById( - payloadIdentifier, new BlockWithReceipts(bestBlock, resultBest.getReceipts())); + mergeContext.putPayload(new PayloadWrapper( + payloadIdentifier, new BlockWithReceipts(bestBlock, resultBest.getReceipts()))); LOG.atDebug() .setMessage( "Successfully built block {} for proposal identified by {}, with {} transactions, in {}ms") From 5990eeec10806d764a08aea71fc64485b20a6dd1 Mon Sep 17 00:00:00 2001 From: Sally MacFarlane Date: Fri, 22 Sep 2023 12:01:52 +1000 Subject: [PATCH 07/11] formatting Signed-off-by: Sally MacFarlane --- .../besu/consensus/merge/PayloadWrapper.java | 4 +++- .../merge/blockcreation/MergeCoordinator.java | 10 ++++++---- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/PayloadWrapper.java b/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/PayloadWrapper.java index fecf5b8f5b9..cf37948fe51 100644 --- a/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/PayloadWrapper.java +++ b/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/PayloadWrapper.java @@ -44,7 +44,9 @@ public PayloadWrapper( * @param blockWithReceipts the block with receipts */ public PayloadWrapper( - final PayloadIdentifier payloadIdentifier, final BlockWithReceipts blockWithReceipts, final OperationTracer operationTracer) { + final PayloadIdentifier payloadIdentifier, + final BlockWithReceipts blockWithReceipts, + final OperationTracer operationTracer) { this.payloadIdentifier = payloadIdentifier; this.blockWithReceipts = blockWithReceipts; this.operationTracer = operationTracer; diff --git a/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/blockcreation/MergeCoordinator.java b/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/blockcreation/MergeCoordinator.java index 5c75e50678e..27375d5a3de 100644 --- a/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/blockcreation/MergeCoordinator.java +++ b/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/blockcreation/MergeCoordinator.java @@ -289,8 +289,9 @@ public PayloadIdentifier preparePayload( BlockProcessingResult result = validateProposedBlock(emptyBlock); if (result.isSuccessful()) { - mergeContext.putPayload(new PayloadWrapper( - payloadIdentifier, new BlockWithReceipts(emptyBlock, result.getReceipts()))); + mergeContext.putPayload( + new PayloadWrapper( + payloadIdentifier, new BlockWithReceipts(emptyBlock, result.getReceipts()))); LOG.info( "Start building proposals for block {} identified by {}", emptyBlock.getHeader().getNumber(), @@ -444,8 +445,9 @@ private void evaluateNewBlock( if (isBlockCreationCancelled(payloadIdentifier)) return; - mergeContext.putPayload(new PayloadWrapper( - payloadIdentifier, new BlockWithReceipts(bestBlock, resultBest.getReceipts()))); + mergeContext.putPayload( + new PayloadWrapper( + payloadIdentifier, new BlockWithReceipts(bestBlock, resultBest.getReceipts()))); LOG.atDebug() .setMessage( "Successfully built block {} for proposal identified by {}, with {} transactions, in {}ms") From f0c476e22d0bb94fb0f29cf4bef29d6c52bc0d77 Mon Sep 17 00:00:00 2001 From: Sally MacFarlane Date: Fri, 22 Sep 2023 15:18:55 +1000 Subject: [PATCH 08/11] javadoc Signed-off-by: Sally MacFarlane --- .../besu/consensus/merge/PayloadWrapper.java | 18 +----------------- 1 file changed, 1 insertion(+), 17 deletions(-) diff --git a/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/PayloadWrapper.java b/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/PayloadWrapper.java index cf37948fe51..7257fa4eb02 100644 --- a/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/PayloadWrapper.java +++ b/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/PayloadWrapper.java @@ -16,15 +16,13 @@ import org.hyperledger.besu.consensus.merge.blockcreation.PayloadIdentifier; import org.hyperledger.besu.ethereum.core.BlockWithReceipts; -import org.hyperledger.besu.evm.tracing.OperationTracer; +/** Wrapper for payload plus extra info. */ public class PayloadWrapper { /** The Payload identifier. */ final PayloadIdentifier payloadIdentifier; /** The Block with receipts. */ final BlockWithReceipts blockWithReceipts; - /** Tracer */ - OperationTracer operationTracer = OperationTracer.NO_TRACING; /** * Instantiates a new PayloadWrapper. @@ -37,18 +35,4 @@ public PayloadWrapper( this.payloadIdentifier = payloadIdentifier; this.blockWithReceipts = blockWithReceipts; } - /** - * Instantiates a new PayloadWrapper. - * - * @param payloadIdentifier the payload identifier - * @param blockWithReceipts the block with receipts - */ - public PayloadWrapper( - final PayloadIdentifier payloadIdentifier, - final BlockWithReceipts blockWithReceipts, - final OperationTracer operationTracer) { - this.payloadIdentifier = payloadIdentifier; - this.blockWithReceipts = blockWithReceipts; - this.operationTracer = operationTracer; - } } From 2dc88f09a37fb1116ad627367303cf44931c8f13 Mon Sep 17 00:00:00 2001 From: Sally MacFarlane Date: Mon, 25 Sep 2023 09:42:38 +1000 Subject: [PATCH 09/11] made a record Signed-off-by: Sally MacFarlane --- .../besu/consensus/merge/MergeContext.java | 2 +- .../besu/consensus/merge/PayloadWrapper.java | 19 ++----------------- .../consensus/merge/PostMergeContext.java | 4 ++-- 3 files changed, 5 insertions(+), 20 deletions(-) diff --git a/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/MergeContext.java b/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/MergeContext.java index 34ce297a002..985b4b31c77 100644 --- a/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/MergeContext.java +++ b/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/MergeContext.java @@ -160,7 +160,7 @@ void fireNewUnverifiedForkchoiceEvent( * @param payload the payload */ default void putPayload(final PayloadWrapper payload) { - putPayloadById(payload.payloadIdentifier, payload.blockWithReceipts); + putPayloadById(payload.payloadIdentifier(), payload.blockWithReceipts()); } /** diff --git a/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/PayloadWrapper.java b/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/PayloadWrapper.java index 7257fa4eb02..3a6ebcb41c8 100644 --- a/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/PayloadWrapper.java +++ b/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/PayloadWrapper.java @@ -18,21 +18,6 @@ import org.hyperledger.besu.ethereum.core.BlockWithReceipts; /** Wrapper for payload plus extra info. */ -public class PayloadWrapper { - /** The Payload identifier. */ - final PayloadIdentifier payloadIdentifier; - /** The Block with receipts. */ - final BlockWithReceipts blockWithReceipts; - - /** - * Instantiates a new PayloadWrapper. - * - * @param payloadIdentifier the payload identifier - * @param blockWithReceipts the block with receipts - */ - public PayloadWrapper( - final PayloadIdentifier payloadIdentifier, final BlockWithReceipts blockWithReceipts) { - this.payloadIdentifier = payloadIdentifier; - this.blockWithReceipts = blockWithReceipts; - } +public record PayloadWrapper( + PayloadIdentifier payloadIdentifier, BlockWithReceipts blockWithReceipts) { } diff --git a/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/PostMergeContext.java b/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/PostMergeContext.java index 25dcb683581..1954e4de8bb 100644 --- a/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/PostMergeContext.java +++ b/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/PostMergeContext.java @@ -277,14 +277,14 @@ private void logCurrentBestBlock(final BlockWithReceipts blockWithReceipts) { public Optional retrieveBlockById(final PayloadIdentifier payloadId) { synchronized (blocksInProgress) { return retrievePayloadsById(payloadId) - .map(tuple -> tuple.blockWithReceipts) + .map(payloadWrapper -> payloadWrapper.blockWithReceipts()) .sorted(compareByGasUsedDesc) .findFirst(); } } private Stream retrievePayloadsById(final PayloadIdentifier payloadId) { - return blocksInProgress.stream().filter(z -> z.payloadIdentifier.equals(payloadId)); + return blocksInProgress.stream().filter(z -> z.payloadIdentifier().equals(payloadId)); } private String logBlockProposal(final Block block) { From b9fa065f34c5e335daf3a1550ab90390657354af Mon Sep 17 00:00:00 2001 From: Sally MacFarlane Date: Mon, 25 Sep 2023 10:57:20 +1000 Subject: [PATCH 10/11] formatting Signed-off-by: Sally MacFarlane --- .../org/hyperledger/besu/consensus/merge/PayloadWrapper.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/PayloadWrapper.java b/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/PayloadWrapper.java index 3a6ebcb41c8..4697f8d9b8c 100644 --- a/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/PayloadWrapper.java +++ b/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/PayloadWrapper.java @@ -19,5 +19,4 @@ /** Wrapper for payload plus extra info. */ public record PayloadWrapper( - PayloadIdentifier payloadIdentifier, BlockWithReceipts blockWithReceipts) { -} + PayloadIdentifier payloadIdentifier, BlockWithReceipts blockWithReceipts) {} From a28f3af6670a126bb89938da83745b350a10ce80 Mon Sep 17 00:00:00 2001 From: Sally MacFarlane Date: Mon, 25 Sep 2023 15:57:42 +1000 Subject: [PATCH 11/11] refactor tests to use PayloadWrapper Signed-off-by: Sally MacFarlane --- .../besu/consensus/merge/MergeContext.java | 14 +- .../consensus/merge/PostMergeContext.java | 28 ++-- .../consensus/merge/TransitionContext.java | 5 +- .../merge/blockcreation/MergeCoordinator.java | 4 +- .../consensus/merge/PostMergeContextTest.java | 12 +- .../blockcreation/MergeCoordinatorTest.java | 157 ++++++++++++------ 6 files changed, 133 insertions(+), 87 deletions(-) diff --git a/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/MergeContext.java b/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/MergeContext.java index 985b4b31c77..9db6a97a0ca 100644 --- a/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/MergeContext.java +++ b/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/MergeContext.java @@ -154,22 +154,12 @@ void fireNewUnverifiedForkchoiceEvent( */ boolean validateCandidateHead(final BlockHeader candidateHeader); - /** - * Put payloadWrapper containing Identifier. - * - * @param payload the payload - */ - default void putPayload(final PayloadWrapper payload) { - putPayloadById(payload.payloadIdentifier(), payload.blockWithReceipts()); - } - /** * Put payload by Identifier. * - * @param payloadId the payload identifier - * @param blockWithReceipts the block with receipts + * @param payloadWrapper payload wrapper */ - void putPayloadById(final PayloadIdentifier payloadId, final BlockWithReceipts blockWithReceipts); + void putPayloadById(final PayloadWrapper payloadWrapper); /** * Retrieve block by id. diff --git a/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/PostMergeContext.java b/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/PostMergeContext.java index 1954e4de8bb..08985ed326b 100644 --- a/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/PostMergeContext.java +++ b/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/PostMergeContext.java @@ -232,27 +232,35 @@ public boolean validateCandidateHead(final BlockHeader candidateHeader) { } @Override - public void putPayloadById( - final PayloadIdentifier payloadId, final BlockWithReceipts newBlockWithReceipts) { + public void putPayloadById(final PayloadWrapper payloadWrapper) { synchronized (blocksInProgress) { - final Optional maybeCurrBestBlock = retrieveBlockById(payloadId); + final Optional maybeCurrBestBlock = + retrieveBlockById(payloadWrapper.payloadIdentifier()); maybeCurrBestBlock.ifPresentOrElse( currBestBlock -> { - if (compareByGasUsedDesc.compare(newBlockWithReceipts, currBestBlock) < 0) { + if (compareByGasUsedDesc.compare(payloadWrapper.blockWithReceipts(), currBestBlock) + < 0) { LOG.atDebug() .setMessage("New proposal for payloadId {} {} is better than the previous one {}") - .addArgument(payloadId) - .addArgument(() -> logBlockProposal(newBlockWithReceipts.getBlock())) + .addArgument(payloadWrapper.payloadIdentifier()) + .addArgument( + () -> logBlockProposal(payloadWrapper.blockWithReceipts().getBlock())) .addArgument(() -> logBlockProposal(currBestBlock.getBlock())) .log(); blocksInProgress.removeAll( - retrievePayloadsById(payloadId).collect(Collectors.toUnmodifiableList())); - blocksInProgress.add(new PayloadWrapper(payloadId, newBlockWithReceipts)); - logCurrentBestBlock(newBlockWithReceipts); + retrievePayloadsById(payloadWrapper.payloadIdentifier()) + .collect(Collectors.toUnmodifiableList())); + blocksInProgress.add( + new PayloadWrapper( + payloadWrapper.payloadIdentifier(), payloadWrapper.blockWithReceipts())); + logCurrentBestBlock(payloadWrapper.blockWithReceipts()); } }, - () -> blocksInProgress.add(new PayloadWrapper(payloadId, newBlockWithReceipts))); + () -> + blocksInProgress.add( + new PayloadWrapper( + payloadWrapper.payloadIdentifier(), payloadWrapper.blockWithReceipts()))); } } diff --git a/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/TransitionContext.java b/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/TransitionContext.java index 69183cb0c53..35e51db563e 100644 --- a/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/TransitionContext.java +++ b/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/TransitionContext.java @@ -140,9 +140,8 @@ public boolean validateCandidateHead(final BlockHeader candidateHeader) { } @Override - public void putPayloadById( - final PayloadIdentifier payloadId, final BlockWithReceipts blockWithReceipts) { - postMergeContext.putPayloadById(payloadId, blockWithReceipts); + public void putPayloadById(final PayloadWrapper payloadWrapper) { + postMergeContext.putPayloadById(payloadWrapper); } @Override diff --git a/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/blockcreation/MergeCoordinator.java b/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/blockcreation/MergeCoordinator.java index 27375d5a3de..d7e2856941f 100644 --- a/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/blockcreation/MergeCoordinator.java +++ b/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/blockcreation/MergeCoordinator.java @@ -289,7 +289,7 @@ public PayloadIdentifier preparePayload( BlockProcessingResult result = validateProposedBlock(emptyBlock); if (result.isSuccessful()) { - mergeContext.putPayload( + mergeContext.putPayloadById( new PayloadWrapper( payloadIdentifier, new BlockWithReceipts(emptyBlock, result.getReceipts()))); LOG.info( @@ -445,7 +445,7 @@ private void evaluateNewBlock( if (isBlockCreationCancelled(payloadIdentifier)) return; - mergeContext.putPayload( + mergeContext.putPayloadById( new PayloadWrapper( payloadIdentifier, new BlockWithReceipts(bestBlock, resultBest.getReceipts()))); LOG.atDebug() diff --git a/consensus/merge/src/test/java/org/hyperledger/besu/consensus/merge/PostMergeContextTest.java b/consensus/merge/src/test/java/org/hyperledger/besu/consensus/merge/PostMergeContextTest.java index 0e5f246a273..d28d6677c81 100644 --- a/consensus/merge/src/test/java/org/hyperledger/besu/consensus/merge/PostMergeContextTest.java +++ b/consensus/merge/src/test/java/org/hyperledger/besu/consensus/merge/PostMergeContextTest.java @@ -138,7 +138,7 @@ public void putAndRetrieveFirstPayload() { BlockWithReceipts mockBlockWithReceipts = createBlockWithReceipts(1, 21000, 1); PayloadIdentifier firstPayloadId = new PayloadIdentifier(1L); - postMergeContext.putPayloadById(firstPayloadId, mockBlockWithReceipts); + postMergeContext.putPayloadById(new PayloadWrapper(firstPayloadId, mockBlockWithReceipts)); assertThat(postMergeContext.retrieveBlockById(firstPayloadId)).contains(mockBlockWithReceipts); } @@ -149,8 +149,8 @@ public void puttingTwoBlocksWithTheSamePayloadIdWeRetrieveTheBest() { BlockWithReceipts betterBlockWithReceipts = createBlockWithReceipts(2, 11, 1); PayloadIdentifier payloadId = new PayloadIdentifier(1L); - postMergeContext.putPayloadById(payloadId, zeroTxBlockWithReceipts); - postMergeContext.putPayloadById(payloadId, betterBlockWithReceipts); + postMergeContext.putPayloadById(new PayloadWrapper(payloadId, zeroTxBlockWithReceipts)); + postMergeContext.putPayloadById(new PayloadWrapper(payloadId, betterBlockWithReceipts)); assertThat(postMergeContext.retrieveBlockById(payloadId)).contains(betterBlockWithReceipts); } @@ -162,9 +162,9 @@ public void puttingABlockWithTheSamePayloadIdSmallerThanAnExistingOneWeRetrieveT BlockWithReceipts smallBlockWithReceipts = createBlockWithReceipts(3, 5, 1); PayloadIdentifier payloadId = new PayloadIdentifier(1L); - postMergeContext.putPayloadById(payloadId, zeroTxBlockWithReceipts); - postMergeContext.putPayloadById(payloadId, betterBlockWithReceipts); - postMergeContext.putPayloadById(payloadId, smallBlockWithReceipts); + postMergeContext.putPayloadById(new PayloadWrapper(payloadId, zeroTxBlockWithReceipts)); + postMergeContext.putPayloadById(new PayloadWrapper(payloadId, betterBlockWithReceipts)); + postMergeContext.putPayloadById(new PayloadWrapper(payloadId, smallBlockWithReceipts)); assertThat(postMergeContext.retrieveBlockById(payloadId)).contains(betterBlockWithReceipts); } diff --git a/consensus/merge/src/test/java/org/hyperledger/besu/consensus/merge/blockcreation/MergeCoordinatorTest.java b/consensus/merge/src/test/java/org/hyperledger/besu/consensus/merge/blockcreation/MergeCoordinatorTest.java index 1948bced86b..232140fac57 100644 --- a/consensus/merge/src/test/java/org/hyperledger/besu/consensus/merge/blockcreation/MergeCoordinatorTest.java +++ b/consensus/merge/src/test/java/org/hyperledger/besu/consensus/merge/blockcreation/MergeCoordinatorTest.java @@ -35,6 +35,7 @@ import org.hyperledger.besu.config.MergeConfigOptions; import org.hyperledger.besu.consensus.merge.MergeContext; +import org.hyperledger.besu.consensus.merge.PayloadWrapper; import org.hyperledger.besu.consensus.merge.blockcreation.MergeCoordinator.ProposalBuilderExecutor; import org.hyperledger.besu.consensus.merge.blockcreation.MergeMiningCoordinator.ForkchoiceResult; import org.hyperledger.besu.crypto.KeyPair; @@ -55,7 +56,6 @@ import org.hyperledger.besu.ethereum.core.BlockBody; import org.hyperledger.besu.ethereum.core.BlockHeader; import org.hyperledger.besu.ethereum.core.BlockHeaderTestFixture; -import org.hyperledger.besu.ethereum.core.BlockWithReceipts; import org.hyperledger.besu.ethereum.core.Difficulty; import org.hyperledger.besu.ethereum.core.MiningParameters; import org.hyperledger.besu.ethereum.core.Transaction; @@ -242,11 +242,12 @@ public void setUp() { public void coinbaseShouldMatchSuggestedFeeRecipient() { doAnswer( invocation -> { - coordinator.finalizeProposalById(invocation.getArgument(0, PayloadIdentifier.class)); + coordinator.finalizeProposalById( + invocation.getArgument(0, PayloadWrapper.class).payloadIdentifier()); return null; }) .when(mergeContext) - .putPayloadById(any(), any()); + .putPayloadById(any()); var payloadId = coordinator.preparePayload( @@ -257,12 +258,12 @@ public void coinbaseShouldMatchSuggestedFeeRecipient() { Optional.empty(), Optional.empty()); - ArgumentCaptor blockWithReceipts = - ArgumentCaptor.forClass(BlockWithReceipts.class); + ArgumentCaptor payloadWrapper = ArgumentCaptor.forClass(PayloadWrapper.class); - verify(mergeContext, atLeastOnce()).putPayloadById(eq(payloadId), blockWithReceipts.capture()); + verify(mergeContext, atLeastOnce()).putPayloadById(payloadWrapper.capture()); - assertThat(blockWithReceipts.getValue().getHeader().getCoinbase()) + assertThat(payloadWrapper.getValue().payloadIdentifier()).isEqualTo(payloadId); + assertThat(payloadWrapper.getValue().blockWithReceipts().getHeader().getCoinbase()) .isEqualTo(suggestedFeeRecipient); } @@ -317,12 +318,13 @@ public void exceptionDuringBuildingBlockShouldNotBeInvalid() createTransaction(retries.get() - 1), Optional.empty()); } else { // when we have 5 transactions finalize block creation - willThrow.finalizeProposalById(invocation.getArgument(0, PayloadIdentifier.class)); + willThrow.finalizeProposalById( + invocation.getArgument(0, PayloadWrapper.class).payloadIdentifier()); } return null; }) .when(mergeContext) - .putPayloadById(any(), any()); + .putPayloadById(any()); var payloadId = willThrow.preparePayload( @@ -336,12 +338,19 @@ public void exceptionDuringBuildingBlockShouldNotBeInvalid() verify(willThrow, never()).addBadBlock(any(), any()); blockCreationTask.get(); - ArgumentCaptor blockWithReceipts = - ArgumentCaptor.forClass(BlockWithReceipts.class); + ArgumentCaptor payloadWrapper = ArgumentCaptor.forClass(PayloadWrapper.class); verify(mergeContext, times(txPerBlock + 1)) - .putPayloadById(eq(payloadId), blockWithReceipts.capture()); // +1 for the empty - assertThat(blockWithReceipts.getValue().getBlock().getBody().getTransactions().size()) + .putPayloadById(payloadWrapper.capture()); // +1 for the empty + assertThat(payloadWrapper.getValue().payloadIdentifier()).isEqualTo(payloadId); + assertThat( + payloadWrapper + .getValue() + .blockWithReceipts() + .getBlock() + .getBody() + .getTransactions() + .size()) .isEqualTo(txPerBlock); // this only verifies that adding the bad block didn't happen through the mergeCoordinator, it // still may be called directly. @@ -383,12 +392,12 @@ public void shouldContinueBuildingBlocksUntilFinalizeIsCalled() } else { // when we have 5 transactions finalize block creation coordinator.finalizeProposalById( - invocation.getArgument(0, PayloadIdentifier.class)); + invocation.getArgument(0, PayloadWrapper.class).payloadIdentifier()); } return null; }) .when(mergeContext) - .putPayloadById(any(), any()); + .putPayloadById(any()); var payloadId = coordinator.preparePayload( @@ -401,15 +410,21 @@ public void shouldContinueBuildingBlocksUntilFinalizeIsCalled() blockCreationTask.get(); - ArgumentCaptor blockWithReceipts = - ArgumentCaptor.forClass(BlockWithReceipts.class); + ArgumentCaptor payloadWrapper = ArgumentCaptor.forClass(PayloadWrapper.class); - verify(mergeContext, times(retries.intValue())) - .putPayloadById(eq(payloadId), blockWithReceipts.capture()); + verify(mergeContext, times(retries.intValue())).putPayloadById(payloadWrapper.capture()); - assertThat(blockWithReceipts.getAllValues().size()).isEqualTo(retries.intValue()); + assertThat(payloadWrapper.getValue().payloadIdentifier()).isEqualTo(payloadId); + assertThat(payloadWrapper.getAllValues().size()).isEqualTo(retries.intValue()); for (int i = 0; i < retries.intValue(); i++) { - assertThat(blockWithReceipts.getAllValues().get(i).getBlock().getBody().getTransactions()) + assertThat( + payloadWrapper + .getAllValues() + .get(i) + .blockWithReceipts() + .getBlock() + .getBody() + .getTransactions()) .hasSize(i); } } @@ -435,12 +450,12 @@ public void blockCreationRepetitionShouldTakeNotLessThanRepetitionMinDuration() } else { // finalize after 5 repetitions coordinator.finalizeProposalById( - invocation.getArgument(0, PayloadIdentifier.class)); + invocation.getArgument(0, PayloadWrapper.class).payloadIdentifier()); } return null; }) .when(mergeContext) - .putPayloadById(any(), any()); + .putPayloadById(any()); var payloadId = coordinator.preparePayload( @@ -453,7 +468,10 @@ public void blockCreationRepetitionShouldTakeNotLessThanRepetitionMinDuration() blockCreationTask.get(); - verify(mergeContext, times(retries.intValue())).putPayloadById(eq(payloadId), any()); + ArgumentCaptor payloadWrapper = ArgumentCaptor.forClass(PayloadWrapper.class); + + verify(mergeContext, times(retries.intValue())).putPayloadById(payloadWrapper.capture()); + assertThat(payloadWrapper.getValue().payloadIdentifier()).isEqualTo(payloadId); // check with a tolerance assertThat(repetitionDurations) @@ -466,7 +484,8 @@ public void shouldRetryBlockCreationOnRecoverableError() doAnswer( invocation -> { if (invocation - .getArgument(1, BlockWithReceipts.class) + .getArgument(0, PayloadWrapper.class) + .blockWithReceipts() .getBlock() .getBody() .getTransactions() @@ -480,12 +499,12 @@ public void shouldRetryBlockCreationOnRecoverableError() } else { // stop block creation loop when we see a not empty block coordinator.finalizeProposalById( - invocation.getArgument(0, PayloadIdentifier.class)); + invocation.getArgument(0, PayloadWrapper.class).payloadIdentifier()); } return null; }) .when(mergeContext) - .putPayloadById(any(), any()); + .putPayloadById(any()); transactions.addLocalTransaction(createTransaction(0), Optional.empty()); @@ -500,15 +519,29 @@ public void shouldRetryBlockCreationOnRecoverableError() blockCreationTask.get(); - ArgumentCaptor blockWithReceipts = - ArgumentCaptor.forClass(BlockWithReceipts.class); + ArgumentCaptor payloadWrapper = ArgumentCaptor.forClass(PayloadWrapper.class); - verify(mergeContext, times(2)).putPayloadById(eq(payloadId), blockWithReceipts.capture()); + verify(mergeContext, times(2)).putPayloadById(payloadWrapper.capture()); + assertThat(payloadWrapper.getValue().payloadIdentifier()).isEqualTo(payloadId); - assertThat(blockWithReceipts.getAllValues().size()).isEqualTo(2); - assertThat(blockWithReceipts.getAllValues().get(0).getBlock().getBody().getTransactions()) + assertThat(payloadWrapper.getAllValues().size()).isEqualTo(2); + assertThat( + payloadWrapper + .getAllValues() + .get(0) + .blockWithReceipts() + .getBlock() + .getBody() + .getTransactions()) .hasSize(0); - assertThat(blockWithReceipts.getAllValues().get(1).getBlock().getBody().getTransactions()) + assertThat( + payloadWrapper + .getAllValues() + .get(1) + .blockWithReceipts() + .getBlock() + .getBody() + .getTransactions()) .hasSize(1); } @@ -522,7 +555,7 @@ public void shouldStopRetryBlockCreationIfTimeExpired() throws InterruptedExcept return null; }) .when(mergeContext) - .putPayloadById(any(), any()); + .putPayloadById(any()); var payloadId = coordinator.preparePayload( @@ -540,7 +573,10 @@ public void shouldStopRetryBlockCreationIfTimeExpired() throws InterruptedExcept assertThat(e).hasCauseInstanceOf(TimeoutException.class); } - verify(mergeContext, atLeast(retries.intValue())).putPayloadById(eq(payloadId), any()); + ArgumentCaptor payloadWrapper = ArgumentCaptor.forClass(PayloadWrapper.class); + + verify(mergeContext, atLeast(retries.intValue())).putPayloadById(payloadWrapper.capture()); + assertThat(payloadWrapper.getValue().payloadIdentifier()).isEqualTo(payloadId); } @Test @@ -565,7 +601,7 @@ public void shouldStopInProgressBlockCreationIfFinalizedIsCalled() .when(blockchain) .getBlockHeader(any())) .when(mergeContext) - .putPayloadById(any(), any()); + .putPayloadById(any()); var payloadId = coordinator.preparePayload( @@ -582,13 +618,20 @@ public void shouldStopInProgressBlockCreationIfFinalizedIsCalled() blockCreationTask.get(); // check that we only the empty block has been built - ArgumentCaptor blockWithReceipts = - ArgumentCaptor.forClass(BlockWithReceipts.class); + ArgumentCaptor payloadWrapper = ArgumentCaptor.forClass(PayloadWrapper.class); - verify(mergeContext, times(1)).putPayloadById(eq(payloadId), blockWithReceipts.capture()); + verify(mergeContext, times(1)).putPayloadById(payloadWrapper.capture()); + assertThat(payloadWrapper.getValue().payloadIdentifier()).isEqualTo(payloadId); - assertThat(blockWithReceipts.getAllValues().size()).isEqualTo(1); - assertThat(blockWithReceipts.getAllValues().get(0).getBlock().getBody().getTransactions()) + assertThat(payloadWrapper.getAllValues().size()).isEqualTo(1); + assertThat( + payloadWrapper + .getAllValues() + .get(0) + .blockWithReceipts() + .getBlock() + .getBody() + .getTransactions()) .hasSize(0); } @@ -605,12 +648,12 @@ public void shouldNotStartAnotherBlockCreationJobIfCalledAgainWithTheSamePayload } else { // when we have 5 transactions finalize block creation coordinator.finalizeProposalById( - invocation.getArgument(0, PayloadIdentifier.class)); + invocation.getArgument(0, PayloadWrapper.class).payloadIdentifier()); } return null; }) .when(mergeContext) - .putPayloadById(any(), any()); + .putPayloadById(any()); final long timestamp = System.currentTimeMillis() / 1000; @@ -642,17 +685,22 @@ public void shouldNotStartAnotherBlockCreationJobIfCalledAgainWithTheSamePayload blockCreationTask.get(); - ArgumentCaptor blockWithReceipts = - ArgumentCaptor.forClass(BlockWithReceipts.class); + ArgumentCaptor payloadWrapper = ArgumentCaptor.forClass(PayloadWrapper.class); - verify(mergeContext, times(retries.intValue())) - .putPayloadById(eq(payloadId1), blockWithReceipts.capture()); + verify(mergeContext, times(retries.intValue())).putPayloadById(payloadWrapper.capture()); - assertThat(blockWithReceipts.getAllValues().size()).isEqualTo(retries.intValue()); + assertThat(payloadWrapper.getAllValues().size()).isEqualTo(retries.intValue()); for (int i = 0; i < retries.intValue(); i++) { - assertThat(blockWithReceipts.getAllValues().get(i).getBlock().getBody().getTransactions()) + assertThat( + payloadWrapper + .getAllValues() + .get(i) + .blockWithReceipts() + .getBlock() + .getBody() + .getTransactions()) .hasSize(i); - assertThat(blockWithReceipts.getAllValues().get(i).getReceipts()).hasSize(i); + assertThat(payloadWrapper.getAllValues().get(i).blockWithReceipts().getReceipts()).hasSize(i); } } @@ -711,12 +759,13 @@ public void shouldUseExtraDataFromMiningParameters() { Optional.empty(), Optional.empty()); - ArgumentCaptor blockWithReceipts = - ArgumentCaptor.forClass(BlockWithReceipts.class); + ArgumentCaptor payloadWrapper = ArgumentCaptor.forClass(PayloadWrapper.class); - verify(mergeContext, atLeastOnce()).putPayloadById(eq(payloadId), blockWithReceipts.capture()); + verify(mergeContext, atLeastOnce()).putPayloadById(payloadWrapper.capture()); - assertThat(blockWithReceipts.getValue().getHeader().getExtraData()).isEqualTo(extraData); + assertThat(payloadWrapper.getValue().payloadIdentifier()).isEqualTo(payloadId); + assertThat(payloadWrapper.getValue().blockWithReceipts().getHeader().getExtraData()) + .isEqualTo(extraData); } @Test