diff --git a/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java b/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java index 21cb787f52b..2ff3c390651 100644 --- a/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java +++ b/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java @@ -1805,10 +1805,8 @@ public BesuControllerBuilder setupControllerBuilder() { if (DataStorageFormat.BONSAI.equals(getDataStorageConfiguration().getDataStorageFormat())) { final DiffBasedSubStorageConfiguration subStorageConfiguration = getDataStorageConfiguration().getDiffBasedSubStorageConfiguration(); - if (subStorageConfiguration.getLimitTrieLogsEnabled()) { - besuControllerBuilder.isParallelTxProcessingEnabled( - subStorageConfiguration.getUnstable().isParallelTxProcessingEnabled()); - } + besuControllerBuilder.isParallelTxProcessingEnabled( + subStorageConfiguration.getUnstable().isParallelTxProcessingEnabled()); } return besuControllerBuilder; } diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/MainnetBlockValidator.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/MainnetBlockValidator.java index c2925d651bd..0b715f66633 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/MainnetBlockValidator.java +++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/MainnetBlockValidator.java @@ -241,7 +241,7 @@ private void handleFailedBlockProcessing( protected BlockProcessingResult processBlock( final ProtocolContext context, final MutableWorldState worldState, final Block block) { - return blockProcessor.processBlock(context.getBlockchain(), worldState, block); + return blockProcessor.processBlock(context, context.getBlockchain(), worldState, block); } @Override diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/AbstractBlockProcessor.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/AbstractBlockProcessor.java index d6a483a955f..1060b18d074 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/AbstractBlockProcessor.java +++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/AbstractBlockProcessor.java @@ -22,6 +22,7 @@ import org.hyperledger.besu.datatypes.Wei; import org.hyperledger.besu.ethereum.BlockProcessingOutputs; import org.hyperledger.besu.ethereum.BlockProcessingResult; +import org.hyperledger.besu.ethereum.ProtocolContext; import org.hyperledger.besu.ethereum.chain.Blockchain; import org.hyperledger.besu.ethereum.core.BlockHeader; import org.hyperledger.besu.ethereum.core.MutableWorldState; @@ -29,6 +30,7 @@ import org.hyperledger.besu.ethereum.core.Transaction; import org.hyperledger.besu.ethereum.core.TransactionReceipt; import org.hyperledger.besu.ethereum.core.Withdrawal; +import org.hyperledger.besu.ethereum.mainnet.AbstractBlockProcessor.PreprocessingFunction.NoPreprocessing; import org.hyperledger.besu.ethereum.mainnet.requests.ProcessRequestContext; import org.hyperledger.besu.ethereum.mainnet.requests.RequestProcessorCoordinator; import org.hyperledger.besu.ethereum.privacy.storage.PrivateMetadataUpdater; @@ -94,6 +96,7 @@ protected AbstractBlockProcessor( @Override public BlockProcessingResult processBlock( + final ProtocolContext protocolContext, final Blockchain blockchain, final MutableWorldState worldState, final BlockHeader blockHeader, @@ -101,6 +104,28 @@ public BlockProcessingResult processBlock( final List ommers, final Optional> maybeWithdrawals, final PrivateMetadataUpdater privateMetadataUpdater) { + return processBlock( + protocolContext, + blockchain, + worldState, + blockHeader, + transactions, + ommers, + maybeWithdrawals, + privateMetadataUpdater, + new NoPreprocessing()); + } + + protected BlockProcessingResult processBlock( + final ProtocolContext protocolContext, + final Blockchain blockchain, + final MutableWorldState worldState, + final BlockHeader blockHeader, + final List transactions, + final List ommers, + final Optional> maybeWithdrawals, + final PrivateMetadataUpdater privateMetadataUpdater, + final PreprocessingFunction preprocessingBlockFunction) { final List receipts = new ArrayList<>(); long currentGasUsed = 0; long currentBlobGasUsed = 0; @@ -126,8 +151,8 @@ public BlockProcessingResult processBlock( .orElse(Wei.ZERO); final Optional preProcessingContext = - runBlockPreProcessing( - worldState, + preprocessingBlockFunction.run( + protocolContext, privateMetadataUpdater, blockHeader, transactions, @@ -218,7 +243,7 @@ public BlockProcessingResult processBlock( protocolSpec.getRequestProcessorCoordinator(); Optional> maybeRequests = Optional.empty(); if (requestProcessor.isPresent()) { - ProcessRequestContext context = + ProcessRequestContext processRequestContext = new ProcessRequestContext( blockHeader, worldState, @@ -227,7 +252,7 @@ public BlockProcessingResult processBlock( blockHashLookup, OperationTracer.NO_TRACING); - maybeRequests = Optional.of(requestProcessor.get().process(context)); + maybeRequests = Optional.of(requestProcessor.get().process(processRequestContext)); } if (!rewardCoinbase(worldState, blockHeader, ommers, skipZeroBlockRewards)) { @@ -256,17 +281,6 @@ public BlockProcessingResult processBlock( parallelizedTxFound ? Optional.of(nbParallelTx) : Optional.empty()); } - protected Optional runBlockPreProcessing( - final MutableWorldState worldState, - final PrivateMetadataUpdater privateMetadataUpdater, - final BlockHeader blockHeader, - final List transactions, - final Address miningBeneficiary, - final BlockHashOperation.BlockHashLookup blockHashLookup, - final Wei blobGasPrice) { - return Optional.empty(); - } - protected TransactionProcessingResult getTransactionProcessingResult( final Optional preProcessingContext, final MutableWorldState worldState, @@ -319,5 +333,30 @@ abstract boolean rewardCoinbase( final boolean skipZeroBlockRewards); public interface PreprocessingContext {} - ; + + public interface PreprocessingFunction { + Optional run( + final ProtocolContext protocolContext, + final PrivateMetadataUpdater privateMetadataUpdater, + final BlockHeader blockHeader, + final List transactions, + final Address miningBeneficiary, + final BlockHashOperation.BlockHashLookup blockHashLookup, + final Wei blobGasPrice); + + class NoPreprocessing implements PreprocessingFunction { + + @Override + public Optional run( + final ProtocolContext protocolContext, + final PrivateMetadataUpdater privateMetadataUpdater, + final BlockHeader blockHeader, + final List transactions, + final Address miningBeneficiary, + final BlockHashLookup blockHashLookup, + final Wei blobGasPrice) { + return Optional.empty(); + } + } + } } diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/BlockProcessor.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/BlockProcessor.java index d9f2e7874cc..04e90eb5811 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/BlockProcessor.java +++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/BlockProcessor.java @@ -16,6 +16,7 @@ import org.hyperledger.besu.datatypes.Wei; import org.hyperledger.besu.ethereum.BlockProcessingResult; +import org.hyperledger.besu.ethereum.ProtocolContext; import org.hyperledger.besu.ethereum.chain.Blockchain; import org.hyperledger.besu.ethereum.core.Block; import org.hyperledger.besu.ethereum.core.BlockHeader; @@ -68,14 +69,19 @@ default boolean isFailed() { /** * Processes the block. * + * @param protocolContext the current context of the protocol * @param blockchain the blockchain to append the block to * @param worldState the world state to apply changes to * @param block the block to process * @return the block processing result */ default BlockProcessingResult processBlock( - final Blockchain blockchain, final MutableWorldState worldState, final Block block) { + final ProtocolContext protocolContext, + final Blockchain blockchain, + final MutableWorldState worldState, + final Block block) { return processBlock( + protocolContext, blockchain, worldState, block.getHeader(), @@ -88,6 +94,7 @@ default BlockProcessingResult processBlock( /** * Processes the block. * + * @param protocolContext the current context of the protocol * @param blockchain the blockchain to append the block to * @param worldState the world state to apply changes to * @param blockHeader the block header for the block @@ -96,18 +103,27 @@ default BlockProcessingResult processBlock( * @return the block processing result */ default BlockProcessingResult processBlock( + final ProtocolContext protocolContext, final Blockchain blockchain, final MutableWorldState worldState, final BlockHeader blockHeader, final List transactions, final List ommers) { return processBlock( - blockchain, worldState, blockHeader, transactions, ommers, Optional.empty(), null); + protocolContext, + blockchain, + worldState, + blockHeader, + transactions, + ommers, + Optional.empty(), + null); } /** * Processes the block. * + * @param protocolContext the current context of the protocol * @param blockchain the blockchain to append the block to * @param worldState the world state to apply changes to * @param blockHeader the block header for the block @@ -118,6 +134,7 @@ default BlockProcessingResult processBlock( * @return the block processing result */ BlockProcessingResult processBlock( + ProtocolContext protocolContext, Blockchain blockchain, MutableWorldState worldState, BlockHeader blockHeader, @@ -129,6 +146,7 @@ BlockProcessingResult processBlock( /** * Processes the block when running Besu in GoQuorum-compatible mode * + * @param protocolContext the current context of the protocol * @param blockchain the blockchain to append the block to * @param worldState the world state to apply public transactions to * @param privateWorldState the private world state to apply private transaction to @@ -136,6 +154,7 @@ BlockProcessingResult processBlock( * @return the block processing result */ default BlockProcessingResult processBlock( + final ProtocolContext protocolContext, final Blockchain blockchain, final MutableWorldState worldState, final MutableWorldState privateWorldState, diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/MainnetProtocolSpecs.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/MainnetProtocolSpecs.java index 26c9e69c8f5..d836b25ca57 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/MainnetProtocolSpecs.java +++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/MainnetProtocolSpecs.java @@ -25,6 +25,7 @@ import org.hyperledger.besu.datatypes.Wei; import org.hyperledger.besu.ethereum.BlockProcessingResult; import org.hyperledger.besu.ethereum.MainnetBlockValidator; +import org.hyperledger.besu.ethereum.ProtocolContext; import org.hyperledger.besu.ethereum.chain.Blockchain; import org.hyperledger.besu.ethereum.core.BlockHeader; import org.hyperledger.besu.ethereum.core.MiningConfiguration; @@ -988,6 +989,7 @@ private record DaoBlockProcessor(BlockProcessor wrapped) implements BlockProcess @Override public BlockProcessingResult processBlock( + final ProtocolContext protocolContext, final Blockchain blockchain, final MutableWorldState worldState, final BlockHeader blockHeader, @@ -997,6 +999,7 @@ public BlockProcessingResult processBlock( final PrivateMetadataUpdater privateMetadataUpdater) { updateWorldStateForDao(worldState); return wrapped.processBlock( + protocolContext, blockchain, worldState, blockHeader, diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/PrivacyBlockProcessor.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/PrivacyBlockProcessor.java index abfdf3652e6..4bab7ee79f2 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/PrivacyBlockProcessor.java +++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/PrivacyBlockProcessor.java @@ -21,6 +21,7 @@ import org.hyperledger.besu.enclave.EnclaveClientException; import org.hyperledger.besu.enclave.types.ReceiveResponse; import org.hyperledger.besu.ethereum.BlockProcessingResult; +import org.hyperledger.besu.ethereum.ProtocolContext; import org.hyperledger.besu.ethereum.chain.Blockchain; import org.hyperledger.besu.ethereum.core.BlockHeader; import org.hyperledger.besu.ethereum.core.MutableWorldState; @@ -83,6 +84,7 @@ public void setPublicWorldStateArchive(final WorldStateArchive publicWorldStateA @Override public BlockProcessingResult processBlock( + final ProtocolContext protocolContext, final Blockchain blockchain, final MutableWorldState worldState, final BlockHeader blockHeader, @@ -102,6 +104,7 @@ public BlockProcessingResult processBlock( final BlockProcessingResult result = blockProcessor.processBlock( + protocolContext, blockchain, worldState, blockHeader, diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/parallelization/MainnetParallelBlockProcessor.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/parallelization/MainnetParallelBlockProcessor.java index d1f0d9c5127..a008af35bf1 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/parallelization/MainnetParallelBlockProcessor.java +++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/parallelization/MainnetParallelBlockProcessor.java @@ -16,9 +16,14 @@ import org.hyperledger.besu.datatypes.Address; import org.hyperledger.besu.datatypes.Wei; +import org.hyperledger.besu.ethereum.BlockProcessingResult; +import org.hyperledger.besu.ethereum.ProtocolContext; +import org.hyperledger.besu.ethereum.chain.Blockchain; import org.hyperledger.besu.ethereum.core.BlockHeader; import org.hyperledger.besu.ethereum.core.MutableWorldState; import org.hyperledger.besu.ethereum.core.Transaction; +import org.hyperledger.besu.ethereum.core.Withdrawal; +import org.hyperledger.besu.ethereum.mainnet.AbstractBlockProcessor.PreprocessingFunction.NoPreprocessing; import org.hyperledger.besu.ethereum.mainnet.BlockProcessor; import org.hyperledger.besu.ethereum.mainnet.MainnetBlockProcessor; import org.hyperledger.besu.ethereum.mainnet.MainnetTransactionProcessor; @@ -27,7 +32,7 @@ import org.hyperledger.besu.ethereum.mainnet.ProtocolSpecBuilder; import org.hyperledger.besu.ethereum.privacy.storage.PrivateMetadataUpdater; import org.hyperledger.besu.ethereum.processing.TransactionProcessingResult; -import org.hyperledger.besu.ethereum.trie.diffbased.common.worldview.DiffBasedWorldState; +import org.hyperledger.besu.ethereum.trie.diffbased.common.DiffBasedWorldStateProvider; import org.hyperledger.besu.evm.operation.BlockHashOperation; import org.hyperledger.besu.evm.worldstate.WorldUpdater; import org.hyperledger.besu.metrics.BesuMetricCategory; @@ -37,8 +42,13 @@ import java.util.List; import java.util.Optional; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + public class MainnetParallelBlockProcessor extends MainnetBlockProcessor { + private static final Logger LOG = LoggerFactory.getLogger(MainnetParallelBlockProcessor.class); + private final Optional metricsSystem; private final Optional confirmedParallelizedTransactionCounter; private final Optional conflictingButCachedTransactionCounter; @@ -78,34 +88,6 @@ public MainnetParallelBlockProcessor( "Counter for the number of conflicted transactions during block processing")); } - @Override - protected Optional runBlockPreProcessing( - final MutableWorldState worldState, - final PrivateMetadataUpdater privateMetadataUpdater, - final BlockHeader blockHeader, - final List transactions, - final Address miningBeneficiary, - final BlockHashOperation.BlockHashLookup blockHashLookup, - final Wei blobGasPrice) { - if ((worldState instanceof DiffBasedWorldState)) { - ParallelizedConcurrentTransactionProcessor parallelizedConcurrentTransactionProcessor = - new ParallelizedConcurrentTransactionProcessor(transactionProcessor); - // runAsyncBlock, if activated, facilitates the non-blocking parallel execution of - // transactions in the background through an optimistic strategy. - parallelizedConcurrentTransactionProcessor.runAsyncBlock( - worldState, - blockHeader, - transactions, - miningBeneficiary, - blockHashLookup, - blobGasPrice, - privateMetadataUpdater); - return Optional.of( - new ParallelizedPreProcessingContext(parallelizedConcurrentTransactionProcessor)); - } - return Optional.empty(); - } - @Override protected TransactionProcessingResult getTransactionProcessingResult( final Optional preProcessingContext, @@ -126,7 +108,7 @@ protected TransactionProcessingResult getTransactionProcessingResult( (ParallelizedPreProcessingContext) preProcessingContext.get(); transactionProcessingResult = parallelizedPreProcessingContext - .getParallelizedConcurrentTransactionProcessor() + .parallelizedConcurrentTransactionProcessor() .applyParallelizedTransactionResult( worldState, miningBeneficiary, @@ -154,21 +136,52 @@ protected TransactionProcessingResult getTransactionProcessingResult( } } - static class ParallelizedPreProcessingContext implements PreprocessingContext { - final ParallelizedConcurrentTransactionProcessor parallelizedConcurrentTransactionProcessor; - - public ParallelizedPreProcessingContext( - final ParallelizedConcurrentTransactionProcessor - parallelizedConcurrentTransactionProcessor) { - this.parallelizedConcurrentTransactionProcessor = parallelizedConcurrentTransactionProcessor; - } - - public ParallelizedConcurrentTransactionProcessor - getParallelizedConcurrentTransactionProcessor() { - return parallelizedConcurrentTransactionProcessor; + @Override + public BlockProcessingResult processBlock( + final ProtocolContext protocolContext, + final Blockchain blockchain, + final MutableWorldState worldState, + final BlockHeader blockHeader, + final List transactions, + final List ommers, + final Optional> maybeWithdrawals, + final PrivateMetadataUpdater privateMetadataUpdater) { + final BlockProcessingResult blockProcessingResult = + super.processBlock( + protocolContext, + blockchain, + worldState, + blockHeader, + transactions, + ommers, + maybeWithdrawals, + privateMetadataUpdater, + new ParallelTransactionPreprocessing()); + + if (blockProcessingResult.isFailed()) { + // Fallback to non-parallel processing if there is a block processing exception . + LOG.warn( + "Block processing failed. Falling back to non-parallel processing for block #{} ({})", + blockHeader.getNumber(), + blockHeader.getBlockHash()); + return super.processBlock( + protocolContext, + blockchain, + worldState, + blockHeader, + transactions, + ommers, + maybeWithdrawals, + privateMetadataUpdater, + new NoPreprocessing()); } + return blockProcessingResult; } + record ParallelizedPreProcessingContext( + ParallelizedConcurrentTransactionProcessor parallelizedConcurrentTransactionProcessor) + implements PreprocessingContext {} + public static class ParallelBlockProcessorBuilder implements ProtocolSpecBuilder.BlockProcessorBuilder { @@ -196,4 +209,35 @@ public BlockProcessor apply( metricsSystem); } } + + class ParallelTransactionPreprocessing implements PreprocessingFunction { + + @Override + public Optional run( + final ProtocolContext protocolContext, + final PrivateMetadataUpdater privateMetadataUpdater, + final BlockHeader blockHeader, + final List transactions, + final Address miningBeneficiary, + final BlockHashOperation.BlockHashLookup blockHashLookup, + final Wei blobGasPrice) { + if ((protocolContext.getWorldStateArchive() instanceof DiffBasedWorldStateProvider)) { + ParallelizedConcurrentTransactionProcessor parallelizedConcurrentTransactionProcessor = + new ParallelizedConcurrentTransactionProcessor(transactionProcessor); + // runAsyncBlock, if activated, facilitates the non-blocking parallel execution of + // transactions in the background through an optimistic strategy. + parallelizedConcurrentTransactionProcessor.runAsyncBlock( + protocolContext, + blockHeader, + transactions, + miningBeneficiary, + blockHashLookup, + blobGasPrice, + privateMetadataUpdater); + return Optional.of( + new ParallelizedPreProcessingContext(parallelizedConcurrentTransactionProcessor)); + } + return Optional.empty(); + } + } } diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/parallelization/ParallelizedConcurrentTransactionProcessor.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/parallelization/ParallelizedConcurrentTransactionProcessor.java index a62cc1fffa5..ec6fae0157a 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/parallelization/ParallelizedConcurrentTransactionProcessor.java +++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/parallelization/ParallelizedConcurrentTransactionProcessor.java @@ -16,6 +16,7 @@ import org.hyperledger.besu.datatypes.Address; import org.hyperledger.besu.datatypes.Wei; +import org.hyperledger.besu.ethereum.ProtocolContext; import org.hyperledger.besu.ethereum.core.BlockHeader; import org.hyperledger.besu.ethereum.core.MutableWorldState; import org.hyperledger.besu.ethereum.core.Transaction; @@ -23,7 +24,6 @@ import org.hyperledger.besu.ethereum.mainnet.TransactionValidationParams; import org.hyperledger.besu.ethereum.privacy.storage.PrivateMetadataUpdater; import org.hyperledger.besu.ethereum.processing.TransactionProcessingResult; -import org.hyperledger.besu.ethereum.trie.diffbased.bonsai.cache.NoopBonsaiCachedMerkleTrieLoader; import org.hyperledger.besu.ethereum.trie.diffbased.bonsai.worldview.BonsaiWorldState; import org.hyperledger.besu.ethereum.trie.diffbased.common.worldview.DiffBasedWorldState; import org.hyperledger.besu.ethereum.trie.diffbased.common.worldview.accumulator.DiffBasedWorldStateUpdateAccumulator; @@ -88,8 +88,7 @@ public ParallelizedConcurrentTransactionProcessor( * state, ensuring that the original world state passed as a parameter remains unmodified during * this process. * - * @param worldState Mutable world state intended for applying transaction results. This world - * state is not modified directly; instead, copies are made for transaction execution. + * @param protocolContext the current context of the protocol * @param blockHeader Header of the current block containing the transactions. * @param transactions List of transactions to be processed. * @param miningBeneficiary Address of the beneficiary to receive mining rewards. @@ -98,7 +97,7 @@ public ParallelizedConcurrentTransactionProcessor( * @param privateMetadataUpdater Updater for private transaction metadata. */ public void runAsyncBlock( - final MutableWorldState worldState, + final ProtocolContext protocolContext, final BlockHeader blockHeader, final List transactions, final Address miningBeneficiary, @@ -114,7 +113,7 @@ public void runAsyncBlock( CompletableFuture.runAsync( () -> runTransaction( - worldState, + protocolContext, blockHeader, transactionLocation, transaction, @@ -128,7 +127,7 @@ public void runAsyncBlock( @VisibleForTesting public void runTransaction( - final MutableWorldState worldState, + final ProtocolContext protocolContext, final BlockHeader blockHeader, final int transactionLocation, final Transaction transaction, @@ -136,65 +135,77 @@ public void runTransaction( final BlockHashOperation.BlockHashLookup blockHashLookup, final Wei blobGasPrice, final PrivateMetadataUpdater privateMetadataUpdater) { - try (final DiffBasedWorldState roundWorldState = - new BonsaiWorldState( - (BonsaiWorldState) worldState, new NoopBonsaiCachedMerkleTrieLoader())) { - roundWorldState.freeze(); // make the clone frozen - final ParallelizedTransactionContext.Builder contextBuilder = - new ParallelizedTransactionContext.Builder(); - final DiffBasedWorldStateUpdateAccumulator roundWorldStateUpdater = - (DiffBasedWorldStateUpdateAccumulator) roundWorldState.updater(); - final TransactionProcessingResult result = - transactionProcessor.processTransaction( - roundWorldStateUpdater, - blockHeader, - transaction, - miningBeneficiary, - new OperationTracer() { - @Override - public void traceBeforeRewardTransaction( - final WorldView worldView, - final org.hyperledger.besu.datatypes.Transaction tx, - final Wei miningReward) { - /* - * This part checks if the mining beneficiary's account was accessed before increasing its balance for rewards. - * Indeed, if the transaction has interacted with the address to read or modify it, - * it means that the value is necessary for the proper execution of the transaction and will therefore be considered in collision detection. - * If this is not the case, we can ignore this address during conflict detection. - */ - if (transactionCollisionDetector - .getAddressesTouchedByTransaction( - transaction, Optional.of(roundWorldStateUpdater)) - .contains(miningBeneficiary)) { - contextBuilder.isMiningBeneficiaryTouchedPreRewardByTransaction(true); - } - contextBuilder.miningBeneficiaryReward(miningReward); - } - }, - blockHashLookup, - true, - TransactionValidationParams.processingBlock(), - privateMetadataUpdater, - blobGasPrice); - // commit the accumulator in order to apply all the modifications - roundWorldState.getAccumulator().commit(); + final BlockHeader chainHeadHeader = protocolContext.getBlockchain().getChainHeadHeader(); + if (chainHeadHeader.getHash().equals(blockHeader.getParentHash())) { + try (BonsaiWorldState ws = + (BonsaiWorldState) + protocolContext + .getWorldStateArchive() + .getMutable(chainHeadHeader, false) + .orElse(null)) { + if (ws != null) { + ws.disableCacheMerkleTrieLoader(); + final ParallelizedTransactionContext.Builder contextBuilder = + new ParallelizedTransactionContext.Builder(); + final DiffBasedWorldStateUpdateAccumulator roundWorldStateUpdater = + (DiffBasedWorldStateUpdateAccumulator) ws.updater(); + final TransactionProcessingResult result = + transactionProcessor.processTransaction( + roundWorldStateUpdater, + blockHeader, + transaction, + miningBeneficiary, + new OperationTracer() { + @Override + public void traceBeforeRewardTransaction( + final WorldView worldView, + final org.hyperledger.besu.datatypes.Transaction tx, + final Wei miningReward) { + /* + * This part checks if the mining beneficiary's account was accessed before increasing its balance for rewards. + * Indeed, if the transaction has interacted with the address to read or modify it, + * it means that the value is necessary for the proper execution of the transaction and will therefore be considered in collision detection. + * If this is not the case, we can ignore this address during conflict detection. + */ + if (transactionCollisionDetector + .getAddressesTouchedByTransaction( + transaction, Optional.of(roundWorldStateUpdater)) + .contains(miningBeneficiary)) { + contextBuilder.isMiningBeneficiaryTouchedPreRewardByTransaction(true); + } + contextBuilder.miningBeneficiaryReward(miningReward); + } + }, + blockHashLookup, + true, + TransactionValidationParams.processingBlock(), + privateMetadataUpdater, + blobGasPrice); + + // commit the accumulator in order to apply all the modifications + ws.getAccumulator().commit(); - contextBuilder - .transactionAccumulator(roundWorldState.getAccumulator()) - .transactionProcessingResult(result); + contextBuilder + .transactionAccumulator(ws.getAccumulator()) + .transactionProcessingResult(result); - final ParallelizedTransactionContext parallelizedTransactionContext = contextBuilder.build(); - if (!parallelizedTransactionContext.isMiningBeneficiaryTouchedPreRewardByTransaction()) { - /* - * If the address of the mining beneficiary has been touched only for adding rewards, - * we remove it from the accumulator to avoid a false positive collision. - * The balance will be increased during the sequential processing. - */ - roundWorldStateUpdater.getAccountsToUpdate().remove(miningBeneficiary); + final ParallelizedTransactionContext parallelizedTransactionContext = + contextBuilder.build(); + if (!parallelizedTransactionContext.isMiningBeneficiaryTouchedPreRewardByTransaction()) { + /* + * If the address of the mining beneficiary has been touched only for adding rewards, + * we remove it from the accumulator to avoid a false positive collision. + * The balance will be increased during the sequential processing. + */ + roundWorldStateUpdater.getAccountsToUpdate().remove(miningBeneficiary); + } + parallelizedTransactionContextByLocation.put( + transactionLocation, parallelizedTransactionContext); + } + } catch (Exception ex) { + // no op as failing to get worldstate } - parallelizedTransactionContextByLocation.put( - transactionLocation, parallelizedTransactionContext); } } @@ -254,6 +265,7 @@ public Optional applyParallelizedTransactionResult( if (confirmedParallelizedTransactionCounter.isPresent()) { confirmedParallelizedTransactionCounter.get().inc(); transactionProcessingResult.setIsProcessedInParallel(Optional.of(Boolean.TRUE)); + transactionProcessingResult.accumulator = transactionAccumulator; } return Optional.of(transactionProcessingResult); } else { diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/parallelization/TransactionCollisionDetector.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/parallelization/TransactionCollisionDetector.java index 4120c0edb1e..dba0ec66047 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/parallelization/TransactionCollisionDetector.java +++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/parallelization/TransactionCollisionDetector.java @@ -15,49 +15,75 @@ package org.hyperledger.besu.ethereum.mainnet.parallelization; import org.hyperledger.besu.datatypes.Address; +import org.hyperledger.besu.datatypes.StorageSlotKey; import org.hyperledger.besu.ethereum.core.Transaction; +import org.hyperledger.besu.ethereum.trie.diffbased.common.DiffBasedAccount; +import org.hyperledger.besu.ethereum.trie.diffbased.common.DiffBasedValue; import org.hyperledger.besu.ethereum.trie.diffbased.common.worldview.accumulator.DiffBasedWorldStateUpdateAccumulator; +import org.hyperledger.besu.ethereum.trie.diffbased.common.worldview.accumulator.preload.StorageConsumingMap; import java.util.HashSet; -import java.util.Iterator; +import java.util.Objects; import java.util.Optional; import java.util.Set; +import org.apache.tuweni.units.bigints.UInt256; + public class TransactionCollisionDetector { /** - * Determines if a transaction has a collision based on the addresses it touches. A collision - * occurs if the transaction touches the mining beneficiary address or if there are common - * addresses touched by both the transaction and other transactions within the same block. + * Checks if there is a conflict between the current block's state and the given transaction. + * + *

This method detects conflicts between the transaction and the block's state by checking if + * the transaction modifies the same addresses and storage slots that are already modified by the + * block. A conflict occurs in two cases: 1. If the transaction touches an address that is also + * modified in the block, and the account details (excluding storage) are identical. In this case, + * it checks if there is an overlap in the storage slots affected by both the transaction and the + * block. 2. If the account details differ between the transaction and the block (excluding + * storage), it immediately detects a conflict. * - * @param transaction The transaction to check for collisions. - * @param miningBeneficiary The address of the mining beneficiary. - * @param parallelizedTransactionContext The context containing the accumulator for the + *

The method returns `true` if any such conflict is found, otherwise `false`. + * + * @param transaction The transaction to check for conflicts with the block's state. + * @param parallelizedTransactionContext The context for the parallelized execution of the * transaction. - * @param blockAccumulator The accumulator for the block. - * @return true if there is a collision; false otherwise. + * @param blockAccumulator The accumulator containing the state updates of the current block. + * @return true if there is a conflict between the transaction and the block's state, otherwise + * false. */ public boolean hasCollision( final Transaction transaction, final Address miningBeneficiary, final ParallelizedTransactionContext parallelizedTransactionContext, - final DiffBasedWorldStateUpdateAccumulator blockAccumulator) { + final DiffBasedWorldStateUpdateAccumulator blockAccumulator) { final Set

addressesTouchedByTransaction = getAddressesTouchedByTransaction( transaction, Optional.of(parallelizedTransactionContext.transactionAccumulator())); if (addressesTouchedByTransaction.contains(miningBeneficiary)) { return true; } - final Set
addressesTouchedByBlock = - getAddressesTouchedByBlock(Optional.of(blockAccumulator)); - final Iterator
it = addressesTouchedByTransaction.iterator(); - boolean commonAddressFound = false; - while (it.hasNext() && !commonAddressFound) { - if (addressesTouchedByBlock.contains(it.next())) { - commonAddressFound = true; + for (final Address next : addressesTouchedByTransaction) { + final Optional maybeAddressTouchedByBlock = + getAddressTouchedByBlock(next, Optional.of(blockAccumulator)); + if (maybeAddressTouchedByBlock.isPresent()) { + if (maybeAddressTouchedByBlock.get().areAccountDetailsEqualExcludingStorage()) { + final Set slotsTouchedByBlockAndByAddress = + getSlotsTouchedByBlockAndByAddress(Optional.of(blockAccumulator), next); + final Set slotsTouchedByTransactionAndByAddress = + getSlotsTouchedByTransactionAndByAddress( + Optional.of(parallelizedTransactionContext.transactionAccumulator()), next); + for (final StorageSlotKey touchedByTransactionAndByAddress : + slotsTouchedByTransactionAndByAddress) { + if (slotsTouchedByBlockAndByAddress.contains(touchedByTransactionAndByAddress)) { + return true; + } + } + } else { + return true; + } } } - return commonAddressFound; + return false; } /** @@ -81,34 +107,179 @@ public Set
getAddressesTouchedByTransaction( diffBasedWorldStateUpdateAccumulator -> { diffBasedWorldStateUpdateAccumulator .getAccountsToUpdate() - .forEach((address, diffBasedValue) -> addresses.add(address)); + .forEach( + (address, diffBasedValue) -> { + addresses.add(address); + }); addresses.addAll(diffBasedWorldStateUpdateAccumulator.getDeletedAccountAddresses()); }); + return addresses; } /** - * Retrieves the set of addresses that were touched by all transactions within a block. This - * method filters out addresses that were only read and not modified. + * Retrieves the set of storage slot keys that have been touched by the given transaction for the + * specified address, based on the provided world state update accumulator. * - * @param accumulator An optional accumulator containing state changes made by the block. - * @return A set of addresses that were modified by the block's transactions. + *

This method checks if the accumulator contains storage updates for the specified address. If + * such updates are found, it adds the touched storage slot keys to the returned set. The method + * does not distinguish between changes or unchanged slots; it simply collects all the storage + * slot keys that have been touched by the transaction for the given address. + * + * @param accumulator An {@link Optional} containing the world state update accumulator, which + * holds the updates for storage slots. + * @param address The address for which the touched storage slots are being retrieved. + * @return A set of storage slot keys that have been touched by the transaction for the given + * address. If no updates are found, or the address has no associated updates, an empty set is + * returned. */ - private Set

getAddressesTouchedByBlock( - final Optional> accumulator) { - HashSet
addresses = new HashSet<>(); + private Set getSlotsTouchedByTransactionAndByAddress( + final Optional> accumulator, final Address address) { + HashSet slots = new HashSet<>(); accumulator.ifPresent( diffBasedWorldStateUpdateAccumulator -> { - diffBasedWorldStateUpdateAccumulator - .getAccountsToUpdate() - .forEach( - (address, diffBasedValue) -> { - if (!diffBasedValue.isUnchanged()) { - addresses.add(address); - } - }); - addresses.addAll(diffBasedWorldStateUpdateAccumulator.getDeletedAccountAddresses()); + final StorageConsumingMap> map = + diffBasedWorldStateUpdateAccumulator.getStorageToUpdate().get(address); + if (map != null) { + map.forEach( + (storageSlotKey, slot) -> { + slots.add(storageSlotKey); + }); + } }); - return addresses; + return slots; + } + + /** + * Retrieves the update context for the given address from the block's world state update + * accumulator. + * + *

This method checks if the provided accumulator contains updates for the given address. If an + * update is found, it compares the prior and updated states of the account to determine if the + * key account details (excluding storage) are considered equal. It then returns an {@link + * AccountUpdateContext} containing the address and the result of that comparison. + * + *

If no update is found for the address or the accumulator is absent, the method returns an + * empty {@link Optional}. + * + * @param addressToFind The address for which the update context is being queried. + * @param maybeBlockAccumulator An {@link Optional} containing the block's world state update + * accumulator, which holds the updates for the accounts in the block. + * @return An {@link Optional} containing the {@link AccountUpdateContext} if the address is found + * in the block's updates, otherwise an empty {@link Optional}. + */ + private Optional getAddressTouchedByBlock( + final Address addressToFind, + final Optional> + maybeBlockAccumulator) { + if (maybeBlockAccumulator.isPresent()) { + final DiffBasedWorldStateUpdateAccumulator blockAccumulator = + maybeBlockAccumulator.get(); + final DiffBasedValue diffBasedValue = + blockAccumulator.getAccountsToUpdate().get(addressToFind); + if (diffBasedValue != null) { + return Optional.of( + new AccountUpdateContext( + addressToFind, + areAccountDetailsEqualExcludingStorage( + diffBasedValue.getPrior(), diffBasedValue.getUpdated()))); + } + } + return Optional.empty(); + } + + /** + * Retrieves the set of storage slot keys that have been updated in the block accumulator for the + * specified address. + * + *

This method checks if the accumulator contains a storage map for the provided address. If + * the address has associated storage updates, it iterates over the storage slots and add it to + * the list only if the corresponding storage value has been modified (i.e., is not unchanged). + * + * @param accumulator An Optional containing the world state block update accumulator, which holds + * the storage updates. + * @param address The address for which the storage slots are being queried. + * @return A set of storage slot keys that have been updated for the given address. If no updates + * are found, or the address has no associated updates, an empty set is returned. + */ + private Set getSlotsTouchedByBlockAndByAddress( + final Optional> accumulator, final Address address) { + HashSet slots = new HashSet<>(); + accumulator.ifPresent( + diffBasedWorldStateUpdateAccumulator -> { + final StorageConsumingMap> map = + diffBasedWorldStateUpdateAccumulator.getStorageToUpdate().get(address); + if (map != null) { + map.forEach( + (storageSlotKey, slot) -> { + if (!slot.isUnchanged()) { + slots.add(storageSlotKey); + } + }); + } + }); + return slots; + } + + /** + * Compares the state of two accounts to check if their key properties are identical, excluding + * any differences in their storage. + * + *

This method compares the following account properties: - Nonce - Balance - Code Hash + * + *

It returns true if these properties are equal for both accounts, and false otherwise. Note + * that this comparison does not include the account's storage. + * + * @param prior The first account to compare (could be null). + * @param next The second account to compare (could be null). + * @return true if the account state properties are equal excluding storage, false otherwise. + */ + private boolean areAccountDetailsEqualExcludingStorage( + final DiffBasedAccount prior, final DiffBasedAccount next) { + return (prior == null && next == null) + || (prior != null + && next != null + && prior.getNonce() == next.getNonce() + && prior.getBalance().equals(next.getBalance()) + && prior.getCodeHash().equals(next.getCodeHash())); + } + + /** + * Represents the context of an account update, including the account's address and whether the + * key details of the account (excluding storage) are considered equal. + * + *

This record holds two main pieces of information: - `address`: The address of the account + * being updated. - `areAccountDetailsEqualExcludingStorage`: A boolean value indicating whether + * the account details, excluding the storage (nonce, balance, and code hash), are considered + * equal when compared to a previous state. + * + *

This record is used to track changes to account states and determine if key properties are + * unchanged, which helps in detecting whether further action is needed for the account update. + */ + private record AccountUpdateContext( + Address address, boolean areAccountDetailsEqualExcludingStorage) { + + @Override + public boolean equals(final Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + AccountUpdateContext that = (AccountUpdateContext) o; + return address.equals(that.address); + } + + @Override + public int hashCode() { + return Objects.hashCode(address); + } + + @Override + public String toString() { + return "AccountUpdateContext{" + + "address=" + + address + + ", areAccountDetailsEqualExcludingStorage=" + + areAccountDetailsEqualExcludingStorage + + '}'; + } } } diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/processing/TransactionProcessingResult.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/processing/TransactionProcessingResult.java index a05ecf3d8fb..1701aa4ebe0 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/processing/TransactionProcessingResult.java +++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/processing/TransactionProcessingResult.java @@ -16,6 +16,7 @@ import org.hyperledger.besu.ethereum.mainnet.ValidationResult; import org.hyperledger.besu.ethereum.transaction.TransactionInvalidReason; +import org.hyperledger.besu.ethereum.trie.diffbased.common.worldview.accumulator.DiffBasedWorldStateUpdateAccumulator; import org.hyperledger.besu.evm.log.Log; import java.util.List; @@ -54,6 +55,8 @@ public enum Status { private final ValidationResult validationResult; private final Optional revertReason; + public DiffBasedWorldStateUpdateAccumulator accumulator; + public static TransactionProcessingResult invalid( final ValidationResult validationResult) { return new TransactionProcessingResult( diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/trie/diffbased/bonsai/worldview/BonsaiWorldState.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/trie/diffbased/bonsai/worldview/BonsaiWorldState.java index b62805c1fda..98adc2b4fec 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/trie/diffbased/bonsai/worldview/BonsaiWorldState.java +++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/trie/diffbased/bonsai/worldview/BonsaiWorldState.java @@ -28,6 +28,7 @@ import org.hyperledger.besu.ethereum.trie.diffbased.bonsai.BonsaiAccount; import org.hyperledger.besu.ethereum.trie.diffbased.bonsai.BonsaiWorldStateProvider; import org.hyperledger.besu.ethereum.trie.diffbased.bonsai.cache.BonsaiCachedMerkleTrieLoader; +import org.hyperledger.besu.ethereum.trie.diffbased.bonsai.cache.NoopBonsaiCachedMerkleTrieLoader; import org.hyperledger.besu.ethereum.trie.diffbased.bonsai.storage.BonsaiWorldStateKeyValueStorage; import org.hyperledger.besu.ethereum.trie.diffbased.bonsai.storage.BonsaiWorldStateLayerStorage; import org.hyperledger.besu.ethereum.trie.diffbased.common.DiffBasedValue; @@ -60,7 +61,7 @@ public class BonsaiWorldState extends DiffBasedWorldState { - protected final BonsaiCachedMerkleTrieLoader bonsaiCachedMerkleTrieLoader; + protected BonsaiCachedMerkleTrieLoader bonsaiCachedMerkleTrieLoader; public BonsaiWorldState( final BonsaiWorldStateProvider archive, @@ -76,18 +77,6 @@ public BonsaiWorldState( diffBasedWorldStateConfig); } - public BonsaiWorldState( - final BonsaiWorldState worldState, - final BonsaiCachedMerkleTrieLoader cachedMerkleTrieLoader) { - this( - new BonsaiWorldStateLayerStorage(worldState.getWorldStateStorage()), - cachedMerkleTrieLoader, - worldState.cachedWorldStorageManager, - worldState.trieLogManager, - worldState.accumulator.getEvmConfiguration(), - new DiffBasedWorldStateConfig(worldState.worldStateConfig)); - } - public BonsaiWorldState( final BonsaiWorldStateKeyValueStorage worldStateKeyValueStorage, final BonsaiCachedMerkleTrieLoader bonsaiCachedMerkleTrieLoader, @@ -454,6 +443,10 @@ public MutableWorldState freeze() { return this; } + public void disableCacheMerkleTrieLoader() { + this.bonsaiCachedMerkleTrieLoader = new NoopBonsaiCachedMerkleTrieLoader(); + } + private MerkleTrie createTrie(final NodeLoader nodeLoader, final Bytes32 rootHash) { if (worldStateConfig.isTrieDisabled()) { return new NoOpMerkleTrie<>(); diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/trie/diffbased/common/worldview/accumulator/DiffBasedWorldStateUpdateAccumulator.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/trie/diffbased/common/worldview/accumulator/DiffBasedWorldStateUpdateAccumulator.java index 37e9c9c5830..8cc4c67e00d 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/trie/diffbased/common/worldview/accumulator/DiffBasedWorldStateUpdateAccumulator.java +++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/trie/diffbased/common/worldview/accumulator/DiffBasedWorldStateUpdateAccumulator.java @@ -153,6 +153,7 @@ public void importStateChangesFromSource( }); }); storageToClear.addAll(source.storageToClear); + storageKeyHashLookup.putAll(source.storageKeyHashLookup); this.isAccumulatorStateChanged = true; } @@ -211,6 +212,7 @@ public void importPriorStateFromSource( uInt256DiffBasedValue.getPrior(), uInt256DiffBasedValue.getPrior())); }); }); + storageKeyHashLookup.putAll(source.storageKeyHashLookup); this.isAccumulatorStateChanged = true; } diff --git a/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/MainnetBlockValidatorTest.java b/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/MainnetBlockValidatorTest.java index 96e8cafe191..55674ef7a1d 100644 --- a/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/MainnetBlockValidatorTest.java +++ b/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/MainnetBlockValidatorTest.java @@ -101,12 +101,13 @@ public void setup() { when(blockBodyValidator.validateBody(any(), any(), any(), any(), any(), any())) .thenReturn(true); when(blockBodyValidator.validateBodyLight(any(), any(), any(), any())).thenReturn(true); - when(blockProcessor.processBlock(any(), any(), any())).thenReturn(successfulProcessingResult); + when(blockProcessor.processBlock(protocolContext, any(), any(), any())) + .thenReturn(successfulProcessingResult); when(blockProcessor.processBlock(any(), any(), any(), any())) .thenReturn(successfulProcessingResult); when(blockProcessor.processBlock(any(), any(), any(), any(), any())) .thenReturn(successfulProcessingResult); - when(blockProcessor.processBlock(any(), any(), any(), any(), any(), any(), any())) + when(blockProcessor.processBlock(any(), any(), any(), any(), any(), any(), any(), any())) .thenReturn(successfulProcessingResult); assertNoBadBlocks(); @@ -202,7 +203,8 @@ public void validateAndProcessBlock_whenParentWorldStateNotAvailable() { @Test public void validateAndProcessBlock_whenProcessBlockFails() { - when(blockProcessor.processBlock(eq(blockchain), any(MutableWorldState.class), eq(block))) + when(blockProcessor.processBlock( + eq(protocolContext), eq(blockchain), any(MutableWorldState.class), eq(block))) .thenReturn(BlockProcessingResult.FAILED); BlockProcessingResult result = @@ -240,7 +242,7 @@ public void validateAndProcessBlock_whenStorageExceptionThrownProcessingBlock( final String caseName, final Exception storageException) { doThrow(storageException) .when(blockProcessor) - .processBlock(eq(blockchain), any(MutableWorldState.class), eq(block)); + .processBlock(eq(protocolContext), eq(blockchain), any(MutableWorldState.class), eq(block)); BlockProcessingResult result = mainnetBlockValidator.validateAndProcessBlock( @@ -277,7 +279,8 @@ public void validateAndProcessBlock_whenProcessBlockYieldsExceptionalResult( final String caseName, final Exception cause) { final BlockProcessingResult exceptionalResult = new BlockProcessingResult(Optional.empty(), cause); - when(blockProcessor.processBlock(eq(blockchain), any(MutableWorldState.class), eq(block))) + when(blockProcessor.processBlock( + eq(protocolContext), eq(blockchain), any(MutableWorldState.class), eq(block))) .thenReturn(exceptionalResult); BlockProcessingResult result = @@ -293,7 +296,8 @@ public void validateAndProcessBlock_whenProcessBlockYieldsExceptionalResult( @Test public void validateAndProcessBlock_withShouldRecordBadBlockFalse() { - when(blockProcessor.processBlock(eq(blockchain), any(MutableWorldState.class), eq(block))) + when(blockProcessor.processBlock( + eq(protocolContext), eq(blockchain), any(MutableWorldState.class), eq(block))) .thenReturn(BlockProcessingResult.FAILED); BlockProcessingResult result = @@ -311,7 +315,8 @@ public void validateAndProcessBlock_withShouldRecordBadBlockFalse() { @Test public void validateAndProcessBlock_withShouldRecordBadBlockTrue() { - when(blockProcessor.processBlock(eq(blockchain), any(MutableWorldState.class), eq(block))) + when(blockProcessor.processBlock( + eq(protocolContext), eq(blockchain), any(MutableWorldState.class), eq(block))) .thenReturn(BlockProcessingResult.FAILED); BlockProcessingResult result = @@ -329,7 +334,8 @@ public void validateAndProcessBlock_withShouldRecordBadBlockTrue() { @Test public void validateAndProcessBlock_withShouldRecordBadBlockNotSet() { - when(blockProcessor.processBlock(eq(blockchain), any(MutableWorldState.class), eq(block))) + when(blockProcessor.processBlock( + eq(protocolContext), eq(blockchain), any(MutableWorldState.class), eq(block))) .thenReturn(BlockProcessingResult.FAILED); BlockProcessingResult result = diff --git a/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/mainnet/AbstractBlockProcessorIntegrationTest.java b/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/mainnet/AbstractBlockProcessorIntegrationTest.java index c9e43327d96..5b4c710746c 100644 --- a/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/mainnet/AbstractBlockProcessorIntegrationTest.java +++ b/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/mainnet/AbstractBlockProcessorIntegrationTest.java @@ -27,6 +27,7 @@ import org.hyperledger.besu.datatypes.TransactionType; import org.hyperledger.besu.datatypes.Wei; import org.hyperledger.besu.ethereum.BlockProcessingResult; +import org.hyperledger.besu.ethereum.ProtocolContext; import org.hyperledger.besu.ethereum.chain.DefaultBlockchain; import org.hyperledger.besu.ethereum.core.Block; import org.hyperledger.besu.ethereum.core.BlockBody; @@ -79,6 +80,7 @@ class AbstractBlockProcessorIntegrationTest { private static final KeyPair ACCOUNT_GENESIS_2_KEYPAIR = generateKeyPair("fc5141e75bf622179f8eedada7fab3e2e6b3e3da8eb9df4f46d84df22df7430e"); + private ProtocolContext protocolContext; private WorldStateArchive worldStateArchive; private DefaultBlockchain blockchain; private Address coinbase; @@ -94,6 +96,7 @@ public void setUp() { final BlockHeader blockHeader = new BlockHeaderTestFixture().number(0L).buildHeader(); coinbase = blockHeader.getCoinbase(); worldStateArchive = contextTestFixture.getStateArchive(); + protocolContext = contextTestFixture.getProtocolContext(); blockchain = (DefaultBlockchain) contextTestFixture.getBlockchain(); } @@ -227,7 +230,7 @@ private void processSimpleTransfers(final BlockProcessor blockProcessor) { transactionTransfer2); BlockProcessingResult blockProcessingResult = - blockProcessor.processBlock(blockchain, worldState, blockWithTransactions); + blockProcessor.processBlock(protocolContext, blockchain, worldState, blockWithTransactions); BonsaiAccount updatedSenderAccount1 = (BonsaiAccount) worldState.get(transactionTransfer1.getSender()); @@ -269,7 +272,7 @@ private void processConflictedSimpleTransfersSameSender(final BlockProcessor blo transferTransaction3); BlockProcessingResult blockProcessingResult = - blockProcessor.processBlock(blockchain, worldState, blockWithTransactions); + blockProcessor.processBlock(protocolContext, blockchain, worldState, blockWithTransactions); BonsaiAccount updatedSenderAccount = (BonsaiAccount) worldState.get(transferTransaction1.getSender()); @@ -322,7 +325,7 @@ private void processConflictedSimpleTransfersSameAddressReceiverAndSender( transferTransaction2); BlockProcessingResult blockProcessingResult = - blockProcessor.processBlock(blockchain, worldState, blockWithTransactions); + blockProcessor.processBlock(protocolContext, blockchain, worldState, blockWithTransactions); BonsaiAccount updatedSenderAccount1 = (BonsaiAccount) worldState.get(transferTransaction1.getSender()); @@ -380,7 +383,7 @@ private void processConflictedSimpleTransfersWithCoinbase(final BlockProcessor b transferTransaction2); BlockProcessingResult blockProcessingResult = - blockProcessor.processBlock(blockchain, worldState, blockWithTransactions); + blockProcessor.processBlock(protocolContext, blockchain, worldState, blockWithTransactions); BonsaiAccount updatedSenderAccount1 = (BonsaiAccount) worldState.get(transferTransaction1.getSender()); @@ -429,7 +432,7 @@ void processContractSlotUpdateThenReadTx(final BlockProcessor blockProcessor) { MutableWorldState worldState = worldStateArchive.getMutable(); BlockProcessingResult blockProcessingResult = - blockProcessor.processBlock(blockchain, worldState, blockWithTransactions); + blockProcessor.processBlock(protocolContext, blockchain, worldState, blockWithTransactions); assertTrue(blockProcessingResult.isSuccessful()); @@ -465,7 +468,7 @@ void processSlotReadThenUpdateTx(final BlockProcessor blockProcessor) { MutableWorldState worldState = worldStateArchive.getMutable(); BlockProcessingResult blockProcessingResult = - blockProcessor.processBlock(blockchain, worldState, blockWithTransactions); + blockProcessor.processBlock(protocolContext, blockchain, worldState, blockWithTransactions); assertTrue(blockProcessingResult.isSuccessful()); @@ -508,7 +511,7 @@ void processAccountReadThenUpdateTx(final BlockProcessor blockProcessor) { MutableWorldState worldState = worldStateArchive.getMutable(); BlockProcessingResult blockProcessingResult = - blockProcessor.processBlock(blockchain, worldState, blockWithTransactions); + blockProcessor.processBlock(protocolContext, blockchain, worldState, blockWithTransactions); assertTrue(blockProcessingResult.isSuccessful()); @@ -554,7 +557,7 @@ void processAccountUpdateThenReadTx(final BlockProcessor blockProcessor) { MutableWorldState worldState = worldStateArchive.getMutable(); BlockProcessingResult blockProcessingResult = - blockProcessor.processBlock(blockchain, worldState, blockWithTransactions); + blockProcessor.processBlock(protocolContext, blockchain, worldState, blockWithTransactions); assertTrue(blockProcessingResult.isSuccessful()); // Verify the state @@ -599,7 +602,7 @@ void processAccountReadThenUpdateTxWithTwoAccounts(final BlockProcessor blockPro MutableWorldState worldState = worldStateArchive.getMutable(); BlockProcessingResult blockProcessingResult = - blockProcessor.processBlock(blockchain, worldState, blockWithTransactions); + blockProcessor.processBlock(protocolContext, blockchain, worldState, blockWithTransactions); assertTrue(blockProcessingResult.isSuccessful()); @@ -645,7 +648,7 @@ void processAccountUpdateThenReadTxWithTwoAccounts(final BlockProcessor blockPro MutableWorldState worldState = worldStateArchive.getMutable(); BlockProcessingResult blockProcessingResult = - blockProcessor.processBlock(blockchain, worldState, blockWithTransactions); + blockProcessor.processBlock(protocolContext, blockchain, worldState, blockWithTransactions); assertTrue(blockProcessingResult.isSuccessful()); diff --git a/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/mainnet/AbstractBlockProcessorTest.java b/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/mainnet/AbstractBlockProcessorTest.java index 33347929cf0..40a5bb537d3 100644 --- a/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/mainnet/AbstractBlockProcessorTest.java +++ b/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/mainnet/AbstractBlockProcessorTest.java @@ -27,6 +27,7 @@ import org.hyperledger.besu.datatypes.GWei; import org.hyperledger.besu.datatypes.Hash; import org.hyperledger.besu.datatypes.Wei; +import org.hyperledger.besu.ethereum.ProtocolContext; import org.hyperledger.besu.ethereum.chain.Blockchain; import org.hyperledger.besu.ethereum.core.BlockHeader; import org.hyperledger.besu.ethereum.core.BlockHeaderTestFixture; @@ -49,6 +50,7 @@ @ExtendWith(MockitoExtension.class) abstract class AbstractBlockProcessorTest { + @Mock private ProtocolContext protocolContext; @Mock private MainnetTransactionProcessor transactionProcessor; @Mock private AbstractBlockProcessor.TransactionReceiptFactory transactionReceiptFactory; @Mock private ProtocolSchedule protocolSchedule; @@ -84,7 +86,14 @@ void baseSetup() { void withProcessorAndEmptyWithdrawals_WithdrawalsAreNotProcessed() { when(protocolSpec.getWithdrawalsProcessor()).thenReturn(Optional.empty()); blockProcessor.processBlock( - blockchain, worldState, emptyBlockHeader, emptyList(), emptyList(), Optional.empty(), null); + protocolContext, + blockchain, + worldState, + emptyBlockHeader, + emptyList(), + emptyList(), + Optional.empty(), + null); verify(withdrawalsProcessor, never()).processWithdrawals(any(), any()); } @@ -92,7 +101,14 @@ void withProcessorAndEmptyWithdrawals_WithdrawalsAreNotProcessed() { void withNoProcessorAndEmptyWithdrawals_WithdrawalsAreNotProcessed() { when(protocolSpec.getWithdrawalsProcessor()).thenReturn(Optional.empty()); blockProcessor.processBlock( - blockchain, worldState, emptyBlockHeader, emptyList(), emptyList(), Optional.empty(), null); + protocolContext, + blockchain, + worldState, + emptyBlockHeader, + emptyList(), + emptyList(), + Optional.empty(), + null); verify(withdrawalsProcessor, never()).processWithdrawals(any(), any()); } @@ -102,6 +118,7 @@ void withProcessorAndWithdrawals_WithdrawalsAreProcessed() { final List withdrawals = List.of(new Withdrawal(UInt64.ONE, UInt64.ONE, Address.fromHexString("0x1"), GWei.ONE)); blockProcessor.processBlock( + protocolContext, blockchain, worldState, emptyBlockHeader, @@ -119,6 +136,7 @@ void withNoProcessorAndWithdrawals_WithdrawalsAreNotProcessed() { final List withdrawals = List.of(new Withdrawal(UInt64.ONE, UInt64.ONE, Address.fromHexString("0x1"), GWei.ONE)); blockProcessor.processBlock( + protocolContext, blockchain, worldState, emptyBlockHeader, diff --git a/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/mainnet/MainnetBlockProcessorTest.java b/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/mainnet/MainnetBlockProcessorTest.java index 8baf4d8d037..4082f5195b0 100644 --- a/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/mainnet/MainnetBlockProcessorTest.java +++ b/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/mainnet/MainnetBlockProcessorTest.java @@ -23,6 +23,7 @@ import org.hyperledger.besu.datatypes.Hash; import org.hyperledger.besu.datatypes.Wei; +import org.hyperledger.besu.ethereum.ProtocolContext; import org.hyperledger.besu.ethereum.chain.Blockchain; import org.hyperledger.besu.ethereum.core.BlockHeader; import org.hyperledger.besu.ethereum.core.BlockHeaderTestFixture; @@ -45,6 +46,7 @@ public class MainnetBlockProcessorTest extends AbstractBlockProcessorTest { mock(AbstractBlockProcessor.TransactionReceiptFactory.class); private final ProtocolSchedule protocolSchedule = mock(ProtocolSchedule.class); private final ProtocolSpec protocolSpec = mock(ProtocolSpec.class); + private final ProtocolContext protocolContext = mock(ProtocolContext.class); @BeforeEach public void setup() { @@ -72,7 +74,8 @@ public void noAccountCreatedWhenBlockRewardIsZeroAndSkipped() { .transactionsRoot(Hash.EMPTY_LIST_HASH) .ommersHash(Hash.EMPTY_LIST_HASH) .buildHeader(); - blockProcessor.processBlock(blockchain, worldState, emptyBlockHeader, emptyList(), emptyList()); + blockProcessor.processBlock( + protocolContext, blockchain, worldState, emptyBlockHeader, emptyList(), emptyList()); // An empty block with 0 reward should not change the world state assertThat(worldState.rootHash()).isEqualTo(initialHash); @@ -101,7 +104,8 @@ public void accountCreatedWhenBlockRewardIsZeroAndNotSkipped() { "0xa6b5d50f7b3c39b969c2fe8fed091939c674fef49b4826309cb6994361e39b71")) .ommersHash(Hash.EMPTY_LIST_HASH) .buildHeader(); - blockProcessor.processBlock(blockchain, worldState, emptyBlockHeader, emptyList(), emptyList()); + blockProcessor.processBlock( + protocolContext, blockchain, worldState, emptyBlockHeader, emptyList(), emptyList()); // An empty block with 0 reward should change the world state prior to EIP158 assertThat(worldState.rootHash()).isNotEqualTo(initialHash); diff --git a/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/mainnet/PrivacyBlockProcessorTest.java b/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/mainnet/PrivacyBlockProcessorTest.java index 2749b37fcdc..874cb18d214 100644 --- a/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/mainnet/PrivacyBlockProcessorTest.java +++ b/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/mainnet/PrivacyBlockProcessorTest.java @@ -27,6 +27,7 @@ import org.hyperledger.besu.datatypes.Hash; import org.hyperledger.besu.datatypes.Wei; import org.hyperledger.besu.enclave.Enclave; +import org.hyperledger.besu.ethereum.ProtocolContext; import org.hyperledger.besu.ethereum.chain.Blockchain; import org.hyperledger.besu.ethereum.chain.TransactionLocation; import org.hyperledger.besu.ethereum.core.Block; @@ -65,6 +66,7 @@ class PrivacyBlockProcessorTest { private AbstractBlockProcessor blockProcessor; private WorldStateArchive privateWorldStateArchive; private Enclave enclave; + private ProtocolContext protocolContext; private ProtocolSchedule protocolSchedule; private WorldStateArchive publicWorldStateArchive; @@ -74,6 +76,7 @@ public void setUp() { privateStateStorage = new PrivateStateKeyValueStorage(new InMemoryKeyValueStorage()); privateWorldStateArchive = mock(WorldStateArchive.class); enclave = mock(Enclave.class); + protocolContext = mock(ProtocolContext.class); protocolSchedule = mock(ProtocolSchedule.class); this.privacyBlockProcessor = new PrivacyBlockProcessor( @@ -100,16 +103,17 @@ void mustCopyPreviousPrivacyGroupBlockHeadMap() { final Block secondBlock = blockDataGenerator.block( BlockDataGenerator.BlockOptions.create().setParentHash(firstBlock.getHash())); - privacyBlockProcessor.processBlock(blockchain, mutableWorldState, firstBlock); + privacyBlockProcessor.processBlock(protocolContext, blockchain, mutableWorldState, firstBlock); privateStateStorage .updater() .putPrivacyGroupHeadBlockMap(firstBlock.getHash(), expected) .commit(); - privacyBlockProcessor.processBlock(blockchain, mutableWorldState, secondBlock); + privacyBlockProcessor.processBlock(protocolContext, blockchain, mutableWorldState, secondBlock); assertThat(privateStateStorage.getPrivacyGroupHeadBlockMap(secondBlock.getHash())) .contains(expected); verify(blockProcessor) .processBlock( + eq(protocolContext), eq(blockchain), eq(mutableWorldState), eq(firstBlock.getHeader()), @@ -119,6 +123,7 @@ void mustCopyPreviousPrivacyGroupBlockHeadMap() { any()); verify(blockProcessor) .processBlock( + eq(protocolContext), eq(blockchain), eq(mutableWorldState), eq(secondBlock.getHeader()), @@ -172,9 +177,10 @@ void mustPerformRehydration() { firstBlock.getHash(), VALID_BASE64_ENCLAVE_KEY, PrivateBlockMetadata.empty()) .commit(); - privacyBlockProcessor.processBlock(blockchain, mutableWorldState, secondBlock); + privacyBlockProcessor.processBlock(protocolContext, blockchain, mutableWorldState, secondBlock); verify(blockProcessor) .processBlock( + eq(protocolContext), eq(blockchain), eq(mutableWorldState), eq(secondBlock.getHeader()), diff --git a/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/mainnet/parallelization/TransactionCollisionDetectorTest.java b/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/mainnet/parallelization/TransactionCollisionDetectorTest.java index 0cd1bad3c57..077dd8e3675 100644 --- a/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/mainnet/parallelization/TransactionCollisionDetectorTest.java +++ b/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/mainnet/parallelization/TransactionCollisionDetectorTest.java @@ -19,17 +19,21 @@ import org.hyperledger.besu.datatypes.Address; import org.hyperledger.besu.datatypes.Hash; +import org.hyperledger.besu.datatypes.StorageSlotKey; import org.hyperledger.besu.datatypes.Wei; import org.hyperledger.besu.ethereum.core.Transaction; import org.hyperledger.besu.ethereum.trie.diffbased.bonsai.BonsaiAccount; import org.hyperledger.besu.ethereum.trie.diffbased.bonsai.worldview.BonsaiWorldState; import org.hyperledger.besu.ethereum.trie.diffbased.bonsai.worldview.BonsaiWorldStateUpdateAccumulator; import org.hyperledger.besu.ethereum.trie.diffbased.common.DiffBasedValue; +import org.hyperledger.besu.ethereum.trie.diffbased.common.worldview.accumulator.preload.StorageConsumingMap; import org.hyperledger.besu.evm.internal.EvmConfiguration; import java.math.BigInteger; +import java.util.concurrent.ConcurrentHashMap; import org.apache.tuweni.bytes.Bytes; +import org.apache.tuweni.units.bigints.UInt256; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -168,16 +172,22 @@ void testCollisionWithModifiedCode() { } @Test - void testCollisionWithModifiedStorageRoot() { + void testCollisionWithModifiedStorageRootAndSameSlot() { final Address address = Address.fromHexString("0x1"); final BonsaiAccount priorAccountValue = createAccount(address); final BonsaiAccount nextAccountValue = new BonsaiAccount(priorAccountValue, worldState, true); nextAccountValue.setStorageRoot(Hash.EMPTY); - - // Simulate that the address was already modified in the block + final StorageSlotKey updateStorageSlotKey = new StorageSlotKey(UInt256.ONE); + // Simulate that the address slot was already modified in the block bonsaiUpdater .getAccountsToUpdate() .put(address, new DiffBasedValue<>(priorAccountValue, nextAccountValue)); + bonsaiUpdater + .getStorageToUpdate() + .computeIfAbsent( + address, + __ -> new StorageConsumingMap<>(address, new ConcurrentHashMap<>(), (___, ____) -> {})) + .put(updateStorageSlotKey, new DiffBasedValue<>(UInt256.ONE, UInt256.ZERO)); final Transaction transaction = createTransaction(address, address); @@ -185,6 +195,12 @@ void testCollisionWithModifiedStorageRoot() { trxUpdater .getAccountsToUpdate() .put(address, new DiffBasedValue<>(priorAccountValue, priorAccountValue)); + trxUpdater + .getStorageToUpdate() + .computeIfAbsent( + address, + __ -> new StorageConsumingMap<>(address, new ConcurrentHashMap<>(), (___, ____) -> {})) + .put(updateStorageSlotKey, new DiffBasedValue<>(UInt256.ONE, UInt256.ONE)); boolean hasCollision = collisionDetector.hasCollision( @@ -196,6 +212,48 @@ void testCollisionWithModifiedStorageRoot() { assertTrue(hasCollision, "Expected a collision with the modified address"); } + @Test + void testCollisionWithModifiedStorageRootNotSameSlot() { + final Address address = Address.fromHexString("0x1"); + final BonsaiAccount priorAccountValue = createAccount(address); + final BonsaiAccount nextAccountValue = new BonsaiAccount(priorAccountValue, worldState, true); + nextAccountValue.setStorageRoot(Hash.EMPTY); + // Simulate that the address slot was already modified in the block + bonsaiUpdater + .getAccountsToUpdate() + .put(address, new DiffBasedValue<>(priorAccountValue, nextAccountValue)); + bonsaiUpdater + .getStorageToUpdate() + .computeIfAbsent( + address, + __ -> new StorageConsumingMap<>(address, new ConcurrentHashMap<>(), (___, ____) -> {})) + .put(new StorageSlotKey(UInt256.ZERO), new DiffBasedValue<>(UInt256.ONE, UInt256.ZERO)); + + final Transaction transaction = createTransaction(address, address); + + // Simulate that the address is read in the next transaction + trxUpdater + .getAccountsToUpdate() + .put(address, new DiffBasedValue<>(priorAccountValue, priorAccountValue)); + trxUpdater + .getStorageToUpdate() + .computeIfAbsent( + address, + __ -> new StorageConsumingMap<>(address, new ConcurrentHashMap<>(), (___, ____) -> {})) + .put(new StorageSlotKey(UInt256.ONE), new DiffBasedValue<>(UInt256.ONE, UInt256.ONE)); + + boolean hasCollision = + collisionDetector.hasCollision( + transaction, + Address.ZERO, + new ParallelizedTransactionContext(trxUpdater, null, false, Wei.ZERO), + bonsaiUpdater); + + assertFalse( + hasCollision, + "Expected no collision when storage roots are modified but different slots are updated."); + } + @Test void testCollisionWithMiningBeneficiaryAddress() { final Address miningBeneficiary = Address.ZERO; @@ -244,7 +302,7 @@ void testCollisionWithDeletedAddress() { final BonsaiAccount accountValue = createAccount(address); // Simulate that the address was deleted in the block - bonsaiUpdater.getDeletedAccountAddresses().add(address); + bonsaiUpdater.getAccountsToUpdate().put(address, new DiffBasedValue<>(accountValue, null)); final Transaction transaction = createTransaction(address, address); diff --git a/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/trie/diffbased/bonsai/AbstractIsolationTests.java b/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/trie/diffbased/bonsai/AbstractIsolationTests.java index 275adc9ff43..ca43e0ee03e 100644 --- a/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/trie/diffbased/bonsai/AbstractIsolationTests.java +++ b/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/trie/diffbased/bonsai/AbstractIsolationTests.java @@ -343,7 +343,7 @@ protected BlockProcessingResult executeBlock(final MutableWorldState ws, final B protocolSchedule .getByBlockHeader(blockHeader(0)) .getBlockProcessor() - .processBlock(blockchain, ws, block); + .processBlock(protocolContext, blockchain, ws, block); blockchain.appendBlock(block, res.getReceipts()); return res; }