Don't set finalized block below head

.github/workflows/check-pr.yml

@@ -16,7 +16,6 @@ jobs:
     timeout-minutes: 45
     env:
       JAVA_OPTS: -Dfile.encoding=UTF-8
-      BRANCH_NAME: ${{ github.head_ref || github.ref_name }}
     steps:
       - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
       - uses: actions/setup-java@be666c2fcd27ec809703dec50e508c2fdc7f6654 # v5.2.0
@@ -40,7 +39,7 @@ jobs:
         uses: actions/upload-artifact@v4
         if: always()
         with:
-          name: test-logs_${{ env.BRANCH_NAME }}_${{ github.run_id }}
+          name: test-logs_${{ github.run_id }}_${{ github.run_attempt }}
           path: consensus-client-it/target/test-logs
           if-no-files-found: warn
           retention-days: 14

consensus-client-it/src/test/scala/units/BaseDockerTestSuite.scala

@@ -21,6 +21,8 @@ import units.el.{NativeBridgeClient, StandardBridgeClient}
 import units.eth.{EthAddress, Gwei}
 import units.test.{CustomMatchers, IntegrationTestEventually, TestEnvironment}
 
+import java.nio.channels.FileChannel
+import java.nio.file.{Path, StandardOpenOption}
 import scala.sys.process.{Process, ProcessLogger}
 
 trait BaseDockerTestSuite
