From 3428cd56dd9c91632db4b1df6041e44e8ec6d12d Mon Sep 17 00:00:00 2001 From: marioevz Date: Mon, 25 Apr 2022 16:21:54 +0000 Subject: [PATCH 1/7] simulators/engine: reorg to missing invalid payload chain tests --- simulators/ethereum/engine/enginetests.go | 110 ++++++++++++++++++++++ 1 file changed, 110 insertions(+) diff --git a/simulators/ethereum/engine/enginetests.go b/simulators/ethereum/engine/enginetests.go index b0b4ad2809..c02136a730 100644 --- a/simulators/ethereum/engine/enginetests.go +++ b/simulators/ethereum/engine/enginetests.go @@ -197,6 +197,16 @@ var engineTests = []TestSpec{ Run: invalidPayloadTestCaseGen("Transaction/Value", true), }, + // Invalid Payload Re-Org Tests + { + Name: "Re-Org to Chain Missing Invalid Parent (Invalid after common ancestor)", + Run: reOrgToChainMissingInvalidParentGen(1), + }, + { + Name: "Re-Org to Chain Missing Invalid Parent (Invalid two payloads after common ancestor)", + Run: reOrgToChainMissingInvalidParentGen(3), + }, + // Eth RPC Status on ForkchoiceUpdated Events { Name: "Latest Block after NewPayload", @@ -962,6 +972,106 @@ func invalidPayloadTestCaseGen(payloadField string, syncing bool) func(*TestEnv) } } +// Attempt to re-org to a chain which at some point contains an unknown payload which is also invalid. +// Then reveal the invalid payload and expect that the client rejects it and rejects forkchoice updated calls to this chain. +// The invalid_index parameter determines how many payloads apart is the common ancestor from the block that invalidates the chain, +// with a value of 1 meaning that the immediate payload after the common ancestor will be invalid. +func reOrgToChainMissingInvalidParentGen(invalid_index int) func(*TestEnv) { + return func(t *TestEnv) { + // Wait until TTD is reached by this client + t.CLMock.waitForTTD() + + // Produce blocks before starting the test + t.CLMock.produceBlocks(5, BlockProcessCallbacks{}) + + // Save the common ancestor + cA := t.CLMock.LatestPayloadBuilt + + // Amount of blocks to deviate starting from the common ancestor + n := 10 + + // Slice to save the alternate B chain + altChainPayloads := make([]*ExecutableDataV1, 0) + + // Append the common ancestor + altChainPayloads = append(altChainPayloads, &cA) + + // Produce blocks but at the same time create an alternate chain which contains an invalid parent (INV_P1) + // CommonAncestor◄─▲── P1 ◄──── P2 ◄─ P3 ◄─ ... ◄─ Pn + // │ + // └──INV_P1 ◄─ P2' ◄─ P3' ◄─ ... ◄─ Pn' + t.CLMock.produceBlocks(n, BlockProcessCallbacks{ + OnGetPayload: func() { + var ( + alternatePayload *ExecutableDataV1 + err error + ) + // Create another prevRandao to ensure we deviate from the main payload + prevRandao := common.Hash{} + rand.Read(prevRandao[:]) + if len(altChainPayloads) == invalid_index { + // This is the payload we are looking to invalidate, this must be invalid + invStateRoot := common.Hash{} + rand.Read(invStateRoot[:]) + alternatePayload, err = customizePayload(&t.CLMock.LatestPayloadBuilt, &CustomPayloadData{ + StateRoot: &invStateRoot, + ParentHash: &altChainPayloads[len(altChainPayloads)-1].BlockHash, + PrevRandao: &prevRandao, + }) + } else { + // Modify the created payload only to reference the previous parent in the B chain + alternatePayload, err = customizePayload(&t.CLMock.LatestPayloadBuilt, &CustomPayloadData{ + ParentHash: &altChainPayloads[len(altChainPayloads)-1].BlockHash, + PrevRandao: &prevRandao, + }) + } + if err != nil { + t.Fatalf("FAIL (%s): Unable to customize payload: %v", t.TestName, err) + } + altChainPayloads = append(altChainPayloads, alternatePayload) + }, + }) + + // Now let's send the alternate chain to the client using newPayload in reverse order + for i := n - 1; i >= 0; i-- { + // Send the payload + r := t.TestEngine.TestEngineNewPayloadV1(altChainPayloads[i]) + if i == 1 { + if i == invalid_index { + // If this is the first payload after the common ancestor, and this is the payload we invalidated, + // then we have all the information to determine that this payload is invalid. + r.ExpectStatus(Invalid) + r.ExpectLatestValidHash(&cA.BlockHash) + } else { + // This is the first payload, but we did not invalidated the chain until after this one, + // therefore the expected status is valid + r.ExpectStatus(Valid) + } + + } else if i > 1 { + r.ExpectStatus(Accepted) + r.ExpectLatestValidHash(nil) + } + + if i == n-1 { + // Send the fcU, this is before we send all the payloads from the alternative chain. + // At this point is impossible to give a latestValidHash, since we don't know the required information. + s := t.TestEngine.TestEngineForkchoiceUpdatedV1(&ForkchoiceStateV1{ + HeadBlockHash: altChainPayloads[i].BlockHash, + SafeBlockHash: cA.BlockHash, + FinalizedBlockHash: cA.BlockHash, + }, nil) + s.ExpectPayloadStatus(Syncing) + s.ExpectLatestValidHash(nil) + } + } + + // Resend the latest correct fcU + r := t.TestEngine.TestEngineForkchoiceUpdatedV1(&t.CLMock.LatestForkchoice, nil) + r.ExpectNoError() + } +} + // Test to verify Block information available at the Eth RPC after NewPayload func blockStatusExecPayload(t *TestEnv) { // Wait until this client catches up with latest PoS Block From 977c2035afc50564a62f43b4ea4be02a624c4d88 Mon Sep 17 00:00:00 2001 From: marioevz Date: Wed, 4 May 2022 15:32:26 +0000 Subject: [PATCH 2/7] simulators/ethereum/engine: add contract transaction helper --- simulators/ethereum/engine/testenv.go | 51 +++++++++++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/simulators/ethereum/engine/testenv.go b/simulators/ethereum/engine/testenv.go index 874e54249b..cff91bc688 100644 --- a/simulators/ethereum/engine/testenv.go +++ b/simulators/ethereum/engine/testenv.go @@ -2,6 +2,7 @@ package main import ( "context" + "encoding/binary" "fmt" "math/big" "net/http" @@ -168,6 +169,56 @@ func (t *TestEnv) sendNextTransaction(sender *EngineClient, recipient common.Add } } +// Method that attempts to create a contract filled with zeros without going over the specified gasLimit +func (t *TestEnv) makeNextBigContractTransaction(gasLimit uint64) *types.Transaction { + // Total GAS: Gtransaction == 21000, Gcreate == 32000, Gcodedeposit == 200 + contractLength := uint64(0) + if gasLimit > (21000 + 32000) { + contractLength = (gasLimit - 21000 - 32000) / 200 + if contractLength >= 1 { + // Reduce by 1 to guarantee using less gas than requested + contractLength -= 1 + } + } + buf := make([]byte, 8) + binary.BigEndian.PutUint64(buf, contractLength) + + initCode := []byte{ + 0x67, // PUSH8 + } + initCode = append(initCode, buf...) // Size of the contract in byte length + initCode = append(initCode, 0x38) // CODESIZE == 0x00 + initCode = append(initCode, 0xF3) // RETURN(offset, length) + + txData := types.LegacyTx{ + Nonce: t.nonce, + GasPrice: gasPrice, + Gas: gasLimit, + To: nil, + Value: big0, + Data: initCode, + } + signer := types.NewEIP155Signer(chainID) + signedTx := types.MustSignNewTx(vaultKey, signer, &txData) + t.nonce++ + return signedTx +} + +func (t *TestEnv) sendNextBigContractTransaction(sender *EngineClient, gasLimit uint64) *types.Transaction { + tx := t.makeNextBigContractTransaction(gasLimit) + for { + err := sender.Eth.SendTransaction(sender.Ctx(), tx) + if err == nil { + return tx + } + select { + case <-time.After(time.Second): + case <-t.Timeout: + t.Fatalf("FAIL (%s): Timeout while trying to send transaction: %v", t.TestName, err) + } + } +} + // CallContext is a helper method that forwards a raw RPC request to // the underlying RPC client. This can be used to call RPC methods // that are not supported by the ethclient.Client. From c04a54948fae7ef4c501734508b7dc42f600bcac Mon Sep 17 00:00:00 2001 From: marioevz Date: Fri, 6 May 2022 00:16:35 +0000 Subject: [PATCH 3/7] simulators/ethereum/engine: Add p2p sync to invalid payloads test --- simulators/ethereum/engine/enginetests.go | 474 ++++++++++++---------- simulators/ethereum/engine/helper.go | 151 +++++++ 2 files changed, 415 insertions(+), 210 deletions(-) diff --git a/simulators/ethereum/engine/enginetests.go b/simulators/ethereum/engine/enginetests.go index c02136a730..94a61f62b2 100644 --- a/simulators/ethereum/engine/enginetests.go +++ b/simulators/ethereum/engine/enginetests.go @@ -1,14 +1,13 @@ package main import ( + "encoding/json" "fmt" "math/big" "math/rand" - "strings" "time" "github.com/ethereum/go-ethereum/common" - "github.com/ethereum/go-ethereum/common/hexutil" "github.com/ethereum/go-ethereum/core/types" "github.com/ethereum/hive/hivesim" ) @@ -64,6 +63,7 @@ var engineTests = []TestSpec{ Run: preTTDFinalizedBlockHash, TTD: 2, }, + // Invalid Payload Tests { Name: "Bad Hash on NewPayload", Run: badHashOnNewPayloadGen(false, false), @@ -86,125 +86,199 @@ var engineTests = []TestSpec{ }, { Name: "Invalid ParentHash NewPayload", - Run: invalidPayloadTestCaseGen("ParentHash", false), + Run: invalidPayloadTestCaseGen(InvalidParentHash, false, false), }, { Name: "Invalid ParentHash NewPayload (Syncing)", - Run: invalidPayloadTestCaseGen("ParentHash", true), + Run: invalidPayloadTestCaseGen(InvalidParentHash, true, false), }, { Name: "Invalid StateRoot NewPayload", - Run: invalidPayloadTestCaseGen("StateRoot", false), + Run: invalidPayloadTestCaseGen(InvalidStateRoot, false, false), }, { Name: "Invalid StateRoot NewPayload (Syncing)", - Run: invalidPayloadTestCaseGen("StateRoot", true), + Run: invalidPayloadTestCaseGen(InvalidStateRoot, true, false), + }, + { + Name: "Invalid StateRoot NewPayload, Empty Transactions", + Run: invalidPayloadTestCaseGen(InvalidStateRoot, false, true), + }, + { + Name: "Invalid StateRoot NewPayload, Empty Transactions (Syncing)", + Run: invalidPayloadTestCaseGen(InvalidStateRoot, true, true), }, { Name: "Invalid ReceiptsRoot NewPayload", - Run: invalidPayloadTestCaseGen("ReceiptsRoot", false), + Run: invalidPayloadTestCaseGen(InvalidReceiptsRoot, false, false), }, { Name: "Invalid ReceiptsRoot NewPayload (Syncing)", - Run: invalidPayloadTestCaseGen("ReceiptsRoot", true), + Run: invalidPayloadTestCaseGen(InvalidReceiptsRoot, true, false), }, { Name: "Invalid Number NewPayload", - Run: invalidPayloadTestCaseGen("Number", false), + Run: invalidPayloadTestCaseGen(InvalidNumber, false, false), }, { Name: "Invalid Number NewPayload (Syncing)", - Run: invalidPayloadTestCaseGen("Number", true), + Run: invalidPayloadTestCaseGen(InvalidNumber, true, false), }, { Name: "Invalid GasLimit NewPayload", - Run: invalidPayloadTestCaseGen("GasLimit", false), + Run: invalidPayloadTestCaseGen(InvalidGasLimit, false, false), }, { Name: "Invalid GasLimit NewPayload (Syncing)", - Run: invalidPayloadTestCaseGen("GasLimit", true), + Run: invalidPayloadTestCaseGen(InvalidGasLimit, true, false), }, { Name: "Invalid GasUsed NewPayload", - Run: invalidPayloadTestCaseGen("GasUsed", false), + Run: invalidPayloadTestCaseGen(InvalidGasUsed, false, false), }, { Name: "Invalid GasUsed NewPayload (Syncing)", - Run: invalidPayloadTestCaseGen("GasUsed", true), + Run: invalidPayloadTestCaseGen(InvalidGasUsed, true, false), }, { Name: "Invalid Timestamp NewPayload", - Run: invalidPayloadTestCaseGen("Timestamp", false), + Run: invalidPayloadTestCaseGen(InvalidTimestamp, false, false), }, { Name: "Invalid Timestamp NewPayload (Syncing)", - Run: invalidPayloadTestCaseGen("Timestamp", true), + Run: invalidPayloadTestCaseGen(InvalidTimestamp, true, false), }, { Name: "Invalid PrevRandao NewPayload", - Run: invalidPayloadTestCaseGen("PrevRandao", false), + Run: invalidPayloadTestCaseGen(InvalidPrevRandao, false, false), }, { Name: "Invalid PrevRandao NewPayload (Syncing)", - Run: invalidPayloadTestCaseGen("PrevRandao", true), + Run: invalidPayloadTestCaseGen(InvalidPrevRandao, true, false), }, { Name: "Invalid Incomplete Transactions NewPayload", - Run: invalidPayloadTestCaseGen("RemoveTransaction", false), + Run: invalidPayloadTestCaseGen(RemoveTransaction, false, false), }, { Name: "Invalid Incomplete Transactions NewPayload (Syncing)", - Run: invalidPayloadTestCaseGen("RemoveTransaction", true), + Run: invalidPayloadTestCaseGen(RemoveTransaction, true, false), }, { Name: "Invalid Transaction Signature NewPayload", - Run: invalidPayloadTestCaseGen("Transaction/Signature", false), + Run: invalidPayloadTestCaseGen(InvalidTransactionSignature, false, false), }, { Name: "Invalid Transaction Signature NewPayload (Syncing)", - Run: invalidPayloadTestCaseGen("Transaction/Signature", true), + Run: invalidPayloadTestCaseGen(InvalidTransactionSignature, true, false), }, { Name: "Invalid Transaction Nonce NewPayload", - Run: invalidPayloadTestCaseGen("Transaction/Nonce", false), + Run: invalidPayloadTestCaseGen(InvalidTransactionNonce, false, false), }, { Name: "Invalid Transaction Nonce NewPayload (Syncing)", - Run: invalidPayloadTestCaseGen("Transaction/Nonce", true), + Run: invalidPayloadTestCaseGen(InvalidTransactionNonce, true, false), }, { Name: "Invalid Transaction GasPrice NewPayload", - Run: invalidPayloadTestCaseGen("Transaction/GasPrice", false), + Run: invalidPayloadTestCaseGen(InvalidTransactionGasPrice, false, false), }, { Name: "Invalid Transaction GasPrice NewPayload (Syncing)", - Run: invalidPayloadTestCaseGen("Transaction/GasPrice", true), + Run: invalidPayloadTestCaseGen(InvalidTransactionGasPrice, true, false), }, { Name: "Invalid Transaction Gas NewPayload", - Run: invalidPayloadTestCaseGen("Transaction/Gas", false), + Run: invalidPayloadTestCaseGen(InvalidTransactionGas, false, false), }, { Name: "Invalid Transaction Gas NewPayload (Syncing)", - Run: invalidPayloadTestCaseGen("Transaction/Gas", true), + Run: invalidPayloadTestCaseGen(InvalidTransactionGas, true, false), }, { Name: "Invalid Transaction Value NewPayload", - Run: invalidPayloadTestCaseGen("Transaction/Value", false), + Run: invalidPayloadTestCaseGen(InvalidTransactionValue, false, false), }, { Name: "Invalid Transaction Value NewPayload (Syncing)", - Run: invalidPayloadTestCaseGen("Transaction/Value", true), + Run: invalidPayloadTestCaseGen(InvalidTransactionValue, true, false), }, - // Invalid Payload Re-Org Tests + // Invalid Ancestor Re-Org Tests (Reveal via newPayload) + { + Name: "Invalid Ancestor Chain Re-Org, Invalid StateRoot, INV_P1', Reveal using newPayload", + Run: invalidMissingAncestorReOrgGen(1, InvalidStateRoot, false, true), + }, + { + Name: "Invalid Ancestor Chain Re-Org, Invalid StateRoot, INV_P5', Reveal using newPayload", + Run: invalidMissingAncestorReOrgGen(5, InvalidStateRoot, false, true), + }, + { + Name: "Invalid Ancestor Chain Re-Org, Invalid StateRoot, INV_P10', Reveal using newPayload", + Run: invalidMissingAncestorReOrgGen(10, InvalidStateRoot, false, true), + }, + + // Invalid Ancestor Re-Org Tests (Reveal via sync through secondary client) + { + Name: "Invalid Ancestor Chain Re-Org, Invalid StateRoot, INV_P5', Reveal using sync", + Run: invalidMissingAncestorReOrgGen(5, InvalidStateRoot, true, false), + }, + { + Name: "Invalid Ancestor Chain Re-Org, Invalid StateRoot, Empty Txs, INV_P5', Reveal using sync", + Run: invalidMissingAncestorReOrgGen(5, InvalidStateRoot, true, true), + }, + { + Name: "Invalid Ancestor Chain Re-Org, Invalid ReceiptsRoot, INV_P5', Reveal using sync", + Run: invalidMissingAncestorReOrgGen(5, InvalidReceiptsRoot, true, false), + }, + { + Name: "Invalid Ancestor Chain Re-Org, Invalid Number, INV_P5', Reveal using sync", + Run: invalidMissingAncestorReOrgGen(5, InvalidNumber, true, false), + }, + { + Name: "Invalid Ancestor Chain Re-Org, Invalid GasLimit, INV_P5', Reveal using sync", + Run: invalidMissingAncestorReOrgGen(5, InvalidGasLimit, true, false), + }, + { + Name: "Invalid Ancestor Chain Re-Org, Invalid GasUsed, INV_P5', Reveal using sync", + Run: invalidMissingAncestorReOrgGen(5, InvalidGasUsed, true, false), + }, + { + Name: "Invalid Ancestor Chain Re-Org, Invalid Timestamp, INV_P5', Reveal using sync", + Run: invalidMissingAncestorReOrgGen(5, InvalidTimestamp, true, false), + }, + { + Name: "Invalid Ancestor Chain Re-Org, Invalid PrevRandao, INV_P5', Reveal using sync", + Run: invalidMissingAncestorReOrgGen(5, InvalidPrevRandao, true, false), + }, + { + Name: "Invalid Ancestor Chain Re-Org, Incomplete Transactions, INV_P5', Reveal using sync", + Run: invalidMissingAncestorReOrgGen(5, RemoveTransaction, true, false), + }, { - Name: "Re-Org to Chain Missing Invalid Parent (Invalid after common ancestor)", - Run: reOrgToChainMissingInvalidParentGen(1), + Name: "Invalid Ancestor Chain Re-Org, Invalid Transaction Signature, INV_P5', Reveal using sync", + Run: invalidMissingAncestorReOrgGen(5, InvalidTransactionSignature, true, false), }, { - Name: "Re-Org to Chain Missing Invalid Parent (Invalid two payloads after common ancestor)", - Run: reOrgToChainMissingInvalidParentGen(3), + Name: "Invalid Ancestor Chain Re-Org, Invalid Transaction Nonce, INV_P5', Reveal using sync", + Run: invalidMissingAncestorReOrgGen(5, InvalidTransactionNonce, true, false), + }, + { + Name: "Invalid Ancestor Chain Re-Org, Invalid Transaction Gas, INV_P5', Reveal using sync", + Run: invalidMissingAncestorReOrgGen(5, InvalidTransactionGas, true, false), + }, + { + Name: "Invalid Ancestor Chain Re-Org, Invalid Transaction GasPrice, INV_P5', Reveal using sync", + Run: invalidMissingAncestorReOrgGen(5, InvalidTransactionGasPrice, true, false), + }, + { + Name: "Invalid Ancestor Chain Re-Org, Invalid Transaction Value, INV_P5', Reveal using sync", + Run: invalidMissingAncestorReOrgGen(5, InvalidTransactionValue, true, false), + }, + { + Name: "Invalid Ancestor Chain Re-Org, Invalid StateRoot, INV_P10', Reveal using sync", + Run: invalidMissingAncestorReOrgGen(10, InvalidStateRoot, true, true), }, // Eth RPC Status on ForkchoiceUpdated Events @@ -670,7 +744,7 @@ func parentHashOnExecPayload(t *TestEnv) { } // Generate test cases for each field of NewPayload, where the payload contains a single invalid field and a valid hash. -func invalidPayloadTestCaseGen(payloadField string, syncing bool) func(*TestEnv) { +func invalidPayloadTestCaseGen(payloadField InvalidPayloadField, syncing bool, emptyTxs bool) func(*TestEnv) { return func(t *TestEnv) { if syncing { @@ -686,9 +760,11 @@ func invalidPayloadTestCaseGen(payloadField string, syncing bool) func(*TestEnv) t.CLMock.waitForTTD() txFunc := func() { - // Function to send at least one transaction each block produced - // Send the transaction to the prevRandaoContractAddr - t.sendNextTransaction(t.CLMock.NextBlockProducer, prevRandaoContractAddr, big1, nil) + if !emptyTxs { + // Function to send at least one transaction each block produced + // Send the transaction to the prevRandaoContractAddr + t.sendNextTransaction(t.CLMock.NextBlockProducer, prevRandaoContractAddr, big1, nil) + } } // Produce blocks before starting the test @@ -716,144 +792,24 @@ func invalidPayloadTestCaseGen(payloadField string, syncing bool) func(*TestEnv) OnPayloadProducerSelected: txFunc, // Run test after the new payload has been obtained OnGetPayload: func() { - // Alter the payload while maintaining a valid hash and send it to the client, should produce an error - basePayload := t.CLMock.LatestPayloadBuilt - var customPayloadMod *CustomPayloadData - payloadFieldSplit := strings.Split(payloadField, "/") - switch payloadFieldSplit[0] { - case "ParentHash": - modParentHash := basePayload.ParentHash - modParentHash[common.HashLength-1] = byte(255 - modParentHash[common.HashLength-1]) - customPayloadMod = &CustomPayloadData{ - ParentHash: &modParentHash, - } - case "StateRoot": - modStateRoot := basePayload.StateRoot - modStateRoot[common.HashLength-1] = byte(255 - modStateRoot[common.HashLength-1]) - customPayloadMod = &CustomPayloadData{ - StateRoot: &modStateRoot, - } - case "ReceiptsRoot": - modReceiptsRoot := basePayload.ReceiptsRoot - modReceiptsRoot[common.HashLength-1] = byte(255 - modReceiptsRoot[common.HashLength-1]) - customPayloadMod = &CustomPayloadData{ - ReceiptsRoot: &modReceiptsRoot, - } - case "Number": - modNumber := basePayload.Number - 1 - customPayloadMod = &CustomPayloadData{ - Number: &modNumber, - } - case "GasLimit": - modGasLimit := basePayload.GasLimit * 2 - customPayloadMod = &CustomPayloadData{ - GasLimit: &modGasLimit, - } - case "GasUsed": - modGasUsed := basePayload.GasUsed - 1 - customPayloadMod = &CustomPayloadData{ - GasUsed: &modGasUsed, - } - case "Timestamp": - modTimestamp := basePayload.Timestamp - 1 - customPayloadMod = &CustomPayloadData{ - Timestamp: &modTimestamp, - } - case "PrevRandao": - // This should fail since we are inserting a transaction that uses the PREVRANDAO opcode. - // The expected outcome will change if we modify the payload. - if len(basePayload.Transactions) == 0 { - // But if the payload has no transactions, the test is invalid - t.Fatalf("FAIL (%s): No transactions in the base payload", t.TestName) - } - modPrevRandao := common.Hash{} - rand.Read(modPrevRandao[:]) - customPayloadMod = &CustomPayloadData{ - PrevRandao: &modPrevRandao, - } - case "RemoveTransaction": - emptyTxs := make([][]byte, 0) - customPayloadMod = &CustomPayloadData{ - Transactions: &emptyTxs, - } - case "Transaction": - if len(payloadFieldSplit) < 2 { - t.Fatalf("FAIL (%s): No transaction field specified: %s", t.TestName, payloadField) - } - if len(basePayload.Transactions) == 0 { - t.Fatalf("FAIL (%s): No transactions available for modification", t.TestName) - } - var baseTx types.Transaction - if err := baseTx.UnmarshalBinary(basePayload.Transactions[0]); err != nil { - t.Fatalf("FAIL (%s): Unable to unmarshal binary tx: %v", t.TestName, err) - } - var customTxData CustomTransactionData - switch payloadFieldSplit[1] { - case "Signature": - modifiedSignature := SignatureValuesFromRaw(baseTx.RawSignatureValues()) - modifiedSignature.R = modifiedSignature.R.Sub(modifiedSignature.R, big1) - customTxData = CustomTransactionData{ - Signature: &modifiedSignature, - } - case "Nonce": - customNonce := baseTx.Nonce() - 1 - customTxData = CustomTransactionData{ - Nonce: &customNonce, - } - case "Gas": - customGas := uint64(0) - customTxData = CustomTransactionData{ - Gas: &customGas, - } - case "GasPrice": - customTxData = CustomTransactionData{ - GasPrice: big0, - } - case "Value": - // Vault account initially has 0x123450000000000000000, so this value should overflow - customValue, err := hexutil.DecodeBig("0x123450000000000000001") - if err != nil { - t.Fatalf("FAIL (%s): Unable to prepare custom tx value: %v", t.TestName, err) - } - customTxData = CustomTransactionData{ - Value: customValue, - } - } - - modifiedTx, err := customizeTransaction(&baseTx, vaultKey, &customTxData) - if err != nil { - t.Fatalf("FAIL (%s): Unable to modify tx: %v", t.TestName, err) - } - t.Logf("INFO (%s): Modified tx %v / original tx %v", t.TestName, baseTx, modifiedTx) - modifiedTxBytes, err := modifiedTx.MarshalBinary() - if err != nil { - } - modifiedTransactions := [][]byte{ - modifiedTxBytes, - } - customPayloadMod = &CustomPayloadData{ - Transactions: &modifiedTransactions, - } - } - - if customPayloadMod == nil { - t.Fatalf("FAIL (%s): Invalid test case: %s", t.TestName, payloadField) + // We need at least one transaction for most test cases to work + if !emptyTxs && len(t.CLMock.LatestPayloadBuilt.Transactions) == 0 { + // But if the payload has no transactions, the test is invalid + t.Fatalf("FAIL (%s): No transactions in the base payload", t.TestName) } - t.Logf("INFO (%v) customizing payload using: %s\n", t.TestName, customPayloadMod.String()) - alteredPayload, err := customizePayload(&t.CLMock.LatestPayloadBuilt, customPayloadMod) - t.Logf("INFO (%v) latest real getPayload (not executed): hash=%v contents=%v\n", t.TestName, t.CLMock.LatestPayloadBuilt.BlockHash, t.CLMock.LatestPayloadBuilt) - t.Logf("INFO (%v) customized payload: hash=%v contents=%v\n", t.TestName, alteredPayload.BlockHash, alteredPayload) + alteredPayload, err := generateInvalidPayload(&t.CLMock.LatestPayloadBuilt, payloadField) if err != nil { - t.Fatalf("FAIL (%s): Unable to modify payload (%v): %v", t.TestName, customPayloadMod, err) + t.Fatalf("FAIL (%s): Unable to modify payload (%v): %v", t.TestName, payloadField, err) } + invalidPayloadHash = alteredPayload.BlockHash // Depending on the field we modified, we expect a different status r := t.TestEngine.TestEngineNewPayloadV1(alteredPayload) - if syncing || payloadField == "ParentHash" { + if syncing || payloadField == InvalidParentHash { // Execution specification:: // {status: ACCEPTED, latestValidHash: null, validationError: null} if the following conditions are met: // - the blockHash of the payload is valid @@ -911,7 +867,7 @@ func invalidPayloadTestCaseGen(payloadField string, syncing bool) func(*TestEnv) */ q := t.TestEngine.TestEngineNewPayloadV1(alteredPayload) - if payloadField == "ParentHash" { + if payloadField == InvalidParentHash { // There is no invalid parentHash, if this value is incorrect, // it is assumed that the block is missing and we need to sync. // ACCEPTED also valid since the CLs normally use these interchangeably @@ -923,7 +879,7 @@ func invalidPayloadTestCaseGen(payloadField string, syncing bool) func(*TestEnv) // Try sending the fcU again, this time we should get the proper invalid response. // At this moment the response should be INVALID - if payloadField != "ParentHash" { + if payloadField != InvalidParentHash { s := t.TestEngine.TestEngineForkchoiceUpdatedV1(&fcState, nil) // Note: SYNCING is acceptable here as long as the block produced after this test is produced successfully s.ExpectAnyPayloadStatus(Syncing, Invalid) @@ -976,8 +932,25 @@ func invalidPayloadTestCaseGen(payloadField string, syncing bool) func(*TestEnv) // Then reveal the invalid payload and expect that the client rejects it and rejects forkchoice updated calls to this chain. // The invalid_index parameter determines how many payloads apart is the common ancestor from the block that invalidates the chain, // with a value of 1 meaning that the immediate payload after the common ancestor will be invalid. -func reOrgToChainMissingInvalidParentGen(invalid_index int) func(*TestEnv) { +func invalidMissingAncestorReOrgGen(invalid_index int, payloadField InvalidPayloadField, p2psync bool, emptyTxs bool) func(*TestEnv) { return func(t *TestEnv) { + var secondaryEngineTest *TestEngineClient + if p2psync { + // To allow having the invalid payload delivered via P2P, we need a second client to serve the payload + enode, err := t.Engine.EnodeURL() + if err != nil { + t.Fatalf("FAIL (%s): Unable to obtain bootnode: %v", t.TestName, err) + } + newParams := t.ClientParams.Set("HIVE_BOOTNODE", fmt.Sprintf("%s", enode)) + newParams = newParams.Set("HIVE_MINER", "") + secondaryClient, secondaryEngine, err := t.StartClient(t.Client.Type, newParams, t.MainTTD()) + if err != nil { + t.Fatalf("FAIL (%s): Unable to spawn a secondary client: %v", t.TestName, err) + } + t.CLMock.AddEngineClient(t.T, secondaryClient, t.MainTTD()) + secondaryEngineTest = NewTestEngineClient(t, secondaryEngine) + } + // Wait until TTD is reached by this client t.CLMock.waitForTTD() @@ -996,73 +969,154 @@ func reOrgToChainMissingInvalidParentGen(invalid_index int) func(*TestEnv) { // Append the common ancestor altChainPayloads = append(altChainPayloads, &cA) - // Produce blocks but at the same time create an alternate chain which contains an invalid parent (INV_P1) - // CommonAncestor◄─▲── P1 ◄──── P2 ◄─ P3 ◄─ ... ◄─ Pn + // Produce blocks but at the same time create an alternate chain which contains an invalid payload at some point (INV_P) + // CommonAncestor◄─▲── P1 ◄─ P2 ◄─ P3 ◄─ ... ◄─ Pn // │ - // └──INV_P1 ◄─ P2' ◄─ P3' ◄─ ... ◄─ Pn' + // └── P1' ◄─ P2' ◄─ ... ◄─ INV_P ◄─ ... ◄─ Pn' t.CLMock.produceBlocks(n, BlockProcessCallbacks{ + + OnPayloadProducerSelected: func() { + // Function to send at least one transaction each block produced. + // Empty Txs Payload with invalid stateRoot discovered an issue in geth sync, hence this is customizable. + if !emptyTxs { + // Send the transaction to the prevRandaoContractAddr + t.sendNextTransaction(t.CLMock.NextBlockProducer, prevRandaoContractAddr, big1, nil) + } + }, OnGetPayload: func() { var ( alternatePayload *ExecutableDataV1 err error ) - // Create another prevRandao to ensure we deviate from the main payload - prevRandao := common.Hash{} - rand.Read(prevRandao[:]) - if len(altChainPayloads) == invalid_index { - // This is the payload we are looking to invalidate, this must be invalid - invStateRoot := common.Hash{} - rand.Read(invStateRoot[:]) - alternatePayload, err = customizePayload(&t.CLMock.LatestPayloadBuilt, &CustomPayloadData{ - StateRoot: &invStateRoot, - ParentHash: &altChainPayloads[len(altChainPayloads)-1].BlockHash, - PrevRandao: &prevRandao, - }) - } else { - // Modify the created payload only to reference the previous parent in the B chain - alternatePayload, err = customizePayload(&t.CLMock.LatestPayloadBuilt, &CustomPayloadData{ - ParentHash: &altChainPayloads[len(altChainPayloads)-1].BlockHash, - PrevRandao: &prevRandao, - }) - } + // Insert extraData to ensure we deviate from the main payload, which contains empty extradata + alternatePayload, err = customizePayload(&t.CLMock.LatestPayloadBuilt, &CustomPayloadData{ + ParentHash: &altChainPayloads[len(altChainPayloads)-1].BlockHash, + ExtraData: &([]byte{0x01}), + }) if err != nil { t.Fatalf("FAIL (%s): Unable to customize payload: %v", t.TestName, err) } + if len(altChainPayloads) == invalid_index { + alternatePayload, err = generateInvalidPayload(alternatePayload, payloadField) + if err != nil { + t.Fatalf("FAIL (%s): Unable to customize payload: %v", t.TestName, err) + } + } altChainPayloads = append(altChainPayloads, alternatePayload) }, }) - // Now let's send the alternate chain to the client using newPayload in reverse order - for i := n - 1; i >= 0; i-- { + // Now let's send the alternate chain to the client using newPayload/sync + for i := 1; i <= n; i++ { // Send the payload - r := t.TestEngine.TestEngineNewPayloadV1(altChainPayloads[i]) - if i == 1 { + payloadValidStr := "VALID" + if i == invalid_index { + payloadValidStr = "INVALID" + } else if i > invalid_index { + payloadValidStr = "VALID with INVALID ancestor" + } + t.Logf("INFO (%s): Invalid chain payload %d (%s): %v", t.TestName, i, payloadValidStr, altChainPayloads[i].BlockHash) + + if p2psync { + // We are syncing the main client via p2p, therefore we need to send all valid payloads to the secondary + // client, and since they are valid, the client will send them via p2p without problems. + if i < invalid_index { + // Payloads before the invalid payload are sent to the secondary client + r := secondaryEngineTest.TestEngineNewPayloadV1(altChainPayloads[i]) + r.ExpectStatus(Valid) + s := secondaryEngineTest.TestEngineForkchoiceUpdatedV1(&ForkchoiceStateV1{ + HeadBlockHash: altChainPayloads[i].BlockHash, + SafeBlockHash: cA.BlockHash, + FinalizedBlockHash: cA.BlockHash, + }, nil) + s.ExpectPayloadStatus(Valid) + /* + p := NewTestEthClient(t, secondaryEngineTest.Engine.Eth).TestBlockByNumber(nil) + p.ExpectHash(altChainPayloads[invalid_index-1].BlockHash) + */ + + } else { + // Payloads on and after the invalid payload are sent to the main client, + // which at first won't be fully verified because the client has to sync with the secondary client + // to obtain all the information + r := t.TestEngine.TestEngineNewPayloadV1(altChainPayloads[i]) + t.Logf("INFO (%s): Response from main client: %v", t.TestName, r.Status) + s := t.TestEngine.TestEngineForkchoiceUpdatedV1(&ForkchoiceStateV1{ + HeadBlockHash: altChainPayloads[i].BlockHash, + SafeBlockHash: altChainPayloads[i].BlockHash, + FinalizedBlockHash: altChainPayloads[i].BlockHash, + }, nil) + t.Logf("INFO (%s): Response from main client fcu: %v", t.TestName, s.Response.PayloadStatus) + } + } else { + r := t.TestEngine.TestEngineNewPayloadV1(altChainPayloads[i]) + p := t.TestEngine.TestEngineForkchoiceUpdatedV1(&ForkchoiceStateV1{ + HeadBlockHash: altChainPayloads[i].BlockHash, + SafeBlockHash: altChainPayloads[i].BlockHash, + FinalizedBlockHash: altChainPayloads[i].BlockHash, + }, nil) if i == invalid_index { // If this is the first payload after the common ancestor, and this is the payload we invalidated, // then we have all the information to determine that this payload is invalid. r.ExpectStatus(Invalid) - r.ExpectLatestValidHash(&cA.BlockHash) + r.ExpectLatestValidHash(&altChainPayloads[i-1].BlockHash) + } else if i > invalid_index { + // We have already sent the invalid payload, but the client could've discarded it. + // In reality the CL will not get to this point because it will have already received the `INVALID` + // response from the previous payload. + r.ExpectStatusEither(Accepted, Syncing) + r.ExpectLatestValidHash(nil) } else { - // This is the first payload, but we did not invalidated the chain until after this one, - // therefore the expected status is valid + // This is one of the payloads before the invalid one, therefore is valid. r.ExpectStatus(Valid) + p.ExpectPayloadStatus(Valid) + p.ExpectLatestValidHash(&altChainPayloads[i].BlockHash) } - } else if i > 1 { - r.ExpectStatus(Accepted) - r.ExpectLatestValidHash(nil) } + } - if i == n-1 { - // Send the fcU, this is before we send all the payloads from the alternative chain. - // At this point is impossible to give a latestValidHash, since we don't know the required information. + if p2psync { + // If we are syncing through p2p, we need to keep polling until the client syncs the missing payloads + for { + r := t.TestEngine.TestEngineNewPayloadV1(altChainPayloads[n]) + t.Logf("INFO (%s): Response from main client: %v", t.TestName, r.Status) s := t.TestEngine.TestEngineForkchoiceUpdatedV1(&ForkchoiceStateV1{ - HeadBlockHash: altChainPayloads[i].BlockHash, - SafeBlockHash: cA.BlockHash, - FinalizedBlockHash: cA.BlockHash, + HeadBlockHash: altChainPayloads[n].BlockHash, + SafeBlockHash: altChainPayloads[n].BlockHash, + FinalizedBlockHash: altChainPayloads[n].BlockHash, }, nil) - s.ExpectPayloadStatus(Syncing) - s.ExpectLatestValidHash(nil) + t.Logf("INFO (%s): Response from main client fcu: %v", t.TestName, s.Response.PayloadStatus) + + if r.Status.Status == Invalid { + // We also expect that the client properly returns the LatestValidHash of the block on the + // alternate chain that is immediately prior to the invalid payload + r.ExpectLatestValidHash(&altChainPayloads[invalid_index-1].BlockHash) + break + } else if r.Status.Status == Valid { + latestBlock, err := t.Eth.BlockByNumber(t.Ctx(), nil) + if err != nil { + t.Fatalf("FAIL (%s): Unable to get latest block: %v", t.TestName, err) + } + + // Print last 10 blocks, for debugging + for k := latestBlock.Number().Int64() - 10; k <= latestBlock.Number().Int64(); k++ { + latestBlock, err := t.Eth.BlockByNumber(t.Ctx(), big.NewInt(k)) + if err != nil { + t.Fatalf("FAIL (%s): Unable to get block %d: %v", t.TestName, k, err) + } + js, _ := json.MarshalIndent(latestBlock.Header(), "", " ") + t.Logf("INFO (%s): Block %d: %s", t.TestName, k, js) + } + + t.Fatalf("FAIL (%s): Client returned VALID on an invalid chain: %v", t.TestName, r.Status) + } + + select { + case <-time.After(time.Second): + case <-t.Timeout: + t.Fatalf("FAIL (%s): Timeout waiting for main client to sync to secondary client", t.TestName) + } } } diff --git a/simulators/ethereum/engine/helper.go b/simulators/ethereum/engine/helper.go index 97b4271b63..2ae20e0414 100644 --- a/simulators/ethereum/engine/helper.go +++ b/simulators/ethereum/engine/helper.go @@ -9,6 +9,7 @@ import ( "fmt" "io/ioutil" "math/big" + "math/rand" "net/http" "strings" "time" @@ -314,6 +315,156 @@ func (customData *CustomPayloadData) String() string { return strings.Join(customFieldsList, ", ") } +type InvalidPayloadField string + +const ( + InvalidParentHash InvalidPayloadField = "ParentHash" + InvalidStateRoot = "StateRoot" + InvalidReceiptsRoot = "ReceiptsRoot" + InvalidNumber = "Number" + InvalidGasLimit = "GasLimit" + InvalidGasUsed = "GasUsed" + InvalidTimestamp = "Timestamp" + InvalidPrevRandao = "PrevRandao" + RemoveTransaction = "Incomplete Transactions" + InvalidTransactionSignature = "Transaction Signature" + InvalidTransactionNonce = "Transaction Nonce" + InvalidTransactionGas = "Transaction Gas" + InvalidTransactionGasPrice = "Transaction GasPrice" + InvalidTransactionValue = "Transaction Value" +) + +// This function generates an invalid payload by taking a base payload and modifying the specified field such that it ends up being invalid. +// One small consideration is that the payload needs to contain transactions and specially transactions using the PREVRANDAO opcode for all the fields to be compatible with this function. +func generateInvalidPayload(basePayload *ExecutableDataV1, payloadField InvalidPayloadField) (*ExecutableDataV1, error) { + + var customPayloadMod *CustomPayloadData + switch payloadField { + case InvalidParentHash: + modParentHash := basePayload.ParentHash + modParentHash[common.HashLength-1] = byte(255 - modParentHash[common.HashLength-1]) + customPayloadMod = &CustomPayloadData{ + ParentHash: &modParentHash, + } + case InvalidStateRoot: + modStateRoot := basePayload.StateRoot + modStateRoot[common.HashLength-1] = byte(255 - modStateRoot[common.HashLength-1]) + customPayloadMod = &CustomPayloadData{ + StateRoot: &modStateRoot, + } + case InvalidReceiptsRoot: + modReceiptsRoot := basePayload.ReceiptsRoot + modReceiptsRoot[common.HashLength-1] = byte(255 - modReceiptsRoot[common.HashLength-1]) + customPayloadMod = &CustomPayloadData{ + ReceiptsRoot: &modReceiptsRoot, + } + case InvalidNumber: + modNumber := basePayload.Number - 1 + customPayloadMod = &CustomPayloadData{ + Number: &modNumber, + } + case InvalidGasLimit: + modGasLimit := basePayload.GasLimit * 2 + customPayloadMod = &CustomPayloadData{ + GasLimit: &modGasLimit, + } + case InvalidGasUsed: + modGasUsed := basePayload.GasUsed - 1 + customPayloadMod = &CustomPayloadData{ + GasUsed: &modGasUsed, + } + case InvalidTimestamp: + modTimestamp := basePayload.Timestamp - 1 + customPayloadMod = &CustomPayloadData{ + Timestamp: &modTimestamp, + } + case InvalidPrevRandao: + // This option potentially requires a transaction that uses the PREVRANDAO opcode. + // Otherwise the payload will still be valid. + modPrevRandao := common.Hash{} + rand.Read(modPrevRandao[:]) + customPayloadMod = &CustomPayloadData{ + PrevRandao: &modPrevRandao, + } + case RemoveTransaction: + emptyTxs := make([][]byte, 0) + customPayloadMod = &CustomPayloadData{ + Transactions: &emptyTxs, + } + case InvalidTransactionSignature, + InvalidTransactionNonce, + InvalidTransactionGas, + InvalidTransactionGasPrice, + InvalidTransactionValue: + + if len(basePayload.Transactions) == 0 { + return nil, fmt.Errorf("No transactions available for modification") + } + var baseTx types.Transaction + if err := baseTx.UnmarshalBinary(basePayload.Transactions[0]); err != nil { + return nil, err + } + var customTxData CustomTransactionData + switch payloadField { + case InvalidTransactionSignature: + modifiedSignature := SignatureValuesFromRaw(baseTx.RawSignatureValues()) + modifiedSignature.R = modifiedSignature.R.Sub(modifiedSignature.R, big1) + customTxData = CustomTransactionData{ + Signature: &modifiedSignature, + } + case InvalidTransactionNonce: + customNonce := baseTx.Nonce() - 1 + customTxData = CustomTransactionData{ + Nonce: &customNonce, + } + case InvalidTransactionGas: + customGas := uint64(0) + customTxData = CustomTransactionData{ + Gas: &customGas, + } + case InvalidTransactionGasPrice: + customTxData = CustomTransactionData{ + GasPrice: big0, + } + case InvalidTransactionValue: + // Vault account initially has 0x123450000000000000000, so this value should overflow + customValue, err := hexutil.DecodeBig("0x123450000000000000001") + if err != nil { + return nil, err + } + customTxData = CustomTransactionData{ + Value: customValue, + } + } + + modifiedTx, err := customizeTransaction(&baseTx, vaultKey, &customTxData) + if err != nil { + return nil, err + } + + modifiedTxBytes, err := modifiedTx.MarshalBinary() + if err != nil { + } + modifiedTransactions := [][]byte{ + modifiedTxBytes, + } + customPayloadMod = &CustomPayloadData{ + Transactions: &modifiedTransactions, + } + } + + if customPayloadMod == nil { + return nil, fmt.Errorf("Invalid payload field to corrupt: %s", payloadField) + } + + alteredPayload, err := customizePayload(basePayload, customPayloadMod) + if err != nil { + return nil, err + } + + return alteredPayload, nil +} + // Use client specific rpc methods to debug a transaction that includes the PREVRANDAO opcode func debugPrevRandaoTransaction(ctx context.Context, c *rpc.Client, clientType string, tx *types.Transaction, expectedPrevRandao *common.Hash) error { switch clientType { From 91293a5c3f54a3d57a4702a94d11f7bba3e8f2dc Mon Sep 17 00:00:00 2001 From: marioevz Date: Fri, 6 May 2022 16:54:22 +0000 Subject: [PATCH 4/7] simulators/ethereum/engine: add readme info --- simulators/ethereum/engine/README.md | 74 +++++++++++++++++----------- 1 file changed, 45 insertions(+), 29 deletions(-) diff --git a/simulators/ethereum/engine/README.md b/simulators/ethereum/engine/README.md index aa7c48dc04..b8ebd62e4e 100644 --- a/simulators/ethereum/engine/README.md +++ b/simulators/ethereum/engine/README.md @@ -71,6 +71,38 @@ Expected outcome is that the forkchoiceUpdate proceeds, but the call returns an - Pre-TTD Block Hash: Perform a forkchoiceUpdated call using a block hash part of the canonical chain that precedes the block where the TTD occurred. (Behavior is undefined for this edge case and not verified, but should not produce unrecoverable error) +### Eth RPC Status on ForkchoiceUpdated Events: +- Latest Block after NewPayload: +Verify the Block returned by the Eth RPC after a new payload is executed. Eth RPC should still return previous block. + +- Latest Block after New HeadBlock: +Verify the Block returned by the Eth RPC after a new HeadBlockHash is set using forkchoiceUpdated. Eth RPC should return new block. + +- Latest Block after New SafeBlock: +Verify the Block returned by the Eth RPC after a new SafeBlockHash is set using forkchoiceUpdated. Eth RPC should return new block. + +- Latest Block after New FinalizedBlock: +Verify the Block returned by the Eth RPC after a new FinalizedBlockHash is set using forkchoiceUpdated. Eth RPC should return new block. + +- Latest Block after Reorg: +Verify the Block returned by the Eth RPC after a forkchoiceUpdated reorgs HeadBlockHash/SafeBlockHash to a sidechain and back. Eth RPC should return the appropriate block everytime. + +### Payload Execution +- Re-Execute Payload: +Re-execute already executed payloads (10) and verify that no errors occur. + +- Multiple New Payloads Extending Canonical Chain: +Send 80 valid NewPayload directives extending the canonical chain. Only one of the payloads is selected using ForkchoiceUpdated directive. + +- Out of Order Payload Execution: +Launch a first client and produce N payloads. +Launch a second client and send payloads (NewPayload) in reverse order (N, N - 1, ..., 1). +The payloads should be ACCEPTED/SYNCING, and the last payload should be VALID (since payload 1 successfully links the chain with the Genesis). + +- Valid NewPayload->ForkchoiceUpdated on Syncing Client: +Skip sending NewPayload to the client, but send the ForkchoiceUpdated to this missing payload, which will send the client to Syncing, then send the valid payload. Response should be either `ACCEPTED` or `SYNCING`. + +### Invalid Payload Tests - Bad blockhash on NewPayload: Send a NewPayload directive to the client including an incorrect BlockHash, should result in an error in all the following cases: - NewPayload while not syncing, on canonical chain @@ -103,36 +135,20 @@ Modify fields including: - Gas - Value -### Eth RPC Status on ForkchoiceUpdated Events: -- Latest Block after NewPayload: -Verify the Block returned by the Eth RPC after a new payload is executed. Eth RPC should still return previous block. - -- Latest Block after New HeadBlock: -Verify the Block returned by the Eth RPC after a new HeadBlockHash is set using forkchoiceUpdated. Eth RPC should return new block. - -- Latest Block after New SafeBlock: -Verify the Block returned by the Eth RPC after a new SafeBlockHash is set using forkchoiceUpdated. Eth RPC should return new block. - -- Latest Block after New FinalizedBlock: -Verify the Block returned by the Eth RPC after a new FinalizedBlockHash is set using forkchoiceUpdated. Eth RPC should return new block. - -- Latest Block after Reorg: -Verify the Block returned by the Eth RPC after a forkchoiceUpdated reorgs HeadBlockHash/SafeBlockHash to a sidechain and back. Eth RPC should return the appropriate block everytime. - -### Payload Execution -- Re-Execute Payload: -Re-execute already executed payloads (10) and verify that no errors occur. - -- Multiple New Payloads Extending Canonical Chain: -Send 80 valid NewPayload directives extending the canonical chain. Only one of the payloads is selected using ForkchoiceUpdated directive. +- Invalid Ancestor Re-Org Tests +Attempt to re-org to an unknown side chain which at some point contains an invalid payload. +The side chain is constructed in parallel while the CL Mock builds the canonical chain, but changing the extraData to simply produce a different hash. +At a given point, the side chain invalidates one of the payloads by modifying one of the payload fields. +Once the side chain reaches a certain deviation height (N) from the commonAncestor, the CL switches to it by either of the following methods: +a) `newPayload` each of the payloads in the side chain, and the invalid payload shall return `INVALID` +b) Force the main client to partially sync the side chain by sending the initial slice of the chain (before the invalid payload) to another client, and the rest to the primary client. +Method (b) results in the client returning `ACCEPTED` or `SYNCING` on the `newPayload(INV_P)`, but eventually the client must return `INVALID` to the head of the side chain because it was built on top of an invalid payload. +``` +commonAncestor◄─▲── P1 ◄─ P2 ◄─ P3 ◄─ ... ◄─ Pn + │ + └── P1' ◄─ P2' ◄─ ... ◄─ INV_P ◄─ ... ◄─ Pn' +``` -- Out of Order Payload Execution: -Launch a first client and produce N payloads. -Launch a second client and send payloads (NewPayload) in reverse order (N, N - 1, ..., 1). -The payloads should be ACCEPTED/SYNCING, and the last payload should be VALID (since payload 1 successfully links the chain with the Genesis). - -- Valid NewPayload->ForkchoiceUpdated on Syncing Client: -Skip sending NewPayload to the client, but send the ForkchoiceUpdated to this missing payload, which will send the client to Syncing, then send the valid payload. Response should be either `ACCEPTED` or `SYNCING`. ### Re-org using Engine API - Transaction Reorg using ForkchoiceUpdated: From 8cd9dfb389a764a0efe76958298065deaa2d825e Mon Sep 17 00:00:00 2001 From: marioevz Date: Fri, 6 May 2022 18:00:06 +0000 Subject: [PATCH 5/7] simulators/ethereum/engine: minor change to invalid ancestor test --- simulators/ethereum/engine/enginetests.go | 223 +++++++++++----------- 1 file changed, 115 insertions(+), 108 deletions(-) diff --git a/simulators/ethereum/engine/enginetests.go b/simulators/ethereum/engine/enginetests.go index 94a61f62b2..987cb6f161 100644 --- a/simulators/ethereum/engine/enginetests.go +++ b/simulators/ethereum/engine/enginetests.go @@ -1005,124 +1005,131 @@ func invalidMissingAncestorReOrgGen(invalid_index int, payloadField InvalidPaylo altChainPayloads = append(altChainPayloads, alternatePayload) }, }) + t.CLMock.produceSingleBlock(BlockProcessCallbacks{ + // Note: We perform the test in the middle of payload creation by the CL Mock, in order to be able to + // re-org back into this chain and use the new payload without issues. + OnGetPayload: func() { - // Now let's send the alternate chain to the client using newPayload/sync - for i := 1; i <= n; i++ { - // Send the payload - payloadValidStr := "VALID" - if i == invalid_index { - payloadValidStr = "INVALID" - } else if i > invalid_index { - payloadValidStr = "VALID with INVALID ancestor" - } - t.Logf("INFO (%s): Invalid chain payload %d (%s): %v", t.TestName, i, payloadValidStr, altChainPayloads[i].BlockHash) - - if p2psync { - // We are syncing the main client via p2p, therefore we need to send all valid payloads to the secondary - // client, and since they are valid, the client will send them via p2p without problems. - if i < invalid_index { - // Payloads before the invalid payload are sent to the secondary client - r := secondaryEngineTest.TestEngineNewPayloadV1(altChainPayloads[i]) - r.ExpectStatus(Valid) - s := secondaryEngineTest.TestEngineForkchoiceUpdatedV1(&ForkchoiceStateV1{ - HeadBlockHash: altChainPayloads[i].BlockHash, - SafeBlockHash: cA.BlockHash, - FinalizedBlockHash: cA.BlockHash, - }, nil) - s.ExpectPayloadStatus(Valid) - /* - p := NewTestEthClient(t, secondaryEngineTest.Engine.Eth).TestBlockByNumber(nil) - p.ExpectHash(altChainPayloads[invalid_index-1].BlockHash) - */ + // Now let's send the alternate chain to the client using newPayload/sync + for i := 1; i <= n; i++ { + // Send the payload + payloadValidStr := "VALID" + if i == invalid_index { + payloadValidStr = "INVALID" + } else if i > invalid_index { + payloadValidStr = "VALID with INVALID ancestor" + } + t.Logf("INFO (%s): Invalid chain payload %d (%s): %v", t.TestName, i, payloadValidStr, altChainPayloads[i].BlockHash) + + if p2psync { + // We are syncing the main client via p2p, therefore we need to send all valid payloads to the secondary + // client, and since they are valid, the client will send them via p2p without problems. + if i < invalid_index { + // Payloads before the invalid payload are sent to the secondary client + r := secondaryEngineTest.TestEngineNewPayloadV1(altChainPayloads[i]) + r.ExpectStatus(Valid) + s := secondaryEngineTest.TestEngineForkchoiceUpdatedV1(&ForkchoiceStateV1{ + HeadBlockHash: altChainPayloads[i].BlockHash, + SafeBlockHash: cA.BlockHash, + FinalizedBlockHash: cA.BlockHash, + }, nil) + s.ExpectPayloadStatus(Valid) + /* + p := NewTestEthClient(t, secondaryEngineTest.Engine.Eth).TestBlockByNumber(nil) + p.ExpectHash(altChainPayloads[invalid_index-1].BlockHash) + */ + + } else { + // Payloads on and after the invalid payload are sent to the main client, + // which at first won't be fully verified because the client has to sync with the secondary client + // to obtain all the information + r := t.TestEngine.TestEngineNewPayloadV1(altChainPayloads[i]) + t.Logf("INFO (%s): Response from main client: %v", t.TestName, r.Status) + s := t.TestEngine.TestEngineForkchoiceUpdatedV1(&ForkchoiceStateV1{ + HeadBlockHash: altChainPayloads[i].BlockHash, + SafeBlockHash: altChainPayloads[i].BlockHash, + FinalizedBlockHash: altChainPayloads[i].BlockHash, + }, nil) + t.Logf("INFO (%s): Response from main client fcu: %v", t.TestName, s.Response.PayloadStatus) + } + } else { + r := t.TestEngine.TestEngineNewPayloadV1(altChainPayloads[i]) + p := t.TestEngine.TestEngineForkchoiceUpdatedV1(&ForkchoiceStateV1{ + HeadBlockHash: altChainPayloads[i].BlockHash, + SafeBlockHash: altChainPayloads[i].BlockHash, + FinalizedBlockHash: altChainPayloads[i].BlockHash, + }, nil) + if i == invalid_index { + // If this is the first payload after the common ancestor, and this is the payload we invalidated, + // then we have all the information to determine that this payload is invalid. + r.ExpectStatus(Invalid) + r.ExpectLatestValidHash(&altChainPayloads[i-1].BlockHash) + } else if i > invalid_index { + // We have already sent the invalid payload, but the client could've discarded it. + // In reality the CL will not get to this point because it will have already received the `INVALID` + // response from the previous payload. + r.ExpectStatusEither(Accepted, Syncing) + r.ExpectLatestValidHash(nil) + } else { + // This is one of the payloads before the invalid one, therefore is valid. + r.ExpectStatus(Valid) + p.ExpectPayloadStatus(Valid) + p.ExpectLatestValidHash(&altChainPayloads[i].BlockHash) + } - } else { - // Payloads on and after the invalid payload are sent to the main client, - // which at first won't be fully verified because the client has to sync with the secondary client - // to obtain all the information - r := t.TestEngine.TestEngineNewPayloadV1(altChainPayloads[i]) - t.Logf("INFO (%s): Response from main client: %v", t.TestName, r.Status) - s := t.TestEngine.TestEngineForkchoiceUpdatedV1(&ForkchoiceStateV1{ - HeadBlockHash: altChainPayloads[i].BlockHash, - SafeBlockHash: altChainPayloads[i].BlockHash, - FinalizedBlockHash: altChainPayloads[i].BlockHash, - }, nil) - t.Logf("INFO (%s): Response from main client fcu: %v", t.TestName, s.Response.PayloadStatus) - } - } else { - r := t.TestEngine.TestEngineNewPayloadV1(altChainPayloads[i]) - p := t.TestEngine.TestEngineForkchoiceUpdatedV1(&ForkchoiceStateV1{ - HeadBlockHash: altChainPayloads[i].BlockHash, - SafeBlockHash: altChainPayloads[i].BlockHash, - FinalizedBlockHash: altChainPayloads[i].BlockHash, - }, nil) - if i == invalid_index { - // If this is the first payload after the common ancestor, and this is the payload we invalidated, - // then we have all the information to determine that this payload is invalid. - r.ExpectStatus(Invalid) - r.ExpectLatestValidHash(&altChainPayloads[i-1].BlockHash) - } else if i > invalid_index { - // We have already sent the invalid payload, but the client could've discarded it. - // In reality the CL will not get to this point because it will have already received the `INVALID` - // response from the previous payload. - r.ExpectStatusEither(Accepted, Syncing) - r.ExpectLatestValidHash(nil) - } else { - // This is one of the payloads before the invalid one, therefore is valid. - r.ExpectStatus(Valid) - p.ExpectPayloadStatus(Valid) - p.ExpectLatestValidHash(&altChainPayloads[i].BlockHash) + } } - } - } - - if p2psync { - // If we are syncing through p2p, we need to keep polling until the client syncs the missing payloads - for { - r := t.TestEngine.TestEngineNewPayloadV1(altChainPayloads[n]) - t.Logf("INFO (%s): Response from main client: %v", t.TestName, r.Status) - s := t.TestEngine.TestEngineForkchoiceUpdatedV1(&ForkchoiceStateV1{ - HeadBlockHash: altChainPayloads[n].BlockHash, - SafeBlockHash: altChainPayloads[n].BlockHash, - FinalizedBlockHash: altChainPayloads[n].BlockHash, - }, nil) - t.Logf("INFO (%s): Response from main client fcu: %v", t.TestName, s.Response.PayloadStatus) - - if r.Status.Status == Invalid { - // We also expect that the client properly returns the LatestValidHash of the block on the - // alternate chain that is immediately prior to the invalid payload - r.ExpectLatestValidHash(&altChainPayloads[invalid_index-1].BlockHash) - break - } else if r.Status.Status == Valid { - latestBlock, err := t.Eth.BlockByNumber(t.Ctx(), nil) - if err != nil { - t.Fatalf("FAIL (%s): Unable to get latest block: %v", t.TestName, err) - } + if p2psync { + // If we are syncing through p2p, we need to keep polling until the client syncs the missing payloads + for { + r := t.TestEngine.TestEngineNewPayloadV1(altChainPayloads[n]) + t.Logf("INFO (%s): Response from main client: %v", t.TestName, r.Status) + s := t.TestEngine.TestEngineForkchoiceUpdatedV1(&ForkchoiceStateV1{ + HeadBlockHash: altChainPayloads[n].BlockHash, + SafeBlockHash: altChainPayloads[n].BlockHash, + FinalizedBlockHash: altChainPayloads[n].BlockHash, + }, nil) + t.Logf("INFO (%s): Response from main client fcu: %v", t.TestName, s.Response.PayloadStatus) + + if r.Status.Status == Invalid { + // We also expect that the client properly returns the LatestValidHash of the block on the + // alternate chain that is immediately prior to the invalid payload + r.ExpectLatestValidHash(&altChainPayloads[invalid_index-1].BlockHash) + break + } else if r.Status.Status == Valid { + latestBlock, err := t.Eth.BlockByNumber(t.Ctx(), nil) + if err != nil { + t.Fatalf("FAIL (%s): Unable to get latest block: %v", t.TestName, err) + } + + // Print last 10 blocks, for debugging + for k := latestBlock.Number().Int64() - 10; k <= latestBlock.Number().Int64(); k++ { + latestBlock, err := t.Eth.BlockByNumber(t.Ctx(), big.NewInt(k)) + if err != nil { + t.Fatalf("FAIL (%s): Unable to get block %d: %v", t.TestName, k, err) + } + js, _ := json.MarshalIndent(latestBlock.Header(), "", " ") + t.Logf("INFO (%s): Block %d: %s", t.TestName, k, js) + } + + t.Fatalf("FAIL (%s): Client returned VALID on an invalid chain: %v", t.TestName, r.Status) + } - // Print last 10 blocks, for debugging - for k := latestBlock.Number().Int64() - 10; k <= latestBlock.Number().Int64(); k++ { - latestBlock, err := t.Eth.BlockByNumber(t.Ctx(), big.NewInt(k)) - if err != nil { - t.Fatalf("FAIL (%s): Unable to get block %d: %v", t.TestName, k, err) + select { + case <-time.After(time.Second): + case <-t.Timeout: + t.Fatalf("FAIL (%s): Timeout waiting for main client to sync to secondary client", t.TestName) } - js, _ := json.MarshalIndent(latestBlock.Header(), "", " ") - t.Logf("INFO (%s): Block %d: %s", t.TestName, k, js) } - - t.Fatalf("FAIL (%s): Client returned VALID on an invalid chain: %v", t.TestName, r.Status) } - select { - case <-time.After(time.Second): - case <-t.Timeout: - t.Fatalf("FAIL (%s): Timeout waiting for main client to sync to secondary client", t.TestName) - } - } - } + // Resend the latest correct fcU + r := t.TestEngine.TestEngineForkchoiceUpdatedV1(&t.CLMock.LatestForkchoice, nil) + r.ExpectNoError() + }, + }) - // Resend the latest correct fcU - r := t.TestEngine.TestEngineForkchoiceUpdatedV1(&t.CLMock.LatestForkchoice, nil) - r.ExpectNoError() } } From a10e944a64e0478df3d30c948e860154f72cfdc3 Mon Sep 17 00:00:00 2001 From: marioevz Date: Mon, 9 May 2022 15:56:46 +0000 Subject: [PATCH 6/7] simulators/ethereum/engine: check fcu on invalid ancestor too --- simulators/ethereum/engine/enginetests.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/simulators/ethereum/engine/enginetests.go b/simulators/ethereum/engine/enginetests.go index 987cb6f161..d42f3a3aa4 100644 --- a/simulators/ethereum/engine/enginetests.go +++ b/simulators/ethereum/engine/enginetests.go @@ -1096,6 +1096,9 @@ func invalidMissingAncestorReOrgGen(invalid_index int, payloadField InvalidPaylo // We also expect that the client properly returns the LatestValidHash of the block on the // alternate chain that is immediately prior to the invalid payload r.ExpectLatestValidHash(&altChainPayloads[invalid_index-1].BlockHash) + // Response on ForkchoiceUpdated should be the same + s.ExpectPayloadStatus(Invalid) + s.ExpectLatestValidHash(&altChainPayloads[invalid_index-1].BlockHash) break } else if r.Status.Status == Valid { latestBlock, err := t.Eth.BlockByNumber(t.Ctx(), nil) @@ -1119,7 +1122,7 @@ func invalidMissingAncestorReOrgGen(invalid_index int, payloadField InvalidPaylo select { case <-time.After(time.Second): case <-t.Timeout: - t.Fatalf("FAIL (%s): Timeout waiting for main client to sync to secondary client", t.TestName) + t.Fatalf("FAIL (%s): Timeout waiting for main client to detect invalid chain", t.TestName) } } } @@ -1127,6 +1130,7 @@ func invalidMissingAncestorReOrgGen(invalid_index int, payloadField InvalidPaylo // Resend the latest correct fcU r := t.TestEngine.TestEngineForkchoiceUpdatedV1(&t.CLMock.LatestForkchoice, nil) r.ExpectNoError() + // After this point, the CL Mock will send the next payload of the canonical chain }, }) From eb6605631e9994ea60f700e3ae7918aa184847e0 Mon Sep 17 00:00:00 2001 From: marioevz Date: Tue, 10 May 2022 19:14:48 +0000 Subject: [PATCH 7/7] simulators/ethereum/engine: invalid ancestor tests change --- simulators/ethereum/engine/enginetests.go | 66 +++++++++++------------ 1 file changed, 33 insertions(+), 33 deletions(-) diff --git a/simulators/ethereum/engine/enginetests.go b/simulators/ethereum/engine/enginetests.go index d42f3a3aa4..d90d4c4e2a 100644 --- a/simulators/ethereum/engine/enginetests.go +++ b/simulators/ethereum/engine/enginetests.go @@ -207,77 +207,77 @@ var engineTests = []TestSpec{ // Invalid Ancestor Re-Org Tests (Reveal via newPayload) { - Name: "Invalid Ancestor Chain Re-Org, Invalid StateRoot, INV_P1', Reveal using newPayload", + Name: "Invalid Ancestor Chain Re-Org, Invalid StateRoot, Invalid P1', Reveal using newPayload", Run: invalidMissingAncestorReOrgGen(1, InvalidStateRoot, false, true), }, { - Name: "Invalid Ancestor Chain Re-Org, Invalid StateRoot, INV_P5', Reveal using newPayload", - Run: invalidMissingAncestorReOrgGen(5, InvalidStateRoot, false, true), + Name: "Invalid Ancestor Chain Re-Org, Invalid StateRoot, Invalid P9', Reveal using newPayload", + Run: invalidMissingAncestorReOrgGen(9, InvalidStateRoot, false, true), }, { - Name: "Invalid Ancestor Chain Re-Org, Invalid StateRoot, INV_P10', Reveal using newPayload", + Name: "Invalid Ancestor Chain Re-Org, Invalid StateRoot, Invalid P10', Reveal using newPayload", Run: invalidMissingAncestorReOrgGen(10, InvalidStateRoot, false, true), }, // Invalid Ancestor Re-Org Tests (Reveal via sync through secondary client) { - Name: "Invalid Ancestor Chain Re-Org, Invalid StateRoot, INV_P5', Reveal using sync", - Run: invalidMissingAncestorReOrgGen(5, InvalidStateRoot, true, false), + Name: "Invalid Ancestor Chain Re-Org, Invalid StateRoot, Invalid P9', Reveal using sync", + Run: invalidMissingAncestorReOrgGen(9, InvalidStateRoot, true, false), }, { - Name: "Invalid Ancestor Chain Re-Org, Invalid StateRoot, Empty Txs, INV_P5', Reveal using sync", - Run: invalidMissingAncestorReOrgGen(5, InvalidStateRoot, true, true), + Name: "Invalid Ancestor Chain Re-Org, Invalid StateRoot, Empty Txs, Invalid P9', Reveal using sync", + Run: invalidMissingAncestorReOrgGen(9, InvalidStateRoot, true, true), }, { - Name: "Invalid Ancestor Chain Re-Org, Invalid ReceiptsRoot, INV_P5', Reveal using sync", - Run: invalidMissingAncestorReOrgGen(5, InvalidReceiptsRoot, true, false), + Name: "Invalid Ancestor Chain Re-Org, Invalid ReceiptsRoot, Invalid P9', Reveal using sync", + Run: invalidMissingAncestorReOrgGen(9, InvalidReceiptsRoot, true, false), }, { - Name: "Invalid Ancestor Chain Re-Org, Invalid Number, INV_P5', Reveal using sync", - Run: invalidMissingAncestorReOrgGen(5, InvalidNumber, true, false), + Name: "Invalid Ancestor Chain Re-Org, Invalid Number, Invalid P9', Reveal using sync", + Run: invalidMissingAncestorReOrgGen(9, InvalidNumber, true, false), }, { - Name: "Invalid Ancestor Chain Re-Org, Invalid GasLimit, INV_P5', Reveal using sync", - Run: invalidMissingAncestorReOrgGen(5, InvalidGasLimit, true, false), + Name: "Invalid Ancestor Chain Re-Org, Invalid GasLimit, Invalid P9', Reveal using sync", + Run: invalidMissingAncestorReOrgGen(9, InvalidGasLimit, true, false), }, { - Name: "Invalid Ancestor Chain Re-Org, Invalid GasUsed, INV_P5', Reveal using sync", - Run: invalidMissingAncestorReOrgGen(5, InvalidGasUsed, true, false), + Name: "Invalid Ancestor Chain Re-Org, Invalid GasUsed, Invalid P9', Reveal using sync", + Run: invalidMissingAncestorReOrgGen(9, InvalidGasUsed, true, false), }, { - Name: "Invalid Ancestor Chain Re-Org, Invalid Timestamp, INV_P5', Reveal using sync", - Run: invalidMissingAncestorReOrgGen(5, InvalidTimestamp, true, false), + Name: "Invalid Ancestor Chain Re-Org, Invalid Timestamp, Invalid P9', Reveal using sync", + Run: invalidMissingAncestorReOrgGen(9, InvalidTimestamp, true, false), }, { - Name: "Invalid Ancestor Chain Re-Org, Invalid PrevRandao, INV_P5', Reveal using sync", - Run: invalidMissingAncestorReOrgGen(5, InvalidPrevRandao, true, false), + Name: "Invalid Ancestor Chain Re-Org, Invalid PrevRandao, Invalid P9', Reveal using sync", + Run: invalidMissingAncestorReOrgGen(9, InvalidPrevRandao, true, false), }, { - Name: "Invalid Ancestor Chain Re-Org, Incomplete Transactions, INV_P5', Reveal using sync", - Run: invalidMissingAncestorReOrgGen(5, RemoveTransaction, true, false), + Name: "Invalid Ancestor Chain Re-Org, Incomplete Transactions, Invalid P9', Reveal using sync", + Run: invalidMissingAncestorReOrgGen(9, RemoveTransaction, true, false), }, { - Name: "Invalid Ancestor Chain Re-Org, Invalid Transaction Signature, INV_P5', Reveal using sync", - Run: invalidMissingAncestorReOrgGen(5, InvalidTransactionSignature, true, false), + Name: "Invalid Ancestor Chain Re-Org, Invalid Transaction Signature, Invalid P9', Reveal using sync", + Run: invalidMissingAncestorReOrgGen(9, InvalidTransactionSignature, true, false), }, { - Name: "Invalid Ancestor Chain Re-Org, Invalid Transaction Nonce, INV_P5', Reveal using sync", - Run: invalidMissingAncestorReOrgGen(5, InvalidTransactionNonce, true, false), + Name: "Invalid Ancestor Chain Re-Org, Invalid Transaction Nonce, Invalid P9', Reveal using sync", + Run: invalidMissingAncestorReOrgGen(9, InvalidTransactionNonce, true, false), }, { - Name: "Invalid Ancestor Chain Re-Org, Invalid Transaction Gas, INV_P5', Reveal using sync", - Run: invalidMissingAncestorReOrgGen(5, InvalidTransactionGas, true, false), + Name: "Invalid Ancestor Chain Re-Org, Invalid Transaction Gas, Invalid P9', Reveal using sync", + Run: invalidMissingAncestorReOrgGen(9, InvalidTransactionGas, true, false), }, { - Name: "Invalid Ancestor Chain Re-Org, Invalid Transaction GasPrice, INV_P5', Reveal using sync", - Run: invalidMissingAncestorReOrgGen(5, InvalidTransactionGasPrice, true, false), + Name: "Invalid Ancestor Chain Re-Org, Invalid Transaction GasPrice, Invalid P9', Reveal using sync", + Run: invalidMissingAncestorReOrgGen(9, InvalidTransactionGasPrice, true, false), }, { - Name: "Invalid Ancestor Chain Re-Org, Invalid Transaction Value, INV_P5', Reveal using sync", - Run: invalidMissingAncestorReOrgGen(5, InvalidTransactionValue, true, false), + Name: "Invalid Ancestor Chain Re-Org, Invalid Transaction Value, Invalid P9', Reveal using sync", + Run: invalidMissingAncestorReOrgGen(9, InvalidTransactionValue, true, false), }, { - Name: "Invalid Ancestor Chain Re-Org, Invalid StateRoot, INV_P10', Reveal using sync", + Name: "Invalid Ancestor Chain Re-Org, Invalid StateRoot, Invalid P10', Reveal using sync", Run: invalidMissingAncestorReOrgGen(10, InvalidStateRoot, true, true), },