Skip to content

Commit

Permalink
small fixes
Browse files Browse the repository at this point in the history 
Signed-off-by: stefan.pingel@consensys.net <stefan.pingel@consensys.net>
  • Loading branch information
@pinges
pinges committed Oct 25, 2024
1 parent 845b564 commit 023445f
Show file tree
Hide file tree
Showing 4 changed files with 58 additions and 57 deletions.
20 changes: 8 additions & 12 deletions ...n/java/org/hyperledger/besu/ethereum/eth/manager/peertask/task/GetBodiesFromPeerTask.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,12 @@
import org.hyperledger.besu.ethereum.eth.messages.GetBlockBodiesMessage;
import org.hyperledger.besu.ethereum.mainnet.BodyValidation;
import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule;
import org.hyperledger.besu.ethereum.mainnet.ProtocolSpec;
import org.hyperledger.besu.ethereum.p2p.rlpx.wire.MessageData;
import org.hyperledger.besu.ethereum.p2p.rlpx.wire.SubProtocol;

import java.util.ArrayList;
import java.util.List;
import java.util.function.Predicate;
import java.util.function.Supplier;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand All @@ -42,26 +40,27 @@ public class GetBodiesFromPeerTask implements PeerTask<List<Block>> {
private static final Logger LOG = LoggerFactory.getLogger(GetBodiesFromPeerTask.class);

private final List<BlockHeader> blockHeaders;
private final Supplier<ProtocolSpec> currentProtocolSpecSupplier;
private final ProtocolSchedule protocolSchedule;

private final long requiredBlockchainHeight;
private final List<Block> blocks = new ArrayList<>();
private final boolean isPoS;

public GetBodiesFromPeerTask(
final List<BlockHeader> blockHeaders,
final Supplier<ProtocolSpec> currentProtocolSpecSupplier,
final ProtocolSchedule protocolSchedule) {
final List<BlockHeader> blockHeaders, final ProtocolSchedule protocolSchedule) {
if (blockHeaders == null || blockHeaders.isEmpty()) {
throw new IllegalArgumentException("Block headers must not be empty");
}

this.blockHeaders = blockHeaders;
this.currentProtocolSpecSupplier = currentProtocolSpecSupplier;
this.protocolSchedule = protocolSchedule;

this.requiredBlockchainHeight =
blockHeaders.stream()
.mapToLong(BlockHeader::getNumber)
.max()
.orElse(BlockHeader.GENESIS_BLOCK_NUMBER);
this.isPoS = protocolSchedule.getByBlockHeader(blockHeaders.getLast()).isPoS();
}

@Override
Expand Down Expand Up @@ -105,8 +104,7 @@ public List<Block> parseResponse(final MessageData messageData)
private boolean blockBodyMatchesBlockHeader(
final BlockBody blockBody, final BlockHeader blockHeader) {
// this method validates that the block body matches the block header by calculating the roots
// of the block body
// and comparing them to the roots in the block header
// of the block body and comparing them to the roots in the block header
if (!BodyValidation.transactionsRoot(blockBody.getTransactions())
.equals(blockHeader.getTransactionsRoot())) {
return false;
Expand Down Expand Up @@ -142,9 +140,7 @@ private boolean blockBodyMatchesBlockHeader(
@Override
public Predicate<EthPeer> getPeerRequirementFilter() {
return (ethPeer) ->
ethPeer.getProtocolName().equals(getSubProtocol().getName())
&& (currentProtocolSpecSupplier.get().isPoS()
|| ethPeer.chainState().getEstimatedHeight() >= requiredBlockchainHeight);
isPoS || ethPeer.chainState().getEstimatedHeight() >= requiredBlockchainHeight;
}

@Override
Expand Down
8 changes: 2 additions & 6 deletions ...rc/main/java/org/hyperledger/besu/ethereum/eth/sync/tasks/CompleteBlocksWithPeerTask.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ public class CompleteBlocksWithPeerTask {
private final Block[] result;
private final int resultSize;
private int nextIndex = 0;
private int remainingBlocks = 0;
private int remainingBlocks;

public CompleteBlocksWithPeerTask(
final ProtocolSchedule protocolSchedule,
Expand Down Expand Up @@ -97,11 +97,7 @@ public List<Block> getBlocks() {
.setMessage("Requesting {} bodies from peer")
.addArgument(headersToGet.size())
.log();
final GetBodiesFromPeerTask task =
new GetBodiesFromPeerTask(
headersToGet,
() -> protocolSchedule.getByBlockHeader(headersToGet.getLast()),
protocolSchedule);
final GetBodiesFromPeerTask task = new GetBodiesFromPeerTask(headersToGet, protocolSchedule);
final PeerTaskExecutorResult<List<Block>> executionResult = peerTaskExecutor.execute(task);
if (executionResult.responseCode() == PeerTaskExecutorResponseCode.SUCCESS
&& executionResult.result().isPresent()) {
Expand Down
42 changes: 26 additions & 16 deletions ...va/org/hyperledger/besu/ethereum/eth/manager/peertask/task/GetBodiesFromPeerTaskTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import org.hyperledger.besu.ethereum.eth.messages.EthPV62;
import org.hyperledger.besu.ethereum.eth.messages.GetBlockBodiesMessage;
import org.hyperledger.besu.ethereum.mainnet.BodyValidation;
import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule;
import org.hyperledger.besu.ethereum.mainnet.ProtocolSpec;
import org.hyperledger.besu.ethereum.p2p.rlpx.wire.MessageData;
import org.hyperledger.besu.ethereum.rlp.BytesValueRLPInput;
Expand All @@ -45,6 +46,7 @@
import org.apache.commons.lang3.StringUtils;
import org.apache.tuweni.bytes.Bytes;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;
import org.mockito.Mockito;

Expand All @@ -60,18 +62,29 @@ public class GetBodiesFromPeerTaskTest {
public static final List<Transaction> TRANSACTION_LIST = List.of(TX);
public static final BlockBody BLOCK_BODY =
new BlockBody(TRANSACTION_LIST, Collections.emptyList(), Optional.empty(), Optional.empty());
private static ProtocolSchedule protocolSchedule;

@BeforeAll
public static void setup() {
protocolSchedule = mock(ProtocolSchedule.class);
final ProtocolSpec protocolSpec = mock(ProtocolSpec.class);
when(protocolSpec.isPoS()).thenReturn(true);
when(protocolSchedule.getByBlockHeader(Mockito.any())).thenReturn(protocolSpec);
}

@Test
public void testGetSubProtocol() {
GetBodiesFromPeerTask task = new GetBodiesFromPeerTask(Collections.emptyList(), null, null);

GetBodiesFromPeerTask task =
new GetBodiesFromPeerTask(List.of(mockBlockHeader(0)), protocolSchedule);
Assertions.assertEquals(EthProtocol.get(), task.getSubProtocol());
}

@Test
public void testGetRequestMessage() {
GetBodiesFromPeerTask task =
new GetBodiesFromPeerTask(
List.of(mockBlockHeader(1), mockBlockHeader(2), mockBlockHeader(3)), null, null);
List.of(mockBlockHeader(1), mockBlockHeader(2), mockBlockHeader(3)), protocolSchedule);

MessageData messageData = task.getRequestMessage();
GetBlockBodiesMessage getBlockBodiesMessage = GetBlockBodiesMessage.readFrom(messageData);
Expand All @@ -93,13 +106,15 @@ public void testGetRequestMessage() {

@Test
public void testParseResponseWithNullResponseMessage() {
GetBodiesFromPeerTask task = new GetBodiesFromPeerTask(Collections.emptyList(), null, null);
Assertions.assertThrows(InvalidPeerTaskResponseException.class, () -> task.parseResponse(null));
Assertions.assertThrows(
IllegalArgumentException.class,
() -> new GetBodiesFromPeerTask(Collections.emptyList(), protocolSchedule));
}

@Test
public void testParseResponseForInvalidResponse() {
GetBodiesFromPeerTask task = new GetBodiesFromPeerTask(List.of(mockBlockHeader(1)), null, null);
GetBodiesFromPeerTask task =
new GetBodiesFromPeerTask(List.of(mockBlockHeader(1)), protocolSchedule);
// body does not match header
BlockBodiesMessage bodiesMessage = BlockBodiesMessage.create(List.of(BLOCK_BODY));

Expand All @@ -113,7 +128,7 @@ public void testParseResponse() throws InvalidPeerTaskResponseException {
getNonEmptyBlockHeaderMock(BodyValidation.transactionsRoot(TRANSACTION_LIST).toString());

GetBodiesFromPeerTask task =
new GetBodiesFromPeerTask(List.of(nonEmptyBlockHeaderMock), null, null);
new GetBodiesFromPeerTask(List.of(nonEmptyBlockHeaderMock), protocolSchedule);

final BlockBodiesMessage blockBodiesMessage = BlockBodiesMessage.create(List.of(BLOCK_BODY));

Expand All @@ -129,32 +144,27 @@ public void testGetPeerRequirementFilter() {
BlockHeader blockHeader2 = mockBlockHeader(2);
BlockHeader blockHeader3 = mockBlockHeader(3);

ProtocolSpec protocolSpec = Mockito.mock(ProtocolSpec.class);
Mockito.when(protocolSpec.isPoS()).thenReturn(false);

GetBodiesFromPeerTask task =
new GetBodiesFromPeerTask(
List.of(blockHeader1, blockHeader2, blockHeader3), () -> protocolSpec, null);
List.of(blockHeader1, blockHeader2, blockHeader3), protocolSchedule);

EthPeer failForIncorrectProtocol = mockPeer("incorrectProtocol", 5);
EthPeer failForShortChainHeight = mockPeer("incorrectProtocol", 1);
EthPeer successfulCandidate = mockPeer(EthProtocol.NAME, 5);

Assertions.assertFalse(task.getPeerRequirementFilter().test(failForIncorrectProtocol));
Assertions.assertFalse(task.getPeerRequirementFilter().test(failForShortChainHeight));
Assertions.assertTrue(task.getPeerRequirementFilter().test(successfulCandidate));
}

@Test
public void testIsSuccessForPartialSuccess() {
GetBodiesFromPeerTask task = new GetBodiesFromPeerTask(Collections.emptyList(), null, null);
GetBodiesFromPeerTask task =
new GetBodiesFromPeerTask(List.of(mockBlockHeader(1)), protocolSchedule);

Assertions.assertFalse(task.isSuccess(Collections.emptyList()));
}

@Test
public void testIsSuccessForFullSuccess() {
GetBodiesFromPeerTask task = new GetBodiesFromPeerTask(Collections.emptyList(), null, null);
GetBodiesFromPeerTask task =
new GetBodiesFromPeerTask(List.of(mockBlockHeader(1)), protocolSchedule);

final List<Block> blockHeaders = List.of(mock(Block.class));

Expand Down
45 changes: 22 additions & 23 deletions ...est/java/org/hyperledger/besu/ethereum/eth/sync/tasks/CompleteBlocksWithPeerTaskTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,10 +63,10 @@ public void shouldReturnEmptyBlock() {

CompleteBlocksWithPeerTask completeBlocksWithPeerTask =
new CompleteBlocksWithPeerTask(protocolSchedule, List.of(blockHeader), peerTaskExecutor);
assertThat(completeBlocksWithPeerTask.getBlocks()).isNotEmpty();
assertThat(completeBlocksWithPeerTask.getBlocks().size()).isEqualTo(1);
assertThat(BlockHeader.hasEmptyBlock(completeBlocksWithPeerTask.getBlocks().get(0).getHeader()))
.isTrue();
final List<Block> blocks = completeBlocksWithPeerTask.getBlocks();
assertThat(blocks).isNotEmpty();
assertThat(blocks.size()).isEqualTo(1);
assertThat(BlockHeader.hasEmptyBlock(blocks.get(0).getHeader())).isTrue();

verify(peerTaskExecutor, Mockito.never()).execute(any());
}
Expand Down Expand Up @@ -120,9 +120,10 @@ public void shouldReturnNonEmptyBlock() {
new CompleteBlocksWithPeerTask(
protocolSchedule, List.of(nonEmptyBlockHeaderMock), peerTaskExecutor);

assertThat(completeBlocksWithPeerTask.getBlocks()).isNotEmpty();
assertThat(completeBlocksWithPeerTask.getBlocks().size()).isEqualTo(1);
assertThat(completeBlocksWithPeerTask.getBlocks().get(0)).isEqualTo(block);
final List<Block> blocks = completeBlocksWithPeerTask.getBlocks();
assertThat(blocks).isNotEmpty();
assertThat(blocks.size()).isEqualTo(1);
assertThat(blocks.get(0)).isEqualTo(block);
}

@Test
Expand Down Expand Up @@ -150,14 +151,13 @@ public void shouldReturnBlocksInRightOrderWhenEmptyAndNonEmptyBlocksRequested()
emptyBlockHeaderMock),
peerTaskExecutor);

assertThat(completeBlocksWithPeerTask.getBlocks()).isNotEmpty();
assertThat(completeBlocksWithPeerTask.getBlocks().size()).isEqualTo(4);
assertThat(completeBlocksWithPeerTask.getBlocks().get(0)).isEqualTo(block1);
assertThat(BlockHeader.hasEmptyBlock(completeBlocksWithPeerTask.getBlocks().get(1).getHeader()))
.isTrue();
assertThat(completeBlocksWithPeerTask.getBlocks().get(2)).isEqualTo(block3);
assertThat(BlockHeader.hasEmptyBlock(completeBlocksWithPeerTask.getBlocks().get(3).getHeader()))
.isTrue();
final List<Block> blocks = completeBlocksWithPeerTask.getBlocks();
assertThat(blocks).isNotEmpty();
assertThat(blocks.size()).isEqualTo(4);
assertThat(blocks.get(0)).isEqualTo(block1);
assertThat(BlockHeader.hasEmptyBlock(blocks.get(1).getHeader())).isTrue();
assertThat(blocks.get(2)).isEqualTo(block3);
assertThat(BlockHeader.hasEmptyBlock(blocks.get(3).getHeader())).isTrue();
}

@Test
Expand Down Expand Up @@ -188,14 +188,13 @@ public void shouldRequestMoreBodiesUntilFinished() {
emptyBlockHeaderMock),
peerTaskExecutor);

assertThat(completeBlocksWithPeerTask.getBlocks()).isNotEmpty();
assertThat(completeBlocksWithPeerTask.getBlocks().size()).isEqualTo(4);
assertThat(completeBlocksWithPeerTask.getBlocks().get(0)).isEqualTo(block1);
assertThat(BlockHeader.hasEmptyBlock(completeBlocksWithPeerTask.getBlocks().get(1).getHeader()))
.isTrue();
assertThat(completeBlocksWithPeerTask.getBlocks().get(2)).isEqualTo(block3);
assertThat(BlockHeader.hasEmptyBlock(completeBlocksWithPeerTask.getBlocks().get(3).getHeader()))
.isTrue();
final List<Block> blocks = completeBlocksWithPeerTask.getBlocks();
assertThat(blocks).isNotEmpty();
assertThat(blocks.size()).isEqualTo(4);
assertThat(blocks.get(0)).isEqualTo(block1);
assertThat(BlockHeader.hasEmptyBlock(blocks.get(1).getHeader())).isTrue();
assertThat(blocks.get(2)).isEqualTo(block3);
assertThat(BlockHeader.hasEmptyBlock(blocks.get(3).getHeader())).isTrue();
}

private static ProtocolSchedule getProtocolScheduleMock() {
Expand Down

0 comments on commit 023445f

Please sign in to comment.