fix(runner): verify the datadir head for a bundle inside the fixtures artifact

pkg/builder/prerun_bundle.go

@@ -375,7 +375,20 @@ func ReadPreRunBundleInfo(bundleParentDir string) (*PreRunBundleInfo, error) {
 		return nil, nil
 	}
 
-	dir := filepath.Join(bundleParentDir, config.PreRunBundleSubdir)
+	return ReadPreRunBundleInfoAt(filepath.Join(bundleParentDir, config.PreRunBundleSubdir))
+}
+
+// ReadPreRunBundleInfoAt is ReadPreRunBundleInfo for a caller that already holds
+// the bundle directory itself rather than its parent. A runner-side pre_runs
+// source names that directory outright (it may sit anywhere inside an extracted
+// fixtures artifact), so it cannot go through the parent + PreRunBundleSubdir
+// form above.
+func ReadPreRunBundleInfoAt(bundleDir string) (*PreRunBundleInfo, error) {
+	if bundleDir == "" {
+		return nil, nil
+	}
+
+	dir := bundleDir
 	path := filepath.Join(dir, preRunBundleMetaFile)
 
 	data, err := os.ReadFile(path)

pkg/builder/prerun_bundle_info_test.go

@@ -178,3 +178,38 @@ func TestReadPreRunBundleInfoNoBundleAtAll(t *testing.T) {
 	require.NoError(t, err)
 	assert.Nil(t, info)
 }
+
+func TestReadPreRunBundleInfoAt(t *testing.T) {
+	// A runner-side pre_runs source names the bundle directory outright — it
+	// can sit anywhere inside an extracted fixtures artifact — so it cannot go
+	// through the parent + PreRunBundleSubdir form.
+	parent := t.TempDir()
+	writeBundleFile(t, parent, [][3]string{
+		{"0x17466ef", "0xaaa", "0x111"},
+		{"0x17466f0", "0xbbb", "0xaaa"},
+	}, 4096)
+
+	bundleDir := filepath.Join(parent, config.PreRunBundleSubdir)
+
+	direct, err := ReadPreRunBundleInfoAt(bundleDir)
+	require.NoError(t, err)
+	require.NotNil(t, direct)
+
+	// Same bundle reached the old way: the two must agree.
+	viaParent, err := ReadPreRunBundleInfo(parent)
+	require.NoError(t, err)
+	require.NotNil(t, viaParent)
+	assert.Equal(t, viaParent, direct)
+
+	assert.Equal(t, uint64(0x17466f0), direct.EndBlockNumber)
+	assert.Equal(t, "0xbbb", direct.EndBlockHash)
+
+	// Empty and absent stay non-errors, as for ReadPreRunBundleInfo.
+	got, err := ReadPreRunBundleInfoAt("")
+	require.NoError(t, err)
+	assert.Nil(t, got)
+
+	got, err = ReadPreRunBundleInfoAt(filepath.Join(t.TempDir(), "nope"))
+	require.NoError(t, err)
+	assert.Nil(t, got)
+}

pkg/executor/eest_source.go

@@ -1001,18 +1001,22 @@ func (s *EESTSource) discoverTests() (*PreparedSource, error) {
 	return result, nil
 }
 
-// loadPreRunBundleSteps returns the configured builder.pre_runs bundle as
-// pre-run steps (the runner replays them before the fixtures). Returns nil when
-// no pre_runs source is configured.
+// PreRunBundleDir resolves the directory holding this source's builder.pre_runs
+// bundle, or "" when there is none to resolve.
 //
 // The bundle is read from LocalFixturesDir when set, and otherwise from the
 // already-extracted fixtures artifact — resolved against the same root
 // FixturesSubdir resolves the fixtures against. A build ships the bundle and
 // the fixtures in one tarball, so a release consumer can reach both from a
 // single fixtures_url instead of staging the bundle on every runner host.
-func (s *EESTSource) loadPreRunBundleSteps() ([]*StepFile, error) {
-	if s.cfg.PreRuns == nil {
-		return nil, nil
+//
+// This is the one place that resolution happens: the runner reads the bundle's
+// metadata to check the datadir is on the bundle's chain, and a second copy of
+// this logic derived from config alone would miss the artifact case and skip
+// that check.
+func (s *EESTSource) PreRunBundleDir() string {
+	if s.cfg == nil || s.cfg.PreRuns == nil {
+		return ""
 	}
 
 	base := s.cfg.PreRuns.LocalFixturesDir
@@ -1023,15 +1027,31 @@ func (s *EESTSource) loadPreRunBundleSteps() ([]*StepFile, error) {
 	// Nothing to resolve against: no local directory configured and no
 	// fixtures extracted for this source.
 	if base == "" {
-		return nil, nil
+		return ""
 	}
 
 	subdir := s.cfg.PreRuns.FixturesSubdir
 	if subdir == "" {
 		subdir = config.PreRunBundleSubdir
 	}
 
-	bundleDir := filepath.Join(base, subdir)
+	return filepath.Join(base, subdir)
+}
+
+// loadPreRunBundleSteps returns the configured builder.pre_runs bundle as
+// pre-run steps (the runner replays them before the fixtures). Returns nil when
+// no pre_runs source is configured.
+//
+// The bundle is read from LocalFixturesDir when set, and otherwise from the
+// already-extracted fixtures artifact — resolved against the same root
+// FixturesSubdir resolves the fixtures against. A build ships the bundle and
+// the fixtures in one tarball, so a release consumer can reach both from a
+// single fixtures_url instead of staging the bundle on every runner host.
+func (s *EESTSource) loadPreRunBundleSteps() ([]*StepFile, error) {
+	bundleDir := s.PreRunBundleDir()
+	if bundleDir == "" {
+		return nil, nil
+	}
 
 	entries, err := filepath.Glob(filepath.Join(bundleDir, "*.request"))
 	if err != nil {

pkg/executor/eest_source_test.go

@@ -183,3 +183,56 @@ func TestLoadPreRunBundleSteps(t *testing.T) {
 		assert.Contains(t, err.Error(), "no pre-run bundle")
 	})
 }
+
+func TestPreRunBundleDir(t *testing.T) {
+	tests := []struct {
+		name        string
+		preRuns     *config.EESTPreRunsSource
+		fixturesDir string
+		want        func(fixtures string) string
+	}{
+		{
+			name:    "no pre_runs source",
+			preRuns: nil,
+			want:    func(string) string { return "" },
+		},
+		{
+			name:    "nothing to resolve against",
+			preRuns: &config.EESTPreRunsSource{},
+			want:    func(string) string { return "" },
+		},
+		{
+			name:        "local dir wins over the extracted artifact",
+			preRuns:     &config.EESTPreRunsSource{LocalFixturesDir: "/local"},
+			fixturesDir: "/artifact",
+			want:        func(string) string { return filepath.Join("/local", config.PreRunBundleSubdir) },
+		},
+		{
+			name:        "falls back to the extracted artifact",
+			preRuns:     &config.EESTPreRunsSource{FixturesSubdir: "a/b/pre_run_bundle"},
+			fixturesDir: "/artifact",
+			want:        func(string) string { return filepath.Join("/artifact", "a/b/pre_run_bundle") },
+		},
+		{
+			name:        "subdir defaults under the artifact",
+			preRuns:     &config.EESTPreRunsSource{},
+			fixturesDir: "/artifact",
+			want:        func(string) string { return filepath.Join("/artifact", config.PreRunBundleSubdir) },
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			s := &EESTSource{
+				log:         logrus.New(),
+				fixturesDir: tt.fixturesDir,
+				cfg:         &config.EESTFixturesSource{PreRuns: tt.preRuns},
+			}
+			assert.Equal(t, tt.want(tt.fixturesDir), s.PreRunBundleDir())
+		})
+	}
+
+	// The locator is what lets a caller outside this package find the bundle
+	// without re-deriving it from config.
+	var _ PreRunBundleLocator = (*EESTSource)(nil)
+}

pkg/executor/source.go

@@ -73,6 +73,18 @@ type Source interface {
 	GetSourceInfo() (*SuiteSource, error)
 }
 
+// PreRunBundleLocator is an optional interface for sources that resolve a
+// builder.pre_runs bundle. It exists so a caller can read the bundle's metadata
+// without re-deriving the path: the bundle may sit at a configured local
+// directory OR inside the fixtures artifact the source extracted, and only the
+// source knows which. Re-deriving it from config alone silently misses the
+// artifact case.
+type PreRunBundleLocator interface {
+	// PreRunBundleDir returns the directory holding the pre-run bundle, or an
+	// empty string when this source provides none.
+	PreRunBundleDir() string
+}
+
 // GenesisProvider is an optional interface that sources can implement
 // to provide genesis files for clients.
 type GenesisProvider interface {

pkg/runner/lifecycle.go

@@ -1649,6 +1649,38 @@ func copyStateActorFiles(
 // Anything in between is a partially applied bundle, which the replay resumes
 // from. Only a hash mismatch at a block the bundle actually names is an error —
 // that is the case that cannot be anything but the wrong chain.
+// preRunBundleDir locates the pre-run bundle for head verification.
+//
+// The source is asked first: the bundle may sit at a configured
+// local_fixtures_dir OR inside the fixtures artifact the source extracted, and
+// only the source knows which. Deriving the path from config alone resolves the
+// local case and silently misses the artifact one, which leaves the by-number
+// replay skip unguarded — a datadir at the right height on a different chain
+// then replays as a no-op and benchmarks the wrong state.
+//
+// The config-derived path remains as a fallback for callers without a live
+// source.
+func (r *runner) preRunBundleDir(preRuns *config.EESTPreRunsSource) string {
+	if r.executor != nil {
+		if loc, ok := r.executor.GetSource().(executor.PreRunBundleLocator); ok {
+			if dir := loc.PreRunBundleDir(); dir != "" {
+				return dir
+			}
+		}
+	}
+
+	if preRuns.LocalFixturesDir == "" {
+		return ""
+	}
+
+	subdir := preRuns.FixturesSubdir
+	if subdir == "" {
+		subdir = config.PreRunBundleSubdir
+	}
+
+	return filepath.Join(preRuns.LocalFixturesDir, subdir)
+}
+
 func (r *runner) verifyPreRunBundleHead(
 	log logrus.FieldLogger, headNumber uint64, headHash string,
 ) (bool, error) {
@@ -1657,11 +1689,22 @@ func (r *runner) verifyPreRunBundleHead(
 	}
 
 	src := r.cfg.FullConfig.Runner.Benchmark.Tests.Source.EESTFixtures
-	if src == nil || src.PreRuns == nil || src.PreRuns.LocalFixturesDir == "" {
+	if src == nil || src.PreRuns == nil {
+		return false, nil
+	}
+
+	bundleDir := r.preRunBundleDir(src.PreRuns)
+	if bundleDir == "" {
+		log.Warn(
+			"A pre_runs source is configured but its bundle could not be located; " +
+				"skipping head verification — the replay skips by block number alone, " +
+				"so a datadir at the right height on a different chain will not be caught",
+		)
+
 		return false, nil
 	}
 
-	info, err := builder.ReadPreRunBundleInfo(src.PreRuns.LocalFixturesDir)
+	info, err := builder.ReadPreRunBundleInfoAt(bundleDir)
 	if err != nil {
 		// Metadata is a convenience; an unreadable sidecar must not block a run
 		// that would otherwise replay correctly.

pkg/runner/prerun_head_test.go

@@ -11,6 +11,7 @@ import (
 	"github.com/stretchr/testify/require"
 
 	"github.com/ethpandaops/benchmarkoor/pkg/config"
+	"github.com/ethpandaops/benchmarkoor/pkg/executor"
 )
 
 // bundleAt writes a pre-run bundle sidecar describing blocks 100..181.
@@ -102,3 +103,81 @@ func TestVerifyPreRunBundleHead(t *testing.T) {
 		assert.False(t, applied)
 	})
 }
+
+// stubLocator is a source that reports where its bundle is, as an EEST source
+// resolving one out of an extracted fixtures artifact does.
+type stubLocator struct {
+	executor.Source
+	dir string
+}
+
+func (s stubLocator) PreRunBundleDir() string { return s.dir }
+
+// stubExecutor hands back a source; nothing else on the interface is exercised.
+type stubExecutor struct {
+	executor.Executor
+	src executor.Source
+}
+
+func (e stubExecutor) GetSource() executor.Source { return e.src }
+
+// The regression behind #305: with the bundle inside the fixtures artifact,
+// pre_runs carries no local_fixtures_dir, so a config-derived lookup finds
+// nothing and the head check silently no-ops — leaving the by-number replay
+// skip free to benchmark a datadir that is on a different chain.
+func TestVerifyPreRunBundleHeadFromFixturesArtifact(t *testing.T) {
+	log := logrus.New()
+	log.SetOutput(os.NewFile(0, os.DevNull))
+
+	dir := bundleAt(t)
+	bundleDir := filepath.Join(dir, config.PreRunBundleSubdir)
+
+	// pre_runs configured WITHOUT local_fixtures_dir, as an artifact-resolved
+	// source is.
+	r := &runner{
+		cfg: &Config{FullConfig: &config.Config{
+			Runner: config.RunnerConfig{
+				Benchmark: config.BenchmarkConfig{
+					Tests: config.TestsConfig{
+						Source: config.SourceConfig{
+							EESTFixtures: &config.EESTFixturesSource{
+								PreRuns: &config.EESTPreRunsSource{
+									FixturesSubdir: "does/not/matter",
+								},
+							},
+						},
+					},
+				},
+			},
+		}},
+		executor: stubExecutor{src: stubLocator{dir: bundleDir}},
+	}
+
+	t.Run("right height wrong hash is now rejected", func(t *testing.T) {
+		_, err := r.verifyPreRunBundleHead(log, 181, "0xdifferent")
+		require.Error(t, err)
+		assert.Contains(t, err.Error(), "different chain")
+	})
+
+	t.Run("matching head still reports already applied", func(t *testing.T) {
+		applied, err := r.verifyPreRunBundleHead(log, 181, "0xend")
+		require.NoError(t, err)
+		assert.True(t, applied)
+	})
+
+	t.Run("no locator and no local dir cannot verify", func(t *testing.T) {
+		bare := &runner{cfg: &Config{FullConfig: &config.Config{
+			Runner: config.RunnerConfig{Benchmark: config.BenchmarkConfig{
+				Tests: config.TestsConfig{Source: config.SourceConfig{
+					EESTFixtures: &config.EESTFixturesSource{
+						PreRuns: &config.EESTPreRunsSource{},
+					},
+				}},
+			}},
+		}}}
+
+		applied, err := bare.verifyPreRunBundleHead(log, 181, "0xend")
+		require.NoError(t, err)
+		assert.False(t, applied)
+	})
+}