-
Notifications
You must be signed in to change notification settings - Fork 885
feat(evmrpc): bound debug_trace struct-logger memory via max_trace_struct_log_bytes (PLT-205) #3677
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
base: main
Are you sure you want to change the base?
Changes from all commits
0cee80c
fae41aa
d21f39d
a1c065d
b6e3302
038c768
f5246ee
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 |
|---|---|---|
|
|
@@ -131,6 +131,16 @@ type Config struct { | |
| // Timeout for each trace call | ||
| TraceTimeout time.Duration `mapstructure:"trace_timeout"` | ||
|
|
||
| // MaxTraceStructLogBytes bounds the retained struct-logger output (in bytes) per | ||
| // traced transaction on the default debug_trace* endpoints | ||
| // (debug_traceCall/traceTransaction/traceBlock*), guarding against quadratic memory | ||
| // growth from traces that read many distinct storage slots. The bound is per | ||
| // transaction, not per RPC call: geth builds a fresh struct logger for each tx, so a | ||
| // debug_traceBlock* call over N transactions retains up to N times this value (and the | ||
| // parallelized path holds several concurrent traces live). Set to 0 for unlimited | ||
| // (matches upstream geth behavior). | ||
| MaxTraceStructLogBytes uint64 `mapstructure:"max_trace_struct_log_bytes"` | ||
|
|
||
|
amir-deris marked this conversation as resolved.
|
||
| // EnableParallelizedBlockTrace enables the parallelized default debug_traceBlock* path. | ||
| EnableParallelizedBlockTrace bool `mapstructure:"enable_parallelized_block_trace"` | ||
|
|
||
|
|
@@ -227,6 +237,7 @@ var DefaultConfig = Config{ | |
| MaxConcurrentSimulationCalls: runtime.NumCPU(), | ||
| MaxTraceLookbackBlocks: 10000, | ||
| TraceTimeout: 30 * time.Second, | ||
| MaxTraceStructLogBytes: 32 * 1024 * 1024, // 32 MiB | ||
|
amir-deris marked this conversation as resolved.
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. [suggestion] The 32 MiB default is assigned to an entry-count |
||
| EnableParallelizedBlockTrace: false, | ||
| RPCStatsInterval: 10 * time.Second, | ||
| WorkerPoolSize: min(MaxWorkerPoolSize, runtime.NumCPU()*2), // Default: min(64, CPU cores × 2) | ||
|
|
@@ -280,6 +291,7 @@ const ( | |
| flagMaxConcurrentSimulationCalls = "evm.max_concurrent_simulation_calls" | ||
| flagMaxTraceLookbackBlocks = "evm.max_trace_lookback_blocks" | ||
| flagTraceTimeout = "evm.trace_timeout" | ||
| flagMaxTraceStructLogBytes = "evm.max_trace_struct_log_bytes" | ||
| flagEnableParallelizedBlockTrace = "evm.enable_parallelized_block_trace" | ||
| flagRPCStatsInterval = "evm.rpc_stats_interval" | ||
| flagWorkerPoolSize = "evm.worker_pool_size" | ||
|
|
@@ -439,6 +451,11 @@ func ReadConfig(opts servertypes.AppOptions) (Config, error) { | |
| return cfg, err | ||
| } | ||
| } | ||
| if v := opts.Get(flagMaxTraceStructLogBytes); v != nil { | ||
| if cfg.MaxTraceStructLogBytes, err = cast.ToUint64E(v); err != nil { | ||
| return cfg, err | ||
| } | ||
| } | ||
| if v := opts.Get(flagEnableParallelizedBlockTrace); v != nil { | ||
| if cfg.EnableParallelizedBlockTrace, err = cast.ToBoolE(v); err != nil { | ||
| return cfg, err | ||
|
|
@@ -684,6 +701,14 @@ max_trace_lookback_blocks = {{ .EVM.MaxTraceLookbackBlocks }} | |
| # Timeout for each trace call | ||
| trace_timeout = "{{ .EVM.TraceTimeout }}" | ||
|
|
||
| # MaxTraceStructLogBytes bounds the retained struct-logger output (in bytes) per traced | ||
| # transaction on the default debug_trace* endpoints, guarding against quadratic memory growth | ||
| # from traces that read many distinct storage slots. The bound is per transaction, not per RPC | ||
| # call: a debug_traceBlock* call over N transactions retains up to N times this value (and the | ||
| # parallelized path holds several concurrent traces live). Set to 0 for unlimited (matches | ||
| # upstream geth behavior). | ||
| max_trace_struct_log_bytes = {{ .EVM.MaxTraceStructLogBytes }} | ||
|
|
||
| # Enable the parallelized default debug_traceBlock* path. | ||
| enable_parallelized_block_trace = {{ .EVM.EnableParallelizedBlockTrace }} | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,6 +5,7 @@ import ( | |
| "encoding/json" | ||
| "errors" | ||
| "fmt" | ||
| "math" | ||
| "runtime/debug" | ||
| "sync" | ||
| "time" | ||
|
|
@@ -13,7 +14,8 @@ import ( | |
| "github.com/ethereum/go-ethereum/common/hexutil" | ||
| gethtypes "github.com/ethereum/go-ethereum/core/types" | ||
| "github.com/ethereum/go-ethereum/eth/tracers" | ||
| _ "github.com/ethereum/go-ethereum/eth/tracers/js" // run init()s to register JS tracers | ||
| _ "github.com/ethereum/go-ethereum/eth/tracers/js" // run init()s to register JS tracers | ||
| traceLogger "github.com/ethereum/go-ethereum/eth/tracers/logger" | ||
| _ "github.com/ethereum/go-ethereum/eth/tracers/native" // run init()s to register native tracers | ||
| "github.com/ethereum/go-ethereum/export" | ||
| "github.com/ethereum/go-ethereum/rpc" | ||
|
|
@@ -50,6 +52,7 @@ type DebugAPI struct { | |
| traceCallSemaphore chan struct{} // Semaphore for limiting concurrent trace calls | ||
| maxBlockLookback int64 | ||
| traceTimeout time.Duration | ||
| maxStructLogBytes int // per-call cap on retained default struct-logger output; 0 = unlimited | ||
| profiledBlockTrace bool | ||
| } | ||
|
|
||
|
|
@@ -203,10 +206,37 @@ func NewDebugAPI( | |
| traceCallSemaphore: sem, | ||
| maxBlockLookback: debugCfg.MaxTraceLookbackBlocks, | ||
| traceTimeout: debugCfg.TraceTimeout, | ||
| maxStructLogBytes: clampUint64ToInt(debugCfg.MaxTraceStructLogBytes), | ||
| profiledBlockTrace: debugCfg.EnableParallelizedBlockTrace, | ||
| } | ||
| } | ||
|
|
||
| // clampUint64ToInt converts an operator-configured uint64 to int, saturating at | ||
| // math.MaxInt instead of wrapping to a negative value. A negative maxStructLogBytes | ||
| // would be treated as "disabled" by clampDefaultStructLogLimit, silently defeating | ||
| // the cap — the opposite of an operator setting a very large limit. | ||
| func clampUint64ToInt(v uint64) int { | ||
| if v > uint64(math.MaxInt) { | ||
| return math.MaxInt | ||
| } | ||
| return int(v) | ||
| } | ||
|
|
||
| // clampDefaultStructLogLimit caps the default struct logger's retained output at | ||
| // api.maxStructLogBytes. No-op for custom tracers, a disabled cap (0), or a | ||
| // smaller caller-supplied Limit. | ||
| func (api *DebugAPI) clampDefaultStructLogLimit(config *tracers.TraceConfig) { | ||
| if config == nil || config.Tracer != nil || api.maxStructLogBytes <= 0 { | ||
| return | ||
|
amir-deris marked this conversation as resolved.
|
||
| } | ||
| if config.Config == nil { | ||
| config.Config = &traceLogger.Config{} | ||
| } | ||
| if config.Limit <= 0 || config.Limit > api.maxStructLogBytes { | ||
| config.Limit = api.maxStructLogBytes | ||
|
amir-deris marked this conversation as resolved.
amir-deris marked this conversation as resolved.
amir-deris marked this conversation as resolved.
amir-deris marked this conversation as resolved.
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. [blocker] |
||
| } | ||
| } | ||
|
claude[bot] marked this conversation as resolved.
|
||
|
|
||
| func (api *DebugAPI) TraceTransaction(ctx context.Context, hash common.Hash, config *tracers.TraceConfig) (result interface{}, returnErr error) { | ||
| startTime := time.Now() | ||
| defer func() { | ||
|
|
@@ -227,6 +257,10 @@ func (api *DebugAPI) TraceTransaction(ctx context.Context, hash common.Hash, con | |
| } | ||
| defer done() | ||
|
|
||
| if config == nil { | ||
| config = &tracers.TraceConfig{} | ||
| } | ||
| api.clampDefaultStructLogLimit(config) | ||
|
cursor[bot] marked this conversation as resolved.
|
||
| return api.tracersAPI.TraceTransaction(ctx, hash, config) | ||
| } | ||
|
|
||
|
|
@@ -430,6 +464,10 @@ func (api *DebugAPI) TraceBlockByNumber(ctx context.Context, number rpc.BlockNum | |
| return cached, nil | ||
| } | ||
|
|
||
| if config == nil { | ||
| config = &tracers.TraceConfig{} | ||
| } | ||
| api.clampDefaultStructLogLimit(config) | ||
| if api.shouldUseProfiledBlockTrace(config) { | ||
| result, returnErr = api.profiledTraceBlockByNumber(ctx, number, config) | ||
| } else { | ||
|
|
@@ -458,6 +496,10 @@ func (api *DebugAPI) TraceBlockByHash(ctx context.Context, hash common.Hash, con | |
| return cached, nil | ||
| } | ||
|
|
||
| if config == nil { | ||
| config = &tracers.TraceConfig{} | ||
| } | ||
| api.clampDefaultStructLogLimit(config) | ||
| if api.shouldUseProfiledBlockTrace(config) { | ||
| result, returnErr = api.profiledTraceBlockByHash(ctx, hash, config) | ||
| } else { | ||
|
|
@@ -482,6 +524,10 @@ func (api *DebugAPI) TraceCall(ctx context.Context, args export.TransactionArgs, | |
| return nil, returnErr | ||
| } | ||
|
|
||
| if config == nil { | ||
| config = &tracers.TraceCallConfig{} | ||
| } | ||
| api.clampDefaultStructLogLimit(&config.TraceConfig) | ||
| result, returnErr = api.tracersAPI.TraceCall(ctx, args, blockNrOrHash, config) | ||
| return | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,72 @@ | ||
| package evmrpc | ||
|
|
||
| import ( | ||
| "math" | ||
| "testing" | ||
|
|
||
| "github.com/ethereum/go-ethereum/eth/tracers" | ||
| traceLogger "github.com/ethereum/go-ethereum/eth/tracers/logger" | ||
| "github.com/stretchr/testify/require" | ||
| ) | ||
|
|
||
| func TestClampUint64ToInt(t *testing.T) { | ||
| require.Equal(t, 0, clampUint64ToInt(0)) | ||
| require.Equal(t, 1024, clampUint64ToInt(1024)) | ||
| require.Equal(t, math.MaxInt, clampUint64ToInt(math.MaxInt)) | ||
| require.Equal(t, math.MaxInt, clampUint64ToInt(math.MaxInt+1), "values above MaxInt must saturate, not wrap negative") | ||
| require.Equal(t, math.MaxInt, clampUint64ToInt(math.MaxUint64)) | ||
| } | ||
|
|
||
| func TestClampDefaultStructLogLimit(t *testing.T) { | ||
| const capacity = 256 * 1024 * 1024 | ||
|
|
||
| t.Run("imposes capacity when caller sets no limit", func(t *testing.T) { | ||
| api := &DebugAPI{maxStructLogBytes: capacity} | ||
| cfg := &tracers.TraceConfig{} | ||
| api.clampDefaultStructLogLimit(cfg) | ||
| require.NotNil(t, cfg.Config) | ||
| require.Equal(t, capacity, cfg.Config.Limit) | ||
| }) | ||
|
|
||
| t.Run("imposes capacity when nested Config is nil", func(t *testing.T) { | ||
| api := &DebugAPI{maxStructLogBytes: capacity} | ||
| cfg := &tracers.TraceConfig{Config: nil} | ||
| api.clampDefaultStructLogLimit(cfg) | ||
| require.NotNil(t, cfg.Config) | ||
| require.Equal(t, capacity, cfg.Config.Limit) | ||
| }) | ||
|
|
||
| t.Run("clamps a larger caller-supplied limit down to the capacity", func(t *testing.T) { | ||
| api := &DebugAPI{maxStructLogBytes: capacity} | ||
| cfg := &tracers.TraceConfig{Config: &traceLogger.Config{Limit: capacity * 4}} | ||
| api.clampDefaultStructLogLimit(cfg) | ||
| require.Equal(t, capacity, cfg.Config.Limit) | ||
| }) | ||
|
|
||
| t.Run("honors a smaller caller-supplied limit", func(t *testing.T) { | ||
| api := &DebugAPI{maxStructLogBytes: capacity} | ||
| cfg := &tracers.TraceConfig{Config: &traceLogger.Config{Limit: 1024}} | ||
| api.clampDefaultStructLogLimit(cfg) | ||
| require.Equal(t, 1024, cfg.Config.Limit) | ||
| }) | ||
|
|
||
| t.Run("no-op for custom tracers", func(t *testing.T) { | ||
| api := &DebugAPI{maxStructLogBytes: capacity} | ||
| name := callTracerName | ||
| cfg := &tracers.TraceConfig{Tracer: &name} | ||
| api.clampDefaultStructLogLimit(cfg) | ||
| require.Nil(t, cfg.Config, "custom tracers must not be given a struct-logger limit") | ||
| }) | ||
|
|
||
| t.Run("no-op when capacity is disabled", func(t *testing.T) { | ||
| api := &DebugAPI{maxStructLogBytes: 0} | ||
| cfg := &tracers.TraceConfig{} | ||
| api.clampDefaultStructLogLimit(cfg) | ||
| require.Nil(t, cfg.Config, "disabled capacity (0) must preserve upstream unlimited behavior") | ||
| }) | ||
|
|
||
| t.Run("nil config is safe", func(t *testing.T) { | ||
| api := &DebugAPI{maxStructLogBytes: capacity} | ||
| require.NotPanics(t, func() { api.clampDefaultStructLogLimit(nil) }) | ||
| }) | ||
| } |
Uh oh!
There was an error while loading. Please reload this page.