From dfcd960d8bd6cc2e790c9050b766803b3c083008 Mon Sep 17 00:00:00 2001 From: Matt Whitehead Date: Fri, 15 Sep 2023 05:47:15 +0100 Subject: [PATCH 1/6] Don't start BFT mining coordinators until initial sync has completed (#5861) * Don't start BFT mining coordinators until initial sync has completed Signed-off-by: Matthew Whitehead * Fix unit tests Signed-off-by: Matthew Whitehead * Fix 'enable' logic Signed-off-by: Matthew Whitehead --------- Signed-off-by: Matthew Whitehead --- .../controller/IbftBesuControllerBuilder.java | 26 ++++++++++++++++++- .../controller/QbftBesuControllerBuilder.java | 26 ++++++++++++++++++- .../common/MigratingMiningCoordinator.java | 1 + .../blockcreation/BftMiningCoordinator.java | 14 +++++++--- .../BftMiningCoordinatorTest.java | 1 + 5 files changed, 63 insertions(+), 5 deletions(-) diff --git a/besu/src/main/java/org/hyperledger/besu/controller/IbftBesuControllerBuilder.java b/besu/src/main/java/org/hyperledger/besu/controller/IbftBesuControllerBuilder.java index 53d4771b5cc..40765769e6e 100644 --- a/besu/src/main/java/org/hyperledger/besu/controller/IbftBesuControllerBuilder.java +++ b/besu/src/main/java/org/hyperledger/besu/controller/IbftBesuControllerBuilder.java @@ -72,6 +72,7 @@ import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule; import org.hyperledger.besu.ethereum.p2p.config.SubProtocolConfiguration; import org.hyperledger.besu.ethereum.worldstate.WorldStateArchive; +import org.hyperledger.besu.plugin.services.BesuEvents; import org.hyperledger.besu.util.Subscribers; import java.util.HashMap; @@ -231,7 +232,30 @@ protected MiningCoordinator createMiningCoordinator( blockCreatorFactory, blockchain, bftEventQueue); - ibftMiningCoordinator.enable(); + + if (syncState.isInitialSyncPhaseDone()) { + LOG.info("Starting IBFT mining coordinator"); + ibftMiningCoordinator.enable(); + ibftMiningCoordinator.start(); + } else { + LOG.info("IBFT mining coordinator not starting while initial sync in progress"); + } + + syncState.subscribeCompletionReached( + new BesuEvents.InitialSyncCompletionListener() { + @Override + public void onInitialSyncCompleted() { + LOG.info("Starting IBFT mining coordinator following initial sync"); + ibftMiningCoordinator.enable(); + ibftMiningCoordinator.start(); + } + + @Override + public void onInitialSyncRestart() { + // Nothing to do. The mining coordinator won't be started until + // sync has completed. + } + }); return ibftMiningCoordinator; } diff --git a/besu/src/main/java/org/hyperledger/besu/controller/QbftBesuControllerBuilder.java b/besu/src/main/java/org/hyperledger/besu/controller/QbftBesuControllerBuilder.java index 45fc52f3695..11049e7f3f4 100644 --- a/besu/src/main/java/org/hyperledger/besu/controller/QbftBesuControllerBuilder.java +++ b/besu/src/main/java/org/hyperledger/besu/controller/QbftBesuControllerBuilder.java @@ -82,6 +82,7 @@ import org.hyperledger.besu.ethereum.p2p.config.SubProtocolConfiguration; import org.hyperledger.besu.ethereum.transaction.TransactionSimulator; import org.hyperledger.besu.ethereum.worldstate.WorldStateArchive; +import org.hyperledger.besu.plugin.services.BesuEvents; import org.hyperledger.besu.util.Subscribers; import java.util.HashMap; @@ -271,7 +272,30 @@ protected MiningCoordinator createMiningCoordinator( blockCreatorFactory, blockchain, bftEventQueue); - miningCoordinator.enable(); + + if (syncState.isInitialSyncPhaseDone()) { + LOG.info("Starting QBFT mining coordinator"); + miningCoordinator.enable(); + miningCoordinator.start(); + } else { + LOG.info("QBFT mining coordinator not starting while initial sync in progress"); + } + + syncState.subscribeCompletionReached( + new BesuEvents.InitialSyncCompletionListener() { + @Override + public void onInitialSyncCompleted() { + LOG.info("Starting QBFT mining coordinator following initial sync"); + miningCoordinator.enable(); + miningCoordinator.start(); + } + + @Override + public void onInitialSyncRestart() { + // Nothing to do. The mining coordinator won't be started until + // sync has completed. + } + }); return miningCoordinator; } diff --git a/consensus/common/src/main/java/org/hyperledger/besu/consensus/common/MigratingMiningCoordinator.java b/consensus/common/src/main/java/org/hyperledger/besu/consensus/common/MigratingMiningCoordinator.java index 84fbac198a5..de3fc3a3a0f 100644 --- a/consensus/common/src/main/java/org/hyperledger/besu/consensus/common/MigratingMiningCoordinator.java +++ b/consensus/common/src/main/java/org/hyperledger/besu/consensus/common/MigratingMiningCoordinator.java @@ -65,6 +65,7 @@ public void start() { } private void startActiveMiningCoordinator() { + activeMiningCoordinator.enable(); activeMiningCoordinator.start(); if (activeMiningCoordinator instanceof BlockAddedObserver) { ((BlockAddedObserver) activeMiningCoordinator).removeObserver(); diff --git a/consensus/common/src/main/java/org/hyperledger/besu/consensus/common/bft/blockcreation/BftMiningCoordinator.java b/consensus/common/src/main/java/org/hyperledger/besu/consensus/common/bft/blockcreation/BftMiningCoordinator.java index 6158bda8ba3..5107a00fe15 100644 --- a/consensus/common/src/main/java/org/hyperledger/besu/consensus/common/bft/blockcreation/BftMiningCoordinator.java +++ b/consensus/common/src/main/java/org/hyperledger/besu/consensus/common/bft/blockcreation/BftMiningCoordinator.java @@ -46,7 +46,9 @@ private enum State { /** Running state. */ RUNNING, /** Stopped state. */ - STOPPED + STOPPED, + /** Paused state. */ + PAUSED, } private static final Logger LOG = LoggerFactory.getLogger(BftMiningCoordinator.class); @@ -61,7 +63,7 @@ private enum State { private final BftExecutors bftExecutors; private long blockAddedObserverId; - private final AtomicReference state = new AtomicReference<>(State.IDLE); + private final AtomicReference state = new AtomicReference<>(State.PAUSED); /** * Instantiates a new Bft mining coordinator. @@ -122,7 +124,13 @@ public void awaitStop() throws InterruptedException { @Override public boolean enable() { - return true; + // Return true if we're already running or idle, or successfully switch to idle + if (state.get() == State.RUNNING + || state.get() == State.IDLE + || state.compareAndSet(State.PAUSED, State.IDLE)) { + return true; + } + return false; } @Override diff --git a/consensus/common/src/test/java/org/hyperledger/besu/consensus/common/bft/blockcreation/BftMiningCoordinatorTest.java b/consensus/common/src/test/java/org/hyperledger/besu/consensus/common/bft/blockcreation/BftMiningCoordinatorTest.java index 5af20546941..5b1d7a3cf64 100644 --- a/consensus/common/src/test/java/org/hyperledger/besu/consensus/common/bft/blockcreation/BftMiningCoordinatorTest.java +++ b/consensus/common/src/test/java/org/hyperledger/besu/consensus/common/bft/blockcreation/BftMiningCoordinatorTest.java @@ -75,6 +75,7 @@ public void stopsMining() { bftMiningCoordinator.stop(); verify(bftProcessor, never()).stop(); + bftMiningCoordinator.enable(); bftMiningCoordinator.start(); bftMiningCoordinator.stop(); verify(bftProcessor).stop(); From 3bb95be26feb2f44ffd7d83266b316f8d15ca387 Mon Sep 17 00:00:00 2001 From: matkt Date: Fri, 15 Sep 2023 08:26:44 +0200 Subject: [PATCH 2/6] display only peers ready for requets on ethstats (#5880) * display only ready for requets peers in ethstats Signed-off-by: Karim TAAM * cast to int Signed-off-by: Sally MacFarlane --------- Signed-off-by: Karim TAAM Signed-off-by: Sally MacFarlane Co-authored-by: Sally MacFarlane Co-authored-by: Stefan Pingel <16143240+pinges@users.noreply.github.com> --- .../java/org/hyperledger/besu/ethstats/EthStatsService.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/ethereum/ethstats/src/main/java/org/hyperledger/besu/ethstats/EthStatsService.java b/ethereum/ethstats/src/main/java/org/hyperledger/besu/ethstats/EthStatsService.java index 5adbf8978fb..0fa93e1b68c 100644 --- a/ethereum/ethstats/src/main/java/org/hyperledger/besu/ethstats/EthStatsService.java +++ b/ethereum/ethstats/src/main/java/org/hyperledger/besu/ethstats/EthStatsService.java @@ -424,7 +424,9 @@ private void sendNodeStatsReport() { final boolean isSyncing = syncState.isInSync(); final long gasPrice = suggestGasPrice(blockchainQueries.getBlockchain().getChainHeadBlock()); final long hashrate = miningCoordinator.hashesPerSecond().orElse(0L); - final int peersNumber = protocolManager.ethContext().getEthPeers().peerCount(); + // safe to cast to int since it isn't realistic to have more than max int peers + final int peersNumber = + (int) protocolManager.ethContext().getEthPeers().streamAvailablePeers().count(); final NodeStatsReport nodeStatsReport = ImmutableNodeStatsReport.builder() From 76459c2dc95d14a30eb06722a16d0413662866e3 Mon Sep 17 00:00:00 2001 From: Sally MacFarlane Date: Fri, 15 Sep 2023 16:59:06 +1000 Subject: [PATCH 3/6] [MINOR] test RLP used for encode/decode blob tx should contain to field (#5883) * validate to field on encode/decode for blob tx Signed-off-by: Sally MacFarlane * revert decode/encode checks - tis done later in tx validation Signed-off-by: Sally MacFarlane --------- Signed-off-by: Sally MacFarlane --- .../ethereum/core/encoding/BlobTransactionEncodingTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/core/encoding/BlobTransactionEncodingTest.java b/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/core/encoding/BlobTransactionEncodingTest.java index 49461a75303..9487e476685 100644 --- a/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/core/encoding/BlobTransactionEncodingTest.java +++ b/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/core/encoding/BlobTransactionEncodingTest.java @@ -37,9 +37,9 @@ private static Stream provideOpaqueBytesNoBlobsWithCommitments() { createArgument( "0x03f89d850120b996ed3685012a1a646085012a1a64608303345094ffb38a7a99e3e2335be83fc74b7faa19d55312418308a80280c085012a1a6460e1a00153a6a1e053cf4c5a09e84088ed8ad7cb53d76c8168f1b82f7cfebfcd06da1a01a007785223eec68459d72265f10bdb30ec3415252a63100605a03142fa211ebbe9a07dbbf9e081fa7b9a01202e4d9ee0e0e513f80efbbab6c784635429905389ce86"), createArgument( - "0x03f889850120b996ed81f0847735940084b2d05e158307a1208001855f495f4955c084b2d05e15e1a001d343d3cd62abd9c5754cbe5128c25ea90786a8ae75fb79c8cf95f4dcdd08ec80a014103732b5a9789bbf5ea859ed904155398abbef343f8fd63007efb70795d382a07272e847382789a092eadf08e2b9002e727376f8466fff0e4d4639fd60a528f2"), + "0x03f89d850120b996ed81f0847735940084b2d05e158307a12094000000000000000000000000000000000010101001855f495f4955c084b2d05e15e1a001d343d3cd62abd9c5754cbe5128c25ea90786a8ae75fb79c8cf95f4dcdd08ec80a014103732b5a9789bbf5ea859ed904155398abbef343f8fd63007efb70795d382a07272e847382789a092eadf08e2b9002e727376f8466fff0e4d4639fd60a528f2"), createArgument( - "0x03f889850120b996ed81f1843b9aca00847735940e8307a1208001855f495f4955c0847735940ee1a001d552e24560ec2f168be1d4a6385df61c70afe4288f00a3ad172da1a6f2b4f280a0b6690786e5fe79df67dcb60e8a9e8555142c3c96ffd5097c838717f0a7f64129a0112f01ed0cd3b86495f01736fbbc1b793f71565223aa26f093471a4d8605d198"), + "0x03f89d850120b996ed81f1843b9aca00847735940e8307a12094000000000000000000000000000000000010101001855f495f4955c0847735940ee1a001d552e24560ec2f168be1d4a6385df61c70afe4288f00a3ad172da1a6f2b4f280a0b6690786e5fe79df67dcb60e8a9e8555142c3c96ffd5097c838717f0a7f64129a0112f01ed0cd3b86495f01736fbbc1b793f71565223aa26f093471a4d8605d198"), createArgument( "0x03f897850120b996ed80840bebc200843b9aca078303345094c8d369b164361a8961286cfbab3bc10f962185a88080c08411e1a300e1a0011df88a2971c8a7ac494a7ba37ec1acaa1fc1edeeb38c839b5d1693d47b69b080a032f122f06e5802224db4c8a58fd22c75173a713f63f89936f811c144b9e40129a043a2a872cbfa5727007adf6a48febe5f190d2e4cd5ed6122823fb6ff47ecda32")); } From 80d9adaea5f497b508546ae1c6df1352cb6bc39d Mon Sep 17 00:00:00 2001 From: Fabio Di Fabio Date: Mon, 18 Sep 2023 01:26:46 +0200 Subject: [PATCH 4/6] Fix: correctly convert percentage options in TOML configuration file (#5886) Signed-off-by: Fabio Di Fabio --- .../besu/cli/util/TomlConfigFileDefaultProvider.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/besu/src/main/java/org/hyperledger/besu/cli/util/TomlConfigFileDefaultProvider.java b/besu/src/main/java/org/hyperledger/besu/cli/util/TomlConfigFileDefaultProvider.java index 9ec3f1ef640..c765b7f5dfc 100644 --- a/besu/src/main/java/org/hyperledger/besu/cli/util/TomlConfigFileDefaultProvider.java +++ b/besu/src/main/java/org/hyperledger/besu/cli/util/TomlConfigFileDefaultProvider.java @@ -15,6 +15,7 @@ package org.hyperledger.besu.cli.util; import org.hyperledger.besu.datatypes.Wei; +import org.hyperledger.besu.util.number.Fraction; import org.hyperledger.besu.util.number.Percentage; import java.io.File; @@ -92,6 +93,8 @@ private String getConfigurationValue(final OptionSpec optionSpec) { defaultValue = getNumericEntryAsString(optionSpec); } else if (optionSpec.type().equals(Percentage.class)) { defaultValue = getNumericEntryAsString(optionSpec); + } else if (optionSpec.type().equals(Fraction.class)) { + defaultValue = getNumericEntryAsString(optionSpec); } else { // else will be treated as String defaultValue = getEntryAsString(optionSpec); } From c4f73aa6431918a9d7c47a8059e715bd630ebebc Mon Sep 17 00:00:00 2001 From: Gabriel-Trintinalia Date: Mon, 18 Sep 2023 20:08:41 +1000 Subject: [PATCH 5/6] EIP7516 - Add BlobBaseFee opcode to Cancun EVM (#5884) Signed-off-by: Gabriel-Trintinalia --- .../mainnet/MainnetTransactionProcessor.java | 1 + .../core/MessageFrameTestFixture.java | 7 ++++ .../besu/evmtool/EvmToolCommand.java | 7 ++++ .../org/hyperledger/besu/evm/MainnetEVMs.java | 4 ++ .../besu/evm/fluent/EVMExecutor.java | 13 ++++++ .../besu/evm/frame/MessageFrame.java | 23 +++++++++++ .../hyperledger/besu/evm/frame/TxValues.java | 1 + .../evm/operation/BlobBaseFeeOperation.java | 41 +++++++++++++++++++ .../testutils/TestMessageFrameBuilder.java | 7 ++++ 9 files changed, 104 insertions(+) create mode 100644 evm/src/main/java/org/hyperledger/besu/evm/operation/BlobBaseFeeOperation.java diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/MainnetTransactionProcessor.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/MainnetTransactionProcessor.java index 7d235fb3a39..927e4cd8a29 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/MainnetTransactionProcessor.java +++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/MainnetTransactionProcessor.java @@ -363,6 +363,7 @@ public TransactionProcessingResult processTransaction( .initialGas(gasAvailable) .originator(senderAddress) .gasPrice(transactionGasPrice) + .blobGasPrice(blobGasPrice) .sender(senderAddress) .value(transaction.getValue()) .apparentValue(transaction.getValue()) diff --git a/ethereum/core/src/test-support/java/org/hyperledger/besu/ethereum/core/MessageFrameTestFixture.java b/ethereum/core/src/test-support/java/org/hyperledger/besu/ethereum/core/MessageFrameTestFixture.java index bfd56cbaa5d..262657872f5 100644 --- a/ethereum/core/src/test-support/java/org/hyperledger/besu/ethereum/core/MessageFrameTestFixture.java +++ b/ethereum/core/src/test-support/java/org/hyperledger/besu/ethereum/core/MessageFrameTestFixture.java @@ -48,6 +48,7 @@ public class MessageFrameTestFixture { private Address originator = DEFAUT_ADDRESS; private Address contract = DEFAUT_ADDRESS; private Wei gasPrice = Wei.ZERO; + private Wei blobGasPrice = Wei.ZERO; private Wei value = Wei.ZERO; private Bytes inputData = Bytes.EMPTY; private Code code = CodeV0.EMPTY_CODE; @@ -117,6 +118,11 @@ public MessageFrameTestFixture gasPrice(final Wei gasPrice) { return this; } + public MessageFrameTestFixture blobGasPrice(final Wei blobGasPrice) { + this.blobGasPrice = blobGasPrice; + return this; + } + public MessageFrameTestFixture value(final Wei value) { this.value = value; return this; @@ -160,6 +166,7 @@ public MessageFrame build() { .address(address) .originator(originator) .gasPrice(gasPrice) + .blobGasPrice(blobGasPrice) .inputData(inputData) .sender(sender) .value(value) diff --git a/ethereum/evmtool/src/main/java/org/hyperledger/besu/evmtool/EvmToolCommand.java b/ethereum/evmtool/src/main/java/org/hyperledger/besu/evmtool/EvmToolCommand.java index 4c51fc55998..d41dd56093b 100644 --- a/ethereum/evmtool/src/main/java/org/hyperledger/besu/evmtool/EvmToolCommand.java +++ b/ethereum/evmtool/src/main/java/org/hyperledger/besu/evmtool/EvmToolCommand.java @@ -116,6 +116,12 @@ void setBytes(final String optionValue) { paramLabel = "") private final Wei gasPriceGWei = Wei.ZERO; + @Option( + names = {"--blob-price"}, + description = "Price of blob gas for this invocation", + paramLabel = "") + private final Wei blobGasPrice = Wei.ZERO; + @Option( names = {"--sender"}, paramLabel = "
", @@ -376,6 +382,7 @@ public void run() { .originator(sender) .sender(sender) .gasPrice(gasPriceGWei) + .blobGasPrice(blobGasPrice) .inputData(callData) .value(ethValue) .apparentValue(ethValue) diff --git a/evm/src/main/java/org/hyperledger/besu/evm/MainnetEVMs.java b/evm/src/main/java/org/hyperledger/besu/evm/MainnetEVMs.java index 3aad40735d8..7884816ecf0 100644 --- a/evm/src/main/java/org/hyperledger/besu/evm/MainnetEVMs.java +++ b/evm/src/main/java/org/hyperledger/besu/evm/MainnetEVMs.java @@ -33,6 +33,7 @@ import org.hyperledger.besu.evm.operation.AndOperation; import org.hyperledger.besu.evm.operation.BalanceOperation; import org.hyperledger.besu.evm.operation.BaseFeeOperation; +import org.hyperledger.besu.evm.operation.BlobBaseFeeOperation; import org.hyperledger.besu.evm.operation.BlobHashOperation; import org.hyperledger.besu.evm.operation.BlockHashOperation; import org.hyperledger.besu.evm.operation.ByteOperation; @@ -856,6 +857,9 @@ public static void registerCancunOperations( // EIP-6780 nerf self destruct registry.put(new SelfDestructOperation(gasCalculator, true)); + + // EIP-7516 BLOBBASEFEE + registry.put(new BlobBaseFeeOperation(gasCalculator)); } /** diff --git a/evm/src/main/java/org/hyperledger/besu/evm/fluent/EVMExecutor.java b/evm/src/main/java/org/hyperledger/besu/evm/fluent/EVMExecutor.java index f25d5259b96..9921f56e64b 100644 --- a/evm/src/main/java/org/hyperledger/besu/evm/fluent/EVMExecutor.java +++ b/evm/src/main/java/org/hyperledger/besu/evm/fluent/EVMExecutor.java @@ -58,6 +58,7 @@ public class EVMExecutor { private Address receiver = Address.ZERO; private Address sender = Address.ZERO; private Wei gasPriceGWei = Wei.ZERO; + private Wei blobGasPrice = Wei.ZERO; private Bytes callData = Bytes.EMPTY; private Wei ethValue = Wei.ZERO; private Code code = CodeV0.EMPTY_CODE; @@ -354,6 +355,7 @@ public Bytes execute() { .originator(sender) .sender(sender) .gasPrice(gasPriceGWei) + .blobGasPrice(blobGasPrice) .inputData(callData) .value(ethValue) .apparentValue(ethValue) @@ -457,6 +459,17 @@ public EVMExecutor gasPriceGWei(final Wei gasPriceGWei) { return this; } + /** + * Sets Blob Gas price. + * + * @param blobGasPrice the blob gas price g wei + * @return the evm executor + */ + public EVMExecutor blobGasPrice(final Wei blobGasPrice) { + this.blobGasPrice = blobGasPrice; + return this; + } + /** * Sets Call data. * diff --git a/evm/src/main/java/org/hyperledger/besu/evm/frame/MessageFrame.java b/evm/src/main/java/org/hyperledger/besu/evm/frame/MessageFrame.java index a315e899b7a..ed711e3f181 100644 --- a/evm/src/main/java/org/hyperledger/besu/evm/frame/MessageFrame.java +++ b/evm/src/main/java/org/hyperledger/besu/evm/frame/MessageFrame.java @@ -1134,6 +1134,15 @@ public Wei getGasPrice() { return txValues.gasPrice(); } + /** + * Returns the current blob gas price. + * + * @return the current blob gas price + */ + public Wei getBlobGasPrice() { + return txValues.blobGasPrice(); + } + /** * Returns the recipient of the sender. * @@ -1365,6 +1374,7 @@ public static class Builder { private Address originator; private Address contract; private Wei gasPrice; + private Wei blobGasPrice = Wei.ZERO; private Bytes inputData; private Address sender; private Wei value; @@ -1472,6 +1482,17 @@ public Builder gasPrice(final Wei gasPrice) { return this; } + /** + * Sets Blob Gas price. + * + * @param blobGasPrice the blob gas price + * @return the builder + */ + public Builder blobGasPrice(final Wei blobGasPrice) { + this.blobGasPrice = blobGasPrice; + return this; + } + /** * Sets Input data. * @@ -1653,6 +1674,7 @@ private void validate() { checkState(worldUpdater != null, "Missing message frame world updater"); checkState(originator != null, "Missing message frame originator"); checkState(gasPrice != null, "Missing message frame getGasRemaining price"); + checkState(blobGasPrice != null, "Missing message frame blob gas price"); checkState(blockValues != null, "Missing message frame block header"); checkState(miningBeneficiary != null, "Missing mining beneficiary"); checkState(blockHashLookup != null, "Missing block hash lookup"); @@ -1689,6 +1711,7 @@ public MessageFrame build() { UndoTable.of(HashBasedTable.create()), originator, gasPrice, + blobGasPrice, blockValues, new ArrayDeque<>(), miningBeneficiary, diff --git a/evm/src/main/java/org/hyperledger/besu/evm/frame/TxValues.java b/evm/src/main/java/org/hyperledger/besu/evm/frame/TxValues.java index 65f7f47b569..6ef1143530f 100644 --- a/evm/src/main/java/org/hyperledger/besu/evm/frame/TxValues.java +++ b/evm/src/main/java/org/hyperledger/besu/evm/frame/TxValues.java @@ -40,6 +40,7 @@ public record TxValues( UndoTable warmedUpStorage, Address originator, Wei gasPrice, + Wei blobGasPrice, BlockValues blockValues, Deque messageFrameStack, Address miningBeneficiary, diff --git a/evm/src/main/java/org/hyperledger/besu/evm/operation/BlobBaseFeeOperation.java b/evm/src/main/java/org/hyperledger/besu/evm/operation/BlobBaseFeeOperation.java new file mode 100644 index 00000000000..a7168473882 --- /dev/null +++ b/evm/src/main/java/org/hyperledger/besu/evm/operation/BlobBaseFeeOperation.java @@ -0,0 +1,41 @@ +/* + * Copyright contributors to Hyperledger Besu + * + * 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.evm.operation; + +import org.hyperledger.besu.datatypes.Wei; +import org.hyperledger.besu.evm.EVM; +import org.hyperledger.besu.evm.frame.MessageFrame; +import org.hyperledger.besu.evm.gascalculator.GasCalculator; + +/** The Blob Base fee operation. */ +public class BlobBaseFeeOperation extends AbstractFixedCostOperation { + + /** + * Instantiates a new Blob Base fee operation. + * + * @param gasCalculator the gas calculator + */ + public BlobBaseFeeOperation(final GasCalculator gasCalculator) { + super(0x4a, "BLOBBASEFEE", 0, 1, gasCalculator, gasCalculator.getBaseTierGasCost()); + } + + @Override + public OperationResult executeFixedCostOperation(final MessageFrame frame, final EVM evm) { + + final Wei blobGasPrice = frame.getBlobGasPrice(); + frame.pushStackItem(blobGasPrice.toBytes()); + return successResponse; + } +} diff --git a/evm/src/test/java/org/hyperledger/besu/evm/testutils/TestMessageFrameBuilder.java b/evm/src/test/java/org/hyperledger/besu/evm/testutils/TestMessageFrameBuilder.java index 0e7fdb50418..f6b2abd69d9 100644 --- a/evm/src/test/java/org/hyperledger/besu/evm/testutils/TestMessageFrameBuilder.java +++ b/evm/src/test/java/org/hyperledger/besu/evm/testutils/TestMessageFrameBuilder.java @@ -47,6 +47,7 @@ public class TestMessageFrameBuilder { private Address originator = DEFAUT_ADDRESS; private Address contract = DEFAUT_ADDRESS; private Wei gasPrice = Wei.ZERO; + private Wei blobGasPrice = Wei.ZERO; private Wei value = Wei.ZERO; private Bytes inputData = Bytes.EMPTY; private Code code = CodeV0.EMPTY_CODE; @@ -91,6 +92,11 @@ public TestMessageFrameBuilder gasPrice(final Wei gasPrice) { return this; } + public TestMessageFrameBuilder blobGasPrice(final Wei blobGasPrice) { + this.blobGasPrice = blobGasPrice; + return this; + } + public TestMessageFrameBuilder value(final Wei value) { this.value = value; return this; @@ -145,6 +151,7 @@ public MessageFrame build() { .address(address) .originator(originator) .gasPrice(gasPrice) + .blobGasPrice(blobGasPrice) .inputData(inputData) .sender(sender) .value(value) From 3e724a01f5ec49e101aaa897af14209bece538dd Mon Sep 17 00:00:00 2001 From: matkt Date: Mon, 18 Sep 2023 17:58:26 +0200 Subject: [PATCH 6/6] Fix snapsync heal (#5838) Signed-off-by: Karim TAAM --- .../CheckpointDownloaderFactory.java | 2 +- .../sync/snapsync/SnapDownloaderFactory.java | 2 +- .../sync/snapsync/SnapWorldDownloadState.java | 127 +++++----- .../snapsync/SnapWorldStateDownloader.java | 10 +- .../SnapSyncStatePersistenceManager.java | 18 +- .../request/StorageRangeDataRequest.java | 7 +- ...ccountFlatDatabaseHealingRangeRequest.java | 46 ++-- .../heal/AccountTrieNodeHealingRequest.java | 36 ++- ...torageFlatDatabaseHealingRangeRequest.java | 19 +- .../heal/StorageTrieNodeHealingRequest.java | 30 +-- .../request/heal/TrieNodeHealingRequest.java | 2 + .../snapsync/AccountHealingTrackingTest.java | 218 ++++++++++++++++++ .../snapsync/SnapWorldDownloadStateTest.java | 35 ++- ...ntFlatDatabaseHealingRangeRequestTest.java | 4 +- .../StorageTrieNodeHealingRequestTest.java | 3 +- 15 files changed, 415 insertions(+), 144 deletions(-) create mode 100644 ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/AccountHealingTrackingTest.java diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/checkpointsync/CheckpointDownloaderFactory.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/checkpointsync/CheckpointDownloaderFactory.java index 441dcfeb9d5..ff8187d8d21 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/checkpointsync/CheckpointDownloaderFactory.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/checkpointsync/CheckpointDownloaderFactory.java @@ -88,7 +88,7 @@ public static Optional> createCheckpointDownloader( .getAccountToRepair() .ifPresent( address -> - snapContext.addAccountsToBeRepaired( + snapContext.addAccountToHealingList( CompactEncoding.bytesToPath(address.addressHash()))); } else if (fastSyncState.getPivotBlockHeader().isEmpty() && protocolContext.getBlockchain().getChainHeadBlockNumber() diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/SnapDownloaderFactory.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/SnapDownloaderFactory.java index 3f6596cf0b8..65fb117788e 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/SnapDownloaderFactory.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/SnapDownloaderFactory.java @@ -83,7 +83,7 @@ public static Optional> createSnapDownloader( .getAccountToRepair() .ifPresent( address -> - snapContext.addAccountsToBeRepaired( + snapContext.addAccountToHealingList( CompactEncoding.bytesToPath(address.addressHash()))); } else if (fastSyncState.getPivotBlockHeader().isEmpty() && protocolContext.getBlockchain().getChainHeadBlockNumber() diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/SnapWorldDownloadState.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/SnapWorldDownloadState.java index 3abd1b0d5ec..dd7481cdd00 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/SnapWorldDownloadState.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/SnapWorldDownloadState.java @@ -28,7 +28,6 @@ import org.hyperledger.besu.ethereum.eth.sync.snapsync.request.heal.AccountFlatDatabaseHealingRangeRequest; import org.hyperledger.besu.ethereum.eth.sync.snapsync.request.heal.StorageFlatDatabaseHealingRangeRequest; import org.hyperledger.besu.ethereum.eth.sync.worldstate.WorldDownloadState; -import org.hyperledger.besu.ethereum.worldstate.DataStorageFormat; import org.hyperledger.besu.ethereum.worldstate.FlatDbMode; import org.hyperledger.besu.ethereum.worldstate.WorldStateStorage; import org.hyperledger.besu.metrics.BesuMetricCategory; @@ -43,6 +42,7 @@ import java.util.List; import java.util.Map; import java.util.OptionalLong; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.Consumer; import java.util.function.Predicate; import java.util.stream.Stream; @@ -72,7 +72,7 @@ public class SnapWorldDownloadState extends WorldDownloadState protected final InMemoryTasksPriorityQueues pendingStorageFlatDatabaseHealingRequests = new InMemoryTasksPriorityQueues<>(); - private HashSet accountsToBeRepaired = new HashSet<>(); + private HashSet accountsHealingList = new HashSet<>(); private DynamicPivotBlockSelector pivotBlockSelector; private final SnapSyncStatePersistenceManager snapContext; @@ -156,6 +156,7 @@ protected synchronized void markAsStalled(final int maxNodeRequestRetries) { @Override public synchronized boolean checkCompletion(final BlockHeader header) { + // Check if all snapsync tasks are completed if (!internalFuture.isDone() && pendingAccountRequests.allTasksCompleted() && pendingCodeRequests.allTasksCompleted() @@ -164,29 +165,50 @@ public synchronized boolean checkCompletion(final BlockHeader header) { && pendingTrieNodeRequests.allTasksCompleted() && pendingAccountFlatDatabaseHealingRequests.allTasksCompleted() && pendingStorageFlatDatabaseHealingRequests.allTasksCompleted()) { + + // if all snapsync tasks are completed and the healing process was not running if (!snapSyncState.isHealTrieInProgress()) { + // Register blockchain observer if not already registered + blockObserverId = + blockObserverId.isEmpty() + ? OptionalLong.of(blockchain.observeBlockAdded(createBlockchainObserver())) + : blockObserverId; + // Start the healing process startTrieHeal(); - } else if (pivotBlockSelector.isBlockchainBehind()) { + } + // if all snapsync tasks are completed and the healing was running and blockchain is behind + // the pivot block + else if (pivotBlockSelector.isBlockchainBehind()) { LOG.info("Pausing world state download while waiting for sync to complete"); - if (blockObserverId.isEmpty()) - blockObserverId = OptionalLong.of(blockchain.observeBlockAdded(getBlockAddedListener())); + // Set the snapsync to wait for the blockchain to catch up snapSyncState.setWaitingBlockchain(true); - } else if (!snapSyncState.isHealFlatDatabaseInProgress() - && worldStateStorage.getFlatDbMode().equals(FlatDbMode.FULL)) { - // only doing a flat db heal for bonsai - startFlatDatabaseHeal(header); - } else { - final WorldStateStorage.Updater updater = worldStateStorage.updater(); - updater.saveWorldState(header.getHash(), header.getStateRoot(), rootNodeData); - updater.commit(); - metricsManager.notifySnapSyncCompleted(); - snapContext.clear(); - internalFuture.complete(null); - - return true; + } + // if all snapsync tasks are completed and the healing was running and the blockchain is not + // behind the pivot block + else { + // Remove the blockchain observer + blockObserverId.ifPresent(blockchain::removeObserver); + // If the flat database healing process is not in progress and the flat database mode is + // FULL + if (!snapSyncState.isHealFlatDatabaseInProgress() + && worldStateStorage.getFlatDbMode().equals(FlatDbMode.FULL)) { + // Start the flat database healing process + startFlatDatabaseHeal(header); + } + // If the flat database healing process is in progress or the flat database mode is not FULL + else { + final WorldStateStorage.Updater updater = worldStateStorage.updater(); + updater.saveWorldState(header.getHash(), header.getStateRoot(), rootNodeData); + updater.commit(); + // Notify that the snap sync has completed + metricsManager.notifySnapSyncCompleted(); + // Clear the snap context + snapContext.clear(); + internalFuture.complete(null); + return true; + } } } - return false; } @@ -200,10 +222,11 @@ protected synchronized void cleanupQueues() { pendingTrieNodeRequests.clear(); } + /** Method to start the healing process of the trie */ public synchronized void startTrieHeal() { snapContext.clearAccountRangeTasks(); snapSyncState.setHealTrieStatus(true); - // try to find new pivot block before healing + // Try to find a new pivot block before starting the healing process pivotBlockSelector.switchToNewPivotBlock( (blockHeader, newPivotBlockFound) -> { snapContext.clearAccountRangeTasks(); @@ -212,21 +235,25 @@ public synchronized void startTrieHeal() { blockHeader.getNumber()); enqueueRequest( createAccountTrieNodeDataRequest( - blockHeader.getStateRoot(), Bytes.EMPTY, accountsToBeRepaired)); + blockHeader.getStateRoot(), Bytes.EMPTY, accountsHealingList)); }); } + /** Method to reload the healing process of the trie */ public synchronized void reloadTrieHeal() { + // Clear the flat database and trie log from the world state storage if needed worldStateStorage.clearFlatDatabase(); worldStateStorage.clearTrieLog(); + // Clear pending trie node and code requests pendingTrieNodeRequests.clear(); pendingCodeRequests.clear(); + snapSyncState.setHealTrieStatus(false); checkCompletion(snapSyncState.getPivotBlockHeader().orElseThrow()); } public synchronized void startFlatDatabaseHeal(final BlockHeader header) { - LOG.info("Running flat database heal process"); + LOG.info("Initiating the healing process for the flat database"); snapSyncState.setHealFlatDatabaseInProgress(true); final Map ranges = RangeManager.generateAllRanges(16); ranges.forEach( @@ -235,10 +262,6 @@ public synchronized void startFlatDatabaseHeal(final BlockHeader header) { createAccountFlatHealingRangeRequest(header.getStateRoot(), key, value))); } - public boolean isBonsaiStorageFormat() { - return worldStateStorage.getDataStorageFormat().equals(DataStorageFormat.BONSAI); - } - @Override public synchronized void enqueueRequest(final SnapDataRequest request) { if (!internalFuture.isDone()) { @@ -263,8 +286,8 @@ public synchronized void enqueueRequest(final SnapDataRequest request) { } } - public synchronized void setAccountsToBeRepaired(final HashSet accountsToBeRepaired) { - this.accountsToBeRepaired = accountsToBeRepaired; + public synchronized void setAccountsHealingList(final HashSet addAccountToHealingList) { + this.accountsHealingList = addAccountToHealingList; } /** @@ -274,15 +297,15 @@ public synchronized void setAccountsToBeRepaired(final HashSet accountsTo * * @param account The account to be added for repair. */ - public synchronized void addAccountsToBeRepaired(final Bytes account) { - if (!accountsToBeRepaired.contains(account)) { - snapContext.addAccountsToBeRepaired(account); - accountsToBeRepaired.add(account); + public synchronized void addAccountToHealingList(final Bytes account) { + if (!accountsHealingList.contains(account)) { + snapContext.addAccountToHealingList(account); + accountsHealingList.add(account); } } - public HashSet getAccountsToBeRepaired() { - return accountsToBeRepaired; + public HashSet getAccountsHealingList() { + return accountsHealingList; } @Override @@ -385,25 +408,25 @@ public void setPivotBlockSelector(final DynamicPivotBlockSelector pivotBlockSele this.pivotBlockSelector = pivotBlockSelector; } - public BlockAddedObserver getBlockAddedListener() { + public BlockAddedObserver createBlockchainObserver() { return addedBlockContext -> { - if (snapSyncState.isWaitingBlockchain()) { - // if we receive a new pivot block we can restart the heal - pivotBlockSelector.check( - (____, isNewPivotBlock) -> { - if (isNewPivotBlock) { - snapSyncState.setWaitingBlockchain(false); - } - }); - // if we are close to the head we can also restart the heal and finish snapsync - if (!pivotBlockSelector.isBlockchainBehind()) { - snapSyncState.setWaitingBlockchain(false); - } - if (!snapSyncState.isWaitingBlockchain()) { - blockObserverId.ifPresent(blockchain::removeObserver); - blockObserverId = OptionalLong.empty(); - reloadTrieHeal(); - } + final AtomicBoolean foundNewPivotBlock = new AtomicBoolean(false); + pivotBlockSelector.check( + (____, isNewPivotBlock) -> { + if (isNewPivotBlock) { + foundNewPivotBlock.set(true); + } + }); + + final boolean isNewPivotBlockFound = foundNewPivotBlock.get(); + final boolean isBlockchainCaughtUp = + snapSyncState.isWaitingBlockchain() && !pivotBlockSelector.isBlockchainBehind(); + + if (isNewPivotBlockFound + || isBlockchainCaughtUp) { // restart heal if we found a new pivot block or if close to + // head again + snapSyncState.setWaitingBlockchain(false); + reloadTrieHeal(); } }; } diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/SnapWorldStateDownloader.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/SnapWorldStateDownloader.java index ca2afbab347..91bdd83a0f3 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/SnapWorldStateDownloader.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/SnapWorldStateDownloader.java @@ -153,10 +153,10 @@ public CompletableFuture run( final List currentAccountRange = snapContext.getCurrentAccountRange(); - final HashSet inconsistentAccounts = snapContext.getAccountsToBeRepaired(); + final HashSet inconsistentAccounts = snapContext.getAccountsHealingList(); if (!currentAccountRange.isEmpty()) { // continue to download worldstate ranges - newDownloadState.setAccountsToBeRepaired(inconsistentAccounts); + newDownloadState.setAccountsHealingList(inconsistentAccounts); snapContext .getCurrentAccountRange() .forEach( @@ -165,14 +165,14 @@ public CompletableFuture run( DOWNLOAD, snapDataRequest.getStartKeyHash(), snapDataRequest.getEndKeyHash()); newDownloadState.enqueueRequest(snapDataRequest); }); - } else if (!snapContext.getAccountsToBeRepaired().isEmpty()) { // restart only the heal step + } else if (!snapContext.getAccountsHealingList().isEmpty()) { // restart only the heal step snapSyncState.setHealTrieStatus(true); worldStateStorage.clearFlatDatabase(); worldStateStorage.clearTrieLog(); - newDownloadState.setAccountsToBeRepaired(inconsistentAccounts); + newDownloadState.setAccountsHealingList(inconsistentAccounts); newDownloadState.enqueueRequest( SnapDataRequest.createAccountTrieNodeDataRequest( - stateRoot, Bytes.EMPTY, snapContext.getAccountsToBeRepaired())); + stateRoot, Bytes.EMPTY, snapContext.getAccountsHealingList())); } else { // start from scratch worldStateStorage.clear(); diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/context/SnapSyncStatePersistenceManager.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/context/SnapSyncStatePersistenceManager.java index d038c3d4ba3..75ee91352ad 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/context/SnapSyncStatePersistenceManager.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/context/SnapSyncStatePersistenceManager.java @@ -41,7 +41,7 @@ */ public class SnapSyncStatePersistenceManager { - private final byte[] SNAP_ACCOUNT_TO_BE_REPAIRED_INDEX = + private final byte[] SNAP_ACCOUNT_HEALING_LIST_INDEX = "snapInconsistentAccountsStorageIndex".getBytes(StandardCharsets.UTF_8); private final GenericKeyValueStorageFacade @@ -104,20 +104,20 @@ public void updatePersistedTasks(final List accountRa } /** - * Persists the current accounts to be repaired in the database. + * Persists the current accounts to heal in the database. * - * @param accountsToBeRepaired The current list of accounts to persist. + * @param accountsHealingList The current list of accounts to heal. */ - public void addAccountsToBeRepaired(final Bytes accountsToBeRepaired) { + public void addAccountToHealingList(final Bytes accountsHealingList) { final BigInteger index = healContext - .get(SNAP_ACCOUNT_TO_BE_REPAIRED_INDEX) + .get(SNAP_ACCOUNT_HEALING_LIST_INDEX) .map(bytes -> new BigInteger(bytes.toArrayUnsafe()).add(BigInteger.ONE)) .orElse(BigInteger.ZERO); healContext.putAll( keyValueStorageTransaction -> { - keyValueStorageTransaction.put(SNAP_ACCOUNT_TO_BE_REPAIRED_INDEX, index.toByteArray()); - keyValueStorageTransaction.put(index.toByteArray(), accountsToBeRepaired.toArrayUnsafe()); + keyValueStorageTransaction.put(SNAP_ACCOUNT_HEALING_LIST_INDEX, index.toByteArray()); + keyValueStorageTransaction.put(index.toByteArray(), accountsHealingList.toArrayUnsafe()); }); } @@ -127,9 +127,9 @@ public List getCurrentAccountRange() { .collect(Collectors.toList()); } - public HashSet getAccountsToBeRepaired() { + public HashSet getAccountsHealingList() { return healContext - .streamValuesFromKeysThat(notEqualsTo(SNAP_ACCOUNT_TO_BE_REPAIRED_INDEX)) + .streamValuesFromKeysThat(notEqualsTo(SNAP_ACCOUNT_HEALING_LIST_INDEX)) .collect(Collectors.toCollection(HashSet::new)); } diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/request/StorageRangeDataRequest.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/request/StorageRangeDataRequest.java index 58c1702cbf5..c18d063d74d 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/request/StorageRangeDataRequest.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/request/StorageRangeDataRequest.java @@ -124,6 +124,11 @@ public void addResponse( if (!slots.isEmpty() || !proofs.isEmpty()) { if (!worldStateProofProvider.isValidRangeProof( startKeyHash, endKeyHash, storageRoot, proofs, slots)) { + // If the proof is invalid, it means that the storage will be a mix of several blocks. + // Therefore, it will be necessary to heal the account's storage subsequently + downloadState.addAccountToHealingList(CompactEncoding.bytesToPath(accountHash)); + // We will request the new storage root of the account because it is apparently no longer + // valid with the new pivot block. downloadState.enqueueRequest( createAccountDataRequest( getRootHash(), Hash.wrap(accountHash), startKeyHash, endKeyHash)); @@ -173,7 +178,7 @@ public Stream getChildRequests( }); if (startKeyHash.equals(MIN_RANGE) && endKeyHash.equals(MAX_RANGE)) { // need to heal this account storage - downloadState.addAccountsToBeRepaired(CompactEncoding.bytesToPath(accountHash)); + downloadState.addAccountToHealingList(CompactEncoding.bytesToPath(accountHash)); } }); diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/request/heal/AccountFlatDatabaseHealingRangeRequest.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/request/heal/AccountFlatDatabaseHealingRangeRequest.java index c96b39e44c6..a40a4fb11a3 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/request/heal/AccountFlatDatabaseHealingRangeRequest.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/request/heal/AccountFlatDatabaseHealingRangeRequest.java @@ -59,7 +59,7 @@ public class AccountFlatDatabaseHealingRangeRequest extends SnapDataRequest { private final Bytes32 endKeyHash; private TreeMap existingAccounts; - private TreeMap removedAccounts; + private TreeMap flatDbAccounts; private boolean isProofValid; public AccountFlatDatabaseHealingRangeRequest( @@ -68,7 +68,7 @@ public AccountFlatDatabaseHealingRangeRequest( this.startKeyHash = startKeyHash; this.endKeyHash = endKeyHash; this.existingAccounts = new TreeMap<>(); - this.removedAccounts = new TreeMap<>(); + this.flatDbAccounts = new TreeMap<>(); this.isProofValid = false; } @@ -95,12 +95,12 @@ public Stream getChildRequests( downloadState.getMetricsManager().notifyRangeProgress(HEAL_FLAT, endKeyHash, endKeyHash); } - Stream.of(existingAccounts.entrySet(), removedAccounts.entrySet()) + Stream.of(existingAccounts.entrySet(), flatDbAccounts.entrySet()) .flatMap(Collection::stream) .forEach( account -> { if (downloadState - .getAccountsToBeRepaired() + .getAccountsHealingList() .contains(CompactEncoding.bytesToPath(account.getKey()))) { final StateTrieAccountValue accountValue = StateTrieAccountValue.readFrom(RLP.input(account.getValue())); @@ -174,7 +174,7 @@ protected int doPersist( // put all flat accounts in the list, and gradually keep only those that are not in the trie // to remove and heal them. - removedAccounts = new TreeMap<>(existingAccounts); + flatDbAccounts = new TreeMap<>(existingAccounts); final TrieIterator visitor = RangeStorageEntriesCollector.createVisitor(collector); existingAccounts = @@ -184,27 +184,33 @@ protected int doPersist( RangeStorageEntriesCollector.collectEntries( collector, visitor, root, startKeyHash)); - // doing the fix + // Process each existing account existingAccounts.forEach( (key, value) -> { - if (removedAccounts.containsKey(key)) { - removedAccounts.remove(key); - } else { - final Hash accountHash = Hash.wrap(key); - // if the account was missing in the flat db we need to heal the storage - downloadState.addAccountsToBeRepaired(CompactEncoding.bytesToPath(accountHash)); + // Remove the key from the flat db list and get its associated value + Bytes flatDbEntry = flatDbAccounts.remove(key); + // If the key was in flat db and its associated value is different from the + // current value + if (!value.equals(flatDbEntry)) { + Hash accountHash = Hash.wrap(key); + // Add the account to the list of accounts to be repaired + downloadState.addAccountToHealingList(CompactEncoding.bytesToPath(accountHash)); + // Update the account info state bonsaiUpdater.putAccountInfoState(accountHash, value); } }); - removedAccounts.forEach( - (key, value) -> { - final Hash accountHash = Hash.wrap(key); - // if the account was removed we will have to heal the storage - downloadState.addAccountsToBeRepaired(CompactEncoding.bytesToPath(accountHash)); - bonsaiUpdater.removeAccountInfoState(accountHash); - }); + // For each remaining account in flat db list, remove the account info state and add it to + // the list of accounts to be repaired + flatDbAccounts + .keySet() + .forEach( + key -> { + Hash accountHash = Hash.wrap(key); + downloadState.addAccountToHealingList(CompactEncoding.bytesToPath(accountHash)); + bonsaiUpdater.removeAccountInfoState(accountHash); + }); } - return existingAccounts.size() + removedAccounts.size(); + return existingAccounts.size() + flatDbAccounts.size(); } } diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/request/heal/AccountTrieNodeHealingRequest.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/request/heal/AccountTrieNodeHealingRequest.java index 07089b20878..d9297217146 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/request/heal/AccountTrieNodeHealingRequest.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/request/heal/AccountTrieNodeHealingRequest.java @@ -112,6 +112,15 @@ public Stream getRootStorageRequests(final WorldStateStorage wo account.size() - getLocation().size())) .map(RLP::input) .map(StateTrieAccountValue::readFrom) + .filter( + stateTrieAccountValue -> + // We need to ensure that the accounts to be healed do not have empty storage. + // Therefore, it is unnecessary to create trie heal requests for storage in this + // case. + // If we were to do so, we would be attempting to request storage that does not + // exist from our peers, + // which would cause sync issues. + !stateTrieAccountValue.getStorageRoot().equals(MerkleTrie.EMPTY_TRIE_NODE_HASH)) .ifPresent( stateTrieAccountValue -> { // an account need a heal step @@ -129,6 +138,7 @@ public Stream getRootStorageRequests(final WorldStateStorage wo @Override protected Stream getRequestsFromTrieNodeValue( final WorldStateStorage worldStateStorage, + final SnapWorldDownloadState downloadState, final Bytes location, final Bytes path, final Bytes value) { @@ -151,13 +161,25 @@ protected Stream getRequestsFromTrieNodeValue( if (!accountValue.getCodeHash().equals(Hash.EMPTY)) { builder.add(createBytecodeRequest(accountHash, getRootHash(), accountValue.getCodeHash())); } - // Add storage, if appropriate - if (!accountValue.getStorageRoot().equals(MerkleTrie.EMPTY_TRIE_NODE_HASH)) { - // If we detect an account storage we fill it with snapsync before completing with a heal - final SnapDataRequest storageTrieRequest = - createStorageTrieNodeDataRequest( - accountValue.getStorageRoot(), accountHash, getRootHash(), Bytes.EMPTY); - builder.add(storageTrieRequest); + + // Retrieve the storage root from the database, if available + final Hash storageRootFoundInDb = + worldStateStorage + .getTrieNodeUnsafe(Bytes.concatenate(accountHash, Bytes.EMPTY)) + .map(Hash::hash) + .orElse(Hash.wrap(MerkleTrie.EMPTY_TRIE_NODE_HASH)); + if (!storageRootFoundInDb.equals(accountValue.getStorageRoot())) { + // If the storage root is not found in the database, add the account to the list of accounts + // to be repaired + downloadState.addAccountToHealingList(CompactEncoding.bytesToPath(accountHash)); + // If the account's storage root is not empty, + // fill it with trie heal before completing with a flat heal + if (!accountValue.getStorageRoot().equals(MerkleTrie.EMPTY_TRIE_NODE_HASH)) { + SnapDataRequest storageTrieRequest = + createStorageTrieNodeDataRequest( + accountValue.getStorageRoot(), accountHash, getRootHash(), Bytes.EMPTY); + builder.add(storageTrieRequest); + } } return builder.build(); } diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/request/heal/StorageFlatDatabaseHealingRangeRequest.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/request/heal/StorageFlatDatabaseHealingRangeRequest.java index 45380e49985..a94424f023f 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/request/heal/StorageFlatDatabaseHealingRangeRequest.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/request/heal/StorageFlatDatabaseHealingRangeRequest.java @@ -152,7 +152,7 @@ protected int doPersist( Function.identity(), Function.identity()); - Map remainingKeys = new TreeMap<>(slots); + Map flatDbSlots = new TreeMap<>(slots); // Retrieve the data from the trie in order to know what needs to be fixed in the flat // database @@ -172,18 +172,23 @@ protected int doPersist( RangeStorageEntriesCollector.collectEntries( collector, visitor, root, startKeyHash)); - // Perform the fix by updating the flat database + // Process each slot slots.forEach( (key, value) -> { - if (remainingKeys.containsKey(key)) { - remainingKeys.remove(key); - } else { + // Remove the key from the flat db and get its associated value + final Bytes flatDbEntry = flatDbSlots.remove(key); + // If the key was not in flat db and its associated value is different from the + // current value + if (!value.equals(flatDbEntry)) { + // Update the storage value bonsaiUpdater.putStorageValueBySlotHash( accountHash, Hash.wrap(key), Bytes32.leftPad(RLP.decodeValue(value))); } }); - remainingKeys.forEach( - (key, value) -> bonsaiUpdater.removeStorageValueBySlotHash(accountHash, Hash.wrap(key))); + // For each remaining key, remove the storage value + flatDbSlots + .keySet() + .forEach(key -> bonsaiUpdater.removeStorageValueBySlotHash(accountHash, Hash.wrap(key))); } return slots.size(); } diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/request/heal/StorageTrieNodeHealingRequest.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/request/heal/StorageTrieNodeHealingRequest.java index 779a5a72c8a..ffcdb3e77b7 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/request/heal/StorageTrieNodeHealingRequest.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/request/heal/StorageTrieNodeHealingRequest.java @@ -21,8 +21,6 @@ import org.hyperledger.besu.ethereum.eth.sync.snapsync.SnapWorldDownloadState; import org.hyperledger.besu.ethereum.eth.sync.snapsync.request.SnapDataRequest; import org.hyperledger.besu.ethereum.trie.CompactEncoding; -import org.hyperledger.besu.ethereum.trie.MerkleTrie; -import org.hyperledger.besu.ethereum.worldstate.DataStorageFormat; import org.hyperledger.besu.ethereum.worldstate.FlatDbMode; import org.hyperledger.besu.ethereum.worldstate.WorldStateStorage; import org.hyperledger.besu.ethereum.worldstate.WorldStateStorage.Updater; @@ -60,31 +58,8 @@ protected int doPersist( @Override public Optional getExistingData( final SnapWorldDownloadState downloadState, final WorldStateStorage worldStateStorage) { - - final Optional storageTrieNode; - if (worldStateStorage.getDataStorageFormat().equals(DataStorageFormat.FOREST)) { - storageTrieNode = worldStateStorage.getTrieNodeUnsafe(getNodeHash()); - } else { - storageTrieNode = - worldStateStorage.getTrieNodeUnsafe(Bytes.concatenate(getAccountHash(), getLocation())); - } - - if (storageTrieNode.isPresent()) { - return storageTrieNode - .filter(node -> Hash.hash(node).equals(getNodeHash())) - .or( - () -> { // if we have a storage in database but not the good one we will need to fix - // the account later - downloadState.addAccountsToBeRepaired( - CompactEncoding.bytesToPath(getAccountHash())); - return Optional.empty(); - }); - } else { - if (getNodeHash().equals(MerkleTrie.EMPTY_TRIE_NODE_HASH)) { - return Optional.of(MerkleTrie.EMPTY_TRIE_NODE); - } - return Optional.empty(); - } + return worldStateStorage.getAccountStorageTrieNode( + getAccountHash(), getLocation(), getNodeHash()); } @Override @@ -96,6 +71,7 @@ protected SnapDataRequest createChildNodeDataRequest(final Hash childHash, final @Override protected Stream getRequestsFromTrieNodeValue( final WorldStateStorage worldStateStorage, + final SnapWorldDownloadState downloadState, final Bytes location, final Bytes path, final Bytes value) { diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/request/heal/TrieNodeHealingRequest.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/request/heal/TrieNodeHealingRequest.java index 2cb8ee3ca8a..c04066141d8 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/request/heal/TrieNodeHealingRequest.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/request/heal/TrieNodeHealingRequest.java @@ -104,6 +104,7 @@ public Stream getChildRequests( value -> getRequestsFromTrieNodeValue( worldStateStorage, + downloadState, node.getLocation().orElse(Bytes.EMPTY), node.getPath(), value)) @@ -179,6 +180,7 @@ public Stream getRootStorageRequests(final WorldStateStorage wo protected abstract Stream getRequestsFromTrieNodeValue( final WorldStateStorage worldStateStorage, + final SnapWorldDownloadState downloadState, final Bytes location, final Bytes path, final Bytes value); diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/AccountHealingTrackingTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/AccountHealingTrackingTest.java new file mode 100644 index 00000000000..ca39d0c86fe --- /dev/null +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/AccountHealingTrackingTest.java @@ -0,0 +1,218 @@ +/* + * 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.ethereum.eth.sync.snapsync; + +import static org.hyperledger.besu.ethereum.eth.sync.snapsync.RangeManager.MAX_RANGE; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; + +import org.hyperledger.besu.datatypes.Address; +import org.hyperledger.besu.datatypes.Hash; +import org.hyperledger.besu.ethereum.bonsai.storage.BonsaiWorldStateKeyValueStorage; +import org.hyperledger.besu.ethereum.core.InMemoryKeyValueStorageProvider; +import org.hyperledger.besu.ethereum.core.TrieGenerator; +import org.hyperledger.besu.ethereum.eth.sync.snapsync.request.SnapDataRequest; +import org.hyperledger.besu.ethereum.eth.sync.snapsync.request.StorageRangeDataRequest; +import org.hyperledger.besu.ethereum.eth.sync.snapsync.request.heal.StorageTrieNodeHealingRequest; +import org.hyperledger.besu.ethereum.proof.WorldStateProofProvider; +import org.hyperledger.besu.ethereum.rlp.RLP; +import org.hyperledger.besu.ethereum.trie.MerkleTrie; +import org.hyperledger.besu.ethereum.trie.RangeStorageEntriesCollector; +import org.hyperledger.besu.ethereum.trie.TrieIterator; +import org.hyperledger.besu.ethereum.trie.patricia.StoredMerklePatriciaTrie; +import org.hyperledger.besu.ethereum.trie.patricia.StoredNodeFactory; +import org.hyperledger.besu.ethereum.worldstate.StateTrieAccountValue; +import org.hyperledger.besu.ethereum.worldstate.WorldStateStorage; +import org.hyperledger.besu.metrics.noop.NoOpMetricsSystem; + +import java.util.List; +import java.util.TreeMap; +import java.util.function.Function; +import java.util.stream.Collectors; + +import kotlin.collections.ArrayDeque; +import org.apache.tuweni.bytes.Bytes; +import org.apache.tuweni.bytes.Bytes32; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; + +@ExtendWith(MockitoExtension.class) +public class AccountHealingTrackingTest { + + private final List
accounts = List.of(Address.fromHexString("0xdeadbeef")); + private final WorldStateStorage worldStateStorage = + new BonsaiWorldStateKeyValueStorage( + new InMemoryKeyValueStorageProvider(), new NoOpMetricsSystem()); + + private WorldStateProofProvider worldStateProofProvider; + + private MerkleTrie accountStateTrie; + + @Mock SnapWorldDownloadState snapWorldDownloadState; + + @BeforeEach + public void setup() { + accountStateTrie = + TrieGenerator.generateTrie( + worldStateStorage, + accounts.stream().map(Address::addressHash).collect(Collectors.toList())); + worldStateProofProvider = new WorldStateProofProvider(worldStateStorage); + } + + @Test + void avoidMarkingAccountWhenStorageProofValid() { + + // generate valid proof + final Hash accountHash = Hash.hash(accounts.get(0)); + final StateTrieAccountValue stateTrieAccountValue = + StateTrieAccountValue.readFrom(RLP.input(accountStateTrie.get(accountHash).orElseThrow())); + + final StoredMerklePatriciaTrie storageTrie = + new StoredMerklePatriciaTrie<>( + new StoredNodeFactory<>( + (location, hash) -> + worldStateStorage.getAccountStorageTrieNode(accountHash, location, hash), + Function.identity(), + Function.identity()), + stateTrieAccountValue.getStorageRoot()); + + final RangeStorageEntriesCollector collector = + RangeStorageEntriesCollector.createCollector(Hash.ZERO, MAX_RANGE, 10, Integer.MAX_VALUE); + final TrieIterator visitor = RangeStorageEntriesCollector.createVisitor(collector); + final TreeMap slots = + (TreeMap) + storageTrie.entriesFrom( + root -> + RangeStorageEntriesCollector.collectEntries( + collector, visitor, root, Hash.ZERO)); + // generate the proof + final List proofs = + worldStateProofProvider.getStorageProofRelatedNodes( + Hash.wrap(storageTrie.getRootHash()), accountHash, Hash.ZERO); + proofs.addAll( + worldStateProofProvider.getStorageProofRelatedNodes( + Hash.wrap(storageTrie.getRootHash()), accountHash, slots.lastKey())); + + final StorageRangeDataRequest storageRangeDataRequest = + SnapDataRequest.createStorageRangeDataRequest( + Hash.wrap(accountStateTrie.getRootHash()), + accountHash, + storageTrie.getRootHash(), + Hash.ZERO, + MAX_RANGE); + storageRangeDataRequest.addResponse( + snapWorldDownloadState, worldStateProofProvider, slots, new ArrayDeque<>(proofs)); + storageRangeDataRequest.getChildRequests(snapWorldDownloadState, worldStateStorage, null); + verify(snapWorldDownloadState, never()).addAccountToHealingList(any(Bytes.class)); + } + + @Test + void markAccountOnInvalidStorageProof() { + final Hash accountHash = Hash.hash(accounts.get(0)); + final StateTrieAccountValue stateTrieAccountValue = + StateTrieAccountValue.readFrom(RLP.input(accountStateTrie.get(accountHash).orElseThrow())); + + final List proofs = + List.of( + worldStateStorage + .getAccountStorageTrieNode( + accountHash, Bytes.EMPTY, stateTrieAccountValue.getStorageRoot()) + .get()); + + final StorageRangeDataRequest storageRangeDataRequest = + SnapDataRequest.createStorageRangeDataRequest( + Hash.wrap(accountStateTrie.getRootHash()), + accountHash, + stateTrieAccountValue.getStorageRoot(), + Hash.ZERO, + MAX_RANGE); + storageRangeDataRequest.addResponse( + snapWorldDownloadState, worldStateProofProvider, new TreeMap<>(), new ArrayDeque<>(proofs)); + + verify(snapWorldDownloadState).addAccountToHealingList(any(Bytes.class)); + } + + @Test + void markAccountOnPartialStorageRange() { + // generate valid proof + final Hash accountHash = Hash.hash(accounts.get(0)); + final StateTrieAccountValue stateTrieAccountValue = + StateTrieAccountValue.readFrom(RLP.input(accountStateTrie.get(accountHash).orElseThrow())); + + final StoredMerklePatriciaTrie storageTrie = + new StoredMerklePatriciaTrie<>( + new StoredNodeFactory<>( + (location, hash) -> + worldStateStorage.getAccountStorageTrieNode(accountHash, location, hash), + Function.identity(), + Function.identity()), + stateTrieAccountValue.getStorageRoot()); + + final RangeStorageEntriesCollector collector = + RangeStorageEntriesCollector.createCollector( + Hash.ZERO, + MAX_RANGE, + 1, + Integer.MAX_VALUE); // limit to 1 in order to have a partial range + final TrieIterator visitor = RangeStorageEntriesCollector.createVisitor(collector); + final TreeMap slots = + (TreeMap) + storageTrie.entriesFrom( + root -> + RangeStorageEntriesCollector.collectEntries( + collector, visitor, root, Hash.ZERO)); + // generate the proof + final List proofs = + worldStateProofProvider.getStorageProofRelatedNodes( + Hash.wrap(storageTrie.getRootHash()), accountHash, Hash.ZERO); + proofs.addAll( + worldStateProofProvider.getStorageProofRelatedNodes( + Hash.wrap(storageTrie.getRootHash()), accountHash, slots.lastKey())); + + final StorageRangeDataRequest storageRangeDataRequest = + SnapDataRequest.createStorageRangeDataRequest( + Hash.wrap(accountStateTrie.getRootHash()), + accountHash, + storageTrie.getRootHash(), + Hash.ZERO, + MAX_RANGE); + storageRangeDataRequest.addResponse( + snapWorldDownloadState, worldStateProofProvider, slots, new ArrayDeque<>(proofs)); + verify(snapWorldDownloadState, never()).addAccountToHealingList(any(Bytes.class)); + + // should mark during the getchild request + storageRangeDataRequest.getChildRequests(snapWorldDownloadState, worldStateStorage, null); + verify(snapWorldDownloadState).addAccountToHealingList(any(Bytes.class)); + } + + @Test + void avoidMarkingAccountOnValidStorageTrieNodeDetection() { + final Hash accountHash = Hash.hash(accounts.get(0)); + final StateTrieAccountValue stateTrieAccountValue = + StateTrieAccountValue.readFrom(RLP.input(accountStateTrie.get(accountHash).orElseThrow())); + final StorageTrieNodeHealingRequest storageTrieNodeHealingRequest = + SnapDataRequest.createStorageTrieNodeDataRequest( + stateTrieAccountValue.getStorageRoot(), + accountHash, + Hash.wrap(accountStateTrie.getRootHash()), + Bytes.EMPTY); + storageTrieNodeHealingRequest.getExistingData(snapWorldDownloadState, worldStateStorage); + verify(snapWorldDownloadState, never()).addAccountToHealingList(any(Bytes.class)); + } +} diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/SnapWorldDownloadStateTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/SnapWorldDownloadStateTest.java index 14305363cd3..22c2b73765d 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/SnapWorldDownloadStateTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/SnapWorldDownloadStateTest.java @@ -325,13 +325,12 @@ public void shouldRestartHealWhenNewPivotBlock( @ParameterizedTest @ArgumentsSource(SnapWorldDownloadStateTestArguments.class) - public void shouldWaitingBlockchainWhenTooBehind( + public void shouldListeningBlockchainDuringHeal( final DataStorageFormat storageFormat, final boolean isFlatDbHealingEnabled) { setUp(storageFormat); - when(snapSyncState.isHealTrieInProgress()).thenReturn(true); + when(snapSyncState.isHealTrieInProgress()).thenReturn(false); downloadState.setPivotBlockSelector(dynamicPivotBlockManager); - when(dynamicPivotBlockManager.isBlockchainBehind()).thenReturn(true); downloadState.checkCompletion(header); @@ -339,7 +338,7 @@ public void shouldWaitingBlockchainWhenTooBehind( // should register only one time verify(blockchain, times(1)).observeBlockAdded(any()); - verify(snapSyncState, atLeastOnce()).setWaitingBlockchain(true); + verify(snapSyncState, atLeastOnce()).setHealTrieStatus(true); assertThat(future).isNotDone(); assertThat(worldStateStorage.getAccountStateTrieNode(Bytes.EMPTY, ROOT_NODE_HASH)).isEmpty(); @@ -374,7 +373,14 @@ public void shouldStopWaitingBlockchainWhenNewPivotBlockAvailable( .when(dynamicPivotBlockManager) .check(any()); - final BlockAddedObserver blockAddedListener = downloadState.getBlockAddedListener(); + final Block newBlock = + new Block( + new BlockHeaderTestFixture().number(500).buildHeader(), + new BlockBody(emptyList(), emptyList())); + + when(snapSyncState.getPivotBlockHeader()).thenReturn(Optional.of(newBlock.getHeader())); + + final BlockAddedObserver blockAddedListener = downloadState.createBlockchainObserver(); blockAddedListener.onBlockAdded( BlockAddedEvent.createForHeadAdvancement( new Block( @@ -383,7 +389,9 @@ public void shouldStopWaitingBlockchainWhenNewPivotBlockAvailable( Collections.emptyList(), Collections.emptyList())); + // reload heal verify(snapSyncState).setWaitingBlockchain(false); + verify(snapSyncState).setHealTrieStatus(false); } @ParameterizedTest @@ -395,6 +403,7 @@ public void shouldStopWaitingBlockchainWhenCloseToTheHead( when(snapSyncState.isHealTrieInProgress()).thenReturn(true); downloadState.setPivotBlockSelector(dynamicPivotBlockManager); + when(dynamicPivotBlockManager.isBlockchainBehind()).thenReturn(true); downloadState.checkCompletion(header); @@ -402,17 +411,21 @@ public void shouldStopWaitingBlockchainWhenCloseToTheHead( verify(snapSyncState).setWaitingBlockchain(true); when(snapSyncState.isWaitingBlockchain()).thenReturn(true); + final Block newBlock = + new Block( + new BlockHeaderTestFixture().number(500).buildHeader(), + new BlockBody(emptyList(), emptyList())); + when(dynamicPivotBlockManager.isBlockchainBehind()).thenReturn(false); - final BlockAddedObserver blockAddedListener = downloadState.getBlockAddedListener(); + when(snapSyncState.getPivotBlockHeader()).thenReturn(Optional.of(newBlock.getHeader())); + + final BlockAddedObserver blockAddedListener = downloadState.createBlockchainObserver(); blockAddedListener.onBlockAdded( BlockAddedEvent.createForHeadAdvancement( - new Block( - new BlockHeaderTestFixture().number(500).buildHeader(), - new BlockBody(emptyList(), emptyList())), - Collections.emptyList(), - Collections.emptyList())); + newBlock, Collections.emptyList(), Collections.emptyList())); verify(snapSyncState).setWaitingBlockchain(false); + verify(snapSyncState).setHealTrieStatus(false); } @ParameterizedTest diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/request/heal/AccountFlatDatabaseHealingRangeRequestTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/request/heal/AccountFlatDatabaseHealingRangeRequestTest.java index 0f07056296e..a95d4c34114 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/request/heal/AccountFlatDatabaseHealingRangeRequestTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/request/heal/AccountFlatDatabaseHealingRangeRequestTest.java @@ -67,7 +67,7 @@ public class AccountFlatDatabaseHealingRangeRequestTest { public void setup() { Mockito.when(downloadState.getMetricsManager()) .thenReturn(Mockito.mock(SnapsyncMetricsManager.class)); - Mockito.when(downloadState.getAccountsToBeRepaired()).thenReturn(new HashSet<>()); + Mockito.when(downloadState.getAccountsHealingList()).thenReturn(new HashSet<>()); } @Test @@ -120,7 +120,7 @@ public void shouldReturnChildRequests() { Assertions.assertThat(snapDataRequest.getStartKeyHash()).isGreaterThan(accounts.lastKey()); // Verify that we have storage healing request when the account need to be repaired - Mockito.when(downloadState.getAccountsToBeRepaired()) + Mockito.when(downloadState.getAccountsHealingList()) .thenReturn( new HashSet<>( accounts.keySet().stream() diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/request/heal/StorageTrieNodeHealingRequestTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/request/heal/StorageTrieNodeHealingRequestTest.java index 14e4d551f72..447b37d14ce 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/request/heal/StorageTrieNodeHealingRequestTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/request/heal/StorageTrieNodeHealingRequestTest.java @@ -57,7 +57,6 @@ class StorageTrieNodeHealingRequestTest { Address.fromHexString("0xdeadbeeb")); private WorldStateStorage worldStateStorage; - private Hash account0Hash; private Hash account0StorageRoot; @@ -81,6 +80,7 @@ public void setup(final DataStorageFormat storageFormat) { TrieGenerator.generateTrie( worldStateStorage, accounts.stream().map(Address::addressHash).collect(Collectors.toList())); + account0Hash = accounts.get(0).addressHash(); account0StorageRoot = trie.get(account0Hash) @@ -94,6 +94,7 @@ public void setup(final DataStorageFormat storageFormat) { @ArgumentsSource(StorageFormatArguments.class) void shouldDetectExistingData(final DataStorageFormat storageFormat) { setup(storageFormat); + final StorageTrieNodeHealingRequest request = new StorageTrieNodeHealingRequest( account0StorageRoot, account0Hash, Hash.EMPTY, Bytes.EMPTY);