From 31c4792db11bee2570910e030df9fc2fcc994327 Mon Sep 17 00:00:00 2001 From: Leonor Boga Date: Tue, 23 Mar 2021 15:27:45 +0100 Subject: [PATCH] [ETCM-709] Improve ommers validations Replaced OmmersAncestorsError by more specific OmmerIsAncestorError and OmmerParentIsNotAncestorError. Renamed OmmersNotValidError to OmmersHeaderError and added the underlying reason for failure Refactored validateOmmersAncestors method. Refactored validateOmmersHeaders method to not hide the underlying failing reason. Reorganized some packages because some classes were not in the correct package and/or the Spec was not in the correct package. Renamed some specs to better reflect the class being tested Raised the log level of some logs in BlockFetcher because there are barely any logs this actor when debug logs are disabled --- .../ets/blockchain/BlockchainTestConfig.scala | 2 +- .../ets/blockchain/ScenarioSetup.scala | 3 +- .../ethereum/ledger/BlockImporterItSpec.scala | 5 +- .../ethereum/sync/RegularSyncItSpec.scala | 7 +- .../sync/util/RegularSyncItSpecUtils.scala | 11 ++- .../sync/regular/BlockFetcher.scala | 44 +++++++----- .../sync/regular/BlockImporter.scala | 9 ++- .../ethereum/consensus/ConsensusBuilder.scala | 2 +- .../consensus/ethash/EthashConsensus.scala | 3 +- .../ethash/blocks/EthashBlockGenerator.scala | 2 +- .../RestrictedEthashBlockGeneratorImpl.scala | 2 +- .../ethash/validators/OmmersValidator.scala | 6 +- .../std}/StdOmmersValidator.scala | 68 ++++++++++--------- .../validators/std/StdValidators.scala | 2 +- .../std}/StdValidatorsExecutor.scala | 7 +- .../std}/ValidatorsExecutor.scala | 15 ++-- .../io/iohk/ethereum/ledger/Ledger.scala | 13 +++- src/test/scala/io/iohk/ethereum/Mocks.scala | 9 +-- .../blockchain/sync/ScenarioSetup.scala | 4 +- .../{ => blocks}/BlockGeneratorSpec.scala | 8 +-- .../consensus/ethash/EthashMinerSpec.scala | 8 ++- .../EthashBlockHeaderValidatorSpec.scala} | 13 ++-- ...rictedEthashBlockHeaderValidatorSpec.scala | 4 +- .../StdBlockValidatorSpec.scala} | 7 +- .../StdOmmersValidatorSpec.scala} | 63 +++++++++++++---- .../StdSignedTransactionValidatorSpec.scala} | 12 ++-- .../jsonrpc/CheckpointingJRCSpec.scala | 3 +- .../jsonrpc/JsonRpcControllerFixture.scala | 2 +- .../ethereum/ledger/LedgerTestSetup.scala | 5 +- .../handshaker/EtcHandshakerSpec.scala | 3 +- 30 files changed, 219 insertions(+), 123 deletions(-) rename src/main/scala/io/iohk/ethereum/consensus/{ethash/validators => validators/std}/StdOmmersValidator.scala (71%) rename src/main/scala/io/iohk/ethereum/consensus/{ethash/validators => validators/std}/StdValidatorsExecutor.scala (68%) rename src/main/scala/io/iohk/ethereum/consensus/{ethash/validators => validators/std}/ValidatorsExecutor.scala (89%) rename src/test/scala/io/iohk/ethereum/consensus/{ => blocks}/BlockGeneratorSpec.scala (99%) rename src/test/scala/io/iohk/ethereum/consensus/{validators/BlockHeaderValidatorSpec.scala => ethash/validators/EthashBlockHeaderValidatorSpec.scala} (99%) rename src/test/scala/io/iohk/ethereum/consensus/{ => ethash}/validators/RestrictedEthashBlockHeaderValidatorSpec.scala (98%) rename src/test/scala/io/iohk/ethereum/consensus/validators/{BlockValidatorSpec.scala => std/StdBlockValidatorSpec.scala} (97%) rename src/test/scala/io/iohk/ethereum/consensus/validators/{OmmersValidatorSpec.scala => std/StdOmmersValidatorSpec.scala} (89%) rename src/test/scala/io/iohk/ethereum/consensus/validators/{SignedTransactionValidatorSpec.scala => std/StdSignedTransactionValidatorSpec.scala} (98%) diff --git a/src/ets/scala/io/iohk/ethereum/ets/blockchain/BlockchainTestConfig.scala b/src/ets/scala/io/iohk/ethereum/ets/blockchain/BlockchainTestConfig.scala index fe97161430..b8cf286ba1 100644 --- a/src/ets/scala/io/iohk/ethereum/ets/blockchain/BlockchainTestConfig.scala +++ b/src/ets/scala/io/iohk/ethereum/ets/blockchain/BlockchainTestConfig.scala @@ -2,7 +2,7 @@ package io.iohk.ethereum.ets.blockchain import akka.util.ByteString import io.iohk.ethereum.consensus.Protocol -import io.iohk.ethereum.consensus.ethash.validators.ValidatorsExecutor +import io.iohk.ethereum.consensus.validators.std.ValidatorsExecutor import io.iohk.ethereum.domain.{Address, UInt256} import io.iohk.ethereum.utils.{BlockchainConfig, DaoForkConfig, MonetaryPolicyConfig} import org.bouncycastle.util.encoders.Hex diff --git a/src/ets/scala/io/iohk/ethereum/ets/blockchain/ScenarioSetup.scala b/src/ets/scala/io/iohk/ethereum/ets/blockchain/ScenarioSetup.scala index 9a7ff6fcf1..b06da2b53f 100644 --- a/src/ets/scala/io/iohk/ethereum/ets/blockchain/ScenarioSetup.scala +++ b/src/ets/scala/io/iohk/ethereum/ets/blockchain/ScenarioSetup.scala @@ -3,7 +3,7 @@ package io.iohk.ethereum.ets.blockchain import akka.util.ByteString import io.iohk.ethereum.consensus.Protocol.NoAdditionalEthashData import io.iohk.ethereum.consensus.ethash.EthashConsensus -import io.iohk.ethereum.consensus.ethash.validators.ValidatorsExecutor +import io.iohk.ethereum.consensus.validators.std.ValidatorsExecutor import io.iohk.ethereum.consensus.{ConsensusConfig, FullConsensusConfig, TestConsensus, ethash} import io.iohk.ethereum.db.components.Storages.PruningModeComponent import io.iohk.ethereum.db.components.{EphemDataSourceComponent, Storages} @@ -18,6 +18,7 @@ import io.iohk.ethereum.utils.BigIntExtensionMethods._ import io.iohk.ethereum.utils.{BlockchainConfig, Config} import monix.execution.Scheduler import org.bouncycastle.util.encoders.Hex + import scala.util.{Failure, Success, Try} object ScenarioSetup { diff --git a/src/it/scala/io/iohk/ethereum/ledger/BlockImporterItSpec.scala b/src/it/scala/io/iohk/ethereum/ledger/BlockImporterItSpec.scala index 960735af54..fe2f42e0be 100644 --- a/src/it/scala/io/iohk/ethereum/ledger/BlockImporterItSpec.scala +++ b/src/it/scala/io/iohk/ethereum/ledger/BlockImporterItSpec.scala @@ -8,8 +8,9 @@ import io.iohk.ethereum.blockchain.sync.regular.{BlockFetcher, BlockImporter} import io.iohk.ethereum.checkpointing.CheckpointingTestHelpers import io.iohk.ethereum.consensus.{GetBlockHeaderByHash, GetNBlocksBack} import io.iohk.ethereum.consensus.blocks.CheckpointBlockGenerator -import io.iohk.ethereum.consensus.ethash.validators.{OmmersValidator, StdOmmersValidator} +import io.iohk.ethereum.consensus.ethash.validators.OmmersValidator import io.iohk.ethereum.consensus.validators.Validators +import io.iohk.ethereum.consensus.validators.std.StdOmmersValidator import io.iohk.ethereum.domain._ import io.iohk.ethereum.mpt.MerklePatriciaTrie import io.iohk.ethereum.utils.Config.SyncConfig @@ -74,7 +75,7 @@ class BlockImporterItSpec getBlockHeaderByHash: GetBlockHeaderByHash, getNBlocksBack: GetNBlocksBack ) => - new StdOmmersValidator(blockchainConfig, blockHeaderValidator) + new StdOmmersValidator(blockHeaderValidator) .validate(parentHash, blockNumber, ommers, getBlockHeaderByHash, getNBlocksBack) } diff --git a/src/it/scala/io/iohk/ethereum/sync/RegularSyncItSpec.scala b/src/it/scala/io/iohk/ethereum/sync/RegularSyncItSpec.scala index bddfa98dc2..44f04bc746 100644 --- a/src/it/scala/io/iohk/ethereum/sync/RegularSyncItSpec.scala +++ b/src/it/scala/io/iohk/ethereum/sync/RegularSyncItSpec.scala @@ -152,7 +152,12 @@ class RegularSyncItSpec extends FreeSpecBase with Matchers with BeforeAndAfterAl _ <- peer1.waitForRegularSyncLoadLastBlock(length) } yield { assert(peer1.bl.getBestBlock().get.hash == peer2.bl.getBestBlock().get.hash) - assert(peer1.bl.getBestBlock().get.number == peer2.bl.getBestBlock().get.number && peer1.bl.getBestBlock().get.number == length) + assert( + peer1.bl.getBestBlock().get.number == peer2.bl.getBestBlock().get.number && peer1.bl + .getBestBlock() + .get + .number == length + ) assert(peer1.bl.getLatestCheckpointBlockNumber() == peer2.bl.getLatestCheckpointBlockNumber()) } } diff --git a/src/it/scala/io/iohk/ethereum/sync/util/RegularSyncItSpecUtils.scala b/src/it/scala/io/iohk/ethereum/sync/util/RegularSyncItSpecUtils.scala index 6f4d1725c9..2bcf63fe50 100644 --- a/src/it/scala/io/iohk/ethereum/sync/util/RegularSyncItSpecUtils.scala +++ b/src/it/scala/io/iohk/ethereum/sync/util/RegularSyncItSpecUtils.scala @@ -7,7 +7,13 @@ import io.iohk.ethereum.Mocks.MockValidatorsAlwaysSucceed import io.iohk.ethereum.blockchain.sync.regular.BlockBroadcast.BlockToBroadcast import io.iohk.ethereum.blockchain.sync.regular.BlockBroadcasterActor.BroadcastBlock import io.iohk.ethereum.blockchain.sync.regular.BlockImporter.Start -import io.iohk.ethereum.blockchain.sync.regular.{BlockBroadcast, BlockBroadcasterActor, BlockFetcher, BlockImporter, RegularSync} +import io.iohk.ethereum.blockchain.sync.regular.{ + BlockBroadcast, + BlockBroadcasterActor, + BlockFetcher, + BlockImporter, + RegularSync +} import io.iohk.ethereum.blockchain.sync.regular.RegularSync.NewCheckpoint import io.iohk.ethereum.blockchain.sync.{PeersClient, SyncProtocol} import io.iohk.ethereum.checkpointing.CheckpointingTestHelpers @@ -96,7 +102,8 @@ object RegularSyncItSpecUtils { broadcasterRef, pendingTransactionsManager, regularSync - )) + ) + ) lazy val regularSync = system.actorOf( RegularSync.props( diff --git a/src/main/scala/io/iohk/ethereum/blockchain/sync/regular/BlockFetcher.scala b/src/main/scala/io/iohk/ethereum/blockchain/sync/regular/BlockFetcher.scala index c034c614d7..adcf689614 100644 --- a/src/main/scala/io/iohk/ethereum/blockchain/sync/regular/BlockFetcher.scala +++ b/src/main/scala/io/iohk/ethereum/blockchain/sync/regular/BlockFetcher.scala @@ -98,6 +98,7 @@ class BlockFetcher( private def handleCommands(state: BlockFetcherState): Receive = { case PickBlocks(amount) => state.pickBlocks(amount) |> handlePickedBlocks(state) |> fetchBlocks + case StrictPickBlocks(from, atLeastWith) => // FIXME: Consider having StrictPickBlocks calls guaranteeing this // from parameter could be negative or 0 so we should cap it to 1 if that's the case @@ -113,9 +114,10 @@ class BlockFetcher( } fetchBlocks(newState) + case InvalidateBlocksFrom(blockNr, reason, withBlacklist) => val (blockProvider, newState) = state.invalidateBlocksFrom(blockNr, withBlacklist) - log.debug("Invalidate blocks from {}", blockNr) + log.info("Invalidate blocks from {}", blockNr) blockProvider.foreach(peersClient ! BlacklistPeer(_, reason)) fetchBlocks(newState) } @@ -124,7 +126,7 @@ class BlockFetcher( case Response(_, BlockHeaders(headers)) if state.isFetchingHeaders => val newState = if (state.fetchingHeadersState == AwaitingHeadersToBeIgnored) { - log.debug( + log.info( "Received {} headers starting from block {} that will be ignored", headers.size, headers.headOption.map(_.number) @@ -149,7 +151,7 @@ class BlockFetcher( fetchBlocks(newState) case RetryHeadersRequest if state.isFetchingHeaders => - log.debug("Something failed on a headers request, cancelling the request and re-fetching") + log.info("Something failed on a headers request, cancelling the request and re-fetching") val newState = state.withHeaderFetchReceived fetchBlocks(newState) @@ -157,9 +159,9 @@ class BlockFetcher( private def handleBodiesMessages(state: BlockFetcherState): Receive = { case Response(peer, BlockBodies(bodies)) if state.isFetchingBodies => - log.debug(s"Received ${bodies.size} block bodies") + log.debug("Received {} block bodies", bodies.size) if (state.fetchingBodiesState == AwaitingBodiesToBeIgnored) { - log.debug("Block bodies will be ignored due to an invalidation was requested for them") + log.info("Block bodies will be ignored due to an invalidation was requested for them") fetchBlocks(state.withBodiesFetchReceived) } else { val newState = @@ -171,19 +173,22 @@ class BlockFetcher( state.withBodiesFetchReceived.handleRequestedBlocks(newBlocks, peer.id) } val waitingHeadersDequeued = state.waitingHeaders.size - newState.waitingHeaders.size - log.debug(s"Processed $waitingHeadersDequeued new blocks from received block bodies") + log.debug("Processed {} new blocks from received block bodies", waitingHeadersDequeued) fetchBlocks(newState) } + case RetryBodiesRequest if state.isFetchingBodies => - log.debug("Something failed on a bodies request, cancelling the request and re-fetching") + log.info("Something failed on a bodies request, cancelling the request and re-fetching") val newState = state.withBodiesFetchReceived fetchBlocks(newState) } private def handleStateNodeMessages(state: BlockFetcherState): Receive = { case FetchStateNode(hash) => fetchStateNode(hash, sender(), state) + case RetryFetchStateNode if state.isFetchingStateNode => state.stateNodeFetcher.foreach(fetcher => fetchStateNode(fetcher.hash, fetcher.replyTo, state)) + case Response(peer, NodeData(values)) if state.isFetchingStateNode => log.debug("Received state node response from peer {}", peer) state.stateNodeFetcher.foreach(fetcher => { @@ -196,7 +201,7 @@ class BlockFetcher( validatedNode match { case Left(err) => - log.debug(err) + log.info(err) peersClient ! BlacklistPeer(peer.id, err) fetchStateNode(fetcher.hash, fetcher.replyTo, state) case Right(node) => @@ -215,10 +220,13 @@ class BlockFetcher( } supervisor ! ProgressProtocol.GotNewBlock(newState.knownTop) fetchBlocks(newState) + case MessageFromPeer(CommonMessages.NewBlock(block, _), peerId) => handleNewBlock(block, peerId, state) + case MessageFromPeer(PV64.NewBlock(block, _), peerId) => handleNewBlock(block, peerId, state) + case BlockImportFailed(blockNr, reason) => val (peerId, newState) = state.invalidateBlocksFrom(blockNr) peerId.foreach(id => peersClient ! BlacklistPeer(id, reason)) @@ -247,7 +255,7 @@ class BlockFetcher( } private def handleFutureBlock(block: Block, state: BlockFetcherState): Unit = { - log.debug("Ignoring received block as it doesn't match local state or fetch side is not on top") + log.info("Ignoring received block as it doesn't match local state or fetch side is not on top") val newState = state.withPossibleNewTopAt(block.number) supervisor ! ProgressProtocol.GotNewBlock(newState.knownTop) fetchBlocks(newState) @@ -259,8 +267,9 @@ class BlockFetcher( state.tryInsertBlock(block, peerId) match { case Left(_) if block.number <= state.lastBlock => log.debug( - s"Checkpoint block ${ByteStringUtils.hash2string(blockHash)} is older than current last block ${state.lastBlock}" + - s" - clearing the queues and putting checkpoint to ready blocks queue" + "Checkpoint block {} is older than current last block {} - clearing the queues and putting checkpoint to ready blocks queue", + ByteStringUtils.hash2string(blockHash), + state.lastBlock ) val newState = state .clearQueues() @@ -279,7 +288,7 @@ class BlockFetcher( log.debug(error) handleFutureBlock(block, state) case Right(state) => - log.debug(s"Checkpoint block [${ByteStringUtils.hash2string(blockHash)}] fit into queues") + log.debug("Checkpoint block [{}] fit into queues", ByteStringUtils.hash2string(blockHash)) fetchBlocks(state) } } @@ -289,20 +298,20 @@ class BlockFetcher( //ex. After a successful handshake, fetcher will receive the info about the header of the peer best block case MessageFromPeer(BlockHeaders(headers), _) => headers.lastOption.map { bh => - log.debug(s"Candidate for new top at block ${bh.number}, current known top ${state.knownTop}") + log.info("Candidate for new top at block {}, current known top {}", bh.number, state.knownTop) val newState = state.withPossibleNewTopAt(bh.number) fetchBlocks(newState) } //keep fetcher state updated in case new mined block was imported case InternalLastBlockImport(blockNr) => - log.debug(s"New mined block $blockNr imported from the inside") + log.info("New mined block {} imported from the inside", blockNr) val newState = state.withLastBlock(blockNr).withPossibleNewTopAt(blockNr) fetchBlocks(newState) //keep fetcher state updated in case new checkpoint block was imported case InternalCheckpointImport(blockNr) => - log.debug(s"New checkpoint block $blockNr imported from the inside") + log.info("New checkpoint block {} imported from the inside", blockNr) val newState = state .clearQueues() @@ -414,13 +423,16 @@ class BlockFetcher( private def handleRequestResult(fallback: FetchMsg)(msg: Any): Task[Any] = msg match { case failed: RequestFailed => - log.debug("Request failed due to {}", failed) + log.error("Request failed due to {}", failed) Task.now(fallback) + case NoSuitablePeer => Task.now(fallback).delayExecution(syncConfig.syncRetryInterval) + case Failure(cause) => log.error(cause, "Unexpected error on the request result") Task.now(fallback) + case m => Task.now(m) } diff --git a/src/main/scala/io/iohk/ethereum/blockchain/sync/regular/BlockImporter.scala b/src/main/scala/io/iohk/ethereum/blockchain/sync/regular/BlockImporter.scala index 0e719638c5..cb47c625d3 100644 --- a/src/main/scala/io/iohk/ethereum/blockchain/sync/regular/BlockImporter.scala +++ b/src/main/scala/io/iohk/ethereum/blockchain/sync/regular/BlockImporter.scala @@ -15,10 +15,12 @@ import io.iohk.ethereum.network.PeerId import io.iohk.ethereum.ommers.OmmersPool.AddOmmers import io.iohk.ethereum.transactions.PendingTransactionsManager import io.iohk.ethereum.transactions.PendingTransactionsManager.{AddUncheckedTransactions, RemoveTransactions} +import io.iohk.ethereum.utils.ByteStringUtils import io.iohk.ethereum.utils.Config.SyncConfig import io.iohk.ethereum.utils.FunctorOps._ import monix.eval.Task import monix.execution.Scheduler +import org.bouncycastle.util.encoders.Hex import scala.concurrent.duration._ @@ -207,7 +209,12 @@ class BlockImporter( tryImportBlocks(restOfBlocks, importedBlocks) case err @ (UnknownParent | BlockImportFailed(_)) => - log.error("Block {} import failed", blocks.head.number) + log.error( + "Block {} import failed, with hash {} and parent hash {}", + blocks.head.number, + blocks.head.header.hashAsHexString, + ByteStringUtils.hash2string(blocks.head.header.parentHash) + ) Task.now((importedBlocks, Some(err))) } .onErrorHandle { diff --git a/src/main/scala/io/iohk/ethereum/consensus/ConsensusBuilder.scala b/src/main/scala/io/iohk/ethereum/consensus/ConsensusBuilder.scala index f3f84622bf..1bf94498fd 100644 --- a/src/main/scala/io/iohk/ethereum/consensus/ConsensusBuilder.scala +++ b/src/main/scala/io/iohk/ethereum/consensus/ConsensusBuilder.scala @@ -2,7 +2,7 @@ package io.iohk.ethereum.consensus import io.iohk.ethereum.consensus.Protocol.{NoAdditionalEthashData, RestrictedEthashMinerData} import io.iohk.ethereum.consensus.ethash.EthashConsensus -import io.iohk.ethereum.consensus.ethash.validators.ValidatorsExecutor +import io.iohk.ethereum.consensus.validators.std.ValidatorsExecutor import io.iohk.ethereum.nodebuilder._ import io.iohk.ethereum.utils.{Config, Logger} diff --git a/src/main/scala/io/iohk/ethereum/consensus/ethash/EthashConsensus.scala b/src/main/scala/io/iohk/ethereum/consensus/ethash/EthashConsensus.scala index 1af3019a98..16e6cfe121 100644 --- a/src/main/scala/io/iohk/ethereum/consensus/ethash/EthashConsensus.scala +++ b/src/main/scala/io/iohk/ethereum/consensus/ethash/EthashConsensus.scala @@ -13,8 +13,8 @@ import io.iohk.ethereum.consensus.ethash.blocks.{ EthashBlockGeneratorImpl, RestrictedEthashBlockGeneratorImpl } -import io.iohk.ethereum.consensus.ethash.validators.ValidatorsExecutor import io.iohk.ethereum.consensus.validators.Validators +import io.iohk.ethereum.consensus.validators.std.ValidatorsExecutor import io.iohk.ethereum.domain.BlockchainImpl import io.iohk.ethereum.jsonrpc.AkkaTaskOps.TaskActorOps import io.iohk.ethereum.ledger.BlockPreparator @@ -22,6 +22,7 @@ import io.iohk.ethereum.ledger.Ledger.VMImpl import io.iohk.ethereum.nodebuilder.Node import io.iohk.ethereum.utils.{BlockchainConfig, Logger} import monix.eval.Task + import scala.concurrent.duration._ /** diff --git a/src/main/scala/io/iohk/ethereum/consensus/ethash/blocks/EthashBlockGenerator.scala b/src/main/scala/io/iohk/ethereum/consensus/ethash/blocks/EthashBlockGenerator.scala index cf1b908199..020d9255ee 100644 --- a/src/main/scala/io/iohk/ethereum/consensus/ethash/blocks/EthashBlockGenerator.scala +++ b/src/main/scala/io/iohk/ethereum/consensus/ethash/blocks/EthashBlockGenerator.scala @@ -5,12 +5,12 @@ import akka.util.ByteString import io.iohk.ethereum.consensus.ConsensusConfig import io.iohk.ethereum.consensus.blocks._ import io.iohk.ethereum.consensus.difficulty.DifficultyCalculator -import io.iohk.ethereum.consensus.ethash.validators.ValidatorsExecutor import io.iohk.ethereum.crypto.kec256 import io.iohk.ethereum.domain._ import io.iohk.ethereum.ledger.{BlockPreparator, InMemoryWorldStateProxy} import io.iohk.ethereum.utils.BlockchainConfig import io.iohk.ethereum.consensus.ConsensusMetrics +import io.iohk.ethereum.consensus.validators.std.ValidatorsExecutor /** Internal API, used for testing (especially mocks) */ trait EthashBlockGenerator extends TestBlockGenerator { diff --git a/src/main/scala/io/iohk/ethereum/consensus/ethash/blocks/RestrictedEthashBlockGeneratorImpl.scala b/src/main/scala/io/iohk/ethereum/consensus/ethash/blocks/RestrictedEthashBlockGeneratorImpl.scala index 387e54b3f5..7b8f16d0d3 100644 --- a/src/main/scala/io/iohk/ethereum/consensus/ethash/blocks/RestrictedEthashBlockGeneratorImpl.scala +++ b/src/main/scala/io/iohk/ethereum/consensus/ethash/blocks/RestrictedEthashBlockGeneratorImpl.scala @@ -4,12 +4,12 @@ import io.iohk.ethereum.consensus.ConsensusConfig import io.iohk.ethereum.consensus.blocks.{BlockTimestampProvider, DefaultBlockTimestampProvider, PendingBlockAndState} import io.iohk.ethereum.consensus.difficulty.DifficultyCalculator import io.iohk.ethereum.consensus.ethash.RestrictedEthashSigner -import io.iohk.ethereum.consensus.ethash.validators.ValidatorsExecutor import io.iohk.ethereum.domain.{Address, Block, Blockchain, SignedTransaction} import io.iohk.ethereum.ledger.{BlockPreparator, InMemoryWorldStateProxy} import io.iohk.ethereum.utils.BlockchainConfig import org.bouncycastle.crypto.AsymmetricCipherKeyPair import io.iohk.ethereum.consensus.ConsensusMetrics +import io.iohk.ethereum.consensus.validators.std.ValidatorsExecutor class RestrictedEthashBlockGeneratorImpl( validators: ValidatorsExecutor, diff --git a/src/main/scala/io/iohk/ethereum/consensus/ethash/validators/OmmersValidator.scala b/src/main/scala/io/iohk/ethereum/consensus/ethash/validators/OmmersValidator.scala index 271a6be2e8..c4e1c9be47 100644 --- a/src/main/scala/io/iohk/ethereum/consensus/ethash/validators/OmmersValidator.scala +++ b/src/main/scala/io/iohk/ethereum/consensus/ethash/validators/OmmersValidator.scala @@ -2,6 +2,7 @@ package io.iohk.ethereum.consensus.ethash.validators import akka.util.ByteString import io.iohk.ethereum.consensus.ethash.validators.OmmersValidator.{OmmersError, OmmersValid} +import io.iohk.ethereum.consensus.validators.BlockHeaderError import io.iohk.ethereum.consensus.{GetBlockHeaderByHash, GetNBlocksBack} import io.iohk.ethereum.domain.{Block, BlockHeader, Blockchain} @@ -36,9 +37,10 @@ object OmmersValidator { object OmmersError { case object OmmersLengthError extends OmmersError - case object OmmersNotValidError extends OmmersError + case class OmmersHeaderError(error: Option[BlockHeaderError]) extends OmmersError case object OmmersUsedBeforeError extends OmmersError - case object OmmersAncestorsError extends OmmersError + case object OmmerIsAncestorError extends OmmersError + case object OmmerParentIsNotAncestorError extends OmmersError case object OmmersDuplicatedError extends OmmersError } diff --git a/src/main/scala/io/iohk/ethereum/consensus/ethash/validators/StdOmmersValidator.scala b/src/main/scala/io/iohk/ethereum/consensus/validators/std/StdOmmersValidator.scala similarity index 71% rename from src/main/scala/io/iohk/ethereum/consensus/ethash/validators/StdOmmersValidator.scala rename to src/main/scala/io/iohk/ethereum/consensus/validators/std/StdOmmersValidator.scala index 5a2fd5907d..f75a95c7b5 100644 --- a/src/main/scala/io/iohk/ethereum/consensus/ethash/validators/StdOmmersValidator.scala +++ b/src/main/scala/io/iohk/ethereum/consensus/validators/std/StdOmmersValidator.scala @@ -1,15 +1,14 @@ -package io.iohk.ethereum.consensus.ethash.validators +package io.iohk.ethereum.consensus.validators.std import akka.util.ByteString +import io.iohk.ethereum.consensus.ethash.validators.OmmersValidator import io.iohk.ethereum.consensus.ethash.validators.OmmersValidator.OmmersError._ import io.iohk.ethereum.consensus.ethash.validators.OmmersValidator.{OmmersError, OmmersValid} import io.iohk.ethereum.consensus.validators.BlockHeaderValidator import io.iohk.ethereum.consensus.{GetBlockHeaderByHash, GetNBlocksBack} import io.iohk.ethereum.domain.BlockHeader -import io.iohk.ethereum.utils.BlockchainConfig -class StdOmmersValidator(blockchainConfig: BlockchainConfig, blockHeaderValidator: BlockHeaderValidator) - extends OmmersValidator { +class StdOmmersValidator(blockHeaderValidator: BlockHeaderValidator) extends OmmersValidator { val OmmerGenerationLimit: Int = 6 // Stated on section 11.1, eq. (143) of the YP val OmmerSizeLimit: Int = 2 @@ -30,7 +29,8 @@ class StdOmmersValidator(blockchainConfig: BlockchainConfig, blockHeaderValidato * @param getBlockHeaderByHash function to obtain an ancestor block header by hash * @param getNBlocksBack function to obtain N blocks including one given by hash and its N-1 ancestors * - * @return [[OmmersValidator.OmmersValid]] if valid, an [[OmmersValidator.OmmersError]] otherwise + * @return [[io.iohk.ethereum.consensus.ethash.validators.OmmersValidator.OmmersValid]] if valid, + * an [[io.iohk.ethereum.consensus.ethash.validators.OmmersValidator.OmmersError]] otherwise */ def validate( parentHash: ByteString, @@ -66,16 +66,22 @@ class StdOmmersValidator(blockchainConfig: BlockchainConfig, blockHeaderValidato /** Validates that each ommer's header is valid based on validations stated in section 11.1 of the YP * * @param ommers the list of ommers to validate - * @param getBlockHeaderByHash function to obtain ommers' parents - * - * @return [[OmmersValidator.OmmersValid]] if valid, an [[OmmersValidator.OmmersError.OmmersNotValidError]] otherwise + * @param getBlockParentsHeaderByHash function to obtain ommers' parents + * @return [[OmmersValidator.OmmersValid]] if valid, an [[OmmersValidator.OmmersError.OmmersHeaderError]] otherwise */ private def validateOmmersHeaders( ommers: Seq[BlockHeader], - getBlockHeaderByHash: GetBlockHeaderByHash + getBlockParentsHeaderByHash: GetBlockHeaderByHash ): Either[OmmersError, OmmersValid] = { - if (ommers.forall(blockHeaderValidator.validate(_, getBlockHeaderByHash).isRight)) Right(OmmersValid) - else Left(OmmersNotValidError) + val validationsResult = ommers.map(blockHeaderValidator.validate(_, getBlockParentsHeaderByHash)) + + if (validationsResult.forall(_.isRight)) Right(OmmersValid) + else { + validationsResult.filterNot(_.isRight) match { + case Left(headerError) :: _ => Left(OmmersHeaderError(Some(headerError))) + case _ => Left(OmmersHeaderError(None)) + } + } } /** Validates that each ommer is not too old and that it is a sibling as one of the current block's ancestors @@ -85,26 +91,28 @@ class StdOmmersValidator(blockchainConfig: BlockchainConfig, blockHeaderValidato * @param blockNumber the number of the block to which the ommers belong * @param ommers the list of ommers to validate * @param getNBlocksBack from where the ommers' parents will be obtained - * - * @return [[OmmersValidator.OmmersValid]] if valid, an [[OmmersValidator.OmmersError.OmmersUsedBeforeError]] otherwise + * @return [[OmmersValidator.OmmersValid]] if valid, an [[OmmersValidator.OmmersError.OmmerIsAncestorError]] otherwise */ - private def validateOmmersAncestors( + private[std] def validateOmmersAncestors( parentHash: ByteString, blockNumber: BigInt, ommers: Seq[BlockHeader], getNBlocksBack: GetNBlocksBack ): Either[OmmersError, OmmersValid] = { - val ancestorsOpt = collectAncestors(parentHash, blockNumber, getNBlocksBack) + val ancestors = collectAncestors(parentHash, blockNumber, getNBlocksBack) + lazy val ommersHashes: Seq[ByteString] = ommers.map(_.hash) + lazy val ommersThatAreAncestors: Seq[ByteString] = ancestors.map(_.hash).intersect(ommersHashes) - val validOmmersAncestors: Seq[BlockHeader] => Boolean = ancestors => - ommers.forall { ommer => - val ommerIsNotAncestor = ancestors.forall(_.hash != ommer.hash) - val ommersParentIsAncestor = ancestors.exists(_.parentHash == ommer.parentHash) - ommerIsNotAncestor && ommersParentIsAncestor - } - if (ancestorsOpt.exists(validOmmersAncestors)) Right(OmmersValid) - else Left(OmmersAncestorsError) + lazy val ancestorsParents: Seq[ByteString] = ancestors.map(_.parentHash) + lazy val ommersParentsHashes: Seq[ByteString] = ommers.map(_.parentHash) + + // parent not an ancestor or is too old (we only compare up to 6 previous ancestors) + lazy val ommersParentsAreAllAncestors: Boolean = ommersParentsHashes.forall(ancestorsParents.contains) + + if (ommersThatAreAncestors.nonEmpty) Left(OmmerIsAncestorError) + else if (!ommersParentsAreAllAncestors) Left(OmmerParentIsNotAncestorError) + else Right(OmmersValid) } /** Validates that each ommer was not previously used @@ -123,9 +131,9 @@ class StdOmmersValidator(blockchainConfig: BlockchainConfig, blockHeaderValidato getNBlocksBack: GetNBlocksBack ): Either[OmmersError, OmmersValid] = { - val ommersFromAncestorsOpt = collectOmmersFromAncestors(parentHash, blockNumber, getNBlocksBack) + val ommersFromAncestors = collectOmmersFromAncestors(parentHash, blockNumber, getNBlocksBack) - if (ommersFromAncestorsOpt.exists(ommers.intersect(_).isEmpty)) Right(OmmersValid) + if (ommers.intersect(ommersFromAncestors).isEmpty) Right(OmmersValid) else Left(OmmersUsedBeforeError) } @@ -144,19 +152,17 @@ class StdOmmersValidator(blockchainConfig: BlockchainConfig, blockHeaderValidato parentHash: ByteString, blockNumber: BigInt, getNBlocksBack: GetNBlocksBack - ): Option[Seq[BlockHeader]] = { + ): Seq[BlockHeader] = { val numberOfBlocks = blockNumber.min(OmmerGenerationLimit).toInt - val ancestors = getNBlocksBack(parentHash, numberOfBlocks).map(_.header) - Some(ancestors).filter(_.length == numberOfBlocks) + getNBlocksBack(parentHash, numberOfBlocks).map(_.header) } private def collectOmmersFromAncestors( parentHash: ByteString, blockNumber: BigInt, getNBlocksBack: GetNBlocksBack - ): Option[Seq[BlockHeader]] = { + ): Seq[BlockHeader] = { val numberOfBlocks = blockNumber.min(OmmerGenerationLimit).toInt - val ancestors = getNBlocksBack(parentHash, numberOfBlocks).map(_.body.uncleNodesList) - Some(ancestors).filter(_.length == numberOfBlocks).map(_.flatten) + getNBlocksBack(parentHash, numberOfBlocks).flatMap(_.body.uncleNodesList) } } diff --git a/src/main/scala/io/iohk/ethereum/consensus/validators/std/StdValidators.scala b/src/main/scala/io/iohk/ethereum/consensus/validators/std/StdValidators.scala index e1cf035bec..f06d9bd21f 100644 --- a/src/main/scala/io/iohk/ethereum/consensus/validators/std/StdValidators.scala +++ b/src/main/scala/io/iohk/ethereum/consensus/validators/std/StdValidators.scala @@ -12,7 +12,7 @@ import org.bouncycastle.util.encoders.Hex * Implements validators that adhere to the original [[io.iohk.ethereum.consensus.validators.Validators Validators]] * interface. * - * @see [[io.iohk.ethereum.consensus.ethash.validators.StdValidatorsExecutor StdEthashValidators]] + * @see [[StdValidatorsExecutor StdEthashValidators]] * for the PoW-specific counterpart. */ final class StdValidators( diff --git a/src/main/scala/io/iohk/ethereum/consensus/ethash/validators/StdValidatorsExecutor.scala b/src/main/scala/io/iohk/ethereum/consensus/validators/std/StdValidatorsExecutor.scala similarity index 68% rename from src/main/scala/io/iohk/ethereum/consensus/ethash/validators/StdValidatorsExecutor.scala rename to src/main/scala/io/iohk/ethereum/consensus/validators/std/StdValidatorsExecutor.scala index 4e522ac0ef..7b67bab94d 100644 --- a/src/main/scala/io/iohk/ethereum/consensus/ethash/validators/StdValidatorsExecutor.scala +++ b/src/main/scala/io/iohk/ethereum/consensus/validators/std/StdValidatorsExecutor.scala @@ -1,13 +1,14 @@ -package io.iohk.ethereum.consensus.ethash.validators +package io.iohk.ethereum.consensus.validators.std +import io.iohk.ethereum.consensus.ethash.validators.OmmersValidator import io.iohk.ethereum.consensus.validators.{BlockHeaderValidator, BlockValidator, SignedTransactionValidator} /** * Implements validators that adhere to the PoW-specific - * [[io.iohk.ethereum.consensus.ethash.validators.ValidatorsExecutor]] + * [[ValidatorsExecutor]] * interface. */ -final class StdValidatorsExecutor private[validators] ( +final class StdValidatorsExecutor private[std] ( val blockValidator: BlockValidator, val blockHeaderValidator: BlockHeaderValidator, val signedTransactionValidator: SignedTransactionValidator, diff --git a/src/main/scala/io/iohk/ethereum/consensus/ethash/validators/ValidatorsExecutor.scala b/src/main/scala/io/iohk/ethereum/consensus/validators/std/ValidatorsExecutor.scala similarity index 89% rename from src/main/scala/io/iohk/ethereum/consensus/ethash/validators/ValidatorsExecutor.scala rename to src/main/scala/io/iohk/ethereum/consensus/validators/std/ValidatorsExecutor.scala index cae57e80a2..f867c7e4ff 100644 --- a/src/main/scala/io/iohk/ethereum/consensus/ethash/validators/ValidatorsExecutor.scala +++ b/src/main/scala/io/iohk/ethereum/consensus/validators/std/ValidatorsExecutor.scala @@ -1,9 +1,14 @@ -package io.iohk.ethereum.consensus -package ethash.validators +package io.iohk.ethereum.consensus.validators.std import akka.util.ByteString -import io.iohk.ethereum.consensus.validators.std.{StdBlockValidator, StdSignedTransactionValidator, StdValidators} +import io.iohk.ethereum.consensus.ethash.validators.{ + EthashBlockHeaderValidator, + MockedPowBlockHeaderValidator, + OmmersValidator, + RestrictedEthashBlockHeaderValidator +} import io.iohk.ethereum.consensus.validators.{BlockHeaderValidator, Validators} +import io.iohk.ethereum.consensus.{GetBlockHeaderByHash, GetNBlocksBack, Protocol} import io.iohk.ethereum.domain.{Block, Receipt} import io.iohk.ethereum.ledger.BlockExecutionError.ValidationBeforeExecError import io.iohk.ethereum.ledger.{BlockExecutionError, BlockExecutionSuccess} @@ -55,7 +60,7 @@ object ValidatorsExecutor { StdBlockValidator, blockHeaderValidator, new StdSignedTransactionValidator(blockchainConfig), - new StdOmmersValidator(blockchainConfig, blockHeaderValidator) + new StdOmmersValidator(blockHeaderValidator) ) } @@ -66,7 +71,7 @@ object ValidatorsExecutor { StdBlockValidator, blockHeaderValidator, new StdSignedTransactionValidator(blockchainConfig), - new StdOmmersValidator(blockchainConfig, blockHeaderValidator) + new StdOmmersValidator(blockHeaderValidator) ) } diff --git a/src/main/scala/io/iohk/ethereum/ledger/Ledger.scala b/src/main/scala/io/iohk/ethereum/ledger/Ledger.scala index dd063b34f5..65e4b6860f 100644 --- a/src/main/scala/io/iohk/ethereum/ledger/Ledger.scala +++ b/src/main/scala/io/iohk/ethereum/ledger/Ledger.scala @@ -137,13 +137,20 @@ class LedgerImpl( importResult.foreach(measureBlockMetrics) importResult case None => - log.error(s"Getting total difficulty for current best block with hash: $hash failed") - Task.now(BlockImportFailed(s"Couldn't get total difficulty for current best block with hash: $hash")) + log.error( + "Getting total difficulty for current best block with hash: {} failed", + bestBlock.header.hashAsHexString + ) + Task.now( + BlockImportFailed( + "Couldn't get total difficulty for current best block with hash: ${bestBlock.header.hashAsHexString}" + ) + ) } } case None => log.error("Getting current best block failed") - Task.now(BlockImportFailed(s"Couldn't find the current best block")) + Task.now(BlockImportFailed("Couldn't find the current best block")) } private def isBlockADuplicate(block: BlockHeader, currentBestBlockNumber: BigInt): Boolean = { diff --git a/src/test/scala/io/iohk/ethereum/Mocks.scala b/src/test/scala/io/iohk/ethereum/Mocks.scala index c2c577e4ca..3a30ac7177 100644 --- a/src/test/scala/io/iohk/ethereum/Mocks.scala +++ b/src/test/scala/io/iohk/ethereum/Mocks.scala @@ -2,12 +2,13 @@ package io.iohk.ethereum import akka.util.ByteString import cats.data.NonEmptyList -import io.iohk.ethereum.consensus.ethash.validators.OmmersValidator.OmmersError.OmmersNotValidError +import io.iohk.ethereum.consensus.ethash.validators.OmmersValidator.OmmersError.OmmersHeaderError import io.iohk.ethereum.consensus.ethash.validators.OmmersValidator.OmmersValid -import io.iohk.ethereum.consensus.ethash.validators.{OmmersValidator, ValidatorsExecutor} -import io.iohk.ethereum.consensus.validators.BlockHeaderError.HeaderNumberError +import io.iohk.ethereum.consensus.ethash.validators.OmmersValidator +import io.iohk.ethereum.consensus.validators.BlockHeaderError.{HeaderDifficultyError, HeaderNumberError} import io.iohk.ethereum.consensus.validators._ import io.iohk.ethereum.consensus.validators.std.StdBlockValidator.{BlockError, BlockTransactionsHashError, BlockValid} +import io.iohk.ethereum.consensus.validators.std.ValidatorsExecutor import io.iohk.ethereum.consensus.{Consensus, GetBlockHeaderByHash, GetNBlocksBack} import io.iohk.ethereum.domain._ import io.iohk.ethereum.ledger.BlockExecutionError.ValidationAfterExecError @@ -103,7 +104,7 @@ object Mocks { override val ommersValidator: OmmersValidator = (_: ByteString, _: BigInt, _: Seq[BlockHeader], _: GetBlockHeaderByHash, _: GetNBlocksBack) => - Left(OmmersNotValidError) + Left(OmmersHeaderError(Some(HeaderDifficultyError))) override val blockValidator: BlockValidator = new BlockValidator { override def validateHeaderAndBody(blockHeader: BlockHeader, blockBody: BlockBody) = Left( diff --git a/src/test/scala/io/iohk/ethereum/blockchain/sync/ScenarioSetup.scala b/src/test/scala/io/iohk/ethereum/blockchain/sync/ScenarioSetup.scala index f90ca004b3..a386d86c4b 100644 --- a/src/test/scala/io/iohk/ethereum/blockchain/sync/ScenarioSetup.scala +++ b/src/test/scala/io/iohk/ethereum/blockchain/sync/ScenarioSetup.scala @@ -2,16 +2,18 @@ package io.iohk.ethereum.blockchain.sync import io.iohk.ethereum.Mocks import io.iohk.ethereum.Mocks.MockVM -import io.iohk.ethereum.consensus.ethash.validators.ValidatorsExecutor import io.iohk.ethereum.consensus.validators.Validators +import io.iohk.ethereum.consensus.validators.std.ValidatorsExecutor import io.iohk.ethereum.consensus.{Consensus, Protocol, StdTestConsensusBuilder, TestConsensus} import io.iohk.ethereum.domain.BlockchainImpl import io.iohk.ethereum.ledger.Ledger.VMImpl import io.iohk.ethereum.ledger.LedgerImpl import io.iohk.ethereum.nodebuilder._ import io.iohk.ethereum.utils.BlockchainConfig + import java.util.concurrent.Executors import monix.execution.Scheduler + import scala.concurrent.ExecutionContext /** diff --git a/src/test/scala/io/iohk/ethereum/consensus/BlockGeneratorSpec.scala b/src/test/scala/io/iohk/ethereum/consensus/blocks/BlockGeneratorSpec.scala similarity index 99% rename from src/test/scala/io/iohk/ethereum/consensus/BlockGeneratorSpec.scala rename to src/test/scala/io/iohk/ethereum/consensus/blocks/BlockGeneratorSpec.scala index 73084158ed..093e883c6e 100644 --- a/src/test/scala/io/iohk/ethereum/consensus/BlockGeneratorSpec.scala +++ b/src/test/scala/io/iohk/ethereum/consensus/blocks/BlockGeneratorSpec.scala @@ -1,11 +1,10 @@ -package io.iohk.ethereum.consensus +package io.iohk.ethereum.consensus.blocks import akka.util.ByteString import io.iohk.ethereum.blockchain.data.GenesisDataLoader import io.iohk.ethereum.blockchain.sync.EphemBlockchainTestSetup -import io.iohk.ethereum.consensus.blocks.BlockTimestampProvider -import io.iohk.ethereum.consensus.ethash.validators.ValidatorsExecutor import io.iohk.ethereum.consensus.validators._ +import io.iohk.ethereum.consensus.validators.std.ValidatorsExecutor import io.iohk.ethereum.crypto import io.iohk.ethereum.crypto._ import io.iohk.ethereum.domain.BlockHeader.HeaderExtraFields @@ -15,7 +14,6 @@ import io.iohk.ethereum.domain._ import io.iohk.ethereum.ledger.{BlockExecution, BlockQueue, BlockValidation} import io.iohk.ethereum.mpt.MerklePatriciaTrie.MPTException import io.iohk.ethereum.utils._ -import java.time.Instant import monix.execution.Scheduler import org.bouncycastle.crypto.AsymmetricCipherKeyPair import org.bouncycastle.crypto.params.ECPublicKeyParameters @@ -24,6 +22,8 @@ import org.scalatest.flatspec.AnyFlatSpec import org.scalatest.matchers.should.Matchers import org.scalatestplus.scalacheck.ScalaCheckPropertyChecks +import java.time.Instant + class BlockGeneratorSpec extends AnyFlatSpec with Matchers with ScalaCheckPropertyChecks with Logger { implicit val testContext = Scheduler.fixedPool("block-generator-spec-pool", 4) diff --git a/src/test/scala/io/iohk/ethereum/consensus/ethash/EthashMinerSpec.scala b/src/test/scala/io/iohk/ethereum/consensus/ethash/EthashMinerSpec.scala index ac43c9c80b..1a07ec6759 100644 --- a/src/test/scala/io/iohk/ethereum/consensus/ethash/EthashMinerSpec.scala +++ b/src/test/scala/io/iohk/ethereum/consensus/ethash/EthashMinerSpec.scala @@ -49,7 +49,8 @@ class EthashMinerSpec it should "mine valid blocks on the end of the epoch" taggedAs EthashMinerSpecTag in new TestSetup { val epochLength: Int = EthashUtils.EPOCH_LENGTH_BEFORE_ECIP_1099 - val parentBlockNumber: Int = 2 * epochLength - 2 // 59998, mined block will be 59999 (last block of the current epoch) + val parentBlockNumber: Int = + 2 * epochLength - 2 // 59998, mined block will be 59999 (last block of the current epoch) val parentBlock: Block = origin.copy(header = origin.header.copy(number = parentBlockNumber)) val bfm: Block = blockForMining(parentBlock.header) @@ -84,8 +85,9 @@ class EthashMinerSpec ommersPool = ommersPool.ref ) - val miner: TestActorRef[Nothing] = TestActorRef(EthashMiner.props(blockchain, blockCreator, sync.ref, ethService, ethMiningService)) - + val miner: TestActorRef[Nothing] = TestActorRef( + EthashMiner.props(blockchain, blockCreator, sync.ref, ethService, ethMiningService) + ) protected def executeTest(parentBlock: Block, blockForMining: Block): Unit = { prepareMocks(parentBlock, blockForMining) diff --git a/src/test/scala/io/iohk/ethereum/consensus/validators/BlockHeaderValidatorSpec.scala b/src/test/scala/io/iohk/ethereum/consensus/ethash/validators/EthashBlockHeaderValidatorSpec.scala similarity index 99% rename from src/test/scala/io/iohk/ethereum/consensus/validators/BlockHeaderValidatorSpec.scala rename to src/test/scala/io/iohk/ethereum/consensus/ethash/validators/EthashBlockHeaderValidatorSpec.scala index f930a58d26..375c75213f 100644 --- a/src/test/scala/io/iohk/ethereum/consensus/validators/BlockHeaderValidatorSpec.scala +++ b/src/test/scala/io/iohk/ethereum/consensus/ethash/validators/EthashBlockHeaderValidatorSpec.scala @@ -1,26 +1,25 @@ -package io.iohk.ethereum.consensus.validators +package io.iohk.ethereum.consensus.ethash.validators import akka.util.ByteString import io.iohk.ethereum.blockchain.sync.EphemBlockchainTestSetup import io.iohk.ethereum.consensus.difficulty.DifficultyCalculator import io.iohk.ethereum.consensus.ethash.difficulty.EthashDifficultyCalculator -import io.iohk.ethereum.consensus.ethash.validators.EthashBlockHeaderValidator import io.iohk.ethereum.consensus.validators.BlockHeaderError._ import io.iohk.ethereum.consensus.validators.BlockHeaderValidator._ +import io.iohk.ethereum.consensus.validators.{BlockHeaderError, BlockHeaderValid, BlockHeaderValidatorSkeleton} +import io.iohk.ethereum.domain.BlockHeader.HeaderExtraFields._ import io.iohk.ethereum.domain.{UInt256, _} import io.iohk.ethereum.utils.{BlockchainConfig, DaoForkConfig} -import io.iohk.ethereum.{Fixtures, ObjectGenerators} +import io.iohk.ethereum.{Fixtures, ObjectGenerators, SuperSlow} import org.bouncycastle.util.encoders.Hex import org.scalamock.scalatest.MockFactory -import org.scalatestplus.scalacheck.ScalaCheckPropertyChecks import org.scalatest.flatspec.AnyFlatSpec import org.scalatest.matchers.should.Matchers -import io.iohk.ethereum.domain.BlockHeader.HeaderExtraFields._ -import io.iohk.ethereum.SuperSlow import org.scalatest.prop.TableFor4 +import org.scalatestplus.scalacheck.ScalaCheckPropertyChecks // scalastyle:off magic.number -class BlockHeaderValidatorSpec +class EthashBlockHeaderValidatorSpec extends AnyFlatSpec with Matchers with ScalaCheckPropertyChecks diff --git a/src/test/scala/io/iohk/ethereum/consensus/validators/RestrictedEthashBlockHeaderValidatorSpec.scala b/src/test/scala/io/iohk/ethereum/consensus/ethash/validators/RestrictedEthashBlockHeaderValidatorSpec.scala similarity index 98% rename from src/test/scala/io/iohk/ethereum/consensus/validators/RestrictedEthashBlockHeaderValidatorSpec.scala rename to src/test/scala/io/iohk/ethereum/consensus/ethash/validators/RestrictedEthashBlockHeaderValidatorSpec.scala index 770e3a2c1d..c5c37e024e 100644 --- a/src/test/scala/io/iohk/ethereum/consensus/validators/RestrictedEthashBlockHeaderValidatorSpec.scala +++ b/src/test/scala/io/iohk/ethereum/consensus/ethash/validators/RestrictedEthashBlockHeaderValidatorSpec.scala @@ -1,9 +1,9 @@ -package io.iohk.ethereum.consensus.validators +package io.iohk.ethereum.consensus.ethash.validators import akka.util.ByteString import io.iohk.ethereum.consensus.ethash.RestrictedEthashSigner -import io.iohk.ethereum.consensus.ethash.validators.RestrictedEthashBlockHeaderValidator import io.iohk.ethereum.consensus.validators.BlockHeaderError.{HeaderPoWError, RestrictedEthashHeaderExtraDataError} +import io.iohk.ethereum.consensus.validators.BlockHeaderValid import io.iohk.ethereum.crypto import io.iohk.ethereum.crypto.ECDSASignature import io.iohk.ethereum.domain.{Address, BlockHeader, UInt256} diff --git a/src/test/scala/io/iohk/ethereum/consensus/validators/BlockValidatorSpec.scala b/src/test/scala/io/iohk/ethereum/consensus/validators/std/StdBlockValidatorSpec.scala similarity index 97% rename from src/test/scala/io/iohk/ethereum/consensus/validators/BlockValidatorSpec.scala rename to src/test/scala/io/iohk/ethereum/consensus/validators/std/StdBlockValidatorSpec.scala index ad6b473590..759521c824 100644 --- a/src/test/scala/io/iohk/ethereum/consensus/validators/BlockValidatorSpec.scala +++ b/src/test/scala/io/iohk/ethereum/consensus/validators/std/StdBlockValidatorSpec.scala @@ -1,19 +1,18 @@ -package io.iohk.ethereum.consensus.validators +package io.iohk.ethereum.consensus.validators.std import akka.util.ByteString import io.iohk.ethereum.checkpointing.CheckpointingTestHelpers import io.iohk.ethereum.consensus.blocks.CheckpointBlockGenerator -import io.iohk.ethereum.consensus.validators.std.StdBlockValidator import io.iohk.ethereum.consensus.validators.std.StdBlockValidator._ import io.iohk.ethereum.crypto import io.iohk.ethereum.domain._ -import io.iohk.ethereum.security.SecureRandomBuilder import io.iohk.ethereum.ledger.BloomFilter +import io.iohk.ethereum.security.SecureRandomBuilder import org.bouncycastle.util.encoders.Hex import org.scalatest.flatspec.AnyFlatSpec import org.scalatest.matchers.should.Matchers -class BlockValidatorSpec extends AnyFlatSpec with Matchers with SecureRandomBuilder { +class StdBlockValidatorSpec extends AnyFlatSpec with Matchers with SecureRandomBuilder { "Block" should "created based on valid data" in { val block = Block(validBlockHeader, validBlockBody) diff --git a/src/test/scala/io/iohk/ethereum/consensus/validators/OmmersValidatorSpec.scala b/src/test/scala/io/iohk/ethereum/consensus/validators/std/StdOmmersValidatorSpec.scala similarity index 89% rename from src/test/scala/io/iohk/ethereum/consensus/validators/OmmersValidatorSpec.scala rename to src/test/scala/io/iohk/ethereum/consensus/validators/std/StdOmmersValidatorSpec.scala index 11de518411..e0774d9c40 100644 --- a/src/test/scala/io/iohk/ethereum/consensus/validators/OmmersValidatorSpec.scala +++ b/src/test/scala/io/iohk/ethereum/consensus/validators/std/StdOmmersValidatorSpec.scala @@ -1,17 +1,17 @@ -package io.iohk.ethereum.consensus.validators +package io.iohk.ethereum.consensus.validators.std import akka.util.ByteString import io.iohk.ethereum.ObjectGenerators import io.iohk.ethereum.blockchain.sync.EphemBlockchainTestSetup +import io.iohk.ethereum.consensus.ethash.validators.EthashBlockHeaderValidator import io.iohk.ethereum.consensus.ethash.validators.OmmersValidator.OmmersError._ -import io.iohk.ethereum.consensus.ethash.validators.{EthashBlockHeaderValidator, StdOmmersValidator} import io.iohk.ethereum.domain.{Block, BlockBody, BlockHeader} import org.bouncycastle.util.encoders.Hex -import org.scalatestplus.scalacheck.ScalaCheckPropertyChecks import org.scalatest.flatspec.AnyFlatSpec import org.scalatest.matchers.should.Matchers +import org.scalatestplus.scalacheck.ScalaCheckPropertyChecks -class OmmersValidatorSpec extends AnyFlatSpec with Matchers with ScalaCheckPropertyChecks with ObjectGenerators { +class StdOmmersValidatorSpec extends AnyFlatSpec with Matchers with ScalaCheckPropertyChecks with ObjectGenerators { it should "validate correctly a valid list of ommers" in new BlockUtils { ommersValidator.validate(ommersBlockParentHash, ommersBlockNumber, ommers, blockchain) match { @@ -32,7 +32,7 @@ class OmmersValidatorSpec extends AnyFlatSpec with Matchers with ScalaCheckPrope val invalidOmmer1: BlockHeader = ommer1.copy(number = ommer1.number + 1) ommersValidator.validate(ommersBlockParentHash, ommersBlockNumber, Seq(invalidOmmer1, ommer2), blockchain) match { - case Left(OmmersNotValidError) => succeed + case Left(OmmersHeaderError(Some(_))) => succeed case Left(err) => fail(s"Unexpected validation error: $err") case Right(_) => fail("Unexpected validation success") } @@ -51,22 +51,37 @@ class OmmersValidatorSpec extends AnyFlatSpec with Matchers with ScalaCheckPrope } } - it should "report a failure if there is an ommer which is of the last ancestors" in new BlockUtils { + it should "report a failure if there is an ommer which is also an ancestor" in new BlockUtils { ommersValidator.validate(ommersBlockParentHash, ommersBlockNumber, Seq(ommer1, block92.header), blockchain) match { - case Left(OmmersAncestorsError) => succeed + case Left(OmmerIsAncestorError) => succeed case Left(err) => fail(s"Unexpected validation error: $err") case Right(_) => fail("Unexpected validation success") } } - it should "report a failure if there is an ommer too old" in new BlockUtils { + it should "report a failure if there is an ommer which that is not parent of an ancestor" in new BlockUtils { + val getNBlocksBack: (ByteString, Int) => List[Block] = + (_, n) => ((ommersBlockNumber - n) until ommersBlockNumber).toList.flatMap(blockchain.getBlockByNumber) + + ommersValidator.validateOmmersAncestors( + ommersBlockParentHash, + ommersBlockNumber, + Seq(ommer1, ommerInvalidBranch), + getNBlocksBack + ) match { + case Left(OmmerParentIsNotAncestorError) => succeed + case err => fail(err.toString) + } + } + + it should "report a failure if there is an ommer that is too old" in new BlockUtils { ommersValidator.validate(ommersBlockParentHash, ommersBlockNumber, Seq(ommer1, block90.header), blockchain) match { - case Left(OmmersAncestorsError) => succeed - case _ => fail() + case Left(OmmerParentIsNotAncestorError) => succeed + case err => fail(err.toString) } } - it should "report a failure if there is a duplicated ommer in the ommer list" in new BlockUtils { + it should "report a failure if there is a duplicated ommer in the ommers list" in new BlockUtils { ommersValidator.validate(ommersBlockParentHash, ommersBlockNumber, Seq(ommer1, ommer1), blockchain) match { case Left(OmmersDuplicatedError) => succeed case Left(err) => fail(s"Unexpected validation error: $err") @@ -77,7 +92,29 @@ class OmmersValidatorSpec extends AnyFlatSpec with Matchers with ScalaCheckPrope // scalastyle:off magic.number trait BlockUtils extends EphemBlockchainTestSetup { - val ommersValidator = new StdOmmersValidator(blockchainConfig, new EthashBlockHeaderValidator(blockchainConfig)) + val ommersValidator = new StdOmmersValidator(new EthashBlockHeaderValidator(blockchainConfig)) + + val ommerInvalidBranch = BlockHeader( + parentHash = ByteString(Hex.decode("fd07e36cfaf327801e5696134b12345f6a89fb1e8f017f2411a29d0ae810ab8b")), + ommersHash = ByteString(Hex.decode("7766c4251396a6833ccbe4be86fbda3a200dccbe6a15d80ae3de5378b1540e04")), + beneficiary = ByteString(Hex.decode("28921e4e2c9d84f4c0f0c0ceb991f45751a0fe93")), + stateRoot = ByteString(Hex.decode("e766f9c51536e9038849e5eb0a143c3b3409b5385098359837cbf3324ad22328")), + transactionsRoot = ByteString(Hex.decode("56e81f171bcc55a6ff8345e692c0f86e5b48e01b996cadc001622fb5e363b421")), + receiptsRoot = ByteString(Hex.decode("56e81f171bcc55a6ff8345e692c0f86e5b48e01b996cadc001622fb5e363b421")), + logsBloom = ByteString( + Hex.decode( + "00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000" + ) + ), + difficulty = BigInt("17864037202"), + number = 94, + gasLimit = 5000, + gasUsed = 0, + unixTimestamp = 1438270431, + extraData = ByteString(Hex.decode("476574682f76312e302e302f6c696e75782f676f312e342e32")), + mixHash = ByteString(Hex.decode("8c1ed8037984be0fe9065f8f8663c3baeeb6436868ac6915dd3c2cd5fd46fa96")), + nonce = ByteString(Hex.decode("40b0b2c0b6d14706")) + ) //Ommers from block 0xe9fb121a7ee5cb03b33adbf59e95321a2453f09db98068e1f31f0da79860c50c (of number 97) val ommer1 = BlockHeader( @@ -123,7 +160,7 @@ class OmmersValidatorSpec extends AnyFlatSpec with Matchers with ScalaCheckPrope nonce = ByteString(Hex.decode("40b0b2c0b6d14706")) ) val ommers: Seq[BlockHeader] = Seq[BlockHeader](ommer1, ommer2) - val ommersBlockNumber = 97 + val ommersBlockNumber: BigInt = 97 val block90 = Block( BlockHeader( diff --git a/src/test/scala/io/iohk/ethereum/consensus/validators/SignedTransactionValidatorSpec.scala b/src/test/scala/io/iohk/ethereum/consensus/validators/std/StdSignedTransactionValidatorSpec.scala similarity index 98% rename from src/test/scala/io/iohk/ethereum/consensus/validators/SignedTransactionValidatorSpec.scala rename to src/test/scala/io/iohk/ethereum/consensus/validators/std/StdSignedTransactionValidatorSpec.scala index 974bdf9b94..6258beafb7 100644 --- a/src/test/scala/io/iohk/ethereum/consensus/validators/SignedTransactionValidatorSpec.scala +++ b/src/test/scala/io/iohk/ethereum/consensus/validators/std/StdSignedTransactionValidatorSpec.scala @@ -1,11 +1,8 @@ -package io.iohk.ethereum.consensus.validators - -import java.math.BigInteger -import java.security.SecureRandom +package io.iohk.ethereum.consensus.validators.std import akka.util.ByteString import io.iohk.ethereum.consensus.validators.SignedTransactionError.{TransactionSignatureError, _} -import io.iohk.ethereum.consensus.validators.std.StdSignedTransactionValidator +import io.iohk.ethereum.consensus.validators.{SignedTransactionError, SignedTransactionValid} import io.iohk.ethereum.crypto.ECDSASignature import io.iohk.ethereum.domain._ import io.iohk.ethereum.utils.Config @@ -15,7 +12,10 @@ import org.bouncycastle.util.encoders.Hex import org.scalatest.flatspec.AnyFlatSpec import org.scalatest.matchers.should.Matchers -class SignedTransactionValidatorSpec extends AnyFlatSpec with Matchers { +import java.math.BigInteger +import java.security.SecureRandom + +class StdSignedTransactionValidatorSpec extends AnyFlatSpec with Matchers { val blockchainConfig = Config.blockchains.blockchainConfig diff --git a/src/test/scala/io/iohk/ethereum/jsonrpc/CheckpointingJRCSpec.scala b/src/test/scala/io/iohk/ethereum/jsonrpc/CheckpointingJRCSpec.scala index 84d5128718..8dc965d20d 100644 --- a/src/test/scala/io/iohk/ethereum/jsonrpc/CheckpointingJRCSpec.scala +++ b/src/test/scala/io/iohk/ethereum/jsonrpc/CheckpointingJRCSpec.scala @@ -41,7 +41,8 @@ class CheckpointingJRCSpec "block" -> JObject( "hash" -> JString("0x" + ByteStringUtils.hash2string(block.hash)), "number" -> JInt(block.number) - )) + ) + ) val response = jsonRpcController.handleRequest(request).runSyncUnsafe() response should haveResult(expectedResult) diff --git a/src/test/scala/io/iohk/ethereum/jsonrpc/JsonRpcControllerFixture.scala b/src/test/scala/io/iohk/ethereum/jsonrpc/JsonRpcControllerFixture.scala index cfdc4f556d..16157feaae 100644 --- a/src/test/scala/io/iohk/ethereum/jsonrpc/JsonRpcControllerFixture.scala +++ b/src/test/scala/io/iohk/ethereum/jsonrpc/JsonRpcControllerFixture.scala @@ -6,7 +6,7 @@ import akka.util.ByteString import io.iohk.ethereum.blockchain.sync.EphemBlockchainTestSetup import io.iohk.ethereum.consensus.blocks.CheckpointBlockGenerator import io.iohk.ethereum.consensus.ethash.blocks.EthashBlockGenerator -import io.iohk.ethereum.consensus.ethash.validators.ValidatorsExecutor +import io.iohk.ethereum.consensus.validators.std.ValidatorsExecutor import io.iohk.ethereum.consensus.{ConsensusConfigs, TestConsensus} import io.iohk.ethereum.crypto.ECDSASignature import io.iohk.ethereum.db.storage.AppStateStorage diff --git a/src/test/scala/io/iohk/ethereum/ledger/LedgerTestSetup.scala b/src/test/scala/io/iohk/ethereum/ledger/LedgerTestSetup.scala index 4999d3fb00..629c591445 100644 --- a/src/test/scala/io/iohk/ethereum/ledger/LedgerTestSetup.scala +++ b/src/test/scala/io/iohk/ethereum/ledger/LedgerTestSetup.scala @@ -5,8 +5,9 @@ import akka.util.ByteString.{empty => bEmpty} import cats.data.NonEmptyList import io.iohk.ethereum.blockchain.sync.EphemBlockchainTestSetup import io.iohk.ethereum.consensus.blocks.CheckpointBlockGenerator -import io.iohk.ethereum.consensus.ethash.validators.{OmmersValidator, StdOmmersValidator} +import io.iohk.ethereum.consensus.ethash.validators.OmmersValidator import io.iohk.ethereum.consensus.validators.BlockHeaderError.HeaderParentNotFoundError +import io.iohk.ethereum.consensus.validators.std.StdOmmersValidator import io.iohk.ethereum.consensus.validators.{BlockHeaderError, BlockHeaderValid, BlockHeaderValidator, Validators} import io.iohk.ethereum.consensus.{GetBlockHeaderByHash, GetNBlocksBack, TestConsensus} import io.iohk.ethereum.crypto.{generateKeyPair, kec256} @@ -434,7 +435,7 @@ trait OmmersTestSetup extends EphemBlockchain { getBlockHeaderByHash: GetBlockHeaderByHash, getNBlocksBack: GetNBlocksBack ) => - new StdOmmersValidator(blockchainConfig, blockHeaderValidator) + new StdOmmersValidator(blockHeaderValidator) .validate(parentHash, blockNumber, ommers, getBlockHeaderByHash, getNBlocksBack) } diff --git a/src/test/scala/io/iohk/ethereum/network/handshaker/EtcHandshakerSpec.scala b/src/test/scala/io/iohk/ethereum/network/handshaker/EtcHandshakerSpec.scala index 039d639745..a7f11a712c 100644 --- a/src/test/scala/io/iohk/ethereum/network/handshaker/EtcHandshakerSpec.scala +++ b/src/test/scala/io/iohk/ethereum/network/handshaker/EtcHandshakerSpec.scala @@ -269,8 +269,7 @@ class EtcHandshakerSpec extends AnyFlatSpec with Matchers { ) lazy val nodeStatusHolder = new AtomicReference(nodeStatus) - class MockEtcHandshakerConfiguration(pv: Int = Config.Network.protocolVersion) - extends EtcHandshakerConfiguration { + class MockEtcHandshakerConfiguration(pv: Int = Config.Network.protocolVersion) extends EtcHandshakerConfiguration { override val forkResolverOpt: Option[ForkResolver] = None override val nodeStatusHolder: AtomicReference[NodeStatus] = TestSetup.this.nodeStatusHolder override val peerConfiguration: PeerConfiguration = Config.Network.peer