@@ -117,11 +119,14 @@ trait BaseDockerTestSuite
 
   protected def deploySolidityContracts(): Unit = {
     step("Deploy contracts on EL")
-    Process(
-      s"forge script -vvvvv scripts/IT.s.sol:IT --private-key $elRichAccount1PrivateKey --fork-url http://localhost:${ec1.rpcPort} --broadcast",
-      TestEnvironment.ContractsDir,
-      "CHAIN_ID" -> EcContainer.ChainId.toString
-    ).!(ProcessLogger(out => log.info(out), err => log.error(err)))
+    val exitCode = BaseDockerTestSuite.withForgeLock {
+      Process(
+        s"forge script -vvvvv scripts/IT.s.sol:IT --private-key $elRichAccount1PrivateKey --fork-url http://localhost:${ec1.rpcPort} --broadcast",
+        TestEnvironment.ContractsDir,
+        "CHAIN_ID" -> EcContainer.ChainId.toString
+      ).!(ProcessLogger(out => log.info(out), err => log.error(err)))
+    }
+    withClue("forge script exit code:")(exitCode shouldBe 0)
 
     waitForContract(StandardBridgeAddress)
     waitForContract(WWavesAddress)
@@ -154,6 +159,20 @@ trait BaseDockerTestSuite
 object BaseDockerTestSuite {
   private val initialized = AtomicBoolean(false)
 
+  private val ForgeLockFile = Path.of(System.getProperty("java.io.tmpdir"), "consensus-client-it-forge.lock")
+
+  /** forge script keeps its broadcast state per script and chain id in the shared contracts cache directory and refuses to run while another process
+    * modifies it. Suites run in parallel JVMs with the same chain id, so deployments are serialized with an OS-level file lock.
+    */
+  def withForgeLock[A](f: => A): A = {
+    val channel = FileChannel.open(ForgeLockFile, StandardOpenOption.CREATE, StandardOpenOption.WRITE)
+    try {
+      val lock = channel.lock()
+      try f
+      finally lock.release()
+    } finally channel.close()
+  }
+
   def init(): Unit =
     if (initialized.compareAndSet(expect = false, update = true))
       AddressScheme.current = new AddressScheme {

consensus-client-it/src/test/scala/units/ForkchoiceNoFinalizedBlockTestSuite.scala

@@ -0,0 +1,82 @@
+package units
+
+import com.wavesplatform.common.utils.EitherExt2.explicitGet
+import org.web3j.protocol.core.DefaultBlockParameterName
+import units.ELUpdater.NoFinalizedBlockHash
+import units.client.engine.model.{BlockNumber, EcBlock}
+import units.eth.EthAddress
+
+/** The consensus client reports no finalized block (an all-zero hash) in forkchoiceUpdated when the new head is below the contract's finalized block.
+  * Checks that op-geth accepts it, moves the head, and keeps the previous finalized block.
+  *
+  * Drives op-geth directly through the engine API, so only the EL node is started.
+  */
+class ForkchoiceNoFinalizedBlockTestSuite extends BaseDockerTestSuite {
+  private val prevRandao = "0x" + "0" * 64
+  private val minerA     = EthAddress.unsafeFrom("0x" + "a" * 40)
+  private val minerB     = EthAddress.unsafeFrom("0x" + "b" * 40)
+
+  /** Builds a block on top of the current head `parent` and imports it without making it the head. */
+  private def buildBlock(parent: EcBlock, finalized: BlockHash, feeRecipient: EthAddress): EcBlock = {
+    val payloadId = ec1.engineApi
+      .forkchoiceUpdatedWithPayload(parent.hash, finalized, parent.timestamp + 1, feeRecipient, prevRandao, Vector.empty, Vector.empty)
+      .explicitGet()
+    val payload = ec1.engineApi.getPayload(payloadId).explicitGet()
+    val hash    = ec1.engineApi.newPayload(payload).explicitGet().value
+    ec1.engineApi.getBlockByHash(hash).explicitGet().value
+  }
+
+  private def head: BlockHash = ec1.engineApi.getLastExecutionBlock().explicitGet().hash
+
+  private def finalizedHash: Option[BlockHash] = {
+    val r = ec1.web3j.ethGetBlockByNumber(DefaultBlockParameterName.FINALIZED, false).send()
+    Option.when(!r.hasError)(r.getBlock).flatMap(Option(_)).map(b => BlockHash(b.getHash))
+  }
+
+  "forkchoiceUpdated with no finalized block" in {
+    val genesis = ec1.engineApi.getBlockByNumber(BlockNumber.Number(0)).explicitGet().value
+
+    step("Build a1 on genesis with no finalized block")
+    val a1 = buildBlock(genesis, NoFinalizedBlockHash, minerA)
+
+    step("Make a1 the head with no finalized block")
+    ec1.engineApi.forkchoiceUpdated(a1.hash, NoFinalizedBlockHash).explicitGet()
+    head shouldBe a1.hash
+    finalizedHash shouldBe empty
+
+    step("Build siblings a2 and b2 on a1")
+    val a2 = buildBlock(a1, NoFinalizedBlockHash, minerA)
+    val b2 = buildBlock(a1, NoFinalizedBlockHash, minerB)
+    a2.hash should not be b2.hash
+
+    step("Make a2 the head, a1 finalized")
+    ec1.engineApi.forkchoiceUpdated(a2.hash, a1.hash).explicitGet()
+    head shouldBe a2.hash
+    finalizedHash.value shouldBe a1.hash
+
+    step("Switch the head to the sibling b2 with no finalized block: accepted, a1 stays finalized")
+    ec1.engineApi.forkchoiceUpdated(b2.hash, NoFinalizedBlockHash).explicitGet()
+    head shouldBe b2.hash
+    finalizedHash.value shouldBe a1.hash
+
+    step("Switch back to a2 with no finalized block")
+    ec1.engineApi.forkchoiceUpdated(a2.hash, NoFinalizedBlockHash).explicitGet()
+    head shouldBe a2.hash
+    finalizedHash.value shouldBe a1.hash
+
+    step("A finalized block that isn't on the head's chain is rejected, the finalized block doesn't change")
+    ec1.engineApi.forkchoiceUpdated(a2.hash, b2.hash).isLeft shouldBe true
+    finalizedHash.value shouldBe a1.hash
+  }
+
+  override protected def startNodes(): Unit = {
+    ec1.start()
+    ec1.logPorts()
+  }
+
+  override protected def stopNodes(): Unit = ec1.stop()
+
+  override protected def setupChain(): Unit = {}
+
+  override protected def step(text: String): Unit = log.debug(text) // No Waves node to print to
+}

consensus-client-it/src/test/scala/units/block/validation/BaseBlockValidationSuite.scala

@@ -221,7 +221,7 @@ trait BaseBlockValidationSuite extends BaseDockerTestSuite {
     }
 
     step(s"Wait actingMiner epoch")
-    chainContract.waitForMinerEpoch(actingMiner)
+    chainContract.waitForUnusedMinerEpoch(actingMiner)
   }
 
   protected def setupForNativeTokenTransfer(): Unit = {

consensus-client-it/src/test/scala/units/client/HttpChainContractClient.scala

@@ -63,6 +63,23 @@ class HttpChainContractClient(api: NodeHttpApi, override val contract: Address)
     }
   }
 
+  /** Waits for an epoch in which `minerAccount` is the designated miner and no block has been registered yet. Unlike [[waitForMinerEpoch]], it
+    * doesn't return in an epoch that changed hands midway, e.g. after the previous miner left having already mined in it: the epoch's blocks then
+    * belong to the previous miner, and `minerAccount` can only mine in a later epoch.
+    */
+  def waitForUnusedMinerEpoch(minerAccount: KeyPair)(implicit loggingOptions: LoggingOptions = LoggingOptions(logRequest = false)): Unit = {
+    val expectedGenerator = minerAccount.toAddress
+    if (loggingOptions.logCall) log.debug(s"${loggingOptions.prefix} waitForUnusedMinerEpoch($expectedGenerator)")
+
+    val subsequentLoggingOptions = loggingOptions.copy(logCall = false)
+    eventually(timeout(fiveBlocks * 2)) {
+      val epoch = api.height()(using subsequentLoggingOptions)
+      computedGenerator()(using subsequentLoggingOptions) shouldBe expectedGenerator
+      withClue(s"Blocks registered in epoch $epoch: ")(getEpochMeta(epoch) shouldBe empty)
+      withClue("Epoch changed during the check: ")(api.height()(using subsequentLoggingOptions) shouldBe epoch)
+    }
+  }
+
   // This is different from blockchain.height, because we wait this from a contract block
   def waitForEpoch(atLeast: Height, chainId: Long = DefaultMainChainId): Unit = {
     log.debug(s"waitForEpoch($atLeast)")

local-network/deploy/deploy.yml

@@ -2,7 +2,6 @@ services:
   deploy:
     build:
       context: .
-      network: host
     pull_policy: never
     restart: no # Manual start
     platform: linux/amd64 # HACK for Apple Silicon

src/main/scala/units/ELUpdater.scala

@@ -247,7 +247,7 @@ class ELUpdater(
       engineApiClient
         .forkchoiceUpdatedWithPayload(
           parentBlock.hash,
-          if (finalizedBlock.height > parentBlock.height) parentBlock.hash else finalizedBlock.hash,
+          finalizedHashFor(parentBlock, finalizedBlock),
           nextBlockUnixTs,
           epochInfo.rewardAddress,
           prevRandao,
@@ -1733,10 +1733,8 @@ class ELUpdater(
     loop(curState.lastContractBlock, List.empty)
   }
 
-  private def confirmBlock(block: L2BlockLike, finalizedBlock: L2BlockLike): Result[PayloadStatus] = {
-    val finalizedBlockHash = if (finalizedBlock.height > block.height) block.hash else finalizedBlock.hash
-    engineApiClient.forkchoiceUpdated(block.hash, finalizedBlockHash)
-  }
+  private def confirmBlock(block: L2BlockLike, finalizedBlock: L2BlockLike): Result[PayloadStatus] =
+    engineApiClient.forkchoiceUpdated(block.hash, finalizedHashFor(block, finalizedBlock))
 
   private def canSupportAnotherAltChain(nodeChainInfo: ChainInfo): Boolean = {
     val chainSupporters = chainContractClient.getSupporters(nodeChainInfo.id)
@@ -1773,6 +1771,19 @@ object ELUpdater {
 
   case class EpochInfo(number: Height, miner: Address, rewardAddress: EthAddress, hitSource: ByteStr, prevEpochLastBlockHash: Option[BlockHash])
 
+  /** "No finalized block" in forkchoiceUpdated: EL keeps its previous finalized block. */
+  val NoFinalizedBlockHash: BlockHash = BlockHash(EthereumConstants.EmptyBlockHashHex)
+
+  /** The finalized block to report to EL along with the new head `head`.
+    *
+    * EL rejects a finalized block above the head, and freezes everything up to the finalized block, assuming it can never be reorged. When `head` is
+    * below the contract's finalized block, it may not be on the finalized chain (e.g. a block received from a peer and never registered on the
+    * contract), so it must not be reported as finalized: EL would freeze it, and a later switch to the finalized chain could not replace it. No
+    * finalized block is reported instead.
+    */
+  def finalizedHashFor(head: L2BlockLike, finalizedBlock: L2BlockLike): BlockHash =
+    if (finalizedBlock.height > head.height) NoFinalizedBlockHash else finalizedBlock.hash
+
   sealed trait State
   object State {
     case object Starting extends State

src/main/scala/units/client/contract/ChainContractClient.scala

@@ -24,7 +24,7 @@ trait ChainContractClient {
 
   def extractData(key: String): Option[DataEntry[?]]
 
-  def isContractSetup: Boolean = getLongData("minerReward").isDefined
+  def isContractSetup: Boolean = extractData("minerReward").isDefined
 
   def isStopped: Boolean = getBooleanData("stopped").getOrElse(false)
 

src/test/scala/units/FinalizedHashForTestSuite.scala

@@ -0,0 +1,41 @@
+package units
+
+import org.scalatest.freespec.AnyFreeSpec
+import org.scalatest.matchers.should.Matchers
+import units.ELUpdater.{NoFinalizedBlockHash, finalizedHashFor}
+import units.client.L2BlockLike
+import units.eth.{EthAddress, EthereumConstants}
+
+class FinalizedHashForTestSuite extends AnyFreeSpec with Matchers {
+  private def block(blockHeight: Long, n: Int): L2BlockLike = {
+    val h = BlockHash(f"0x$n%064x")
+    new L2BlockLike {
+      override val hash: BlockHash                  = h
+      override val parentHash: BlockHash            = BlockHash(f"0x${n + 1000}%064x")
+      override val height: Long                     = blockHeight
+      override val minerRewardL2Address: EthAddress = EthAddress.empty
+    }
+  }
+
+  private val finalized = block(10, 1)
+
+  "finalizedHashFor" - {
+    "reports the contract's finalized block when the head is above it" in {
+      finalizedHashFor(block(11, 2), finalized) shouldBe finalized.hash
+    }
+
+    "reports the contract's finalized block when it is the head" in {
+      finalizedHashFor(finalized, finalized) shouldBe finalized.hash
+    }
+
+    "reports no finalized block when the head is below the contract's finalized block" in {
+      // The head may be a block that never made it to the contract. Reporting it as finalized makes EL freeze it for good.
+      finalizedHashFor(block(9, 3), finalized) shouldBe NoFinalizedBlockHash
+    }
+  }
+
+  "NoFinalizedBlockHash is the all-zero hash" in {
+    NoFinalizedBlockHash.str shouldBe EthereumConstants.EmptyBlockHashHex
+    NoFinalizedBlockHash.str shouldBe "0x" + "0" * 64
+  }
+}

src/test/scala/units/client/TestEcClients.scala

@@ -10,7 +10,7 @@ import units.client.engine.EngineApiClient.PayloadId
 import units.client.engine.model.*
 import units.client.engine.{EngineApiClient, LoggedEngineApiClient}
 import units.eth.{EthAddress, EthereumConstants}
-import units.{BlockHash, Result, NetworkL2Block}
+import units.{BlockHash, ELUpdater, NetworkL2Block, Result}
 import units.test.take
 
 /** Life of a block:
@@ -144,8 +144,10 @@ class TestEcClients private (
         }
 
         log.debug(s"Chain: ${updatedChain.map(_.hash.take(LogChars)).mkString(" -> ")}")
-        PayloadStatus.Valid
-      }.asRight
+        // Like geth: an all-zero finalized hash means "no finalized block", otherwise it must be on the chain of the new head
+        if (finalizedBlockHash == ELUpdater.NoFinalizedBlockHash || updatedChain.exists(_.hash == finalizedBlockHash)) PayloadStatus.Valid.asRight
+        else s"Payload validation error: final block $finalizedBlockHash not in canonical chain".asLeft
+      }
 
       override def forkchoiceUpdatedWithPayload(
           lastBlockHash: BlockHash,
@@ -158,8 +160,10 @@ class TestEcClients private (
           requestId: Int
       ): Result[PayloadId] = {
         forkChoiceUpdateWithPayloadIdCalls.increment()
-        forkchoiceUpdated(lastBlockHash, finalizedBlockHash, requestId)
+        forkchoiceUpdated(lastBlockHash, finalizedBlockHash, requestId).flatMap(_ => buildPayload(lastBlockHash))
+      }
 
+      private def buildPayload(lastBlockHash: BlockHash): Result[PayloadId] = {
         val nextFutureBlock = futureBlocks.transformAndExtract { xs =>
           val (found, rest) = xs.partition(_.parentHash == lastBlockHash)
           found match {