From a5bcb8a7bd88107953ae96543795c22d5947e638 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20Richez?= Date: Thu, 22 Jul 2021 10:24:49 +0200 Subject: [PATCH 1/9] [ETCM-1042] store only hash and number in best branch --- .../ethereum/domain/BlockchainReader.scala | 22 ++++++++++--------- .../ethereum/domain/branch/BestBranch.scala | 9 ++++---- .../ethereum/domain/branch/EmptyBranch$.scala | 2 +- 3 files changed, 18 insertions(+), 15 deletions(-) diff --git a/src/main/scala/io/iohk/ethereum/domain/BlockchainReader.scala b/src/main/scala/io/iohk/ethereum/domain/BlockchainReader.scala index 93e5468583..adbaa74148 100644 --- a/src/main/scala/io/iohk/ethereum/domain/BlockchainReader.scala +++ b/src/main/scala/io/iohk/ethereum/domain/BlockchainReader.scala @@ -1,16 +1,13 @@ package io.iohk.ethereum.domain import akka.util.ByteString - import io.iohk.ethereum.db.storage.AppStateStorage import io.iohk.ethereum.db.storage.BlockBodiesStorage import io.iohk.ethereum.db.storage.BlockHeadersStorage import io.iohk.ethereum.db.storage.BlockNumberMappingStorage import io.iohk.ethereum.db.storage.ReceiptStorage import io.iohk.ethereum.db.storage.StateStorage -import io.iohk.ethereum.domain.branch.BestBranch -import io.iohk.ethereum.domain.branch.Branch -import io.iohk.ethereum.domain.branch.EmptyBranch$ +import io.iohk.ethereum.domain.branch.{BestBranch, Branch, EmptyBranch} import io.iohk.ethereum.mpt.MptNode import io.iohk.ethereum.utils.Logger @@ -71,16 +68,21 @@ class BlockchainReader( def getReceiptsByHash(blockhash: ByteString): Option[Seq[Receipt]] = receiptStorage.get(blockhash) /** get the current best stored branch */ - def getBestBranch(): Branch = - getBestBlock() - .map { block => + def getBestBranch(): Branch = { + val number = getBestBlockNumber() + blockNumberMappingStorage + .get(number) + .map(hash => new BestBranch( - block.header, + hash, + number, blockNumberMappingStorage, this ) - } - .getOrElse(EmptyBranch$) + ) + .getOrElse(EmptyBranch) + + } def getBestBlockNumber(): BigInt = { val bestSavedBlockNumber = appStateStorage.getBestBlockNumber() diff --git a/src/main/scala/io/iohk/ethereum/domain/branch/BestBranch.scala b/src/main/scala/io/iohk/ethereum/domain/branch/BestBranch.scala index 52d63bf0bc..575a482095 100644 --- a/src/main/scala/io/iohk/ethereum/domain/branch/BestBranch.scala +++ b/src/main/scala/io/iohk/ethereum/domain/branch/BestBranch.scala @@ -10,7 +10,8 @@ import io.iohk.ethereum.domain.BlockchainReader * This implementation uses the existing storage indexes to access blocks by number more efficiently. */ class BestBranch( - tipBlockHeader: BlockHeader, + tipBlockHash: ByteString, + tipBlockNumber: BigInt, bestChainBlockNumberMappingStorage: BlockNumberMappingStorage, blockchainReader: BlockchainReader ) extends Branch { @@ -21,7 +22,7 @@ class BestBranch( */ override def getBlockByNumber(number: BigInt): Option[Block] = - if (tipBlockHeader.number >= number && number >= 0) { + if (tipBlockNumber >= number && number >= 0) { for { hash <- getHashByBlockNumber(number) block <- blockchainReader.getBlockByHash(hash) @@ -29,13 +30,13 @@ class BestBranch( } else None override def getHashByBlockNumber(number: BigInt): Option[ByteString] = - if (tipBlockHeader.number >= number && number >= 0) { + if (tipBlockNumber >= number && number >= 0) { bestChainBlockNumberMappingStorage.get(number) } else None override def isInChain(hash: ByteString): Boolean = (for { - header <- blockchainReader.getBlockHeaderByHash(hash) if header.number <= tipBlockHeader.number + header <- blockchainReader.getBlockHeaderByHash(hash) if header.number <= tipBlockNumber hash <- getHashByBlockNumber(header.number) } yield header.hash == hash).getOrElse(false) } diff --git a/src/main/scala/io/iohk/ethereum/domain/branch/EmptyBranch$.scala b/src/main/scala/io/iohk/ethereum/domain/branch/EmptyBranch$.scala index 53f79212f5..16f23f8e0f 100644 --- a/src/main/scala/io/iohk/ethereum/domain/branch/EmptyBranch$.scala +++ b/src/main/scala/io/iohk/ethereum/domain/branch/EmptyBranch$.scala @@ -3,7 +3,7 @@ import akka.util.ByteString import io.iohk.ethereum.domain.Block -object EmptyBranch$ extends Branch { +object EmptyBranch extends Branch { override def getBlockByNumber(number: BigInt): Option[Block] = None From 6aabfe3d887b020a776c649fae50b5d61a441ce4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20Richez?= Date: Thu, 22 Jul 2021 12:22:56 +0200 Subject: [PATCH 2/9] [ETCM-1042] move getBlockByNumber to BlockchainReader and out of Branch interface --- .../ethereum/sync/RegularSyncItSpec.scala | 8 ++++--- .../sync/util/RegularSyncItSpecUtils.scala | 3 +-- .../pow/validators/OmmersValidator.scala | 4 ++-- .../io/iohk/ethereum/domain/Blockchain.scala | 2 +- .../ethereum/domain/BlockchainReader.scala | 23 ++++++++++++++++++- .../ethereum/domain/branch/BestBranch.scala | 8 ------- .../iohk/ethereum/domain/branch/Branch.scala | 9 +++++--- .../ethereum/domain/branch/EmptyBranch$.scala | 2 -- .../jsonrpc/CheckpointingService.scala | 2 +- .../ethereum/jsonrpc/EthProofService.scala | 3 +-- .../iohk/ethereum/jsonrpc/EthTxService.scala | 4 ++-- .../iohk/ethereum/jsonrpc/ResolveBlock.scala | 3 +-- .../iohk/ethereum/jsonrpc/TestService.scala | 4 ++-- .../io/iohk/ethereum/ledger/BlockImport.scala | 2 +- .../ethereum/ledger/BlockValidation.scala | 4 ++-- .../ethereum/ledger/BranchResolution.scala | 4 ++-- .../testmode/TestEthBlockServiceWrapper.scala | 4 ++-- .../TransactionHistoryService.scala | 4 +++- .../validators/StdOmmersValidatorSpec.scala | 2 +- .../iohk/ethereum/domain/BlockchainSpec.scala | 5 ++-- .../jsonrpc/CheckpointingServiceSpec.scala | 22 ++++++++---------- .../ethereum/ledger/LedgerTestSetup.scala | 11 ++++----- 22 files changed, 71 insertions(+), 62 deletions(-) diff --git a/src/it/scala/io/iohk/ethereum/sync/RegularSyncItSpec.scala b/src/it/scala/io/iohk/ethereum/sync/RegularSyncItSpec.scala index c29838ad23..e68f9f0c03 100644 --- a/src/it/scala/io/iohk/ethereum/sync/RegularSyncItSpec.scala +++ b/src/it/scala/io/iohk/ethereum/sync/RegularSyncItSpec.scala @@ -124,7 +124,9 @@ class RegularSyncItSpec extends FreeSpecBase with Matchers with BeforeAndAfterAl _ <- peer2.importBlocksUntil(30)(IdentityUpdate) _ <- peer1.startRegularSync() _ <- peer2.startRegularSync() - _ <- peer2.addCheckpointedBlock(peer2.blockchainReader.getBestBranch().getBlockByNumber(25).get) + _ <- peer2.addCheckpointedBlock( + peer2.blockchainReader.getBlockByNumber(peer2.blockchainReader.getBestBranchNew(), 25).get + ) _ <- peer2.waitForRegularSyncLoadLastBlock(length) _ <- peer1.connectToPeers(Set(peer2.node)) _ <- peer1.waitForRegularSyncLoadLastBlock(length) @@ -181,8 +183,8 @@ class RegularSyncItSpec extends FreeSpecBase with Matchers with BeforeAndAfterAl ) ) ( - peer1.blockchainReader.getBestBranch().getBlockByNumber(blockNumer + 1), - peer2.blockchainReader.getBestBranch().getBlockByNumber(blockNumer + 1) + peer1.blockchainReader.getBlockByNumber(peer1.blockchainReader.getBestBranchNew(), blockNumer + 1), + peer2.blockchainReader.getBlockByNumber(peer2.blockchainReader.getBestBranchNew(), blockNumer + 1) ) match { case (Some(blockP1), Some(blockP2)) => assert(peer1.bl.getChainWeightByHash(blockP1.hash) == peer2.bl.getChainWeightByHash(blockP2.hash)) 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 f7c674bd30..1b8972b164 100644 --- a/src/it/scala/io/iohk/ethereum/sync/util/RegularSyncItSpecUtils.scala +++ b/src/it/scala/io/iohk/ethereum/sync/util/RegularSyncItSpecUtils.scala @@ -189,8 +189,7 @@ object RegularSyncItSpecUtils { Task(blockNumber match { case Some(bNumber) => blockchainReader - .getBestBranch() - .getBlockByNumber(bNumber) + .getBlockByNumber(blockchainReader.getBestBranchNew(), bNumber) .getOrElse(throw new RuntimeException(s"block by number: $bNumber doesn't exist")) case None => blockchainReader.getBestBlock().get }).flatMap { block => diff --git a/src/main/scala/io/iohk/ethereum/consensus/pow/validators/OmmersValidator.scala b/src/main/scala/io/iohk/ethereum/consensus/pow/validators/OmmersValidator.scala index 777b6cafd0..a978a5dc37 100644 --- a/src/main/scala/io/iohk/ethereum/consensus/pow/validators/OmmersValidator.scala +++ b/src/main/scala/io/iohk/ethereum/consensus/pow/validators/OmmersValidator.scala @@ -30,11 +30,11 @@ trait OmmersValidator { )(implicit blockchainConfig: BlockchainConfig): Either[OmmersError, OmmersValid] = { val getBlockHeaderByHash: ByteString => Option[BlockHeader] = blockchainReader.getBlockHeaderByHash - val bestBranch = blockchainReader.getBestBranch() + val bestBranch = blockchainReader.getBestBranchNew() val getNBlocksBack: (ByteString, Int) => List[Block] = (_, n) => ((blockNumber - n) until blockNumber).toList - .flatMap(nb => bestBranch.getBlockByNumber(nb)) + .flatMap(nb => blockchainReader.getBlockByNumber(bestBranch, nb)) validate(parentHash, blockNumber, ommers, getBlockHeaderByHash, getNBlocksBack) } diff --git a/src/main/scala/io/iohk/ethereum/domain/Blockchain.scala b/src/main/scala/io/iohk/ethereum/domain/Blockchain.scala index beabb0169d..5033df0799 100644 --- a/src/main/scala/io/iohk/ethereum/domain/Blockchain.scala +++ b/src/main/scala/io/iohk/ethereum/domain/Blockchain.scala @@ -280,7 +280,7 @@ class BlockchainImpl( ): BigInt = if (blockNumberToCheck > 0) { val maybePreviousCheckpointBlockNumber = for { - currentBlock <- blockchainReader.getBestBranch().getBlockByNumber(blockNumberToCheck) + currentBlock <- blockchainReader.getBlockByNumber(blockchainReader.getBestBranchNew(), blockNumberToCheck) if currentBlock.hasCheckpoint && currentBlock.number < latestCheckpointBlockNumber } yield currentBlock.number diff --git a/src/main/scala/io/iohk/ethereum/domain/BlockchainReader.scala b/src/main/scala/io/iohk/ethereum/domain/BlockchainReader.scala index adbaa74148..ae86b6d962 100644 --- a/src/main/scala/io/iohk/ethereum/domain/BlockchainReader.scala +++ b/src/main/scala/io/iohk/ethereum/domain/BlockchainReader.scala @@ -7,7 +7,7 @@ import io.iohk.ethereum.db.storage.BlockHeadersStorage import io.iohk.ethereum.db.storage.BlockNumberMappingStorage import io.iohk.ethereum.db.storage.ReceiptStorage import io.iohk.ethereum.db.storage.StateStorage -import io.iohk.ethereum.domain.branch.{BestBranch, Branch, EmptyBranch} +import io.iohk.ethereum.domain.branch.{BestBranch, BestBranchSubset, Branch, EmptyBranch, NewBranch, NewEmptyBranch} import io.iohk.ethereum.mpt.MptNode import io.iohk.ethereum.utils.Logger @@ -84,6 +84,15 @@ class BlockchainReader( } + /** get the current best stored branch */ + def getBestBranchNew(): NewBranch = { + val number = getBestBlockNumber() + blockNumberMappingStorage + .get(number) + .map(hash => BestBranchSubset(hash, number)) + .getOrElse(NewEmptyBranch) + } + def getBestBlockNumber(): BigInt = { val bestSavedBlockNumber = appStateStorage.getBestBlockNumber() val bestKnownBlockNumber = blockchainMetadata.bestKnownBlockAndLatestCheckpoint.get().bestBlockNumber @@ -115,6 +124,18 @@ class BlockchainReader( def genesisBlock: Block = getBlockByNumber(0).get + /** Returns a block inside this branch based on its number */ + def getBlockByNumber(branch: NewBranch, number: BigInt): Option[Block] = branch match { + case BestBranchSubset(_, tipBlockNumber) => + if (tipBlockNumber >= number && number >= 0) { + for { + hash <- getHashByBlockNumber(number) + block <- getBlockByHash(hash) + } yield block + } else None + case NewEmptyBranch => None + } + /** Allows to query for a block based on it's number * * @param number Block number diff --git a/src/main/scala/io/iohk/ethereum/domain/branch/BestBranch.scala b/src/main/scala/io/iohk/ethereum/domain/branch/BestBranch.scala index 575a482095..0dfc35be3b 100644 --- a/src/main/scala/io/iohk/ethereum/domain/branch/BestBranch.scala +++ b/src/main/scala/io/iohk/ethereum/domain/branch/BestBranch.scala @@ -21,14 +21,6 @@ class BestBranch( * - The various metadata and index are consistent */ - override def getBlockByNumber(number: BigInt): Option[Block] = - if (tipBlockNumber >= number && number >= 0) { - for { - hash <- getHashByBlockNumber(number) - block <- blockchainReader.getBlockByHash(hash) - } yield block - } else None - override def getHashByBlockNumber(number: BigInt): Option[ByteString] = if (tipBlockNumber >= number && number >= 0) { bestChainBlockNumberMappingStorage.get(number) diff --git a/src/main/scala/io/iohk/ethereum/domain/branch/Branch.scala b/src/main/scala/io/iohk/ethereum/domain/branch/Branch.scala index af1b23850f..013941b5ee 100644 --- a/src/main/scala/io/iohk/ethereum/domain/branch/Branch.scala +++ b/src/main/scala/io/iohk/ethereum/domain/branch/Branch.scala @@ -4,12 +4,15 @@ import akka.util.ByteString import io.iohk.ethereum.domain.Block +sealed trait NewBranch + +case class BestBranchSubset(tipBlockHash: ByteString, tipBlockNumber: BigInt) extends NewBranch + +case object NewEmptyBranch extends NewBranch + /** An interface to manipulate blockchain branches */ trait Branch { - /** Returns a block inside this branch based on its number */ - def getBlockByNumber(number: BigInt): Option[Block] - /** Returns a block hash for the block at the given height if any */ def getHashByBlockNumber(number: BigInt): Option[ByteString] diff --git a/src/main/scala/io/iohk/ethereum/domain/branch/EmptyBranch$.scala b/src/main/scala/io/iohk/ethereum/domain/branch/EmptyBranch$.scala index 16f23f8e0f..b523024e97 100644 --- a/src/main/scala/io/iohk/ethereum/domain/branch/EmptyBranch$.scala +++ b/src/main/scala/io/iohk/ethereum/domain/branch/EmptyBranch$.scala @@ -5,8 +5,6 @@ import io.iohk.ethereum.domain.Block object EmptyBranch extends Branch { - override def getBlockByNumber(number: BigInt): Option[Block] = None - override def getHashByBlockNumber(number: BigInt): Option[ByteString] = None override def isInChain(hash: ByteString): Boolean = false diff --git a/src/main/scala/io/iohk/ethereum/jsonrpc/CheckpointingService.scala b/src/main/scala/io/iohk/ethereum/jsonrpc/CheckpointingService.scala index d76fa40550..3b38182afe 100644 --- a/src/main/scala/io/iohk/ethereum/jsonrpc/CheckpointingService.scala +++ b/src/main/scala/io/iohk/ethereum/jsonrpc/CheckpointingService.scala @@ -36,7 +36,7 @@ class CheckpointingService( req.parentCheckpoint.forall(blockchainReader.getBlockHeaderByHash(_).exists(_.number < blockToReturnNum)) Task { - blockchainReader.getBestBranch().getBlockByNumber(blockToReturnNum) + blockchainReader.getBlockByNumber(blockchainReader.getBestBranchNew(), blockToReturnNum) }.flatMap { case Some(b) if isValidParent => Task.now(Right(GetLatestBlockResponse(Some(BlockInfo(b.hash, b.number))))) diff --git a/src/main/scala/io/iohk/ethereum/jsonrpc/EthProofService.scala b/src/main/scala/io/iohk/ethereum/jsonrpc/EthProofService.scala index a020450b54..36e6713dd6 100644 --- a/src/main/scala/io/iohk/ethereum/jsonrpc/EthProofService.scala +++ b/src/main/scala/io/iohk/ethereum/jsonrpc/EthProofService.scala @@ -205,8 +205,7 @@ class EthProofService( private def resolveBlock(blockParam: BlockParam): Either[JsonRpcError, ResolvedBlock] = { def getBlock(number: BigInt): Either[JsonRpcError, Block] = blockchainReader - .getBestBranch() - .getBlockByNumber(number) + .getBlockByNumber(blockchainReader.getBestBranchNew(), number) .toRight(JsonRpcError.InvalidParams(s"Block $number not found")) def getLatestBlock(): Either[JsonRpcError, Block] = diff --git a/src/main/scala/io/iohk/ethereum/jsonrpc/EthTxService.scala b/src/main/scala/io/iohk/ethereum/jsonrpc/EthTxService.scala index 7f63c4f5ff..1a613476a0 100644 --- a/src/main/scala/io/iohk/ethereum/jsonrpc/EthTxService.scala +++ b/src/main/scala/io/iohk/ethereum/jsonrpc/EthTxService.scala @@ -156,9 +156,9 @@ class EthTxService( val bestBlock = blockchainReader.getBestBlockNumber() Task { - val bestBranch = blockchainReader.getBestBranch() + val bestBranch = blockchainReader.getBestBranchNew() val gasPrice = ((bestBlock - blockDifference) to bestBlock) - .flatMap(nb => bestBranch.getBlockByNumber(nb)) + .flatMap(nb => blockchainReader.getBlockByNumber(bestBranch, nb)) .flatMap(_.body.transactionList) .map(_.tx.gasPrice) if (gasPrice.nonEmpty) { diff --git a/src/main/scala/io/iohk/ethereum/jsonrpc/ResolveBlock.scala b/src/main/scala/io/iohk/ethereum/jsonrpc/ResolveBlock.scala index d89476ac23..2a98719997 100644 --- a/src/main/scala/io/iohk/ethereum/jsonrpc/ResolveBlock.scala +++ b/src/main/scala/io/iohk/ethereum/jsonrpc/ResolveBlock.scala @@ -34,8 +34,7 @@ trait ResolveBlock { private def getBlock(number: BigInt): Either[JsonRpcError, Block] = blockchainReader - .getBestBranch() - .getBlockByNumber(number) + .getBlockByNumber(blockchainReader.getBestBranchNew(), number) .toRight(JsonRpcError.InvalidParams(s"Block $number not found")) private def getLatestBlock(): Either[JsonRpcError, Block] = diff --git a/src/main/scala/io/iohk/ethereum/jsonrpc/TestService.scala b/src/main/scala/io/iohk/ethereum/jsonrpc/TestService.scala index 1a62ad0a65..e70c423b41 100644 --- a/src/main/scala/io/iohk/ethereum/jsonrpc/TestService.scala +++ b/src/main/scala/io/iohk/ethereum/jsonrpc/TestService.scala @@ -368,7 +368,7 @@ class TestService( val blockOpt = request.parameters.blockHashOrNumber .fold( - number => blockchainReader.getBestBranch().getBlockByNumber(number), + number => blockchainReader.getBlockByNumber(blockchainReader.getBestBranchNew(), number), blockHash => blockchainReader.getBlockByHash(blockHash) ) @@ -408,7 +408,7 @@ class TestService( val blockOpt = request.parameters.blockHashOrNumber .fold( - number => blockchainReader.getBestBranch().getBlockByNumber(number), + number => blockchainReader.getBlockByNumber(blockchainReader.getBestBranchNew(), number), hash => blockchainReader.getBlockByHash(hash) ) diff --git a/src/main/scala/io/iohk/ethereum/ledger/BlockImport.scala b/src/main/scala/io/iohk/ethereum/ledger/BlockImport.scala index 82c72c2ec4..c1fee6ace3 100644 --- a/src/main/scala/io/iohk/ethereum/ledger/BlockImport.scala +++ b/src/main/scala/io/iohk/ethereum/ledger/BlockImport.scala @@ -318,7 +318,7 @@ class BlockImport( private def removeBlocksUntil(parent: ByteString, fromNumber: BigInt): List[BlockData] = { @tailrec def removeBlocksUntil(parent: ByteString, fromNumber: BigInt, acc: List[BlockData]): List[BlockData] = - blockchainReader.getBestBranch().getBlockByNumber(fromNumber) match { + blockchainReader.getBlockByNumber(blockchainReader.getBestBranchNew(), fromNumber) match { case Some(block) if block.header.hash == parent || fromNumber == 0 => acc diff --git a/src/main/scala/io/iohk/ethereum/ledger/BlockValidation.scala b/src/main/scala/io/iohk/ethereum/ledger/BlockValidation.scala index 463d82ecfe..1c2bd1fff1 100644 --- a/src/main/scala/io/iohk/ethereum/ledger/BlockValidation.scala +++ b/src/main/scala/io/iohk/ethereum/ledger/BlockValidation.scala @@ -44,9 +44,9 @@ class BlockValidation( val remaining = n - queuedBlocks.length - 1 val numbers = (block.header.number - remaining) until block.header.number - val bestBranch = blockchainReader.getBestBranch() + val bestBranch = blockchainReader.getBestBranchNew() val blocks = - (numbers.toList.flatMap(nb => bestBranch.getBlockByNumber(nb)) :+ block) ::: queuedBlocks + (numbers.toList.flatMap(nb => blockchainReader.getBlockByNumber(bestBranch, nb)) :+ block) ::: queuedBlocks blocks } } diff --git a/src/main/scala/io/iohk/ethereum/ledger/BranchResolution.scala b/src/main/scala/io/iohk/ethereum/ledger/BranchResolution.scala index e568a5284b..450ae109e4 100644 --- a/src/main/scala/io/iohk/ethereum/ledger/BranchResolution.scala +++ b/src/main/scala/io/iohk/ethereum/ledger/BranchResolution.scala @@ -74,9 +74,9 @@ class BranchResolution(blockchain: Blockchain, blockchainReader: BlockchainReade } private def getTopBlocksFromNumber(from: BigInt): List[Block] = { - val bestBranch = blockchainReader.getBestBranch() + val bestBranch = blockchainReader.getBestBranchNew() (from to blockchainReader.getBestBlockNumber()) - .flatMap(nb => bestBranch.getBlockByNumber(nb)) + .flatMap(nb => blockchainReader.getBlockByNumber(bestBranch, nb)) .toList } } diff --git a/src/main/scala/io/iohk/ethereum/testmode/TestEthBlockServiceWrapper.scala b/src/main/scala/io/iohk/ethereum/testmode/TestEthBlockServiceWrapper.scala index f481a5cf85..58904d633c 100644 --- a/src/main/scala/io/iohk/ethereum/testmode/TestEthBlockServiceWrapper.scala +++ b/src/main/scala/io/iohk/ethereum/testmode/TestEthBlockServiceWrapper.scala @@ -75,8 +75,8 @@ class TestEthBlockServiceWrapper( .getBlockByNumber(request) .map( _.map { blockByBlockResponse => - val bestBranch = blockchainReader.getBestBranch() - val fullBlock = bestBranch.getBlockByNumber(blockByBlockResponse.blockResponse.get.number).get + val bestBranch = blockchainReader.getBestBranchNew() + val fullBlock = blockchainReader.getBlockByNumber(bestBranch, blockByBlockResponse.blockResponse.get.number).get BlockByNumberResponse(blockByBlockResponse.blockResponse.map(response => toEthResponse(fullBlock, response))) } ) diff --git a/src/main/scala/io/iohk/ethereum/transactions/TransactionHistoryService.scala b/src/main/scala/io/iohk/ethereum/transactions/TransactionHistoryService.scala index 753a6b1ca0..9719576fe0 100644 --- a/src/main/scala/io/iohk/ethereum/transactions/TransactionHistoryService.scala +++ b/src/main/scala/io/iohk/ethereum/transactions/TransactionHistoryService.scala @@ -33,7 +33,9 @@ class TransactionHistoryService( val getLastCheckpoint = Task(blockchain.getLatestCheckpointBlockNumber()).memoizeOnSuccess val txnsFromBlocks = Observable .from(fromBlocks.reverse) - .mapParallelOrdered(10)(blockNr => Task(blockchainReader.getBestBranch().getBlockByNumber(blockNr)))( + .mapParallelOrdered(10)(blockNr => + Task(blockchainReader.getBlockByNumber(blockchainReader.getBestBranchNew(), blockNr)) + )( OverflowStrategy.Unbounded ) .collect { case Some(block) => block } diff --git a/src/test/scala/io/iohk/ethereum/consensus/pow/validators/StdOmmersValidatorSpec.scala b/src/test/scala/io/iohk/ethereum/consensus/pow/validators/StdOmmersValidatorSpec.scala index 35cc11230b..c09c325f6e 100644 --- a/src/test/scala/io/iohk/ethereum/consensus/pow/validators/StdOmmersValidatorSpec.scala +++ b/src/test/scala/io/iohk/ethereum/consensus/pow/validators/StdOmmersValidatorSpec.scala @@ -81,7 +81,7 @@ class StdOmmersValidatorSpec extends AnyFlatSpec with Matchers with ScalaCheckPr val getNBlocksBack: (ByteString, Int) => List[Block] = (_, n) => ((ommersBlockNumber - n) until ommersBlockNumber).toList - .flatMap(nb => blockchainReader.getBestBranch().getBlockByNumber(nb)) + .flatMap(nb => blockchainReader.getBlockByNumber(blockchainReader.getBestBranchNew(), nb)) ommersValidator.validateOmmersAncestors( ommersBlockParentHash, diff --git a/src/test/scala/io/iohk/ethereum/domain/BlockchainSpec.scala b/src/test/scala/io/iohk/ethereum/domain/BlockchainSpec.scala index 7fc47c4fe7..7d68a8828b 100644 --- a/src/test/scala/io/iohk/ethereum/domain/BlockchainSpec.scala +++ b/src/test/scala/io/iohk/ethereum/domain/BlockchainSpec.scala @@ -45,7 +45,7 @@ class BlockchainSpec extends AnyFlatSpec with Matchers with ScalaCheckPropertyCh val validBlock = Fixtures.Blocks.ValidBlock.block blockchainWriter.storeBlock(validBlock).commit() blockchain.saveBestKnownBlocks(validBlock.number) - val block = blockchainReader.getBestBranch().getBlockByNumber(validBlock.header.number) + val block = blockchainReader.getBlockByNumber(blockchainReader.getBestBranchNew(), validBlock.header.number) block.isDefined should ===(true) validBlock should ===(block.get) } @@ -75,8 +75,7 @@ class BlockchainSpec extends AnyFlatSpec with Matchers with ScalaCheckPropertyCh it should "not return a value if not stored" in new EphemBlockchainTestSetup { blockchainReader - .getBestBranch() - .getBlockByNumber(Fixtures.Blocks.ValidBlock.header.number) shouldBe None + .getBlockByNumber(blockchainReader.getBestBranchNew(), Fixtures.Blocks.ValidBlock.header.number) shouldBe None blockchainReader.getBlockByHash(Fixtures.Blocks.ValidBlock.header.hash) shouldBe None } diff --git a/src/test/scala/io/iohk/ethereum/jsonrpc/CheckpointingServiceSpec.scala b/src/test/scala/io/iohk/ethereum/jsonrpc/CheckpointingServiceSpec.scala index 7ab0ef4c0c..c0adbd5505 100644 --- a/src/test/scala/io/iohk/ethereum/jsonrpc/CheckpointingServiceSpec.scala +++ b/src/test/scala/io/iohk/ethereum/jsonrpc/CheckpointingServiceSpec.scala @@ -3,16 +3,13 @@ package io.iohk.ethereum.jsonrpc import akka.actor.ActorSystem import akka.testkit.TestKit import akka.testkit.TestProbe - import monix.execution.Scheduler.Implicits.global - import org.scalacheck.Gen import org.scalamock.scalatest.MockFactory import org.scalatest.concurrent.ScalaFutures import org.scalatest.flatspec.AnyFlatSpecLike import org.scalatest.matchers.should.Matchers import org.scalatestplus.scalacheck.ScalaCheckPropertyChecks - import io.iohk.ethereum.Fixtures import io.iohk.ethereum.NormalPatience import io.iohk.ethereum.WithActorSystemShutDown @@ -23,7 +20,7 @@ import io.iohk.ethereum.domain.BlockBody import io.iohk.ethereum.domain.BlockchainImpl import io.iohk.ethereum.domain.BlockchainReader import io.iohk.ethereum.domain.Checkpoint -import io.iohk.ethereum.domain.branch.Branch +import io.iohk.ethereum.domain.branch.{Branch, NewEmptyBranch} import io.iohk.ethereum.jsonrpc.CheckpointingService._ import io.iohk.ethereum.ledger.BlockQueue @@ -54,7 +51,7 @@ class CheckpointingServiceSpec val expectedResponse = GetLatestBlockResponse(Some(BlockInfo(block.hash, block.number))) (blockchainReader.getBestBlockNumber _).expects().returning(bestBlockNum) - (bestChain.getBlockByNumber _).expects(checkpointedBlockNum).returning(Some(block)) + (blockchainReader.getBlockByNumber _).expects(*, checkpointedBlockNum).returning(Some(block)) val result = service.getLatestBlock(request) result.runSyncUnsafe() shouldEqual Right(expectedResponse) @@ -84,7 +81,7 @@ class CheckpointingServiceSpec (blockchainReader.getBlockHeaderByHash _) .expects(hash) .returning(Some(previousCheckpoint.header.copy(number = 0))) - (bestChain.getBlockByNumber _).expects(checkpointedBlockNum).returning(Some(block)) + (blockchainReader.getBlockByNumber _).expects(*, checkpointedBlockNum).returning(Some(block)) val result = service.getLatestBlock(request) result.runSyncUnsafe() shouldEqual Right(expectedResponse) @@ -112,7 +109,7 @@ class CheckpointingServiceSpec (blockchainReader.getBlockHeaderByHash _) .expects(hash) .returning(Some(previousCheckpoint.header.copy(number = bestBlockNum))) - (bestChain.getBlockByNumber _).expects(*).returning(Some(previousCheckpoint)) + (blockchainReader.getBlockByNumber _).expects(*, *).returning(Some(previousCheckpoint)) val result = service.getLatestBlock(request) result.runSyncUnsafe() shouldEqual Right(expectedResponse) @@ -140,7 +137,7 @@ class CheckpointingServiceSpec (blockchainReader.getBestBlockNumber _).expects().returning(bestBlockNum) (blockchainReader.getBlockHeaderByHash _).expects(hash).returning(None) - (bestChain.getBlockByNumber _).expects(checkpointedBlockNum).returning(Some(block)) + (blockchainReader.getBlockByNumber _).expects(*, checkpointedBlockNum).returning(Some(block)) val result = service.getLatestBlock(request) result.runSyncUnsafe() shouldEqual Right(expectedResponse) @@ -168,14 +165,14 @@ class CheckpointingServiceSpec (blockchainReader.getBestBlockNumber _) .expects() .returning(7) - (bestChain.getBlockByNumber _) - .expects(BigInt(4)) + (blockchainReader.getBlockByNumber _) + .expects(*, BigInt(4)) .returning(None) (blockchainReader.getBestBlockNumber _) .expects() .returning(7) - (bestChain.getBlockByNumber _) - .expects(BigInt(4)) + (blockchainReader.getBlockByNumber _) + .expects(*, BigInt(4)) .returning(Some(block)) val result = service.getLatestBlock(GetLatestBlockRequest(4, None)) @@ -188,6 +185,7 @@ class CheckpointingServiceSpec val blockchainReader: BlockchainReader = mock[BlockchainReader] val bestChain: Branch = mock[Branch] (blockchainReader.getBestBranch _).expects().anyNumberOfTimes().returning(bestChain) + (blockchainReader.getBestBranchNew _).expects().anyNumberOfTimes().returning(NewEmptyBranch) val blockQueue: BlockQueue = mock[BlockQueue] val syncController: TestProbe = TestProbe() val checkpointBlockGenerator: CheckpointBlockGenerator = new CheckpointBlockGenerator() diff --git a/src/test/scala/io/iohk/ethereum/ledger/LedgerTestSetup.scala b/src/test/scala/io/iohk/ethereum/ledger/LedgerTestSetup.scala index 61bc72c2e0..735c18dfd2 100644 --- a/src/test/scala/io/iohk/ethereum/ledger/LedgerTestSetup.scala +++ b/src/test/scala/io/iohk/ethereum/ledger/LedgerTestSetup.scala @@ -2,11 +2,8 @@ package io.iohk.ethereum.ledger import akka.util.ByteString import akka.util.ByteString.{empty => bEmpty} - import cats.data.NonEmptyList - import monix.execution.Scheduler - import org.bouncycastle.crypto.AsymmetricCipherKeyPair import org.bouncycastle.crypto.params.ECPublicKeyParameters import org.bouncycastle.util.encoders.Hex @@ -14,7 +11,6 @@ import org.scalamock.handlers.CallHandler0 import org.scalamock.handlers.CallHandler1 import org.scalamock.handlers.CallHandler4 import org.scalamock.scalatest.MockFactory - import io.iohk.ethereum.Fixtures import io.iohk.ethereum.Mocks import io.iohk.ethereum.ObjectGenerators @@ -31,7 +27,7 @@ import io.iohk.ethereum.consensus.validators.BlockHeaderValidator import io.iohk.ethereum.crypto.generateKeyPair import io.iohk.ethereum.crypto.kec256 import io.iohk.ethereum.domain._ -import io.iohk.ethereum.domain.branch.Branch +import io.iohk.ethereum.domain.branch.{Branch, NewEmptyBranch} import io.iohk.ethereum.ledger.BlockExecutionError.ValidationAfterExecError import io.iohk.ethereum.ledger.PC import io.iohk.ethereum.ledger.PR @@ -420,6 +416,7 @@ trait MockBlockchain extends MockFactory { self: TestSetupWithVmAndValidators => override lazy val blockchainReader: BlockchainReader = mock[BlockchainReader] override lazy val blockchainWriter: BlockchainWriter = mock[BlockchainWriter] (blockchainReader.getBestBranch _).expects().anyNumberOfTimes().returning(bestChain) + (blockchainReader.getBestBranchNew _).expects().anyNumberOfTimes().returning(NewEmptyBranch) override lazy val blockchain: BlockchainImpl = mock[BlockchainImpl] //- cake overrides @@ -462,8 +459,8 @@ trait MockBlockchain extends MockFactory { self: TestSetupWithVmAndValidators => def setHeaderInChain(hash: ByteString, result: Boolean = true): CallHandler1[ByteString, Boolean] = (bestChain.isInChain _).expects(hash).returning(result) - def setBlockByNumber(number: BigInt, block: Option[Block]): CallHandler1[BigInt, Option[Block]] = - (bestChain.getBlockByNumber _).expects(number).returning(block) + def setBlockByNumber(number: BigInt, block: Option[Block]) = + (blockchainReader.getBlockByNumber _).expects(*, number).returning(block) def setGenesisHeader(header: BlockHeader): Unit = (() => blockchainReader.genesisHeader).expects().returning(header) From 7b52b62e940bcd24c1b76d210fae2fc9b48d50f3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20Richez?= Date: Thu, 22 Jul 2021 12:37:49 +0200 Subject: [PATCH 3/9] [ETCM-1042] move getHashByBlockNumber to BlockchainReader and out of Branch interface --- .../iohk/ethereum/txExecTest/util/DumpChainApp.scala | 2 +- .../scala/io/iohk/ethereum/domain/Blockchain.scala | 2 +- .../io/iohk/ethereum/domain/BlockchainReader.scala | 10 ++++++++++ .../io/iohk/ethereum/domain/branch/BestBranch.scala | 7 +------ .../scala/io/iohk/ethereum/domain/branch/Branch.scala | 5 ----- .../io/iohk/ethereum/domain/branch/EmptyBranch$.scala | 4 ---- 6 files changed, 13 insertions(+), 17 deletions(-) diff --git a/src/it/scala/io/iohk/ethereum/txExecTest/util/DumpChainApp.scala b/src/it/scala/io/iohk/ethereum/txExecTest/util/DumpChainApp.scala index 86b55cbb43..d00a2616aa 100644 --- a/src/it/scala/io/iohk/ethereum/txExecTest/util/DumpChainApp.scala +++ b/src/it/scala/io/iohk/ethereum/txExecTest/util/DumpChainApp.scala @@ -104,7 +104,7 @@ object DumpChainApp val blockchainReader: BlockchainReader = mock[BlockchainReader] val bestChain: Branch = mock[Branch] (blockchainReader.getBestBranch _).expects().anyNumberOfTimes().returning(bestChain) - (bestChain.getHashByBlockNumber _).expects(*).returning(Some(genesisHash)) + (blockchainReader.getHashByBlockNumber _).expects(*, *).returning(Some(genesisHash)) val nodeStatus: NodeStatus = NodeStatus(key = nodeKey, serverStatus = ServerStatus.NotListening, discoveryStatus = ServerStatus.NotListening) diff --git a/src/main/scala/io/iohk/ethereum/domain/Blockchain.scala b/src/main/scala/io/iohk/ethereum/domain/Blockchain.scala index 5033df0799..21d986b5fb 100644 --- a/src/main/scala/io/iohk/ethereum/domain/Blockchain.scala +++ b/src/main/scala/io/iohk/ethereum/domain/Blockchain.scala @@ -211,7 +211,7 @@ class BlockchainImpl( val latestCheckpointNumber = getLatestCheckpointBlockNumber() val blockNumberMappingUpdates = - if (blockchainReader.getBestBranch().getHashByBlockNumber(block.number).contains(blockHash)) + if (blockchainReader.getHashByBlockNumber(blockchainReader.getBestBranchNew(), block.number).contains(blockHash)) removeBlockNumberMapping(block.number) else blockNumberMappingStorage.emptyBatchUpdate diff --git a/src/main/scala/io/iohk/ethereum/domain/BlockchainReader.scala b/src/main/scala/io/iohk/ethereum/domain/BlockchainReader.scala index ae86b6d962..8cfcdbecdb 100644 --- a/src/main/scala/io/iohk/ethereum/domain/BlockchainReader.scala +++ b/src/main/scala/io/iohk/ethereum/domain/BlockchainReader.scala @@ -136,6 +136,16 @@ class BlockchainReader( case NewEmptyBranch => None } + /** Returns a block hash for the block at the given height if any */ + def getHashByBlockNumber(branch: NewBranch, number: BigInt): Option[ByteString] = branch match { + case BestBranchSubset(_, tipBlockNumber) => + if (tipBlockNumber >= number && number >= 0) { + blockNumberMappingStorage.get(number) + } else None + + case NewEmptyBranch => None + } + /** Allows to query for a block based on it's number * * @param number Block number diff --git a/src/main/scala/io/iohk/ethereum/domain/branch/BestBranch.scala b/src/main/scala/io/iohk/ethereum/domain/branch/BestBranch.scala index 0dfc35be3b..fbb525ba1c 100644 --- a/src/main/scala/io/iohk/ethereum/domain/branch/BestBranch.scala +++ b/src/main/scala/io/iohk/ethereum/domain/branch/BestBranch.scala @@ -21,14 +21,9 @@ class BestBranch( * - The various metadata and index are consistent */ - override def getHashByBlockNumber(number: BigInt): Option[ByteString] = - if (tipBlockNumber >= number && number >= 0) { - bestChainBlockNumberMappingStorage.get(number) - } else None - override def isInChain(hash: ByteString): Boolean = (for { header <- blockchainReader.getBlockHeaderByHash(hash) if header.number <= tipBlockNumber - hash <- getHashByBlockNumber(header.number) + hash <- blockchainReader.getHashByBlockNumber(BestBranchSubset(tipBlockHash, tipBlockNumber), header.number) } yield header.hash == hash).getOrElse(false) } diff --git a/src/main/scala/io/iohk/ethereum/domain/branch/Branch.scala b/src/main/scala/io/iohk/ethereum/domain/branch/Branch.scala index 013941b5ee..4a530693e5 100644 --- a/src/main/scala/io/iohk/ethereum/domain/branch/Branch.scala +++ b/src/main/scala/io/iohk/ethereum/domain/branch/Branch.scala @@ -2,8 +2,6 @@ package io.iohk.ethereum.domain.branch import akka.util.ByteString -import io.iohk.ethereum.domain.Block - sealed trait NewBranch case class BestBranchSubset(tipBlockHash: ByteString, tipBlockNumber: BigInt) extends NewBranch @@ -13,9 +11,6 @@ case object NewEmptyBranch extends NewBranch /** An interface to manipulate blockchain branches */ trait Branch { - /** Returns a block hash for the block at the given height if any */ - def getHashByBlockNumber(number: BigInt): Option[ByteString] - /** Checks if given block hash is in this chain. (i.e. is an ancestor of the tip block) */ def isInChain(hash: ByteString): Boolean } diff --git a/src/main/scala/io/iohk/ethereum/domain/branch/EmptyBranch$.scala b/src/main/scala/io/iohk/ethereum/domain/branch/EmptyBranch$.scala index b523024e97..d579c7d639 100644 --- a/src/main/scala/io/iohk/ethereum/domain/branch/EmptyBranch$.scala +++ b/src/main/scala/io/iohk/ethereum/domain/branch/EmptyBranch$.scala @@ -1,11 +1,7 @@ package io.iohk.ethereum.domain.branch import akka.util.ByteString -import io.iohk.ethereum.domain.Block - object EmptyBranch extends Branch { - override def getHashByBlockNumber(number: BigInt): Option[ByteString] = None - override def isInChain(hash: ByteString): Boolean = false } From ea6936e75c44d569503a9085778412167d4190cc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20Richez?= Date: Thu, 22 Jul 2021 12:49:04 +0200 Subject: [PATCH 4/9] [ETCM-1042] move isInChain to BlockchainReader and out of Branch interface --- .../io/iohk/ethereum/domain/BlockchainReader.scala | 10 ++++++++++ .../iohk/ethereum/domain/branch/BestBranch.scala | 14 +------------- .../io/iohk/ethereum/domain/branch/Branch.scala | 6 +----- .../iohk/ethereum/domain/branch/EmptyBranch$.scala | 5 +---- .../io/iohk/ethereum/ledger/BranchResolution.scala | 6 ++++-- .../io/iohk/ethereum/domain/BlockchainSpec.scala | 4 ++-- .../io/iohk/ethereum/ledger/LedgerTestSetup.scala | 4 ++-- 7 files changed, 21 insertions(+), 28 deletions(-) diff --git a/src/main/scala/io/iohk/ethereum/domain/BlockchainReader.scala b/src/main/scala/io/iohk/ethereum/domain/BlockchainReader.scala index 8cfcdbecdb..1e19b57b88 100644 --- a/src/main/scala/io/iohk/ethereum/domain/BlockchainReader.scala +++ b/src/main/scala/io/iohk/ethereum/domain/BlockchainReader.scala @@ -146,6 +146,16 @@ class BlockchainReader( case NewEmptyBranch => None } + /** Checks if given block hash is in this chain. (i.e. is an ancestor of the tip block) */ + def isInChain(branch: NewBranch, hash: ByteString): Boolean = branch match { + case BestBranchSubset(_, tipBlockNumber) => + (for { + header <- getBlockHeaderByHash(hash) if header.number <= tipBlockNumber + hash <- getHashByBlockNumber(branch, header.number) + } yield header.hash == hash).getOrElse(false) + case NewEmptyBranch => false + } + /** Allows to query for a block based on it's number * * @param number Block number diff --git a/src/main/scala/io/iohk/ethereum/domain/branch/BestBranch.scala b/src/main/scala/io/iohk/ethereum/domain/branch/BestBranch.scala index fbb525ba1c..b2bae03a2d 100644 --- a/src/main/scala/io/iohk/ethereum/domain/branch/BestBranch.scala +++ b/src/main/scala/io/iohk/ethereum/domain/branch/BestBranch.scala @@ -14,16 +14,4 @@ class BestBranch( tipBlockNumber: BigInt, bestChainBlockNumberMappingStorage: BlockNumberMappingStorage, blockchainReader: BlockchainReader -) extends Branch { - - /* The following assumptions are made in this class : - * - The whole branch exists in storage - * - The various metadata and index are consistent - */ - - override def isInChain(hash: ByteString): Boolean = - (for { - header <- blockchainReader.getBlockHeaderByHash(hash) if header.number <= tipBlockNumber - hash <- blockchainReader.getHashByBlockNumber(BestBranchSubset(tipBlockHash, tipBlockNumber), header.number) - } yield header.hash == hash).getOrElse(false) -} +) extends Branch {} diff --git a/src/main/scala/io/iohk/ethereum/domain/branch/Branch.scala b/src/main/scala/io/iohk/ethereum/domain/branch/Branch.scala index 4a530693e5..b9cee26165 100644 --- a/src/main/scala/io/iohk/ethereum/domain/branch/Branch.scala +++ b/src/main/scala/io/iohk/ethereum/domain/branch/Branch.scala @@ -9,8 +9,4 @@ case class BestBranchSubset(tipBlockHash: ByteString, tipBlockNumber: BigInt) ex case object NewEmptyBranch extends NewBranch /** An interface to manipulate blockchain branches */ -trait Branch { - - /** Checks if given block hash is in this chain. (i.e. is an ancestor of the tip block) */ - def isInChain(hash: ByteString): Boolean -} +trait Branch {} diff --git a/src/main/scala/io/iohk/ethereum/domain/branch/EmptyBranch$.scala b/src/main/scala/io/iohk/ethereum/domain/branch/EmptyBranch$.scala index d579c7d639..c01888d30a 100644 --- a/src/main/scala/io/iohk/ethereum/domain/branch/EmptyBranch$.scala +++ b/src/main/scala/io/iohk/ethereum/domain/branch/EmptyBranch$.scala @@ -1,7 +1,4 @@ package io.iohk.ethereum.domain.branch import akka.util.ByteString -object EmptyBranch extends Branch { - - override def isInChain(hash: ByteString): Boolean = false -} +object EmptyBranch extends Branch {} diff --git a/src/main/scala/io/iohk/ethereum/ledger/BranchResolution.scala b/src/main/scala/io/iohk/ethereum/ledger/BranchResolution.scala index 450ae109e4..035da7cc4b 100644 --- a/src/main/scala/io/iohk/ethereum/ledger/BranchResolution.scala +++ b/src/main/scala/io/iohk/ethereum/ledger/BranchResolution.scala @@ -16,8 +16,10 @@ class BranchResolution(blockchain: Blockchain, blockchainReader: BlockchainReade InvalidBranch } else { val knownParentOrGenesis = blockchainReader - .getBestBranch() - .isInChain(headers.head.parentHash) || headers.head.hash == blockchainReader.genesisHeader.hash + .isInChain( + blockchainReader.getBestBranchNew(), + headers.head.parentHash + ) || headers.head.hash == blockchainReader.genesisHeader.hash if (!knownParentOrGenesis) UnknownBranch diff --git a/src/test/scala/io/iohk/ethereum/domain/BlockchainSpec.scala b/src/test/scala/io/iohk/ethereum/domain/BlockchainSpec.scala index 7d68a8828b..8549b6f976 100644 --- a/src/test/scala/io/iohk/ethereum/domain/BlockchainSpec.scala +++ b/src/test/scala/io/iohk/ethereum/domain/BlockchainSpec.scala @@ -59,10 +59,10 @@ class BlockchainSpec extends AnyFlatSpec with Matchers with ScalaCheckPropertyCh saveAsBestBlock = true ) blockchainWriter.save(validBlock, Seq.empty, ChainWeight(100, 100), saveAsBestBlock = true) - blockchainReader.getBestBranch().isInChain(validBlock.hash) should ===(true) + blockchainReader.isInChain(blockchainReader.getBestBranchNew(), validBlock.hash) should ===(true) // simulation of node restart blockchain.saveBestKnownBlocks(validBlock.header.number - 1) - blockchainReader.getBestBranch().isInChain(validBlock.hash) should ===(false) + blockchainReader.isInChain(blockchainReader.getBestBranchNew(), validBlock.hash) should ===(false) } it should "be able to query a stored blockHeader by it's number" in new EphemBlockchainTestSetup { diff --git a/src/test/scala/io/iohk/ethereum/ledger/LedgerTestSetup.scala b/src/test/scala/io/iohk/ethereum/ledger/LedgerTestSetup.scala index 735c18dfd2..17799857c7 100644 --- a/src/test/scala/io/iohk/ethereum/ledger/LedgerTestSetup.scala +++ b/src/test/scala/io/iohk/ethereum/ledger/LedgerTestSetup.scala @@ -456,8 +456,8 @@ trait MockBlockchain extends MockFactory { self: TestSetupWithVmAndValidators => .expects(block, receipts, weight, saveAsBestBlock) .once() - def setHeaderInChain(hash: ByteString, result: Boolean = true): CallHandler1[ByteString, Boolean] = - (bestChain.isInChain _).expects(hash).returning(result) + def setHeaderInChain(hash: ByteString, result: Boolean = true) = + (blockchainReader.isInChain _).expects(*, hash).returning(result) def setBlockByNumber(number: BigInt, block: Option[Block]) = (blockchainReader.getBlockByNumber _).expects(*, number).returning(block) From 148d6a3ed3aba08465d2d3cfab4564f407641631 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20Richez?= Date: Thu, 22 Jul 2021 12:59:30 +0200 Subject: [PATCH 5/9] [ETCM-1042] remove old branch interface --- .../ethereum/sync/RegularSyncItSpec.scala | 6 ++-- .../sync/util/RegularSyncItSpecUtils.scala | 2 +- .../txExecTest/util/DumpChainApp.scala | 3 -- .../pow/validators/OmmersValidator.scala | 2 +- .../io/iohk/ethereum/domain/Blockchain.scala | 4 +-- .../ethereum/domain/BlockchainReader.scala | 36 ++++++------------- .../ethereum/domain/branch/BestBranch.scala | 17 --------- .../iohk/ethereum/domain/branch/Branch.scala | 9 ++--- .../ethereum/domain/branch/EmptyBranch$.scala | 4 --- .../jsonrpc/CheckpointingService.scala | 2 +- .../ethereum/jsonrpc/EthProofService.scala | 2 +- .../iohk/ethereum/jsonrpc/EthTxService.scala | 2 +- .../iohk/ethereum/jsonrpc/ResolveBlock.scala | 2 +- .../iohk/ethereum/jsonrpc/TestService.scala | 4 +-- .../io/iohk/ethereum/ledger/BlockImport.scala | 2 +- .../ethereum/ledger/BlockValidation.scala | 2 +- .../ethereum/ledger/BranchResolution.scala | 4 +-- .../testmode/TestEthBlockServiceWrapper.scala | 2 +- .../TransactionHistoryService.scala | 2 +- .../validators/StdOmmersValidatorSpec.scala | 2 +- .../iohk/ethereum/domain/BlockchainSpec.scala | 8 ++--- .../jsonrpc/CheckpointingServiceSpec.scala | 9 ++--- .../ethereum/ledger/LedgerTestSetup.scala | 16 +++++---- 23 files changed, 53 insertions(+), 89 deletions(-) delete mode 100644 src/main/scala/io/iohk/ethereum/domain/branch/BestBranch.scala delete mode 100644 src/main/scala/io/iohk/ethereum/domain/branch/EmptyBranch$.scala diff --git a/src/it/scala/io/iohk/ethereum/sync/RegularSyncItSpec.scala b/src/it/scala/io/iohk/ethereum/sync/RegularSyncItSpec.scala index e68f9f0c03..6551c0d579 100644 --- a/src/it/scala/io/iohk/ethereum/sync/RegularSyncItSpec.scala +++ b/src/it/scala/io/iohk/ethereum/sync/RegularSyncItSpec.scala @@ -125,7 +125,7 @@ class RegularSyncItSpec extends FreeSpecBase with Matchers with BeforeAndAfterAl _ <- peer1.startRegularSync() _ <- peer2.startRegularSync() _ <- peer2.addCheckpointedBlock( - peer2.blockchainReader.getBlockByNumber(peer2.blockchainReader.getBestBranchNew(), 25).get + peer2.blockchainReader.getBlockByNumber(peer2.blockchainReader.getBestBranch(), 25).get ) _ <- peer2.waitForRegularSyncLoadLastBlock(length) _ <- peer1.connectToPeers(Set(peer2.node)) @@ -183,8 +183,8 @@ class RegularSyncItSpec extends FreeSpecBase with Matchers with BeforeAndAfterAl ) ) ( - peer1.blockchainReader.getBlockByNumber(peer1.blockchainReader.getBestBranchNew(), blockNumer + 1), - peer2.blockchainReader.getBlockByNumber(peer2.blockchainReader.getBestBranchNew(), blockNumer + 1) + peer1.blockchainReader.getBlockByNumber(peer1.blockchainReader.getBestBranch(), blockNumer + 1), + peer2.blockchainReader.getBlockByNumber(peer2.blockchainReader.getBestBranch(), blockNumer + 1) ) match { case (Some(blockP1), Some(blockP2)) => assert(peer1.bl.getChainWeightByHash(blockP1.hash) == peer2.bl.getChainWeightByHash(blockP2.hash)) 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 1b8972b164..422c13df99 100644 --- a/src/it/scala/io/iohk/ethereum/sync/util/RegularSyncItSpecUtils.scala +++ b/src/it/scala/io/iohk/ethereum/sync/util/RegularSyncItSpecUtils.scala @@ -189,7 +189,7 @@ object RegularSyncItSpecUtils { Task(blockNumber match { case Some(bNumber) => blockchainReader - .getBlockByNumber(blockchainReader.getBestBranchNew(), bNumber) + .getBlockByNumber(blockchainReader.getBestBranch(), bNumber) .getOrElse(throw new RuntimeException(s"block by number: $bNumber doesn't exist")) case None => blockchainReader.getBestBlock().get }).flatMap { block => diff --git a/src/it/scala/io/iohk/ethereum/txExecTest/util/DumpChainApp.scala b/src/it/scala/io/iohk/ethereum/txExecTest/util/DumpChainApp.scala index d00a2616aa..3009015165 100644 --- a/src/it/scala/io/iohk/ethereum/txExecTest/util/DumpChainApp.scala +++ b/src/it/scala/io/iohk/ethereum/txExecTest/util/DumpChainApp.scala @@ -26,7 +26,6 @@ import io.iohk.ethereum.db.storage.pruning.PruningMode import io.iohk.ethereum.domain.BlockHeader.HeaderExtraFields.HefEmpty import io.iohk.ethereum.domain.Blockchain import io.iohk.ethereum.domain._ -import io.iohk.ethereum.domain.branch.Branch import io.iohk.ethereum.jsonrpc.ProofService.EmptyStorageValueProof import io.iohk.ethereum.jsonrpc.ProofService.StorageProof import io.iohk.ethereum.jsonrpc.ProofService.StorageProofKey @@ -102,8 +101,6 @@ object DumpChainApp val blockchain: Blockchain = new BlockchainMock(genesisHash) val blockchainReader: BlockchainReader = mock[BlockchainReader] - val bestChain: Branch = mock[Branch] - (blockchainReader.getBestBranch _).expects().anyNumberOfTimes().returning(bestChain) (blockchainReader.getHashByBlockNumber _).expects(*, *).returning(Some(genesisHash)) val nodeStatus: NodeStatus = diff --git a/src/main/scala/io/iohk/ethereum/consensus/pow/validators/OmmersValidator.scala b/src/main/scala/io/iohk/ethereum/consensus/pow/validators/OmmersValidator.scala index a978a5dc37..2dc1be729e 100644 --- a/src/main/scala/io/iohk/ethereum/consensus/pow/validators/OmmersValidator.scala +++ b/src/main/scala/io/iohk/ethereum/consensus/pow/validators/OmmersValidator.scala @@ -30,7 +30,7 @@ trait OmmersValidator { )(implicit blockchainConfig: BlockchainConfig): Either[OmmersError, OmmersValid] = { val getBlockHeaderByHash: ByteString => Option[BlockHeader] = blockchainReader.getBlockHeaderByHash - val bestBranch = blockchainReader.getBestBranchNew() + val bestBranch = blockchainReader.getBestBranch() val getNBlocksBack: (ByteString, Int) => List[Block] = (_, n) => ((blockNumber - n) until blockNumber).toList diff --git a/src/main/scala/io/iohk/ethereum/domain/Blockchain.scala b/src/main/scala/io/iohk/ethereum/domain/Blockchain.scala index 21d986b5fb..3d616e62b0 100644 --- a/src/main/scala/io/iohk/ethereum/domain/Blockchain.scala +++ b/src/main/scala/io/iohk/ethereum/domain/Blockchain.scala @@ -211,7 +211,7 @@ class BlockchainImpl( val latestCheckpointNumber = getLatestCheckpointBlockNumber() val blockNumberMappingUpdates = - if (blockchainReader.getHashByBlockNumber(blockchainReader.getBestBranchNew(), block.number).contains(blockHash)) + if (blockchainReader.getHashByBlockNumber(blockchainReader.getBestBranch(), block.number).contains(blockHash)) removeBlockNumberMapping(block.number) else blockNumberMappingStorage.emptyBatchUpdate @@ -280,7 +280,7 @@ class BlockchainImpl( ): BigInt = if (blockNumberToCheck > 0) { val maybePreviousCheckpointBlockNumber = for { - currentBlock <- blockchainReader.getBlockByNumber(blockchainReader.getBestBranchNew(), blockNumberToCheck) + currentBlock <- blockchainReader.getBlockByNumber(blockchainReader.getBestBranch(), blockNumberToCheck) if currentBlock.hasCheckpoint && currentBlock.number < latestCheckpointBlockNumber } yield currentBlock.number diff --git a/src/main/scala/io/iohk/ethereum/domain/BlockchainReader.scala b/src/main/scala/io/iohk/ethereum/domain/BlockchainReader.scala index 1e19b57b88..098b56e7fc 100644 --- a/src/main/scala/io/iohk/ethereum/domain/BlockchainReader.scala +++ b/src/main/scala/io/iohk/ethereum/domain/BlockchainReader.scala @@ -1,13 +1,16 @@ package io.iohk.ethereum.domain import akka.util.ByteString + import io.iohk.ethereum.db.storage.AppStateStorage import io.iohk.ethereum.db.storage.BlockBodiesStorage import io.iohk.ethereum.db.storage.BlockHeadersStorage import io.iohk.ethereum.db.storage.BlockNumberMappingStorage import io.iohk.ethereum.db.storage.ReceiptStorage import io.iohk.ethereum.db.storage.StateStorage -import io.iohk.ethereum.domain.branch.{BestBranch, BestBranchSubset, Branch, EmptyBranch, NewBranch, NewEmptyBranch} +import io.iohk.ethereum.domain.branch.BestBranchSubset +import io.iohk.ethereum.domain.branch.Branch +import io.iohk.ethereum.domain.branch.EmptyBranch import io.iohk.ethereum.mpt.MptNode import io.iohk.ethereum.utils.Logger @@ -69,28 +72,11 @@ class BlockchainReader( /** get the current best stored branch */ def getBestBranch(): Branch = { - val number = getBestBlockNumber() - blockNumberMappingStorage - .get(number) - .map(hash => - new BestBranch( - hash, - number, - blockNumberMappingStorage, - this - ) - ) - .getOrElse(EmptyBranch) - - } - - /** get the current best stored branch */ - def getBestBranchNew(): NewBranch = { val number = getBestBlockNumber() blockNumberMappingStorage .get(number) .map(hash => BestBranchSubset(hash, number)) - .getOrElse(NewEmptyBranch) + .getOrElse(EmptyBranch) } def getBestBlockNumber(): BigInt = { @@ -125,7 +111,7 @@ class BlockchainReader( getBlockByNumber(0).get /** Returns a block inside this branch based on its number */ - def getBlockByNumber(branch: NewBranch, number: BigInt): Option[Block] = branch match { + def getBlockByNumber(branch: Branch, number: BigInt): Option[Block] = branch match { case BestBranchSubset(_, tipBlockNumber) => if (tipBlockNumber >= number && number >= 0) { for { @@ -133,27 +119,27 @@ class BlockchainReader( block <- getBlockByHash(hash) } yield block } else None - case NewEmptyBranch => None + case EmptyBranch => None } /** Returns a block hash for the block at the given height if any */ - def getHashByBlockNumber(branch: NewBranch, number: BigInt): Option[ByteString] = branch match { + def getHashByBlockNumber(branch: Branch, number: BigInt): Option[ByteString] = branch match { case BestBranchSubset(_, tipBlockNumber) => if (tipBlockNumber >= number && number >= 0) { blockNumberMappingStorage.get(number) } else None - case NewEmptyBranch => None + case EmptyBranch => None } /** Checks if given block hash is in this chain. (i.e. is an ancestor of the tip block) */ - def isInChain(branch: NewBranch, hash: ByteString): Boolean = branch match { + def isInChain(branch: Branch, hash: ByteString): Boolean = branch match { case BestBranchSubset(_, tipBlockNumber) => (for { header <- getBlockHeaderByHash(hash) if header.number <= tipBlockNumber hash <- getHashByBlockNumber(branch, header.number) } yield header.hash == hash).getOrElse(false) - case NewEmptyBranch => false + case EmptyBranch => false } /** Allows to query for a block based on it's number diff --git a/src/main/scala/io/iohk/ethereum/domain/branch/BestBranch.scala b/src/main/scala/io/iohk/ethereum/domain/branch/BestBranch.scala deleted file mode 100644 index b2bae03a2d..0000000000 --- a/src/main/scala/io/iohk/ethereum/domain/branch/BestBranch.scala +++ /dev/null @@ -1,17 +0,0 @@ -package io.iohk.ethereum.domain.branch -import akka.util.ByteString - -import io.iohk.ethereum.db.storage.BlockNumberMappingStorage -import io.iohk.ethereum.domain.Block -import io.iohk.ethereum.domain.BlockHeader -import io.iohk.ethereum.domain.BlockchainReader - -/** A Branch instance which only works for the best canonical branch or a subset of this branch. - * This implementation uses the existing storage indexes to access blocks by number more efficiently. - */ -class BestBranch( - tipBlockHash: ByteString, - tipBlockNumber: BigInt, - bestChainBlockNumberMappingStorage: BlockNumberMappingStorage, - blockchainReader: BlockchainReader -) extends Branch {} diff --git a/src/main/scala/io/iohk/ethereum/domain/branch/Branch.scala b/src/main/scala/io/iohk/ethereum/domain/branch/Branch.scala index b9cee26165..c4597756f1 100644 --- a/src/main/scala/io/iohk/ethereum/domain/branch/Branch.scala +++ b/src/main/scala/io/iohk/ethereum/domain/branch/Branch.scala @@ -2,11 +2,8 @@ package io.iohk.ethereum.domain.branch import akka.util.ByteString -sealed trait NewBranch +sealed trait Branch -case class BestBranchSubset(tipBlockHash: ByteString, tipBlockNumber: BigInt) extends NewBranch +case class BestBranchSubset(tipBlockHash: ByteString, tipBlockNumber: BigInt) extends Branch -case object NewEmptyBranch extends NewBranch - -/** An interface to manipulate blockchain branches */ -trait Branch {} +case object EmptyBranch extends Branch diff --git a/src/main/scala/io/iohk/ethereum/domain/branch/EmptyBranch$.scala b/src/main/scala/io/iohk/ethereum/domain/branch/EmptyBranch$.scala deleted file mode 100644 index c01888d30a..0000000000 --- a/src/main/scala/io/iohk/ethereum/domain/branch/EmptyBranch$.scala +++ /dev/null @@ -1,4 +0,0 @@ -package io.iohk.ethereum.domain.branch -import akka.util.ByteString - -object EmptyBranch extends Branch {} diff --git a/src/main/scala/io/iohk/ethereum/jsonrpc/CheckpointingService.scala b/src/main/scala/io/iohk/ethereum/jsonrpc/CheckpointingService.scala index 3b38182afe..cdd5418d5a 100644 --- a/src/main/scala/io/iohk/ethereum/jsonrpc/CheckpointingService.scala +++ b/src/main/scala/io/iohk/ethereum/jsonrpc/CheckpointingService.scala @@ -36,7 +36,7 @@ class CheckpointingService( req.parentCheckpoint.forall(blockchainReader.getBlockHeaderByHash(_).exists(_.number < blockToReturnNum)) Task { - blockchainReader.getBlockByNumber(blockchainReader.getBestBranchNew(), blockToReturnNum) + blockchainReader.getBlockByNumber(blockchainReader.getBestBranch(), blockToReturnNum) }.flatMap { case Some(b) if isValidParent => Task.now(Right(GetLatestBlockResponse(Some(BlockInfo(b.hash, b.number))))) diff --git a/src/main/scala/io/iohk/ethereum/jsonrpc/EthProofService.scala b/src/main/scala/io/iohk/ethereum/jsonrpc/EthProofService.scala index 36e6713dd6..d2c1601e1d 100644 --- a/src/main/scala/io/iohk/ethereum/jsonrpc/EthProofService.scala +++ b/src/main/scala/io/iohk/ethereum/jsonrpc/EthProofService.scala @@ -205,7 +205,7 @@ class EthProofService( private def resolveBlock(blockParam: BlockParam): Either[JsonRpcError, ResolvedBlock] = { def getBlock(number: BigInt): Either[JsonRpcError, Block] = blockchainReader - .getBlockByNumber(blockchainReader.getBestBranchNew(), number) + .getBlockByNumber(blockchainReader.getBestBranch(), number) .toRight(JsonRpcError.InvalidParams(s"Block $number not found")) def getLatestBlock(): Either[JsonRpcError, Block] = diff --git a/src/main/scala/io/iohk/ethereum/jsonrpc/EthTxService.scala b/src/main/scala/io/iohk/ethereum/jsonrpc/EthTxService.scala index 1a613476a0..c453c75bf4 100644 --- a/src/main/scala/io/iohk/ethereum/jsonrpc/EthTxService.scala +++ b/src/main/scala/io/iohk/ethereum/jsonrpc/EthTxService.scala @@ -156,7 +156,7 @@ class EthTxService( val bestBlock = blockchainReader.getBestBlockNumber() Task { - val bestBranch = blockchainReader.getBestBranchNew() + val bestBranch = blockchainReader.getBestBranch() val gasPrice = ((bestBlock - blockDifference) to bestBlock) .flatMap(nb => blockchainReader.getBlockByNumber(bestBranch, nb)) .flatMap(_.body.transactionList) diff --git a/src/main/scala/io/iohk/ethereum/jsonrpc/ResolveBlock.scala b/src/main/scala/io/iohk/ethereum/jsonrpc/ResolveBlock.scala index 2a98719997..7270bb9d93 100644 --- a/src/main/scala/io/iohk/ethereum/jsonrpc/ResolveBlock.scala +++ b/src/main/scala/io/iohk/ethereum/jsonrpc/ResolveBlock.scala @@ -34,7 +34,7 @@ trait ResolveBlock { private def getBlock(number: BigInt): Either[JsonRpcError, Block] = blockchainReader - .getBlockByNumber(blockchainReader.getBestBranchNew(), number) + .getBlockByNumber(blockchainReader.getBestBranch(), number) .toRight(JsonRpcError.InvalidParams(s"Block $number not found")) private def getLatestBlock(): Either[JsonRpcError, Block] = diff --git a/src/main/scala/io/iohk/ethereum/jsonrpc/TestService.scala b/src/main/scala/io/iohk/ethereum/jsonrpc/TestService.scala index e70c423b41..d1bf3bdaaa 100644 --- a/src/main/scala/io/iohk/ethereum/jsonrpc/TestService.scala +++ b/src/main/scala/io/iohk/ethereum/jsonrpc/TestService.scala @@ -368,7 +368,7 @@ class TestService( val blockOpt = request.parameters.blockHashOrNumber .fold( - number => blockchainReader.getBlockByNumber(blockchainReader.getBestBranchNew(), number), + number => blockchainReader.getBlockByNumber(blockchainReader.getBestBranch(), number), blockHash => blockchainReader.getBlockByHash(blockHash) ) @@ -408,7 +408,7 @@ class TestService( val blockOpt = request.parameters.blockHashOrNumber .fold( - number => blockchainReader.getBlockByNumber(blockchainReader.getBestBranchNew(), number), + number => blockchainReader.getBlockByNumber(blockchainReader.getBestBranch(), number), hash => blockchainReader.getBlockByHash(hash) ) diff --git a/src/main/scala/io/iohk/ethereum/ledger/BlockImport.scala b/src/main/scala/io/iohk/ethereum/ledger/BlockImport.scala index c1fee6ace3..56a53573b6 100644 --- a/src/main/scala/io/iohk/ethereum/ledger/BlockImport.scala +++ b/src/main/scala/io/iohk/ethereum/ledger/BlockImport.scala @@ -318,7 +318,7 @@ class BlockImport( private def removeBlocksUntil(parent: ByteString, fromNumber: BigInt): List[BlockData] = { @tailrec def removeBlocksUntil(parent: ByteString, fromNumber: BigInt, acc: List[BlockData]): List[BlockData] = - blockchainReader.getBlockByNumber(blockchainReader.getBestBranchNew(), fromNumber) match { + blockchainReader.getBlockByNumber(blockchainReader.getBestBranch(), fromNumber) match { case Some(block) if block.header.hash == parent || fromNumber == 0 => acc diff --git a/src/main/scala/io/iohk/ethereum/ledger/BlockValidation.scala b/src/main/scala/io/iohk/ethereum/ledger/BlockValidation.scala index 1c2bd1fff1..02fd295a9e 100644 --- a/src/main/scala/io/iohk/ethereum/ledger/BlockValidation.scala +++ b/src/main/scala/io/iohk/ethereum/ledger/BlockValidation.scala @@ -44,7 +44,7 @@ class BlockValidation( val remaining = n - queuedBlocks.length - 1 val numbers = (block.header.number - remaining) until block.header.number - val bestBranch = blockchainReader.getBestBranchNew() + val bestBranch = blockchainReader.getBestBranch() val blocks = (numbers.toList.flatMap(nb => blockchainReader.getBlockByNumber(bestBranch, nb)) :+ block) ::: queuedBlocks blocks diff --git a/src/main/scala/io/iohk/ethereum/ledger/BranchResolution.scala b/src/main/scala/io/iohk/ethereum/ledger/BranchResolution.scala index 035da7cc4b..c8ea4a49f6 100644 --- a/src/main/scala/io/iohk/ethereum/ledger/BranchResolution.scala +++ b/src/main/scala/io/iohk/ethereum/ledger/BranchResolution.scala @@ -17,7 +17,7 @@ class BranchResolution(blockchain: Blockchain, blockchainReader: BlockchainReade } else { val knownParentOrGenesis = blockchainReader .isInChain( - blockchainReader.getBestBranchNew(), + blockchainReader.getBestBranch(), headers.head.parentHash ) || headers.head.hash == blockchainReader.genesisHeader.hash @@ -76,7 +76,7 @@ class BranchResolution(blockchain: Blockchain, blockchainReader: BlockchainReade } private def getTopBlocksFromNumber(from: BigInt): List[Block] = { - val bestBranch = blockchainReader.getBestBranchNew() + val bestBranch = blockchainReader.getBestBranch() (from to blockchainReader.getBestBlockNumber()) .flatMap(nb => blockchainReader.getBlockByNumber(bestBranch, nb)) .toList diff --git a/src/main/scala/io/iohk/ethereum/testmode/TestEthBlockServiceWrapper.scala b/src/main/scala/io/iohk/ethereum/testmode/TestEthBlockServiceWrapper.scala index 58904d633c..c8af2131cb 100644 --- a/src/main/scala/io/iohk/ethereum/testmode/TestEthBlockServiceWrapper.scala +++ b/src/main/scala/io/iohk/ethereum/testmode/TestEthBlockServiceWrapper.scala @@ -75,7 +75,7 @@ class TestEthBlockServiceWrapper( .getBlockByNumber(request) .map( _.map { blockByBlockResponse => - val bestBranch = blockchainReader.getBestBranchNew() + val bestBranch = blockchainReader.getBestBranch() val fullBlock = blockchainReader.getBlockByNumber(bestBranch, blockByBlockResponse.blockResponse.get.number).get BlockByNumberResponse(blockByBlockResponse.blockResponse.map(response => toEthResponse(fullBlock, response))) } diff --git a/src/main/scala/io/iohk/ethereum/transactions/TransactionHistoryService.scala b/src/main/scala/io/iohk/ethereum/transactions/TransactionHistoryService.scala index 9719576fe0..524aca3011 100644 --- a/src/main/scala/io/iohk/ethereum/transactions/TransactionHistoryService.scala +++ b/src/main/scala/io/iohk/ethereum/transactions/TransactionHistoryService.scala @@ -34,7 +34,7 @@ class TransactionHistoryService( val txnsFromBlocks = Observable .from(fromBlocks.reverse) .mapParallelOrdered(10)(blockNr => - Task(blockchainReader.getBlockByNumber(blockchainReader.getBestBranchNew(), blockNr)) + Task(blockchainReader.getBlockByNumber(blockchainReader.getBestBranch(), blockNr)) )( OverflowStrategy.Unbounded ) diff --git a/src/test/scala/io/iohk/ethereum/consensus/pow/validators/StdOmmersValidatorSpec.scala b/src/test/scala/io/iohk/ethereum/consensus/pow/validators/StdOmmersValidatorSpec.scala index c09c325f6e..388e18a488 100644 --- a/src/test/scala/io/iohk/ethereum/consensus/pow/validators/StdOmmersValidatorSpec.scala +++ b/src/test/scala/io/iohk/ethereum/consensus/pow/validators/StdOmmersValidatorSpec.scala @@ -81,7 +81,7 @@ class StdOmmersValidatorSpec extends AnyFlatSpec with Matchers with ScalaCheckPr val getNBlocksBack: (ByteString, Int) => List[Block] = (_, n) => ((ommersBlockNumber - n) until ommersBlockNumber).toList - .flatMap(nb => blockchainReader.getBlockByNumber(blockchainReader.getBestBranchNew(), nb)) + .flatMap(nb => blockchainReader.getBlockByNumber(blockchainReader.getBestBranch(), nb)) ommersValidator.validateOmmersAncestors( ommersBlockParentHash, diff --git a/src/test/scala/io/iohk/ethereum/domain/BlockchainSpec.scala b/src/test/scala/io/iohk/ethereum/domain/BlockchainSpec.scala index 8549b6f976..4b87784fc9 100644 --- a/src/test/scala/io/iohk/ethereum/domain/BlockchainSpec.scala +++ b/src/test/scala/io/iohk/ethereum/domain/BlockchainSpec.scala @@ -45,7 +45,7 @@ class BlockchainSpec extends AnyFlatSpec with Matchers with ScalaCheckPropertyCh val validBlock = Fixtures.Blocks.ValidBlock.block blockchainWriter.storeBlock(validBlock).commit() blockchain.saveBestKnownBlocks(validBlock.number) - val block = blockchainReader.getBlockByNumber(blockchainReader.getBestBranchNew(), validBlock.header.number) + val block = blockchainReader.getBlockByNumber(blockchainReader.getBestBranch(), validBlock.header.number) block.isDefined should ===(true) validBlock should ===(block.get) } @@ -59,10 +59,10 @@ class BlockchainSpec extends AnyFlatSpec with Matchers with ScalaCheckPropertyCh saveAsBestBlock = true ) blockchainWriter.save(validBlock, Seq.empty, ChainWeight(100, 100), saveAsBestBlock = true) - blockchainReader.isInChain(blockchainReader.getBestBranchNew(), validBlock.hash) should ===(true) + blockchainReader.isInChain(blockchainReader.getBestBranch(), validBlock.hash) should ===(true) // simulation of node restart blockchain.saveBestKnownBlocks(validBlock.header.number - 1) - blockchainReader.isInChain(blockchainReader.getBestBranchNew(), validBlock.hash) should ===(false) + blockchainReader.isInChain(blockchainReader.getBestBranch(), validBlock.hash) should ===(false) } it should "be able to query a stored blockHeader by it's number" in new EphemBlockchainTestSetup { @@ -75,7 +75,7 @@ class BlockchainSpec extends AnyFlatSpec with Matchers with ScalaCheckPropertyCh it should "not return a value if not stored" in new EphemBlockchainTestSetup { blockchainReader - .getBlockByNumber(blockchainReader.getBestBranchNew(), Fixtures.Blocks.ValidBlock.header.number) shouldBe None + .getBlockByNumber(blockchainReader.getBestBranch(), Fixtures.Blocks.ValidBlock.header.number) shouldBe None blockchainReader.getBlockByHash(Fixtures.Blocks.ValidBlock.header.hash) shouldBe None } diff --git a/src/test/scala/io/iohk/ethereum/jsonrpc/CheckpointingServiceSpec.scala b/src/test/scala/io/iohk/ethereum/jsonrpc/CheckpointingServiceSpec.scala index c0adbd5505..a49cef3402 100644 --- a/src/test/scala/io/iohk/ethereum/jsonrpc/CheckpointingServiceSpec.scala +++ b/src/test/scala/io/iohk/ethereum/jsonrpc/CheckpointingServiceSpec.scala @@ -3,13 +3,16 @@ package io.iohk.ethereum.jsonrpc import akka.actor.ActorSystem import akka.testkit.TestKit import akka.testkit.TestProbe + import monix.execution.Scheduler.Implicits.global + import org.scalacheck.Gen import org.scalamock.scalatest.MockFactory import org.scalatest.concurrent.ScalaFutures import org.scalatest.flatspec.AnyFlatSpecLike import org.scalatest.matchers.should.Matchers import org.scalatestplus.scalacheck.ScalaCheckPropertyChecks + import io.iohk.ethereum.Fixtures import io.iohk.ethereum.NormalPatience import io.iohk.ethereum.WithActorSystemShutDown @@ -20,7 +23,7 @@ import io.iohk.ethereum.domain.BlockBody import io.iohk.ethereum.domain.BlockchainImpl import io.iohk.ethereum.domain.BlockchainReader import io.iohk.ethereum.domain.Checkpoint -import io.iohk.ethereum.domain.branch.{Branch, NewEmptyBranch} +import io.iohk.ethereum.domain.branch.EmptyBranch import io.iohk.ethereum.jsonrpc.CheckpointingService._ import io.iohk.ethereum.ledger.BlockQueue @@ -183,9 +186,7 @@ class CheckpointingServiceSpec trait TestSetup { val blockchain: BlockchainImpl = mock[BlockchainImpl] val blockchainReader: BlockchainReader = mock[BlockchainReader] - val bestChain: Branch = mock[Branch] - (blockchainReader.getBestBranch _).expects().anyNumberOfTimes().returning(bestChain) - (blockchainReader.getBestBranchNew _).expects().anyNumberOfTimes().returning(NewEmptyBranch) + (blockchainReader.getBestBranch _).expects().anyNumberOfTimes().returning(EmptyBranch) val blockQueue: BlockQueue = mock[BlockQueue] val syncController: TestProbe = TestProbe() val checkpointBlockGenerator: CheckpointBlockGenerator = new CheckpointBlockGenerator() diff --git a/src/test/scala/io/iohk/ethereum/ledger/LedgerTestSetup.scala b/src/test/scala/io/iohk/ethereum/ledger/LedgerTestSetup.scala index 17799857c7..2002637fb7 100644 --- a/src/test/scala/io/iohk/ethereum/ledger/LedgerTestSetup.scala +++ b/src/test/scala/io/iohk/ethereum/ledger/LedgerTestSetup.scala @@ -2,15 +2,20 @@ package io.iohk.ethereum.ledger import akka.util.ByteString import akka.util.ByteString.{empty => bEmpty} + import cats.data.NonEmptyList + import monix.execution.Scheduler + import org.bouncycastle.crypto.AsymmetricCipherKeyPair import org.bouncycastle.crypto.params.ECPublicKeyParameters import org.bouncycastle.util.encoders.Hex import org.scalamock.handlers.CallHandler0 import org.scalamock.handlers.CallHandler1 +import org.scalamock.handlers.CallHandler2 import org.scalamock.handlers.CallHandler4 import org.scalamock.scalatest.MockFactory + import io.iohk.ethereum.Fixtures import io.iohk.ethereum.Mocks import io.iohk.ethereum.ObjectGenerators @@ -27,7 +32,8 @@ import io.iohk.ethereum.consensus.validators.BlockHeaderValidator import io.iohk.ethereum.crypto.generateKeyPair import io.iohk.ethereum.crypto.kec256 import io.iohk.ethereum.domain._ -import io.iohk.ethereum.domain.branch.{Branch, NewEmptyBranch} +import io.iohk.ethereum.domain.branch.Branch +import io.iohk.ethereum.domain.branch.EmptyBranch import io.iohk.ethereum.ledger.BlockExecutionError.ValidationAfterExecError import io.iohk.ethereum.ledger.PC import io.iohk.ethereum.ledger.PR @@ -412,11 +418,9 @@ trait TestSetupWithVmAndValidators extends EphemBlockchainTestSetup { trait MockBlockchain extends MockFactory { self: TestSetupWithVmAndValidators => //+ cake overrides - val bestChain: Branch = mock[Branch] override lazy val blockchainReader: BlockchainReader = mock[BlockchainReader] override lazy val blockchainWriter: BlockchainWriter = mock[BlockchainWriter] - (blockchainReader.getBestBranch _).expects().anyNumberOfTimes().returning(bestChain) - (blockchainReader.getBestBranchNew _).expects().anyNumberOfTimes().returning(NewEmptyBranch) + (blockchainReader.getBestBranch _).expects().anyNumberOfTimes().returning(EmptyBranch) override lazy val blockchain: BlockchainImpl = mock[BlockchainImpl] //- cake overrides @@ -456,10 +460,10 @@ trait MockBlockchain extends MockFactory { self: TestSetupWithVmAndValidators => .expects(block, receipts, weight, saveAsBestBlock) .once() - def setHeaderInChain(hash: ByteString, result: Boolean = true) = + def setHeaderInChain(hash: ByteString, result: Boolean = true): CallHandler2[Branch, ByteString, Boolean] = (blockchainReader.isInChain _).expects(*, hash).returning(result) - def setBlockByNumber(number: BigInt, block: Option[Block]) = + def setBlockByNumber(number: BigInt, block: Option[Block]): CallHandler2[Branch, BigInt, Option[Block]] = (blockchainReader.getBlockByNumber _).expects(*, number).returning(block) def setGenesisHeader(header: BlockHeader): Unit = From 7b21a362bb8fd742403926e4b6ed74aa5b9a333f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20Richez?= Date: Fri, 23 Jul 2021 09:52:32 +0200 Subject: [PATCH 6/9] [ETCM-1042] rename BestBranchSubset to BestBranch --- .../io/iohk/ethereum/domain/BlockchainReader.scala | 10 +++++----- .../scala/io/iohk/ethereum/domain/branch/Branch.scala | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/main/scala/io/iohk/ethereum/domain/BlockchainReader.scala b/src/main/scala/io/iohk/ethereum/domain/BlockchainReader.scala index 098b56e7fc..cb7d2143bc 100644 --- a/src/main/scala/io/iohk/ethereum/domain/BlockchainReader.scala +++ b/src/main/scala/io/iohk/ethereum/domain/BlockchainReader.scala @@ -8,7 +8,7 @@ import io.iohk.ethereum.db.storage.BlockHeadersStorage import io.iohk.ethereum.db.storage.BlockNumberMappingStorage import io.iohk.ethereum.db.storage.ReceiptStorage import io.iohk.ethereum.db.storage.StateStorage -import io.iohk.ethereum.domain.branch.BestBranchSubset +import io.iohk.ethereum.domain.branch.BestBranch import io.iohk.ethereum.domain.branch.Branch import io.iohk.ethereum.domain.branch.EmptyBranch import io.iohk.ethereum.mpt.MptNode @@ -75,7 +75,7 @@ class BlockchainReader( val number = getBestBlockNumber() blockNumberMappingStorage .get(number) - .map(hash => BestBranchSubset(hash, number)) + .map(hash => BestBranch(hash, number)) .getOrElse(EmptyBranch) } @@ -112,7 +112,7 @@ class BlockchainReader( /** Returns a block inside this branch based on its number */ def getBlockByNumber(branch: Branch, number: BigInt): Option[Block] = branch match { - case BestBranchSubset(_, tipBlockNumber) => + case BestBranch(_, tipBlockNumber) => if (tipBlockNumber >= number && number >= 0) { for { hash <- getHashByBlockNumber(number) @@ -124,7 +124,7 @@ class BlockchainReader( /** Returns a block hash for the block at the given height if any */ def getHashByBlockNumber(branch: Branch, number: BigInt): Option[ByteString] = branch match { - case BestBranchSubset(_, tipBlockNumber) => + case BestBranch(_, tipBlockNumber) => if (tipBlockNumber >= number && number >= 0) { blockNumberMappingStorage.get(number) } else None @@ -134,7 +134,7 @@ class BlockchainReader( /** Checks if given block hash is in this chain. (i.e. is an ancestor of the tip block) */ def isInChain(branch: Branch, hash: ByteString): Boolean = branch match { - case BestBranchSubset(_, tipBlockNumber) => + case BestBranch(_, tipBlockNumber) => (for { header <- getBlockHeaderByHash(hash) if header.number <= tipBlockNumber hash <- getHashByBlockNumber(branch, header.number) diff --git a/src/main/scala/io/iohk/ethereum/domain/branch/Branch.scala b/src/main/scala/io/iohk/ethereum/domain/branch/Branch.scala index c4597756f1..b7d2e5a3fd 100644 --- a/src/main/scala/io/iohk/ethereum/domain/branch/Branch.scala +++ b/src/main/scala/io/iohk/ethereum/domain/branch/Branch.scala @@ -4,6 +4,6 @@ import akka.util.ByteString sealed trait Branch -case class BestBranchSubset(tipBlockHash: ByteString, tipBlockNumber: BigInt) extends Branch +case class BestBranch(tipBlockHash: ByteString, tipBlockNumber: BigInt) extends Branch case object EmptyBranch extends Branch From b1f28b9fceb818ca9da192e8de187dc12079cb9d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20Richez?= Date: Tue, 27 Jul 2021 15:09:16 +0200 Subject: [PATCH 7/9] fix some warnings about unused values. --- .../scala/io/iohk/ethereum/jsonrpc/CheckpointingService.scala | 2 -- src/main/scala/io/iohk/ethereum/ledger/BlockImport.scala | 2 +- src/main/scala/io/iohk/ethereum/nodebuilder/NodeBuilder.scala | 1 - .../io/iohk/ethereum/jsonrpc/CheckpointingServiceSpec.scala | 2 +- 4 files changed, 2 insertions(+), 5 deletions(-) diff --git a/src/main/scala/io/iohk/ethereum/jsonrpc/CheckpointingService.scala b/src/main/scala/io/iohk/ethereum/jsonrpc/CheckpointingService.scala index cdd5418d5a..3cf40ca8fc 100644 --- a/src/main/scala/io/iohk/ethereum/jsonrpc/CheckpointingService.scala +++ b/src/main/scala/io/iohk/ethereum/jsonrpc/CheckpointingService.scala @@ -9,7 +9,6 @@ import io.iohk.ethereum.blockchain.sync.regular.RegularSync.NewCheckpoint import io.iohk.ethereum.consensus.blocks.CheckpointBlockGenerator import io.iohk.ethereum.crypto.ECDSASignature import io.iohk.ethereum.domain.Block -import io.iohk.ethereum.domain.Blockchain import io.iohk.ethereum.domain.BlockchainReader import io.iohk.ethereum.domain.Checkpoint import io.iohk.ethereum.ledger.BlockQueue @@ -17,7 +16,6 @@ import io.iohk.ethereum.utils.ByteStringUtils import io.iohk.ethereum.utils.Logger class CheckpointingService( - blockchain: Blockchain, blockchainReader: BlockchainReader, blockQueue: BlockQueue, checkpointBlockGenerator: CheckpointBlockGenerator, diff --git a/src/main/scala/io/iohk/ethereum/ledger/BlockImport.scala b/src/main/scala/io/iohk/ethereum/ledger/BlockImport.scala index 56a53573b6..788634fe9a 100644 --- a/src/main/scala/io/iohk/ethereum/ledger/BlockImport.scala +++ b/src/main/scala/io/iohk/ethereum/ledger/BlockImport.scala @@ -86,7 +86,7 @@ class BlockImport( private def isPossibleNewBestBlock(newBlock: BlockHeader, currentBestBlock: BlockHeader): Boolean = newBlock.parentHash == currentBestBlock.hash && newBlock.number == currentBestBlock.number + 1 - private def measureBlockMetrics(importResult: BlockImportResult)(implicit blockchainConfig: BlockchainConfig): Unit = + private def measureBlockMetrics(importResult: BlockImportResult): Unit = importResult match { case BlockImportedToTop(blockImportData) => blockImportData.foreach(blockData => BlockMetrics.measure(blockData.block, blockchainReader.getBlockByHash)) diff --git a/src/main/scala/io/iohk/ethereum/nodebuilder/NodeBuilder.scala b/src/main/scala/io/iohk/ethereum/nodebuilder/NodeBuilder.scala index 41b1d0c4aa..126e80b097 100644 --- a/src/main/scala/io/iohk/ethereum/nodebuilder/NodeBuilder.scala +++ b/src/main/scala/io/iohk/ethereum/nodebuilder/NodeBuilder.scala @@ -570,7 +570,6 @@ trait CheckpointingServiceBuilder { lazy val checkpointingService = new CheckpointingService( - blockchain, blockchainReader, blockQueue, checkpointBlockGenerator, diff --git a/src/test/scala/io/iohk/ethereum/jsonrpc/CheckpointingServiceSpec.scala b/src/test/scala/io/iohk/ethereum/jsonrpc/CheckpointingServiceSpec.scala index a49cef3402..d79f606c79 100644 --- a/src/test/scala/io/iohk/ethereum/jsonrpc/CheckpointingServiceSpec.scala +++ b/src/test/scala/io/iohk/ethereum/jsonrpc/CheckpointingServiceSpec.scala @@ -191,6 +191,6 @@ class CheckpointingServiceSpec val syncController: TestProbe = TestProbe() val checkpointBlockGenerator: CheckpointBlockGenerator = new CheckpointBlockGenerator() val service = - new CheckpointingService(blockchain, blockchainReader, blockQueue, checkpointBlockGenerator, syncController.ref) + new CheckpointingService(blockchainReader, blockQueue, checkpointBlockGenerator, syncController.ref) } } From f632017118e185ec3031dd8b9cb79f76ce852c36 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20Richez?= Date: Wed, 28 Jul 2021 16:23:24 +0200 Subject: [PATCH 8/9] fallback to stored best block number in getBestBranch --- src/main/scala/io/iohk/ethereum/domain/BlockchainReader.scala | 1 + 1 file changed, 1 insertion(+) diff --git a/src/main/scala/io/iohk/ethereum/domain/BlockchainReader.scala b/src/main/scala/io/iohk/ethereum/domain/BlockchainReader.scala index cb7d2143bc..2222d4a163 100644 --- a/src/main/scala/io/iohk/ethereum/domain/BlockchainReader.scala +++ b/src/main/scala/io/iohk/ethereum/domain/BlockchainReader.scala @@ -75,6 +75,7 @@ class BlockchainReader( val number = getBestBlockNumber() blockNumberMappingStorage .get(number) + .orElse(blockNumberMappingStorage.get(appStateStorage.getBestBlockNumber())) .map(hash => BestBranch(hash, number)) .getOrElse(EmptyBranch) } From 6130c0570ed5c05d6935ad19e866500da7615a07 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20Richez?= Date: Fri, 30 Jul 2021 09:42:51 +0200 Subject: [PATCH 9/9] review fixes --- .../ethereum/domain/BlockchainReader.scala | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/src/main/scala/io/iohk/ethereum/domain/BlockchainReader.scala b/src/main/scala/io/iohk/ethereum/domain/BlockchainReader.scala index 2222d4a163..64fc0c1d6d 100644 --- a/src/main/scala/io/iohk/ethereum/domain/BlockchainReader.scala +++ b/src/main/scala/io/iohk/ethereum/domain/BlockchainReader.scala @@ -113,14 +113,12 @@ class BlockchainReader( /** Returns a block inside this branch based on its number */ def getBlockByNumber(branch: Branch, number: BigInt): Option[Block] = branch match { - case BestBranch(_, tipBlockNumber) => - if (tipBlockNumber >= number && number >= 0) { - for { - hash <- getHashByBlockNumber(number) - block <- getBlockByHash(hash) - } yield block - } else None - case EmptyBranch => None + case BestBranch(_, tipBlockNumber) if tipBlockNumber >= number && number >= 0 => + for { + hash <- getHashByBlockNumber(number) + block <- getBlockByHash(hash) + } yield block + case EmptyBranch | BestBranch(_, _) => None } /** Returns a block hash for the block at the given height if any */ @@ -138,8 +136,8 @@ class BlockchainReader( case BestBranch(_, tipBlockNumber) => (for { header <- getBlockHeaderByHash(hash) if header.number <= tipBlockNumber - hash <- getHashByBlockNumber(branch, header.number) - } yield header.hash == hash).getOrElse(false) + hashFromBestChain <- getHashByBlockNumber(branch, header.number) + } yield header.hash == hashFromBestChain).getOrElse(false) case EmptyBranch => false }