Skip to content

Commit

Permalink
Clean up pivot selector from peers and unit test (#8020)
Browse files Browse the repository at this point in the history
* 7582: Add waitForPeer method to PeerSelector and EthPeers

Signed-off-by: Matilda Clerke <[email protected]>

* 7582: Replace all usages of WaitForPeer[s]Task with new EthPeers.waitForPeer method

Signed-off-by: Matilda Clerke <[email protected]>

* 7582: Fix PivotBlockConfirmerTest

Signed-off-by: Matilda Clerke <[email protected]>

* 7582: spotless

Signed-off-by: Matilda Clerke <[email protected]>

* 7582: Fix broken PivotBlockRetrieverTest

Signed-off-by: Matilda Clerke <[email protected]>

* 7582: Fix broken FastSyncActionsTest

Signed-off-by: Matilda Clerke <[email protected]>

* 7582: spotless

Signed-off-by: Matilda Clerke <[email protected]>

* 7582: Simplify PivotSelectorFromPeers.selectNewPivotBlock and add unit tests

Signed-off-by: Matilda Clerke <[email protected]>

* 7582: Fix issues after merge

Signed-off-by: Matilda Clerke <[email protected]>

* 7582: Put AbstractSyncTargetManager.waitForPeerAndThenSetSyncTarget code back separate thread to avoid infinite loop waiting for peers during acceptance tests

Signed-off-by: Matilda Clerke <[email protected]>

* 7582: Remove pivot block checks when waiting for peer in FastSyncActions

Signed-off-by: Matilda Clerke <[email protected]>

* 7582: Remove estimated chain height check from PivotBlockConfirmer when waiting for peers

Signed-off-by: Matilda Clerke <[email protected]>

* 7582: Fix broken PivotBlockRetrieverTest

Signed-off-by: Matilda Clerke <[email protected]>

* Fix compile errors

Signed-off-by: Matilda Clerke <[email protected]>

* spotless

Signed-off-by: Matilda Clerke <[email protected]>

* Refactor mockito usage

Signed-off-by: Matilda Clerke <[email protected]>

---------

Signed-off-by: Matilda Clerke <[email protected]>
  • Loading branch information
Matilda-Clerke authored and garyschulte committed Dec 20, 2024
1 parent 979493f commit aa678d2
Show file tree
Hide file tree
Showing 2 changed files with 105 additions and 30 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import org.hyperledger.besu.ethereum.eth.sync.TrailingPeerRequirements;
import org.hyperledger.besu.ethereum.eth.sync.state.SyncState;

import java.util.List;
import java.util.Optional;
import java.util.concurrent.CompletableFuture;

Expand Down Expand Up @@ -86,41 +87,24 @@ protected Optional<FastSyncState> fromBestPeer(final EthPeer peer) {
}

protected Optional<EthPeer> selectBestPeer() {
return ethContext
.getEthPeers()
.bestPeerMatchingCriteria(this::canPeerDeterminePivotBlock)
// Only select a pivot block number when we have a minimum number of height estimates
.filter(unused -> enoughFastSyncPeersArePresent());
}

private boolean enoughFastSyncPeersArePresent() {
final long peerCount = countPeersThatCanDeterminePivotBlock();
List<EthPeer> peers =
ethContext
.getEthPeers()
.streamAvailablePeers()
.filter((peer) -> peer.chainState().hasEstimatedHeight() && peer.isFullyValidated())
.toList();

// Only select a pivot block number when we have a minimum number of height estimates
final int minPeerCount = syncConfig.getSyncMinimumPeerCount();
if (peerCount < minPeerCount) {
if (peers.size() < minPeerCount) {
LOG.info(
"Waiting for valid peers with chain height information. {} / {} required peers currently available.",
peerCount,
peers.size(),
minPeerCount);
return false;
return Optional.empty();
} else {
return peers.stream().max(ethContext.getEthPeers().getBestPeerComparator());
}
return true;
}

private long countPeersThatCanDeterminePivotBlock() {
return ethContext
.getEthPeers()
.streamAvailablePeers()
.filter(this::canPeerDeterminePivotBlock)
.count();
}

private boolean canPeerDeterminePivotBlock(final EthPeer peer) {
LOG.debug(
"peer {} hasEstimatedHeight {} isFullyValidated? {}",
peer.getLoggableId(),
peer.chainState().hasEstimatedHeight(),
peer.isFullyValidated());
return peer.chainState().hasEstimatedHeight() && peer.isFullyValidated();
}

private long conservativelyEstimatedPivotBlock() {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
/*
* Copyright contributors to 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.ethereum.eth.sync.fastsync;

import org.hyperledger.besu.ethereum.eth.manager.ChainState;
import org.hyperledger.besu.ethereum.eth.manager.EthContext;
import org.hyperledger.besu.ethereum.eth.manager.EthPeer;
import org.hyperledger.besu.ethereum.eth.manager.EthPeers;
import org.hyperledger.besu.ethereum.eth.sync.SynchronizerConfiguration;
import org.hyperledger.besu.ethereum.eth.sync.state.SyncState;

import java.util.Optional;
import java.util.stream.Stream;

import org.junit.jupiter.api.Assertions;
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.Mockito;
import org.mockito.junit.jupiter.MockitoExtension;
import org.mockito.junit.jupiter.MockitoSettings;
import org.mockito.quality.Strictness;

@ExtendWith(MockitoExtension.class)
@MockitoSettings(strictness = Strictness.LENIENT)
public class PivotSelectorFromPeersTest {
private @Mock EthContext ethContext;
private @Mock EthPeers ethPeers;
private @Mock SyncState syncState;

private PivotSelectorFromPeers selector;

@BeforeEach
public void beforeTest() {
SynchronizerConfiguration syncConfig =
SynchronizerConfiguration.builder().syncMinimumPeerCount(2).syncPivotDistance(1).build();

selector = new PivotSelectorFromPeers(ethContext, syncConfig, syncState);
}

@Test
public void testSelectNewPivotBlock() {
EthPeer peer1 = mockPeer(true, 10, true);
EthPeer peer2 = mockPeer(true, 8, true);

Mockito.when(ethContext.getEthPeers()).thenReturn(ethPeers);
Mockito.when(ethPeers.streamAvailablePeers()).thenReturn(Stream.of(peer1, peer2));
Mockito.when(ethPeers.getBestPeerComparator())
.thenReturn((p1, ignored) -> p1 == peer1 ? 1 : -1);

Optional<FastSyncState> result = selector.selectNewPivotBlock();

Assertions.assertTrue(result.isPresent());
Assertions.assertEquals(9, result.get().getPivotBlockNumber().getAsLong());
}

@Test
public void testSelectNewPivotBlockWithInsufficientPeers() {
Mockito.when(ethContext.getEthPeers()).thenReturn(ethPeers);
Mockito.when(ethPeers.streamAvailablePeers()).thenReturn(Stream.empty());

Optional<FastSyncState> result = selector.selectNewPivotBlock();

Assertions.assertTrue(result.isEmpty());
}

private EthPeer mockPeer(
final boolean hasEstimatedHeight, final long chainHeight, final boolean isFullyValidated) {
EthPeer ethPeer = Mockito.mock(EthPeer.class);
ChainState chainState = Mockito.mock(ChainState.class);
Mockito.when(ethPeer.chainState()).thenReturn(chainState);
Mockito.when(chainState.hasEstimatedHeight()).thenReturn(hasEstimatedHeight);
Mockito.when(chainState.getEstimatedHeight()).thenReturn(chainHeight);
Mockito.when(ethPeer.isFullyValidated()).thenReturn(isFullyValidated);

return ethPeer;
}
}

0 comments on commit aa678d2

Please sign in to comment.