-
Notifications
You must be signed in to change notification settings - Fork 886
fix(evmrpc): honor trace_timeout in profiled debug_traceBlock path (PLT-989) #3914
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
6b9b0b6
466a319
e921de0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,56 @@ | ||
| package evmrpc_test | ||
|
|
||
| import ( | ||
| "context" | ||
| "math/big" | ||
| "testing" | ||
|
|
||
| gethtypes "github.com/ethereum/go-ethereum/core/types" | ||
| "github.com/ethereum/go-ethereum/core/vm" | ||
| "github.com/ethereum/go-ethereum/eth/tracers/tracersutils" | ||
| "github.com/ethereum/go-ethereum/trie" | ||
| "github.com/sei-protocol/sei-chain/evmrpc" | ||
| sdk "github.com/sei-protocol/sei-chain/sei-cosmos/types" | ||
| "github.com/sei-protocol/sei-chain/x/evm/state" | ||
| "github.com/stretchr/testify/require" | ||
| ) | ||
|
|
||
| func TestProfiledTraceBlockParallelMetadataLoopRespectsContext(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| header := &gethtypes.Header{Number: big.NewInt(1)} | ||
| block := gethtypes.NewBlock(header, &gethtypes.Body{}, nil, trie.NewStackTrie(nil)) | ||
| blockHash := block.Hash() | ||
| signer := gethtypes.LatestSignerForChainID(big.NewInt(1)) | ||
|
|
||
| ctx, cancel := context.WithCancel(t.Context()) | ||
| defer cancel() | ||
|
|
||
| var secondRunnableCalls int | ||
| metadata := []tracersutils.TraceBlockMetadata{ | ||
| { | ||
| TraceRunnable: func(vm.StateDB) { cancel() }, | ||
| }, | ||
| { | ||
| TraceRunnable: func(vm.StateDB) { secondRunnableCalls++ }, | ||
| }, | ||
| } | ||
|
|
||
| stateDB := state.NewDBImpl(Ctx, EVMKeeper, false) | ||
| api := evmrpc.NewDebugAPIForTest(evmrpc.NewTraceBackendForTest(EVMKeeper, func(int64) sdk.Context { return Ctx })) | ||
| got, err := evmrpc.ProfiledTraceBlockParallelForTest( | ||
| api, | ||
| ctx, | ||
| block, | ||
| metadata, | ||
| nil, | ||
| stateDB, | ||
| signer, | ||
| blockHash, | ||
| nil, | ||
| 2, | ||
| ) | ||
| require.ErrorIs(t, err, context.Canceled) | ||
| require.Nil(t, got) | ||
| require.Zero(t, secondRunnableCalls) | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,9 +1,16 @@ | ||
| package evmrpc | ||
|
|
||
| import ( | ||
| "context" | ||
| "math/big" | ||
| "testing" | ||
|
|
||
| gethcommon "github.com/ethereum/go-ethereum/common" | ||
| gethtypes "github.com/ethereum/go-ethereum/core/types" | ||
| "github.com/ethereum/go-ethereum/core/vm" | ||
| "github.com/ethereum/go-ethereum/eth/tracers" | ||
| "github.com/ethereum/go-ethereum/eth/tracers/tracersutils" | ||
| "github.com/ethereum/go-ethereum/trie" | ||
| "github.com/stretchr/testify/require" | ||
| ) | ||
|
|
||
|
|
@@ -66,3 +73,80 @@ func TestShouldUseProfiledBlockTrace(t *testing.T) { | |
| }) | ||
| } | ||
| } | ||
|
|
||
| func testProfiledTraceBlock(t *testing.T) (*gethtypes.Block, gethcommon.Hash, gethtypes.Signer) { | ||
| t.Helper() | ||
|
|
||
| header := &gethtypes.Header{Number: big.NewInt(1)} | ||
| block := gethtypes.NewBlock(header, &gethtypes.Body{}, nil, trie.NewStackTrie(nil)) | ||
| blockHash := block.Hash() | ||
| signer := gethtypes.LatestSignerForChainID(big.NewInt(1)) | ||
| return block, blockHash, signer | ||
| } | ||
|
|
||
| func TestProfiledTraceBlockSequentialMetadataLoopRespectsContext(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| block, blockHash, signer := testProfiledTraceBlock(t) | ||
| ctx, cancel := context.WithCancel(t.Context()) | ||
| defer cancel() | ||
|
|
||
| var secondRunnableCalls int | ||
| metadata := []tracersutils.TraceBlockMetadata{ | ||
| { | ||
| // Expire the trace context mid-replay; iteration 2 must not run. | ||
| TraceRunnable: func(vm.StateDB) { cancel() }, | ||
| }, | ||
| { | ||
| TraceRunnable: func(vm.StateDB) { secondRunnableCalls++ }, | ||
| }, | ||
| } | ||
|
|
||
| api := &DebugAPI{} | ||
| got, err := api.profiledTraceBlockSequential( | ||
| ctx, | ||
| block, | ||
| metadata, | ||
| nil, | ||
| nil, | ||
| vm.BlockContext{}, | ||
| signer, | ||
| blockHash, | ||
| make([]*tracers.TxTraceResult, 0), | ||
| ) | ||
| require.ErrorIs(t, err, context.Canceled) | ||
| require.Nil(t, got) | ||
| require.Zero(t, secondRunnableCalls) | ||
| } | ||
|
|
||
| func TestProfiledTraceBlockSequentialEVMOnlyLoopRespectsContext(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| block, blockHash, signer := testProfiledTraceBlock(t) | ||
| body := &gethtypes.Body{ | ||
| Transactions: gethtypes.Transactions{ | ||
| gethtypes.NewTx(&gethtypes.LegacyTx{}), | ||
| gethtypes.NewTx(&gethtypes.LegacyTx{}), | ||
| }, | ||
| } | ||
| block = gethtypes.NewBlock(block.Header(), body, nil, trie.NewStackTrie(nil)) | ||
|
|
||
| ctx, cancel := context.WithCancel(t.Context()) | ||
| cancel() | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [nit] Cancelling before the call means this only exercises the guard on iteration 0 — it proves the loop never starts, not that it stops. The scenario the PR is fixing is cancellation mid-loop, which the metadata test above covers properly via Also worth noting: the pre-fix failure mode here is a nil-interface panic in |
||
|
|
||
| api := &DebugAPI{} | ||
| results := make([]*tracers.TxTraceResult, len(body.Transactions)) | ||
| got, err := api.profiledTraceBlockSequential( | ||
| ctx, | ||
| block, | ||
| nil, | ||
| nil, | ||
| nil, | ||
| vm.BlockContext{}, | ||
| signer, | ||
| blockHash, | ||
| results, | ||
| ) | ||
| require.ErrorIs(t, err, context.Canceled) | ||
| require.Nil(t, got) | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.