Skip to content

Commit f98d2ef

Browse files
bsuiericbsuieric
authored
Merge pull request #924 from input-output-hk/ETCM-626-remove-best-block-option-get
ETCM-626 - remove Option.get when searching for the best block
2 parents 422e56c + cc2a8ae commit f98d2ef

20 files changed

+137
-122
lines changed

‎src/it/scala/io/iohk/ethereum/sync/RegularSyncItSpec.scala

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ class RegularSyncItSpec extends FreeSpecBase with Matchers with BeforeAndAfterAl
3737
_ <- peer2.connectToPeers(Set(peer1.node))
3838
_ <- peer2.waitForRegularSyncLoadLastBlock(blockNumber)
3939
} yield {
40-
assert(peer1.bl.getBestBlock().hash == peer2.bl.getBestBlock().hash)
40+
assert(peer1.bl.getBestBlock().get.hash == peer2.bl.getBestBlock().get.hash)
4141
}
4242
}
4343

@@ -52,7 +52,7 @@ class RegularSyncItSpec extends FreeSpecBase with Matchers with BeforeAndAfterAl
5252
_ <- peer2.connectToPeers(Set(peer1.node))
5353
_ <- peer2.waitForRegularSyncLoadLastBlock(blockHeadersPerRequest + 1)
5454
} yield {
55-
assert(peer1.bl.getBestBlock().hash == peer2.bl.getBestBlock().hash)
55+
assert(peer1.bl.getBestBlock().get.hash == peer2.bl.getBestBlock().get.hash)
5656
}
5757
}
5858
}
@@ -72,7 +72,7 @@ class RegularSyncItSpec extends FreeSpecBase with Matchers with BeforeAndAfterAl
7272
_ <- peer1.mineNewBlocks(100.milliseconds, 2)(IdentityUpdate)
7373
_ <- peer2.waitForRegularSyncLoadLastBlock(blockNumer + 4)
7474
} yield {
75-
assert(peer1.bl.getBestBlock().hash == peer2.bl.getBestBlock().hash)
75+
assert(peer1.bl.getBestBlock().get.hash == peer2.bl.getBestBlock().get.hash)
7676
}
7777
}
7878

