From 4f13417fade80534063e523a33e822bc097f1111 Mon Sep 17 00:00:00 2001 From: Sergey Nazarov Date: Wed, 14 Oct 2026 13:00:07 +0300 Subject: [PATCH 1/6] Fix setting finalized block --- .../ForkchoiceNoFinalizedBlockTestSuite.scala | 82 +++++++++++++++++++ src/main/scala/units/ELUpdater.scala | 21 +++-- .../units/FinalizedHashForTestSuite.scala | 41 ++++++++++ .../scala/units/client/TestEcClients.scala | 12 ++- 4 files changed, 147 insertions(+), 9 deletions(-) create mode 100644 consensus-client-it/src/test/scala/units/ForkchoiceNoFinalizedBlockTestSuite.scala create mode 100644 src/test/scala/units/FinalizedHashForTestSuite.scala diff --git a/consensus-client-it/src/test/scala/units/ForkchoiceNoFinalizedBlockTestSuite.scala b/consensus-client-it/src/test/scala/units/ForkchoiceNoFinalizedBlockTestSuite.scala new file mode 100644 index 00000000..0c33d942 --- /dev/null +++ b/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 +} diff --git a/src/main/scala/units/ELUpdater.scala b/src/main/scala/units/ELUpdater.scala index e71638db..a7718df2 100644 --- a/src/main/scala/units/ELUpdater.scala +++ b/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 diff --git a/src/test/scala/units/FinalizedHashForTestSuite.scala b/src/test/scala/units/FinalizedHashForTestSuite.scala new file mode 100644 index 00000000..8341a141 --- /dev/null +++ b/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 + } +} diff --git a/src/test/scala/units/client/TestEcClients.scala b/src/test/scala/units/client/TestEcClients.scala index 272a53bf..09d13838 100644 --- a/src/test/scala/units/client/TestEcClients.scala +++ b/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 { From 9737c0771eb741cb02b6418d5a3206f3f88519a2 Mon Sep 17 00:00:00 2001 From: Sergey Nazarov Date: Wed, 14 Oct 2026 13:35:03 +0300 Subject: [PATCH 2/6] Build local network deploy image without host network Docker Compose builds via buildx bake, which rejects `network: host` without an explicit --allow=network.host entitlement, failing the Local Network tests job. The build only fetches Foundry and pip packages, which works on the default build network. Co-Authored-By: Claude Opus 5.5 --- local-network/deploy/deploy.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/local-network/deploy/deploy.yml b/local-network/deploy/deploy.yml index 0c40649d..c5977d95 100644 --- a/local-network/deploy/deploy.yml +++ b/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 From cde4e9962eaccd0a841a8b91807c02d2af65b4de Mon Sep 17 00:00:00 2001 From: Sergey Nazarov Date: Wed, 14 Oct 2026 13:35:03 +0300 Subject: [PATCH 3/6] Name test log artifacts by run instead of branch upload-artifact rejects names containing "/", so test logs were lost for branches like fix/... The run attempt keeps names unique on re-runs. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/check-pr.yml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.github/workflows/check-pr.yml b/.github/workflows/check-pr.yml index 7328f1de..a30f84e1 100644 --- a/.github/workflows/check-pr.yml +++ b/.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 From c0f49a5d2149620e7a3665119532ec5442141652 Mon Sep 17 00:00:00 2001 From: Sergey Nazarov Date: Wed, 14 Oct 2026 14:12:47 +0300 Subject: [PATCH 4/6] Serialize forge deployments across parallel IT suites forge script keeps broadcast state per script and chain id in contracts/eth/cache and now refuses to run while another process modifies it ("another process is already modifying recovery state"). Suites run in parallel JVMs with the same chain id, so a concurrent deployment failed silently and the suite aborted after waiting for the contracts. Deployments now take an OS-level file lock, and a non-zero forge exit code fails the setup right away. Co-Authored-By: Claude Opus 5.5 --- .../scala/units/BaseDockerTestSuite.scala | 29 +++++++++++++++---- 1 file changed, 24 insertions(+), 5 deletions(-) diff --git a/consensus-client-it/src/test/scala/units/BaseDockerTestSuite.scala b/consensus-client-it/src/test/scala/units/BaseDockerTestSuite.scala index 474b92a7..71bbcd31 100644 --- a/consensus-client-it/src/test/scala/units/BaseDockerTestSuite.scala +++ b/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 { From 24ac2dc8ea6ac0b3096155d9ff0a38f7ff77a260 Mon Sep 17 00:00:00 2001 From: Sergey Nazarov Date: Wed, 14 Oct 2026 14:12:47 +0300 Subject: [PATCH 5/6] Wait for an unused acting miner epoch in block validation tests After the setup miner leaves, the acting miner can become the designated miner of an epoch in which the setup miner already mined. waitForMinerEpoch returned there, the tests then waited for the next epoch to register their block, and failed when another miner was designated in it ("is not allowed to mine in 5 epoch"). Wait for an epoch of the acting miner without registered blocks instead. Co-Authored-By: Claude Opus 5.5 --- .../validation/BaseBlockValidationSuite.scala | 2 +- .../units/client/HttpChainContractClient.scala | 17 +++++++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/consensus-client-it/src/test/scala/units/block/validation/BaseBlockValidationSuite.scala b/consensus-client-it/src/test/scala/units/block/validation/BaseBlockValidationSuite.scala index 4e6d15f4..36c0d578 100644 --- a/consensus-client-it/src/test/scala/units/block/validation/BaseBlockValidationSuite.scala +++ b/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 = { diff --git a/consensus-client-it/src/test/scala/units/client/HttpChainContractClient.scala b/consensus-client-it/src/test/scala/units/client/HttpChainContractClient.scala index 92db866c..f640a58d 100644 --- a/consensus-client-it/src/test/scala/units/client/HttpChainContractClient.scala +++ b/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)") From 04637baec27b48a1a04f204887e0bc1613dacb87 Mon Sep 17 00:00:00 2001 From: Sergey Nazarov Date: Wed, 14 Oct 2026 14:25:43 +0300 Subject: [PATCH 6/6] Fix startup check --- src/main/scala/units/client/contract/ChainContractClient.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/scala/units/client/contract/ChainContractClient.scala b/src/main/scala/units/client/contract/ChainContractClient.scala index 20f01a4a..18c1216a 100644 --- a/src/main/scala/units/client/contract/ChainContractClient.scala +++ b/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)