From f9b74de6f76fae256dad0d15c27e258e855a07c9 Mon Sep 17 00:00:00 2001 From: blindchaser Date: Fri, 31 Jul 2026 17:41:48 -0400 Subject: [PATCH] fix(seidb-bench): stop the state-store bench backends panicking on a nil config MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit runBenchmark hands NewDBImpl a nil dbConfig for every backend, but the SSComposite, SSHistoricalOffload and CompositeDual_SSComposite cases still asserted dbConfig.(*T) without the comma-ok form. A single-value assertion on a nil interface panics, so BenchmarkSSCompositeWrite, BenchmarkSSHistoricalOffloadWrite and BenchmarkCombinedCompositeDualSSComposite crashed at startup — the same failure #3770 fixed for FlatKV, in the same switch. Merely copying that fix would only move the crash one frame later, since both SS Composite constructors dereference the config. Route every case through one generic helper that treats nil as ordinary and only rejects a wrong type, and let the two SS Composite constructors fall back to DefaultBenchStateStoreConfig the way the MemIAVL and FlatKV ones already fall back to theirs. SSHistoricalOffload keeps no default: its stream needs brokers only the caller knows, so it now reports "historical offload config is required" instead of crashing. The first two benchmarks run to completion again; the offload one fails with that message. Co-authored-by: Cursor --- .../bench/wrappers/db_implementations.go | 61 +++++++++++++++---- .../state_db/bench/wrappers/wrappers_test.go | 33 +++++++--- 2 files changed, 76 insertions(+), 18 deletions(-) diff --git a/sei-db/state_db/bench/wrappers/db_implementations.go b/sei-db/state_db/bench/wrappers/db_implementations.go index 64fcf818cd..8d067417ed 100644 --- a/sei-db/state_db/bench/wrappers/db_implementations.go +++ b/sei-db/state_db/bench/wrappers/db_implementations.go @@ -126,6 +126,9 @@ func openSSComposite(dir string, cfg config.StateStoreConfig) (*ssComposite.Comp } func newSSCompositeStateStore(dbDir string, ssConfig *config.StateStoreConfig) (DBWrapper, error) { + if ssConfig == nil { + ssConfig = DefaultBenchStateStoreConfig() + } fmt.Printf("Opening composite state store from directory %s\n", dbDir) store, err := openSSComposite(dbDir, *ssConfig) if err != nil { @@ -139,6 +142,9 @@ func newCombinedCompositeDualSSComposite( dbDir string, ssConfig *config.StateStoreConfig, ) (DBWrapper, error) { + if ssConfig == nil { + ssConfig = DefaultBenchStateStoreConfig() + } fmt.Printf("Opening CompositeDual (SC) + Composite (SS) from directory %s\n", dbDir) sc, err := newCompositeCommitStore(ctx, filepath.Join(dbDir, "sc"), sctypes.TestOnlyDualWrite) @@ -153,23 +159,42 @@ func newCombinedCompositeDualSSComposite( return NewCombinedWrapper(sc, ss), nil } +// backendConfig converts the untyped config NewDBImpl is handed into the one its backend expects. +// +// A nil config is ordinary rather than exceptional: runBenchmark passes nil for every backend, and +// each constructor supplies its own default. So nil is passed straight through, and only a config of +// the wrong type is an error. Asserting without this — dbConfig.(*T) on a nil interface — panics +// before the constructor can apply that default, which is how three bench backends came to crash at +// startup. +func backendConfig[T any](dbType DBType, dbConfig any) (*T, error) { + if dbConfig == nil { + return nil, nil + } + typed, ok := dbConfig.(*T) + if !ok { + var want T + return nil, fmt.Errorf("invalid %s config type %T, want *%T", dbType, dbConfig, want) + } + return typed, nil +} + // NewDBImpl instantiates a new empty DBWrapper based on the given DBType. func NewDBImpl(ctx context.Context, dbType DBType, dataDir string, dbConfig any) (DBWrapper, error) { switch dbType { case NoOp: return NewNoOpWrapper(), nil case MemIAVL: - memiavlCfg, ok := dbConfig.(*memiavl.Config) - if dbConfig != nil && !ok { - return nil, fmt.Errorf("invalid MemIAVL config type %T", dbConfig) + cfg, err := backendConfig[memiavl.Config](dbType, dbConfig) + if err != nil { + return nil, err } - return newMemIAVLCommitStore(dataDir, memiavlCfg) + return newMemIAVLCommitStore(dataDir, cfg) case FlatKV: - flatKVConfig, ok := dbConfig.(*flatkvConfig.Config) - if dbConfig != nil && !ok { - return nil, fmt.Errorf("invalid FlatKV config type %T", dbConfig) + cfg, err := backendConfig[flatkvConfig.Config](dbType, dbConfig) + if err != nil { + return nil, err } - return newFlatKVCommitStore(ctx, dataDir, flatKVConfig) + return newFlatKVCommitStore(ctx, dataDir, cfg) case CompositeDual: return newCompositeCommitStore(ctx, dataDir, sctypes.TestOnlyDualWrite) case CompositeSplit: @@ -177,11 +202,25 @@ func NewDBImpl(ctx context.Context, dbType DBType, dataDir string, dbConfig any) case CompositeCosmos: return newCompositeCommitStore(ctx, dataDir, sctypes.MemiavlOnly) case SSComposite: - return newSSCompositeStateStore(dataDir, dbConfig.(*config.StateStoreConfig)) + cfg, err := backendConfig[config.StateStoreConfig](dbType, dbConfig) + if err != nil { + return nil, err + } + return newSSCompositeStateStore(dataDir, cfg) case SSHistoricalOffload: - return newSSHistoricalOffloadStateStore(ctx, dataDir, dbConfig.(*HistoricalOffloadConfig)) + // No default: the stream needs brokers only the caller knows, so a missing config is + // reported by HistoricalOffloadConfig.Validate rather than invented here. + cfg, err := backendConfig[HistoricalOffloadConfig](dbType, dbConfig) + if err != nil { + return nil, err + } + return newSSHistoricalOffloadStateStore(ctx, dataDir, cfg) case CompositeDual_SSComposite: - return newCombinedCompositeDualSSComposite(ctx, dataDir, dbConfig.(*config.StateStoreConfig)) + cfg, err := backendConfig[config.StateStoreConfig](dbType, dbConfig) + if err != nil { + return nil, err + } + return newCombinedCompositeDualSSComposite(ctx, dataDir, cfg) default: return nil, fmt.Errorf("unsupported DB type: %s", dbType) } diff --git a/sei-db/state_db/bench/wrappers/wrappers_test.go b/sei-db/state_db/bench/wrappers/wrappers_test.go index 3d556fbc1c..7c2f35e86c 100644 --- a/sei-db/state_db/bench/wrappers/wrappers_test.go +++ b/sei-db/state_db/bench/wrappers/wrappers_test.go @@ -192,15 +192,34 @@ func TestNoOpWrapperTracksVersionWithoutReadsOrWrites(t *testing.T) { require.Equal(t, int64(9), version) } -func TestNewDBImplFlatKVUsesDefaultConfigWhenNil(t *testing.T) { - wrapper, err := NewDBImpl(t.Context(), FlatKV, t.TempDir(), nil) - require.NoError(t, err) - require.NoError(t, wrapper.Close()) +// runBenchmark passes nil for every backend, so each of these opened with a panic before their +// config was made optional. +func TestNewDBImplUsesDefaultConfigWhenNil(t *testing.T) { + for _, dbType := range []DBType{MemIAVL, FlatKV, SSComposite, CompositeDual_SSComposite} { + t.Run(string(dbType), func(t *testing.T) { + wrapper, err := NewDBImpl(t.Context(), dbType, t.TempDir(), nil) + require.NoError(t, err) + require.NoError(t, wrapper.Close()) + }) + } } -func TestNewDBImplFlatKVRejectsInvalidConfigType(t *testing.T) { - wrapper, err := NewDBImpl(t.Context(), FlatKV, t.TempDir(), "invalid") +// The offload stream needs brokers that only the caller knows, so this backend has no default to +// fall back on and must say so rather than panic. +func TestNewDBImplSSHistoricalOffloadReportsMissingConfig(t *testing.T) { + wrapper, err := NewDBImpl(t.Context(), SSHistoricalOffload, t.TempDir(), nil) require.Error(t, err) require.Nil(t, wrapper) - require.ErrorContains(t, err, "invalid FlatKV config type string") + require.ErrorContains(t, err, "historical offload config is required") +} + +func TestNewDBImplRejectsInvalidConfigType(t *testing.T) { + for _, dbType := range []DBType{MemIAVL, FlatKV, SSComposite, SSHistoricalOffload, CompositeDual_SSComposite} { + t.Run(string(dbType), func(t *testing.T) { + wrapper, err := NewDBImpl(t.Context(), dbType, t.TempDir(), "invalid") + require.Error(t, err) + require.Nil(t, wrapper) + require.ErrorContains(t, err, "invalid "+string(dbType)+" config type string") + }) + } }