[FIX] Return executed blocks sorted on execution error (#772)

Author: Nicolás Tallar

src/main/scala/io/iohk/ethereum/ledger/BlockExecution.scala

@@ -123,33 +123,33 @@ class BlockExecution(
     * @param blocks   blocks to be executed
     * @param parentTd transaction difficulty of the parent
     *
-    * @return a list of blocks that were correctly executed and an optional [[BlockExecutionError]]
+    * @return a list of blocks in incremental order that were correctly executed and an optional [[BlockExecutionError]]
     */
   def executeAndValidateBlocks(
       blocks: List[Block],
       parentTd: BigInt
   ): (List[BlockData], Option[BlockExecutionError]) = {
     @tailrec
     def go(
-        executedBlocks: List[BlockData],
-        remainingBlocks: List[Block],
+        executedBlocksDecOrder: List[BlockData],
+        remainingBlocksIncOrder: List[Block],
         parentTd: BigInt,
         error: Option[BlockExecutionError]
     ): (List[BlockData], Option[BlockExecutionError]) = {
-      if (remainingBlocks.isEmpty) {
-        (executedBlocks.reverse, None)
+      if (remainingBlocksIncOrder.isEmpty) {
+        (executedBlocksDecOrder.reverse, None)
       } else if (error.isDefined) {
-        (executedBlocks, error)
+        (executedBlocksDecOrder.reverse, error)
       } else {
-        val blockToExecute = remainingBlocks.head
+        val blockToExecute = remainingBlocksIncOrder.head
         executeAndValidateBlock(blockToExecute, alreadyValidated = true) match {
           case Right(receipts) =>
             val td = parentTd + blockToExecute.header.difficulty
             val newBlockData = BlockData(blockToExecute, receipts, td)
             blockchain.save(newBlockData.block, newBlockData.receipts, newBlockData.td, saveAsBestBlock = true)
-            go(newBlockData :: executedBlocks, remainingBlocks.tail, td, None)
+            go(newBlockData :: executedBlocksDecOrder, remainingBlocksIncOrder.tail, td, None)
           case Left(executionError) =>
-            go(executedBlocks, remainingBlocks, 0, Some(executionError))
+            go(executedBlocksDecOrder, remainingBlocksIncOrder, 0, Some(executionError))
         }
       }
     }

src/test/scala/io/iohk/ethereum/BlockHelpers.scala

@@ -7,7 +7,6 @@ import io.iohk.ethereum.nodebuilder.SecureRandomBuilder
 import org.bouncycastle.crypto.AsymmetricCipherKeyPair
 import mouse.all._
 
-import scala.math.BigInt
 import scala.util.Random
 
 object BlockHelpers extends SecureRandomBuilder {
@@ -37,16 +36,16 @@ object BlockHelpers extends SecureRandomBuilder {
   def randomHash(): ByteString =
     ObjectGenerators.byteStringOfLengthNGen(32).sample.get
 
-  def generateChain(amount: Int, parent: Block, adjustBlock: Block => Block = identity): List[Block] =
-    (1 to amount).toList.foldLeft[List[Block]](Nil)((generated, _) => {
-      val theParent = generated.lastOption.getOrElse(parent)
-      generated :+ (theParent |> generateBlock |> adjustBlock)
-    })
+  def generateChain(amount: Int, branchParent: Block, adjustBlock: Block => Block = identity): List[Block] =
+    (1 to amount).toList.foldLeft[List[Block]](Nil){ (generated, _) =>
+      val parent = generated.lastOption.getOrElse(branchParent)
+      generated :+ (parent |> generateBlock |> adjustBlock)
+    }
 
-  def generateBlock(nr: BigInt, parent: Block): Block = {
-    val header = defaultHeader.copy(
+  def generateBlock(parent: Block): Block = {
+    val header = parent.header.copy(
       extraData = randomHash(),
-      number = nr,
+      number = parent.number + 1,
       parentHash = parent.hash,
       nonce = ByteString(Random.nextLong())
     )
@@ -57,6 +56,4 @@ object BlockHelpers extends SecureRandomBuilder {
     Block(header, BlockBody(List(stx.tx), List(ommer)))
   }
 
-  def generateBlock(parent: Block): Block = generateBlock(parent.number + 1, parent)
-
 }

src/test/scala/io/iohk/ethereum/ledger/BlockExecutionSpec.scala

@@ -8,7 +8,7 @@ import io.iohk.ethereum.crypto.ECDSASignature
 import io.iohk.ethereum.domain._
 import io.iohk.ethereum.ledger.Ledger.BlockResult
 import io.iohk.ethereum.vm.OutOfGas
-import io.iohk.ethereum.{Mocks, ObjectGenerators}
+import io.iohk.ethereum.{BlockHelpers, Mocks, ObjectGenerators}
 import org.scalatest.matchers.should.Matchers
 import org.scalatest.prop.TableFor4
 import org.scalatest.wordspec.AnyWordSpec
@@ -91,6 +91,32 @@ class BlockExecutionSpec extends AnyWordSpec with Matchers with ScalaCheckProper
         error.isDefined shouldBe true
       }
 
+      "executing a long branch where the last block is invalid" in new BlockchainSetup {
+        val chain = BlockHelpers.generateChain(10, validBlockParentBlock)
+
+        val mockVm = new MockVM(c =>
+          createResult(
+            context = c,
+            gasUsed = UInt256(0),
+            gasLimit = UInt256(defaultGasLimit),
+            gasRefund = UInt256.Zero,
+            logs = defaultLogs,
+            addressesToDelete = defaultAddressesToDelete
+          )
+        )
+        val mockValidators = new MockValidatorsFailOnSpecificBlockNumber(chain.last.number)
+        val newConsensus: TestConsensus = consensus.withVM(mockVm).withValidators(mockValidators)
+        val blockValidation = new BlockValidation(newConsensus, blockchain, BlockQueue(blockchain, syncConfig))
+        val blockExecution =
+          new BlockExecution(blockchain, blockchainConfig, newConsensus.blockPreparator, blockValidation)
+
+        val (blocks, error) = blockExecution.executeAndValidateBlocks(chain, defaultBlockHeader.difficulty)
+
+        // All blocks but the last should be executed, and they should be returned in incremental order
+        blocks.map(_.block) shouldBe chain.init
+        error.isDefined shouldBe true
+      }
+
       "block with checkpoint and without txs" in new BlockchainSetup {
         val checkpoint = ObjectGenerators.fakeCheckpointGen(2, 5).sample.get
         val blockWithCheckpoint = new CheckpointBlockGenerator().generate(Block(validBlockParentHeader, validBlockBodyWithNoTxs), checkpoint)

src/test/scala/io/iohk/ethereum/ledger/LedgerTestSetup.scala

@@ -157,6 +157,7 @@ trait BlockchainSetup extends TestSetup {
   val blockchainStorages: storagesInstance.Storages = storagesInstance.storages
 
   val validBlockParentHeader: BlockHeader = defaultBlockHeader.copy(stateRoot = initialWorld.stateRootHash)
+  val validBlockParentBlock: Block = Block(validBlockParentHeader, BlockBody.empty)
   val validBlockHeader: BlockHeader = defaultBlockHeader.copy(
     stateRoot = initialWorld.stateRootHash,
     parentHash = validBlockParentHeader.hash,