@@ -94,8 +94,8 @@ class RegularSyncItSpec extends FreeSpecBase with Matchers with BeforeAndAfterAl
9494
_ <- peer2.waitForRegularSyncLoadLastBlock(blockNumer + 3)
9595
} yield {
9696
assert(
97-
peer1.bl.getChainWeightByHash(peer1.bl.getBestBlock().hash) == peer2.bl.getChainWeightByHash(
98-
peer2.bl.getBestBlock().hash
97+
peer1.bl.getChainWeightByHash(peer1.bl.getBestBlock().get.hash) == peer2.bl.getChainWeightByHash(
98+
peer2.bl.getBestBlock().get.hash
9999
)
100100
)
101101
(peer1.bl.getBlockByNumber(blockNumer + 1), peer2.bl.getBlockByNumber(blockNumer + 1)) match {

‎src/it/scala/io/iohk/ethereum/sync/util/CommonFakePeer.scala

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -246,7 +246,7 @@ abstract class CommonFakePeer(peerName: String, fakePeerCustomConfig: FakePeerCu
246246
}
247247

248248
def getCurrentState(): BlockchainState = {
249-
val bestBlock = bl.getBestBlock()
249+
val bestBlock = bl.getBestBlock().get
250250
val currentWorldState = getMptForBlock(bestBlock)
251251
val currentWeight = bl.getChainWeightByHash(bestBlock.hash).get
252252
BlockchainState(bestBlock, currentWorldState, currentWeight)
@@ -301,13 +301,13 @@ abstract class CommonFakePeer(peerName: String, fakePeerCustomConfig: FakePeerCu
301301
n: BigInt
302302
)(updateWorldForBlock: (BigInt, InMemoryWorldStateProxy) => InMemoryWorldStateProxy): Task[Unit] = {
303303
Task(bl.getBestBlock()).flatMap { block =>
304-
if (block.number >= n) {
304+
if (block.get.number >= n) {
305305
Task(())
306306
} else {
307307
Task {
308-
val currentWeight = bl.getChainWeightByHash(block.hash).get
309-
val currentWolrd = getMptForBlock(block)
310-
val (newBlock, newWeight, _) = createChildBlock(block, currentWeight, currentWolrd)(updateWorldForBlock)
308+
val currentWeight = bl.getChainWeightByHash(block.get.hash).get
309+
val currentWolrd = getMptForBlock(block.get)
310+
val (newBlock, newWeight, _) = createChildBlock(block.get, currentWeight, currentWolrd)(updateWorldForBlock)
311311
bl.save(newBlock, Seq(), newWeight, saveAsBestBlock = true)
312312
broadcastBlock(newBlock, newWeight)
313313
}.flatMap(_ => importBlocksUntil(n)(updateWorldForBlock))

‎src/it/scala/io/iohk/ethereum/sync/util/FastSyncItSpecUtils.scala

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ object FastSyncItSpecUtils {
5555
// Reads whole trie into memory, if the trie lacks nodes in storage it will be None
5656
def getBestBlockTrie(): Option[MptNode] = {
5757
Try {
58-
val bestBlock = bl.getBestBlock()
58+
val bestBlock = bl.getBestBlock().get
5959
val bestStateRoot = bestBlock.header.stateRoot
6060
MptTraversals.parseTrieIntoMemory(
6161
HashNode(bestStateRoot.toArray),
@@ -99,7 +99,7 @@ object FastSyncItSpecUtils {
9999

100100
def startWithState(): Task[Unit] = {
101101
Task {
102-
val currentBest = bl.getBestBlock().header
102+
val currentBest = bl.getBestBlock().get.header
103103
val safeTarget = currentBest.number + syncConfig.fastSyncBlockValidationX
104104
val nextToValidate = currentBest.number + 1
105105
val syncState =

‎src/it/scala/io/iohk/ethereum/sync/util/RegularSyncItSpecUtils.scala

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ object RegularSyncItSpecUtils {
9292
Task(blockNumber match {
9393
case Some(bNumber) =>
9494
bl.getBlockByNumber(bNumber).getOrElse(throw new RuntimeException(s"block by number: $bNumber doesn't exist"))
95-
case None => bl.getBestBlock()
95+
case None => bl.getBestBlock().get
9696
}).flatMap { block =>
9797
Task {
9898
val currentWeight = bl
@@ -112,7 +112,7 @@ object RegularSyncItSpecUtils {
112112
def mineNewBlock(
113113
plusDifficulty: BigInt = 0
114114
)(updateWorldForBlock: (BigInt, InMemoryWorldStateProxy) => InMemoryWorldStateProxy): Task[Unit] = Task {
115-
val block: Block = bl.getBestBlock()
115+
val block: Block = bl.getBestBlock().get
116116
val currentWeight = bl
117117
.getChainWeightByHash(block.hash)
118118
.getOrElse(throw new RuntimeException(s"ChainWeight by hash: ${block.hash} doesn't exist"))

‎src/it/scala/io/iohk/ethereum/txExecTest/util/DumpChainApp.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -212,7 +212,7 @@ class BlockchainMock(genesisHash: ByteString) extends Blockchain {
212212

213213
def saveBestKnownBlocks(bestBlockNumber: BigInt, latestCheckpointNumber: Option[BigInt] = None): Unit = ???
214214

215-
def getBestBlock(): Block = ???
215+
def getBestBlock(): Option[Block] = ???
216216

217217
override def save(block: Block, receipts: Seq[Receipt], weight: ChainWeight, saveAsBestBlock: Boolean): Unit = ???
218218

‎src/main/scala/io/iohk/ethereum/consensus/ethash/EthashBlockCreator.scala

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,7 @@ class EthashBlockCreator(
3232
withTransactions: Boolean = true,
3333
initialWorldStateBeforeExecution: Option[InMemoryWorldStateProxy] = None
3434
): Task[PendingBlockAndState] = {
35-
val transactions =
36-
if (withTransactions) getTransactionsFromPool else Task.now(PendingTransactionsResponse(Nil))
35+
val transactions = if (withTransactions) getTransactionsFromPool else Task.now(PendingTransactionsResponse(Nil))
3736
Task.parZip2(getOmmersFromPool(parentBlock.hash), transactions).map { case (ommers, pendingTxs) =>
3837
blockGenerator.generateBlock(
3938
parentBlock,

‎src/main/scala/io/iohk/ethereum/consensus/ethash/EthashMiner.scala

Lines changed: 38 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -56,39 +56,46 @@ class EthashMiner(
5656
}
5757

5858
def processMining(): Unit = {
59-
val parentBlock = blockchain.getBestBlock()
60-
val blockNumber = parentBlock.header.number.toLong + 1
61-
val epoch = EthashUtils.epoch(blockNumber, blockCreator.blockchainConfig.ecip1099BlockNumber.toLong)
62-
val (dag, dagSize) = calculateDagSize(blockNumber, epoch)
63-
64-
blockCreator
65-
.getBlockForMining(parentBlock)
66-
.map { case PendingBlockAndState(PendingBlock(block, _), _) =>
67-
val headerHash = crypto.kec256(BlockHeader.getEncodedWithoutNonce(block.header))
68-
val startTime = System.nanoTime()
69-
val mineResult =
70-
mine(headerHash, block.header.difficulty.toLong, dagSize, dag, blockCreator.miningConfig.mineRounds)
71-
val time = System.nanoTime() - startTime
72-
//FIXME: consider not reporting hash rate when time delta is zero
73-
val hashRate = if (time > 0) (mineResult.triedHashes.toLong * 1000000000) / time else Long.MaxValue
74-
ethMiningService.submitHashRate(SubmitHashRateRequest(hashRate, ByteString("mantis-miner")))
75-
mineResult match {
76-
case MiningSuccessful(_, pow, nonce) =>
77-
log.info(
78-
s"Mining successful with ${ByteStringUtils.hash2string(pow.mixHash)} and nonce ${ByteStringUtils.hash2string(nonce)}"
79-
)
80-
syncController ! SyncProtocol.MinedBlock(
81-
block.copy(header = block.header.copy(nonce = nonce, mixHash = pow.mixHash))
82-
)
83-
case _ => log.info("Mining unsuccessful")
84-
}
85-
self ! ProcessMining
86-
}
87-
.onErrorHandle { ex =>
88-
log.error(ex, "Unable to get block for mining")
59+
blockchain.getBestBlock() match {
60+
case Some(blockValue) =>
61+
blockCreator
62+
.getBlockForMining(blockValue)
63+
.map {
64+
case PendingBlockAndState(PendingBlock(block, _), _) => {
65+
val blockNumber = block.header.number.toLong + 1
66+
val epoch = EthashUtils.epoch(blockNumber, blockCreator.blockchainConfig.ecip1099BlockNumber.toLong)
67+
val (dag, dagSize) = calculateDagSize(blockNumber, epoch)
68+
val headerHash = crypto.kec256(BlockHeader.getEncodedWithoutNonce(block.header))
69+
val startTime = System.nanoTime()
70+
val mineResult =
71+
mine(headerHash, block.header.difficulty.toLong, dagSize, dag, blockCreator.miningConfig.mineRounds)
72+
val time = System.nanoTime() - startTime
73+
//FIXME: consider not reporting hash rate when time delta is zero
74+
val hashRate = if (time > 0) (mineResult.triedHashes.toLong * 1000000000) / time else Long.MaxValue
75+
ethMiningService.submitHashRate(SubmitHashRateRequest(hashRate, ByteString("mantis-miner")))
76+
mineResult match {
77+
case MiningSuccessful(_, pow, nonce) =>
78+
log.info(
79+
s"Mining successful with ${ByteStringUtils.hash2string(pow.mixHash)} and nonce ${ByteStringUtils.hash2string(nonce)}"
80+
)
81+
syncController ! SyncProtocol.MinedBlock(
82+
block.copy(header = block.header.copy(nonce = nonce, mixHash = pow.mixHash))
83+
)
84+
case _ => log.info("Mining unsuccessful")
85+
}
86+
self ! ProcessMining
87+
}
88+
}
89+
.onErrorHandle { ex =>
90+
log.error(ex, "Unable to get block for mining")
91+
context.system.scheduler.scheduleOnce(10.seconds, self, ProcessMining)
92+
}
93+
.runAsyncAndForget
94+
case None => {
95+
log.error("Unable to get block for mining, getBestBlock() returned None")
8996
context.system.scheduler.scheduleOnce(10.seconds, self, ProcessMining)
9097
}
91-
.runAsyncAndForget
98+
}
9299
}
93100

94101
private def calculateDagSize(blockNumber: Long, epoch: Long): (Array[Array[Int]], Long) = {

‎src/main/scala/io/iohk/ethereum/consensus/ethash/MockedMiner.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ class MockedMiner(
4545
}
4646
case None =>
4747
val parentBlock = blockchain.getBestBlock()
48-
startMiningBlocks(mineBlocks, parentBlock)
48+
startMiningBlocks(mineBlocks, parentBlock.get)
4949
}
5050
}
5151

‎src/main/scala/io/iohk/ethereum/domain/Blockchain.scala

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,7 @@ trait Blockchain {
142142

143143
def getBestBlockNumber(): BigInt
144144

145-
def getBestBlock(): Block
145+
def getBestBlock(): Option[Block]
146146

147147
def getLatestCheckpointBlockNumber(): BigInt
148148

@@ -276,10 +276,10 @@ class BlockchainImpl(
276276
override def getLatestCheckpointBlockNumber(): BigInt =
277277
bestKnownBlockAndLatestCheckpoint.get().latestCheckpointNumber
278278

279-
override def getBestBlock(): Block = {
279+
override def getBestBlock(): Option[Block] = {
280280
val bestBlockNumber = getBestBlockNumber()
281281
log.debug("Trying to get best block with number {}", bestBlockNumber)
282-
getBlockByNumber(bestBlockNumber).get
282+
getBlockByNumber(bestBlockNumber)
283283
}
284284

285285
override def getAccount(address: Address, blockNumber: BigInt): Option[Account] =

‎src/main/scala/io/iohk/ethereum/jsonrpc/EthMiningService.scala

Lines changed: 22 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -78,26 +78,29 @@ class EthMiningService(
7878
def getWork(req: GetWorkRequest): ServiceResponse[GetWorkResponse] =
7979
consensus.ifEthash(ethash => {
8080
reportActive()
81-
val bestBlock = blockchain.getBestBlock()
82-
val response: ServiceResponse[GetWorkResponse] =
83-
Task.parZip2(getOmmersFromPool(bestBlock.hash), getTransactionsFromPool).map { case (ommers, pendingTxs) =>
84-
val blockGenerator = ethash.blockGenerator
85-
val PendingBlockAndState(pb, _) = blockGenerator.generateBlock(
86-
bestBlock,
87-
pendingTxs.pendingTransactions.map(_.stx.tx),
88-
consensusConfig.coinbase,
89-
ommers.headers,
90-
None
91-
)
92-
Right(
93-
GetWorkResponse(
94-
powHeaderHash = ByteString(kec256(BlockHeader.getEncodedWithoutNonce(pb.block.header))),
95-
dagSeed = EthashUtils.seed(pb.block.header.number.toLong),
96-
target = ByteString((BigInt(2).pow(256) / pb.block.header.difficulty).toByteArray)
81+
blockchain.getBestBlock() match {
82+
case Some(block) =>
83+
Task.parZip2(getOmmersFromPool(block.hash), getTransactionsFromPool).map { case (ommers, pendingTxs) =>
84+
val blockGenerator = ethash.blockGenerator
85+
val PendingBlockAndState(pb, _) = blockGenerator.generateBlock(
86+
block,
87+
pendingTxs.pendingTransactions.map(_.stx.tx),
88+
consensusConfig.coinbase,
89+
ommers.headers,
90+
None
9791
)
98-
)
99-
}
100-
response
92+
Right(
93+
GetWorkResponse(
94+
powHeaderHash = ByteString(kec256(BlockHeader.getEncodedWithoutNonce(pb.block.header))),
95+
dagSeed = EthashUtils.seed(pb.block.header.number.toLong),
96+
target = ByteString((BigInt(2).pow(256) / pb.block.header.difficulty).toByteArray)
97+
)
98+
)
99+
}
100+
case None =>
101+
log.error("Getting current best block failed")
102+
Task.now(Left(JsonRpcError.InternalError))
103+
}
101104
})(Task.now(Left(JsonRpcError.ConsensusIsNotEthash)))
102105

103106
def submitWork(req: SubmitWorkRequest): ServiceResponse[SubmitWorkResponse] =

‎src/main/scala/io/iohk/ethereum/jsonrpc/JsonRpcError.scala

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ object JsonRpcError extends JsonMethodsImplicits {
4343

4444
def executionError(reasons: List[EthCustomError]): JsonRpcError = JsonRpcError(3, "Execution error", reasons)
4545
val NodeNotFound = executionError(List(EthCustomError.DoesntExist("State node")))
46+
val BlockNotFound = executionError(List(EthCustomError.DoesntExist("Block")))
4647

4748
// Custom errors based on proposal https://eth.wiki/json-rpc/json-rpc-error-codes-improvement-proposal
4849
sealed abstract class EthCustomError private (val code: Int, val message: String)

‎src/main/scala/io/iohk/ethereum/jsonrpc/QAService.scala

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -44,11 +44,15 @@ class QAService(
4444
def generateCheckpoint(
4545
req: GenerateCheckpointRequest
4646
): ServiceResponse[GenerateCheckpointResponse] = {
47-
Task {
48-
val hash = req.blockHash.getOrElse(blockchain.getBestBlock().hash)
49-
val checkpoint = generateCheckpoint(hash, req.privateKeys)
50-
syncController ! NewCheckpoint(hash, checkpoint.signatures)
51-
Right(GenerateCheckpointResponse(checkpoint))
47+
val hash = req.blockHash.orElse(blockchain.getBestBlock().map(_.hash))
48+
hash match {
49+
case Some(hashValue) =>
50+
Task {
51+
val checkpoint = generateCheckpoint(hashValue, req.privateKeys)
52+
syncController ! NewCheckpoint(hashValue, checkpoint.signatures)
53+
Right(GenerateCheckpointResponse(checkpoint))
54+
}
55+
case None => Task.now(Left(JsonRpcError.BlockNotFound))
5256
}
5357
}
5458

‎src/main/scala/io/iohk/ethereum/jsonrpc/TestService.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ class TestService(
112112

113113
def mineBlocks(request: MineBlocksRequest): ServiceResponse[MineBlocksResponse] = {
114114
def mineBlock(): Task[Unit] = {
115-
getBlockForMining(blockchain.getBestBlock()).map { blockForMining =>
115+
getBlockForMining(blockchain.getBestBlock().get).map { blockForMining =>
116116
val res = testLedgerWrapper.ledger.importBlock(blockForMining.block)
117117
log.info("Block mining result: " + res)
118118
pendingTransactionsManager ! PendingTransactionsManager.ClearPendingTransactions

‎src/main/scala/io/iohk/ethereum/ledger/Ledger.scala

Lines changed: 24 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -119,31 +119,32 @@ class LedgerImpl(
119119

120120
override def importBlock(
121121
block: Block
122-
)(implicit blockExecutionScheduler: Scheduler): Task[BlockImportResult] = {
123-
124-
val currentBestBlock = blockchain.getBestBlock()
125-
126-
if (isBlockADuplicate(block.header, currentBestBlock.header.number)) {
127-
Task(log.debug(s"Ignoring duplicate block: (${block.idTag})"))
128-
.map(_ => DuplicateBlock)
129-
} else {
130-
val hash = currentBestBlock.header.hash
131-
blockchain.getChainWeightByHash(hash) match {
132-
case Some(weight) =>
133-
val importResult = if (isPossibleNewBestBlock(block.header, currentBestBlock.header)) {
134-
blockImport.importToTop(block, currentBestBlock, weight)
135-
} else {
136-
blockImport.reorganise(block, currentBestBlock, weight)
122+
)(implicit blockExecutionScheduler: Scheduler): Task[BlockImportResult] =
123+
blockchain.getBestBlock() match {
124+
case Some(bestBlock) =>
125+
if (isBlockADuplicate(block.header, bestBlock.header.number)) {
126+
Task(log.debug(s"Ignoring duplicate block: (${block.idTag})"))
127+
.map(_ => DuplicateBlock)
128+
} else {
129+
val hash = bestBlock.header.hash
130+
blockchain.getChainWeightByHash(hash) match {
131+
case Some(weight) =>
132+
val importResult = if (isPossibleNewBestBlock(block.header, bestBlock.header)) {
133+
blockImport.importToTop(block, bestBlock, weight)
134+
} else {
135+
blockImport.reorganise(block, bestBlock, weight)
136+
}
137+
importResult.foreach(measureBlockMetrics)
138+
importResult
139+
case None =>
140+
log.error(s"Getting total difficulty for current best block with hash: $hash failed")
141+
Task.now(BlockImportFailed(s"Couldn't get total difficulty for current best block with hash: $hash"))
137142
}
138-
importResult.foreach(measureBlockMetrics)
139-
importResult
140-
141-
case None =>
142-
Task.now(BlockImportFailed(s"Couldn't get total difficulty for current best block with hash: $hash"))
143-
144-
}
143+
}
144+
case None =>
145+
log.error("Getting current best block failed")
146+
Task.now(BlockImportFailed(s"Couldn't find the current best block"))
145147
}
146-
}
147148

148149
private def isBlockADuplicate(block: BlockHeader, currentBestBlockNumber: BigInt): Boolean = {
149150
val hash = block.hash

0 commit comments

Comments
 (0)