Skip to content

Commit 1667f33

Browse files
KonradStaniecKonradStaniec
committed
Merge remote-tracking branch 'origin/develop' into etcm-213/relad-bloom-after-restart
2 parents f46fd50 + cd5ae33 commit 1667f33

21 files changed

+202
-288
lines changed

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

Lines changed: 5 additions & 9 deletions
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

Lines changed: 1 addition & 2 deletions
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/CheckpointBlockGenerator.scala

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package io.iohk.ethereum.consensus.blocks
22

33
import akka.util.ByteString
4+
import io.iohk.ethereum.crypto.ECDSASignatureImplicits.ECDSASignatureOrdering
45
import io.iohk.ethereum.domain.BlockHeader.HeaderExtraFields.HefPostEcip1097
56
import io.iohk.ethereum.domain._
67
import io.iohk.ethereum.ledger.BloomFilter
@@ -12,6 +13,7 @@ class CheckpointBlockGenerator {
1213
// we are using a predictable value for timestamp so that each federation node generates identical block
1314
// see ETCM-173
1415
val timestamp = parent.header.unixTimestamp + 1
16+
val checkpointWithSortedSignatures = Checkpoint(checkpoint.signatures.sorted)
1517

1618
val header = BlockHeader(
1719
parentHash = parent.hash,
@@ -29,7 +31,7 @@ class CheckpointBlockGenerator {
2931
gasUsed = UInt256.Zero,
3032
mixHash = ByteString.empty,
3133
nonce = ByteString.empty,
32-
extraFields = HefPostEcip1097(false, Some(checkpoint))
34+
extraFields = HefPostEcip1097(false, Some(checkpointWithSortedSignatures))
3335
)
3436

3537
Block(header, BlockBody.empty)

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

Lines changed: 3 additions & 3 deletions
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

Lines changed: 3 additions & 5 deletions
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

Lines changed: 11 additions & 13 deletions
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

Lines changed: 0 additions & 4 deletions
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/consensus/validators/BlockHeaderValidator.scala

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ object BlockHeaderError {
4242
case class HeaderWrongNumberOfCheckpointSignatures(sigCount: Int) extends BlockHeaderError
4343
case class HeaderInvalidCheckpointSignatures(invalidSignaturesWithPublics: Seq[(ECDSASignature, Option[String])])
4444
extends BlockHeaderError
45+
case object HeaderInvalidOrderOfCheckpointSignatures extends BlockHeaderError
4546
case class HeaderFieldNotEmptyError(msg: String) extends BlockHeaderError
4647
case class HeaderNotMatchParentError(msg: String) extends BlockHeaderError
4748
case object CheckpointHeaderTreasuryOptOutError extends BlockHeaderError

‎src/main/scala/io/iohk/ethereum/consensus/validators/BlockWithCheckpointHeaderValidator.scala

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ class BlockWithCheckpointHeaderValidator(blockchainConfig: BlockchainConfig) {
1313

1414
def validate(blockHeader: BlockHeader, parentHeader: BlockHeader): Either[BlockHeaderError, BlockHeaderValid] = {
1515
for {
16+
_ <- validateLexicographicalOrderOfSignatures(blockHeader)
1617
_ <- validateCheckpointSignatures(blockHeader, parentHeader)
1718
_ <- validateEmptyFields(blockHeader)
1819
_ <- validateFieldsCopiedFromParent(blockHeader, parentHeader)
@@ -22,6 +23,19 @@ class BlockWithCheckpointHeaderValidator(blockchainConfig: BlockchainConfig) {
2223
} yield BlockHeaderValid
2324
}
2425

26+
private def validateLexicographicalOrderOfSignatures(
27+
header: BlockHeader
28+
): Either[BlockHeaderError, BlockHeaderValid] = {
29+
import io.iohk.ethereum.crypto.ECDSASignatureImplicits.ECDSASignatureOrdering
30+
header.checkpoint
31+
.map { checkpoint =>
32+
if (checkpoint.signatures == checkpoint.signatures.sorted) {
33+
Right(BlockHeaderValid)
34+
} else Left(HeaderInvalidOrderOfCheckpointSignatures)
35+
}
36+
.getOrElse(Left(BlockWithCheckpointHeaderValidator.NoCheckpointInHeaderError))
37+
}
38+
2539
/**
2640
* Validates [[io.iohk.ethereum.domain.BlockHeader.checkpoint]] signatures
2741
*
@@ -64,7 +78,7 @@ class BlockWithCheckpointHeaderValidator(blockchainConfig: BlockchainConfig) {
6478
else
6579
Right(BlockHeaderValid)
6680
}
67-
.getOrElse(Left(HeaderUnexpectedError("Attempted to validate a checkpoint on a block without a checkpoint")))
81+
.getOrElse(Left(BlockWithCheckpointHeaderValidator.NoCheckpointInHeaderError))
6882
}
6983

7084
/**
@@ -157,3 +171,9 @@ class BlockWithCheckpointHeaderValidator(blockchainConfig: BlockchainConfig) {
157171
else Left(CheckpointHeaderTreasuryOptOutError)
158172

159173
}
174+
175+
object BlockWithCheckpointHeaderValidator {
176+
val NoCheckpointInHeaderError: BlockHeaderError = HeaderUnexpectedError(
177+
"Attempted to validate a checkpoint on a block without a checkpoint"
178+
)
179+
}

‎src/main/scala/io/iohk/ethereum/crypto/ECDSASignature.scala

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -183,4 +183,5 @@ object ECDSASignatureImplicits {
183183
}
184184
}
185185

186+
implicit val ECDSASignatureOrdering: Ordering[ECDSASignature] = Ordering.by(sig => (sig.r, sig.s, sig.v))
186187
}

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

Lines changed: 16 additions & 19 deletions
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

Lines changed: 3 additions & 5 deletions
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

Lines changed: 1 addition & 12 deletions
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

Lines changed: 0 additions & 16 deletions
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 {

‎src/test/scala/io/iohk/ethereum/checkpointing/CheckpointingTestHelpers.scala

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import io.iohk.ethereum.domain.BlockHeader.HeaderExtraFields.HefPostEcip1097
66
import io.iohk.ethereum.domain._
77
import io.iohk.ethereum.ledger.BloomFilter
88
import org.bouncycastle.crypto.AsymmetricCipherKeyPair
9+
import io.iohk.ethereum.crypto.ECDSASignatureImplicits.ECDSASignatureOrdering
910

1011
object CheckpointingTestHelpers {
1112
def createBlockWithCheckpoint(
@@ -45,5 +46,5 @@ object CheckpointingTestHelpers {
4546
): Seq[ECDSASignature] =
4647
keys.map { k =>
4748
ECDSASignature.sign(hash.toArray, k)
48-
}
49+
}.sorted
4950
}

0 commit comments

Comments
 (0)