From d07ef2ee56c61c2adf4a852d08ec7d964e59566e Mon Sep 17 00:00:00 2001 From: jangko Date: Fri, 17 Jun 2022 07:53:33 +0700 Subject: [PATCH] fix engine api and angine api test --- hive_integration/nodocker/engine/clmock.nim | 3 + .../nodocker/engine/engine_sim.nim | 3 +- .../nodocker/engine/engine_tests.nim | 269 +++++++++++------- hive_integration/nodocker/engine/helper.nim | 10 +- hive_integration/nodocker/engine/test_env.nim | 23 +- hive_integration/nodocker/sim_utils.nim | 7 +- nimbus/merge/mergeutils.nim | 15 +- nimbus/rpc/engine_api.nim | 13 +- 8 files changed, 224 insertions(+), 119 deletions(-) diff --git a/hive_integration/nodocker/engine/clmock.nim b/hive_integration/nodocker/engine/clmock.nim index 70e15fb17..717ee32d0 100644 --- a/hive_integration/nodocker/engine/clmock.nim +++ b/hive_integration/nodocker/engine/clmock.nim @@ -327,3 +327,6 @@ proc isBlockPoS*(cl: CLMocker, bn: common.BlockNumber): bool = return false return true + +proc posBlockNumber*(cl: CLMocker): uint64 = + cl.firstPoSBlockNumber.get(0'u64) diff --git a/hive_integration/nodocker/engine/engine_sim.nim b/hive_integration/nodocker/engine/engine_sim.nim index ffbac54a4..68b8c9a92 100644 --- a/hive_integration/nodocker/engine/engine_sim.nim +++ b/hive_integration/nodocker/engine/engine_sim.nim @@ -9,7 +9,7 @@ proc main() = let start = getTime() for x in engineTestList: - var t = setupELClient() + var t = setupELClient(x.chainFile) t.setRealTTD(x.ttd) let status = x.run(t) t.stopELClient() @@ -17,5 +17,6 @@ proc main() = let elpd = getTime() - start print(stat, elpd, "engine") + echo stat main() diff --git a/hive_integration/nodocker/engine/engine_tests.nim b/hive_integration/nodocker/engine/engine_tests.nim index 292999893..ca18b49f7 100644 --- a/hive_integration/nodocker/engine/engine_tests.nim +++ b/hive_integration/nodocker/engine/engine_tests.nim @@ -16,18 +16,91 @@ type name*: string run*: proc(t: TestEnv): TestStatus ttd*: int64 + chainFile*: string const prevRandaoContractAddr = hexToByteArray[20]("0000000000000000000000000000000000000316") template testCond(expr: untyped) = if not (expr): - return TestStatus.Failed + when result is bool: + return false + else: + return TestStatus.Failed template testCond(expr, body: untyped) = if not (expr): body - return TestStatus.Failed + when result is bool: + return false + else: + return TestStatus.Failed + +proc `$`(x: Option[Hash256]): string = + if x.isNone: + "none" + else: + x.get().data.toHex + +proc `$`(x: Option[BlockHash]): string = + if x.isNone: + "none" + else: + x.get().toHex + +proc `$`(x: Option[PayloadID]): string = + if x.isNone: + "none" + else: + x.get().toHex + +proc `==`(a: Option[BlockHash], b: Option[Hash256]): bool = + if a.isNone or b.isNone: + return false + a.get() == b.get().data.BlockHash + +template testFCU(res, cond: untyped, validHash: Option[Hash256], id = none(PayloadID)) = + testCond res.isOk + let s = res.get() + testCond s.payloadStatus.status == PayloadExecutionStatus.cond: + error "Unexpected FCU status", expect=PayloadExecutionStatus.cond, get=s.payloadStatus.status + testCond s.payloadStatus.latestValidHash == validHash: + error "Unexpected FCU latestValidHash", expect=validHash, get=s.payloadStatus.latestValidHash + testCond s.payloadId == id: + error "Unexpected FCU payloadID", expect=id, get=s.payloadId + +template testFCU(res, cond: untyped) = + testCond res.isOk + let s = res.get() + testCond s.payloadStatus.status == PayloadExecutionStatus.cond: + error "Unexpected FCU status", expect=PayloadExecutionStatus.cond, get=s.payloadStatus.status + +template testNP(res, cond: untyped, validHash = none(Hash256)) = + testCond res.isOk + let s = res.get() + testCond s.status == PayloadExecutionStatus.cond: + error "Unexpected NewPayload status", expect=PayloadExecutionStatus.cond, get=s.status + testCond s.latestValidHash == validHash: + error "Unexpected NewPayload latestValidHash", expect=validHash, get=s.latestValidHash + +template testNPEither(res, cond: untyped, validHash = none(Hash256)) = + testCond res.isOk + let s = res.get() + testCond s.status in cond: + error "Unexpected NewPayload status", expect=cond, get=s.status + testCond s.latestValidHash == validHash: + error "Unexpected NewPayload latestValidHash", expect=validHash, get=s.latestValidHash + +proc sendTx(t: TestEnv, recipient: EthAddress, val: UInt256, data: openArray[byte] = []): bool = + t.tx = t.makeNextTransaction(recipient, val, data) + let rr = t.rpcClient.sendTransaction(t.tx) + if rr.isErr: + error "Unable to send transaction", msg=rr.error + return false + return true + +proc sendTx(t: TestEnv, val: UInt256): bool = + t.sendTx(prevRandaoContractAddr, val) # Invalid Terminal Block in ForkchoiceUpdated: # Client must reject ForkchoiceUpdated directives if the referenced HeadBlockHash does not meet the TTD requirement. @@ -43,17 +116,11 @@ proc invalidTerminalBlockForkchoiceUpdated(t: TestEnv): TestStatus = ) let res = t.rpcClient.forkchoiceUpdatedV1(forkchoiceState) - # Execution specification: # {payloadStatus: {status: INVALID, latestValidHash=0x00..00}, payloadId: null} # either obtained from the Payload validation process or as a result of validating a PoW block referenced by forkchoiceState.headBlockHash - testCond res.isOk - - let s = res.get() - testCond s.payloadStatus.status == PayloadExecutionStatus.invalid - testCond s.payloadStatus.latestValidHash.isNone - testCond s.payloadId.isNone + testFCU(res, invalid, some(Hash256())) # ValidationError is not validated since it can be either null or a string message # Check that PoW chain progresses @@ -91,13 +158,9 @@ proc invalidTerminalBlockNewPayload(t: TestEnv): TestStatus = let res = t.rpcClient.newPayloadV1(hashedPayload) # Execution specification: - # {status: INVALID_TERMINAL_BLOCK, latestValidHash: null, validationError: errorMessage | null} + # {status: INVALID, latestValidHash=0x00..00} # if terminal block conditions are not satisfied - testCond res.isOk - - let s = res.get() - testCond s.status == PayloadExecutionStatus.invalid - testCond s.latestValidHash.isNone + testNP(res, invalid, some(Hash256())) # Check that PoW chain progresses testCond t.verifyPoWProgress(t.gHeader.blockHash) @@ -346,7 +409,6 @@ template invalidPayloadAttributesGen(procname: untyped, syncingCond: bool) = else: let r = client.forkchoiceUpdatedV1(fcu, some(attr)) if r.isOk: - debugEcho "EEEE" return false # Check that the forkchoice was applied, regardless of the error @@ -385,13 +447,10 @@ proc preTTDFinalizedBlockHash(t: TestEnv): TestStatus = clMock = t.clMock var res = client.forkchoiceUpdatedV1(forkchoiceState) - # TBD: Behavior on this edge-case is undecided, as behavior of the Execution client - # if not defined on re-orgs to a point before the latest finalized block. + testFCU(res, invalid, some(Hash256())) res = client.forkchoiceUpdatedV1(clMock.latestForkchoice) - testCond res.isOk - let s = res.get() - testCond s.payloadStatus.status == PayloadExecutionStatus.valid + testFCU(res, valid) # Corrupt the hash of a valid payload, client should reject the payload. # All possible scenarios: @@ -548,6 +607,55 @@ proc parentHashOnExecPayload(t: TestEnv): TestStatus = )) testCond produceSingleBlockRes +# Attempt to re-org to a chain containing an invalid transition payload +proc invalidTransitionPayload(t: TestEnv): TestStatus = + result = TestStatus.OK + + # Wait until TTD is reached by main client + let ok = waitFor t.clMock.waitForTTD() + testCond ok + + let clMock = t.clMock + let client = t.rpcClient + + # Produce two blocks before trying to re-org + t.nonce = 2 # Initial PoW chain already contains 2 transactions + var pbRes = clMock.produceBlocks(2, BlockProcessCallbacks( + onPayloadProducerSelected: proc(): bool = + t.sendTx(1.u256) + )) + + testCond pbRes + + # Introduce the invalid transition payload + pbRes = clMock.produceSingleBlock(BlockProcessCallbacks( + # This is being done in the middle of the block building + # process simply to be able to re-org back. + onGetPayload: proc(): bool = + let basePayload = clMock.executedPayloadHistory[clMock.posBlockNumber] + let alteredPayload = generateInvalidPayload(basePayload, InvalidStateRoot) + + let res = client.newPayloadV1(alteredPayload) + let cond = {PayloadExecutionStatus.invalid, PayloadExecutionStatus.accepted} + testNPEither(res, cond, some(Hash256())) + + let rr = client.forkchoiceUpdatedV1( + ForkchoiceStateV1(headBlockHash: alteredPayload.blockHash) + ) + testFCU(rr, invalid, some(Hash256())) + + var header: EthBlockHeader + let rz = client.latestHeader(header) + if rz.isErr: + error "unable to get header", msg=rz.error + return false + + let blockHash = BlockHash header.blockHash.data + blockHash == clMock.latestExecutedPayload.blockHash + )) + + testCond pbRes + template invalidPayloadTestCaseGen(procName: untyped, payloadField: InvalidPayloadField, emptyTxs: bool = false) = proc procName(t: TestEnv): TestStatus = result = TestStatus.OK @@ -559,22 +667,17 @@ template invalidPayloadTestCaseGen(procName: untyped, payloadField: InvalidPaylo let clMock = t.clMock let client = t.rpcClient - template txProc() = + template txProc(): bool = when not emptyTxs: - let - tx = t.makeNextTransaction(prevRandaoContractAddr, 0.u256) - rr = client.sendTransaction(tx) - - if rr.isErr: - error "Unable to send transaction", msg=rr.error - return false + t.sendTx(0.u256) + else: + true # Produce blocks before starting the test var pbRes = clMock.produceBlocks(5, BlockProcessCallbacks( # Make sure at least one transaction is included in each block onPayloadProducerSelected: proc(): bool = txProc() - return true )) testCond pbRes @@ -585,7 +688,6 @@ template invalidPayloadTestCaseGen(procName: untyped, payloadField: InvalidPaylo # Make sure at least one transaction is included in the payload onPayloadProducerSelected: proc(): bool = txProc() - return true , # Run test after the new payload has been obtained onGetPayload: proc(): bool = @@ -598,8 +700,7 @@ template invalidPayloadTestCaseGen(procName: untyped, payloadField: InvalidPaylo error "No transactions in the base payload" return false - let execData = clMock.latestPayloadBuilt.toExecutableData - let alteredPayload = generateInvalidPayload(execData, payloadField, t.vaultKey) + let alteredPayload = generateInvalidPayload(clMock.latestPayloadBuilt, payloadField, t.vaultKey) invalidPayload.hash = hash256(alteredPayload.blockHash) # Depending on the field we modified, we expect a different status @@ -735,13 +836,8 @@ template blockStatusExecPayloadGen(procname: untyped, transitionBlock: bool) = var produceSingleBlockRes = clMock.produceSingleBlock(BlockProcessCallbacks( onPayloadProducerSelected: proc(): bool = var address: EthAddress - let tx = t.makeNextTransaction(address, 1.u256) - let res = client.sendTransaction(tx) - if res.isErr: - error "Unable to send transaction" - return false - - shadow.hash = rlpHash(tx) + testCond t.sendTx(address, 1.u256) + shadow.hash = rlpHash(t.tx) return true , onNewPayloadBroadcast: proc(): bool = @@ -804,13 +900,8 @@ template blockStatusHeadBlockGen(procname: untyped, transitionBlock: bool) = var produceSingleBlockRes = clMock.produceSingleBlock(BlockProcessCallbacks( onPayloadProducerSelected: proc(): bool = var address: EthAddress - let tx = t.makeNextTransaction(address, 1.u256) - let res = client.sendTransaction(tx) - if res.isErr: - error "Unable to send transaction" - return false - - shadow.hash = rlpHash(tx) + testCond t.sendTx(address, 1.u256) + shadow.hash = rlpHash(t.tx) return true , # Run test after a forkchoice with new HeadBlockHash has been broadcasted @@ -858,13 +949,8 @@ template blockStatusSafeBlockGen(procname: untyped, transitionBlock: bool) = var produceSingleBlockRes = clMock.produceSingleBlock(BlockProcessCallbacks( onPayloadProducerSelected: proc(): bool = var address: EthAddress - let tx = t.makeNextTransaction(address, 1.u256) - let res = client.sendTransaction(tx) - if res.isErr: - error "Unable to send transaction" - return false - - shadow.hash = rlpHash(tx) + testCond t.sendTx(address, 1.u256) + shadow.hash = rlpHash(t.tx) return true , # Run test after a forkchoice with new HeadBlockHash has been broadcasted @@ -911,13 +997,8 @@ template blockStatusFinalizedBlockGen(procname: untyped, transitionBlock: bool) var produceSingleBlockRes = clMock.produceSingleBlock(BlockProcessCallbacks( onPayloadProducerSelected: proc(): bool = var address: EthAddress - let tx = t.makeNextTransaction(address, 1.u256) - let res = client.sendTransaction(tx) - if res.isErr: - error "Unable to send transaction" - return false - - shadow.hash = rlpHash(tx) + testCond t.sendTx(address, 1.u256) + shadow.hash = rlpHash(t.tx) return true , # Run test after a forkchoice with new HeadBlockHash has been broadcasted @@ -1149,11 +1230,7 @@ proc outOfOrderPayloads(t: TestEnv): TestStatus = # We send the transactions after we got the Payload ID, before the clMocker gets the prepared Payload onPayloadProducerSelected: proc(): bool = for i in 0.. 0: + # disable clique if we are using PoW chain + t.conf.networkParams.config.poaEngine = false + t.ctx = newEthContext() let res = t.ctx.am.importPrivateKey(sealerKey) if res.isErr: @@ -96,7 +103,13 @@ proc setupELClient*(t: TestEnv) = setupEngineAPI(t.sealingEngine, t.rpcServer) setupDebugRpc(t.chainDB, t.rpcServer) - t.sealingEngine.start() + # Do not start clique sealing engine if we are using a Proof of Work chain file + if chainFile.len > 0: + if not importRlpBlock(chainFolder / chainFile, t.chainDB): + quit(QuitFailure) + else: + t.sealingEngine.start() + t.rpcServer.start() t.rpcClient = newRpcSocketClient() @@ -110,11 +123,11 @@ proc setupELClient*(t: TestEnv) = t.vaultKey = kRes.get -proc setupELClient*(): TestEnv = +proc setupELClient*(chainFile: string): TestEnv = result = TestEnv( conf: makeConfig(@["--engine-signer:658bdf435d810c91414ec09147daa6db62406379", "--custom-network:" & genesisFile]) ) - setupELClient(result) + setupELClient(result, chainFile) proc stopELClient*(t: TestEnv) = waitFor t.rpcClient.close() diff --git a/hive_integration/nodocker/sim_utils.nim b/hive_integration/nodocker/sim_utils.nim index a2648b8e1..67b199885 100644 --- a/hive_integration/nodocker/sim_utils.nim +++ b/hive_integration/nodocker/sim_utils.nim @@ -19,6 +19,7 @@ type ok*: int skipped*: int failed*: int + failingCases*: seq[string] proc inc*(stat: var SimStat, name: string, status: TestStatus) = echo name, ", ", status @@ -28,9 +29,13 @@ proc inc*(stat: var SimStat, name: string, status: TestStatus) = inc stat.skipped else: inc stat.failed + stat.failingCases.add name proc `$`*(stat: SimStat): string = - "ok: $1, skipped: $2, failed: $3" % [$stat.ok, $stat.skipped, $stat.failed] + result.add "Failing Cases:\n" + for c in stat.failingCases: + result.add "- $1 \n" % [c] + result.add "ok: $1, skipped: $2, failed: $3" % [$stat.ok, $stat.skipped, $stat.failed] proc print*(stat: SimStat, dur: Duration, name: string) = var f = open(name & ".md", fmWrite) diff --git a/nimbus/merge/mergeutils.nim b/nimbus/merge/mergeutils.nim index 4e1ce58ef..d0f90887c 100644 --- a/nimbus/merge/mergeutils.nim +++ b/nimbus/merge/mergeutils.nim @@ -75,6 +75,14 @@ proc simpleFCU*(status: PayloadExecutionStatus): ForkchoiceUpdatedResponse = proc simpleFCU*(status: PayloadExecutionStatus, msg: string): ForkchoiceUpdatedResponse = ForkchoiceUpdatedResponse(payloadStatus: PayloadStatusV1(status: status, validationError: some(msg))) +proc invalidFCU*(hash: Hash256 = Hash256()): ForkchoiceUpdatedResponse = + ForkchoiceUpdatedResponse(payloadStatus: + PayloadStatusV1( + status: PayloadExecutionStatus.invalid, + latestValidHash: some(BlockHash hash.data) + ) + ) + proc validFCU*(id: Option[PayloadID], validHash: Hash256): ForkchoiceUpdatedResponse = ForkchoiceUpdatedResponse( payloadStatus: PayloadStatusV1( @@ -91,8 +99,13 @@ proc invalidStatus*(validHash: Hash256, msg: string): PayloadStatusV1 = validationError: some(msg) ) +proc invalidStatus*(validHash: Hash256 = Hash256()): PayloadStatusV1 = + PayloadStatusV1( + status: PayloadExecutionStatus.invalid, + latestValidHash: some(BlockHash validHash.data) + ) + proc toBlockBody*(payload: ExecutionPayloadV1): BlockBody = - # TODO the transactions from the payload have to be converted here result.transactions.setLen(payload.transactions.len) for i, tx in payload.transactions: result.transactions[i] = rlp.decode(distinctBase tx, Transaction) diff --git a/nimbus/rpc/engine_api.nim b/nimbus/rpc/engine_api.nim index b82c1d6e7..c718d0b88 100644 --- a/nimbus/rpc/engine_api.nim +++ b/nimbus/rpc/engine_api.nim @@ -84,7 +84,7 @@ proc setupEngineAPI*( if td < ttd: warn "Ignoring pre-merge payload", number = header.blockNumber, hash = blockHash.data.toHex, td, ttd - return PayloadStatusV1(status: PayloadExecutionStatus.invalid) + return invalidStatus() if header.timestamp <= parent.timestamp: warn "Invalid timestamp", @@ -96,7 +96,14 @@ proc setupEngineAPI*( let body = toBlockBody(payload) let vres = sealingEngine.chain.insertBlockWithoutSetHead(header, body) if vres != ValidationResult.OK: - return invalidStatus(db.getHeadBlockHash(), "Failed to insert block") + let ptd = db.getScore(parent.parentHash) + let blockHash = if ptd >= ttd: + db.getHeadBlockHash() + else: + # If the most recent valid ancestor is a PoW block, + # latestValidHash MUST be set to ZERO + Hash256() + return invalidStatus(blockHash, "Failed to insert block") # We've accepted a valid payload from the beacon client. Mark the local # chain transitions to notify other subsystems (e.g. downloader) of the @@ -231,7 +238,7 @@ proc setupEngineAPI*( ptd = ptd, ttd = ttd - return simpleFCU(PayloadExecutionStatus.invalid) + return invalidFCU() # If the head block is already in our canonical chain, the beacon client is # probably resyncing. Ignore the update.