diff --git a/pkg/beacon/state/scheduled_fork.go b/pkg/beacon/state/scheduled_fork.go index d871cf9..6153d3b 100644 --- a/pkg/beacon/state/scheduled_fork.go +++ b/pkg/beacon/state/scheduled_fork.go @@ -14,11 +14,16 @@ type ScheduledFork struct { // ForkScheduleFromForkEpochs returns a fork schedule from a list of forks. func ForkScheduleFromForkEpochs(forks ForkEpochs) ([]*ScheduledFork, error) { - // Sort them by Epoch. - sort.Slice(forks, func(i, j int) bool { - return (forks)[i].Epoch < (forks)[j].Epoch + // Sort a copy by Epoch so we don't reorder the caller's backing array. + sorted := make(ForkEpochs, len(forks)) + copy(sorted, forks) + + sort.Slice(sorted, func(i, j int) bool { + return sorted[i].Epoch < sorted[j].Epoch }) + forks = sorted + scheduled := make([]*ScheduledFork, 0, len(forks)) for i, fork := range forks { diff --git a/pkg/beacon/state/scheduled_fork_test.go b/pkg/beacon/state/scheduled_fork_test.go new file mode 100644 index 0000000..f4c36d4 --- /dev/null +++ b/pkg/beacon/state/scheduled_fork_test.go @@ -0,0 +1,73 @@ +package state + +import ( + "sync" + "testing" + + "github.com/ethpandaops/go-eth2-client/spec" + "github.com/ethpandaops/go-eth2-client/spec/phase0" + "github.com/stretchr/testify/assert" +) + +func TestForkScheduleFromForkEpochs_DoesNotMutateInput(t *testing.T) { + forks := ForkEpochs{ + {Epoch: 74240, Name: spec.DataVersionAltair, Version: "0x01000000"}, + {Epoch: 0, Name: spec.DataVersionPhase0, Version: "0x00000000"}, + {Epoch: 194048, Name: spec.DataVersionBellatrix, Version: "0x02000000"}, + } + + before := make([]spec.DataVersion, len(forks)) + for i, f := range forks { + before[i] = f.Name + } + + scheduled, err := ForkScheduleFromForkEpochs(forks) + assert.NoError(t, err) + assert.Len(t, scheduled, 3) + + after := make([]spec.DataVersion, len(forks)) + for i, f := range forks { + after[i] = f.Name + } + + assert.Equal(t, before, after, "ForkScheduleFromForkEpochs must not reorder the caller's slice") + + // The returned schedule should still be sorted ascending by epoch, + // independent of the input order. + assert.Equal(t, "0", scheduled[0].Epoch) + assert.Equal(t, "74240", scheduled[1].Epoch) + assert.Equal(t, "194048", scheduled[2].Epoch) +} + +func TestForkScheduleFromForkEpochs_ConcurrentWithReader(t *testing.T) { + forks := ForkEpochs{} + for i := range 50 { + forks = append(forks, &ForkEpoch{Epoch: phase0.Epoch(i * 1000), Name: spec.DataVersionPhase0}) + } + + var wg sync.WaitGroup + + // Writer: repeatedly builds a schedule from the shared slice, the same + // way a consumer rendering a fork schedule page would. + for range 4 { + wg.Go(func() { + for range 50 { + _, err := ForkScheduleFromForkEpochs(forks) + assert.NoError(t, err) + } + }) + } + + // Reader: the same access pattern ForkMetrics.calculateCurrent uses. + for range 4 { + wg.Go(func() { + for range 50 { + for _, fork := range forks { + _ = fork.Name.String() + } + } + }) + } + + wg.Wait() +}