From 82e8954851fd240ed93dead4247aef530c84bf81 Mon Sep 17 00:00:00 2001 From: Mario Vega Date: Thu, 26 Oct 2023 12:39:56 -0600 Subject: [PATCH] simulators/ethereum/engine: Add test to re-org to mempool and back in (#900) simulators/ethereum/engine: Add re-org to mempool and back intest --- .../ethereum/engine/suites/engine/reorg.go | 296 ++++++++++++------ .../ethereum/engine/suites/engine/tests.go | 10 +- 2 files changed, 204 insertions(+), 102 deletions(-) diff --git a/simulators/ethereum/engine/suites/engine/reorg.go b/simulators/ethereum/engine/suites/engine/reorg.go index 5e45f8dcff..a36f296b95 100644 --- a/simulators/ethereum/engine/suites/engine/reorg.go +++ b/simulators/ethereum/engine/suites/engine/reorg.go @@ -116,11 +116,19 @@ func (spec SidechainReOrgTest) Execute(t *test.Env) { } // Test performing a re-org that involves removing or modifying a transaction +type TransactionReOrgScenario string + +const ( + TransactionReOrgScenarioReOrgOut TransactionReOrgScenario = "Re-Org Out" + TransactionReOrgScenarioReOrgBackIn TransactionReOrgScenario = "Re-Org Back In" + TransactionReOrgScenarioReOrgDifferentBlock TransactionReOrgScenario = "Re-Org to Different Block" + TransactionReOrgScenarioNewPayloadOnRevert TransactionReOrgScenario = "New Payload on Revert Back" +) + type TransactionReOrgTest struct { test.BaseSpec - ReorgOut bool - ReorgDifferentBlock bool - NewPayloadOnRevert bool + TransactionCount int + Scenario TransactionReOrgScenario } func (s TransactionReOrgTest) WithMainFork(fork config.Fork) test.Spec { @@ -130,19 +138,11 @@ func (s TransactionReOrgTest) WithMainFork(fork config.Fork) test.Spec { } func (s TransactionReOrgTest) GetName() string { - reOrgType := "Out Of Block" - if s.ReorgDifferentBlock { - reOrgType = "To Different Block" + name := "Transaction Re-Org" + if s.Scenario != "" { + name = fmt.Sprintf("%s, %s", name, s.Scenario) } - payloadOnRevertStatus := "False" - if s.NewPayloadOnRevert { - payloadOnRevertStatus = "True" - } - return fmt.Sprintf( - "Transaction ReOrg %s, NewPayloadOnRevert=%s", - reOrgType, - payloadOnRevertStatus, - ) + return name } // Test transaction status after a forkchoiceUpdated re-orgs to an alternative hash where a transaction is not present @@ -155,20 +155,48 @@ func (spec TransactionReOrgTest) Execute(t *test.Env) { // Create transactions that modify the state in order to check after the reorg. var ( - txCount = 5 + err error + txCount = spec.TransactionCount sstoreContractAddr = common.HexToAddress("0000000000000000000000000000000000000317") ) - for i := 0; i < txCount; i++ { - var ( - altPayload *typ.ExecutableData - tx typ.Transaction + if txCount == 0 { + // Default is to send 5 transactions + txCount = 5 + } + + // Send a transaction on each payload of the canonical chain + sendTransaction := func(i int) (typ.Transaction, error) { + data := common.LeftPadBytes([]byte{byte(i)}, 32) + t.Logf("transactionReorg, i=%v, data=%v\n", i, data) + return t.SendNextTransaction( + t.TestContext, + t.Engine, + &helper.BaseTransactionCreator{ + Recipient: &sstoreContractAddr, + Amount: big0, + Payload: data, + TxType: t.TestTransactionType, + GasLimit: 75000, + ForkConfig: t.ForkConfig, + }, ) + + } + + var ( + altPayload *typ.ExecutableData + nextTx typ.Transaction + tx typ.Transaction + ) + + for i := 0; i < txCount; i++ { + // Generate two payloads, one with the transaction and the other one without it t.CLMock.ProduceSingleBlock(clmock.BlockProcessCallbacks{ OnPayloadAttributesGenerated: func() { // At this point we have not broadcast the transaction. - if spec.ReorgOut { + if spec.Scenario == TransactionReOrgScenarioReOrgOut { // Any payload we get should not contain any payloadAttributes := t.CLMock.LatestPayloadAttributes rand.Read(payloadAttributes.Random[:]) @@ -186,41 +214,33 @@ func (spec TransactionReOrgTest) Execute(t *test.Env) { } } - // At this point we can broadcast the transaction and it will be included in the next payload - // Data is the key where a `1` will be stored - data := common.LeftPadBytes([]byte{byte(i)}, 32) - t.Logf("transactionReorg, i=%v, data=%v\n", i, data) - var err error - tx, err = t.SendNextTransaction( - t.TestContext, - t.Engine, - &helper.BaseTransactionCreator{ - Recipient: &sstoreContractAddr, - Amount: big0, - Payload: data, - TxType: t.TestTransactionType, - GasLimit: 75000, - ForkConfig: t.ForkConfig, - }, - ) - if err != nil { - t.Fatalf("FAIL (%s): Error trying to send transaction: %v", t.TestName, err) - } + if spec.Scenario != TransactionReOrgScenarioReOrgBackIn { + // At this point we can broadcast the transaction and it will be included in the next payload + // Data is the key where a `1` will be stored + tx, err = sendTransaction(i) + if err != nil { + t.Fatalf("FAIL (%s): Error trying to send transaction: %v", t.TestName, err) + } - // Get the receipt - ctx, cancel := context.WithTimeout(t.TestContext, globals.RPCTimeout) - defer cancel() - receipt, _ := t.Eth.TransactionReceipt(ctx, tx.Hash()) - if receipt != nil { - t.Fatalf("FAIL (%s): Receipt obtained before tx included in block: %v", t.TestName, receipt) + // Get the receipt + ctx, cancel := context.WithTimeout(t.TestContext, globals.RPCTimeout) + defer cancel() + receipt, _ := t.Eth.TransactionReceipt(ctx, tx.Hash()) + if receipt != nil { + t.Fatalf("FAIL (%s): Receipt obtained before tx included in block: %v", t.TestName, receipt) + } } + }, OnGetPayload: func() { // Check that indeed the payload contains the transaction - if !helper.TransactionInPayload(&t.CLMock.LatestPayloadBuilt, tx) { - t.Fatalf("FAIL (%s): Payload built does not contain the transaction: %v", t.TestName, t.CLMock.LatestPayloadBuilt) + if tx != nil { + if !helper.TransactionInPayload(&t.CLMock.LatestPayloadBuilt, tx) { + t.Fatalf("FAIL (%s): Payload built does not contain the transaction: %v", t.TestName, t.CLMock.LatestPayloadBuilt) + } } - if spec.ReorgDifferentBlock { + + if spec.Scenario == TransactionReOrgScenarioReOrgDifferentBlock || spec.Scenario == TransactionReOrgScenarioNewPayloadOnRevert { // Create side payload with different hash var err error customizer := &helper.CustomPayloadData{ @@ -237,68 +257,148 @@ func (spec TransactionReOrgTest) Execute(t *test.Env) { if altPayload.BlockHash == t.CLMock.LatestPayloadBuilt.BlockHash { t.Fatalf("FAIL (%s): Incorrect hash for payloads: %v == %v", t.TestName, altPayload.BlockHash, t.CLMock.LatestPayloadBuilt.BlockHash) } + } else if spec.Scenario == TransactionReOrgScenarioReOrgBackIn { + // At this point we broadcast the transaction and request a new payload from the client that must + // contain the transaction. + // Since we are re-orging out and back in on the next block, the verification of this transaction + // being included happens on the next block + nextTx, err = sendTransaction(i) + if err != nil { + t.Fatalf("FAIL (%s): Error trying to send transaction: %v", t.TestName, err) + } + + if i == 0 { + // We actually can only do this once because the transaction carries over and we cannot + // impede it from being included in the next payload + forkchoiceUpdated := t.CLMock.LatestForkchoice + payloadAttributes := t.CLMock.LatestPayloadAttributes + rand.Read(payloadAttributes.SuggestedFeeRecipient[:]) + f := t.TestEngine.TestEngineForkchoiceUpdated( + &forkchoiceUpdated, + &payloadAttributes, + t.CLMock.LatestHeader.Time, + ) + f.ExpectPayloadStatus(test.Valid) + + // Wait a second for the client to prepare the payload with the included transaction + + time.Sleep(t.CLMock.PayloadProductionClientDelay) + + g := t.TestEngine.TestEngineGetPayload(f.Response.PayloadID, &t.CLMock.LatestPayloadAttributes) + g.ExpectNoError() + + if !helper.TransactionInPayload(&g.Payload, nextTx) { + t.Fatalf("FAIL (%s): Payload built does not contain the transaction: %v", t.TestName, g.Payload) + } + + // Send the new payload and forkchoiceUpdated to it + n := t.TestEngine.TestEngineNewPayload(&g.Payload) + n.ExpectStatus(test.Valid) + + forkchoiceUpdated.HeadBlockHash = g.Payload.BlockHash + + s := t.TestEngine.TestEngineForkchoiceUpdated(&forkchoiceUpdated, nil, g.Payload.Timestamp) + s.ExpectPayloadStatus(test.Valid) + } } }, OnNewPayloadBroadcast: func() { - // Get the receipt - ctx, cancel := context.WithTimeout(t.TestContext, globals.RPCTimeout) - defer cancel() - receipt, _ := t.Eth.TransactionReceipt(ctx, tx.Hash()) - if receipt != nil { - t.Fatalf("FAIL (%s): Receipt obtained before tx included in block (NewPayload): %v", t.TestName, receipt) + if tx != nil { + // Get the receipt + ctx, cancel := context.WithTimeout(t.TestContext, globals.RPCTimeout) + defer cancel() + receipt, _ := t.Eth.TransactionReceipt(ctx, tx.Hash()) + if receipt != nil { + t.Fatalf("FAIL (%s): Receipt obtained before tx included in block (NewPayload): %v", t.TestName, receipt) + } } }, OnForkchoiceBroadcast: func() { - // Transaction is now in the head of the canonical chain, re-org and verify it's removed - // Get the receipt - txt := t.TestEngine.TestTransactionReceipt(tx.Hash()) - txt.ExpectBlockHash(t.CLMock.LatestForkchoice.HeadBlockHash) + if spec.Scenario != TransactionReOrgScenarioReOrgBackIn { + // Transaction is now in the head of the canonical chain, re-org and verify it's removed + // Get the receipt + txt := t.TestEngine.TestTransactionReceipt(tx.Hash()) + txt.ExpectBlockHash(t.CLMock.LatestForkchoice.HeadBlockHash) - if altPayload.ParentHash != t.CLMock.LatestPayloadBuilt.ParentHash { - t.Fatalf("FAIL (%s): Incorrect parent hash for payloads: %v != %v", t.TestName, altPayload.ParentHash, t.CLMock.LatestPayloadBuilt.ParentHash) - } - if altPayload.BlockHash == t.CLMock.LatestPayloadBuilt.BlockHash { - t.Fatalf("FAIL (%s): Incorrect hash for payloads: %v == %v", t.TestName, altPayload.BlockHash, t.CLMock.LatestPayloadBuilt.BlockHash) - } + if altPayload.ParentHash != t.CLMock.LatestPayloadBuilt.ParentHash { + t.Fatalf("FAIL (%s): Incorrect parent hash for payloads: %v != %v", t.TestName, altPayload.ParentHash, t.CLMock.LatestPayloadBuilt.ParentHash) + } + if altPayload.BlockHash == t.CLMock.LatestPayloadBuilt.BlockHash { + t.Fatalf("FAIL (%s): Incorrect hash for payloads: %v == %v", t.TestName, altPayload.BlockHash, t.CLMock.LatestPayloadBuilt.BlockHash) + } - if altPayload == nil { - t.Fatalf("FAIL (%s): No payload to re-org to", t.TestName) + if altPayload == nil { + t.Fatalf("FAIL (%s): No payload to re-org to", t.TestName) + } + r := t.TestEngine.TestEngineNewPayload(altPayload) + r.ExpectStatus(test.Valid) + r.ExpectLatestValidHash(&altPayload.BlockHash) + + s := t.TestEngine.TestEngineForkchoiceUpdated(&api.ForkchoiceStateV1{ + HeadBlockHash: altPayload.BlockHash, + SafeBlockHash: t.CLMock.LatestForkchoice.SafeBlockHash, + FinalizedBlockHash: t.CLMock.LatestForkchoice.FinalizedBlockHash, + }, nil, altPayload.Timestamp) + s.ExpectPayloadStatus(test.Valid) + + p := t.TestEngine.TestHeaderByNumber(Head) + p.ExpectHash(altPayload.BlockHash) + + txt = t.TestEngine.TestTransactionReceipt(tx.Hash()) + if spec.Scenario == TransactionReOrgScenarioReOrgOut { + if txt.Receipt != nil { + receiptJson, _ := json.MarshalIndent(txt.Receipt, "", " ") + t.Fatalf("FAIL (%s): Receipt was obtained when the tx had been re-org'd out: %s", t.TestName, receiptJson) + } + } else if spec.Scenario == TransactionReOrgScenarioReOrgDifferentBlock || spec.Scenario == TransactionReOrgScenarioNewPayloadOnRevert { + txt.ExpectBlockHash(altPayload.BlockHash) + } + + // Re-org back + if spec.Scenario == TransactionReOrgScenarioNewPayloadOnRevert { + r = t.TestEngine.TestEngineNewPayload(&t.CLMock.LatestPayloadBuilt) + r.ExpectStatus(test.Valid) + r.ExpectLatestValidHash(&t.CLMock.LatestPayloadBuilt.BlockHash) + } + t.CLMock.BroadcastForkchoiceUpdated(&t.CLMock.LatestForkchoice, nil, 1) } - r := t.TestEngine.TestEngineNewPayload(altPayload) - r.ExpectStatus(test.Valid) - r.ExpectLatestValidHash(&altPayload.BlockHash) - - s := t.TestEngine.TestEngineForkchoiceUpdated(&api.ForkchoiceStateV1{ - HeadBlockHash: altPayload.BlockHash, - SafeBlockHash: t.CLMock.LatestForkchoice.SafeBlockHash, - FinalizedBlockHash: t.CLMock.LatestForkchoice.FinalizedBlockHash, - }, nil, altPayload.Timestamp) - s.ExpectPayloadStatus(test.Valid) - - p := t.TestEngine.TestHeaderByNumber(Head) - p.ExpectHash(altPayload.BlockHash) - - txt = t.TestEngine.TestTransactionReceipt(tx.Hash()) - if spec.ReorgOut { - if txt.Receipt != nil { - receiptJson, _ := json.MarshalIndent(txt.Receipt, "", " ") - t.Fatalf("FAIL (%s): Receipt was obtained when the tx had been re-org'd out: %s", t.TestName, receiptJson) + + if tx != nil { + // Now it should be back with main payload + txt := t.TestEngine.TestTransactionReceipt(tx.Hash()) + txt.ExpectBlockHash(t.CLMock.LatestForkchoice.HeadBlockHash) + + if spec.Scenario != TransactionReOrgScenarioReOrgBackIn { + tx = nil } - } else if spec.ReorgDifferentBlock { - txt.ExpectBlockHash(altPayload.BlockHash) } - // Re-org back - if spec.NewPayloadOnRevert { - r = t.TestEngine.TestEngineNewPayload(&t.CLMock.LatestPayloadBuilt) - r.ExpectStatus(test.Valid) - r.ExpectLatestValidHash(&t.CLMock.LatestPayloadBuilt.BlockHash) + if spec.Scenario == TransactionReOrgScenarioReOrgBackIn && i > 0 { + // Reasoning: Most of the clients do not re-add blob transactions to the pool + // after a re-org, so we need to wait until the next tx is sent to actually + // verify. + tx = nextTx } - t.CLMock.BroadcastForkchoiceUpdated(&t.CLMock.LatestForkchoice, nil, 1) - // Not it should be back with main payload - txt = t.TestEngine.TestTransactionReceipt(tx.Hash()) - txt.ExpectBlockHash(t.CLMock.LatestForkchoice.HeadBlockHash) + }, + }) + + } + + if tx != nil { + // Produce one last block and verify that the block contains the transaction + t.CLMock.ProduceSingleBlock(clmock.BlockProcessCallbacks{ + OnForkchoiceBroadcast: func() { + if !helper.TransactionInPayload(&t.CLMock.LatestPayloadBuilt, tx) { + t.Fatalf("FAIL (%s): Payload built does not contain the transaction: %v", t.TestName, t.CLMock.LatestPayloadBuilt) + } + // Get the receipt + ctx, cancel := context.WithTimeout(t.TestContext, globals.RPCTimeout) + defer cancel() + receipt, _ := t.Eth.TransactionReceipt(ctx, tx.Hash()) + if receipt == nil { + t.Fatalf("FAIL (%s): Receipt not obtained after tx included in block: %v", t.TestName, receipt) + } }, }) diff --git a/simulators/ethereum/engine/suites/engine/tests.go b/simulators/ethereum/engine/suites/engine/tests.go index cf35765ea1..e90c12158f 100644 --- a/simulators/ethereum/engine/suites/engine/tests.go +++ b/simulators/ethereum/engine/suites/engine/tests.go @@ -329,14 +329,16 @@ func init() { // Re-org a transaction out of a block, or into a new block Tests = append(Tests, TransactionReOrgTest{ - ReorgOut: true, + Scenario: TransactionReOrgScenarioReOrgOut, }, TransactionReOrgTest{ - ReorgDifferentBlock: true, + Scenario: TransactionReOrgScenarioReOrgDifferentBlock, }, TransactionReOrgTest{ - ReorgDifferentBlock: true, - NewPayloadOnRevert: true, + Scenario: TransactionReOrgScenarioNewPayloadOnRevert, + }, + TransactionReOrgTest{ + Scenario: TransactionReOrgScenarioReOrgBackIn, }, )