diff --git a/cmd/integration/Readme.md b/cmd/integration/Readme.md index a711699e20d..72b0836ea1e 100644 --- a/cmd/integration/Readme.md +++ b/cmd/integration/Readme.md @@ -150,6 +150,23 @@ to the source; `--squeeze` is refused for a bin target. The run prints `commitme `rebuild_ranges` and `rebuild_shards` as tab-separated tables. Start a node on the output with `--experimental.bin-commitment` — the run writes the matching `erigondb.toml` there. +## Convert legacy binary-trie record files + +To convert a pre-version binary-trie datadir without changing the source, stage it into a separate +output datadir: + +```sh +integration commitment convert-format --datadir= --output.datadir= \ + --verify.sample=1000 +``` + +The command is one-way and leaves the source unchanged. The output must be separate from the source +and on the same filesystem because the staging step uses hardlinks. Current-format shards remain +hardlinked; legacy shards are replaced in the output. An interrupted run can be resumed with +`--resume`, and `--verify.sample=N` reads back every Nth converted legacy branch record (`0` +disables sampling). A single-cell input is invalid and can leave partial output; inspect and remove +that output before starting again rather than resuming it. + ## How to re-generate optional Domain/Index ```sh diff --git a/cmd/integration/commands/commitment.go b/cmd/integration/commands/commitment.go index 408f9be07af..1326220339c 100644 --- a/cmd/integration/commands/commitment.go +++ b/cmd/integration/commands/commitment.go @@ -141,6 +141,13 @@ func init() { withConvertFlags(cmdCommitmentConvert) commitmentCmd.AddCommand(cmdCommitmentConvert) + // commitment convert-format + withChain(cmdCommitmentConvertFormat) + withDataDir(cmdCommitmentConvertFormat) + withConfig(cmdCommitmentConvertFormat) + withConvertFormatFlags(cmdCommitmentConvertFormat) + commitmentCmd.AddCommand(cmdCommitmentConvertFormat) + // commitment visualize cmdCommitmentVisualize.Flags().StringVar(&visualizeOutputDir, "output", "", "existing directory to store output HTML. By default, same as commitment files") cmdCommitmentVisualize.Flags().IntVarP(&visualizeConcurrency, "concurrency", "j", 4, "amount of concurrently processed files") @@ -286,6 +293,13 @@ func requireRebuildOutput(target dbstate.RebuildTarget, outPath string) error { return nil } +func requireConvertFormatOutput(outPath string) error { + if outPath == "" { + return errors.New("commitment convert-format needs --output.datadir: the source datadir is a read-only input") + } + return nil +} + // refuseSqueezeForBinTarget rejects --squeeze for a bin rebuild. Squeeze rewrites // commitment values through BranchData, and a bin branch payload is not BranchData: // the same field bits name different things in the two encodings, so the pass would @@ -316,10 +330,24 @@ func refuseRebuildIntoBinSource(target dbstate.RebuildTarget, src datadir.Dirs) src.DataDir, target.Variant, source.TrieHashName()) } -func stageRebuildOutput(src datadir.Dirs, outPath string, target dbstate.RebuildTarget, resume bool, logger log.Logger) (*rebuildOutput, error) { +type stageRebuildOutputMode uint8 + +const ( + writeTargetSettings stageRebuildOutputMode = iota + preserveSourceSettings +) + +func stageRebuildOutput(src datadir.Dirs, outPath string, target dbstate.RebuildTarget, resume bool, logger log.Logger, modes ...stageRebuildOutputMode) (*rebuildOutput, error) { if outPath == "" { return nil, errors.New("commitment rebuild: empty output datadir") } + mode := writeTargetSettings + if len(modes) > 0 { + mode = modes[0] + } + if len(modes) > 1 { + return nil, errors.New("commitment rebuild: more than one output staging mode") + } // Nesting either way makes the hardlink walk descend into what it is creating. // Checked before datadir.New, which would create that tree inside the source. outDataDir := datadir.Open(outPath).DataDir @@ -329,6 +357,15 @@ func stageRebuildOutput(src datadir.Dirs, outPath string, target dbstate.Rebuild return nil, fmt.Errorf("commitment rebuild: output datadir %s overlaps the source datadir %s", outDataDir, src.DataDir) } out := datadir.New(outPath) + if !resume { + hasFiles, err := datadirHasFiles(out.DataDir) + if err != nil { + return nil, err + } + if hasFiles { + return nil, fmt.Errorf("commitment rebuild: output datadir %s is not empty; pass --resume to continue that run or point --output.datadir elsewhere", out.DataDir) + } + } existing, err := commitmentFilesIn(out.SnapDomain) if err != nil { @@ -344,29 +381,114 @@ func stageRebuildOutput(src datadir.Dirs, outPath string, target dbstate.Rebuild } o := &rebuildOutput{dirs: out, target: target, source: source} - if len(existing) > 0 { + if len(existing) > 0 && mode != preserveSourceSettings { if err := requireKeptFilesMatchTarget(out, o.settings()); err != nil { return nil, err } } + if resume { + if err := validateStagedOutput(src, out); err != nil { + return nil, err + } + } linked, err := linkSnapshotsExceptCommitment(src.Snap, out.Snap) if err != nil { return nil, err } - // The toml names the target before the rebuild starts, not after it finishes: - // the rebuild reopens this directory as a datadir, and the settings resolver - // refuses a bin run against a directory that reads as hex. It also leaves an - // interrupted run self-describing rather than passing its bin files off as hex. - if err := dbstate.WriteErigonDBSettings(out, o.settings()); err != nil { - return nil, err + if mode == preserveSourceSettings { + sourceSettingsPath := filepath.Join(src.Snap, dbstate.ERIGONDB_SETTINGS_FILE) + outputSettingsPath := filepath.Join(out.Snap, dbstate.ERIGONDB_SETTINGS_FILE) + settingsData, err := os.ReadFile(sourceSettingsPath) + if err != nil { + return nil, fmt.Errorf("commitment rebuild: read source erigondb.toml: %w", err) + } + if err := os.WriteFile(outputSettingsPath, settingsData, 0o644); err != nil { + return nil, fmt.Errorf("commitment rebuild: copy source erigondb.toml: %w", err) + } + } else { + // The toml names the target before the rebuild starts, not after it finishes: + // the rebuild reopens this directory as a datadir, and the settings resolver + // refuses a bin run against a directory that reads as hex. It also leaves an + // interrupted run self-describing rather than passing its bin files off as hex. + if err := dbstate.WriteErigonDBSettings(out, o.settings()); err != nil { + return nil, err + } } logger.Info("[commitment_rebuild] staged output datadir", "path", out.DataDir, "linkedFiles", linked, "keptCommitmentFiles", len(existing)) return o, nil } +func datadirHasFiles(root string) (bool, error) { + var hasFiles bool + err := filepath.WalkDir(root, func(path string, entry os.DirEntry, err error) error { + if err != nil { + return err + } + if path != root && !entry.IsDir() { + hasFiles = true + } + return nil + }) + return hasFiles, err +} + +func validateStagedOutput(src, out datadir.Dirs) error { + if _, err := os.Stat(out.DataDir); err != nil { + if os.IsNotExist(err) { + return nil + } + return err + } + return filepath.WalkDir(out.DataDir, func(path string, entry os.DirEntry, err error) error { + if err != nil { + return err + } + if entry.IsDir() { + return nil + } + rel, err := filepath.Rel(out.DataDir, path) + if err != nil { + return err + } + snapRel, err := filepath.Rel(out.Snap, path) + if err != nil { + return err + } + if snapRel == "." || strings.HasPrefix(snapRel, ".."+string(filepath.Separator)) { + return fmt.Errorf("commitment rebuild: unexpected file outside snapshots: %s", rel) + } + if snapRel == dbstate.ERIGONDB_SETTINGS_FILE { + return nil + } + commitmentRel, err := filepath.Rel(out.SnapDomain, path) + if err != nil { + return err + } + if commitmentRel != "." && !strings.HasPrefix(commitmentRel, ".."+string(filepath.Separator)) && isCommitmentFileName(entry.Name()) { + return nil + } + sourcePath := filepath.Join(src.Snap, snapRel) + sourceInfo, err := os.Stat(sourcePath) + if err != nil { + if os.IsNotExist(err) { + return fmt.Errorf("commitment rebuild: unexpected file in resumed output: %s", snapRel) + } + return err + } + outputInfo, err := entry.Info() + if err != nil { + return err + } + if !os.SameFile(sourceInfo, outputInfo) { + return fmt.Errorf("commitment rebuild: existing output file %s does not match source; remove it or restart with a clean output datadir", snapRel) + } + return nil + }) +} + // requireKeptFilesMatchTarget refuses a --resume run under a scheme other than the // one the kept commitment files were built with. Staging is about to overwrite the // toml that describes them, which is the only record of what they are. @@ -401,11 +523,11 @@ func (o *rebuildOutput) settings() *dbstate.ErigonDBSettings { // pathsOverlap reports whether either path is the other or contains it. func pathsOverlap(a, b string) (bool, error) { - absA, err := filepath.Abs(a) + absA, err := resolvePathForOverlap(a) if err != nil { return false, err } - absB, err := filepath.Abs(b) + absB, err := resolvePathForOverlap(b) if err != nil { return false, err } @@ -416,6 +538,33 @@ func pathsOverlap(a, b string) (bool, error) { strings.HasPrefix(absB, absA+string(filepath.Separator)), nil } +func resolvePathForOverlap(path string) (string, error) { + abs, err := filepath.Abs(path) + if err != nil { + return "", err + } + abs = filepath.Clean(abs) + var missing []string + for { + resolved, err := filepath.EvalSymlinks(abs) + if err == nil { + for _, part := range slices.Backward(missing) { + resolved = filepath.Join(resolved, part) + } + return filepath.Clean(resolved), nil + } + if !errors.Is(err, fs.ErrNotExist) { + return "", err + } + parent := filepath.Dir(abs) + if parent == abs { + return abs, nil + } + missing = append(missing, filepath.Base(abs)) + abs = parent + } +} + func isCommitmentFileName(name string) bool { return strings.Contains(name, kv.CommitmentDomain.String()) } @@ -562,7 +711,14 @@ func linkSnapshotsExceptCommitment(srcRoot, dstRoot string) (int, error) { if isCommitmentFileName(d.Name()) || d.Name() == dbstate.ERIGONDB_SETTINGS_FILE { return nil } - if _, err := os.Lstat(dst); err == nil { + if dstInfo, err := os.Lstat(dst); err == nil { + srcInfo, err := d.Info() + if err != nil { + return err + } + if !os.SameFile(srcInfo, dstInfo) { + return fmt.Errorf("commitment rebuild: existing output file %s does not match source; remove it or restart with a clean output datadir", rel) + } return nil } else if !os.IsNotExist(err) { return err @@ -576,6 +732,43 @@ func linkSnapshotsExceptCommitment(srcRoot, dstRoot string) (int, error) { return linked, err } +func linkCommitmentSnapshots(srcRoot, dstRoot string) (int, error) { + linked := 0 + err := filepath.WalkDir(srcRoot, func(p string, d os.DirEntry, err error) error { + if err != nil { + return err + } + rel, err := filepath.Rel(srcRoot, p) + if err != nil { + return err + } + dst := filepath.Join(dstRoot, rel) + if d.IsDir() { + if rel == "." { + return nil + } + return os.MkdirAll(dst, 0o755) + } + if !d.Type().IsRegular() { + return fmt.Errorf("commitment convert-format: %s is not a regular file; the output can only be staged from a tree the hardlink walk can reproduce", p) + } + if !isCommitmentFileName(d.Name()) { + return nil + } + if _, err := os.Lstat(dst); err == nil { + return nil + } else if !os.IsNotExist(err) { + return err + } + if err := os.Link(p, dst); err != nil { + return fmt.Errorf("commitment convert-format: hardlink %s: %w (the output datadir must be on the same filesystem as the source)", rel, err) + } + linked++ + return nil + }) + return linked, err +} + // integration commitment rebuild var cmdCommitmentRebuild = &cobra.Command{ Use: "rebuild", @@ -611,7 +804,7 @@ var cmdCommitmentRebuild = &cobra.Command{ var out *rebuildOutput if rebuildOutputDatadir != "" { - if out, err = stageRebuildOutput(datadir.New(datadirCli), rebuildOutputDatadir, target, resume, logger); err != nil { + if out, err = stageRebuildOutput(datadir.Open(datadirCli), rebuildOutputDatadir, target, resume, logger); err != nil { logger.Error(err.Error()) return } @@ -952,6 +1145,120 @@ func commitmentConvert(db kv.TemporalRwDB, ctx context.Context, logger log.Logge return dbstate.ConvertCommitmentFiles(ctx, acRo, opts, logger) } +// integration commitment convert-format +var cmdCommitmentConvertFormat = &cobra.Command{ + Use: "convert-format", + Short: "Rewrite binary-trie commitment .kv files into the current pbin record format", + Long: `Offline, one-way converter for a datadir built before the pbin record format +carried a version. It drops the touchMap/afterMap header and the per-field +lengths from every branch record, omits the prefix on storage leaves, and adds +the format byte to the trie state blob. + +Every rewritten record is read back at its own depth and compared before it is +written, so a record whose omitted prefix is not the derivable one fails the run +rather than shipping. + +An input record naming one cell triggers an intentional panic because it cannot +come from the pbin folding algorithm. That failure can leave partial output; +investigate the output and do not resume that run. + +The command requires --output.datadir. It stages the source tree there with +hardlinks, then replaces only legacy commitment files in the output. The source +datadir remains unchanged; the output must be separate from the source and on +the same filesystem. Files already in the current format stay hardlinked and +are left alone. + +Use --resume to continue an interrupted conversion. Complete output shards are +kept and incomplete shards are retried. Use --verify.sample=N to sequentially +read back every N-th converted legacy branch record; zero disables this check. +There is no backup or restore mode: remove the output datadir to discard it. + +Example: + integration commitment convert-format --datadir /path/to/source --output.datadir /path/to/output --chain mainnet --verify.sample=1000`, + Run: func(cmd *cobra.Command, args []string) { + logger, ctx := debug.SetupCobra(cmd, "integration"), cmd.Context() + if err := requireConvertFormatOutput(rebuildOutputDatadir); err != nil { + logger.Error(err.Error()) + return + } + + src := datadir.Open(datadirCli) + if err := requireConvertFormatSource(src); err != nil { + logger.Error(err.Error()) + return + } + out, err := stageRebuildOutput(src, rebuildOutputDatadir, dbstate.RebuildTarget{}, resume, logger, preserveSourceSettings) + if err != nil { + logger.Error(err.Error()) + return + } + if _, err := linkCommitmentSnapshots(src.Snap, out.dirs.Snap); err != nil { + logger.Error(err.Error()) + return + } + datadirCli = out.dirs.DataDir + + db, err := openDB(ctx, dbCfg(dbcfg.ChainDB, chaindata).Readonly(true), false, chain, logger) + if err != nil { + logger.Error("Opening DB", "error", err) + return + } + defer db.Close() + + if err := commitmentConvertFormat(db, ctx, logger); err != nil { + if !errors.Is(err, context.Canceled) { + logger.Error(err.Error()) + } + return + } + }, +} + +func requireConvertFormatSource(src datadir.Dirs) error { + settings, err := dbstate.ReadErigonDBSettings(src) + if err != nil { + return fmt.Errorf("commitment convert-format: read source erigondb.toml: %w", err) + } + if settings.TrieVariantName() != dbstate.TrieVariantBin { + return fmt.Errorf("commitment convert-format requires a binary-trie source datadir, got %s", settings.TrieVariantName()) + } + return requireNoCommitmentHistory(src) +} + +// Conversion rewrites domain .kv files only, so a datadir built with +// --keep.execution.proofs would keep serving pre-version records out of history. +func requireNoCommitmentHistory(src datadir.Dirs) error { + for _, root := range []string{src.SnapHistory, src.SnapIdx, src.SnapAccessors} { + entries, err := os.ReadDir(root) + if err != nil { + if os.IsNotExist(err) { + continue + } + return err + } + for _, e := range entries { + if e.IsDir() || !isCommitmentFileName(e.Name()) { + continue + } + return fmt.Errorf("commitment convert-format: %s is commitment history, which this command does not rewrite; convert a datadir without it", filepath.Join(root, e.Name())) + } + } + return nil +} + +func commitmentConvertFormat(db kv.TemporalRwDB, ctx context.Context, logger log.Logger) error { + agg := db.(dbstate.HasAgg).Agg().(*dbstate.Aggregator) + agg.PresetOfflineMerge() + agg.SetSnapshotBuildSema(semaphore.NewWeighted(int64(runtime.NumCPU()))) + agg.DisableAllDependencies() + defer agg.MadvNormal().DisableReadAhead() + + acRo := agg.BeginFilesRo() + defer acRo.Close() + + return dbstate.ConvertPBinRecordFiles(ctx, acRo, logger, convertFormatVerifySample) +} + // integration commitment visualize var cmdCommitmentVisualize = &cobra.Command{ Use: "visualize [files...]", diff --git a/cmd/integration/commands/commitment_output_test.go b/cmd/integration/commands/commitment_output_test.go index 83f9fa93624..9243ea93b43 100644 --- a/cmd/integration/commands/commitment_output_test.go +++ b/cmd/integration/commands/commitment_output_test.go @@ -24,6 +24,7 @@ import ( "github.com/stretchr/testify/require" + "github.com/erigontech/erigon/common/dir" "github.com/erigontech/erigon/common/log/v3" "github.com/erigontech/erigon/db/datadir" dbstate "github.com/erigontech/erigon/db/state" @@ -51,7 +52,10 @@ func sourceDatadirFixture(t *testing.T) datadir.Dirs { require.NoError(t, os.WriteFile(filepath.Join(dirs.SnapDomain, name), []byte(name), 0o644)) } require.NoError(t, os.WriteFile(filepath.Join(dirs.SnapHistory, "v1.0-accounts.0-64.v"), []byte("acc-hist"), 0o644)) + require.NoError(t, os.WriteFile(filepath.Join(dirs.SnapHistory, "v1.0-commitment.0-64.v"), []byte("com-hist"), 0o644)) require.NoError(t, os.WriteFile(filepath.Join(dirs.SnapIdx, "v1.0-commitment.0-64.ef"), []byte("com-idx"), 0o644)) + require.NoError(t, os.WriteFile(filepath.Join(dirs.SnapAccessors, "v1.0-commitment.0-64.vi"), []byte("com-vi"), 0o644)) + require.NoError(t, os.WriteFile(filepath.Join(dirs.SnapAccessors, "v1.0-commitment.0-64.efi"), []byte("com-efi"), 0o644)) require.NoError(t, os.WriteFile(filepath.Join(dirs.Snap, "salt-state.txt"), []byte("salt"), 0o644)) refs := true @@ -63,10 +67,19 @@ func sourceDatadirFixture(t *testing.T) datadir.Dirs { return dirs } -// binSourceDatadirFixture is a source datadir that records the bin trie. +// binSourceDatadirFixture is a source datadir that records the bin trie. It +// carries no commitment history: convert-format rewrites domain files only. func binSourceDatadirFixture(t *testing.T) datadir.Dirs { t.Helper() dirs := sourceDatadirFixture(t) + for _, p := range []string{ + filepath.Join(dirs.SnapHistory, "v1.0-commitment.0-64.v"), + filepath.Join(dirs.SnapIdx, "v1.0-commitment.0-64.ef"), + filepath.Join(dirs.SnapAccessors, "v1.0-commitment.0-64.vi"), + filepath.Join(dirs.SnapAccessors, "v1.0-commitment.0-64.efi"), + } { + require.NoError(t, dir.RemoveFile(p)) + } refs := false variant, hash := dbstate.TrieVariantBin, commitment.PBinHashBlake3 require.NoError(t, dbstate.WriteErigonDBSettings(dirs, &dbstate.ErigonDBSettings{ @@ -140,6 +153,28 @@ func TestRequireRebuildOutputForBinTarget(t *testing.T) { require.NoError(t, requireRebuildOutput(hex, "")) } +func TestRequireConvertFormatOutput(t *testing.T) { + require.ErrorContains(t, requireConvertFormatOutput(""), "--output.datadir") + require.NoError(t, requireConvertFormatOutput(t.TempDir())) +} + +func TestConvertFormatRegistersOutputFlags(t *testing.T) { + for _, name := range []string{"output.datadir", "resume", "verify.sample"} { + require.NotNil(t, cmdCommitmentConvertFormat.Flags().Lookup(name), name) + } +} + +func TestConvertFormatHelpDescribesOutputDatadirModel(t *testing.T) { + help := cmdCommitmentConvertFormat.Long + require.Contains(t, help, "--output.datadir") + require.Contains(t, help, "--resume") + require.Contains(t, help, "--verify.sample") + require.Contains(t, help, "datadir remains unchanged") + require.NotContains(t, help, "backup/domains") + require.NotContains(t, help, "--restore") + require.NotContains(t, help, "--continue") +} + func TestStageRebuildOutputLinksInputsAndOmitsCommitment(t *testing.T) { src := sourceDatadirFixture(t) out, err := stageRebuildOutput(src, filepath.Join(t.TempDir(), "out"), binTarget(t), false, log.New()) @@ -166,6 +201,53 @@ func TestStageRebuildOutputLinksInputsAndOmitsCommitment(t *testing.T) { require.NoError(t, err) _, err = os.Stat(filepath.Join(out.dirs.SnapIdx, "v1.0-commitment.0-64.ef")) require.ErrorIs(t, err, os.ErrNotExist) + _, err = os.Stat(filepath.Join(out.dirs.SnapHistory, "v1.0-commitment.0-64.v")) + require.ErrorIs(t, err, os.ErrNotExist) + _, err = os.Stat(filepath.Join(out.dirs.SnapAccessors, "v1.0-commitment.0-64.vi")) + require.ErrorIs(t, err, os.ErrNotExist) + _, err = os.Stat(filepath.Join(out.dirs.SnapAccessors, "v1.0-commitment.0-64.efi")) + require.ErrorIs(t, err, os.ErrNotExist) +} + +func TestLinkCommitmentSnapshotsLinksAllCommitmentFiles(t *testing.T) { + src := sourceDatadirFixture(t) + out, err := stageRebuildOutput(src, filepath.Join(t.TempDir(), "out"), dbstate.RebuildTarget{}, false, log.New(), preserveSourceSettings) + require.NoError(t, err) + + linked, err := linkCommitmentSnapshots(src.Snap, out.dirs.Snap) + require.NoError(t, err) + require.Equal(t, 6, linked) + require.NoError(t, filepath.WalkDir(src.Snap, func(srcPath string, entry os.DirEntry, err error) error { + if err != nil || entry.IsDir() || entry.Name() == dbstate.ERIGONDB_SETTINGS_FILE { + return err + } + require.True(t, entry.Type().IsRegular(), srcPath) + rel, err := filepath.Rel(src.Snap, srcPath) + require.NoError(t, err) + srcInfo, err := os.Stat(srcPath) + require.NoError(t, err) + outInfo, err := os.Stat(filepath.Join(out.dirs.Snap, rel)) + require.NoError(t, err) + require.True(t, os.SameFile(srcInfo, outInfo), "%s must be a hardlink", rel) + return nil + })) + + for _, name := range []string{ + "domain/v1.0-commitment.0-64.kv", + "domain/v1.0-commitment.0-64.kvi", + "history/v1.0-commitment.0-64.v", + "idx/v1.0-commitment.0-64.ef", + "accessor/v1.0-commitment.0-64.vi", + "accessor/v1.0-commitment.0-64.efi", + } { + srcPath := filepath.Join(src.Snap, name) + outPath := filepath.Join(out.dirs.Snap, name) + srcInfo, err := os.Stat(srcPath) + require.NoError(t, err) + outInfo, err := os.Stat(outPath) + require.NoError(t, err) + require.True(t, os.SameFile(srcInfo, outInfo), "%s must be a hardlink", name) + } } func TestStageRebuildOutputLeavesSourceIntact(t *testing.T) { @@ -198,12 +280,42 @@ func TestStageRebuildOutputRefusesExistingCommitmentFiles(t *testing.T) { require.Equal(t, "rebuilt", string(data)) } +func TestStageRebuildOutputRefusesExistingNonCommitmentFiles(t *testing.T) { + src := sourceDatadirFixture(t) + outPath := filepath.Join(t.TempDir(), "out") + require.NoError(t, os.MkdirAll(filepath.Join(outPath, "snapshots"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(outPath, "stale"), []byte("stale"), 0o644)) + + _, err := stageRebuildOutput(src, outPath, binTarget(t), false, log.New()) + require.ErrorContains(t, err, "is not empty") + require.ErrorContains(t, err, "--resume") +} + +func TestStageRebuildOutputResumeRefusesUnrelatedExistingFile(t *testing.T) { + src := sourceDatadirFixture(t) + outPath := filepath.Join(t.TempDir(), "out") + require.NoError(t, os.MkdirAll(filepath.Join(outPath, "snapshots"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(outPath, "snapshots", "stale"), []byte("stale"), 0o644)) + + _, err := stageRebuildOutput(src, outPath, binTarget(t), true, log.New()) + require.ErrorContains(t, err, "unexpected file in resumed output") +} + func TestStageRebuildOutputRefusesSourceAsOutput(t *testing.T) { src := sourceDatadirFixture(t) _, err := stageRebuildOutput(src, src.DataDir, binTarget(t), false, log.New()) require.Error(t, err) } +func TestStageRebuildOutputRefusesSymlinkedOutput(t *testing.T) { + src := sourceDatadirFixture(t) + outPath := filepath.Join(t.TempDir(), "out") + require.NoError(t, os.Symlink(src.DataDir, outPath)) + + _, err := stageRebuildOutput(src, outPath, binTarget(t), false, log.New()) + require.ErrorContains(t, err, "overlaps the source datadir") +} + // Staging creates the output tree before it walks the source, so an output nested // in the source would have the walk descend into what it is writing. func TestStageRebuildOutputRefusesNestedOutput(t *testing.T) { @@ -252,6 +364,67 @@ func TestRebuildOutputSettingsHexTargetCarriesSourceRefs(t *testing.T) { require.True(t, final.RefsInCommitmentBranches()) } +func TestConvertFormatOutputPreservesSourceSettings(t *testing.T) { + src := binSourceDatadirFixture(t) + settingsPath := filepath.Join(src.Snap, dbstate.ERIGONDB_SETTINGS_FILE) + sourceSettings, err := os.ReadFile(settingsPath) + require.NoError(t, err) + + out, err := stageRebuildOutput(src, filepath.Join(t.TempDir(), "out"), dbstate.RebuildTarget{}, false, log.New(), preserveSourceSettings) + require.NoError(t, err) + + outputSettings, err := os.ReadFile(filepath.Join(out.dirs.Snap, dbstate.ERIGONDB_SETTINGS_FILE)) + require.NoError(t, err) + require.Equal(t, sourceSettings, outputSettings) + + sourceInfo, err := os.Stat(settingsPath) + require.NoError(t, err) + outputInfo, err := os.Stat(filepath.Join(out.dirs.Snap, dbstate.ERIGONDB_SETTINGS_FILE)) + require.NoError(t, err) + require.False(t, os.SameFile(sourceInfo, outputInfo)) +} + +func TestConvertFormatStagingLeavesSourceSnapshotsUnchanged(t *testing.T) { + src := binSourceDatadirFixture(t) + before := snapshotTree(t, src.Snap) + + _, err := stageRebuildOutput(src, filepath.Join(t.TempDir(), "out"), dbstate.RebuildTarget{}, false, log.New(), preserveSourceSettings) + require.NoError(t, err) + + require.Equal(t, before, snapshotTree(t, src.Snap)) +} + +func TestConvertFormatRequiresBinarySource(t *testing.T) { + require.ErrorContains(t, requireConvertFormatSource(sourceDatadirFixture(t)), "requires a binary-trie") + require.NoError(t, requireConvertFormatSource(binSourceDatadirFixture(t))) +} + +func TestConvertFormatRefusesCommitmentHistory(t *testing.T) { + for _, planted := range []struct { + dir func(datadir.Dirs) string + name string + }{ + {func(d datadir.Dirs) string { return d.SnapHistory }, "v1.0-commitment.0-64.v"}, + {func(d datadir.Dirs) string { return d.SnapIdx }, "v1.0-commitment.0-64.ef"}, + {func(d datadir.Dirs) string { return d.SnapAccessors }, "v1.0-commitment.0-64.vi"}, + } { + src := binSourceDatadirFixture(t) + require.NoError(t, os.WriteFile(filepath.Join(planted.dir(src), planted.name), []byte{}, 0o644)) + require.ErrorContains(t, requireConvertFormatSource(src), "commitment history", planted.name) + } +} + +func TestStageRebuildOutputDoesNotCreateSourceMigrations(t *testing.T) { + src := sourceDatadirFixture(t) + require.NoError(t, dir.RemoveFile(src.Migrations)) + + _, err := stageRebuildOutput(src, filepath.Join(t.TempDir(), "out"), binTarget(t), false, log.New()) + require.NoError(t, err) + + _, err = os.Stat(src.Migrations) + require.ErrorIs(t, err, os.ErrNotExist) +} + // The output directory on its own is what a node is started on, so the settings // resolver must accept it under the bin flag that the source datadir refuses. func TestRebuildOutputStartsUnderTheBinFlag(t *testing.T) { @@ -275,12 +448,17 @@ func TestRebuildOutputStartsUnderTheBinFlag(t *testing.T) { func withBinCommitmentProcess(t *testing.T, hash string) { t.Helper() bin, prevHash, suite := statecfg.ExperimentalBinCommitment, statecfg.BinCommitmentHash, commitment.PBinHashSuiteName() + parallel := statecfg.ExperimentalParallelCommitment t.Cleanup(func() { statecfg.ExperimentalBinCommitment, statecfg.BinCommitmentHash = bin, prevHash + statecfg.ExperimentalParallelCommitment = parallel require.NoError(t, commitment.SetPBinHashSuite(suite)) }) statecfg.ExperimentalBinCommitment = true statecfg.BinCommitmentHash = hash + // The settings resolver refuses bin together with parallel, so a process-wide + // parallel default would make every bin case here fail on the combination. + statecfg.ExperimentalParallelCommitment = false } // The rebuild reopens the staged directory as a datadir before it writes a single diff --git a/cmd/integration/commands/flags.go b/cmd/integration/commands/flags.go index ae9032235a0..24b58c5dba1 100644 --- a/cmd/integration/commands/flags.go +++ b/cmd/integration/commands/flags.go @@ -65,6 +65,7 @@ var ( noHistory bool rebuildOutputDatadir string rebuildMaxShardSteps uint64 + convertFormatVerifySample uint64 erigondbDomainStepsInFrozenFile string syncCfg = ethconfig.Defaults.Sync @@ -155,6 +156,12 @@ func withRebuildOutputDatadir(cmd *cobra.Command) { must(cmd.MarkFlagDirname("output.datadir")) } +func withConvertFormatFlags(cmd *cobra.Command) { + withResume(cmd) + withRebuildOutputDatadir(cmd) + cmd.Flags().Uint64Var(&convertFormatVerifySample, "verify.sample", 0, "verify every N-th converted legacy branch record by sequential read-back; 0 disables sampling") +} + func withNoHistory(cmd *cobra.Command) { cmd.Flags().BoolVar(&noHistory, "no-history", false, "skip history regeneration and only rebuild commitment KV files") } diff --git a/db/seg/decompress.go b/db/seg/decompress.go index 5ecce9a3a24..d744dd75e58 100644 --- a/db/seg/decompress.go +++ b/db/seg/decompress.go @@ -195,7 +195,7 @@ type Decompressor struct { readAheadRefcnt atomic.Int32 // ref-counter: allow enable/disable read-ahead from goroutines. only when refcnt=0 - disable read-ahead once residency atomic.Pointer[residencyBitmap] // page-residency bitmap for the async-io gate; nil unless enabled - residencyOnce sync.Once + residencyOnce sync.Once //nolint:unused // used by the Linux-only residency gate } const ( diff --git a/db/state/commitment_convert_export_test.go b/db/state/commitment_convert_export_test.go index 6355aafd617..24963f8b8d9 100644 --- a/db/state/commitment_convert_export_test.go +++ b/db/state/commitment_convert_export_test.go @@ -16,6 +16,8 @@ package state +import "github.com/erigontech/erigon/execution/commitment" + // Test-only bridge: convertCommitmentFile and its sentinels are package-private, // but the full-aggregator round-trip tests live in package state_test (the // aggregator-setup helpers — testDbAggregatorWithFiles, etc. — are defined @@ -33,3 +35,24 @@ var ( func SetConvertPhase1AfterFileHookForTest(fn func(idx int)) { convertPhase1AfterFileHook = fn } + +func SetPBinConvertPairHookForTest(fn func()) { + pbinConvertPairHook = fn +} + +func SetPBinConvertDropPairHookForTest(fn func(pair uint64) bool) { + pbinConvertDropPairHook = fn +} + +func SetPBinConvertAfterBuildHookForTest(fn func(string)) { + pbinConvertAfterBuildHook = fn +} + +// CloseMappedFilesForTest drops the aggregator's file mmaps so a test can remove +// or rename the files underneath it; Windows refuses either while a mapping is +// open. ReloadFiles re-opens them. +func (a *Aggregator) CloseMappedFilesForTest() { a.closeDirtyFilesNoReopen() } + +func VerifyPBinStateConversionForTest(source, converted []byte) error { + return pbinVerifyStateConversion(commitment.NewPBinRecordConverter(), source, converted) +} diff --git a/db/state/commitment_convert_pbin.go b/db/state/commitment_convert_pbin.go new file mode 100644 index 00000000000..d63be271960 --- /dev/null +++ b/db/state/commitment_convert_pbin.go @@ -0,0 +1,509 @@ +// Copyright 2026 The Erigon Authors +// This file is part of Erigon. +// +// Erigon is free software: you can redistribute it and/or modify +// it under the terms of the GNU Lesser General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// Erigon is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Lesser General Public License for more details. +// +// You should have received a copy of the GNU Lesser General Public License +// along with Erigon. If not, see . + +package state + +import ( + "bytes" + "context" + "encoding/binary" + "errors" + "fmt" + "os" + "path/filepath" + "sort" + "strings" + + "github.com/erigontech/erigon/common/background" + "github.com/erigontech/erigon/common/dir" + "github.com/erigontech/erigon/common/log/v3" + "github.com/erigontech/erigon/db/kv" + "github.com/erigontech/erigon/db/seg" + "github.com/erigontech/erigon/db/state/statecfg" + "github.com/erigontech/erigon/execution/commitment" + "github.com/erigontech/erigon/execution/commitment/commitmentdb" +) + +var pbinConvertPairHook func() +var pbinConvertDropPairHook func(pair uint64) bool +var pbinConvertAfterBuildHook func(string) + +// A pre-version branch record opens with the high byte of its touchMap, always +// zero; a current one opens with a cell-fields byte, which always carries a kind +// bit. One byte separates the two formats without decoding either. +func pbinRecordIsLegacy(value []byte) bool { return len(value) > 0 && value[0] == 0 } + +func pbinStatePayload(value []byte) (payload []byte, wrapped bool, err error) { + if commitment.IsPBinState(value) { + return value, false, nil + } + if len(value) < 18 || !commitment.IsPBinState(value[18:]) { + return nil, false, fmt.Errorf("pbin state value has no state blob") + } + rootLen := int(binary.BigEndian.Uint16(value[16:18])) + if rootLen != len(value)-18 { + return nil, false, fmt.Errorf("pbin state value length %d does not match root length %d", len(value), rootLen) + } + return value[18:], true, nil +} + +func pbinConvertState(conv *commitment.PBinRecordConverter, value []byte) ([]byte, error) { + payload, wrapped, err := pbinStatePayload(value) + if err != nil { + return nil, err + } + if commitment.ValidatePBinStateFormat(payload) == nil { + return append([]byte(nil), value...), nil + } + converted, err := conv.ConvertState(payload) + if err != nil { + return nil, err + } + if !wrapped { + return converted, nil + } + out := append([]byte(nil), value[:18]...) + if len(converted) > 1<<16-1 { + return nil, fmt.Errorf("converted pbin state blob is too large: %d bytes", len(converted)) + } + binary.BigEndian.PutUint16(out[16:18], uint16(len(converted))) + return append(out, converted...), nil +} + +func pbinVerifyStateConversion(conv *commitment.PBinRecordConverter, source, converted []byte) error { + sourcePayload, _, err := pbinStatePayload(source) + if err != nil { + return err + } + var sourceRoot []byte + if commitment.ValidatePBinStateFormat(sourcePayload) == nil { + sourceRoot, err = conv.CurrentStateRoot(sourcePayload) + } else { + sourceRoot, err = conv.LegacyStateRoot(sourcePayload) + } + if err != nil { + return fmt.Errorf("read source state root: %w", err) + } + convertedPayload, _, err := pbinStatePayload(converted) + if err != nil { + return err + } + convertedRoot, err := conv.CurrentStateRoot(convertedPayload) + if err != nil { + return fmt.Errorf("read converted state root: %w", err) + } + if !bytes.Equal(sourceRoot, convertedRoot) { + return fmt.Errorf("pbin state root mismatch: source %x, converted %x", sourceRoot, convertedRoot) + } + return nil +} + +func verifyPBinPairCount(sourcePairs uint64, outputWords int) error { + if outputWords%2 != 0 { + return fmt.Errorf("pbin pair count: output has an odd word count %d", outputWords) + } + outputPairs := uint64(outputWords / 2) + if outputPairs != sourcePairs { + return fmt.Errorf("pbin pair count: source has %d pairs, output has %d pairs", sourcePairs, outputPairs) + } + return nil +} + +type pbinLegacySample struct { + pair uint64 + key []byte + legacy []byte +} + +func verifyPBinSamples(ctx context.Context, d *Domain, outputPath string, samples []pbinLegacySample) error { + if len(samples) == 0 { + return nil + } + + decompressor, err := seg.NewDecompressor(outputPath) + if err != nil { + return fmt.Errorf("open sampled pbin output: %w", err) + } + defer decompressor.Close() + + reader := d.dataReader(decompressor) + reader.Reset(0) + converter := commitment.NewPBinRecordConverter() + var key, value []byte + sampleIdx := 0 + for pair := uint64(0); reader.HasNext(); pair++ { + select { + case <-ctx.Done(): + return ctx.Err() + default: + } + + key, _ = reader.Next(key[:0]) + if !reader.HasNext() { + return fmt.Errorf("pbin sample read-back: output has no value at pair %d", pair) + } + value, _ = reader.Next(value[:0]) + if sampleIdx >= len(samples) || samples[sampleIdx].pair != pair { + continue + } + + sample := samples[sampleIdx] + if !bytes.Equal(key, sample.key) { + return fmt.Errorf("pbin sample read-back: output key at pair %d is %x, want %x", pair, key, sample.key) + } + if err := converter.CompareLegacy(sample.key, sample.legacy, value); err != nil { + return fmt.Errorf("pbin sample read-back at pair %d: %w", pair, err) + } + sampleIdx++ + } + if sampleIdx != len(samples) { + return fmt.Errorf("pbin sample read-back: output ended before sample at pair %d", samples[sampleIdx].pair) + } + return nil +} + +func pbinFileHasLegacy(ctx context.Context, d *Domain, file *FilesItem) (bool, error) { + reader := d.dataReader(file.decompressor) + reader.Reset(0) + var key, value []byte + for reader.HasNext() { + select { + case <-ctx.Done(): + return false, ctx.Err() + default: + } + key, _ = reader.Next(key[:0]) + if !reader.HasNext() { + return false, errors.New("truncated commitment file: value missing") + } + value, _ = reader.Next(value[:0]) + if bytes.Equal(key, commitmentdb.KeyCommitmentState) { + payload, _, err := pbinStatePayload(value) + if err != nil || commitment.ValidatePBinStateFormat(payload) != nil { + return true, nil + } + continue + } + if commitment.PBinIsRootKey(key) { + if commitment.PBinRootRecordIsLegacy(value) { + return true, nil + } + continue + } + if pbinRecordIsLegacy(value) { + return true, nil + } + } + return false, nil +} + +func commitmentOutputPaths(d *Domain, stepFrom, stepTo kv.Step, dirPath string) []string { + paths := []string{d.kvNewFilePathIn(dirPath, stepFrom, stepTo)} + if d.Accessors.Has(statecfg.AccessorBTree) { + paths = append(paths, d.kvBtAccessorNewFilePathIn(dirPath, stepFrom, stepTo)) + } + if d.Accessors.Has(statecfg.AccessorHashMap) { + paths = append(paths, d.kviAccessorNewFilePathIn(dirPath, stepFrom, stepTo)) + } + if d.Accessors.Has(statecfg.AccessorExistence) { + paths = append(paths, d.kvExistenceIdxNewFilePathIn(dirPath, stepFrom, stepTo)) + } + return paths +} + +// pbinConvertStageDir names the directory a converted shard is built in before it +// replaces the link in snapshots/domain. Building in place would mean unlinking a +// file the conversion is still reading through its mmap: legal on unix, refused +// outright on Windows. +const pbinConvertStageDir = "pbin_convert" + +// swapCommitmentOutputFiles moves a staged shard over the links it replaces. The +// caller must have dropped the mmaps on those links first. +func swapCommitmentOutputFiles(stagePaths, finalPaths []string) error { + if len(stagePaths) != len(finalPaths) { + return fmt.Errorf("pbin convert: %d staged paths for %d output paths", len(stagePaths), len(finalPaths)) + } + for i, stage := range stagePaths { + if _, err := os.Stat(stage); err != nil { + if errors.Is(err, os.ErrNotExist) { + continue + } + return fmt.Errorf("stat %s: %w", stage, err) + } + if err := os.Rename(stage, finalPaths[i]); err != nil { + return fmt.Errorf("move %s into place: %w", stage, err) + } + } + return nil +} + +func removeCommitmentOutputFiles(paths []string) error { + for _, path := range paths { + if err := dir.RemoveFile(path); err != nil && !errors.Is(err, os.ErrNotExist) { + return fmt.Errorf("remove %s: %w", path, err) + } + } + return nil +} + +func commitmentFilesForConversion(at *AggregatorRoTx) (VisibleFiles, error) { + d := at.d[kv.CommitmentDomain].d + filesByPath := make(map[string]VisibleFile) + d.dirtyFiles.Scan(func(item *FilesItem) bool { + if item.decompressor == nil || filepath.Ext(item.decompressor.FilePath()) != ".kv" { + return true + } + file := visibleFile{ + startTxNum: item.startTxNum, + endTxNum: item.endTxNum, + src: item, + } + filesByPath[filepath.Clean(item.decompressor.FilePath())] = file + return true + }) + + entries, err := os.ReadDir(d.dirs.SnapDomain) + if err != nil { + return nil, fmt.Errorf("enumerate commitment files: %w", err) + } + for _, entry := range entries { + if entry.IsDir() || filepath.Ext(entry.Name()) != ".kv" || !strings.Contains(entry.Name(), d.FilenameBase) { + continue + } + path := filepath.Clean(filepath.Join(d.dirs.SnapDomain, entry.Name())) + if _, ok := filesByPath[path]; !ok { + return nil, fmt.Errorf("commitment file %q is present on disk but is not readable", path) + } + } + + files := make(VisibleFiles, 0, len(filesByPath)) + for _, file := range filesByPath { + files = append(files, file) + } + sort.Slice(files, func(i, j int) bool { + if files[i].StartRootNum() != files[j].StartRootNum() { + return files[i].StartRootNum() < files[j].StartRootNum() + } + return files[i].Fullpath() < files[j].Fullpath() + }) + return files, nil +} + +func commitmentOutputComplete(paths []string) (bool, error) { + for _, path := range paths { + if _, err := os.Stat(path); err != nil { + if errors.Is(err, os.ErrNotExist) { + return false, nil + } + return false, fmt.Errorf("stat %s: %w", path, err) + } + } + return true, nil +} + +func convertPBinFile(ctx context.Context, at *AggregatorRoTx, file VisibleFile, logger log.Logger, verifySample uint64) (pairs uint64, err error) { + // Captured before the mmaps go: closing them leaves every VisibleFile handle + // pointing at a nil decompressor. + srcPath := file.Fullpath() + vf, ok := file.(visibleFile) + if !ok { + return 0, fmt.Errorf("convertPBinFile %q: VisibleFile is not state.visibleFile (got %T)", srcPath, file) + } + if vf.src == nil || vf.src.decompressor == nil { + return 0, fmt.Errorf("convertPBinFile %q: source has no decompressor", srcPath) + } + + d := at.d[kv.CommitmentDomain].d + stepSize := at.StepSize() + stepFrom, stepTo := kv.Step(file.StartRootNum()/stepSize), kv.Step(file.EndRootNum()/stepSize) + outputPath := d.kvNewFilePathIn(d.dirs.SnapDomain, stepFrom, stepTo) + if filepath.Base(outputPath) != filepath.Base(srcPath) { + return 0, fmt.Errorf("convertPBinFile %q: output basename %q does not match source basename %q", srcPath, filepath.Base(outputPath), filepath.Base(srcPath)) + } + stageDir := filepath.Join(d.dirs.Tmp, pbinConvertStageDir) + if err := os.MkdirAll(stageDir, 0o755); err != nil { + return 0, fmt.Errorf("convertPBinFile %q: create staging dir: %w", srcPath, err) + } + stagePath := d.kvNewFilePathIn(stageDir, stepFrom, stepTo) + stagePaths := commitmentOutputPaths(d, stepFrom, stepTo, stageDir) + paths := commitmentOutputPaths(d, stepFrom, stepTo, d.dirs.SnapDomain) + swapped := false + defer func() { + if cleanupErr := removeCommitmentOutputFiles(stagePaths); cleanupErr != nil && err == nil { + err = cleanupErr + } + // Until the swap the output still holds the source links, and those are + // what a resumed run reconverts from; only a half-done swap has to go. + if err != nil && swapped { + if cleanupErr := removeCommitmentOutputFiles(paths); cleanupErr != nil { + err = cleanupErr + } + } + }() + + hasLegacy, err := pbinFileHasLegacy(ctx, d, vf.src) + if err != nil { + return 0, fmt.Errorf("convertPBinFile %q: classify: %w", srcPath, err) + } + if !hasLegacy { + complete, err := commitmentOutputComplete(paths) + if err != nil { + return 0, fmt.Errorf("convertPBinFile %q: check output: %w", srcPath, err) + } + if complete { + return 0, errSkip + } + } + sourceWords := vf.src.decompressor.Count() + if sourceWords%2 != 0 { + return 0, fmt.Errorf("convertPBinFile %q: source has an odd word count %d", srcPath, sourceWords) + } + sourcePairs := uint64(sourceWords / 2) + + comp, err := seg.NewCompressor(ctx, "pbin_convert", stagePath, d.dirs.Tmp, d.CompressCfg, log.LvlTrace, logger) + if err != nil { + return 0, fmt.Errorf("convertPBinFile %q: create compressor: %w", srcPath, err) + } + compOwned := true + defer func() { + if compOwned { + comp.Close() + } + }() + writer := d.dataWriter(comp, false) + reader := d.dataReader(vf.src.decompressor) + reader.Reset(0) + converter := commitment.NewPBinRecordConverter() + var legacyBranches uint64 + var samples []pbinLegacySample + var key, value []byte + for reader.HasNext() { + key, _ = reader.Next(key[:0]) + if !reader.HasNext() { + return pairs, fmt.Errorf("convertPBinFile %q: truncated at pair %d (value missing)", srcPath, pairs) + } + value, _ = reader.Next(value[:0]) + if pbinConvertPairHook != nil { + pbinConvertPairHook() + } + select { + case <-ctx.Done(): + return pairs, ctx.Err() + default: + } + if pbinConvertDropPairHook != nil && pbinConvertDropPairHook(pairs) { + continue + } + var outputValue []byte + switch { + case bytes.Equal(key, commitmentdb.KeyCommitmentState): + outputValue, err = pbinConvertState(converter, value) + if err == nil { + err = pbinVerifyStateConversion(converter, value, outputValue) + } + case commitment.PBinIsRootKey(key) && commitment.PBinRootRecordIsLegacy(value): + outputValue, err = converter.ConvertRootRecord(value) + case pbinRecordIsLegacy(value): + outputValue, err = converter.ConvertBranch(key, value) + if err == nil { + legacyBranches++ + if verifySample > 0 && legacyBranches%verifySample == 0 { + samples = append(samples, pbinLegacySample{ + pair: pairs, + key: append([]byte(nil), key...), + legacy: append([]byte(nil), value...), + }) + } + } + default: + outputValue = append([]byte(nil), value...) + } + if err != nil { + return pairs, fmt.Errorf("convertPBinFile %q: pair %d key=%x: %w", srcPath, pairs, key, err) + } + if _, err = writer.Write(key); err != nil { + return pairs, fmt.Errorf("convertPBinFile %q: write key at pair %d: %w", srcPath, pairs, err) + } + if _, err = writer.Write(outputValue); err != nil { + return pairs, fmt.Errorf("convertPBinFile %q: write value at pair %d: %w", srcPath, pairs, err) + } + pairs++ + select { + case <-ctx.Done(): + return pairs, ctx.Err() + default: + } + } + + coll := Collation{valuesComp: comp, valuesPath: stagePath, valuesCount: comp.Count() / 2} + if err := verifyPBinPairCount(sourcePairs, coll.valuesComp.Count()); err != nil { + return pairs, fmt.Errorf("convertPBinFile %q: %w", srcPath, err) + } + static, err := d.buildFileRange(ctx, stepFrom, stepTo, coll, background.NewProgressSet(), stageDir) + compOwned = false + if err != nil { + return pairs, fmt.Errorf("convertPBinFile %q: build output: %w", srcPath, err) + } + // Releases the handles buildFileRange left open; the staged files cannot be + // moved while they are held. + static.CleanupOnError() + if pbinConvertAfterBuildHook != nil { + pbinConvertAfterBuildHook(stagePath) + } + if err := verifyPBinSamples(ctx, d, stagePath, samples); err != nil { + return pairs, fmt.Errorf("convertPBinFile %q: %w", srcPath, err) + } + + vf.src.closeFiles() + swapped = true + if err := removeCommitmentOutputFiles(paths); err != nil { + return pairs, fmt.Errorf("convertPBinFile %q: %w", srcPath, err) + } + if err := swapCommitmentOutputFiles(stagePaths, paths); err != nil { + return pairs, fmt.Errorf("convertPBinFile %q: %w", srcPath, err) + } + logger.Info("[pbin_convert] converted", "file", filepath.Base(srcPath), "pairs", pairs) + return pairs, nil +} + +// ConvertPBinRecordFiles rewrites pre-version pbin commitment files in the +// output datadir. Files already in the current format remain hardlinks to the +// source datadir; converted files replace those links before they are written. +func ConvertPBinRecordFiles(ctx context.Context, at *AggregatorRoTx, logger log.Logger, sampleStride uint64) error { + files, err := commitmentFilesForConversion(at) + if err != nil { + return err + } + if len(files) == 0 { + logger.Info("[pbin_convert] no commitment files to convert") + return nil + } + + names := make([]string, len(files)) + for i, file := range files { + names[i] = filepath.Base(file.Fullpath()) + } + for i, file := range files { + if _, err := convertPBinFile(ctx, at, file, logger, sampleStride); err != nil { + if errors.Is(err, errSkip) { + logger.Info("[pbin_convert] already current", "file", names[i]) + continue + } + return err + } + } + return nil +} diff --git a/db/state/commitment_convert_pbin_e2e_test.go b/db/state/commitment_convert_pbin_e2e_test.go new file mode 100644 index 00000000000..725d37d8f5c --- /dev/null +++ b/db/state/commitment_convert_pbin_e2e_test.go @@ -0,0 +1,238 @@ +// Copyright 2026 The Erigon Authors +// This file is part of Erigon. +// +// Erigon is free software: you can redistribute it and/or modify +// it under the terms of the GNU Lesser General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// Erigon is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Lesser General Public License for more details. +// +// You should have received a copy of the GNU Lesser General Public License +// along with Erigon. If not, see . + +package state_test + +import ( + "bytes" + "crypto/sha256" + "fmt" + "os" + "path/filepath" + "sort" + "strings" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/erigontech/erigon/common/dir" + "github.com/erigontech/erigon/common/log/v3" + "github.com/erigontech/erigon/db/datadir" + "github.com/erigontech/erigon/db/kv" + "github.com/erigontech/erigon/db/kv/rawdbv3" + "github.com/erigontech/erigon/db/state" + "github.com/erigontech/erigon/execution/commitment" + "github.com/erigontech/erigon/execution/commitment/commitmentdb" +) + +type e2ePBinFile struct { + keys [][]byte + values [][]byte +} + +func TestConvertPBinRecordFilesEndToEnd(t *testing.T) { + setPBinTestFlags(t) + db, source, sourceDirs := rebuildVariantDatadir(t) + + _, report, err := state.RebuildCommitmentFiles(t.Context(), db, &rawdbv3.TxNums, log.New(), false, + state.RebuildTarget{ + Variant: commitment.VariantBinPatriciaTrie, + HashName: commitment.PBinHashBlake3, + MaxShardSteps: 2, + }) + require.NoError(t, err) + require.NotEmpty(t, report.Ranges) + + view := source.BeginFilesRo() + files := view.Files(kv.CommitmentDomain) + view.Close() + require.Len(t, files, 2) + + settings, err := state.ReadErigonDBSettings(sourceDirs) + require.NoError(t, err) + variant, hash := state.TrieVariantBin, commitment.PBinHashBlake3 + settings.TrieVariant = &variant + settings.TrieHash = &hash + require.NoError(t, state.WriteErigonDBSettings(sourceDirs, settings)) + + paths := make([]string, 0, len(files)) + for _, file := range files { + paths = append(paths, file.Fullpath()) + } + for _, path := range paths { + rewritePBinFileAsLegacy(t, source, path) + } + + sourceFiles := make(map[string]e2ePBinFile, len(paths)) + for _, path := range paths { + keys, values := readKVFileWithCompression(t, path, source.Cfg(kv.CommitmentDomain).Compression) + sourceFiles[filepath.Base(path)] = e2ePBinFile{keys: keys, values: values} + } + + require.NoError(t, dir.RemoveAll(sourceDirs.Migrations)) + sourceChecksum := checksumDataDir(t, sourceDirs.DataDir) + tempBefore := regularFileSet(t, sourceDirs.Tmp) + + outputDirs := datadir.New(t.TempDir()) + linkSnapshotTree(t, sourceDirs.Snap, outputDirs.Snap) + outputSettings, err := state.ReadErigonDBSettings(outputDirs) + require.NoError(t, err) + output := state.NewTest(outputDirs). + StepSize(source.StepSize()). + WithErigonDBSettings(outputSettings). + Logger(log.New()). + MustOpen(t.Context(), db) + t.Cleanup(output.Close) + require.NoError(t, output.OpenFolder()) + + at := output.BeginFilesRo() + err = state.ConvertPBinRecordFiles(t.Context(), at, log.New(), 2) + at.Close() + require.NoError(t, err) + + assertE2EStagedNonCommitmentHardlinks(t, sourceDirs.Snap, outputDirs.Snap) + assertE2EConvertedPBinFiles(t, sourceFiles, output, sourceDirs.Snap, outputDirs.Snap) + + require.Equal(t, sourceChecksum, checksumDataDir(t, sourceDirs.DataDir)) + require.Equal(t, tempBefore, regularFileSet(t, sourceDirs.Tmp)) + _, err = os.Stat(sourceDirs.Migrations) + require.ErrorIs(t, err, os.ErrNotExist) +} + +func assertE2EStagedNonCommitmentHardlinks(t *testing.T, sourceRoot, outputRoot string) { + t.Helper() + require.NoError(t, filepath.WalkDir(sourceRoot, func(path string, entry os.DirEntry, walkErr error) error { + if walkErr != nil { + return walkErr + } + if entry.IsDir() || strings.Contains(entry.Name(), kv.CommitmentDomain.String()) { + return nil + } + rel, err := filepath.Rel(sourceRoot, path) + if err != nil { + return err + } + sourceInfo, err := os.Stat(path) + if err != nil { + return err + } + outputInfo, err := os.Stat(filepath.Join(outputRoot, rel)) + if err != nil { + return err + } + if !os.SameFile(sourceInfo, outputInfo) { + return fmt.Errorf("%s is not staged as a hardlink", rel) + } + return nil + })) +} + +func assertE2EConvertedPBinFiles(t *testing.T, sourceFiles map[string]e2ePBinFile, output *state.Aggregator, sourceRoot, outputRoot string) { + t.Helper() + converter := commitment.NewPBinRecordConverter() + sampledCells := 0 + stateRoots := 0 + for name, sourceFile := range sourceFiles { + outputPath := filepath.Join(output.Dirs().SnapDomain, name) + keys, values := readKVFileWithCompression(t, outputPath, output.Cfg(kv.CommitmentDomain).Compression) + require.Len(t, keys, len(sourceFile.keys), name) + require.Len(t, values, len(sourceFile.values), name) + for i, key := range sourceFile.keys { + require.Equal(t, key, keys[i], "%s pair %d key", name, i) + sourceValue, outputValue := sourceFile.values[i], values[i] + switch { + case bytes.Equal(key, commitmentdb.KeyCommitmentState): + require.NoError(t, state.VerifyPBinStateConversionForTest(sourceValue, outputValue), name) + stateRoots++ + case isPBinRootKey(key): + require.True(t, commitment.PBinRootRecordIsLegacy(sourceValue), "%s pair %d source root is not legacy", name, i) + require.False(t, commitment.PBinRootRecordIsLegacy(outputValue), "%s pair %d root remains legacy", name, i) + case len(sourceValue) > 0: + if i%2 == 0 { + require.NoError(t, converter.CompareLegacy(key, sourceValue, outputValue), "%s pair %d", name, i) + sampledCells++ + } + require.NotEqual(t, byte(0), outputValue[0], "%s pair %d remains legacy", name, i) + } + } + + sourceInfo, err := os.Stat(filepath.Join(sourceRoot, "domain", name)) + require.NoError(t, err) + outputInfo, err := os.Stat(outputPath) + require.NoError(t, err) + require.False(t, os.SameFile(sourceInfo, outputInfo), "%s must be replaced in the output", name) + } + require.Positive(t, sampledCells) + require.Positive(t, stateRoots) +} + +func checksumDataDir(t *testing.T, root string) [sha256.Size]byte { + t.Helper() + h := sha256.New() + err := filepath.WalkDir(root, func(path string, entry os.DirEntry, walkErr error) error { + if walkErr != nil { + return walkErr + } + if entry.IsDir() { + return nil + } + if !entry.Type().IsRegular() { + return fmt.Errorf("%s is not a regular file", path) + } + data, err := os.ReadFile(path) + if err != nil { + return err + } + rel, err := filepath.Rel(root, path) + if err != nil { + return err + } + _, _ = h.Write([]byte(rel)) + _, _ = h.Write([]byte{0}) + _, _ = h.Write(data) + _, _ = h.Write([]byte{0}) + return nil + }) + require.NoError(t, err) + var checksum [sha256.Size]byte + copy(checksum[:], h.Sum(nil)) + return checksum +} + +func regularFileSet(t *testing.T, root string) []string { + t.Helper() + var files []string + err := filepath.WalkDir(root, func(path string, entry os.DirEntry, walkErr error) error { + if walkErr != nil { + return walkErr + } + if entry.IsDir() { + return nil + } + if !entry.Type().IsRegular() { + return fmt.Errorf("%s is not a regular file", path) + } + rel, err := filepath.Rel(root, path) + if err != nil { + return err + } + files = append(files, rel) + return nil + }) + require.NoError(t, err) + sort.Strings(files) + return files +} diff --git a/db/state/commitment_convert_pbin_test.go b/db/state/commitment_convert_pbin_test.go new file mode 100644 index 00000000000..403ac5ef77a --- /dev/null +++ b/db/state/commitment_convert_pbin_test.go @@ -0,0 +1,653 @@ +// Copyright 2026 The Erigon Authors +// This file is part of Erigon. +// +// Erigon is free software: you can redistribute it and/or modify +// it under the terms of the GNU Lesser General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// Erigon is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Lesser General Public License for more details. +// +// You should have received a copy of the GNU Lesser General Public License +// along with Erigon. If not, see . + +package state_test + +import ( + "bytes" + "context" + "encoding/binary" + "fmt" + "os" + "path/filepath" + "strings" + "sync/atomic" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/erigontech/erigon/common/dir" + "github.com/erigontech/erigon/common/log/v3" + "github.com/erigontech/erigon/db/datadir" + "github.com/erigontech/erigon/db/kv" + "github.com/erigontech/erigon/db/kv/rawdbv3" + "github.com/erigontech/erigon/db/seg" + "github.com/erigontech/erigon/db/state" + "github.com/erigontech/erigon/db/state/statecfg" + "github.com/erigontech/erigon/execution/commitment" + "github.com/erigontech/erigon/execution/commitment/commitmentdb" +) + +type pbinOutputFixture struct { + db kv.TemporalRwDB + source *state.Aggregator + output *state.Aggregator + sourcePath string + outputPath string + sourceBytes []byte +} + +func newPBinOutputFixture(t *testing.T, legacy bool, smallOnly bool) pbinOutputFixture { + t.Helper() + setPBinTestFlags(t) + + db, source, _ := rebuildVariantDatadir(t) + _, _, err := state.RebuildCommitmentFiles(t.Context(), db, &rawdbv3.TxNums, log.New(), false, + state.RebuildTarget{Variant: commitment.VariantBinPatriciaTrie, HashName: commitment.PBinHashBlake3}) + require.NoError(t, err) + + sourceView := source.BeginFilesRo() + files := sourceView.Files(kv.CommitmentDomain) + sourceView.Close() + require.NotEmpty(t, files) + var selected kv.VisibleFile + for _, file := range files { + span := file.EndRootNum() - file.StartRootNum() + if (smallOnly && span < state.DomainMinStepsToCompress) || + (!smallOnly && (selected == nil || span > selected.EndRootNum()-selected.StartRootNum())) { + selected = file + } + } + require.NotNil(t, selected) + selectedPath := selected.Fullpath() + + if legacy { + rewritePBinFileAsLegacy(t, source, selectedPath) + } + + sourceBytes, err := os.ReadFile(selectedPath) + require.NoError(t, err) + + outputDirs := datadir.New(t.TempDir()) + linkSnapshotTree(t, source.Dirs().Snap, outputDirs.Snap) + keepOnlyCommitmentRange(t, outputDirs.SnapDomain, filepath.Base(selectedPath)) + settings, err := state.ReadErigonDBSettings(source.Dirs()) + require.NoError(t, err) + output := state.NewTest(outputDirs). + StepSize(source.StepSize()). + WithErigonDBSettings(settings). + Logger(log.New()). + MustOpen(t.Context(), db) + // Windows refuses to unlink a mapped file, so the mmaps must go before + // t.TempDir's own cleanup runs. + t.Cleanup(output.Close) + t.Cleanup(source.Close) + require.NoError(t, output.OpenFolder()) + if legacy { + keys, values := readKVFile(t, output, filepath.Join(outputDirs.SnapDomain, filepath.Base(selectedPath))) + legacyCount := 0 + for i, key := range keys { + if !bytes.Equal(key, commitmentdb.KeyCommitmentState) && isPBinRootKey(key) { + continue + } + if len(values[i]) > 0 && values[i][0] == 0 { + legacyCount++ + } + } + require.Positive(t, legacyCount) + } + + return pbinOutputFixture{ + db: db, + source: source, + output: output, + sourcePath: selectedPath, + outputPath: filepath.Join(outputDirs.SnapDomain, filepath.Base(selectedPath)), + sourceBytes: sourceBytes, + } +} + +func keepOnlyCommitmentRange(t *testing.T, dirPath, selectedName string) { + t.Helper() + _, suffix, ok := strings.Cut(selectedName, "-commitment.") + require.True(t, ok) + rangeName := strings.TrimSuffix(suffix, filepath.Ext(suffix)) + entries, err := os.ReadDir(dirPath) + require.NoError(t, err) + for _, entry := range entries { + if strings.Contains(entry.Name(), "-commitment.") && !strings.Contains(entry.Name(), "."+rangeName+".") { + require.NoError(t, dir.RemoveFile(filepath.Join(dirPath, entry.Name()))) + } + } +} + +func setPBinTestFlags(t *testing.T) { + t.Helper() + oldBin := statecfg.ExperimentalBinCommitment + oldHash := statecfg.BinCommitmentHash + oldSuite := commitment.PBinHashSuiteName() + t.Cleanup(func() { + statecfg.ExperimentalBinCommitment = oldBin + statecfg.BinCommitmentHash = oldHash + require.NoError(t, commitment.SetPBinHashSuite(oldSuite)) + }) + statecfg.ExperimentalBinCommitment = true + statecfg.BinCommitmentHash = commitment.PBinHashBlake3 + require.NoError(t, commitment.SetPBinHashSuite(commitment.PBinHashBlake3)) +} + +// Takes a path, not a kv.VisibleFile: dropping the mmaps invalidates every handle +// the caller is still holding. +func rewritePBinFileAsLegacy(t *testing.T, agg *state.Aggregator, path string) { + t.Helper() + cfg := agg.Cfg(kv.CommitmentDomain) + compression := cfg.Compression + keys, values := readKVFileWithCompression(t, path, compression) + agg.CloseMappedFilesForTest() + require.NoError(t, dir.RemoveFile(path)) + + comp, err := seg.NewCompressor(t.Context(), "pbin legacy fixture", path, agg.Dirs().Tmp, cfg.CompressCfg, log.LvlDebug, log.New()) + require.NoError(t, err) + w := seg.NewWriter(comp, cfg.Compression) + for i := range keys { + value := values[i] + switch { + case bytes.Equal(keys[i], commitmentdb.KeyCommitmentState): + value, err = legacyPBinStateValue(value) + case isPBinRootKey(keys[i]): + value, err = commitment.PBinEncodeLegacyRootRecord(value) + case len(value) > 0: + value, err = commitment.PBinEncodeLegacyRecord(keys[i], value) + } + require.NoError(t, err) + _, err = w.Write(keys[i]) + require.NoError(t, err) + _, err = w.Write(value) + require.NoError(t, err) + } + require.NoError(t, comp.Compress()) + comp.Close() + require.NoError(t, agg.ReloadFiles()) +} + +func legacyPBinStateValue(value []byte) ([]byte, error) { + if commitment.IsPBinState(value) { + return commitment.PBinEncodeLegacyState(value) + } + if len(value) < 18 || !commitment.IsPBinState(value[18:]) { + return nil, fmt.Errorf("unexpected pbin state value %x", value) + } + legacy, err := commitment.PBinEncodeLegacyState(value[18:]) + if err != nil { + return nil, err + } + out := append([]byte(nil), value[:18]...) + binary.BigEndian.PutUint16(out[16:18], uint16(len(legacy))) + return append(out, legacy...), nil +} + +func isPBinRootKey(key []byte) bool { + return len(key) == 1 && key[0] == 0x08 +} + +func readKVFileWithCompression(t *testing.T, path string, compression seg.FileCompression) ([][]byte, [][]byte) { + t.Helper() + d, err := seg.NewDecompressor(path) + require.NoError(t, err) + defer d.Close() + r := seg.NewReader(d.MakeGetter(), compression) + r.Reset(0) + var keys, values [][]byte + for r.HasNext() { + key, _ := r.Next(nil) + require.True(t, r.HasNext(), "value missing for key in %s", path) + value, _ := r.Next(nil) + keys = append(keys, append([]byte(nil), key...)) + values = append(values, append([]byte(nil), value...)) + } + return keys, values +} + +func linkSnapshotTree(t *testing.T, source, output string) { + t.Helper() + require.NoError(t, filepath.WalkDir(source, func(path string, entry os.DirEntry, walkErr error) error { + if walkErr != nil { + return walkErr + } + rel, err := filepath.Rel(source, path) + if err != nil { + return err + } + dst := filepath.Join(output, rel) + if entry.IsDir() { + return os.MkdirAll(dst, 0o755) + } + return os.Link(path, dst) + })) +} + +func convertPBinOutputFixture(t *testing.T, fixture pbinOutputFixture) error { + t.Helper() + at := fixture.output.BeginFilesRo() + defer at.Close() + return state.ConvertPBinRecordFiles(t.Context(), at, log.New(), 0) +} + +func convertPBinOutputFixtureWithSample(t *testing.T, fixture pbinOutputFixture, sample uint64) error { + t.Helper() + at := fixture.output.BeginFilesRo() + defer at.Close() + return state.ConvertPBinRecordFiles(t.Context(), at, log.New(), sample) +} + +func TestConvertPBinRecordFilesKeepsCurrentHardlink(t *testing.T) { + fixture := newPBinOutputFixture(t, false, false) + require.NoError(t, convertPBinOutputFixture(t, fixture)) + + sourceInfo, err := os.Stat(fixture.sourcePath) + require.NoError(t, err) + outputInfo, err := os.Stat(fixture.outputPath) + require.NoError(t, err) + require.True(t, os.SameFile(sourceInfo, outputInfo)) + require.Equal(t, fixture.sourceBytes, readFileBytes(t, fixture.sourcePath)) +} + +func TestConvertPBinRecordFilesReplacesLegacyHardlink(t *testing.T) { + fixture := newPBinOutputFixture(t, true, false) + sourceInfoBefore, err := os.Stat(fixture.sourcePath) + require.NoError(t, err) + require.NoError(t, convertPBinOutputFixture(t, fixture)) + + sourceInfoAfter, err := os.Stat(fixture.sourcePath) + require.NoError(t, err) + outputInfo, err := os.Stat(fixture.outputPath) + require.NoError(t, err) + require.False(t, os.SameFile(sourceInfoAfter, outputInfo)) + require.Equal(t, fixture.sourceBytes, readFileBytes(t, fixture.sourcePath)) + require.True(t, sourceInfoBefore.ModTime().Equal(sourceInfoAfter.ModTime())) + + keys, values := readKVFile(t, fixture.output, fixture.outputPath) + require.NotEmpty(t, keys) + for i, key := range keys { + if bytes.Equal(key, commitmentdb.KeyCommitmentState) { + require.NoError(t, validatePBinStateValue(values[i])) + continue + } + require.NotEmpty(t, values[i]) + require.NotEqual(t, byte(0), values[i][0]) + } +} + +func validatePBinStateValue(value []byte) error { + if commitment.ValidatePBinStateFormat(value) == nil { + return nil + } + if len(value) < 18 { + return fmt.Errorf("short pbin state value") + } + return commitment.ValidatePBinStateFormat(value[18:]) +} + +func TestConvertPBinRecordFilesRejectsOutputBasenameChange(t *testing.T) { + fixture := newPBinOutputFixture(t, true, false) + fixture.output.ForTestReferencesInCommitmentBranches(kv.CommitmentDomain, true) + + err := convertPBinOutputFixture(t, fixture) + require.Error(t, err) + require.Contains(t, err.Error(), "basename") + require.Equal(t, fixture.sourceBytes, readFileBytes(t, fixture.sourcePath)) +} + +func TestConvertPBinRecordFilesUsesDomainCodecForSmallShard(t *testing.T) { + fixture := newPBinOutputFixture(t, true, true) + require.NoError(t, convertPBinOutputFixture(t, fixture)) + + keys, values := readKVFile(t, fixture.output, fixture.outputPath) + require.NotEmpty(t, keys) + require.Len(t, values, len(keys)) +} + +func TestConvertPBinRecordFilesRejectsDroppedRecord(t *testing.T) { + fixture := newPBinOutputFixture(t, true, false) + state.SetPBinConvertDropPairHookForTest(func(pair uint64) bool { return pair == 1 }) + t.Cleanup(func() { + state.SetPBinConvertDropPairHookForTest(nil) + }) + + err := convertPBinOutputFixture(t, fixture) + require.Error(t, err) + require.Contains(t, err.Error(), "pair count") + require.Equal(t, fixture.sourceBytes, readFileBytes(t, fixture.sourcePath)) + assertPBinOutputRemoved(t, fixture) +} + +func TestConvertPBinRecordFilesRejectsMangledStateRoot(t *testing.T) { + fixture := newPBinOutputFixture(t, true, false) + keys, values := readKVFile(t, fixture.output, fixture.outputPath) + var legacy []byte + for i, key := range keys { + if bytes.Equal(key, commitmentdb.KeyCommitmentState) { + legacy = values[i] + break + } + } + require.NotEmpty(t, legacy) + + converter := commitment.NewPBinRecordConverter() + var current []byte + if commitment.IsPBinState(legacy) { + var err error + current, err = converter.ConvertState(legacy) + require.NoError(t, err) + } else { + require.GreaterOrEqual(t, len(legacy), 18) + converted, err := converter.ConvertState(legacy[18:]) + require.NoError(t, err) + current = append([]byte(nil), legacy[:18]...) + binary.BigEndian.PutUint16(current[16:18], uint16(len(converted))) + current = append(current, converted...) + } + require.Greater(t, len(current), 5) + mangled := append([]byte(nil), current...) + mangled[len(mangled)-1] ^= 1 + + err := state.VerifyPBinStateConversionForTest(legacy, mangled) + require.Error(t, err) + require.Contains(t, err.Error(), "state root") +} + +func TestConvertPBinRecordFilesRemovesOutputAfterStateFailure(t *testing.T) { + fixture := newPBinOutputFixture(t, true, false) + keys, values := readKVFile(t, fixture.output, fixture.outputPath) + for i, key := range keys { + if bytes.Equal(key, commitmentdb.KeyCommitmentState) { + values[i] = []byte{0} + break + } + } + rewritePBinFile(t, fixture, keys, values) + require.NoError(t, fixture.output.ReloadFiles()) + + err := convertPBinOutputFixture(t, fixture) + require.Error(t, err) + require.Equal(t, fixture.sourceBytes, readFileBytes(t, fixture.sourcePath)) + assertPBinOutputRemoved(t, fixture) + restagePBinOutputFromSource(t, fixture) + require.NoError(t, fixture.output.ReloadFiles()) + require.NoError(t, convertPBinOutputFixture(t, fixture)) + assertPBinOutputComplete(t, fixture) +} + +func TestConvertPBinRecordFilesPanicsOnSingleCellWithoutChangingSource(t *testing.T) { + fixture := newPBinOutputFixture(t, true, false) + keys, values := readKVFile(t, fixture.output, fixture.outputPath) + replaced := false + for i, key := range keys { + if bytes.Equal(key, commitmentdb.KeyCommitmentState) || isPBinRootKey(key) || len(values[i]) == 0 { + continue + } + values[i] = []byte{0, 1, 0, 1, 2, 0} + replaced = true + break + } + require.True(t, replaced, "fixture has no branch record") + rewritePBinFile(t, fixture, keys, values) + require.NoError(t, fixture.output.ReloadFiles()) + + require.Panics(t, func() { _ = convertPBinOutputFixture(t, fixture) }) + require.Equal(t, fixture.sourceBytes, readFileBytes(t, fixture.sourcePath)) + assertPBinOutputRemoved(t, fixture) +} + +func TestConvertPBinRecordFilesCancellationLeavesRunResumable(t *testing.T) { + fixture := newPBinOutputFixture(t, true, false) + ctx, cancel := context.WithCancel(t.Context()) + var pairs atomic.Int32 + state.SetPBinConvertPairHookForTest(func() { + if pairs.Add(1) == 2 { + cancel() + } + }) + t.Cleanup(func() { + state.SetPBinConvertPairHookForTest(nil) + cancel() + }) + + at := fixture.output.BeginFilesRo() + err := state.ConvertPBinRecordFiles(ctx, at, log.New(), 0) + at.Close() + require.ErrorIs(t, err, context.Canceled) + require.Equal(t, fixture.sourceBytes, readFileBytes(t, fixture.sourcePath)) + assertPBinOutputRemoved(t, fixture) + + state.SetPBinConvertPairHookForTest(nil) + require.NoError(t, fixture.output.ReloadFiles()) + require.NoError(t, convertPBinOutputFixture(t, fixture)) + require.NoError(t, fixture.output.ReloadFiles()) + require.Equal(t, fixture.sourceBytes, readFileBytes(t, fixture.sourcePath)) + assertPBinOutputComplete(t, fixture) +} + +func TestConvertPBinRecordFilesResumeRebuildsIncompleteShard(t *testing.T) { + fixture := newPBinOutputFixture(t, true, false) + require.NoError(t, convertPBinOutputFixture(t, fixture)) + require.NoError(t, fixture.output.ReloadFiles()) + + convertedInfo, err := os.Stat(fixture.outputPath) + require.NoError(t, err) + require.NoError(t, convertPBinOutputFixture(t, fixture)) + skippedInfo, err := os.Stat(fixture.outputPath) + require.NoError(t, err) + require.True(t, os.SameFile(convertedInfo, skippedInfo)) + + removePBinOutputAccessors(t, fixture) + require.NoError(t, fixture.output.ReloadFiles()) + at := fixture.output.BeginFilesRo() + require.Empty(t, at.Files(kv.CommitmentDomain), "an incomplete shard must not be visible") + at.Close() + + require.NoError(t, convertPBinOutputFixture(t, fixture)) + require.NoError(t, fixture.output.ReloadFiles()) + rebuiltInfo, err := os.Stat(fixture.outputPath) + require.NoError(t, err) + require.False(t, os.SameFile(convertedInfo, rebuiltInfo)) + require.Equal(t, fixture.sourceBytes, readFileBytes(t, fixture.sourcePath)) + assertPBinOutputComplete(t, fixture) +} + +func TestConvertPBinRecordFilesSampleRejectsWrongKey(t *testing.T) { + fixture := newPBinOutputFixture(t, true, false) + state.SetPBinConvertAfterBuildHookForTest(func(path string) { + rewritePBinFileWithWrongBranchKey(t, fixture, path) + }) + t.Cleanup(func() { + state.SetPBinConvertAfterBuildHookForTest(nil) + }) + + err := convertPBinOutputFixtureWithSample(t, fixture, 1) + require.Error(t, err) + require.Contains(t, err.Error(), "sample") + require.Equal(t, fixture.sourceBytes, readFileBytes(t, fixture.sourcePath)) + assertPBinOutputRemoved(t, fixture) +} + +func TestConvertPBinRecordFilesSampleZeroDisablesReadBack(t *testing.T) { + fixture := newPBinOutputFixture(t, true, false) + state.SetPBinConvertAfterBuildHookForTest(func(path string) { + rewritePBinFileWithWrongBranchKey(t, fixture, path) + }) + t.Cleanup(func() { + state.SetPBinConvertAfterBuildHookForTest(nil) + }) + + require.NoError(t, convertPBinOutputFixtureWithSample(t, fixture, 0)) + require.Equal(t, fixture.sourceBytes, readFileBytes(t, fixture.sourcePath)) + assertPBinOutputComplete(t, fixture) +} + +func TestConvertPBinRecordFilesSamplesOnlyLegacyBranches(t *testing.T) { + fixture := newPBinOutputFixture(t, true, false) + state.SetPBinConvertAfterBuildHookForTest(func(path string) { + rewritePBinFileWithWrongRootKey(t, fixture, path) + }) + t.Cleanup(func() { + state.SetPBinConvertAfterBuildHookForTest(nil) + }) + + require.NoError(t, convertPBinOutputFixtureWithSample(t, fixture, 1)) + require.Equal(t, fixture.sourceBytes, readFileBytes(t, fixture.sourcePath)) + assertPBinOutputComplete(t, fixture) +} + +func rewritePBinFile(t *testing.T, fixture pbinOutputFixture, keys, values [][]byte) { + t.Helper() + config := fixture.output.Cfg(kv.CommitmentDomain) + fixture.output.CloseMappedFilesForTest() + require.NoError(t, dir.RemoveFile(fixture.outputPath)) + comp, err := seg.NewCompressor(t.Context(), "pbin test rewrite", fixture.outputPath, fixture.output.Dirs().Tmp, config.CompressCfg, log.LvlDebug, log.New()) + require.NoError(t, err) + writer := seg.NewWriter(comp, config.Compression) + for i := range keys { + _, err = writer.Write(keys[i]) + require.NoError(t, err) + _, err = writer.Write(values[i]) + require.NoError(t, err) + } + require.NoError(t, comp.Compress()) + comp.Close() +} + +func rewritePBinFileWithWrongBranchKey(t *testing.T, fixture pbinOutputFixture, path string) { + t.Helper() + keys, values := readKVFile(t, fixture.output, path) + for i, key := range keys { + if bytes.Equal(key, commitmentdb.KeyCommitmentState) || isPBinRootKey(key) || len(values[i]) == 0 { + continue + } + keys[i] = append(append([]byte(nil), key...), 0) + rewritePBinFileAt(t, fixture, path, keys, values) + return + } + require.Fail(t, "fixture has no branch record") +} + +func rewritePBinFileWithWrongRootKey(t *testing.T, fixture pbinOutputFixture, path string) { + t.Helper() + keys, values := readKVFile(t, fixture.output, path) + for i, key := range keys { + if !isPBinRootKey(key) { + continue + } + keys[i] = append(append([]byte(nil), key...), 0) + rewritePBinFileAt(t, fixture, path, keys, values) + return + } + require.Fail(t, "fixture has no root record") +} + +func rewritePBinFileAt(t *testing.T, fixture pbinOutputFixture, path string, keys, values [][]byte) { + t.Helper() + config := fixture.output.Cfg(kv.CommitmentDomain) + require.NoError(t, dir.RemoveFile(path)) + comp, err := seg.NewCompressor(t.Context(), "pbin test post-build rewrite", path, fixture.output.Dirs().Tmp, config.CompressCfg, log.LvlDebug, log.New()) + require.NoError(t, err) + writer := seg.NewWriter(comp, config.Compression) + for i := range keys { + _, err = writer.Write(keys[i]) + require.NoError(t, err) + _, err = writer.Write(values[i]) + require.NoError(t, err) + } + require.NoError(t, comp.Compress()) + comp.Close() +} + +func removePBinOutputAccessors(t *testing.T, fixture pbinOutputFixture) { + t.Helper() + fixture.output.CloseMappedFilesForTest() + entries, err := os.ReadDir(filepath.Dir(fixture.outputPath)) + require.NoError(t, err) + removed := 0 + for _, entry := range entries { + if strings.Contains(entry.Name(), "-commitment.") && filepath.Ext(entry.Name()) != ".kv" { + require.NoError(t, dir.RemoveFile(filepath.Join(filepath.Dir(fixture.outputPath), entry.Name()))) + removed++ + } + } + require.Positive(t, removed) +} + +// A failed conversion is staged, never half-swapped: the shard the output started +// from is still in place and nothing the run built survives. +func assertPBinOutputRemoved(t *testing.T, fixture pbinOutputFixture) { + t.Helper() + require.FileExists(t, fixture.outputPath, "a failed conversion must leave the shard it started from") + assertPBinStageDirEmpty(t, fixture) +} + +func assertPBinStageDirEmpty(t *testing.T, fixture pbinOutputFixture) { + t.Helper() + stageDir := filepath.Join(fixture.output.Dirs().Tmp, "pbin_convert") + entries, err := os.ReadDir(stageDir) + if os.IsNotExist(err) { + return + } + require.NoError(t, err) + for _, entry := range entries { + if strings.Contains(entry.Name(), "-commitment.") { + require.Failf(t, "staged pbin output remains", "found %s", entry.Name()) + } + } +} + +func assertPBinOutputComplete(t *testing.T, fixture pbinOutputFixture) { + t.Helper() + entries, err := os.ReadDir(filepath.Dir(fixture.outputPath)) + require.NoError(t, err) + require.FileExists(t, fixture.outputPath) + accessors := 0 + for _, entry := range entries { + if strings.Contains(entry.Name(), "-commitment.") && filepath.Ext(entry.Name()) != ".kv" { + accessors++ + } + } + require.Positive(t, accessors) + assertPBinStageDirEmpty(t, fixture) +} + +// restagePBinOutputFromSource is what an operator does after a failed run: discard +// the output's commitment shard and link it in again from the untouched source. +func restagePBinOutputFromSource(t *testing.T, fixture pbinOutputFixture) { + t.Helper() + fixture.output.CloseMappedFilesForTest() + outputDir := filepath.Dir(fixture.outputPath) + entries, err := os.ReadDir(outputDir) + require.NoError(t, err) + for _, entry := range entries { + if strings.Contains(entry.Name(), "-commitment.") { + require.NoError(t, dir.RemoveFile(filepath.Join(outputDir, entry.Name()))) + } + } + entries, err = os.ReadDir(filepath.Dir(fixture.sourcePath)) + require.NoError(t, err) + for _, entry := range entries { + if !strings.Contains(entry.Name(), "-commitment.") { + continue + } + source := filepath.Join(filepath.Dir(fixture.sourcePath), entry.Name()) + require.NoError(t, os.Link(source, filepath.Join(outputDir, entry.Name()))) + } +} diff --git a/db/state/rebuild_pbin_state_test.go b/db/state/rebuild_pbin_state_test.go new file mode 100644 index 00000000000..ec4b13d3b47 --- /dev/null +++ b/db/state/rebuild_pbin_state_test.go @@ -0,0 +1,69 @@ +// Copyright 2026 The Erigon Authors +// This file is part of Erigon. +// +// Erigon is free software: you can redistribute it and/or modify +// it under the terms of the GNU Lesser General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// Erigon is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Lesser General Public License for more details. +// +// You should have received a copy of the GNU Lesser General Public License +// along with Erigon. If not, see . + +package state + +import ( + "encoding/binary" + "testing" + + "github.com/stretchr/testify/require" +) + +func pbinRebuildStateValue(t *testing.T, trieState []byte) []byte { + t.Helper() + v := make([]byte, 18+len(trieState)) + binary.BigEndian.PutUint16(v[16:18], uint16(len(trieState))) + copy(v[18:], trieState) + return v +} + +func TestValidatePBinRebuildState(t *testing.T) { + t.Parallel() + + for _, tc := range []struct { + name string + value []byte + ok bool + }{ + {"nothing stored", nil, true}, + {"header truncated", make([]byte, 9), false}, + {"no trie state", make([]byte, 18), true}, + {"length exceeds the value", func() []byte { + v := make([]byte, 18) + binary.BigEndian.PutUint16(v[16:18], 64) + return v + }(), false}, + {"trailing bytes past the declared length", func() []byte { + v := pbinRebuildStateValue(t, []byte{0x03, 0, 0}) + return append(v, 0, 0) + }(), false}, + {"trailing bytes with a zero length", func() []byte { + return append(make([]byte, 18), 0xB1, 0x03, 0, 0) + }(), false}, + {"hex trie state", pbinRebuildStateValue(t, []byte{0x03, 0, 0}), true}, + {"pre-version pbin blob", pbinRebuildStateValue(t, []byte{0xB1, 0x03, 0, 0}), false}, + } { + t.Run(tc.name, func(t *testing.T) { + err := validatePBinRebuildState(tc.value) + if tc.ok { + require.NoError(t, err) + return + } + require.Error(t, err) + }) + } +} diff --git a/db/state/rebuild_variant_bin_shard_tombstone_test.go b/db/state/rebuild_variant_bin_shard_tombstone_test.go index 5c2bc1a72f4..0b47f756bd5 100644 --- a/db/state/rebuild_variant_bin_shard_tombstone_test.go +++ b/db/state/rebuild_variant_bin_shard_tombstone_test.go @@ -123,7 +123,7 @@ func rebuildShardTombstoneDatadir(t *testing.T) (kv.TemporalRwDB, datadir.Dirs) return false, []byte{byte(i + 1), byte(i + 2), 0xAA} }) require.NoError(t, agg.BuildFiles(range1TxCount)) - agg, db = reopenShardTombstoneAgg(t, rawDB, dirs) + agg, db = reopenShardTombstoneAgg(t, agg, rawDB, dirs) writeShardTombstoneRange(t, db, range1TxCount, range2TxCount, 2, func(i int) (drop bool, val []byte) { if i >= shardTombstoneAccounts/2 { @@ -132,16 +132,15 @@ func rebuildShardTombstoneDatadir(t *testing.T) (kv.TemporalRwDB, datadir.Dirs) return false, []byte{byte(i + 1), byte(i + 2), 0xBB} }) require.NoError(t, agg.BuildFiles(range1TxCount+range2TxCount)) - agg, db = reopenShardTombstoneAgg(t, rawDB, dirs) + agg, db = reopenShardTombstoneAgg(t, agg, rawDB, dirs) // Collation holds a step back until a write in the next one proves it closed // (`step+1 records visible`, aggregator.go). Without this, range 2's own last // step never seals into a file and the range never forms. writeShardTombstoneGuard(t, db, range1TxCount+range2TxCount) require.NoError(t, agg.BuildFiles(range1TxCount+range2TxCount+shardTombstoneStepSize)) - _, db = reopenShardTombstoneAgg(t, rawDB, dirs) - return db, dirs + return trimShardTombstoneCommitment(t, agg, rawDB, dirs), dirs } func shardTombstoneGuardAddr() []byte { @@ -216,11 +215,40 @@ func writeShardTombstoneRange(t *testing.T, db kv.TemporalRwDB, rangeFrom, range require.NoError(t, rwTx.Commit()) } -// reopenShardTombstoneAgg drops the commitment domain file BuildFiles seals even -// with commitment writes discarded, and reopens against the trimmed folder: a -// resumed rebuild takes any file covering a range as that range already done. -func reopenShardTombstoneAgg(t *testing.T, rawDB kv.RwDB, dirs datadir.Dirs) (*state.Aggregator, kv.TemporalRwDB) { +func openShardTombstoneDB(t *testing.T, rawDB kv.RwDB, dirs datadir.Dirs) (*state.Aggregator, kv.TemporalRwDB) { t.Helper() + agg := shardTombstoneAgg(t, rawDB, dirs) + db, err := temporal.New(rawDB, agg, nil) + require.NoError(t, err) + t.Cleanup(db.Close) + return agg, db +} + +// reopenShardTombstoneAgg empties the snapshot folder between build rounds and +// reopens against it, so the next BuildFiles rebuilds every step from MDBX. The +// commitment file BuildFiles seals even with commitment writes discarded has to go, +// or SharedDomains rejects the next round's writes over a stale commitment step. +// Dropping it alone takes the aggregator's minimax tx num to zero, so the next +// BuildFiles re-collates and re-merges steps it already wrote, renaming over files +// the reopened aggregator holds mapped — which Windows refuses. +func reopenShardTombstoneAgg(t *testing.T, prev *state.Aggregator, rawDB kv.RwDB, dirs datadir.Dirs) (*state.Aggregator, kv.TemporalRwDB) { + t.Helper() + prev.Close() + for _, d := range []string{dirs.SnapDomain, dirs.SnapIdx, dirs.SnapHistory, dirs.SnapAccessors} { + paths, err := dir.ListFiles(d) + require.NoError(t, err) + for _, p := range paths { + require.NoError(t, dir.RemoveFile(p)) + } + } + return openShardTombstoneDB(t, rawDB, dirs) +} + +// trimShardTombstoneCommitment drops the commitment files from the finished folder: +// a resumed rebuild takes any file covering a range as that range already done. +func trimShardTombstoneCommitment(t *testing.T, prev *state.Aggregator, rawDB kv.RwDB, dirs datadir.Dirs) kv.TemporalRwDB { + t.Helper() + prev.Close() paths, err := dir.ListFiles(dirs.SnapDomain) require.NoError(t, err) for _, p := range paths { @@ -228,12 +256,8 @@ func reopenShardTombstoneAgg(t *testing.T, rawDB kv.RwDB, dirs datadir.Dirs) (*s require.NoError(t, dir.RemoveFile(p)) } } - - agg := shardTombstoneAgg(t, rawDB, dirs) - db, err := temporal.New(rawDB, agg, nil) - require.NoError(t, err) - t.Cleanup(db.Close) - return agg, db + _, db := openShardTombstoneDB(t, rawDB, dirs) + return db } // Shards slice a range in plain-key order while the trie is ordered by tree key, diff --git a/db/state/rebuild_variant_test.go b/db/state/rebuild_variant_test.go index 96914c3f882..b66189e8b40 100644 --- a/db/state/rebuild_variant_test.go +++ b/db/state/rebuild_variant_test.go @@ -20,6 +20,7 @@ package state_test import ( + "encoding/binary" "fmt" "os" "path/filepath" @@ -43,6 +44,7 @@ import ( "github.com/erigontech/erigon/db/state/execctx" "github.com/erigontech/erigon/db/state/statecfg" "github.com/erigontech/erigon/execution/commitment" + "github.com/erigontech/erigon/execution/commitment/commitmentdb" "github.com/erigontech/erigon/execution/types/accounts" ) @@ -177,6 +179,27 @@ func rebuildVariantRestoredRoot(t *testing.T, db kv.TemporalRwDB, agg *state.Agg return root } +func rebuildVariantPutLegacyPBinState(t *testing.T, db kv.TemporalRwDB) { + t.Helper() + tx, err := db.BeginTemporalRw(t.Context()) + require.NoError(t, err) + defer tx.Rollback() + + sd, err := execctx.NewSharedDomains(t.Context(), tx, log.New(), + execctx.WithTrieConfig(rebuildVariantTrieCfg(commitment.VariantHexPatriciaTrie)), + execctx.WithoutCommitmentSeek()) + require.NoError(t, err) + defer sd.Close() + + legacyTrieState := []byte{0xB1, 0, 0, 0} + stateValue := make([]byte, 18+len(legacyTrieState)) + binary.BigEndian.PutUint16(stateValue[16:18], uint16(len(legacyTrieState))) + copy(stateValue[18:], legacyTrieState) + require.NoError(t, sd.DomainPut(kv.CommitmentDomain, tx, commitmentdb.KeyCommitmentState, stateValue, 0, nil)) + require.NoError(t, sd.Flush(t.Context(), tx)) + require.NoError(t, tx.Commit()) +} + func rebuildVariantSettingsStayHex(t *testing.T, dirs datadir.Dirs) { t.Helper() settings, err := state.ResolveErigonDBSettings(dirs, log.New(), true) @@ -184,11 +207,11 @@ func rebuildVariantSettingsStayHex(t *testing.T, dirs datadir.Dirs) { require.Equal(t, state.TrieVariantHex, settings.TrieVariantName()) } -func rebuildVariantProcessStateUntouched(t *testing.T) { +func rebuildVariantProcessStateUntouched(t *testing.T, variantBefore commitment.TrieVariant) { t.Helper() require.False(t, statecfg.ExperimentalBinCommitment, "the rebuild must not enable the bin flag process-wide") require.Empty(t, statecfg.BinCommitmentHash) - require.Equal(t, commitment.VariantHexPatriciaTrie, execctx.PickTrieVariant()) + require.Equal(t, variantBefore, execctx.PickTrieVariant()) require.Equal(t, commitment.PBinHashKeccak, commitment.PBinHashSuiteName(), "the rebuild must restore H it bound") } @@ -220,6 +243,7 @@ func rebuildVariantReportCounts(t *testing.T, report *state.RebuildReport, root func TestRebuildCommitmentFilesBinTargetOnHexDatadir(t *testing.T) { binDB, binAgg, binDirs := rebuildVariantDatadir(t) rebuildVariantSettingsStayHex(t, binDirs) + variantBefore := execctx.PickTrieVariant() binRoot, binReport, err := state.RebuildCommitmentFiles(t.Context(), binDB, &rawdbv3.TxNums, log.New(), false, state.RebuildTarget{Variant: commitment.VariantBinPatriciaTrie}) @@ -227,7 +251,7 @@ func TestRebuildCommitmentFilesBinTargetOnHexDatadir(t *testing.T) { require.NotEmpty(t, binRoot) rebuildVariantReportCounts(t, binReport, binRoot, commitment.VariantBinPatriciaTrie) - rebuildVariantProcessStateUntouched(t) + rebuildVariantProcessStateUntouched(t, variantBefore) rebuildVariantSettingsStayHex(t, binDirs) require.Equal(t, binRoot, rebuildVariantRestoredRoot(t, binDB, binAgg, commitment.VariantBinPatriciaTrie), @@ -236,11 +260,21 @@ func TestRebuildCommitmentFilesBinTargetOnHexDatadir(t *testing.T) { hexDB, hexAgg, _ := rebuildVariantDatadir(t) hexRoot, hexReport, err := state.RebuildCommitmentFiles(t.Context(), hexDB, &rawdbv3.TxNums, log.New(), false, state.RebuildTarget{}) require.NoError(t, err) - rebuildVariantReportCounts(t, hexReport, hexRoot, commitment.VariantHexPatriciaTrie) + rebuildVariantReportCounts(t, hexReport, hexRoot, variantBefore) require.NotEqual(t, hexRoot, binRoot, "bin and hex commit different key spaces under different hashes") require.Equal(t, hexRoot, rebuildVariantRestoredRoot(t, hexDB, hexAgg, commitment.VariantHexPatriciaTrie)) } +func TestRebuildCommitmentFilesBinTargetRejectsLegacyPBinState(t *testing.T) { + db, _, _ := rebuildVariantDatadir(t) + rebuildVariantPutLegacyPBinState(t, db) + + _, _, err := state.RebuildCommitmentFiles(t.Context(), db, &rawdbv3.TxNums, log.New(), false, + state.RebuildTarget{Variant: commitment.VariantBinPatriciaTrie}) + require.Error(t, err) + require.ErrorContains(t, err, "record format") +} + // The commitment files a rebuild left behind, by name and content: a resumed run // must neither rewrite nor add to them. func rebuildVariantCommitmentFiles(t *testing.T, dirs datadir.Dirs) map[string]string { diff --git a/db/state/squeeze.go b/db/state/squeeze.go index 09a2193e992..d5b3ae3ed04 100644 --- a/db/state/squeeze.go +++ b/db/state/squeeze.go @@ -963,6 +963,30 @@ func bindPBinHashSuite(name string) (func(), error) { return func() { _ = commitment.SetPBinHashSuite(prev) }, nil } +func validatePBinRebuildState(stateValue []byte) error { + if len(stateValue) == 0 { + return nil + } + if len(stateValue) < 18 { + return fmt.Errorf("commitment rebuild: commitment state is %d bytes, too short for a header", len(stateValue)) + } + stateLen := int(binary.BigEndian.Uint16(stateValue[16:18])) + if len(stateValue) != 18+stateLen { + return fmt.Errorf("commitment rebuild: trie state claims %d bytes, %d present", stateLen, len(stateValue)-18) + } + if stateLen == 0 { + return nil + } + trieState := stateValue[18 : 18+stateLen] + if !commitment.IsPBinState(trieState) { + return nil + } + if err := commitment.ValidatePBinStateFormat(trieState); err != nil { + return fmt.Errorf("commitment rebuild: invalid pbin state: %w", err) + } + return nil +} + // RebuildCommitmentFiles recreates commitment files from existing accounts and storage kv files // If some commitment exists, they will be accepted as correct and next kv range will be processed. // DB expected to be empty, committed into db keys will be not processed. @@ -983,6 +1007,23 @@ func RebuildCommitmentFiles(ctx context.Context, rwDb kv.TemporalRwDB, txNumsRea } a := rwDb.(HasAgg).Agg().(*Aggregator) + if target.Variant == commitment.VariantBinPatriciaTrie { + roTx, err := rwDb.BeginTemporalRo(ctx) + if err != nil { + return nil, nil, err + } + defer roTx.Rollback() //nolint:gocritic + // A KV read slice only lives as long as its transaction, so the state is + // validated before the rollback rather than after it. + stateValue, _, readErr := roTx.GetLatest(kv.CommitmentDomain, commitmentdb.KeyCommitmentState) + if readErr == nil { + readErr = validatePBinRebuildState(stateValue) + } + roTx.Rollback() + if readErr != nil { + return nil, nil, readErr + } + } // disable hard alignment; allowing commitment and storage/account to have // different visibleFiles diff --git a/docs/pbin-encoding.md b/docs/pbin-encoding.md index 32cd7c4ae0d..3245cbe2aeb 100644 --- a/docs/pbin-encoding.md +++ b/docs/pbin-encoding.md @@ -244,40 +244,44 @@ key), `7+1+264 = 272` (the 34-byte code key), `0+1+527 = 528` (the 66-byte stora ### 5.2 Layout +A branch record is two cell bodies and nothing else. + ``` -+=====================+ written by encode, -| touchMap u16 BE | read by pbinDecodeBranch -| afterMap u16 BE | -+=====================+ -| cell body for bit 0 | present iff afterMap & 1 -| cell body for bit 1 | present iff afterMap & 2 ++=====================+ pbinBranchEncoder.encode, +| cell body for bit 0 | pbinDecodeBranch +| cell body for bit 1 | +=====================+ ``` -Cells are emitted in ascending bit order (`bitset & -bitset` / `TrailingZeros16`, -`encode`, `pbin_branch.go`); the decoder mirrors it exactly (`pbinDecodeBranch`). +Both are always present. A row that does not keep exactly two cells writes no record at all +(§5.4), so a header naming which children follow could only ever say "both". ``` cell body pbinAppendCell / pbinDecodeCell fields 1 byte bitmask, below - bitLen uvarint prefix length in BITS, 0..528 - prefix ceil(bitLen/8) bytes, MSB-first, pad bits zero - [accAddr] uvarint(20)=0x14 || 20 bytes - [stoAddr] uvarint(52)=0x34 || 52 bytes - [value] uvarint(32)=0x20 || 32 bytes - [hash] uvarint(32)=0x20 || 32 bytes + bitLen uvarint prefix length in BITS, 0..528 | omitted together when + prefix ceil(bitLen/8) bytes, MSB-first, pad zero | STORAGE_ADDR is set + [accAddr] 20 bytes + [stoAddr] 52 bytes + [value] 32 bytes + [hash] 32 bytes ``` `fields` (`pbinCellFields`, `pbin_branch.go`): bit0 LEAF, bit1 BRANCH, bit2 ACCOUNT_ADDR, bit3 STORAGE_ADDR, bit4 HASH, bit5 LEAF_VALUE. The optional blocks appear in one fixed order in both encoder and decoder — accAddr, stoAddr, LEAF_VALUE, HASH (`pbinAppendCell` / -`pbinDecodeCell`) — and that is -**not** the bit order: LEAF_VALUE is bit 5 and HASH is bit 4, so LEAF_VALUE is written first. The -fields byte says which blocks are present, not what order to read them in; a decoder that walks it -LSB-to-MSB takes HASH before LEAF_VALUE and desynchronises the cursor on any cell carrying both -(the §5.5 format-ceiling row). The length prefixes are uvarints but `pbinDecodeFixedVal` -demands the one exact width per field, making `0x14` / `0x34` / `0x20` the only legal -tag bytes. +`pbinDecodeCell`) — and that is **not** the bit order: LEAF_VALUE is bit 5 and HASH is bit 4, so +LEAF_VALUE is written first. The fields byte says which blocks are present, not what order to read +them in; a decoder that walks it LSB-to-MSB takes HASH before LEAF_VALUE and desynchronises the +cursor on any cell carrying both (the §5.5 format-ceiling row). + +Every block is a fixed width, so no block carries a length. `fields` and `bitLen` together fix the +body's size exactly, and `pbinDecodeFixedVal` reads each block at its one legal width. + +A STORAGE_ADDR cell stores no prefix. Its tree key is derived from the 52 bytes it already carries +(§1), so the decoder recomputes the prefix as the slice of that key below the cell's depth — the +record's own key bits plus one branch bit (`pbinDecodeCell`). That is the one thing in a record +that cannot be read without the key it was stored under. The cell prefix is relative to the record's own key plus the branch bit: the record's key is `pbinAppendBitPath(currentKey)`, the child sits at `keyBits+1`, and `prefix` carries the remainder @@ -290,87 +294,70 @@ The 265-bit record from §5.1 — a branch child and a header-storage leaf. ``` key 0012b9c2d7398802bddf3d70e0e8cf9074f4819101be174b883975a79061d53e7a0001 -00000000 00 03 00 03 12 05 00 20 de 50 84 4a 66 c2 a7 73 |....... .P.Jf..s| -00000010 d7 15 49 2d 67 9f ee 88 41 64 67 c0 c5 a7 80 2a |..I-g...Adg....*| -00000020 6b 03 31 99 b3 57 b0 a4 09 06 14 34 01 02 03 04 |k.1..W.....4....| -00000030 05 06 07 08 09 0a 0b 0c 0d 0e 0f 10 11 12 13 14 |................| +00000000 12 05 00 de 50 84 4a 66 c2 a7 73 d7 15 49 2d 67 |....P.Jf..s..I-g| +00000010 9f ee 88 41 64 67 c0 c5 a7 80 2a 6b 03 31 99 b3 |...Adg....*k.1..| +00000020 57 b0 a4 09 01 02 03 04 05 06 07 08 09 0a 0b 0c |W...............| +00000030 0d 0e 0f 10 11 12 13 14 00 00 00 00 00 00 00 00 |................| 00000040 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |................| -00000050 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 05 |................| +00000050 00 00 00 00 00 00 00 05 |........| -[00..01] 0003 touchMap = 0b11 -[02..03] 0003 afterMap = 0b11 cell bit 0 -[04] 12 fields = 00010010 BRANCH | HASH -[05] 05 bitLen = uvarint 5 -[06] 00 prefix = 00000 + 3 zero pad bits -[07..27] 20 || hash = de50844a66c2a773d715492d679fee88416467c0c5a7802a6b033199b357b0a4 +[00] 12 fields = 00010010 BRANCH | HASH +[01] 05 bitLen = uvarint 5 +[02] 00 prefix = 00000 + 3 zero pad bits +[03..22] hash = de50844a66c2a773d715492d679fee88416467c0c5a7802a6b033199b357b0a4 cell bit 1 -[28] 09 fields = 00001001 LEAF | STORAGE_ADDR -[29] 06 bitLen = uvarint 6 -[2a] 14 prefix = 000101 + 2 zero pad bits -[2b..5f] 34 || stoAddr = 0102030405060708090a0b0c0d0e0f1011121314 +[23] 09 fields = 00001001 LEAF | STORAGE_ADDR +[24..57] stoAddr = 0102030405060708090a0b0c0d0e0f1011121314 0000…0005 (addr || slot, 52 bytes) ``` -Sub-index reconstruction for cell 1: the record sits at 265 bits, so the sub-index's top bit is -already fixed to `0` by the prefix above it and this record's branch bit supplies the next, `1`. -The cell prefix then supplies `000101`. Full sub-index `0b01000101 = 0x45 = 64 + 5` — storage slot 5 -in the account header (§9). +Cell 1 carries no prefix bytes, and the decoder must rebuild them: the record sits at 265 bits and +its branch bit supplies one more, so the cell's prefix is `H(addr32) || sub` from bit 266 down. That +also reads out the sub-index — the top bit is fixed to `0` by the prefix above and this record's +branch bit supplies the next, `1`, then the derived prefix supplies `000101`. Full sub-index +`0b01000101 = 0x45 = 64 + 5` — storage slot 5 in the account header (§9). The other three records of the same tree: ``` -key 00 [0 bits] 162 bytes - 0003 0003 - 12 06 00 20 c9aca54ec7a6c2fe06fc1cee22bd609b559f1c16217ce2ea48793349b8d61be5 - 09 8f04 fe257385ae7310057bbe7ae1c1d19f20e9e90322037c2e971072eb4f20c3aa7cf4 - 3211d8496e2c633f71a67a015a0551623e46676cc65d3acc04301137a5fc5a8458 - 34 0102030405060708090a0b0c0d0e0f1011121314 - 000000000000000000000000000000000000000000000000000000000000012c +key 00 [0 bits] 88 bytes + 12 06 00 c9aca54ec7a6c2fe06fc1cee22bd609b559f1c16217ce2ea48793349b8d61be5 + 09 0102030405060708090a0b0c0d0e0f1011121314 + 000000000000000000000000000000000000000000000000000000000000012c -key 0007 [7 bits] 142 bytes - 0003 0003 +key 0007 [7 bits] 136 bytes 12 8102 12b9c2d7398802bddf3d70e0e8cf9074f4819101be174b883975a79061d53e7a00 - 20 ac8c75fc4b6f6e25d0831229dd10d3ad353c56dc573141f6f7e62707a5076b5d + ac8c75fc4b6f6e25d0831229dd10d3ad353c56dc573141f6f7e62707a5076b5d 21 8802 073be86901ad75392dc6c8cd03071cf8e0c17da59c33a1911c7b85c09f969b5a00 - 20 0060aabb00010200000000000000000000000000000000000000000000000000 + 0060aabb00010200000000000000000000000000000000000000000000000000 -key 0012b9…7a0007 [271 bits] 50 bytes - 0003 0003 - 05 00 14 0102030405060708090a0b0c0d0e0f1011121314 - 05 00 14 0102030405060708090a0b0c0d0e0f1011121314 +key 0012b9…7a0007 [271 bits] 44 bytes + 05 00 0102030405060708090a0b0c0d0e0f1011121314 + 05 00 0102030405060708090a0b0c0d0e0f1011121314 ``` The 7-bit record shows the two zones side by side and needs no shift to read: seven bits are consumed above it and one more by its own branch, so both cell prefixes start on a byte boundary — cell 0 carries `stem || 0x00` (the account key from byte 1 on), cell 1 the chunk key's own 33 bytes. -The top record is where the shift shows: the storage key starts `ff 12 b9…` and its cell prefix -(bits 1..527) starts `fe 25 73…`, since shifting left by one turns `ff 12 b9` into `fe 25 73` -(`0xff<<1 | 0x12>>7 = 0xfe`). +The `00` record is where the shift used to show, in a 66-byte storage prefix starting `fe 25 73…` +for a key starting `ff 12 b9…`; those bytes are gone now and the decoder derives them, which is why +that record dropped from 162 bytes to 88. The 271-bit record is the account pair: two leaf cells, zero-bit prefixes, both naming the *same* 20-byte plain key. Which leaf each is is decided by the last bit of the reconstructed tree key and resolved at hash time by `pbinLeafValue` (`pbin_hash.go`), not by anything in the record. -### 5.4 touchMap and afterMap - -Both are `uint16` at offsets 0 and 2 (`encode` and `pbinDecodeBranch`, `pbin_branch.go`) purely so -the `OnesCount16` / `TrailingZeros16` arithmetic ports from the hex engine unchanged (`pbinGrid`'s -doc comment, `pbin_cell.go`). Only bits 0 and 1 may be set; `pbinCheckCellMaps` rejects anything -outside `pbinCellBits = 0b11` on both encode and decode. +### 5.4 Why there is no header -`afterMap` is structural — it says which cell bodies follow. `touchMap` is write-time bookkeeping -only. The reader throws it away (`_, afterMap, err := pbinDecodeBranch`, -in `unfoldBranchNode`; the only other call site is `materializeBranch`), and -nothing downstream parses the record either: `TrieContext.PutBranch` hands the bytes straight to -`DomainPut` (`commitmentdb/commitment_context.go`). There is no `BranchData` merge. - -On disk, `afterMap` of a branch record is always `0b11`: `foldBranch` refuses a row that does not -keep exactly two cells (`foldBranch`). A row collapsing to one survivor writes -no record at all — the node moves up and the consumed bits are prepended to the survivor's prefix -(`foldPropagate`); a row keeping nothing writes a zero-length value, which is the deletion -encoding (`foldDelete`). `touchMap` does vary: bits are set at update time (`updateCell`) and -carried upward by `propagateTouch`. +A record used to open with a `touchMap` / `afterMap` pair saying which of the two cell bodies +followed. Neither survived, and the reason is arity: on disk `afterMap` was always `0b11`, because +`foldBranch` refuses a row that does not keep exactly two cells. A row collapsing to one survivor +writes no record at all — the node moves up and the consumed bits are prepended to the survivor's +prefix (`foldPropagate`); a row keeping nothing writes a zero-length value, which is the deletion +encoding (`foldDelete`). `touchMap` was write-time bookkeeping the reader threw away, and nothing +downstream parsed it either: `TrieContext.PutBranch` hands the bytes straight to `DomainPut` +(`commitmentdb/commitment_context.go`). There is no `BranchData` merge. Both non-branch outcomes are reachable: @@ -378,59 +365,64 @@ Both non-branch outcomes are reachable: cell seeds the new row with that one cell (`unfold`), so a row that no later update splits folds straight back through `foldPropagate` — the exact inverse of the unfold that opened it. - **No survivor** needs a parent cell that was touched and is now absent, which `unfoldBranchNode` - loads as `after = 0` through its `deleted` flag. A write of 32 zero bytes is a - deletion (§11), so zeroing a subtree's last leaf reaches it; pinned at - `TestPBinFoldDeleteRunsOnProcess` (`pbin_zerovalue_test.go`). + loads through its `deleted` flag. A write of 32 zero bytes is a deletion (§11), so zeroing a + subtree's last leaf reaches it; pinned at `TestPBinFoldDeleteRunsOnProcess` + (`pbin_zerovalue_test.go`). A reader still needs an answer for a zero-length value, and it differs by key: at a bit-path key `unfoldBranchNode` rejects it as a missing branch, so it is not a shape a decoder has to parse; at the root key `0x08` it is legal and means the empty tree (`loadRoot`). -That every record carries both children is what removes the merge path +That every record carries both children is also what removes the merge path (`pbinBranchEncoder`'s doc comment, `pbin_branch.go`): at arity 2 the untouched sibling is the whole other half of the subtree, so a record read back replaces its predecessor outright. ### 5.5 Size -Per cell: `1 (fields) + 1..2 (bitLen uvarint) + 0..66 (packed prefix) + one value block`. Value -blocks are 21 (account), 53 (storage), 33 (verbatim value), 33 (hash). - -| shape | bytes | reachable | -|---|---:|---| -| `afterMap = 0` | 4 | decodes; `foldBranch` never writes it | -| one bare BRANCH cell `000100010200` | 6 | same | -| two bare BRANCH cells `0003000302000200` | 8 | same | -| two hashed branch cells | 74 | yes | -| **writer floor** — two 0-prefix account leaves | **50** | yes, once in §5.1 (the 271-bit record) | -| **writer ceiling** — two 527-bit-prefix storage leaves | **248** | only at a depth-0 record | -| **format ceiling** — the same plus a HASH block on each | **314** | decodes; writer never emits it | - -All seven rows encode-and-decode round-trip. Measured record sizes for the §5.1 corpus: 162, 142, -96, 50, plus a 35-byte root record. The root record is framed differently and sized in §7. - -Size is driven, in order of weight, by: the two prefix bit lengths (up to 66 bytes each — all the -variance lives here, and it is inverse to depth); which value each child names (53 > 33 > 21); and -the 1-vs-2-byte `bitLen` uvarint at the 128-bit boundary. +Per cell: `1 (fields) + one value block`, plus `1..2 (bitLen uvarint) + 0..66 (packed prefix)` +unless STORAGE_ADDR omits both. Value blocks are 20 (account), 52 (storage), 32 (verbatim value), +32 (hash). + +| shape | bytes | was | reachable | +|---|---:|---:|---| +| two bare BRANCH cells `02000200` | 4 | 8 | decodes; `foldBranch` never writes it | +| two hashed branch cells | 68 | 74 | yes | +| **writer floor** — two 0-prefix account leaves | **44** | 50 | yes, once in §5.1 (the 271-bit record) | +| two 527-bit-key storage leaves | 106 | 248 | only at a depth-0 record | +| **writer ceiling** — two 527-bit-prefix value leaves | **202** | 208 | only at a depth-0 record | +| **format ceiling** — the same plus a HASH block on each | **266** | 274 | decodes; writer never emits it | + +All six rows encode and decode. Measured record sizes for the §5.1 corpus: 88, 136, 88, 44, plus a +34-byte root record. The root record is framed differently and sized in §7. + +The storage row is where the format change bites hardest: those cells used to be the largest a +writer could produce and are now among the smallest, because the 66-byte prefix that dominated them +is derived rather than stored. What is left driving size is the prefix bit lengths of the cells that +still carry one (up to 66 bytes each, inverse to depth), which value each child names (52 > 32 > 20), +and the 1-vs-2-byte `bitLen` uvarint at the 128-bit boundary (100 vs 102 bytes for a pair of +value leaves). ### 5.6 Decoding -`pbinDecodeBranch(data, cells *[2]pbinCell)` (`pbin_branch.go`) resets both cells -unconditionally, requires ≥4 bytes, reads the maps, re-checks them, then walks -`afterMap` in ascending bit order filling `cells[TrailingZeros16(bit)]`. A cell whose bit is clear -stays zeroed — that is how an absent child is spelled. Any leftover byte is an error. +`pbinDecodeBranch(data, cells, depth, keys)` (`pbin_branch.go`) resets both cells unconditionally, +then decodes two bodies back to back. Any leftover byte is an error. `depth` is the record's key +bits plus one, and `keys` is the digest cache a STORAGE_ADDR cell needs to rebuild its prefix — pass +neither and a storage cell cannot be read. -Each body restores kind from the LEAF/BRANCH bits; the prefix from the explicit bit count, never -from the byte length, with pad bits asserted zero (`pbinDecodePrefix`); `accountAddrLen` / -`storageAddrLen` / `hashLen` as side effects of their fields being present; and a LEAF_VALUE as -`Update{Flags: StorageUpdate, StorageLen: 32}`. +Each body restores kind from the LEAF/BRANCH bits; the prefix either from the explicit bit count, +never from the byte length, with pad bits asserted zero (`pbinDecodePrefix`), or from the tree key +derived from `stoAddr`; `accountAddrLen` / `storageAddrLen` / `hashLen` as side effects of their +fields being present; and a LEAF_VALUE as `Update{Flags: StorageUpdate, StorageLen: 32}`. Rejections, each observed firing: ``` pbinDecodeCell unknown field bits; neither or both node kinds; a leaf naming - 0 or 2+ value sources; a branch carrying a leaf value + 0 or 2+ value sources; a branch carrying a leaf value; + a storage leaf with no digest cache; a storage leaf whose + depth exceeds its own key pbinDecodePrefix a prefix over 528 bits; non-zero pad bits -pbinDecodeFixedVal a wrong length tag +pbinDecodeFixedVal a block the record is too short for pbinDecodeBranch trailing bytes leaf with both addrs -> malformed branch record: leaf cell fields 00001101 name no single value source @@ -439,15 +431,20 @@ pbinDecodeBranch trailing bytes trailing byte -> malformed branch record: 1 trailing bytes ``` +Dropping the lengths moved one class of corruption from "caught" to "silent". A truncated or +mistyped block used to be caught by its length tag; now a decoder can only notice at the record +boundary, so a corrupt cell is detected when the total does not land exactly on `len(data)` — and +two compensating errors would not be detected at all. Fixed widths are what make the record cheap; +the record boundary is the only frame check left. + One asymmetry against the "one canonical form" claim in `pbinDecodeBranch`'s doc comment: a BRANCH cell carrying ACCOUNT_ADDR or STORAGE_ADDR decodes cleanly (only LEAF_VALUE is refused for branches). The writer cannot produce it — `foldBranch` resets the upCell before setting kind — so -it is an unreachable spelling the decoder still accepts, -not a live bug. +it is an unreachable spelling the decoder still accepts, not a live bug. -Caller side: `unfoldBranchNode` keeps `afterMap` and discards the record's `touchMap`, setting -`touch=0, after=afterMap` normally, or `touch=afterMap, after=0` when the parent cell was touched -and is now gone, which is how a whole subtree is dropped (`unfoldBranchNode`). +Caller side: `unfoldBranchNode` treats every record as carrying both children, setting `after` to +`pbinCellBits` normally, or `touch = pbinCellBits, after = 0` when the parent cell was touched and +is now gone, which is how a whole subtree is dropped (`unfoldBranchNode`). ## 6. Leaf cells in a record @@ -505,38 +502,65 @@ phase; the cost is roughly one extra node per level of each proved path. ## 7. The root record -`pbinRootKey = {0x08}` (`pbin_patricia_hashed.go`) holds a **bare cell body with no 4-byte -header**: `storeRoot` calls `pbinAppendCell` directly and `loadRoot` calls `pbinDecodeCell` at -position 0, rejecting trailing bytes. A zero-length value at that key is the deletion encoding for -an emptied tree (`storeRoot`). +`pbinRootKey = {0x08}` (`pbin_patricia_hashed.go`) holds a **bare cell body**: `storeRoot` calls +`pbinAppendCell` directly and `loadRoot` calls `pbinDecodeCell` at position 0, rejecting trailing +bytes. A zero-length value at that key is the deletion encoding for an emptied tree (`storeRoot`). ``` -key 08, 35 bytes — the §5.1 tree: - 12 00 20 658b62aba5ac2933e86f1100cce5084bdb35b32d797b05d247abf27a2018c064 +key 08, 34 bytes — the §5.1 tree: + 12 00 658b62aba5ac2933e86f1100cce5084bdb35b32d797b05d247abf27a2018c064 ^ BRANCH|HASH ^ bitLen 0 - ^ len 32 || the state root + ^ the state root ``` +Both calls pass `omitStoragePrefix = false`, so a root cell keeps a prefix a branch record would +drop: nothing sits above the root to derive one from. It is the only cell body written that way. + +Nothing in a root record marks its format, and that is the one place the record change has no +self-describing tell. A branch record opened with a zero byte before the change and cannot now; the +trie state blob carries an explicit version (§7.1). A root cell is the same bytes in both spellings +minus the length tags, so telling them apart means decoding and checking the body ends exactly at +`len(data)` (`PBinRootRecordIsLegacy`, `pbin_convert_legacy.go`). A converter that keys off the +leading zero alone copies the root through untouched, and the datadir then fails at `loadRoot`. + +### 7.1 The trie state blob + +`KeyCommitmentState` in the commitment domain holds the engine's resume state, not a node. It is the +only pbin structure with a version byte: + +``` +B1 marker pbinStateMarker; a hex blob opens with a root-flags byte <= 0x07, + so the marker also refuses a cross-variant restore +10 format pbinRecordFormat, deliberately above pbinStateFlagsAll (0x07) +xx flags rootPresent | rootChecked | rootTouched +xxxx rootLen u16 BE, and the body must end exactly at 5 + rootLen + root cell a bare cell body, framed like the root record above +``` + +The format byte sits where a pre-version blob kept its flags, and every legal flags value is at or +below `0x07` — so `ValidatePBinStateFormat` (`pbin_state.go`) separates the two spellings on that +one byte with no ambiguity. That is what the root record has no room for. + That is the common shape, not the only one. `storeRoot` serialises whatever the root cell is, and a one-key tree's root is the leaf itself with no branch wrapping it (§4), so the record can equally be a LEAF cell — with a full-length prefix, since no descent sits above it to consume any of the key. Measured, the §5.1 address holding slot 300 and nothing else: ``` -key 08, 122 bytes: - 09 9004 ff12b9…d2fe2d42 2c 34 0102…1314 0000…012c +key 08, 121 bytes: + 09 9004 ff12b9…d2fe2d42 2c 0102…1314 0000…012c ^ LEAF|STORAGE_ADDR ^ uvarint 528 bits ^ the whole 66-byte tree key, packed - ^ len 52 || addr || slot + ^ addr || slot ``` -Sizes follow the §5.5 per-cell arithmetic with no 4-byte header: `1 (fields) + 1..2 (bitLen uvarint) -+ 0..66 (packed prefix) + one value block of 21 / 33 / 53`. That is 35 bytes for the branch-and-hash -spelling above, 58 / 70 / 90 for a 272-bit leaf root naming an address, a verbatim value or an -`addr||slot`, and 122 at most — the 528-bit storage leaf shown. A decoder that assumes BRANCH|HASH -fails on every one-key tree. +Sizes follow the §5.5 per-cell arithmetic: `1 (fields) + 1..2 (bitLen uvarint) + 0..66 (packed +prefix) + one value block of 20 / 32 / 52`. That is 34 bytes for the branch-and-hash spelling above, +57 / 69 / 89 for a 272-bit leaf root naming an address, a verbatim value or an `addr||slot`, and 121 +at most — the 528-bit storage leaf shown. A decoder that assumes BRANCH|HASH fails on every one-key +tree. The root cell needs a key of its own, and not because nothing names it — the empty path does. The problem is that the empty path already encodes to the 1-byte key `00`, which is the record of the @@ -560,9 +584,8 @@ pbinDecodeBitPath(7374617465) -> pbin: invalid trailing bit count 101 in bit-pat ``` The witness-side `PatriciaContext` uses the same two record framings — a bare cell for the root -(`rootRecord`, `pbin_witness_context.go`), a full header plus two cells for a branch, with -`touchMap` set equal to `afterMap` because a read discards it anyway (`branchRecord`). The framing -is shared; what goes into a leaf cell is not — see the witness paragraph in §6. +(`rootRecord`, `pbin_witness_context.go`), two cells for a branch (`branchRecord`). The framing is +shared; what goes into a leaf cell is not — see the witness paragraph in §6. ## 8. The account header stem diff --git a/execution/commitment/pbin_branch.go b/execution/commitment/pbin_branch.go index d8008850b86..ac58b209738 100644 --- a/execution/commitment/pbin_branch.go +++ b/execution/commitment/pbin_branch.go @@ -20,7 +20,6 @@ import ( "encoding/binary" "errors" "fmt" - "math/bits" "github.com/erigontech/erigon/common/length" ) @@ -67,21 +66,18 @@ func (e *pbinBranchEncoder) encode(touchMap, afterMap uint16, cells *[2]pbinCell if err := pbinCheckCellMaps(touchMap, afterMap); err != nil { return nil, err } - e.buf = binary.BigEndian.AppendUint16(e.buf[:0], touchMap) - e.buf = binary.BigEndian.AppendUint16(e.buf, afterMap) + e.buf = e.buf[:0] var err error - for bitset := afterMap; bitset != 0; { - bit := bitset & -bitset - if e.buf, err = pbinAppendCell(e.buf, &cells[bits.TrailingZeros16(bit)]); err != nil { + for i := range cells { + if e.buf, err = pbinAppendCell(e.buf, &cells[i], true); err != nil { return nil, err } - bitset ^= bit } return e.buf, nil } -func pbinAppendCell(dst []byte, c *pbinCell) ([]byte, error) { +func pbinAppendCell(dst []byte, c *pbinCell, omitStoragePrefix bool) ([]byte, error) { var fields pbinCellFields switch c.kind { case pbinNodeLeaf: @@ -105,61 +101,49 @@ func pbinAppendCell(dst []byte, c *pbinCell) ([]byte, error) { } dst = append(dst, byte(fields)) - dst = binary.AppendUvarint(dst, uint64(c.prefix.bitLen)) - dst = c.prefix.appendPackedBits(dst) + if !omitStoragePrefix || fields&pbinFieldStorageAddr == 0 { + dst = binary.AppendUvarint(dst, uint64(c.prefix.bitLen)) + dst = c.prefix.appendPackedBits(dst) + } if fields&pbinFieldAccountAddr != 0 { - dst = pbinAppendLenAndVal(dst, c.accountAddr[:c.accountAddrLen]) + dst = append(dst, c.accountAddr[:c.accountAddrLen]...) } if fields&pbinFieldStorageAddr != 0 { - dst = pbinAppendLenAndVal(dst, c.storageAddr[:c.storageAddrLen]) + dst = append(dst, c.storageAddr[:c.storageAddrLen]...) } if fields&pbinFieldLeafValue != 0 { value, err := pbinRecordLeafValue(&c.Update) if err != nil { return nil, err } - dst = pbinAppendLenAndVal(dst, value[:]) + dst = append(dst, value[:]...) } if fields&pbinFieldHash != 0 { - dst = pbinAppendLenAndVal(dst, c.hash[:c.hashLen]) + dst = append(dst, c.hash[:c.hashLen]...) } return dst, nil } -func pbinAppendLenAndVal(dst, val []byte) []byte { - return append(binary.AppendUvarint(dst, uint64(len(val))), val...) -} - // pbinDecodeBranch fills both cells from a record. It rejects every spelling the // encoder would not produce, so a record has one canonical form. -func pbinDecodeBranch(data []byte, cells *[2]pbinCell) (touchMap, afterMap uint16, err error) { +func pbinDecodeBranch(data []byte, cells *[2]pbinCell, depth int16, keys *pbinDigestCache) (afterMap uint16, err error) { cells[0].reset() cells[1].reset() - if len(data) < 4 { - return 0, 0, fmt.Errorf("%w: %d bytes is shorter than the header", errPBinMalformedBranch, len(data)) - } - touchMap, afterMap = binary.BigEndian.Uint16(data), binary.BigEndian.Uint16(data[2:]) - if err := pbinCheckCellMaps(touchMap, afterMap); err != nil { - return 0, 0, err - } - - pos := 4 - for bitset := afterMap; bitset != 0; { - bit := bitset & -bitset - if pos, err = pbinDecodeCell(data, pos, &cells[bits.TrailingZeros16(bit)]); err != nil { - return 0, 0, err + pos := 0 + for i := range cells { + if pos, err = pbinDecodeCell(data, pos, &cells[i], depth, keys, true); err != nil { + return 0, err } - bitset ^= bit } if pos != len(data) { - return 0, 0, fmt.Errorf("%w: %d trailing bytes", errPBinMalformedBranch, len(data)-pos) + return 0, fmt.Errorf("%w: %d trailing bytes", errPBinMalformedBranch, len(data)-pos) } - return touchMap, afterMap, nil + return pbinCellBits, nil } -func pbinDecodeCell(data []byte, pos int, c *pbinCell) (int, error) { +func pbinDecodeCell(data []byte, pos int, c *pbinCell, depth int16, keys *pbinDigestCache, omitStoragePrefix bool) (int, error) { if pos >= len(data) { return 0, fmt.Errorf("%w: no cell body at offset %d", errPBinMalformedBranch, pos) } @@ -180,16 +164,21 @@ func pbinDecodeCell(data []byte, pos int, c *pbinCell) (int, error) { } case pbinFieldBranch: c.kind = pbinNodeBranch - if fields&pbinFieldLeafValue != 0 { - return 0, fmt.Errorf("%w: branch cell carries a leaf value", errPBinMalformedBranch) + // A branch owns no plain key, so an address here would also make the + // omitted-prefix path rebuild a whole leaf path for a partial extension. + if fields&pbinFieldValue != 0 { + return 0, fmt.Errorf("%w: branch cell carries a value field", errPBinMalformedBranch) } default: return 0, fmt.Errorf("%w: cell fields %08b name no single node kind", errPBinMalformedBranch, fields) } - pos, err := pbinDecodePrefix(data, pos, c) - if err != nil { - return 0, err + var err error + if !omitStoragePrefix || fields&pbinFieldStorageAddr == 0 { + pos, err = pbinDecodePrefix(data, pos, c) + if err != nil { + return 0, err + } } if fields&pbinFieldAccountAddr != 0 { if pos, err = pbinDecodeFixedVal(data, pos, c.accountAddr[:], length.Addr); err != nil { @@ -202,6 +191,16 @@ func pbinDecodeCell(data []byte, pos int, c *pbinCell) (int, error) { return 0, err } c.storageAddrLen = length.Addr + length.Hash + if omitStoragePrefix { + if keys == nil { + return 0, fmt.Errorf("%w: storage leaf prefix needs a digest cache", errPBinMalformedBranch) + } + storageKey := pbinPathFromBytes(keys.storageKey(c.storageAddr[:length.Addr], c.storageAddr[length.Addr:])) + if depth < 0 || depth > storageKey.bitLen { + return 0, fmt.Errorf("%w: storage leaf at depth %d exceeds its %d-bit key", errPBinMalformedBranch, depth, storageKey.bitLen) + } + c.prefix = storageKey.slice(depth, storageKey.bitLen) + } } if fields&pbinFieldLeafValue != 0 { if pos, err = pbinDecodeFixedVal(data, pos, c.Storage[:], pbinValueLength); err != nil { @@ -242,16 +241,8 @@ func pbinDecodePrefix(data []byte, pos int, c *pbinCell) (int, error) { } func pbinDecodeFixedVal(data []byte, pos int, dst []byte, want int) (int, error) { - l, n := binary.Uvarint(data[pos:]) - if n <= 0 { - return 0, fmt.Errorf("%w: unreadable value length at offset %d", errPBinMalformedBranch, pos) - } - pos += n - if l != uint64(want) { - return 0, fmt.Errorf("%w: value of %d bytes, want %d", errPBinMalformedBranch, l, want) - } if pos+want > len(data) { - return 0, fmt.Errorf("%w: value of %d bytes needs more than the %d left", errPBinMalformedBranch, want, len(data)-pos) + return 0, fmt.Errorf("%w: fixed value of %d bytes needs more than the %d left", errPBinMalformedBranch, want, len(data)-pos) } copy(dst, data[pos:pos+want]) return pos + want, nil diff --git a/execution/commitment/pbin_cell_test.go b/execution/commitment/pbin_cell_test.go index c78b2fdb4af..a95c63039c8 100644 --- a/execution/commitment/pbin_cell_test.go +++ b/execution/commitment/pbin_cell_test.go @@ -19,6 +19,7 @@ package commitment import ( "bytes" "encoding/binary" + "fmt" "testing" "github.com/stretchr/testify/require" @@ -55,6 +56,16 @@ func pbinTestLeafCell(pattern byte, bitLen int16) pbinCell { return c } +func pbinTestAccountLeafCell(pattern byte, bitLen int16) pbinCell { + c := pbinTestBranchCell(pattern, bitLen) + c.kind = pbinNodeLeaf + for i := range c.accountAddr { + c.accountAddr[i] = pattern + byte(i) + } + c.accountAddrLen = length.Addr + return c +} + // pbinTestChunkLeafCell is the one leaf shape carrying its value in the record // instead of a plain key: a code chunk. func pbinTestChunkLeafCell(pattern byte, bitLen int16) pbinCell { @@ -76,21 +87,177 @@ func TestPBinBranchCodecRoundTripPrefixBitLengths(t *testing.T) { for bitLen := int16(0); bitLen <= pbinMaxPathBits; bitLen++ { cells := [2]pbinCell{ pbinTestBranchCell(0xA5, bitLen), - pbinTestLeafCell(0x5A, pbinMaxPathBits-bitLen), + pbinTestChunkLeafCell(0x5A, pbinMaxPathBits-bitLen), } rec, err := enc.encode(0b11, 0b11, &cells) require.NoErrorf(t, err, "bitLen %d", bitLen) var got [2]pbinCell - touchMap, afterMap, err := pbinDecodeBranch(bytes.Clone(rec), &got) + afterMap, err := pbinDecodeBranch(bytes.Clone(rec), &got, 0, nil) require.NoErrorf(t, err, "bitLen %d", bitLen) - require.Equal(t, uint16(0b11), touchMap) require.Equal(t, uint16(0b11), afterMap) require.Equalf(t, cells, got, "bitLen %d", bitLen) } } +func TestPBinBranchCodecOmitsRecordHeader(t *testing.T) { + t.Parallel() + + cells := [2]pbinCell{pbinTestBranchCell(0xA5, 3), pbinTestBranchCell(0x5A, 7)} + var enc pbinBranchEncoder + rec, err := enc.encode(0b11, 0b11, &cells) + require.NoError(t, err) + require.Equal(t, byte(pbinFieldBranch|pbinFieldHash), rec[0]) +} + +func TestPBinBranchDecodeAcceptsDescentDepthAndDigestCache(t *testing.T) { + t.Parallel() + + var enc pbinBranchEncoder + keys := pbinDigestCache{sum: pbinBlake3Hash} + storage := pbinTestLeafCell(0x5A, 31) + storageKey := pbinPathFromBytes(keys.storageKey(storage.storageAddr[:length.Addr], storage.storageAddr[length.Addr:])) + storage.prefix = storageKey.slice(17, storageKey.bitLen) + want := [2]pbinCell{pbinTestBranchCell(0xA5, 17), storage} + record, err := enc.encode(0b11, 0b11, &want) + require.NoError(t, err) + + var got [2]pbinCell + afterMap, err := pbinDecodeBranch(record, &got, 17, &keys) + require.NoError(t, err) + require.Equal(t, uint16(0b11), afterMap) + require.Equal(t, want, got) +} + +func TestPBinBranchCodecOmitsStoragePrefix(t *testing.T) { + t.Parallel() + + for _, depth := range []int16{0, 17, 271, 528} { + t.Run(fmt.Sprintf("depth %d", depth), func(t *testing.T) { + t.Parallel() + + // pbinDigestCache memoizes into its own fields, so a parallel subtest needs its own. + keys := pbinDigestCache{sum: pbinBlake3Hash} + storage := pbinTestLeafCell(0x5A, 0) + storageKey := pbinPathFromBytes(keys.storageKey(storage.storageAddr[:length.Addr], storage.storageAddr[length.Addr:])) + storage.prefix = storageKey.slice(depth, storageKey.bitLen) + other := pbinTestBranchCell(0xA5, 3) + + var enc pbinBranchEncoder + record, err := enc.encode(pbinCellBits, pbinCellBits, &[2]pbinCell{storage, other}) + require.NoError(t, err) + + fields := byte(pbinFieldLeaf | pbinFieldStorageAddr | pbinFieldHash) + want := append(append([]byte{}, storage.storageAddr[:]...), storage.hash[:]...) + require.Equal(t, fields, record[0]) + require.Equal(t, want, record[1:1+len(want)]) + + var got [2]pbinCell + _, err = pbinDecodeBranch(record, &got, depth, &keys) + require.NoError(t, err) + require.Equal(t, storage, got[0]) + + again, err := enc.encode(pbinCellBits, pbinCellBits, &got) + require.NoError(t, err) + require.Equal(t, record, again) + }) + } +} + +func TestPBinBranchDecodeStoragePrefixRequiresDigestCache(t *testing.T) { + t.Parallel() + + storage := pbinTestLeafCell(0x5A, 0) + var enc pbinBranchEncoder + record, err := enc.encode(pbinCellBits, pbinCellBits, &[2]pbinCell{storage, pbinTestBranchCell(0xA5, 3)}) + require.NoError(t, err) + + var cells [2]pbinCell + _, err = pbinDecodeBranch(record, &cells, 1, nil) + require.ErrorContains(t, err, "digest cache") +} + +func TestPBinBranchCodecKeepsAccountPrefix(t *testing.T) { + t.Parallel() + + account := pbinTestEmptyCell() + account.kind = pbinNodeLeaf + account.prefix = pbinPathFromBits([]byte{0xA0}, 3) + account.accountAddrLen = length.Addr + copy(account.accountAddr[:], bytes.Repeat([]byte{0x42}, length.Addr)) + + var enc pbinBranchEncoder + record, err := enc.encode(pbinCellBits, pbinCellBits, &[2]pbinCell{account, pbinTestBranchCell(0x11, 0)}) + require.NoError(t, err) + require.Equal(t, byte(pbinFieldLeaf|pbinFieldAccountAddr), record[0]) + require.Equal(t, byte(account.prefix.bitLen), record[1]) + require.Equal(t, byte(0xA0), record[2]) +} + +func TestPBinBranchCodecKeepsCodeChunkPrefix(t *testing.T) { + t.Parallel() + + chunk := pbinTestChunkLeafCell(0x31, 7) + var enc pbinBranchEncoder + record, err := enc.encode(pbinCellBits, pbinCellBits, &[2]pbinCell{chunk, pbinTestBranchCell(0x11, 0)}) + require.NoError(t, err) + require.Equal(t, byte(pbinFieldLeaf|pbinFieldLeafValue|pbinFieldHash), record[0]) + require.Equal(t, byte(chunk.prefix.bitLen), record[1]) + require.Equal(t, byte(0x30), record[2]) +} + +func TestPBinCellCodecFixedFieldCosts(t *testing.T) { + t.Parallel() + + account := pbinTestEmptyCell() + account.kind = pbinNodeLeaf + account.accountAddrLen = length.Addr + account.hashLen = 0 + + accountWithHash := account + accountWithHash.hashLen = length.Hash + + storage := pbinTestEmptyCell() + storage.kind = pbinNodeLeaf + storage.storageAddrLen = length.Addr + length.Hash + + storageWithHash := storage + storageWithHash.hashLen = length.Hash + + value := pbinTestChunkLeafCell(0x31, 0) + value.hashLen = 0 + + valueWithHash := value + valueWithHash.hashLen = length.Hash + + branch := pbinTestEmptyCell() + branch.kind = pbinNodeBranch + + for _, tc := range []struct { + name string + cell pbinCell + fixedSize int + }{ + {"branch", branch, 0}, + {"branch and hash", pbinTestBranchCell(0x01, 0), length.Hash}, + {"account address", account, length.Addr}, + {"account address and hash", accountWithHash, length.Addr + length.Hash}, + {"storage address", storage, length.Addr + length.Hash}, + {"storage address and hash", storageWithHash, length.Addr + 2*length.Hash}, + {"record value", value, pbinValueLength}, + {"record value and hash", valueWithHash, pbinValueLength + length.Hash}, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + got, err := pbinAppendCell(nil, &tc.cell, false) + require.NoError(t, err) + require.Len(t, got, 2+tc.fixedSize) + }) + } +} + func TestPBinBranchCodecRoundTripCellShapes(t *testing.T) { t.Parallel() @@ -107,10 +274,9 @@ func TestPBinBranchCodecRoundTripCellShapes(t *testing.T) { cells [2]pbinCell }{ {"both branches", 0b11, 0b11, [2]pbinCell{pbinTestBranchCell(0x01, 3), pbinTestBranchCell(0x02, 528)}}, - {"leaf and branch", 0b11, 0b11, [2]pbinCell{pbinTestLeafCell(0x03, 271), pbinTestBranchCell(0x04, 5)}}, - {"hashless account leaf", 0b11, 0b11, [2]pbinCell{accountLeaf, pbinTestLeafCell(0x05, 64)}}, - {"only the right cell present", 0b10, 0b10, [2]pbinCell{pbinTestEmptyCell(), pbinTestBranchCell(0x07, 9)}}, - {"deleted left cell", 0b11, 0b10, [2]pbinCell{pbinTestEmptyCell(), pbinTestBranchCell(0x08, 9)}}, + {"leaf and branch", 0b11, 0b11, [2]pbinCell{pbinTestChunkLeafCell(0x03, 271), pbinTestBranchCell(0x04, 5)}}, + {"hashless account leaf", 0b11, 0b11, [2]pbinCell{accountLeaf, pbinTestChunkLeafCell(0x05, 64)}}, + {"maps do not control payload", 0b10, 0b10, [2]pbinCell{pbinTestBranchCell(0x07, 9), pbinTestBranchCell(0x08, 9)}}, {"record-resident chunk leaf", 0b11, 0b11, [2]pbinCell{pbinTestChunkLeafCell(0x09, 12), pbinTestBranchCell(0x0A, 21)}}, {"two chunk leaves", 0b11, 0b11, [2]pbinCell{pbinTestChunkLeafCell(0x0B, 0), pbinTestChunkLeafCell(0x0C, 528)}}, } { @@ -122,10 +288,9 @@ func TestPBinBranchCodecRoundTripCellShapes(t *testing.T) { require.NoError(t, err) var got [2]pbinCell - touchMap, afterMap, err := pbinDecodeBranch(rec, &got) + afterMap, err := pbinDecodeBranch(rec, &got, 0, nil) require.NoError(t, err) - require.Equal(t, tc.touchMap, touchMap) - require.Equal(t, tc.afterMap, afterMap) + require.Equal(t, uint16(0b11), afterMap) require.Equal(t, tc.cells, got) }) } @@ -140,7 +305,7 @@ func TestPBinBranchCodecIsCanonical(t *testing.T) { name string cells [2]pbinCell }{ - {"plain-key leaf and branch", [2]pbinCell{pbinTestLeafCell(0x7C, 33), pbinTestBranchCell(0x3E, 528)}}, + {"plain-key leaf and branch", [2]pbinCell{pbinTestAccountLeafCell(0x7C, 33), pbinTestBranchCell(0x3E, 528)}}, {"chunk leaf and branch", [2]pbinCell{pbinTestChunkLeafCell(0x6D, 33), pbinTestBranchCell(0x3E, 528)}}, } { t.Run(tc.name, func(t *testing.T) { @@ -152,7 +317,7 @@ func TestPBinBranchCodecIsCanonical(t *testing.T) { want := bytes.Clone(rec) var got [2]pbinCell - _, _, err = pbinDecodeBranch(want, &got) + _, err = pbinDecodeBranch(want, &got, 0, nil) require.NoError(t, err) again, err := enc.encode(0b11, 0b11, &got) @@ -164,10 +329,8 @@ func TestPBinBranchCodecIsCanonical(t *testing.T) { // pbinTestRecord assembles a record by hand so decode can be probed with bytes // the encoder would never emit. -func pbinTestRecord(touchMap, afterMap uint16, bodies ...[]byte) []byte { - rec := make([]byte, 0, 4) - rec = binary.BigEndian.AppendUint16(rec, touchMap) - rec = binary.BigEndian.AppendUint16(rec, afterMap) +func pbinTestRecord(bodies ...[]byte) []byte { + rec := make([]byte, 0) for _, b := range bodies { rec = append(rec, b...) } @@ -183,8 +346,41 @@ func pbinTestCellBody(fields pbinCellFields, prefixBitLen uint64, prefix []byte, return append(body, tail...) } -func pbinTestLenAndVal(val []byte) []byte { - return append(binary.AppendUvarint(nil, uint64(len(val))), val...) +func pbinTestFixedVal(val []byte) []byte { + return append([]byte(nil), val...) +} + +func TestPBinDecodeRejectsTruncatedFixedFields(t *testing.T) { + t.Parallel() + + account := pbinTestEmptyCell() + account.kind = pbinNodeLeaf + account.accountAddrLen = length.Addr + storage := pbinTestEmptyCell() + storage.kind = pbinNodeLeaf + storage.storageAddrLen = length.Addr + length.Hash + value := pbinTestChunkLeafCell(0x41, 0) + branch := pbinTestBranchCell(0x52, 0) + + for _, tc := range []struct { + name string + cell pbinCell + }{ + {"account address", account}, + {"storage address", storage}, + {"record value", value}, + {"hash", branch}, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + record, err := pbinAppendCell(nil, &tc.cell, false) + require.NoError(t, err) + _, err = pbinDecodeCell(record[:len(record)-1], 0, new(pbinCell), 0, nil, false) + require.Error(t, err) + require.ErrorContains(t, err, "fixed value") + }) + } } // A declared bit count that disagrees with the bytes behind it must be rejected, @@ -199,42 +395,41 @@ func TestPBinBranchDecodeRejects(t *testing.T) { name string rec []byte }{ - {"truncated header", []byte{0x00, 0x03, 0x00}}, - {"cell bit outside the arity", pbinTestRecord(0b100, 0b100, body)}, - {"touched bit outside the arity", pbinTestRecord(0b1011, 0b11, body, body)}, - {"missing cell body", pbinTestRecord(0b11, 0b11, body)}, - {"unknown field bit", pbinTestRecord(0b01, 0b01, pbinTestCellBody(0x80, 0, nil))}, - {"no node kind", pbinTestRecord(0b01, 0b01, pbinTestCellBody(0, 0, nil))}, - {"both node kinds", pbinTestRecord(0b01, 0b01, pbinTestCellBody(pbinFieldBranch|pbinFieldLeaf, 0, nil))}, - {"prefix shorter than its bit count", pbinTestRecord(0b01, 0b01, pbinTestCellBody(pbinFieldBranch, 16, []byte{0xFF}))}, - {"prefix longer than its bit count", pbinTestRecord(0b01, 0b01, pbinTestCellBody(pbinFieldBranch, 8, []byte{0xFF, 0xFF}))}, - {"non-zero pad bits", pbinTestRecord(0b01, 0b01, pbinTestCellBody(pbinFieldBranch, 3, []byte{0xFF}))}, - {"bit count beyond the longest path", pbinTestRecord(0b01, 0b01, pbinTestCellBody(pbinFieldBranch, pbinMaxPathBits+1, bytes.Repeat([]byte{0xFF}, 67)))}, - {"truncated uvarint", pbinTestRecord(0b01, 0b01, []byte{byte(pbinFieldBranch), 0x80})}, - {"hash longer than a digest", pbinTestRecord(0b01, 0b01, pbinTestCellBody(pbinFieldBranch|pbinFieldHash, 0, nil, pbinTestLenAndVal(bytes.Repeat([]byte{0xEE}, 33))...))}, - {"truncated hash", pbinTestRecord(0b01, 0b01, pbinTestCellBody(pbinFieldBranch|pbinFieldHash, 0, nil, 32, 0xEE))}, - {"account address of the wrong length", pbinTestRecord(0b01, 0b01, pbinTestCellBody(pbinFieldLeaf|pbinFieldAccountAddr, 0, nil, pbinTestLenAndVal(bytes.Repeat([]byte{0xEE}, 21))...))}, - {"storage address of the wrong length", pbinTestRecord(0b01, 0b01, pbinTestCellBody(pbinFieldLeaf|pbinFieldStorageAddr, 0, nil, pbinTestLenAndVal(bytes.Repeat([]byte{0xEE}, 51))...))}, - {"trailing bytes", pbinTestRecord(0b01, 0b01, pbinTestCellBody(pbinFieldBranch, 0, nil), []byte{0x00})}, + {"missing first cell body", nil}, + {"missing second cell body", pbinTestRecord(body)}, + {"unknown field bit", pbinTestRecord(pbinTestCellBody(0x80, 0, nil), body)}, + {"no node kind", pbinTestRecord(pbinTestCellBody(0, 0, nil), body)}, + {"both node kinds", pbinTestRecord(pbinTestCellBody(pbinFieldBranch|pbinFieldLeaf, 0, nil), body)}, + {"prefix shorter than its bit count", pbinTestRecord(pbinTestCellBody(pbinFieldBranch, 16, []byte{0xFF}), body)}, + {"prefix longer than its bit count", pbinTestRecord(pbinTestCellBody(pbinFieldBranch, 8, []byte{0xFF, 0xFF}), body)}, + {"non-zero pad bits", pbinTestRecord(pbinTestCellBody(pbinFieldBranch, 3, []byte{0xFF}), body)}, + {"bit count beyond the longest path", pbinTestRecord(pbinTestCellBody(pbinFieldBranch, pbinMaxPathBits+1, bytes.Repeat([]byte{0xFF}, 67)), body)}, + {"truncated uvarint", pbinTestRecord([]byte{byte(pbinFieldBranch), 0x80}, body)}, + {"hash with an extra byte", pbinTestRecord(pbinTestCellBody(pbinFieldBranch|pbinFieldHash, 0, nil, pbinTestFixedVal(bytes.Repeat([]byte{0xEE}, 33))...), body)}, + {"truncated hash", pbinTestRecord(pbinTestCellBody(pbinFieldBranch|pbinFieldHash, 0, nil, pbinTestFixedVal(bytes.Repeat([]byte{0xEE}, 31))...), body)}, + {"account address with an extra byte", pbinTestRecord(pbinTestCellBody(pbinFieldLeaf|pbinFieldAccountAddr, 0, nil, pbinTestFixedVal(bytes.Repeat([]byte{0xEE}, 21))...), body)}, + {"storage address with an extra byte", pbinTestRecord(pbinTestCellBody(pbinFieldLeaf|pbinFieldStorageAddr, 0, nil, pbinTestFixedVal(bytes.Repeat([]byte{0xEE}, 51))...), body)}, + {"trailing bytes", pbinTestRecord(body, body, []byte{0x00})}, // A leaf resolves its value through its plain key, so one without a plain // key would hash a zero-valued state instead of failing. - {"leaf without a plain key", pbinTestRecord(0b01, 0b01, pbinTestCellBody(pbinFieldLeaf, 0, nil))}, - {"leaf naming both plain keys", pbinTestRecord(0b01, 0b01, pbinTestCellBody(pbinFieldLeaf|pbinFieldAccountAddr|pbinFieldStorageAddr, 0, nil, - append(pbinTestLenAndVal(bytes.Repeat([]byte{0xEE}, length.Addr)), pbinTestLenAndVal(bytes.Repeat([]byte{0xEE}, length.Addr+length.Hash))...)...))}, + {"leaf without a plain key", pbinTestRecord(pbinTestCellBody(pbinFieldLeaf, 0, nil), body)}, + {"leaf naming both plain keys", pbinTestRecord(pbinTestCellBody(pbinFieldLeaf|pbinFieldAccountAddr|pbinFieldStorageAddr, 0, nil, + append(pbinTestFixedVal(bytes.Repeat([]byte{0xEE}, length.Addr)), pbinTestFixedVal(bytes.Repeat([]byte{0xEE}, length.Addr+length.Hash))...)...))}, // A record-resident value and a plain key are two answers to the same // question; a branch has no value at all. - {"leaf naming a plain key and a record value", pbinTestRecord(0b01, 0b01, pbinTestCellBody(pbinFieldLeaf|pbinFieldAccountAddr|pbinFieldLeafValue, 0, nil, - append(pbinTestLenAndVal(bytes.Repeat([]byte{0xEE}, length.Addr)), pbinTestLenAndVal(bytes.Repeat([]byte{0xEE}, pbinValueLength))...)...))}, - {"branch carrying a record value", pbinTestRecord(0b01, 0b01, pbinTestCellBody(pbinFieldBranch|pbinFieldLeafValue, 0, nil, - pbinTestLenAndVal(bytes.Repeat([]byte{0xEE}, pbinValueLength))...))}, - {"record value shorter than a leaf value", pbinTestRecord(0b01, 0b01, pbinTestCellBody(pbinFieldLeaf|pbinFieldLeafValue, 0, nil, - pbinTestLenAndVal(bytes.Repeat([]byte{0xEE}, pbinValueLength-1))...))}, - {"truncated record value", pbinTestRecord(0b01, 0b01, pbinTestCellBody(pbinFieldLeaf|pbinFieldLeafValue, 0, nil, pbinValueLength, 0xEE))}, + {"leaf naming a plain key and a record value", pbinTestRecord(pbinTestCellBody(pbinFieldLeaf|pbinFieldAccountAddr|pbinFieldLeafValue, 0, nil, + append(pbinTestFixedVal(bytes.Repeat([]byte{0xEE}, length.Addr)), pbinTestFixedVal(bytes.Repeat([]byte{0xEE}, pbinValueLength))...)...))}, + {"branch carrying a record value", pbinTestRecord(pbinTestCellBody(pbinFieldBranch|pbinFieldLeafValue, 0, nil, + pbinTestFixedVal(bytes.Repeat([]byte{0xEE}, pbinValueLength))...))}, + {"record value shorter than a leaf value", pbinTestRecord(pbinTestCellBody(pbinFieldLeaf|pbinFieldLeafValue, 0, nil, + pbinTestFixedVal(bytes.Repeat([]byte{0xEE}, pbinValueLength-1))...))}, + {"truncated record value", pbinTestRecord(pbinTestCellBody(pbinFieldLeaf|pbinFieldLeafValue, 0, nil, + pbinTestFixedVal(bytes.Repeat([]byte{0xEE}, pbinValueLength-1))...), body)}, } { t.Run(tc.name, func(t *testing.T) { t.Parallel() var cells [2]pbinCell - _, _, err := pbinDecodeBranch(tc.rec, &cells) + _, err := pbinDecodeBranch(tc.rec, &cells, 0, nil) require.Error(t, err) }) } @@ -268,6 +463,9 @@ func TestPBinBranchCodecDropsLoadedState(t *testing.T) { t.Parallel() cells := [2]pbinCell{pbinTestLeafCell(0x2B, 40), pbinTestBranchCell(0x4D, 8)} + keys := pbinDigestCache{sum: pbinSelectedSum} + storageKey := pbinPathFromBytes(keys.storageKey(cells[0].storageAddr[:length.Addr], cells[0].storageAddr[length.Addr:])) + cells[0].prefix = storageKey cells[0].loaded = cellLoadStorage cells[0].Nonce = 9 cells[0].Flags = NonceUpdate @@ -277,7 +475,7 @@ func TestPBinBranchCodecDropsLoadedState(t *testing.T) { require.NoError(t, err) var got [2]pbinCell - _, _, err = pbinDecodeBranch(bytes.Clone(rec), &got) + _, err = pbinDecodeBranch(bytes.Clone(rec), &got, 0, &keys) require.NoError(t, err) require.Equal(t, cellLoadNone, got[0].loaded) require.Zero(t, got[0].Nonce) @@ -289,14 +487,14 @@ func TestPBinBranchCodecDropsLoadedState(t *testing.T) { func TestPBinBranchDecodeClearsReusedCells(t *testing.T) { t.Parallel() - cells := [2]pbinCell{pbinTestLeafCell(0xFF, 528), pbinTestLeafCell(0xFF, 528)} + cells := [2]pbinCell{pbinTestChunkLeafCell(0xFF, 528), pbinTestChunkLeafCell(0xFF, 528)} want := [2]pbinCell{pbinTestBranchCell(0x0F, 3), pbinTestBranchCell(0xF0, 0)} var enc pbinBranchEncoder rec, err := enc.encode(0b11, 0b11, &want) require.NoError(t, err) - _, _, err = pbinDecodeBranch(bytes.Clone(rec), &cells) + _, err = pbinDecodeBranch(bytes.Clone(rec), &cells, 0, nil) require.NoError(t, err) require.Equal(t, want, cells) } @@ -367,3 +565,29 @@ func TestPBinGridBounds(t *testing.T) { require.Equal(t, pbinGridRows, len(g.depths)) require.Equal(t, 2, len(g.rows[0])) } + +// A branch cell never owns a plain key, so the encoder cannot spell one. Left +// admissible, the omitted-prefix path would rebuild a full leaf path for what is +// only a partial extension. +func TestPBinBranchCodecRejectsBranchCellWithAddress(t *testing.T) { + t.Parallel() + + for _, tc := range []struct { + name string + mut func(*pbinCell) + }{ + {"storage address", func(c *pbinCell) { c.storageAddrLen = length.Addr + length.Hash }}, + {"account address", func(c *pbinCell) { c.accountAddrLen = length.Addr }}, + } { + t.Run(tc.name, func(t *testing.T) { + c := pbinTestBranchCell(0xA5, 12) + tc.mut(&c) + rec, err := pbinAppendCell(nil, &c, false) + require.NoError(t, err) + + var got pbinCell + _, err = pbinDecodeCell(rec, 0, &got, 0, nil, false) + require.ErrorIs(t, err, errPBinMalformedBranch) + }) + } +} diff --git a/execution/commitment/pbin_code_test.go b/execution/commitment/pbin_code_test.go index 454f70a70e7..0eaf82b8724 100644 --- a/execution/commitment/pbin_code_test.go +++ b/execution/commitment/pbin_code_test.go @@ -377,7 +377,6 @@ func TestPBinLeafValueRoutesByZone(t *testing.T) { func TestPBinLeafCellHashChecksZoneLength(t *testing.T) { t.Parallel() - var h pbinHasher u := Update{Flags: StorageUpdate, StorageLen: pbinValueLength} for _, tc := range []struct { @@ -390,6 +389,8 @@ func TestPBinLeafCellHashChecksZoneLength(t *testing.T) { } { t.Run(tc.name, func(t *testing.T) { t.Parallel() + // pbinHasher carries a scratch buffer, so a parallel subtest needs its own. + var h pbinHasher c := pbinCell{kind: pbinNodeLeaf, prefix: pbinPathFromBytes(tc.key), Update: u} var path pbinBitpath _, err := h.cellHash(&c, &path) diff --git a/execution/commitment/pbin_convert_legacy.go b/execution/commitment/pbin_convert_legacy.go new file mode 100644 index 00000000000..94fc0428db6 --- /dev/null +++ b/execution/commitment/pbin_convert_legacy.go @@ -0,0 +1,486 @@ +// Copyright 2026 The Erigon Authors +// This file is part of Erigon. +// +// Erigon is free software: you can redistribute it and/or modify +// it under the terms of the GNU Lesser General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// Erigon is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Lesser General Public License for more details. +// +// You should have received a copy of the GNU Lesser General Public License +// along with Erigon. If not, see . + +package commitment + +import ( + "encoding/binary" + "fmt" + "math/bits" + + "github.com/erigontech/erigon/common/length" +) + +// Reading and writing the record format that predates pbinRecordFormat. The +// decoder serves the one-way datadir conversion, while the encoders also build +// legacy test corpora. A legacy record spells its cells with a touchMap/afterMap +// header and a uvarint length on every field; the current one spells neither. + +// PBinRecordConverter rewrites legacy records. It is not safe for concurrent use. +type PBinRecordConverter struct { + enc pbinBranchEncoder + keys pbinDigestCache +} + +func NewPBinRecordConverter() *PBinRecordConverter { + return &PBinRecordConverter{keys: pbinDigestCache{sum: pbinSelectedSum}} +} + +// PBinEncodeLegacyRecord rewrites a current branch record in the pre-version +// format. The key is needed to restore storage prefixes omitted by the current +// record format. +func PBinEncodeLegacyRecord(key, current []byte) ([]byte, error) { + if len(current) > 0 && current[0] == 0 { + return nil, fmt.Errorf("pbin encode legacy: input is already a legacy record") + } + + path, err := pbinDecodeBitPath(key) + if err != nil { + return nil, fmt.Errorf("pbin encode legacy: record key %x: %w", key, err) + } + converter := NewPBinRecordConverter() + var cells [2]pbinCell + if _, err = pbinDecodeBranch(current, &cells, path.bitLen+1, &converter.keys); err != nil { + return nil, fmt.Errorf("pbin encode legacy: record at %x: %w", key, err) + } + + out := binary.BigEndian.AppendUint16(nil, pbinCellBits) + out = binary.BigEndian.AppendUint16(out, pbinCellBits) + for bit := range cells { + if out, err = pbinEncodeLegacyCell(out, &cells[bit]); err != nil { + return nil, fmt.Errorf("pbin encode legacy: record at %x: %w", key, err) + } + } + return out, nil +} + +// PBinEncodeLegacyState rewrites a current trie state blob in the pre-version +// format. The root cell keeps its flags and prefix, but its fields gain lengths. +func PBinEncodeLegacyState(current []byte) ([]byte, error) { + if len(current) >= 2 && current[0] == pbinStateMarker && current[1] != pbinRecordFormat && + current[1] <= pbinStateFlagsAll { + return nil, fmt.Errorf("pbin encode legacy: input is already a legacy state blob") + } + if err := ValidatePBinStateFormat(current); err != nil { + return nil, fmt.Errorf("pbin encode legacy: %w", err) + } + if len(current) < 5 { + return nil, fmt.Errorf("pbin encode legacy: %w: header is %d bytes, want at least 5", errPBinStateBlob, len(current)) + } + flags := current[2] + if flags&^byte(pbinStateFlagsAll) != 0 { + return nil, fmt.Errorf("pbin encode legacy: %w: unknown flags %08b", errPBinStateBlob, flags) + } + rootLen := int(binary.BigEndian.Uint16(current[3:5])) + if len(current) != 5+rootLen { + return nil, fmt.Errorf("pbin encode legacy: %w: root cell of %d bytes in a %d-byte blob", errPBinStateBlob, rootLen, len(current)) + } + + out := []byte{pbinStateMarker, flags, 0, 0} + if rootLen == 0 { + return out, nil + } + var root pbinCell + pos, err := pbinDecodeCell(current, 5, &root, 0, nil, false) + if err != nil { + return nil, fmt.Errorf("pbin encode legacy: state root cell: %w", err) + } + if pos != len(current) { + return nil, fmt.Errorf("pbin encode legacy: %w: %d trailing bytes after the root cell", errPBinStateBlob, len(current)-pos) + } + if out, err = pbinEncodeLegacyCell(out, &root); err != nil { + return nil, fmt.Errorf("pbin encode legacy: state root cell: %w", err) + } + binary.BigEndian.PutUint16(out[2:4], uint16(len(out)-4)) + return out, nil +} + +// PBinEncodeLegacyRootRecord rewrites a current root-cell record in the +// pre-version format. The record is a bare cell: no branch header, and a prefix +// that is never omitted. +func PBinEncodeLegacyRootRecord(current []byte) ([]byte, error) { + if len(current) == 0 { + return nil, nil + } + var root pbinCell + pos, err := pbinDecodeCell(current, 0, &root, 0, nil, false) + if err != nil { + return nil, fmt.Errorf("pbin encode legacy: root record: %w", err) + } + if pos != len(current) { + return nil, fmt.Errorf("%w: %d trailing bytes after the root cell", errPBinMalformedBranch, len(current)-pos) + } + return pbinEncodeLegacyCell(nil, &root) +} + +// PBinRootRecordIsLegacy reports whether a root-cell record still spells its +// fields with lengths. The current format gives every field a fixed width, so a +// legacy record always leaves its length bytes over. +func PBinRootRecordIsLegacy(data []byte) bool { + if len(data) == 0 { + return false + } + var root pbinCell + pos, err := pbinDecodeCell(data, 0, &root, 0, nil, false) + return err != nil || pos != len(data) +} + +func pbinEncodeLegacyCell(dst []byte, c *pbinCell) ([]byte, error) { + var fields pbinCellFields + switch c.kind { + case pbinNodeLeaf: + fields = pbinFieldLeaf + case pbinNodeBranch: + fields = pbinFieldBranch + default: + return nil, fmt.Errorf("%w: cell has no node kind", errPBinMalformedBranch) + } + if c.accountAddrLen > 0 { + fields |= pbinFieldAccountAddr + } + if c.storageAddrLen > 0 { + fields |= pbinFieldStorageAddr + } + if c.kind == pbinNodeLeaf && fields&pbinFieldValue == 0 { + fields |= pbinFieldLeafValue + } + if c.hashLen > 0 { + fields |= pbinFieldHash + } + + dst = append(dst, byte(fields)) + dst = binary.AppendUvarint(dst, uint64(c.prefix.bitLen)) + dst = c.prefix.appendPackedBits(dst) + appendValue := func(value []byte) { + dst = binary.AppendUvarint(dst, uint64(len(value))) + dst = append(dst, value...) + } + if fields&pbinFieldAccountAddr != 0 { + appendValue(c.accountAddr[:c.accountAddrLen]) + } + if fields&pbinFieldStorageAddr != 0 { + appendValue(c.storageAddr[:c.storageAddrLen]) + } + if fields&pbinFieldLeafValue != 0 { + value, err := pbinRecordLeafValue(&c.Update) + if err != nil { + return nil, err + } + appendValue(value[:]) + } + if fields&pbinFieldHash != 0 { + appendValue(c.hash[:c.hashLen]) + } + return dst, nil +} + +// ConvertBranch rewrites one legacy branch record. key is the record's own DB +// key, which carries the node path and therefore the depth the current format +// reconstructs omitted storage prefixes from. +// +// A legacy record naming one cell panics. The fold collapses a single survivor +// into its parent (foldPropagate) and only foldBranch writes a record, so such a +// record cannot come from this algorithm — it means the input was written by +// something else, and converting it would invent a node. +func (c *PBinRecordConverter) ConvertBranch(key, data []byte) ([]byte, error) { + path, err := pbinDecodeBitPath(key) + if err != nil { + return nil, fmt.Errorf("pbin convert: record key %x: %w", key, err) + } + depth := path.bitLen + 1 + + var cells [2]pbinCell + touchMap, afterMap, err := pbinLegacyDecodeBranch(data, &cells) + if err != nil { + return nil, fmt.Errorf("pbin convert: record at %x: %w", key, err) + } + if n := bits.OnesCount16(afterMap); n != 2 { + panic(fmt.Sprintf("pbin convert: record at %x names %d cells (afterMap %04b); "+ + "a one-cell node is collapsed by foldPropagate and never stored", key, n, afterMap)) + } + + out, err := c.enc.encode(touchMap, afterMap, &cells) + if err != nil { + return nil, fmt.Errorf("pbin convert: re-encode at %x: %w", key, err) + } + out = append([]byte(nil), out...) + + // The current format drops a storage leaf's prefix and rebuilds it from the + // address and this depth. Reading the result back is the only thing that + // proves the dropped bits were the derivable ones. + var got [2]pbinCell + if _, err = pbinDecodeBranch(out, &got, depth, &c.keys); err != nil { + return nil, fmt.Errorf("pbin convert: verify at %x: %w", key, err) + } + for bit := range cells { + if got[bit] != cells[bit] { + return nil, fmt.Errorf("pbin convert: record at %x cell %d does not round-trip", key, bit) + } + } + return out, nil +} + +// ConvertRootRecord rewrites the bare cell stored under the root key. It has no +// branch header, so the leading zero that marks a legacy branch record is absent +// and the key is the only thing that names it. +func (c *PBinRecordConverter) ConvertRootRecord(data []byte) ([]byte, error) { + if len(data) == 0 { + return nil, nil + } + var root pbinCell + pos, err := pbinLegacyDecodeCell(data, 0, &root) + if err != nil { + return nil, fmt.Errorf("pbin convert: root record: %w", err) + } + if pos != len(data) { + return nil, fmt.Errorf("%w: %d trailing bytes after the root cell", errPBinMalformedBranch, len(data)-pos) + } + out, err := pbinAppendCell(nil, &root, false) + if err != nil { + return nil, fmt.Errorf("pbin convert: root record: %w", err) + } + var got pbinCell + if pos, err = pbinDecodeCell(out, 0, &got, 0, &c.keys, false); err != nil { + return nil, fmt.Errorf("pbin convert: verify root record: %w", err) + } + if pos != len(out) || got != root { + return nil, fmt.Errorf("pbin convert: root record does not round-trip") + } + return out, nil +} + +// CompareLegacy checks that a current record preserves the cells in its legacy +// spelling. key supplies the depth needed to reconstruct omitted storage prefixes. +func (c *PBinRecordConverter) CompareLegacy(key, legacy, current []byte) error { + path, err := pbinDecodeBitPath(key) + if err != nil { + return fmt.Errorf("pbin compare: record key %x: %w", key, err) + } + + var legacyCells [2]pbinCell + if _, _, err = pbinLegacyDecodeBranch(legacy, &legacyCells); err != nil { + return fmt.Errorf("pbin compare: legacy record at %x: %w", key, err) + } + + var currentCells [2]pbinCell + if _, err = pbinDecodeBranch(current, ¤tCells, path.bitLen+1, &c.keys); err != nil { + return fmt.Errorf("pbin compare: current record at %x: %w", key, err) + } + for bit := range legacyCells { + if legacyCells[bit] != currentCells[bit] { + return fmt.Errorf("pbin compare: record at %x cell %d does not match", key, bit) + } + } + return nil +} + +// ConvertState rewrites the trie state blob, which gains the format byte and +// loses the field lengths inside its root cell. +func (c *PBinRecordConverter) ConvertState(blob []byte) ([]byte, error) { + if len(blob) == 0 { + return nil, nil + } + if len(blob) < 4 || blob[0] != pbinStateMarker { + return nil, fmt.Errorf("%w: not a legacy pbin blob", errPBinStateBlob) + } + flags := blob[1] + if flags&^byte(pbinStateFlagsAll) != 0 { + return nil, fmt.Errorf("%w: unknown flags %08b", errPBinStateBlob, flags) + } + rootLen := int(binary.BigEndian.Uint16(blob[2:4])) + if len(blob) != 4+rootLen { + return nil, fmt.Errorf("%w: root cell of %d bytes in a %d-byte blob", errPBinStateBlob, rootLen, len(blob)) + } + + out := []byte{pbinStateMarker, pbinRecordFormat, flags, 0, 0} + if rootLen > 0 { + var root pbinCell + pos, err := pbinLegacyDecodeCell(blob, 4, &root) + if err != nil { + return nil, fmt.Errorf("pbin convert: state root cell: %w", err) + } + if pos != len(blob) { + return nil, fmt.Errorf("%w: %d trailing bytes after the root cell", errPBinStateBlob, len(blob)-pos) + } + if out, err = pbinAppendCell(out, &root, false); err != nil { + return nil, fmt.Errorf("pbin convert: state root cell: %w", err) + } + } + binary.BigEndian.PutUint16(out[3:5], uint16(len(out)-5)) + return out, nil +} + +// LegacyStateRoot hashes the root cell in a pre-version state blob without +// restoring it into an engine that only accepts the current format. +func (c *PBinRecordConverter) LegacyStateRoot(blob []byte) ([]byte, error) { + if len(blob) < 4 || blob[0] != pbinStateMarker { + return nil, fmt.Errorf("%w: not a legacy pbin blob", errPBinStateBlob) + } + flags := blob[1] + if flags&^byte(pbinStateFlagsAll) != 0 { + return nil, fmt.Errorf("%w: unknown flags %08b", errPBinStateBlob, flags) + } + rootLen := int(binary.BigEndian.Uint16(blob[2:4])) + if len(blob) != 4+rootLen { + return nil, fmt.Errorf("%w: root cell of %d bytes in a %d-byte blob", errPBinStateBlob, rootLen, len(blob)) + } + + var root pbinCell + if rootLen > 0 { + pos, err := pbinLegacyDecodeCell(blob, 4, &root) + if err != nil { + return nil, fmt.Errorf("pbin compare: state root cell: %w", err) + } + if pos != len(blob) { + return nil, fmt.Errorf("%w: %d trailing bytes after the root cell", errPBinStateBlob, len(blob)-pos) + } + } + + hasher := pbinHasher{sum: c.keys.sum} + hash, err := hasher.cellHash(&root, new(pbinBitpath)) + if err != nil { + return nil, fmt.Errorf("pbin compare: state root: %w", err) + } + return hash[:], nil +} + +// CurrentStateRoot hashes the root cell in a current-format state blob without +// restoring it into an engine that needs a database context. +func (c *PBinRecordConverter) CurrentStateRoot(blob []byte) ([]byte, error) { + if err := ValidatePBinStateFormat(blob); err != nil { + return nil, err + } + if len(blob) < 5 { + return nil, fmt.Errorf("%w: header is %d bytes, want at least 5", errPBinStateBlob, len(blob)) + } + flags := blob[2] + if flags&^byte(pbinStateFlagsAll) != 0 { + return nil, fmt.Errorf("%w: unknown flags %08b", errPBinStateBlob, flags) + } + rootLen := int(binary.BigEndian.Uint16(blob[3:5])) + if len(blob) != 5+rootLen { + return nil, fmt.Errorf("%w: root cell of %d bytes in a %d-byte blob", errPBinStateBlob, rootLen, len(blob)) + } + + var root pbinCell + if rootLen > 0 { + pos, err := pbinDecodeCell(blob, 5, &root, 0, &c.keys, false) + if err != nil { + return nil, fmt.Errorf("pbin state root: %w", err) + } + if pos != len(blob) { + return nil, fmt.Errorf("%w: %d trailing bytes after the root cell", errPBinStateBlob, len(blob)-pos) + } + } + + hasher := pbinHasher{sum: c.keys.sum} + hash, err := hasher.cellHash(&root, new(pbinBitpath)) + if err != nil { + return nil, fmt.Errorf("pbin state root: %w", err) + } + return hash[:], nil +} + +func pbinLegacyDecodeBranch(data []byte, cells *[2]pbinCell) (touchMap, afterMap uint16, err error) { + cells[0].reset() + cells[1].reset() + + if len(data) < 4 { + return 0, 0, fmt.Errorf("%w: %d bytes is shorter than the legacy header", errPBinMalformedBranch, len(data)) + } + touchMap, afterMap = binary.BigEndian.Uint16(data), binary.BigEndian.Uint16(data[2:]) + if err := pbinCheckCellMaps(touchMap, afterMap); err != nil { + return 0, 0, err + } + + pos := 4 + for bitset := afterMap; bitset != 0; { + bit := bitset & -bitset + if pos, err = pbinLegacyDecodeCell(data, pos, &cells[bits.TrailingZeros16(bit)]); err != nil { + return 0, 0, err + } + bitset ^= bit + } + if pos != len(data) { + return 0, 0, fmt.Errorf("%w: %d trailing bytes", errPBinMalformedBranch, len(data)-pos) + } + return touchMap, afterMap, nil +} + +func pbinLegacyDecodeCell(data []byte, pos int, c *pbinCell) (int, error) { + if pos >= len(data) { + return 0, fmt.Errorf("%w: no cell body at offset %d", errPBinMalformedBranch, pos) + } + fields := pbinCellFields(data[pos]) + pos++ + if fields&^pbinFieldsAll != 0 { + return 0, fmt.Errorf("%w: unknown cell fields %08b", errPBinMalformedBranch, fields) + } + switch fields & pbinFieldKind { + case pbinFieldLeaf: + c.kind = pbinNodeLeaf + case pbinFieldBranch: + c.kind = pbinNodeBranch + default: + return 0, fmt.Errorf("%w: cell fields %08b name no single node kind", errPBinMalformedBranch, fields) + } + + var err error + if pos, err = pbinDecodePrefix(data, pos, c); err != nil { + return 0, err + } + if fields&pbinFieldAccountAddr != 0 { + if pos, err = pbinLegacyDecodeVal(data, pos, c.accountAddr[:], length.Addr); err != nil { + return 0, err + } + c.accountAddrLen = length.Addr + } + if fields&pbinFieldStorageAddr != 0 { + if pos, err = pbinLegacyDecodeVal(data, pos, c.storageAddr[:], length.Addr+length.Hash); err != nil { + return 0, err + } + c.storageAddrLen = length.Addr + length.Hash + } + if fields&pbinFieldLeafValue != 0 { + if pos, err = pbinLegacyDecodeVal(data, pos, c.Storage[:], pbinValueLength); err != nil { + return 0, err + } + c.Flags, c.StorageLen = StorageUpdate, pbinValueLength + } + if fields&pbinFieldHash != 0 { + if pos, err = pbinLegacyDecodeVal(data, pos, c.hash[:], length.Hash); err != nil { + return 0, err + } + c.hashLen = length.Hash + } + return pos, nil +} + +func pbinLegacyDecodeVal(data []byte, pos int, dst []byte, want int) (int, error) { + n, read := binary.Uvarint(data[pos:]) + if read <= 0 { + return 0, fmt.Errorf("%w: unreadable field length at offset %d", errPBinMalformedBranch, pos) + } + pos += read + if int(n) != want { + return 0, fmt.Errorf("%w: field of %d bytes, want %d", errPBinMalformedBranch, n, want) + } + if pos+want > len(data) { + return 0, fmt.Errorf("%w: field of %d bytes needs more than the %d left", errPBinMalformedBranch, want, len(data)-pos) + } + copy(dst, data[pos:pos+want]) + return pos + want, nil +} diff --git a/execution/commitment/pbin_convert_legacy_test.go b/execution/commitment/pbin_convert_legacy_test.go new file mode 100644 index 00000000000..ab2d709aada --- /dev/null +++ b/execution/commitment/pbin_convert_legacy_test.go @@ -0,0 +1,363 @@ +// Copyright 2026 The Erigon Authors +// This file is part of Erigon. +// +// Erigon is free software: you can redistribute it and/or modify +// it under the terms of the GNU Lesser General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// Erigon is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Lesser General Public License for more details. +// +// You should have received a copy of the GNU Lesser General Public License +// along with Erigon. If not, see . + +package commitment + +import ( + "encoding/binary" + "fmt" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/erigontech/erigon/common/length" +) + +// pbinTestLegacyAppendCell spells a cell the way the pre-version format did: a +// uvarint length ahead of every field, and a prefix on every cell including a +// storage leaf. It exists so the converter can be tested against real legacy +// bytes rather than against its own reader. +func pbinTestLegacyAppendCell(dst []byte, c *pbinCell) []byte { + var fields pbinCellFields + switch c.kind { + case pbinNodeLeaf: + fields = pbinFieldLeaf + case pbinNodeBranch: + fields = pbinFieldBranch + } + if c.accountAddrLen > 0 { + fields |= pbinFieldAccountAddr + } + if c.storageAddrLen > 0 { + fields |= pbinFieldStorageAddr + } + if c.kind == pbinNodeLeaf && fields&pbinFieldValue == 0 { + fields |= pbinFieldLeafValue + } + if c.hashLen > 0 { + fields |= pbinFieldHash + } + + lenAndVal := func(dst, v []byte) []byte { + return append(binary.AppendUvarint(dst, uint64(len(v))), v...) + } + dst = append(dst, byte(fields)) + dst = binary.AppendUvarint(dst, uint64(c.prefix.bitLen)) + dst = c.prefix.appendPackedBits(dst) + if fields&pbinFieldAccountAddr != 0 { + dst = lenAndVal(dst, c.accountAddr[:c.accountAddrLen]) + } + if fields&pbinFieldStorageAddr != 0 { + dst = lenAndVal(dst, c.storageAddr[:c.storageAddrLen]) + } + if fields&pbinFieldLeafValue != 0 { + dst = lenAndVal(dst, c.Storage[:pbinValueLength]) + } + if fields&pbinFieldHash != 0 { + dst = lenAndVal(dst, c.hash[:c.hashLen]) + } + return dst +} + +func pbinTestLegacyRecord(touchMap, afterMap uint16, cells *[2]pbinCell) []byte { + out := binary.BigEndian.AppendUint16(nil, touchMap) + out = binary.BigEndian.AppendUint16(out, afterMap) + for bit := range cells { + if afterMap&(uint16(1)< 0 && buf[0] == pbinStateMarker +} + +// ValidatePBinStateFormat checks the pbin state marker and record format. +func ValidatePBinStateFormat(buf []byte) error { + if len(buf) < 2 || !IsPBinState(buf) { + return fmt.Errorf("%w: not a pbin blob", errPBinStateBlob) + } + if buf[1] != pbinRecordFormat { + return fmt.Errorf("%w: record format version %d, want %d", errPBinStateBlob, buf[1], pbinRecordFormat) + } + return nil +} + // SetState is the inverse of EncodeCurrentState; an empty blob resets the engine. func (pph *PBinPatriciaHashed) SetState(buf []byte) error { if pph.grid.activeRows != 0 { @@ -79,18 +100,24 @@ func (pph *PBinPatriciaHashed) SetState(buf []byte) error { if len(buf) == 0 { return nil } - if len(buf) < 4 || buf[0] != pbinStateMarker { + if len(buf) < 2 || buf[0] != pbinStateMarker { return fmt.Errorf("%w: not a pbin blob", errPBinStateBlob) } - flags := buf[1] + if err := ValidatePBinStateFormat(buf); err != nil { + return err + } + if len(buf) < 5 { + return fmt.Errorf("%w: header is %d bytes, want at least 5", errPBinStateBlob, len(buf)) + } + flags := buf[2] if flags&^byte(pbinStateFlagsAll) != 0 { return fmt.Errorf("%w: unknown flags %08b", errPBinStateBlob, flags) } - if rootLen := int(binary.BigEndian.Uint16(buf[2:4])); len(buf) != 4+rootLen { + if rootLen := int(binary.BigEndian.Uint16(buf[3:5])); len(buf) != 5+rootLen { return fmt.Errorf("%w: root cell of %d bytes in a %d-byte blob", errPBinStateBlob, rootLen, len(buf)) } - if len(buf) > 4 { - pos, err := pbinDecodeCell(buf, 4, &pph.grid.root) + if len(buf) > 5 { + pos, err := pbinDecodeCell(buf, 5, &pph.grid.root, 0, &pph.updateStream.keyDigest, false) if err == nil && pos != len(buf) { err = fmt.Errorf("%w: %d trailing bytes after the root cell", errPBinStateBlob, len(buf)-pos) } diff --git a/execution/commitment/pbin_state_test.go b/execution/commitment/pbin_state_test.go index ab7a83f3987..3e687c98987 100644 --- a/execution/commitment/pbin_state_test.go +++ b/execution/commitment/pbin_state_test.go @@ -44,6 +44,8 @@ func TestPBinRestartRoundTripDeepPath(t *testing.T) { blob, err := pph.EncodeCurrentState(nil) require.NoError(t, err) + require.GreaterOrEqual(t, len(blob), 2) + require.Equal(t, byte(pbinRecordFormat), blob[1]) restored := NewPBinPatriciaHashed(ms) require.NoError(t, restored.SetState(blob)) @@ -85,6 +87,35 @@ func TestPBinStateBlobRoundTripsFlags(t *testing.T) { require.Equal(t, storedRoot, root) } +func TestPBinRootAndStateRoundTripFixedFields(t *testing.T) { + t.Parallel() + + pph, ms := pbinTestEngine(t) + root := pbinTestLeafCell(0x63, 0) + pph.grid.root = root + pph.rootPresent = true + pph.rootChecked = true + pph.rootTouched = true + + rootRecord, err := pbinAppendCell(nil, &root, false) + require.NoError(t, err) + require.NoError(t, ms.PutBranch(pbinRootKey, rootRecord, nil)) + + loaded := NewPBinPatriciaHashed(ms) + require.NoError(t, loaded.loadRoot()) + require.Equal(t, root, loaded.grid.root) + + state, err := pph.EncodeCurrentState(nil) + require.NoError(t, err) + require.Equal(t, rootRecord, state[5:]) + restored := NewPBinPatriciaHashed(ms) + require.NoError(t, restored.SetState(state)) + require.Equal(t, root, restored.grid.root) + require.True(t, restored.rootPresent) + require.True(t, restored.rootChecked) + require.True(t, restored.rootTouched) +} + // Following the hex convention, no state blob resets the engine; the tree is // then found again through the stored root record rather than lost. func TestPBinSetStateEmptyResetsToStored(t *testing.T) { @@ -128,6 +159,43 @@ func TestPBinSetStateRejectsForeignBlob(t *testing.T) { } } +func TestPBinSetStateRejectsUnsupportedRecordFormat(t *testing.T) { + t.Parallel() + + pph, ms := pbinTestEngine(t) + blob, err := pph.EncodeCurrentState(nil) + require.NoError(t, err) + blob[1] = 0x42 + + fresh := NewPBinPatriciaHashed(ms) + err = fresh.SetState(blob) + require.Error(t, err) + require.ErrorContains(t, err, "record format") + require.ErrorContains(t, err, "66") +} + +func TestPBinSetStateRejectsPreVersionBlob(t *testing.T) { + t.Parallel() + + _, ms := pbinTestEngine(t) + + legacy := []byte{pbinStateMarker, 0, 0, 0} + fresh := NewPBinPatriciaHashed(ms) + err := fresh.SetState(legacy) + require.ErrorIs(t, err, errPBinStateBlob) +} + +func TestPBinRejectsEveryPreVersionFlagsByte(t *testing.T) { + t.Parallel() + + _, ms := pbinTestEngine(t) + for flags := byte(0); flags <= pbinStateFlagsAll; flags++ { + legacy := []byte{pbinStateMarker, flags, 0, 0} + require.ErrorIs(t, ValidatePBinStateFormat(legacy), errPBinStateBlob, "flags %08b", flags) + require.ErrorIs(t, NewPBinPatriciaHashed(ms).SetState(legacy), errPBinStateBlob, "flags %08b", flags) + } +} + // With a row still open, part of the tree lives in the grid arrays and a // root-cell snapshot would silently drop it. func TestPBinStateRefusesOpenRows(t *testing.T) { diff --git a/execution/commitment/pbin_unfold_test.go b/execution/commitment/pbin_unfold_test.go index 5035c33bcb2..b7d0f043bc7 100644 --- a/execution/commitment/pbin_unfold_test.go +++ b/execution/commitment/pbin_unfold_test.go @@ -42,9 +42,8 @@ func pbinTestSpecCell(t *testing.T, kind pbinNodeKind, spec string) pbinCell { c.prefix = pbinTestPathFromBits(t, pbinTestBitSpec(t, spec)) switch kind { case pbinNodeLeaf: - // A stored leaf always names a plain key; a record without one is rejected. - c.storageAddrLen = length.Addr + length.Hash - c.storageAddr[0], c.storageAddr[1] = 0xB1, byte(len(spec)) + c.accountAddrLen = length.Addr + c.accountAddr[0], c.accountAddr[1] = 0xB1, byte(len(spec)) case pbinNodeBranch: c.hash = common.Hash{0xB1, byte(len(spec))} c.hashLen = length.Hash @@ -62,7 +61,7 @@ func pbinTestPutRecord(t *testing.T, ms *MockState, path pbinBitpath, cells [2]p func pbinTestPutRootCell(t *testing.T, ms *MockState, c pbinCell) { t.Helper() - rec, err := pbinAppendCell(nil, &c) + rec, err := pbinAppendCell(nil, &c, false) require.NoError(t, err) require.NoError(t, ms.PutBranch(pbinRootKey, rec, nil)) } diff --git a/execution/commitment/pbin_verify_test.go b/execution/commitment/pbin_verify_test.go index ac0b10bbc69..94d70c18f1c 100644 --- a/execution/commitment/pbin_verify_test.go +++ b/execution/commitment/pbin_verify_test.go @@ -101,7 +101,7 @@ func (v *pbinVerifier) rootCell() (pbinCell, error) { if len(data) == 0 { return c, errPBinVerifyNoRecords } - pos, err := pbinDecodeCell(data, 0, &c) + pos, err := pbinDecodeCell(data, 0, &c, 0, nil, false) if err != nil { return c, fmt.Errorf("pbin verify: root cell: %w", err) } @@ -202,7 +202,8 @@ func (v *pbinVerifier) recordAt(nodePath *pbinBitpath) ([2]pbinCell, error) { if len(data) == 0 { return cells, fmt.Errorf("pbin verify: no record for the %d-bit node at %x", nodePath.bitLen, key) } - _, afterMap, err := pbinDecodeBranch(data, &cells) + keys := pbinDigestCache{sum: pbinSelectedSum} + afterMap, err := pbinDecodeBranch(data, &cells, nodePath.bitLen+1, &keys) if err != nil { return cells, fmt.Errorf("pbin verify: record at %x: %w", key, err) } diff --git a/execution/commitment/pbin_witness_context.go b/execution/commitment/pbin_witness_context.go index ec396ba2c99..372012fbf48 100644 --- a/execution/commitment/pbin_witness_context.go +++ b/execution/commitment/pbin_witness_context.go @@ -149,7 +149,7 @@ func (c *pbinWitnessContext) rootRecord() ([]byte, error) { if err := c.fillCell(&cell, c.tree.root, &path); err != nil { return nil, err } - return pbinAppendCell(nil, &cell) + return pbinAppendCell(nil, &cell, false) } func (c *pbinWitnessContext) branchRecord(node *pbinWitnessNode, path *pbinBitpath) ([]byte, error) { diff --git a/execution/state/genesiswrite/pbin_genesis_test.go b/execution/state/genesiswrite/pbin_genesis_test.go index 5b0b582371a..c21e9bdb718 100644 --- a/execution/state/genesiswrite/pbin_genesis_test.go +++ b/execution/state/genesiswrite/pbin_genesis_test.go @@ -45,11 +45,11 @@ func withBinCommitment(t *testing.T, on bool) { statecfg.BinCommitmentHash = origHash }) statecfg.ExperimentalBinCommitment = on - if on { - // erigondb.toml resolution refuses the combination: the bin trie is - // sequential-only, regardless of a process-wide parallel default. - statecfg.ExperimentalParallelCommitment = false - } else { + // erigondb.toml resolution refuses the combination: the bin trie is + // sequential-only, regardless of a process-wide parallel default. Clearing it + // only when the flag is pre-set misses the case the genesis selects the trie. + statecfg.ExperimentalParallelCommitment = false + if !on { // The hash goes with the flag. A run under COMMITMENT_BIN_HASH leaves it set // otherwise, and the resolver refuses a hash without the trie it names. statecfg.BinCommitmentHash = ""