From 9742333f680e392edfd3928e4f334e3a80a3aa0c Mon Sep 17 00:00:00 2001 From: Manu NALEPA Date: Wed, 15 Oct 2025 16:44:49 +0200 Subject: [PATCH] `WithDataColumnRetentionEpochs`: Use `dataColumnRetentionEpoch` instead of `blobColumnRetentionEpoch`. (#15872) --- beacon-chain/db/filesystem/data_column.go | 2 +- beacon-chain/db/filesystem/mock.go | 2 +- .../manu-data-column-retention-period.md | 2 + cmd/beacon-chain/storage/options.go | 27 ++++++++++++- cmd/beacon-chain/storage/options_test.go | 39 +++++++++++++++++++ 5 files changed, 69 insertions(+), 3 deletions(-) create mode 100644 changelog/manu-data-column-retention-period.md diff --git a/beacon-chain/db/filesystem/data_column.go b/beacon-chain/db/filesystem/data_column.go index 73d1edd65b..49331b4a01 100644 --- a/beacon-chain/db/filesystem/data_column.go +++ b/beacon-chain/db/filesystem/data_column.go @@ -1032,5 +1032,5 @@ func extractFileMetadata(path string) (*fileMetadata, error) { // period computes the period of a given epoch. func period(epoch primitives.Epoch) uint64 { - return uint64(epoch / params.BeaconConfig().MinEpochsForBlobsSidecarsRequest) + return uint64(epoch / params.BeaconConfig().MinEpochsForDataColumnSidecarsRequest) } diff --git a/beacon-chain/db/filesystem/mock.go b/beacon-chain/db/filesystem/mock.go index a7c2594fba..1a5eb2f40c 100644 --- a/beacon-chain/db/filesystem/mock.go +++ b/beacon-chain/db/filesystem/mock.go @@ -126,7 +126,7 @@ func NewWarmedEphemeralDataColumnStorageUsingFs(t testing.TB, fs afero.Fs, opts func NewEphemeralDataColumnStorageUsingFs(t testing.TB, fs afero.Fs, opts ...DataColumnStorageOption) *DataColumnStorage { opts = append(opts, - WithDataColumnRetentionEpochs(params.BeaconConfig().MinEpochsForBlobsSidecarsRequest), + WithDataColumnRetentionEpochs(params.BeaconConfig().MinEpochsForDataColumnSidecarsRequest), WithDataColumnFs(fs), ) diff --git a/changelog/manu-data-column-retention-period.md b/changelog/manu-data-column-retention-period.md new file mode 100644 index 0000000000..cec4f0b2c8 --- /dev/null +++ b/changelog/manu-data-column-retention-period.md @@ -0,0 +1,2 @@ +### Fixed +- `WithDataColumnRetentionEpochs`: Use `dataColumnRetentionEpoch` instead of `blobColumnRetentionEpoch`. \ No newline at end of file diff --git a/cmd/beacon-chain/storage/options.go b/cmd/beacon-chain/storage/options.go index bb2b96473a..598ef99a86 100644 --- a/cmd/beacon-chain/storage/options.go +++ b/cmd/beacon-chain/storage/options.go @@ -68,8 +68,13 @@ func BeaconNodeOptions(c *cli.Context) ([]node.Option, error) { filesystem.WithLayout(c.String(BlobStorageLayout.Name)), // This is validated in the Action func for BlobStorageLayout. ) + dataColumnRetentionEpoch, err := dataColumnRetentionEpoch(c) + if err != nil { + return nil, errors.Wrap(err, "data column retention epoch") + } + dataColumnStorageOption := node.WithDataColumnStorageOptions( - filesystem.WithDataColumnRetentionEpochs(blobRetentionEpoch), + filesystem.WithDataColumnRetentionEpochs(dataColumnRetentionEpoch), filesystem.WithDataColumnBasePath(dataColumnStoragePath(c)), ) @@ -116,6 +121,26 @@ func blobRetentionEpoch(cliCtx *cli.Context) (primitives.Epoch, error) { return re, nil } +// dataColumnRetentionEpoch returns the spec default MIN_EPOCHS_FOR_DATA_COLUMN_SIDECARS_REQUEST +// or a user-specified flag overriding this value. If a user-specified override is +// smaller than the spec default, an error will be returned. +func dataColumnRetentionEpoch(cliCtx *cli.Context) (primitives.Epoch, error) { + defaultValue := params.BeaconConfig().MinEpochsForDataColumnSidecarsRequest + if !cliCtx.IsSet(BlobRetentionEpochFlag.Name) { + return defaultValue, nil + } + + // We use on purpose the same retention flag for both blobs and data columns. + customValue := primitives.Epoch(cliCtx.Uint64(BlobRetentionEpochFlag.Name)) + + // Validate the epoch value against the spec default. + if customValue < defaultValue { + return defaultValue, errors.Wrapf(errInvalidBlobRetentionEpochs, "%s=%d, spec=%d", BlobRetentionEpochFlag.Name, customValue, defaultValue) + } + + return customValue, nil +} + func init() { BlobStorageLayout.Action = validateLayoutFlag } diff --git a/cmd/beacon-chain/storage/options_test.go b/cmd/beacon-chain/storage/options_test.go index b4bdcd79a1..7136a8911a 100644 --- a/cmd/beacon-chain/storage/options_test.go +++ b/cmd/beacon-chain/storage/options_test.go @@ -61,6 +61,45 @@ func TestConfigureBlobRetentionEpoch(t *testing.T) { _, err = blobRetentionEpoch(cliCtx) require.ErrorIs(t, err, errInvalidBlobRetentionEpochs) } + +func TestConfigureDataColumnRetentionEpoch(t *testing.T) { + specValue := params.BeaconConfig().MinEpochsForDataColumnSidecarsRequest + + app := cli.App{} + set := flag.NewFlagSet("test", 0) + cliCtx := cli.NewContext(&app, set, nil) + + // Test case: Specification value + expected := specValue + + actual, err := dataColumnRetentionEpoch(cliCtx) + require.NoError(t, err) + require.Equal(t, expected, actual) + + // Manually define the flag in the set, so the following code can use set.Set + set.Uint64(BlobRetentionEpochFlag.Name, 0, "") + + // Test case: Input epoch is greater than or equal to specification value. + expected = specValue + 1 + + err = set.Set(BlobRetentionEpochFlag.Name, fmt.Sprintf("%d", expected)) + require.NoError(t, err) + + actual, err = dataColumnRetentionEpoch(cliCtx) + require.NoError(t, err) + require.Equal(t, primitives.Epoch(expected), actual) + + // Test case: Input epoch is less than specification value. + expected = specValue - 1 + + err = set.Set(BlobRetentionEpochFlag.Name, fmt.Sprintf("%d", expected)) + require.NoError(t, err) + + actual, err = dataColumnRetentionEpoch(cliCtx) + require.ErrorIs(t, err, errInvalidBlobRetentionEpochs) + require.Equal(t, specValue, actual) +} + func TestDataColumnStoragePath_FlagSpecified(t *testing.T) { app := cli.App{} set := flag.NewFlagSet("test", 0)