Skip to content

Commit 65b93ad

Browse files
mirkoAlicmirkoAlic
authored
[ETCM-177] Simplify ommers handling (#745)
1 parent 17358d2 commit 65b93ad

File tree

15 files changed

+147
-277
lines changed

15 files changed

+147
-277
lines changed

‎src/main/scala/io/iohk/ethereum/blockchain/sync/regular/BlockImporter.scala

+5-9
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ import io.iohk.ethereum.ledger._
1515
import io.iohk.ethereum.mpt.MerklePatriciaTrie.MissingNodeException
1616
import io.iohk.ethereum.network.PeerId
1717
import io.iohk.ethereum.network.p2p.messages.CommonMessages.NewBlock
18-
import io.iohk.ethereum.ommers.OmmersPool.{AddOmmers, RemoveOmmers}
18+
import io.iohk.ethereum.ommers.OmmersPool.AddOmmers
1919
import io.iohk.ethereum.transactions.PendingTransactionsManager
2020
import io.iohk.ethereum.transactions.PendingTransactionsManager.{AddUncheckedTransactions, RemoveTransactions}
2121
import io.iohk.ethereum.utils.ByteStringUtils
@@ -221,17 +221,16 @@ class BlockImporter(
221221
case BlockImportedToTop(importedBlocksData) =>
222222
val (blocks, tds) = importedBlocksData.map(data => (data.block, data.td)).unzip
223223
broadcastBlocks(blocks, tds)
224-
updateTxAndOmmerPools(importedBlocksData.map(_.block), Seq.empty)
224+
updateTxPool(importedBlocksData.map(_.block), Seq.empty)
225225

226-
case BlockEnqueued =>
227-
ommersPool ! AddOmmers(block.header)
226+
case BlockEnqueued => ()
228227

229228
case DuplicateBlock => ()
230229

231230
case UnknownParent => () // This is normal when receiving broadcast blocks
232231

233232
case ChainReorganised(oldBranch, newBranch, totalDifficulties) =>
234-
updateTxAndOmmerPools(newBranch, oldBranch)
233+
updateTxPool(newBranch, oldBranch)
235234
broadcastBlocks(newBranch, totalDifficulties)
236235

237236
case BlockImportFailed(error) =>
@@ -256,12 +255,9 @@ class BlockImporter(
256255

257256
private def broadcastNewBlocks(blocks: List[NewBlock]): Unit = broadcaster ! BroadcastBlocks(blocks)
258257

259-
private def updateTxAndOmmerPools(blocksAdded: Seq[Block], blocksRemoved: Seq[Block]): Unit = {
260-
blocksRemoved.headOption.foreach(block => ommersPool ! AddOmmers(block.header))
258+
private def updateTxPool(blocksAdded: Seq[Block], blocksRemoved: Seq[Block]): Unit = {
261259
blocksRemoved.foreach(block => pendingTransactionsManager ! AddUncheckedTransactions(block.body.transactionList))
262-
263260
blocksAdded.foreach { block =>
264-
ommersPool ! RemoveOmmers(block.header :: block.body.uncleNodesList.toList)
265261
pendingTransactionsManager ! RemoveTransactions(block.body.transactionList)
266262
}
267263
}

‎src/main/scala/io/iohk/ethereum/consensus/blocks/BlockGenerator.scala

+1-2
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package io.iohk.ethereum.consensus.blocks
22

33
import io.iohk.ethereum.domain.{Address, Block, SignedTransaction}
4-
import io.iohk.ethereum.ledger.BlockPreparationError
54

65
/**
76
* We use a `BlockGenerator` to create the next block.
@@ -41,7 +40,7 @@ trait BlockGenerator {
4140
transactions: Seq[SignedTransaction],
4241
beneficiary: Address,
4342
x: X
44-
): Either[BlockPreparationError, PendingBlock]
43+
): PendingBlock
4544
}
4645

4746
/**

‎src/main/scala/io/iohk/ethereum/consensus/blocks/NoOmmersBlockGenerator.scala

+3-3
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ package io.iohk.ethereum.consensus.blocks
33
import io.iohk.ethereum.consensus.ConsensusConfig
44
import io.iohk.ethereum.consensus.difficulty.DifficultyCalculator
55
import io.iohk.ethereum.domain._
6-
import io.iohk.ethereum.ledger.{BlockPreparationError, BlockPreparator}
6+
import io.iohk.ethereum.ledger.BlockPreparator
77
import io.iohk.ethereum.utils.BlockchainConfig
88

99
abstract class NoOmmersBlockGenerator(
@@ -43,14 +43,14 @@ abstract class NoOmmersBlockGenerator(
4343
transactions: Seq[SignedTransaction],
4444
beneficiary: Address,
4545
x: Nil.type
46-
): Either[BlockPreparationError, PendingBlock] = {
46+
): PendingBlock = {
4747

4848
val pHeader = parent.header
4949
val blockNumber = pHeader.number + 1
5050

5151
val prepared = prepareBlock(parent, transactions, beneficiary, blockNumber, blockPreparator, x)
5252
cache.updateAndGet((t: List[PendingBlockAndState]) => (prepared :: t).take(blockCacheSize))
5353

54-
Right(prepared.pendingBlock)
54+
prepared.pendingBlock
5555
}
5656
}

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

+3-5
Original file line numberDiff line numberDiff line change
@@ -29,11 +29,9 @@ class EthashBlockCreator(
2929
val transactions =
3030
if (withTransactions) getTransactionsFromPool else Future.successful(PendingTransactionsResponse(Nil))
3131
getOmmersFromPool(parentBlock.hash).zip(transactions).flatMap { case (ommers, pendingTxs) =>
32-
blockGenerator
33-
.generateBlock(parentBlock, pendingTxs.pendingTransactions.map(_.stx.tx), coinbase, ommers.headers) match {
34-
case Right(pb) => Future.successful(pb)
35-
case Left(err) => Future.failed(new RuntimeException(s"Error while generating block for mining: $err"))
36-
}
32+
val pendingBlock = blockGenerator
33+
.generateBlock(parentBlock, pendingTxs.pendingTransactions.map(_.stx.tx), coinbase, ommers.headers)
34+
Future.successful(pendingBlock)
3735
}
3836
}
3937

‎src/main/scala/io/iohk/ethereum/consensus/ethash/blocks/EthashBlockGenerator.scala

+11-13
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import io.iohk.ethereum.consensus.difficulty.DifficultyCalculator
99
import io.iohk.ethereum.consensus.ethash.validators.ValidatorsExecutor
1010
import io.iohk.ethereum.crypto.kec256
1111
import io.iohk.ethereum.domain._
12-
import io.iohk.ethereum.ledger.{BlockPreparationError, BlockPreparator}
12+
import io.iohk.ethereum.ledger.BlockPreparator
1313
import io.iohk.ethereum.utils.BlockchainConfig
1414

1515
/** Internal API, used for testing (especially mocks) */
@@ -73,25 +73,23 @@ class EthashBlockGeneratorImpl(
7373
transactions: Seq[SignedTransaction],
7474
beneficiary: Address,
7575
x: Ommers
76-
): Either[BlockPreparationError, PendingBlock] = {
76+
): PendingBlock = {
7777
val pHeader = parent.header
7878
val blockNumber = pHeader.number + 1
7979
val parentHash = pHeader.hash
8080

81-
val ommersV = validators.ommersValidator
81+
val ommers = validators.ommersValidator.validate(parentHash, blockNumber, x, blockchain) match {
82+
case Left(_) => emptyX
83+
case Right(_) => x
84+
}
8285

83-
val result: Either[InvalidOmmers, PendingBlockAndState] = ommersV
84-
.validate(parentHash, blockNumber, x, blockchain)
85-
.left
86-
.map(InvalidOmmers)
87-
.flatMap { _ =>
88-
val prepared = prepareBlock(parent, transactions, beneficiary, blockNumber, blockPreparator, x)
89-
Right(prepared)
90-
}
86+
val prepared = prepareBlock(parent, transactions, beneficiary, blockNumber, blockPreparator, ommers)
9187

92-
result.right.foreach(b => cache.updateAndGet((t: List[PendingBlockAndState]) => (b :: t).take(blockCacheSize)))
88+
cache.updateAndGet { t: List[PendingBlockAndState] =>
89+
(prepared :: t).take(blockCacheSize)
90+
}
9391

94-
result.map(_.pendingBlock)
92+
prepared.pendingBlock
9593
}
9694

9795
def withBlockTimestampProvider(blockTimestampProvider: BlockTimestampProvider): EthashBlockGeneratorImpl =

‎src/main/scala/io/iohk/ethereum/consensus/ethash/blocks/package.scala

-4
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,7 @@
11
package io.iohk.ethereum.consensus.ethash
22

3-
import io.iohk.ethereum.consensus.ethash.validators.OmmersValidator.OmmersError
43
import io.iohk.ethereum.domain.BlockHeader
54
import io.iohk.ethereum.domain.BlockHeaderImplicits._
6-
import io.iohk.ethereum.ledger.BlockPreparationError
75
import io.iohk.ethereum.rlp.{RLPEncodeable, RLPList, RLPSerializable}
86

97
package object blocks {
@@ -19,6 +17,4 @@ package object blocks {
1917
implicit class OmmersSeqEnc(blockHeaders: Seq[BlockHeader]) extends RLPSerializable {
2018
override def toRLPEncodable: RLPEncodeable = RLPList(blockHeaders.map(_.toRLPEncodable): _*)
2119
}
22-
23-
final case class InvalidOmmers(reason: OmmersError) extends BlockPreparationError
2420
}

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

+16-19
Original file line numberDiff line numberDiff line change
@@ -539,27 +539,24 @@ class EthService(
539539
import io.iohk.ethereum.consensus.ethash.EthashUtils.{epoch, seed}
540540

541541
val bestBlock = blockchain.getBestBlock()
542-
getOmmersFromPool(bestBlock.hash).zip(getTransactionsFromPool).map { case (ommers, pendingTxs) =>
543-
val blockGenerator = ethash.blockGenerator
544-
blockGenerator.generateBlock(
545-
bestBlock,
546-
pendingTxs.pendingTransactions.map(_.stx.tx),
547-
consensusConfig.coinbase,
548-
ommers.headers
549-
) match {
550-
case Right(pb) =>
551-
Right(
552-
GetWorkResponse(
553-
powHeaderHash = ByteString(kec256(BlockHeader.getEncodedWithoutNonce(pb.block.header))),
554-
dagSeed = seed(epoch(pb.block.header.number.toLong)),
555-
target = ByteString((BigInt(2).pow(256) / pb.block.header.difficulty).toByteArray)
556-
)
542+
val response: ServiceResponse[GetWorkResponse] =
543+
getOmmersFromPool(bestBlock.hash).zip(getTransactionsFromPool).map { case (ommers, pendingTxs) =>
544+
val blockGenerator = ethash.blockGenerator
545+
val pb = blockGenerator.generateBlock(
546+
bestBlock,
547+
pendingTxs.pendingTransactions.map(_.stx.tx),
548+
consensusConfig.coinbase,
549+
ommers.headers
550+
)
551+
Right(
552+
GetWorkResponse(
553+
powHeaderHash = ByteString(kec256(BlockHeader.getEncodedWithoutNonce(pb.block.header))),
554+
dagSeed = seed(epoch(pb.block.header.number.toLong)),
555+
target = ByteString((BigInt(2).pow(256) / pb.block.header.difficulty).toByteArray)
557556
)
558-
case Left(err) =>
559-
log.error(s"unable to prepare block because of $err")
560-
Left(JsonRpcErrors.InternalError)
557+
)
561558
}
562-
}
559+
response
563560
})(Future.successful(Left(JsonRpcErrors.ConsensusIsNotEthash)))
564561

565562
private def getOmmersFromPool(parentBlockHash: ByteString): Future[OmmersPool.Ommers] =

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

+3-5
Original file line numberDiff line numberDiff line change
@@ -150,15 +150,13 @@ class TestService(
150150
.mapTo[PendingTransactionsResponse]
151151
.recover { case _ => PendingTransactionsResponse(Nil) }
152152
.flatMap { pendingTxs =>
153-
consensus.blockGenerator.generateBlock(
153+
val pb = consensus.blockGenerator.generateBlock(
154154
parentBlock,
155155
pendingTxs.pendingTransactions.map(_.stx.tx),
156156
etherbase,
157157
Nil
158-
) match {
159-
case Right(pb) => Future.successful(pb)
160-
case Left(err) => Future.failed(new RuntimeException(s"Error while generating block for mining: $err"))
161-
}
158+
)
159+
Future.successful(pb)
162160
}
163161
}
164162
}

‎src/main/scala/io/iohk/ethereum/ommers/OmmersPool.scala

+1-12
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import akka.util.ByteString
44
import akka.actor.{Actor, ActorLogging, Props}
55
import org.bouncycastle.util.encoders.Hex
66
import io.iohk.ethereum.domain.{BlockHeader, Blockchain}
7-
import io.iohk.ethereum.ommers.OmmersPool.{AddOmmers, GetOmmers, RemoveOmmers}
7+
import io.iohk.ethereum.ommers.OmmersPool.{AddOmmers, GetOmmers}
88
import scala.annotation.tailrec
99

1010
class OmmersPool(blockchain: Blockchain, ommersPoolSize: Int, ommerGenerationLimit: Int, returnedOmmersSizeLimit: Int)
@@ -18,11 +18,6 @@ class OmmersPool(blockchain: Blockchain, ommersPoolSize: Int, ommerGenerationLim
1818
ommersPool = (ommers ++ ommersPool).take(ommersPoolSize).distinct
1919
logStatus(event = "Ommers after add", ommers = ommersPool)
2020

21-
case RemoveOmmers(ommers) =>
22-
val toDelete = ommers.map(_.hash).toSet
23-
ommersPool = ommersPool.filter(b => !toDelete.contains(b.hash))
24-
logStatus(event = "Ommers after remove", ommers = ommersPool)
25-
2621
case GetOmmers(parentBlockHash) =>
2722
val ancestors = collectAncestors(parentBlockHash, ommerGenerationLimit)
2823
val ommers = ommersPool
@@ -81,12 +76,6 @@ object OmmersPool {
8176
def apply(b: BlockHeader*): AddOmmers = AddOmmers(b.toList)
8277
}
8378

84-
case class RemoveOmmers(ommers: List[BlockHeader])
85-
86-
object RemoveOmmers {
87-
def apply(b: BlockHeader*): RemoveOmmers = RemoveOmmers(b.toList)
88-
}
89-
9079
case class GetOmmers(parentBlockHash: ByteString)
9180

9281
case class Ommers(headers: Seq[BlockHeader])

‎src/test/scala/io/iohk/ethereum/blockchain/sync/regular/RegularSyncSpec.scala

-16
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@ import io.iohk.ethereum.domain.BlockHeaderImplicits._
2121
import io.iohk.ethereum.network.p2p.messages.PV62._
2222
import io.iohk.ethereum.network.p2p.messages.PV63.{GetNodeData, NodeData}
2323
import io.iohk.ethereum.network.{EtcPeerManagerActor, Peer, PeerEventBusActor}
24-
import io.iohk.ethereum.ommers.OmmersPool.RemoveOmmers
2524
import io.iohk.ethereum.utils.Config.SyncConfig
2625
import org.scalamock.scalatest.MockFactory
2726
import org.scalatest.BeforeAndAfterEach
@@ -406,13 +405,6 @@ class RegularSyncSpec
406405
case _ => false
407406
}
408407
}
409-
"update ommers for imported block" in new OnTopFixture(testSystem) {
410-
goToTop()
411-
412-
sendNewBlock()
413-
414-
ommersPool.expectMsg(RemoveOmmers(newBlock.header :: newBlock.body.uncleNodesList.toList))
415-
}
416408
"fetch hashes if received NewHashes message" in new OnTopFixture(testSystem) {
417409
goToTop()
418410

@@ -480,14 +472,6 @@ class RegularSyncSpec
480472
case _ => false
481473
}
482474
}
483-
484-
"update ommers after successful import" in new OnTopFixture(testSystem) {
485-
goToTop()
486-
487-
regularSync ! RegularSync.MinedBlock(newBlock)
488-
489-
ommersPool.expectMsg(RemoveOmmers(newBlock.header :: newBlock.body.uncleNodesList.toList))
490-
}
491475
}
492476

493477
"handling checkpoints" should {

0 commit comments

Comments
 (0)