Skip to content

[ETCM-995] move isInChain into BlockchainBranch #1052

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 16, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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 = ???
Expand Down
12 changes: 0 additions & 12 deletions src/main/scala/io/iohk/ethereum/domain/Blockchain.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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 =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
2 changes: 2 additions & 0 deletions src/main/scala/io/iohk/ethereum/domain/branch/Branch.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
10 changes: 8 additions & 2 deletions src/test/scala/io/iohk/ethereum/domain/BlockchainSpec.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down