Skip to content

Commit

Permalink
[ETCM-1042] remove old branch interface
Browse files Browse the repository at this point in the history
  • Loading branch information
Aurélien Richez committed Jul 29, 2021
1 parent c1d6edd commit 8211467
Show file tree
Hide file tree
Showing 23 changed files with 53 additions and 89 deletions.
6 changes: 3 additions & 3 deletions src/it/scala/io/iohk/ethereum/sync/RegularSyncItSpec.scala
Expand Up @@ -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))
Expand Down Expand Up @@ -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))
Expand Down
Expand Up @@ -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 =>
Expand Down
Expand Up @@ -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
Expand Down Expand Up @@ -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 =
Expand Down
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions src/main/scala/io/iohk/ethereum/domain/Blockchain.scala
Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand Down
36 changes: 11 additions & 25 deletions 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

Expand Down Expand Up @@ -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 = {
Expand Down Expand Up @@ -125,35 +111,35 @@ 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 {
hash <- getHashByBlockNumber(number)
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
Expand Down
17 changes: 0 additions & 17 deletions src/main/scala/io/iohk/ethereum/domain/branch/BestBranch.scala

This file was deleted.

9 changes: 3 additions & 6 deletions src/main/scala/io/iohk/ethereum/domain/branch/Branch.scala
Expand Up @@ -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

This file was deleted.

Expand Up @@ -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)))))
Expand Down
Expand Up @@ -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] =
Expand Down
2 changes: 1 addition & 1 deletion src/main/scala/io/iohk/ethereum/jsonrpc/EthTxService.scala
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion src/main/scala/io/iohk/ethereum/jsonrpc/ResolveBlock.scala
Expand Up @@ -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] =
Expand Down
4 changes: 2 additions & 2 deletions src/main/scala/io/iohk/ethereum/jsonrpc/TestService.scala
Expand Up @@ -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)
)

Expand Down Expand Up @@ -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)
)

Expand Down
2 changes: 1 addition & 1 deletion src/main/scala/io/iohk/ethereum/ledger/BlockImport.scala
Expand Up @@ -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

Expand Down
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions src/main/scala/io/iohk/ethereum/ledger/BranchResolution.scala
Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand Down
Expand Up @@ -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)))
}
Expand Down
Expand Up @@ -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
)
Expand Down
Expand Up @@ -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,
Expand Down
8 changes: 4 additions & 4 deletions src/test/scala/io/iohk/ethereum/domain/BlockchainSpec.scala
Expand Up @@ -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)
}
Expand All @@ -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 {
Expand All @@ -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
}

Expand Down
Expand Up @@ -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
Expand All @@ -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

Expand Down Expand Up @@ -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()
Expand Down

0 comments on commit 8211467

Please sign in to comment.