Skip to content

Commit a05c76a

Browse files
AnastasiiaLAnastasiiaL
committed
ETCM-636: None.get on best block tests
1 parent 3927be3 commit a05c76a

File tree

7 files changed

+173
-20
lines changed

7 files changed

+173
-20
lines changed

‎project/Dependencies.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ object Dependencies {
5959

6060
val testing: Seq[ModuleID] = Seq(
6161
"org.scalatest" %% "scalatest" % "3.2.2" % "it,test",
62-
"org.scalamock" %% "scalamock" % "5.0.0" % "test",
62+
"org.scalamock" %% "scalamock" % "5.0.0" % "it,test",
6363
"org.scalatestplus" %% "scalacheck-1-15" % "3.2.3.0" % "test",
6464
"org.scalacheck" %% "scalacheck" % "1.15.1" % "it,test",
6565
"com.softwaremill.diffx" %% "diffx-core" % "0.3.30" % "test",

‎src/it/scala/io/iohk/ethereum/ledger/BlockImporterItSpec.scala

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
package io.iohk.ethereum.ledger
2+
3+
import akka.testkit.TestProbe
4+
import akka.util.ByteString
5+
import cats.data.NonEmptyList
6+
import io.iohk.ethereum.blockchain.sync.regular.{BlockFetcher, BlockImporter}
7+
import io.iohk.ethereum.consensus.blocks.CheckpointBlockGenerator
8+
import io.iohk.ethereum.domain._
9+
import io.iohk.ethereum.mpt.MerklePatriciaTrie
10+
import io.iohk.ethereum.utils.Config.SyncConfig
11+
import io.iohk.ethereum.utils.Config
12+
import io.iohk.ethereum.Fixtures
13+
import monix.execution.Scheduler
14+
import org.scalamock.scalatest.MockFactory
15+
import org.scalatest.BeforeAndAfterAll
16+
import org.scalatest.flatspec.AsyncFlatSpecLike
17+
import org.scalatest.matchers.should.Matchers
18+
import scala.concurrent.duration._
19+
20+
class BlockImporterItSpec extends MockFactory with TestSetupWithVmAndValidators with AsyncFlatSpecLike with Matchers with BeforeAndAfterAll {
21+
22+
implicit val testScheduler = Scheduler.fixedPool("test", 32)
23+
24+
override def afterAll(): Unit = {
25+
testScheduler.shutdown()
26+
testScheduler.awaitTermination(60.second)
27+
}
28+
29+
val bl = BlockchainImpl(storagesInstance.storages)
30+
31+
val blockQueue = BlockQueue(bl, SyncConfig(Config.config))
32+
33+
val genesis = Block(
34+
Fixtures.Blocks.Genesis.header.copy(stateRoot = ByteString(MerklePatriciaTrie.EmptyRootHash)),
35+
Fixtures.Blocks.Genesis.body
36+
)
37+
val genesisWeight = ChainWeight.zero.increase(genesis.header)
38+
39+
bl.save(genesis, Seq(), genesisWeight, saveAsBestBlock = true)
40+
41+
lazy val checkpointBlockGenerator: CheckpointBlockGenerator = new CheckpointBlockGenerator
42+
43+
val fetcherProbe = TestProbe()
44+
val ommersPoolProbe = TestProbe()
45+
val broadcasterProbe = TestProbe()
46+
val pendingTransactionsManagerProbe = TestProbe()
47+
val supervisor = TestProbe()
48+
49+
override lazy val ledger: TestLedgerImpl = new TestLedgerImpl(validators) {
50+
override private[ledger] lazy val blockExecution = mock[BlockExecution]
51+
}
52+
53+
val blockImporter = system.actorOf(
54+
BlockImporter.props(
55+
fetcherProbe.ref,
56+
ledger,
57+
bl,
58+
syncConfig,
59+
ommersPoolProbe.ref,
60+
broadcasterProbe.ref,
61+
pendingTransactionsManagerProbe.ref,
62+
checkpointBlockGenerator,
63+
supervisor.ref
64+
))
65+
66+
"BlockImporter" should "return a correct new best block after reorganising longer chain to a shorter one" in {
67+
68+
val genesis = bl.getBestBlock()
69+
val block1: Block = getBlock(genesis.number + 1, parent = genesis.header.hash)
70+
// new chain is shorter but has a higher weight
71+
val newBlock2: Block = getBlock(genesis.number + 2, difficulty = 101, parent = block1.header.hash)
72+
val newBlock3: Block = getBlock(genesis.number + 3, difficulty = 333, parent = newBlock2.header.hash)
73+
val oldBlock2: Block = getBlock(genesis.number + 2, difficulty = 102, parent = block1.header.hash)
74+
val oldBlock3: Block = getBlock(genesis.number + 3, difficulty = 103, parent = oldBlock2.header.hash)
75+
val oldBlock4: Block = getBlock(genesis.number + 4, difficulty = 104, parent = oldBlock3.header.hash)
76+
77+
val weight1 = ChainWeight.totalDifficultyOnly(block1.header.difficulty + 999)
78+
val newWeight2 = weight1.increase(newBlock2.header)
79+
val newWeight3 = newWeight2.increase(newBlock3.header)
80+
val oldWeight2 = weight1.increase(oldBlock2.header)
81+
val oldWeight3 = oldWeight2.increase(oldBlock3.header)
82+
val oldWeight4 = oldWeight3.increase(oldBlock4.header)
83+
84+
//saving initial main chain
85+
bl.save(block1, Nil, weight1, saveAsBestBlock = true)
86+
bl.save(oldBlock2, Nil, oldWeight2, saveAsBestBlock = true)
87+
bl.save(oldBlock3, Nil, oldWeight3, saveAsBestBlock = true)
88+
bl.save(oldBlock4, Nil, oldWeight4, saveAsBestBlock = true)
89+
90+
val blockData2 = BlockData(newBlock2, Seq.empty[Receipt], newWeight2)
91+
val blockData3 = BlockData(newBlock3, Seq.empty[Receipt], newWeight3)
92+
93+
val newBranch = List(newBlock2, newBlock3)
94+
95+
blockImporter ! BlockImporter.Start
96+
blockImporter ! BlockFetcher.PickedBlocks(NonEmptyList.fromListUnsafe(newBranch))
97+
98+
(ledger.blockExecution.executeAndValidateBlocks _)
99+
.expects(newBranch, *)
100+
.returning((List(blockData2, blockData3), None))
101+
102+
// Saving new blocks, because it's part of executeBlocks method mechanism
103+
bl.save(blockData2.block, blockData2.receipts, blockData2.weight, saveAsBestBlock = true)
104+
bl.save(blockData3.block, blockData3.receipts, blockData3.weight, saveAsBestBlock = true)
105+
106+
bl.getBestBlock() shouldEqual newBlock3
107+
}
108+
}

‎src/main/scala/io/iohk/ethereum/blockchain/sync/fast/FastSync.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -823,8 +823,8 @@ class FastSync(
823823
val bestReceivedBlock = fullBlocks.maxBy(_.number)
824824
val lastStoredBestBlockNumber = appStateStorage.getBestBlockNumber()
825825
if (lastStoredBestBlockNumber < bestReceivedBlock.number) {
826-
appStateStorage.putBestBlockNumber(bestReceivedBlock.number).commit()
827826
blockchain.saveBestKnownBlocks(bestReceivedBlock.number)
827+
appStateStorage.putBestBlockNumber(bestReceivedBlock.number).commit()
828828
}
829829
syncState = syncState.copy(lastFullBlockNumber = bestReceivedBlock.number.max(lastStoredBestBlockNumber))
830830
}

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

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -341,12 +341,6 @@ class BlockchainImpl(
341341
}
342342

343343
def save(block: Block, receipts: Seq[Receipt], weight: ChainWeight, saveAsBestBlock: Boolean): Unit = {
344-
log.debug("Saving new block block {} to database", block.idTag)
345-
storeBlock(block)
346-
.and(storeReceipts(block.header.hash, receipts))
347-
.and(storeChainWeight(block.header.hash, weight))
348-
.commit()
349-
350344
if (saveAsBestBlock && block.hasCheckpoint) {
351345
log.debug(
352346
"New best known block block number - {}, new best checkpoint number - {}",
@@ -362,6 +356,12 @@ class BlockchainImpl(
362356
saveBestKnownBlock(block.header.number)
363357
}
364358

359+
log.debug("Saving new block block {} to database", block.idTag)
360+
storeBlock(block)
361+
.and(storeReceipts(block.header.hash, receipts))
362+
.and(storeChainWeight(block.header.hash, weight))
363+
.commit()
364+
365365
// not transactional part
366366
// the best blocks data will be persisted only when the cache will be persisted
367367
stateStorage.onBlockSave(block.header.number, appStateStorage.getBestBlockNumber())(persistBestBlocksData)
@@ -397,20 +397,17 @@ class BlockchainImpl(
397397
}
398398
}
399399

400-
private def saveBestKnownBlock(bestBlockNumber: BigInt): Unit = {
400+
private def saveBestKnownBlock(bestBlockNumber: BigInt): Unit =
401401
bestKnownBlockAndLatestCheckpoint.updateAndGet(_.copy(bestBlockNumber = bestBlockNumber))
402-
}
403402

404-
private def saveBestKnownBlockAndLatestCheckpointNumber(number: BigInt, latestCheckpointNumber: BigInt): Unit = {
403+
private def saveBestKnownBlockAndLatestCheckpointNumber(number: BigInt, latestCheckpointNumber: BigInt): Unit =
405404
bestKnownBlockAndLatestCheckpoint.set(BestBlockLatestCheckpointNumbers(number, latestCheckpointNumber))
406-
}
407405

408406
def storeChainWeight(blockhash: ByteString, weight: ChainWeight): DataSourceBatchUpdate =
409407
chainWeightStorage.put(blockhash, weight)
410408

411-
def saveNode(nodeHash: NodeHash, nodeEncoded: NodeEncoded, blockNumber: BigInt): Unit = {
409+
def saveNode(nodeHash: NodeHash, nodeEncoded: NodeEncoded, blockNumber: BigInt): Unit =
412410
stateStorage.saveNode(nodeHash, nodeEncoded, blockNumber)
413-
}
414411

415412
override protected def getHashByBlockNumber(number: BigInt): Option[ByteString] =
416413
blockNumberMappingStorage.get(number)

‎src/test/scala/io/iohk/ethereum/blockchain/sync/ScenarioSetup.scala

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,8 @@ import scala.concurrent.ExecutionContext
2121
* [[io.iohk.ethereum.nodebuilder.Node Node]].
2222
*/
2323
trait ScenarioSetup extends StdTestConsensusBuilder with SyncConfigBuilder with StdLedgerBuilder {
24-
protected lazy val executionContext = ExecutionContext.fromExecutor(Executors.newFixedThreadPool(4))
25-
protected lazy val monixScheduler = Scheduler(executionContext)
24+
protected lazy val executionContextExecutor = ExecutionContext.fromExecutor(Executors.newFixedThreadPool(4))
25+
protected lazy val monixScheduler = Scheduler(executionContextExecutor)
2626
protected lazy val successValidators: Validators = Mocks.MockValidatorsAlwaysSucceed
2727
protected lazy val failureValidators: Validators = Mocks.MockValidatorsAlwaysFail
2828
protected lazy val ethashValidators: ValidatorsExecutor = ValidatorsExecutor(blockchainConfig, Protocol.Ethash)

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ class BlockFetcherSpec
102102
peersClient.expectMsgPF() { case PeersClient.Request(msg, _, _) if msg == firstGetBlockHeadersRequest => () }
103103
}
104104

105-
"should not enqueue requested blocks if the received bodies does not match" in new TestSetup {
105+
"should not enqueue requested blocks if the received bodies do not match" in new TestSetup {
106106

107107
// Important: Here we are forcing the mismatch between request headers and received bodies
108108
override lazy val validators = new MockValidatorsFailingOnBlockBodies

‎src/test/scala/io/iohk/ethereum/ledger/BlockImportSpec.scala

Lines changed: 51 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,57 @@ class BlockImportSpec extends AnyFlatSpec with Matchers with ScalaFutures {
146146
blockQueue.isQueued(oldBlock3.header.hash) shouldBe true
147147
}
148148

149+
it should "fail to get bestblock after reorganisation of the longer chain to a shorter one if desync state happened between cache and db" in new EphemBlockchain {
150+
val block1: Block = getBlock(bestNum - 2)
151+
// new chain is shorter but has a higher weight
152+
val newBlock2: Block = getBlock(bestNum - 1, difficulty = 101, parent = block1.header.hash)
153+
val newBlock3: Block = getBlock(bestNum, difficulty = 333, parent = newBlock2.header.hash)
154+
val oldBlock2: Block = getBlock(bestNum - 1, difficulty = 102, parent = block1.header.hash)
155+
val oldBlock3: Block = getBlock(bestNum, difficulty = 103, parent = oldBlock2.header.hash)
156+
val oldBlock4: Block = getBlock(bestNum + 1, difficulty = 104, parent = oldBlock3.header.hash)
157+
158+
val weight1 = ChainWeight.totalDifficultyOnly(block1.header.difficulty + 999)
159+
val newWeight2 = weight1.increase(newBlock2.header)
160+
val newWeight3 = newWeight2.increase(newBlock3.header)
161+
val oldWeight2 = weight1.increase(oldBlock2.header)
162+
val oldWeight3 = oldWeight2.increase(oldBlock3.header)
163+
val oldWeight4 = oldWeight3.increase(oldBlock4.header)
164+
165+
blockchain.save(block1, Nil, weight1, saveAsBestBlock = true)
166+
blockchain.save(oldBlock2, receipts, oldWeight2, saveAsBestBlock = true)
167+
blockchain.save(oldBlock3, Nil, oldWeight3, saveAsBestBlock = true)
168+
blockchain.save(oldBlock4, Nil, oldWeight4, saveAsBestBlock = true)
169+
170+
val ancestorForValidation: Block = getBlock(0, difficulty = 1)
171+
blockchain.save(ancestorForValidation, Nil, ChainWeight.totalDifficultyOnly(1), saveAsBestBlock = false)
172+
173+
val oldBranch = List(oldBlock2, oldBlock3, oldBlock4)
174+
val newBranch = List(newBlock2, newBlock3)
175+
val blockData2 = BlockData(newBlock2, Seq.empty[Receipt], newWeight2)
176+
val blockData3 = BlockData(newBlock3, Seq.empty[Receipt], newWeight3)
177+
178+
(ledgerWithMockedBlockExecution.blockExecution.executeAndValidateBlocks _)
179+
.expects(newBranch, *)
180+
.returning((List(blockData2, blockData3), None))
181+
182+
whenReady(ledgerWithMockedBlockExecution.importBlock(newBlock3).runToFuture) { _ shouldEqual BlockEnqueued }
183+
whenReady(ledgerWithMockedBlockExecution.importBlock(newBlock2).runToFuture) { result =>
184+
result shouldEqual ChainReorganised(oldBranch, newBranch, List(newWeight2, newWeight3))
185+
}
186+
187+
// Saving new blocks, because it's part of executeBlocks method mechanism
188+
blockchain.save(blockData2.block, blockData2.receipts, blockData2.weight, saveAsBestBlock = true)
189+
blockchain.save(blockData3.block, blockData3.receipts, blockData3.weight, saveAsBestBlock = true)
190+
191+
//saving to cache the value of the best block from the initial chain. This recreates the bug ETCM-626, where (possiby) because of the thread of execution
192+
// dying before updating the storage but after updating the cache, inconsistency is created
193+
blockchain.saveBestKnownBlocks(oldBlock4.number)
194+
195+
assertThrows[NoSuchElementException] {
196+
blockchain.getBestBlock()
197+
}
198+
}
199+
149200
it should "handle error when trying to reorganise chain" in new EphemBlockchain {
150201
val block1: Block = getBlock(bestNum - 2)
151202
val newBlock2: Block = getBlock(bestNum - 1, difficulty = 101, parent = block1.header.hash)
@@ -166,11 +217,8 @@ class BlockImportSpec extends AnyFlatSpec with Matchers with ScalaFutures {
166217
val ancestorForValidation: Block = getBlock(0, difficulty = 1)
167218
blockchain.save(ancestorForValidation, Nil, ChainWeight.totalDifficultyOnly(1), saveAsBestBlock = false)
168219

169-
//FIXME: unused vals???
170-
val oldBranch = List(oldBlock2, oldBlock3)
171220
val newBranch = List(newBlock2, newBlock3)
172221
val blockData2 = BlockData(newBlock2, Seq.empty[Receipt], newWeight2)
173-
val blockData3 = BlockData(newBlock3, Seq.empty[Receipt], newWeight3)
174222

175223
(ledgerWithMockedBlockExecution.blockExecution.executeAndValidateBlocks _)
176224
.expects(newBranch, *)

0 commit comments

Comments
 (0)