fullhistory: reorganize packages (#824)

[chowbao]

Closes #824. Closes #835. The streaming/ catch-all was already split into geometry/catalog/ingest/backfill/lifecycle + per-domain stores in the #817→#820 stack; this PR does the leftover moves plus the safe cleanups agreed on the issue.

This is a pure reorganization — moves + mechanical renames, zero behavior change. No production logic is altered.

What changed

1. pkg/ → storage/

pkg/ is meaningless inside an already-internal/ tree. Rather than a flat drop (the issue's literal ask), this renames it to storage/ — a meaningful name for the storage tier that sits below the daemon logic, keeping chunk, rocksdb, and the per-domain stores/ grouped.

Deviation from the issue text, which says "drop the pkg/ folder." Flagging explicitly — happy to flatten fully instead if reviewers prefer.

2. Extract config/

config.go + config_lock.go (schema, load/parse, Paths→Layout, root locks) move into a config/ package. Cycle-safe: only main.go imports the root package and it uses RunDaemon(ctx, path) — no config symbols.

config_validate.go stays in root: validateConfig depends on catalog.Catalog and the root-defined NetworkTipBackend, so it's startup orchestration, not static config — moving it would create a config → root cycle.

3. Extract durable/ (fsync helpers)

Per tamirms' review: the 7 os-level durability ops (BarrierNewFile, FsyncDir, FsyncParentDirs, FsyncNewDirs, DeepestExistingDir, DeleteFileIfExists, RmdirIfEmpty) had zero geometry/domain references — they only sat in geometry/paths.go by accident of sharing the file with the path bijection. They move to a durable/ leaf imported by config, catalog, backfill, and the daemon. geometry is now naming + math only (no os import).

4. Safe ride-along cleanups

• Removed the errLockHeld relay sentinel — acquireLock wraps ErrRootLocked directly (one producer, one consumer). issue comment
• Swept the streaming codename from operator-facing error/log messages: the self-locating prefixes are deleted outright (per the full-history: dead pre-wired surface + stale contract docs (~300 lines deletable) #841 plan — an error's location comes from its wrap chain, so a prefix is redundant whether it says streaming: or fullhistory:); the four Info/Debug log lines follow the same decision. Comments naming a now-nonexistent "streaming package/daemon" reworded. Left domain uses of "streaming", the fullhistory_streaming Prometheus subsystem, and metastore test-key literals (data/interface formats, not codenames).

Open-question resolutions (from #824)

• chunk + geometry: kept separate — the dependency graph forbids merging (chunk is imported by the stores; geometry imports the stores → a merge would cycle).
• hotloop: stays in root — it's the daemon's runtime loop, coupled to daemon.go orchestration.
• progress → root; retention stays in lifecycle — since the soroban-rpc: Replace all hex-encoded contract ids with strkeys #25 clamp deletion, progress.go's only production caller is startup.go, so it moves next to it (unexported lastCommittedLedger). Retention remains lifecycle's freeze/prune policy gate.

5. Behavioral cleanups (second commit)

The logic refactors from the review, kept in a separate commit (fullhistory: hot-create choreography, OpenReady, test-fixture cleanup) so the reorg commit above stays a reviewable pure-reorg:

• cat.BeginHotCreate / cat.FinishHotCreate — the hot-DB create choreography (wipe + transient key → hotchunk.Open → fsync chain → flip ready) moves behind the catalog, mirroring MarkChunkFreezing/FlipChunkFrozen. hotloop.openHotDBForChunk now sandwiches hotchunk.Open between the pair and no longer does raw os/fsync itself. discussion_r3517588234
• hotchunk.OpenReadyWrite / OpenReadyView — a thin named pair over one unexported openReady enforcement site for "ready key ⇒ must-exist, never-creating open," so call sites name the handle instead of passing a naked boolean. openHotDBForChunk (write), refineWithHotDB, and tryHotSource (view) all route through it. hotchunk gains a geometry import for HotState (verified acyclic). discussion_r3517588278
• DeleteHotKey → deleteHotKey unexported (only prod caller is same-package DiscardHotChunk); its raw-key round-trip test moves into the catalog package. PutHotTransient/FlipHotReady stay exported — they're genuine cross-package test-setup primitives.
• Shared fhtest package holds the ZeroTxLCMBytes fixture, replacing three byte-identical copies.

Not done: the review also named testSink/execRecorder for consolidation, but each exists in exactly one package — they aren't duplicated, and merging them would only force package-internal interfaces to be exported. Left as-is.

6. Fold metastore into catalog (finishes #824) + lastCommittedLedger(cat) (finishes #835)

Per review comment: metastore was a 115-line rocksdb wrapper with exactly one consumer. The string-KV surface is now catalog-internal (catalog/kv.go, unexported), and NewCatalog(store, …) becomes catalog.Open(path, layout, txLayout, logger) — the catalog owns its backing store, Close releases it. Gone with the fold: the two-step daemon wiring, the metastore.BatchWriter type threaded through the protocol/sweep closures, the ErrNotFound manufacture-then-translate round-trip in Catalog.get, and metastore.New's guards that duplicated rocksdb.New's. The KV test suite moved to catalog/kv_test.go.

The fold hands over the rest of #835: the catalog now carries a logger, so refineWithHotDB uses cat.Logger(), the logger parameter drops, and lastCommittedLedger(cat) lands as the design wrote it. The deriveCompleteThrough/deriveLastCommitted test-shim aliases collapsed into direct calls (the nil-logger seam #835 worried about was already gone — those tests run against real hot DBs).

One test seam changed shape: backfill's skip-when-coverage-frozen test used to seed raw "pruning" chunk keys on the bare store; with no bare store to reach, it now runs the real SweepChunkArtifacts — which is exactly the finalized-window scenario it was simulating.

Verification

go build, go vet, go test -short ./cmd/stellar-rpc/internal/fullhistory/..., and the repo golangci-lint all green (rocksdb cgo) at every commit. Renames are git-mv (history preserved); import rewrites are mechanical (gofmt -r, AST-based).

[tamirms]

The PR description resolves "progress + retention: stay in lifecycle — freeze/prune policy," but that rationale only fits retention (the tick's gate). Nothing in the lifecycle package has called anything in progress.go since the clamp deletion — its only production caller is startup.go — and #835's recorded scope (from your deferral on #820) includes moving it next to that caller in the fullhistory root. Since this PR is exactly the place for pure moves, suggest doing it here: git mv progress.go to the root, unexport LastCommittedLedger (same package as its caller now), and #835 shrinks to just the logger change. If you'd rather not, no problem — but then the resolution text should say "deferred to #835," not "stays."

[chowbao]

Done in 8fa6c1a: git mv to the root next to startup.go, LastCommittedLedger → unexported lastCommittedLedger, and its three test files moved along (the root test helpers already covered the scaffolding, including openLiveHotDB). The lifecycle tests that used deriveCompleteThrough as a fixture oracle now pass their fixture-known values directly — runTickForCatalog collapsed into direct runLifecycle(…, lastChunk) calls, so tick inputs are explicit rather than re-derived by the code under test's own family. PR description updated; noted on #835 that its scope shrinks to the logger-parameter change.

[tamirms]

The sweep renamed streaming: → fullhistory: on the error strings, but the plan recorded on #841 is to delete these prefixes outright: an error's location comes from its wrap chain (callers already wrap with "read config", "lock root", etc.), so a self-locating prefix is redundant — renaming it just moves the problem from a codename to a package name. Since you're already sweeping every one of these strings in this PR, deleting the prefix instead is the same mechanical pass and closes #841's bullet rather than half-addressing it. If you prefer keeping the prefix, that's shippable too — just needs a note on #841 so its bullet shrinks to the four Info/Debug log lines.

[chowbao]

Done in 8fa6c1a — deleted outright rather than renamed: all 19 remaining error strings (config ×2, config_lock ×6, catalog ×6, geometry ×5; the 7th config_lock one was errLockHeld, already dropped in this PR), and the four Info/Debug log lines followed the same decision since it's the same mechanical pass. Noted on #841 that its prefix bullet is closed here.

[tamirms]

The sweep garbled this line: "(storage/rocksdb, storage/rocksdb, os)" — the second one was pkg/packfile in the base (itself stale; packfile lives at internal/packfile). Should read "(storage/rocksdb, internal/packfile, os)".

[chowbao]

Fixed in 8fa6c1a — now reads "(storage/rocksdb, internal/packfile, os)".

[tamirms]

One codename straggler the sweep missed, at rocksdb.go:579: "The streaming ingestion contract requires…" — that's the retired codename, not the domain word. "The ingestion contract" says it.

[chowbao]

Fixed in 8fa6c1a — "The ingestion contract requires…".

[tamirms]

Optional: the trailing readOnly bool makes the call sites read OpenReady(state, dir, id, logger, false) — a naked boolean the reader has to look up. Two thin names over an unexported core — OpenReadyWrite / OpenReadyView (or similar) — would keep the single enforcement site while letting each call site say what it means.

[chowbao]

Done in 8fa6c1a: OpenReadyWrite / OpenReadyView over an unexported openReady core — the enforcement site stays single, and the three call sites (hotloop resume handle, freeze source, last-committed refiner) now say what they mean without the naked boolean.

[tamirms]

Two more code changes belong in this PR, and they compound:

1. Fold the metastore package into catalog. It's a 115-line wrapper over rocksdb with exactly one consumer — the catalog — and folding it was the one fullhistory/streaming: split the streaming package into purpose-named packages #824 item this PR doesn't address (Streaming daemon: Phase 2 — hot store, lifecycle + live ingestion (closes #816, #808) #820 (comment)). The fold deletes the package, the two-step daemon wiring, and the metastore.BatchWriter type threaded through the protocol closures; NewCatalog(store) becomes catalog.Open(path, logger).
2. That fold hands you the rest of fullhistory: simplify LastCommittedLedger to the design signature LastCommittedLedger(cat) #835 for free. Once the catalog is constructed with a logger, refineWithHotDB can use it, the logger parameter drops, and LastCommittedLedger(cat) lands as the design wrote it. Combined with moving progress.go next to its only caller (other comment), fullhistory: simplify LastCommittedLedger to the design signature LastCommittedLedger(cat) #835 is done — in this PR, in a handful of lines.

With those two, this PR finishes #824 and #835 outright. I'd stop there: the #841 deletions (~300 lines of dead API plus test migrations) are a different kind of change and should be their own PR.

[chowbao]

Both done in a051a06 (and the progress.go move from the inline thread landed in 8fa6c1a):

1. metastore folded into catalog. The string-KV surface is catalog-internal now (catalog/kv.go, unexported), and NewCatalog(store, …) became catalog.Open(path, layout, txLayout, logger) — the catalog owns its backing store, Close releases it. The two-step daemon wiring and the metastore.BatchWriter threading through the protocol/sweep closures are gone, and the fold also deleted the ErrNotFound manufacture-then-translate round-trip in Catalog.get plus metastore.New's guards that duplicated rocksdb.New's. The KV test suite moved to catalog/kv_test.go. Net −510 lines.
2. fullhistory: simplify LastCommittedLedger to the design signature LastCommittedLedger(cat) #835 landed. refineWithHotDB uses cat.Logger(), the logger parameter dropped, and lastCommittedLedger(cat) is the design signature. The deriveCompleteThrough/deriveLastCommitted shim aliases collapsed into direct calls — the nil-logger seam fullhistory: simplify LastCommittedLedger to the design signature LastCommittedLedger(cat) #835 worried about was already gone (those tests run against real hot DBs).

One test seam changed shape: backfill's skip-when-coverage-frozen test used to seed raw "pruning" chunk keys on the bare store; with no bare store to reach, it now runs the real SweepChunkArtifacts — exactly the finalized-window scenario it was simulating.

Agreed on stopping there — the only #841 piece taken here is the error-prefix bullet you flagged inline; the ~300-line dead-API deletions stay a separate PR. PR description updated (Closes #824 / Closes #835).