From 02d16cec8ddeb10b554e075bc9aebdbbc0abd0fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20Richez?= Date: Thu, 8 Jul 2021 11:29:12 +0200 Subject: [PATCH] move isInChain into BlockchainBranch --- .../iohk/ethereum/txExecTest/util/DumpChainApp.scala | 2 -- .../scala/io/iohk/ethereum/domain/Blockchain.scala | 12 ------------ .../io/iohk/ethereum/domain/branch/BestBranch.scala | 6 ++++++ .../io/iohk/ethereum/domain/branch/Branch.scala | 2 ++ .../iohk/ethereum/domain/branch/EmptyBranch$.scala | 2 ++ .../io/iohk/ethereum/ledger/BranchResolution.scala | 3 ++- .../io/iohk/ethereum/domain/BlockchainSpec.scala | 10 ++++++++-- .../io/iohk/ethereum/ledger/LedgerTestSetup.scala | 2 +- 8 files changed, 21 insertions(+), 18 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 40126b1709..6c7b18f42a 100644 --- a/src/it/scala/io/iohk/ethereum/txExecTest/util/DumpChainApp.scala +++ b/src/it/scala/io/iohk/ethereum/txExecTest/util/DumpChainApp.scala @@ -208,8 +208,6 @@ class BlockchainMock(genesisHash: ByteString) extends Blockchain { override def getLatestCheckpointBlockNumber(): BigInt = ??? - override def isInChain(hash: NodeHash): Boolean = ??? - override def getBackingMptStorage(blockNumber: BigInt): MptStorage = ??? override def getReadOnlyMptStorage(): MptStorage = ??? diff --git a/src/main/scala/io/iohk/ethereum/domain/Blockchain.scala b/src/main/scala/io/iohk/ethereum/domain/Blockchain.scala index 81cbe4f255..beabb0169d 100644 --- a/src/main/scala/io/iohk/ethereum/domain/Blockchain.scala +++ b/src/main/scala/io/iohk/ethereum/domain/Blockchain.scala @@ -78,12 +78,6 @@ trait Blockchain { def saveBestKnownBlocks(bestBlockNumber: BigInt, latestCheckpointNumber: Option[BigInt] = None): Unit - /** Strict check if given block hash is in chain - * Using any of getXXXByHash is not always accurate - after restart the best block is often lower than before restart - * The result of that is returning data of blocks which we don't consider as a part of the chain anymore - * @param hash block hash - */ - def isInChain(hash: ByteString): Boolean } class BlockchainImpl( @@ -100,12 +94,6 @@ class BlockchainImpl( ) extends Blockchain with Logger { - override def isInChain(hash: ByteString): Boolean = - (for { - header <- blockchainReader.getBlockHeaderByHash(hash) if header.number <= blockchainReader.getBestBlockNumber() - hash <- blockchainReader.getBestBranch().getHashByBlockNumber(header.number) - } yield header.hash == hash).getOrElse(false) - override def getChainWeightByHash(blockhash: ByteString): Option[ChainWeight] = chainWeightStorage.get(blockhash) override def getLatestCheckpointBlockNumber(): BigInt = 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 7c5aea1079..52d63bf0bc 100644 --- a/src/main/scala/io/iohk/ethereum/domain/branch/BestBranch.scala +++ b/src/main/scala/io/iohk/ethereum/domain/branch/BestBranch.scala @@ -32,4 +32,10 @@ class BestBranch( if (tipBlockHeader.number >= number && number >= 0) { bestChainBlockNumberMappingStorage.get(number) } else None + + override def isInChain(hash: ByteString): Boolean = + (for { + header <- blockchainReader.getBlockHeaderByHash(hash) if header.number <= tipBlockHeader.number + hash <- getHashByBlockNumber(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 118910b6e9..af1b23850f 100644 --- a/src/main/scala/io/iohk/ethereum/domain/branch/Branch.scala +++ b/src/main/scala/io/iohk/ethereum/domain/branch/Branch.scala @@ -13,4 +13,6 @@ 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 0f72afe21b..53f79212f5 100644 --- a/src/main/scala/io/iohk/ethereum/domain/branch/EmptyBranch$.scala +++ b/src/main/scala/io/iohk/ethereum/domain/branch/EmptyBranch$.scala @@ -8,4 +8,6 @@ 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/ledger/BranchResolution.scala b/src/main/scala/io/iohk/ethereum/ledger/BranchResolution.scala index 9e2ecc42f4..e568a5284b 100644 --- a/src/main/scala/io/iohk/ethereum/ledger/BranchResolution.scala +++ b/src/main/scala/io/iohk/ethereum/ledger/BranchResolution.scala @@ -15,7 +15,8 @@ class BranchResolution(blockchain: Blockchain, blockchainReader: BlockchainReade if (!doHeadersFormChain(headers)) { InvalidBranch } else { - val knownParentOrGenesis = blockchain + val knownParentOrGenesis = blockchainReader + .getBestBranch() .isInChain(headers.head.parentHash) || headers.head.hash == blockchainReader.genesisHeader.hash if (!knownParentOrGenesis) diff --git a/src/test/scala/io/iohk/ethereum/domain/BlockchainSpec.scala b/src/test/scala/io/iohk/ethereum/domain/BlockchainSpec.scala index d1cdd062cf..7fc47c4fe7 100644 --- a/src/test/scala/io/iohk/ethereum/domain/BlockchainSpec.scala +++ b/src/test/scala/io/iohk/ethereum/domain/BlockchainSpec.scala @@ -52,11 +52,17 @@ class BlockchainSpec extends AnyFlatSpec with Matchers with ScalaCheckPropertyCh it should "be able to do strict check of block existence in the chain" in new EphemBlockchainTestSetup { val validBlock = Fixtures.Blocks.ValidBlock.block + blockchainWriter.save( + validBlock.copy(header = validBlock.header.copy(number = validBlock.number - 1)), + Seq.empty, + ChainWeight(100, 100), + saveAsBestBlock = true + ) blockchainWriter.save(validBlock, Seq.empty, ChainWeight(100, 100), saveAsBestBlock = true) - blockchain.isInChain(validBlock.hash) === false + blockchainReader.getBestBranch().isInChain(validBlock.hash) should ===(true) // simulation of node restart blockchain.saveBestKnownBlocks(validBlock.header.number - 1) - blockchain.isInChain(validBlock.hash) should ===(false) + blockchainReader.getBestBranch().isInChain(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 73f8e93f29..61bc72c2e0 100644 --- a/src/test/scala/io/iohk/ethereum/ledger/LedgerTestSetup.scala +++ b/src/test/scala/io/iohk/ethereum/ledger/LedgerTestSetup.scala @@ -460,7 +460,7 @@ trait MockBlockchain extends MockFactory { self: TestSetupWithVmAndValidators => .once() def setHeaderInChain(hash: ByteString, result: Boolean = true): CallHandler1[ByteString, Boolean] = - (blockchain.isInChain _).expects(hash).returning(result) + (bestChain.isInChain _).expects(hash).returning(result) def setBlockByNumber(number: BigInt, block: Option[Block]): CallHandler1[BigInt, Option[Block]] = (bestChain.getBlockByNumber _).expects(number).returning(block)