From a4acb49e36b8d1064a6619194b4b3e7c972208f5 Mon Sep 17 00:00:00 2001 From: Volodymyr Kravets Date: Thu, 5 Dec 2024 18:00:08 +0200 Subject: [PATCH] feat(snap): address comments; add some tests --- .../rsk/core/bc/BlockRelayValidatorTest.java | 12 +++--- .../co/rsk/core/bc/BlockValidatorBuilder.java | 13 +++--- .../co/rsk/core/bc/BlockValidatorTest.java | 18 +++++---- .../co/rsk/net/sync/SnapSyncStateTest.java | 40 +++++++++++++++++++ 4 files changed, 65 insertions(+), 18 deletions(-) diff --git a/rskj-core/src/test/java/co/rsk/core/bc/BlockRelayValidatorTest.java b/rskj-core/src/test/java/co/rsk/core/bc/BlockRelayValidatorTest.java index 29ab5d1224..95c386be84 100644 --- a/rskj-core/src/test/java/co/rsk/core/bc/BlockRelayValidatorTest.java +++ b/rskj-core/src/test/java/co/rsk/core/bc/BlockRelayValidatorTest.java @@ -47,7 +47,7 @@ void genesisCheck() { verify(block).isGenesis(); verify(blockValidator, never()).isValid(any()); - verify(blockParentValidator, never()).isValid(any(), (Block) any()); + verify(blockParentValidator, never()).isValid(any(), any(Block.class)); } @Test @@ -62,7 +62,7 @@ void blockValidatorCheck() { verify(block).isGenesis(); verify(blockValidator).isValid(any()); - verify(blockParentValidator, never()).isValid(any(), (Block) any()); + verify(blockParentValidator, never()).isValid(any(), any(Block.class)); } @Test @@ -74,7 +74,7 @@ void blockParentValidatorCheck() { when(block.getParentHash()).thenReturn(parentHash); when(blockStore.getBlockByHash(any())).thenReturn(parentBlock); when(blockValidator.isValid(any())).thenReturn(true); - when(blockParentValidator.isValid(any(), (Block) any())).thenReturn(false); + when(blockParentValidator.isValid(any(), any(Block.class))).thenReturn(false); boolean actualResult = blockRelayValidator.isValid(block); @@ -82,7 +82,7 @@ void blockParentValidatorCheck() { verify(block).isGenesis(); verify(blockValidator).isValid(any()); - verify(blockParentValidator).isValid(any(), (Block) any()); + verify(blockParentValidator).isValid(any(), any(Block.class)); } @Test @@ -94,7 +94,7 @@ void allValidatorsCheck() { when(block.getParentHash()).thenReturn(parentHash); when(blockStore.getBlockByHash(any())).thenReturn(parentBlock); when(blockValidator.isValid(any())).thenReturn(true); - when(blockParentValidator.isValid(any(), (Block) any())).thenReturn(true); + when(blockParentValidator.isValid(any(), any(Block.class))).thenReturn(true); boolean actualResult = blockRelayValidator.isValid(block); @@ -102,6 +102,6 @@ void allValidatorsCheck() { verify(block).isGenesis(); verify(blockValidator).isValid(any()); - verify(blockParentValidator).isValid(any(), (Block) any()); + verify(blockParentValidator).isValid(any(), any(Block.class)); } } diff --git a/rskj-core/src/test/java/co/rsk/core/bc/BlockValidatorBuilder.java b/rskj-core/src/test/java/co/rsk/core/bc/BlockValidatorBuilder.java index 34be11c2c6..5fb5e4d3e1 100644 --- a/rskj-core/src/test/java/co/rsk/core/bc/BlockValidatorBuilder.java +++ b/rskj-core/src/test/java/co/rsk/core/bc/BlockValidatorBuilder.java @@ -31,7 +31,10 @@ import org.ethereum.core.ReceivedTxSignatureCache; import org.ethereum.datasource.HashMapDB; import org.ethereum.db.BlockStore; -import org.mockito.Mockito; + +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; /** * Created by mario on 19/01/17. @@ -95,11 +98,11 @@ public BlockValidatorBuilder addTxsMinGasPriceRule() { } public BlockValidatorBuilder addBlockUnclesValidationRule(BlockStore blockStore) { - BlockHeaderValidationRule validationRule = Mockito.mock(BlockHeaderValidationRule.class); - Mockito.when(validationRule.isValid(Mockito.any())).thenReturn(true); + BlockHeaderValidationRule validationRule = mock(BlockHeaderValidationRule.class); + when(validationRule.isValid(any())).thenReturn(true); - BlockHeaderParentDependantValidationRule parentValidationRule = Mockito.mock(BlockHeaderParentDependantValidationRule.class); - Mockito.when(parentValidationRule.isValid(Mockito.any(), (Block) Mockito.any())).thenReturn(true); + BlockHeaderParentDependantValidationRule parentValidationRule = mock(BlockHeaderParentDependantValidationRule.class); + when(parentValidationRule.isValid(any(), any(Block.class))).thenReturn(true); this.addBlockUnclesValidationRule(blockStore, validationRule, parentValidationRule); return this; diff --git a/rskj-core/src/test/java/co/rsk/core/bc/BlockValidatorTest.java b/rskj-core/src/test/java/co/rsk/core/bc/BlockValidatorTest.java index f1f95fd489..215b154284 100644 --- a/rskj-core/src/test/java/co/rsk/core/bc/BlockValidatorTest.java +++ b/rskj-core/src/test/java/co/rsk/core/bc/BlockValidatorTest.java @@ -43,12 +43,16 @@ import org.ethereum.db.IndexedBlockStore; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; -import org.mockito.Mockito; import java.math.BigInteger; -import java.util.*; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.Set; -import static org.mockito.Mockito.*; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; /** * Created by ajlopez on 04/08/2016. @@ -288,7 +292,7 @@ void invalidPOWUncles() { store.saveBlock(uncle1a, TEST_DIFFICULTY, false); BlockHeaderParentDependantValidationRule parentValidationRule = mock(BlockHeaderParentDependantValidationRule.class); - when(parentValidationRule.isValid(Mockito.any(), (Block) Mockito.any())).thenReturn(true); + when(parentValidationRule.isValid(any(), any(Block.class))).thenReturn(true); BlockValidatorImpl validator = new BlockValidatorBuilder() .addBlockUnclesValidationRule(store, new ProofOfWorkRule(config).setFallbackMiningEnabled(false), parentValidationRule) @@ -497,7 +501,7 @@ void processBlockWithInvalidMGPTxs() { BlockStore blockStore = mock(org.ethereum.db.BlockStore.class); Repository repository = mock(Repository.class); - when(repository.getNonce(Mockito.any())).thenReturn(BigInteger.ZERO); + when(repository.getNonce(any())).thenReturn(BigInteger.ZERO); Block parent = new BlockBuilder(null, null, null).minGasPrice(BigInteger.ZERO) .parent(new BlockGenerator().getGenesisBlock()).build(); @@ -533,7 +537,7 @@ void processBlockWithInvalidPrevMGP() { BlockStore blockStore = mock(org.ethereum.db.BlockStore.class); Repository repository = mock(Repository.class); - when(repository.getNonce(Mockito.any())).thenReturn(BigInteger.ZERO); + when(repository.getNonce(any())).thenReturn(BigInteger.ZERO); Block parent = new BlockBuilder(null, null, null).minGasPrice(BigInteger.ZERO) .parent(new BlockGenerator().getGenesisBlock()).build(); @@ -556,7 +560,7 @@ void processValidMGPBlock() { BlockStore blockStore = mock(org.ethereum.db.BlockStore.class); Repository repository = mock(Repository.class); - when(repository.getNonce(Mockito.any())).thenReturn(BigInteger.ZERO); + when(repository.getNonce(any())).thenReturn(BigInteger.ZERO); Block parent = new BlockBuilder(null, null, null).minGasPrice(BigInteger.TEN) .parent(new BlockGenerator().getGenesisBlock()).build(); diff --git a/rskj-core/src/test/java/co/rsk/net/sync/SnapSyncStateTest.java b/rskj-core/src/test/java/co/rsk/net/sync/SnapSyncStateTest.java index 02a8c220b2..f12e508048 100644 --- a/rskj-core/src/test/java/co/rsk/net/sync/SnapSyncStateTest.java +++ b/rskj-core/src/test/java/co/rsk/net/sync/SnapSyncStateTest.java @@ -21,11 +21,14 @@ import co.rsk.core.BlockDifficulty; import co.rsk.net.Peer; import co.rsk.net.SnapshotProcessor; +import co.rsk.net.messages.MessageType; import co.rsk.net.messages.SnapBlocksResponseMessage; import co.rsk.net.messages.SnapStateChunkResponseMessage; import co.rsk.net.messages.SnapStatusResponseMessage; +import co.rsk.scoring.EventType; import org.apache.commons.lang3.tuple.Pair; import org.ethereum.core.Block; +import org.ethereum.core.BlockHeader; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -159,6 +162,29 @@ void givenOnSnapBlocksIsCalled_thenJobIsAddedAndRun() throws InterruptedExceptio assertEquals(msg.getMessageType(), jobArg.getValue().getMsgType()); } + @Test + void givenNewBlockHeadersIsCalled_thenJobIsAddedAndRun() throws InterruptedException { + //given + Peer peer = mock(Peer.class); + //noinspection unchecked + List msg = mock(List.class); + CountDownLatch latch = new CountDownLatch(1); + doCountDownOnQueueEmpty(listener, latch); + underTest.onEnter(); + + //when + underTest.newBlockHeaders(peer, msg); + + //then + assertTrue(latch.await(THREAD_JOIN_TIMEOUT, TimeUnit.MILLISECONDS)); + + ArgumentCaptor jobArg = ArgumentCaptor.forClass(SyncMessageHandler.Job.class); + verify(listener, times(1)).onJobRun(jobArg.capture()); + + assertEquals(peer, jobArg.getValue().getSender()); + assertEquals(MessageType.BLOCK_HEADERS_RESPONSE_MESSAGE, jobArg.getValue().getMsgType()); + } + @Test void givenOnSnapStateChunkIsCalled_thenJobIsAddedAndRun() throws InterruptedException { //given @@ -181,6 +207,20 @@ void givenOnSnapStateChunkIsCalled_thenJobIsAddedAndRun() throws InterruptedExce assertEquals(msg.getMessageType(), jobArg.getValue().getMsgType()); } + @Test + void givenOnMessageTimeOut_thenShouldFail() throws InterruptedException { + //given + Peer peer = mock(Peer.class); + underTest.setLastBlock(mock(Block.class), mock(BlockDifficulty.class), peer); + underTest.setRunning(); + + //when + underTest.onMessageTimeOut(); + + //then + verify(syncEventsHandler, times(1)).onErrorSyncing(eq(peer), eq(EventType.TIMEOUT_MESSAGE), any()); + } + @Test void testSetAndGetLastBlock() { Block mockBlock = mock(Block.class);