From 516559fadcd8657bf5eefa17814e228a216dcd79 Mon Sep 17 00:00:00 2001 From: Fabio Di Fabio Date: Thu, 24 Oct 2024 10:36:21 +0200 Subject: [PATCH] Cleanup: Synchronizer is always present in protocol context (#7791) Signed-off-by: Fabio Di Fabio --- .../besu/controller/BesuControllerBuilder.java | 2 +- .../blockcreation/MergeCoordinatorTest.java | 2 ++ .../besu/ethereum/MainnetBlockValidator.java | 11 +---------- .../besu/ethereum/ProtocolContext.java | 8 +++----- .../besu/ethereum/core/Synchronizer.java | 17 +++++++++++++++++ .../besu/ethereum/core/BlockchainSetupUtil.java | 1 + .../besu/ethereum/core}/DummySynchronizer.java | 13 +++++++++++-- .../ethereum/MainnetBlockValidatorTest.java | 2 ++ .../ethereum/eth/manager/snap/SnapServer.java | 14 +++----------- .../ethereum/eth/sync/DefaultSynchronizer.java | 2 ++ ethereum/retesteth/build.gradle | 1 + .../ethereum/retesteth/RetestethService.java | 1 + 12 files changed, 45 insertions(+), 29 deletions(-) rename ethereum/{retesteth/src/main/java/org/hyperledger/besu/ethereum/retesteth => core/src/test-support/java/org/hyperledger/besu/ethereum/core}/DummySynchronizer.java (89%) diff --git a/besu/src/main/java/org/hyperledger/besu/controller/BesuControllerBuilder.java b/besu/src/main/java/org/hyperledger/besu/controller/BesuControllerBuilder.java index bded2a38ac..91ef6f7cfa 100644 --- a/besu/src/main/java/org/hyperledger/besu/controller/BesuControllerBuilder.java +++ b/besu/src/main/java/org/hyperledger/besu/controller/BesuControllerBuilder.java @@ -717,7 +717,7 @@ public abstract class BesuControllerBuilder implements MiningParameterOverrides ethPeers.snapServerPeersNeeded(false); } - protocolContext.setSynchronizer(Optional.of(synchronizer)); + protocolContext.setSynchronizer(synchronizer); final Optional maybeSnapProtocolManager = createSnapProtocolManager( diff --git a/consensus/merge/src/test/java/org/hyperledger/besu/consensus/merge/blockcreation/MergeCoordinatorTest.java b/consensus/merge/src/test/java/org/hyperledger/besu/consensus/merge/blockcreation/MergeCoordinatorTest.java index 09c1b28af4..4efcdc1e0f 100644 --- a/consensus/merge/src/test/java/org/hyperledger/besu/consensus/merge/blockcreation/MergeCoordinatorTest.java +++ b/consensus/merge/src/test/java/org/hyperledger/besu/consensus/merge/blockcreation/MergeCoordinatorTest.java @@ -59,6 +59,7 @@ import org.hyperledger.besu.ethereum.core.ImmutableMiningParameters; import org.hyperledger.besu.ethereum.core.ImmutableMiningParameters.MutableInitValues; import org.hyperledger.besu.ethereum.core.ImmutableMiningParameters.Unstable; import org.hyperledger.besu.ethereum.core.MiningParameters; +import org.hyperledger.besu.ethereum.core.Synchronizer; import org.hyperledger.besu.ethereum.core.TransactionTestFixture; import org.hyperledger.besu.ethereum.eth.manager.EthContext; import org.hyperledger.besu.ethereum.eth.manager.EthScheduler; @@ -189,6 +190,7 @@ public class MergeCoordinatorTest implements MergeGenesisConfigHelper { protocolContext = new ProtocolContext(blockchain, worldStateArchive, mergeContext, badBlockManager); + protocolContext.setSynchronizer(mock(Synchronizer.class)); var mutable = worldStateArchive.getMutable(); genesisState.writeStateTo(mutable); mutable.persist(null); 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 0c56a419e3..9cd014f25a 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 @@ -181,16 +181,7 @@ public class MainnetBlockValidator implements BlockValidator { Optional.of(new BlockProcessingOutputs(worldState, receipts, maybeRequests))); } } catch (MerkleTrieException ex) { - context - .getSynchronizer() - .ifPresentOrElse( - synchronizer -> synchronizer.healWorldState(ex.getMaybeAddress(), ex.getLocation()), - () -> - handleFailedBlockProcessing( - block, - new BlockProcessingResult(Optional.empty(), ex), - // Do not record bad black due to missing data - false)); + context.getSynchronizer().healWorldState(ex.getMaybeAddress(), ex.getLocation()); return new BlockProcessingResult(Optional.empty(), ex); } catch (StorageException ex) { var retval = new BlockProcessingResult(Optional.empty(), ex); diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/ProtocolContext.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/ProtocolContext.java index e5ec5ae092..4ca40bc0a9 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/ProtocolContext.java +++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/ProtocolContext.java @@ -32,8 +32,7 @@ public class ProtocolContext { private final WorldStateArchive worldStateArchive; private final BadBlockManager badBlockManager; private final ConsensusContext consensusContext; - - private Optional synchronizer; + private Synchronizer synchronizer; /** * Constructs a new ProtocolContext with the given blockchain, world state archive, consensus @@ -52,7 +51,6 @@ public class ProtocolContext { this.blockchain = blockchain; this.worldStateArchive = worldStateArchive; this.consensusContext = consensusContext; - this.synchronizer = Optional.empty(); this.badBlockManager = badBlockManager; } @@ -85,7 +83,7 @@ public class ProtocolContext { * * @return the synchronizer of the protocol context */ - public Optional getSynchronizer() { + public Synchronizer getSynchronizer() { return synchronizer; } @@ -94,7 +92,7 @@ public class ProtocolContext { * * @param synchronizer the synchronizer to set */ - public void setSynchronizer(final Optional synchronizer) { + public void setSynchronizer(final Synchronizer synchronizer) { this.synchronizer = synchronizer; } diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/Synchronizer.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/Synchronizer.java index 7d60c4d357..4a5d3c7ca1 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/Synchronizer.java +++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/Synchronizer.java @@ -17,6 +17,7 @@ package org.hyperledger.besu.ethereum.core; import org.hyperledger.besu.datatypes.Address; import org.hyperledger.besu.plugin.data.SyncStatus; import org.hyperledger.besu.plugin.services.BesuEvents; +import org.hyperledger.besu.plugin.services.BesuEvents.InitialSyncCompletionListener; import java.util.Optional; import java.util.concurrent.CompletableFuture; @@ -82,6 +83,22 @@ public interface Synchronizer { */ boolean unsubscribeInSync(final long listenerId); + /** + * Add a listener that will be notified when this node initial sync status changes. + * + * @param listener The callback to invoke when the initial sync status changes + * @return A subscription id that can be used to unsubscribe from these events + */ + long subscribeInitialSync(final InitialSyncCompletionListener listener); + + /** + * Unsubscribe from initial sync events. + * + * @param listenerId The id returned when subscribing + * @return {@code true} if a subscription was cancelled + */ + boolean unsubscribeInitialSync(final long listenerId); + @FunctionalInterface interface InSyncListener { void onInSyncStatusChange(boolean newSyncStatus); diff --git a/ethereum/core/src/test-support/java/org/hyperledger/besu/ethereum/core/BlockchainSetupUtil.java b/ethereum/core/src/test-support/java/org/hyperledger/besu/ethereum/core/BlockchainSetupUtil.java index 62d670c07a..7be6ad12c4 100644 --- a/ethereum/core/src/test-support/java/org/hyperledger/besu/ethereum/core/BlockchainSetupUtil.java +++ b/ethereum/core/src/test-support/java/org/hyperledger/besu/ethereum/core/BlockchainSetupUtil.java @@ -194,6 +194,7 @@ public class BlockchainSetupUtil { genesisState.writeStateTo(worldArchive.getMutable()); final ProtocolContext protocolContext = protocolContextProvider.get(blockchain, worldArchive); + protocolContext.setSynchronizer(new DummySynchronizer()); final Path blocksPath = Path.of(chainResources.getBlocksURL().toURI()); final List blocks = new ArrayList<>(); diff --git a/ethereum/retesteth/src/main/java/org/hyperledger/besu/ethereum/retesteth/DummySynchronizer.java b/ethereum/core/src/test-support/java/org/hyperledger/besu/ethereum/core/DummySynchronizer.java similarity index 89% rename from ethereum/retesteth/src/main/java/org/hyperledger/besu/ethereum/retesteth/DummySynchronizer.java rename to ethereum/core/src/test-support/java/org/hyperledger/besu/ethereum/core/DummySynchronizer.java index b8d90e9974..0de02d6d52 100644 --- a/ethereum/retesteth/src/main/java/org/hyperledger/besu/ethereum/retesteth/DummySynchronizer.java +++ b/ethereum/core/src/test-support/java/org/hyperledger/besu/ethereum/core/DummySynchronizer.java @@ -12,10 +12,9 @@ * * SPDX-License-Identifier: Apache-2.0 */ -package org.hyperledger.besu.ethereum.retesteth; +package org.hyperledger.besu.ethereum.core; import org.hyperledger.besu.datatypes.Address; -import org.hyperledger.besu.ethereum.core.Synchronizer; import org.hyperledger.besu.plugin.data.SyncStatus; import org.hyperledger.besu.plugin.services.BesuEvents; @@ -81,4 +80,14 @@ public class DummySynchronizer implements Synchronizer { public boolean unsubscribeInSync(final long listenerId) { return false; } + + @Override + public long subscribeInitialSync(final BesuEvents.InitialSyncCompletionListener listener) { + return 0; + } + + @Override + public boolean unsubscribeInitialSync(final long listenerId) { + return false; + } } 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 40573ee8d6..a63b736b0e 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 @@ -31,6 +31,7 @@ import org.hyperledger.besu.ethereum.core.Block; import org.hyperledger.besu.ethereum.core.BlockHeader; import org.hyperledger.besu.ethereum.core.BlockchainSetupUtil; import org.hyperledger.besu.ethereum.core.MutableWorldState; +import org.hyperledger.besu.ethereum.core.Synchronizer; import org.hyperledger.besu.ethereum.mainnet.BlockBodyValidator; import org.hyperledger.besu.ethereum.mainnet.BlockHeaderValidator; import org.hyperledger.besu.ethereum.mainnet.BlockProcessor; @@ -90,6 +91,7 @@ public class MainnetBlockValidatorTest { when(protocolContext.getBlockchain()).thenReturn(blockchain); when(protocolContext.getWorldStateArchive()).thenReturn(worldStateArchive); + when(protocolContext.getSynchronizer()).thenReturn(mock(Synchronizer.class)); when(worldStateArchive.getMutable(any(BlockHeader.class), anyBoolean())) .thenReturn(Optional.of(worldState)); when(worldStateArchive.getMutable(any(Hash.class), any(Hash.class))) diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/SnapServer.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/SnapServer.java index 7de933e137..56c3ae0a4a 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/SnapServer.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/SnapServer.java @@ -26,7 +26,6 @@ import org.hyperledger.besu.ethereum.eth.messages.snap.GetTrieNodesMessage; import org.hyperledger.besu.ethereum.eth.messages.snap.SnapV1; import org.hyperledger.besu.ethereum.eth.messages.snap.StorageRangeMessage; import org.hyperledger.besu.ethereum.eth.messages.snap.TrieNodesMessage; -import org.hyperledger.besu.ethereum.eth.sync.DefaultSynchronizer; import org.hyperledger.besu.ethereum.eth.sync.snapsync.SnapSyncConfiguration; import org.hyperledger.besu.ethereum.p2p.rlpx.wire.MessageData; import org.hyperledger.besu.ethereum.proof.WorldStateProofProvider; @@ -49,7 +48,6 @@ import java.util.NavigableMap; import java.util.Optional; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; -import java.util.concurrent.atomic.AtomicLong; import java.util.function.Function; import java.util.function.Predicate; import java.util.stream.Collectors; @@ -84,7 +82,6 @@ class SnapServer implements BesuEvents.InitialSyncCompletionListener { static final Hash HASH_LAST = Hash.wrap(Bytes32.leftPad(Bytes.fromHexString("FF"), (byte) 0xFF)); private final AtomicBoolean isStarted = new AtomicBoolean(false); - private final AtomicLong listenerId = new AtomicLong(); private final EthMessages snapMessages; private final WorldStateStorageCoordinator worldStateStorageCoordinator; @@ -111,14 +108,9 @@ class SnapServer implements BesuEvents.InitialSyncCompletionListener { this.protocolContext = Optional.of(protocolContext); registerResponseConstructors(); - // subscribe to initial sync completed events to start/stop snap server: - this.protocolContext - .flatMap(ProtocolContext::getSynchronizer) - .filter(z -> z instanceof DefaultSynchronizer) - .map(DefaultSynchronizer.class::cast) - .ifPresentOrElse( - z -> this.listenerId.set(z.subscribeInitialSync(this)), - () -> LOGGER.warn("SnapServer created without reference to sync status")); + // subscribe to initial sync completed events to start/stop snap server, + // not saving the listenerId since we never need to unsubscribe. + protocolContext.getSynchronizer().subscribeInitialSync(this); } /** diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/DefaultSynchronizer.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/DefaultSynchronizer.java index b7dc2adb16..2384437299 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/DefaultSynchronizer.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/DefaultSynchronizer.java @@ -388,10 +388,12 @@ public class DefaultSynchronizer implements Synchronizer, UnverifiedForkchoiceLi return syncState.unsubscribeSyncStatus(listenerId); } + @Override public long subscribeInitialSync(final BesuEvents.InitialSyncCompletionListener listener) { return syncState.subscribeCompletionReached(listener); } + @Override public boolean unsubscribeInitialSync(final long listenerId) { return syncState.unsubscribeInitialConditionReached(listenerId); } diff --git a/ethereum/retesteth/build.gradle b/ethereum/retesteth/build.gradle index 2fe2def439..484953a391 100644 --- a/ethereum/retesteth/build.gradle +++ b/ethereum/retesteth/build.gradle @@ -35,6 +35,7 @@ dependencies { implementation project(':ethereum:api') implementation project(':ethereum:blockcreation') implementation project(':ethereum:core') + implementation project(path: ':ethereum:core', configuration: 'testSupportArtifacts') implementation project(':ethereum:eth') implementation project(':ethereum:p2p') implementation project(':ethereum:rlp') diff --git a/ethereum/retesteth/src/main/java/org/hyperledger/besu/ethereum/retesteth/RetestethService.java b/ethereum/retesteth/src/main/java/org/hyperledger/besu/ethereum/retesteth/RetestethService.java index ceca8ebc11..877c597686 100644 --- a/ethereum/retesteth/src/main/java/org/hyperledger/besu/ethereum/retesteth/RetestethService.java +++ b/ethereum/retesteth/src/main/java/org/hyperledger/besu/ethereum/retesteth/RetestethService.java @@ -30,6 +30,7 @@ import org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods.EthSendRawTran import org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods.JsonRpcMethod; import org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods.Web3ClientVersion; import org.hyperledger.besu.ethereum.api.jsonrpc.internal.results.BlockResultFactory; +import org.hyperledger.besu.ethereum.core.DummySynchronizer; import org.hyperledger.besu.ethereum.core.Synchronizer; import org.hyperledger.besu.ethereum.retesteth.methods.TestGetLogHash; import org.hyperledger.besu.ethereum.retesteth.methods.TestImportRawBlock;