Merge feature/full-history into main - #999
Draft
karthikiyer56 wants to merge 110 commits into
Draft
karthikiyer56 wants to merge 110 commits into
karthikiyer56 wants to merge 110 commits into
Conversation
* Checking in full-history design docs into stellar-rpc for review * Improve architecture overview formatting for readability Split the single crowded system diagram into 4 separate component diagrams, convert horizontal flowcharts to vertical, and use bullet lists for descriptions. * Add glossary, cadence reference, and primer content to design docs Help newcomers orient themselves before diving into the detailed design docs by adding foundational reference content: a "Why This Design?" Q&A table, a glossary of 19 domain-specific terms with cross-reference links, a cadence reference with backfill/streaming timing tables, a data hierarchy mental model diagram, a plain-English pipeline summary, and three cross-reference boxes linking related docs. * Add space efficiency ratios, RecSplit sharding trade-off, and pre-created archives open questions Fill gaps identified by comparing v1 and v2 design docs: - 12-metrics-and-sizing.md: Add "Space Efficiency" subsection with RocksDB → immutable compression ratios (RecSplit ~4.5 bytes/entry vs 36 bytes/entry = ~90% reduction for txhash store) - 14-open-questions.md: Add OQ-5 (RecSplit single index vs 16 shards, ~7h vs ~45min build times, potential pivot) and OQ-6 (pre-created archives as alternative backfill, additive meta store changes only) - 01-architecture-overview.md: Add RecSplit sharding callout in Immutable Stores section and new "Future Design Considerations" section referencing both open questions * Update design docs reflect the parallelized 4-phase RecSplit pipeline and per-CF done flag bookkeeping changes: - 01: Updated meta store diagram, RecSplit crash recovery, mermaid - 02: Separated backfill (all-or-nothing) vs streaming (per-CF) constraints - 05: Rewrote crash recovery for 4-phase flow with flag clear+re-set - 07: Updated glossary, invariants, scenarios B3/B4/B7, resume table - 11: Added range boundary math and transition details - 12: Added transaction density by range section - FAQ: Updated raw file deletion answer - README: Updated cf:XX:done flag definition for both modes * Add query performance metrics doc and fix latency numbers New doc 15-query-performance.md with PoC-measured latencies: - Store lookups are all sub-millisecond (RecSplit ~100μs, RocksDB ~400-700μs) - XDR decode dominates at ~12-13ms per ledger (75% of total) - getTransactionByHash: ~17-18ms end-to-end - getLedgerBySequence: ~15ms end-to-end Updated 08-query-routing.md: replaced incorrect 1-15ms estimates with actual measured sub-millisecond store lookups and ~15-18ms end-to-end. Updated 12-metrics-and-sizing.md: corrected RecSplit lookup claim. * Add query performance summary to architecture overview Brief performance paragraph in the Query Layer section with key numbers (sub-ms lookups, ~15-18ms end-to-end) and links to docs 15 and 08. Added doc 15 to recommended reading order. * Reorganize reading order into core/workflows/query/advanced sections Group meta store design, crash recovery, boundary math, and metrics under "Advanced (reference)" to signal they're deep dives, not required for initial understanding. * Clarify advanced reading section is not required for initial understanding * Reorganize reading order: cover all 15 docs, group by purpose Split into 5 groups: core, ingestion workflows, query path, crash recovery/internals, and sizing/operations/open items. Added missing docs 13, 14, 15. Intro sentence clarifies first 3 groups are the main narrative, rest is reference. * Update README reading order diagram to cover all 15 docs Reorganize mermaid flowchart into 5 subgroups matching the arch overview: core, ingestion workflows, query path, crash recovery/internals, and sizing/operations/open items. All docs now represented. * Remove duplicate reading order from arch overview, point to README Single source of truth for reading order is now README.md only. * Add per-group descriptions to README reading order Each of the 5 groups now has a paragraph explaining what the docs cover and how they relate. Crash recovery/internals explicitly noted as not required for initial understanding. * Add backfill run metrics doc for full 60M-ledger production run Doc 16 captures the complete metrics from the first production backfill: 9.14B transactions across 60M ledgers in 7h 4m. Includes per-range timing breakdowns, RecSplit 4-phase analysis, transaction density evolution, latency percentiles, memory profile, and pipeline concurrency timeline. Added to README index and reading order. * Improve backfill metrics doc: remove ASCII chart, add density insights Remove the poor ASCII bar chart. Enhance the tx density table with range-over-range growth rates and share-of-total percentages. Add analysis: 93.9% of tx in last 30M ledgers, inflection at Range 1→2, plateau across Ranges 3-5. Add RecSplit index efficiency table showing consistent 5.2 bytes/key and build rates. Expand key takeaways. * Fix RecSplit index-to-raw ratio: 15%, not 1% * Improve backfill metrics doc clarity and accuracy - Remove ASCII bar chart (poor representation of tx growth) - Tone down speculative conclusions to factual observations - Redo pipeline concurrency timeline as per-range horizontal bars - Clarify latency percentile sample sets: 6,000 chunk-level samples for LFS/TxHash/fsync, 60M ledger-level samples for BSB GetLedger - Add explanatory context above Per-Range, RecSplit phase, RecSplit efficiency, Disk Usage, Memory, and Latency tables - Explain what Ingestion/RecSplit/Total columns measure in per-range table and how they relate to wall clock via concurrency timeline * Add Total Keys column to RecSplit phase table, fix per-CF build rate The phase timing table now leads with Total Keys and Keys/CF so Count (a phase name) isn't confused with a key count. The efficiency table now shows per-CF build rate (Keys/CF / Build wall time) instead of the misleading Total Keys / Build wall time — each of the 16 builder goroutines processes only 1/16 of the keys. * Remove bytes/key commentary from RecSplit efficiency section * Fix memory profile section to match actual log data Heap alloc during ingestion was 25-45 GB, not 1-7 GB. The 1-7 GB range only appeared during solo RecSplit builds. Added Go Heap Sys column to the table. Removed interpretive claims, now just states the numbers as reported by the 1-minute progress ticker. * Reframe backfill workflow as DAG of idempotent tasks Merge 03-backfill-workflow.md and 05-backfill-transition-workflow.md into a single doc framed around three task types (process_chunk, build_txhash_index, cleanup_txhash) with explicit dependencies. Simplify 07-crash-recovery.md from ~1600 lines to ~340 by replacing enumerated backfill crash scenarios with three invariants (key implies durable file, tasks are idempotent, startup rebuilds full task graph). Fix all cross-references across 11 docs: update anchors to match new headings with cadence suffixes, replace "chunk sub-workflow" terminology with DAG task names. * docs: sync design docs with full review rework - range→index terminology sweep across all docs - merge 04-streaming-workflow + 06-streaming-transition into 04-streaming-and-transition - update meta store key schema, config params, metric names - add getStatus API spec for backfill mode - FAQ updates, crash recovery alignment, language tightening passes * fix: quote all Mermaid diamond nodes to prevent parse errors Unquoted diamond nodes with special characters (colons, nested braces, multiline text) cause Mermaid parse failures in GoLand and other renderers. * fix: purge stale orchestrator/parallel-workers language from all design docs Replace all references to "2 parallel workers", "20 BSB instances per worker", "index orchestrator", and "num_bsb_instances_per_index" with the current flat worker pool + DAG scheduler model across all 10 affected design docs. * docs: eliminate instance/orchestrator language from all design docs Replace "BSB instance" with "process_chunk task" throughout. Rewrite Execution Model in 03-backfill-workflow.md: DAG scheduler, tasks as black boxes, task-level vs internal concurrency (process_chunk is single-threaded; build_txhash_index spawns 100+ goroutines internally). Expand build_txhash_index pseudocode to show 4-phase RecSplit pipeline. * docs: ship consolidated backfill doc, remove standalone docs for separate streaming PR Consolidate meta store keys, directory structure, configuration, crash recovery, and getStatus API into a single self-contained 03-backfill-workflow.md. Delete 13 standalone docs — their backfill content is now in 03, streaming content will return in a future streaming PR. Trim README to backfill scope. * docs: backfill doc review pass — fix contradictions, expand jargon, add flow diagrams * docs: simplify to README + backfill doc only, remove architecture overview * docs: fix README — list primary use cases instead of specific API endpoints * docs: backfill doc v3 — events integration, pack format, 3-pass review fixes * docs: add process_chunk pseudocode showing all 3 outputs + atomic flag write * docs: reorder sections — directory structure and config before internals, move crash invariants to crash recovery section * docs: clean up overview, remove captive core from backfill, fix grouping note * docs: minor wording fixes in config section * docs: fix max_scope_depth description — nesting depth, not magic numbers * docs: move geometry above config * docs: use underscore separators for large ledger numbers * docs: rewrite Tasks and Dependencies intro * docs: convert prose blocks to bullets throughout for readability * docs: rewrite parallelism flow — plain English + Gantt diagram * docs: replace misleading Gantt with worker slot diagram showing interleaved execution * docs: simplify chunk scan explanation * docs: explain orphan detection and why chunks_per_txhash_index changes are disallowed * docs: add orphan detection example * fix: abort on any orphan index, not just multiple * docs: clarify getStatus active array description * docs: add example to getStatus active array explanation * docs: clarify directory numbering — storage group vs indexID vs chunkID * docs: index-first directory layout, uniform %08d everywhere, collapse to immutable_base * cleanup: remove design-docs-temp from design branch — only belongs on code branch * docs: flatten events files — no per-chunk subdirectories * cleanup: remove design-docs-temp from design branch design-docs-temp only belongs on the code branch. Previous removal was accidentally reverted. * docs: apply PR #617 review decisions — type-separated layout, independent flags, cleanup_txhash 19 changes from finalized review threads (DS-1..4, CFG-1..4, PC-1..3, TD-1, SR-1, EM-1..3, API-1, OV-1, EH-1): - Replace index-first directory layout with type-separated top-level dirs (ledgers/, events/, txhash/) bucketed by 1000 chunks - Per-type storage paths in TOML, per-run params moved to CLI flags - Relaxed end_ledger validation (expand to chunk boundary, not index) - Independent flag writes in process_chunk — only missing outputs produced, local NVMe read when LFS already present - cleanup_txhash modeled as explicit DAG task (not inline in build) - Reconciliation simplified — all recovery via DAG dependency resolution - DAG scheduler pseudocode replaces Mermaid, includes retry loop - Default workers changed to GOMAXPROCS - getStatus simplified to task-type summaries - Error handling documents --max-retries behavior * docs: restructure backfill workflow — Geometry→Config→DirStructure, bulletize, fix forward refs Major restructure of 03-backfill-workflow.md: - Reorder sections: Geometry → Configuration → Directory Structure (define concepts before showing filesystem layout) - Collapse 3 concrete examples to 1 (bucket structure is identical regardless of chunks_per_txhash_index) - Rename storage.* → immutable_storage.* (disambiguate from active_storage in streaming) - Remove all forward references to task names before Tasks section - Inline startup triage into DAG setup pseudocode (no separate phase) - Break out Main Flow → Validation → DAG Setup as separate pseudocode - Convert build_txhash_index and cleanup_txhash to pseudocode - Rewrite scheduler pseudocode in Python style (was Go-like) - Expand validation rules: chunk boundary expansion, BSB validation, partial index ranges, streaming workflow implications - Fix max-retries: task-level retry, GCS retries handled by BSB - Normalize all large numbers to underscore notation - Bulletize prose paragraphs throughout * docs: apply PR #617 round 2 review — rename Index→Txhash Index, demote Bucket, simplify DAG, cloud-agnostic language 10 changes from Tamir and Urvi's round 2 review: - GEO-1: "two levels" → "two concepts" - GEO-2: rename bare "Index" → "Txhash Index" throughout - DS-1: demote Bucket from Geometry to Directory Structure - OV-1: overview table uses "Txhash index files" not algorithm details - CFG-1: data_dir → default_data_dir - CFG-2: [backfill.bsb] moved from optional to required config - CFG-3: remove error_file from logging config - CFG-4: GCS-specific prose → "cloud storage" / "object store" - DAG-1: build_dag() only wires edges; triage moved into each task's execute() - PC-1: remove txhash_writer.sort() from process_chunk * docs: use BSB consistently instead of verbose "cloud storage" throughout * docs: rename backfill-workflow → stellar-rpc --mode=full-history-backfill * docs: remove [logging] from config — logging is an implementation detail * docs: expand bucket_id explanation in Directory Structure — formula, scope, hardcoded * docs: tighten Directory Structure — remove redundancy, add chunks_per_txhash_index tradeoff table * docs: remove abort-on-narrow validation — backfill never prunes, streaming catches gaps
This PR adds a thin CGo wrapper around system libzstd (>= 1.5.7) for compression and decompression. - Compressor: reusable context with configurable content checksums - Decompressor: reusable context with buffer reuse - Convenience Encode/Decode functions for one-off use Also updates build infrastructure to provide libzstd >= 1.5.7: - Dockerfile: switch build stage from bookworm to trixie (Debian 13) which ships libzstd 1.5.7 in its default repos - CI: build libzstd 1.5.7 from source in the setup-go action (Linux), or install via brew (macOS) Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* ci: add RocksDB CI support + sample meta store test - scripts/install-rocksdb.sh: self-contained install script matching install-zstd.sh pattern. Builds static lib (.a) instead of shared (.so) to avoid glibc version mismatch between ubuntu-22.04 and ubuntu-24.04 runners — a .so built on 24.04 fails on 22.04 with "undefined reference to GLIBC_2.38". - setup-go/action.yml: add RocksDB cache + build steps, install link deps (snappy, lz4, zstd, zlib, bz2) on every Linux job. Cache key includes ImageOS (ubuntu22/ubuntu24) because different compiler versions may produce different object code. CGO_LDFLAGS only has -L paths, not -l flags — library link flags belong in per-package #cgo directives to avoid leaking into unrelated builds (e.g. ARM64 cross-compile). - cgo_linux.go: per-package #cgo directive adding -lbz2 -ldl for static RocksDB linking. Only activates when the backfill package is imported — does not affect other packages or cross-compilation builds. - cmd/stellar-rpc/internal/backfill/: minimal meta store wrapping grocksdb to validate RocksDB compiles and links correctly in CI. * address PR feedback: remove libzstd-dev apt dep, use Tamir's zstd install - Remove libzstd-dev from apt deps (action.yml + install-rocksdb.sh). RocksDB cmake now uses zstd from ~/.zstd/ (installed by install-zstd.sh) via ZSTD_HOME env var. One zstd installation used everywhere. - install-zstd.sh: install-static instead of install, per Tamir's suggestion. No .so needed since nothing uses shared zstd linking. - meta_store_test.go: switch to stretchr/testify/require for consistency with rest of codebase. * keep libzstd-dev from apt for rocksdb cmake build Revert ZSTD_HOME cmake approach — find_package(zstd REQUIRED) in RocksDB's CMakeLists.txt may not work with custom cmake flags. Keep libzstd-dev from apt for the rocksdb build. The install-zstd.sh static-only change and testify/require changes are retained. * remove libzstd-dev from apt deps RocksDB cmake has WITH_ZSTD=OFF by default and we never enable it. RocksDB builds without zstd compression support. At Go link time, grocksdb's -lzstd resolves from ~/.zstd/lib/ (Tamir's install) via CGO_LDFLAGS=-L~/.zstd/lib. No system libzstd-dev needed. * enable zstd compression in RocksDB, use Tamir's zstd install WITHOUT_ZSTD=ON was default OFF — RocksDB was building without zstd compression support. Calling SetCompression(zstd) via grocksdb would fail at runtime. Now enabled via -DWITH_ZSTD=ON with CMAKE_PREFIX_PATH pointing at ~/.zstd (Tamir's install-zstd.sh output). No apt libzstd-dev needed. * fix testifylint: use require.Empty for empty string checks
* Dockerfile: all stages ubuntu:24.04, zstd/rocksdb from source - All stages ubuntu:24.04 — same glibc everywhere, no mismatch - zstd 1.5.7 built from install-zstd.sh (shared only, no .a) - RocksDB shared lib + tools via cmake - Go/Rust from upstream tarballs (no golang:* image) - stellar-core from apt.stellar.org noble packages - zstd.go: remove compile-time #if check, -lzstd instead of -l:libzstd.a - install-zstd.sh: install-shared only (no .a) - action.yml: add LD_LIBRARY_PATH for shared zstd + rocksdb * fix ARM64 cross-compile: static zstd in CI, shared in Docker install-zstd.sh: SHARED_ONLY=1 installs .so only (Docker). Default installs .a only (CI) — ARM64 cross-compile can't link x86 .so, so static avoids the "file in wrong format" error. Dockerfile: passes SHARED_ONLY=1 when calling install-zstd.sh. action.yml: remove LD_LIBRARY_PATH — both zstd and rocksdb are static (.a) in CI, no runtime shared lib dependency. * unify shared linking: cmake+ninja for zstd, shared .so everywhere Switch from the static/shared split (CI static, Docker shared) to shared linking everywhere. The root cause of the split was two bugs: 1. install-zstd.sh used make, which doesn't propagate CC to the shared library link step — producing x86 .so even with an ARM64 cross-compiler. Fixed by switching to cmake+ninja, which passes -DCMAKE_C_COMPILER explicitly to all compile and link steps. 2. CI runners were mixed ubuntu-22.04 (tests) and ubuntu-24.04 (builds). A .so built on 24.04 fails on 22.04 (GLIBC_2.38 mismatch). Fixed by moving unit tests to ubuntu-24.04. Changes: - install-zstd.sh: make → cmake+ninja, always shared, CC/CXX passthrough with CMAKE_SYSTEM_NAME for ARM64 cross-compilation - install-rocksdb.sh: ROCKSDB_BUILD_SHARED=OFF → ON, CC/CXX passthrough with CMAKE_FIND_ROOT_PATH for ARM64 multiarch library discovery - action.yml: add LD_LIBRARY_PATH for shared .so runtime lookup - stellar-rpc.yml: unit tests ubuntu-22.04 → 24.04, ARM64 job installs g++ cross-compiler + arm64 dev packages via dpkg multiarch - Dockerfile: remove SHARED_ONLY=1 (script now always builds shared), add cmake+ninja to zstd-build stage * fix ARM64 apt: pin amd64 sources, add ports.ubuntu.com for arm64 The default Ubuntu repos (archive.ubuntu.com, security.ubuntu.com) only serve amd64 package indices. dpkg --add-architecture arm64 causes apt-get update to request arm64 indices from these repos, which 404. ARM64 packages are served from ports.ubuntu.com. Fix: restrict existing apt sources to [arch=amd64], then add a separate source file pointing to ports.ubuntu.com for [arch=arm64]. Handles both deb822 (.sources) and traditional (.list) formats. * fix ARM64 cross-compile: use GCC 13+ (C++20 support for RocksDB) RocksDB 10.9.1 uses 'using enum' (C++20, P1099R5) which requires GCC 11+. The GCC 10 cross-compiler doesn't support it, failing with: error: expected nested-name-specifier before 'enum' Switch from gcc-10-aarch64-linux-gnu to gcc-aarch64-linux-gnu (unversioned), which gives GCC 13 on ubuntu-24.04 — full C++20. * rocksdb: add ROCKSDB_BUILD_STATIC=OFF to avoid building both With ROCKSDB_BUILD_SHARED=ON but STATIC not explicitly OFF, cmake builds both .a and .so (709 compile units × 2). Setting STATIC=OFF halves the build time. * rocksdb: build only shared target, skip static (halves build time) RocksDB's cmake has no option to disable the static library target — ROCKSDB_BUILD_STATIC doesn't exist (cmake warned "not used"). The static target is always defined and 'ninja' builds it by default. Fix: target 'rocksdb-shared' explicitly in the ninja command instead of building all targets. This compiles ~355 source files once instead of ~710 (355 static + 355 shared). Manual install since 'ninja install' requires the static lib we didn't build. * use native arm64 runner instead of cross-compilation Replace the x86→arm64 cross-compilation setup with a native ubuntu-24.04-arm runner. Deletes 46 lines of workarounds: - dpkg multiarch + ports.ubuntu.com apt pinning - gcc/g++ cross-compiler installation - arm64 dev packages (libsnappy-dev:arm64, etc.) - CC/CXX environment variable setup Native arm64 runner means the install scripts run natively — cmake finds arm64 libs without CMAKE_SYSTEM_NAME, no CMAKE_FIND_ROOT_PATH needed, system GCC 13 used directly. * pin linux amd64 build to ubuntu-24.04 (not ubuntu-latest) * remove cross-compilation logic from install scripts With native arm64 runners, CC is never set to a cross-compiler. The cross-compilation blocks (CMAKE_SYSTEM_NAME, CMAKE_FIND_ROOT_PATH, CXX derivation from CC, aarch64 detection) were dead code. Removed 77 lines of comments and logic that no longer apply. * trim comment clutter in action.yml and Dockerfile
* Bump soroban-env to next protocol version (#595) * Bump soroban-env to next protocol version * Add required config key to mocked ledger entries * Integrate unstable-next-api into curr (dropped arg) * Bump soroban-env SHA to latest (#611) * Update XDR and Core runs to test P26 integration (#629) * Split the single core_version input into core_deb_version and core_docker_img to accommodate differences. * Bump Core versions for P25 and add P26 run * Bump Golang to 1.25 and linter to be compatible * Bumped soroban-env-host curr to latest commit SHA in repo * Bumped Go SDK to latest @protocol-next * Added P26 limits from quickstart and removed P24 limits * Bump crate dependencies to latest Protocol 26 versions (#640) * Remove unimplemented GitHub Action (#643) Snuck in with a `git add -A` on accident * Bump Core to Protocol 26 release candidate (#642) * Bump Core to p26 release candidate * Bump Go SDK version to latest * Bump to official patched env * Ensure we strip NUL bytes before `CString::new` (#632) * Fix deluge of linter complaints on branch merge (#646) * Release v26.0.0 (#647) * Tag off changelog for major release * Bump Go SDK to official P26 version * Bump CI to latest stable core for a final run * Add GHA to automatically update soroban-env on protocol-next (#645) * Preserve version string in Cargo.toml on automatic dependency bump (#672) * Preserve version= in Cargo.toml on bump * Simplify code: use a function, better names * Bump to soroban-env to latest version (#673) --------- Co-authored-by: George <Shaptic@users.noreply.github.com> Co-authored-by: Siddharth Suresh <siddharth@stellar.org> Co-authored-by: George <george@stellar.org>
…ocation (#700) - Migrate all TOML sections + keys to UPPER_SNAKE_CASE; prose references, placeholder forms, directory-tree annotations, example config block all updated. CLI flags stay kebab-case; pseudocode and filesystem dir names stay lowercase. - Add sectioned [LOGGING] with LEVEL / FORMAT keys; CLI --log-level / --log-format override TOML (specifying both is not an error). Deliberate divergence from legacy flat LOG_LEVEL / LOG_FORMAT layout. - Fix invocation examples to cobra subcommand form (stellar-rpc full-history-backfill, not --mode=...). - README scope blurb: drop (this PR) / (future PR) / "separate PR" PM framing; keep specific PR-number cross-references. Addresses #683. PRD: #678.
* Add event index package for full-history getEvents Implements the events package with bitmap-based term indexing for the hot chunk in the full-history getEvents feature. Core types: - EventIndex: public interface mapping (value, field) pairs to roaring bitmaps of event IDs. Supports variadic Add and Go 1.23+ iterators. - BitmapStore: pluggable storage interface for term bitmaps. Thread-safe implementations manage their own concurrency. - memBitmaps: in-memory BitmapStore using list-to-bitmap promotion (threshold=64) for memory-efficient sparse term storage. Returns clones from Get for safe concurrent read/write access. - TermKey: 16-byte xxh3_128 hash of (field || value) pairs. - LedgerOffsets: maps absolute ledger sequences to half-open event ID ranges [start, end) with sequence validation. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Add immutability lifecycle to memBitmaps and clarify BitmapStore.Close Close() now flips an atomic flag making the store immutable. After Close: - AddTo, Put, Delete return ErrClosed. - All() iterates without acquiring the read lock since no concurrent writes are possible. - Get() returns the live bitmap pointer instead of a clone, avoiding allocations during the freeze read path. Removed Optimize(): benchmarks showed RunOptimize gives no benefit for production event ID patterns (ascending with gaps), only for contiguous ranges which we don't have. Added tests for the closed lifecycle (rejected writes, lock-free iteration, idempotent close, returned-pointer identity). Clarified Close() doc on the BitmapStore and EventIndex interfaces. Added build_sec metric to BenchmarkEventIndex_10M for readability. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Fix golangci-lint issues in events package - errcheck: check error returns from Add/AddTo/Append in tests - gosec G115: add //nolint comments for safe int->uint32 conversions bounded by LedgersPerChunk - intrange: use Go 1.22+ range syntax - modernize bloop: b.Loop() instead of `for range b.N` - modernize fmtappendf: fmt.Appendf instead of []byte(fmt.Sprintf) - modernize waitgroup: wg.Go(...) instead of Add/defer Done - nilnil: //nolint comment on Get's intentional (nil, nil) for not-found - perfsprint: errors.New for static error strings - staticcheck SA4006: remove unused assignment - testifylint: require.ErrorIs for error assertions, assert.Len, drop EqualValues Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Address PR review on LedgerOffsets API - Append takes per-ledger eventCount instead of cumulativeCount; the cumulative is maintained internally so callers don't have to track it. - Replace EventIDRange(start, end) with EventIDs(ledger) returning the half-open [start, end) range for one ledger. Multi-ledger ranges and cross-chunk queries compose by calling EventIDs on each ledger/chunk. - Split the empty-store case from the bounds check for a clearer error message that includes the requested ledger number. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Collapse BitmapStore and EventIndex into a single interface Drop the BitmapStore interface and the eventIndex wrapper struct. Fold the (value, field) methods directly onto memBitmaps and expose EventIndex as the only public concept. With a single in-memory backend today, the two-interface split was premature; reintroduce a storage abstraction when a second backend (disk/cold-tier) actually appears. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* Add Layer-1 pkg/rocksdb wrapper + pkg/format helpers (#685) Establishes the shared Layer-1 RocksDB foundation that every Layer-2 facade (meta store #689, hot ledger store #584, hot txhash store #729, hot events store under #665) builds on. - New cmd/stellar-rpc/internal/fullhistory/{backfill,streaming,pkg}/ tree; meta_store.go relocated out of cmd/stellar-rpc/internal/backfill/ (whose CI-only seed is replaced by the real wrapper). - pkg/rocksdb: two-phase Store lifecycle. New(cfg) (*Store, error) validates Config; (*Store).Open() does the actual grocksdb open and is idempotent via sync.Once. Close is idempotent via atomic.Bool. Auto-mkdir on Open (mode 0700). On-open log line includes elapsed open time, WAL size, L0 file count, total data size, active memtable size — surfaces slow restarts to the operator. - CF-aware: default-only (meta store + hot ledger store), 16 CFs nibble- routed (hot txhash store), arbitrary multi-named (events store). Same Put/Get/Delete/Iterate/Batch path internally for every shape. - Batch: intra-store atomic via grocksdb.WriteBatch. Mid-callback error rolls back zero-visibility; empty batch is a no-op; BatchWriter goes inert after the callback returns; cross-batch ordering is RocksDB's serialization. Same-key Put + Delete in one batch: deletion wins. - Cross-process flock via grocksdb's native LOCK file. Two same-process Stores against the same path collide on it (by design — sharing a directory means sharing one Store, not creating two). - Logger plumbing: components take *supportlog.Entry directly via Config (matches existing daemon.go / event_test.go pattern). Tests use supportlog.New() + SetOutput(&bytes.Buffer{}) — no Logger interface, no NopLogger, no captureLogger fixture. - pkg/format trimmed to two functions actually consumed by this slice: Bytes(int64) and Number(int64), plus the project-wide ByteOrder reference. Demand-driven; future slices add more when they need them. - 22 tests, all real-RocksDB via t.TempDir(). Static guardrail asserts no production code path calls db.Flush() or db.SyncWAL() (per ADR-0002 — durability is the WAL's job; manual flush masks compaction problems). Closes #685 * Polish: idempotent Open via sync.Once, Flush, explicit WAL on, more tests - (*Store).Open uses sync.Once for instance-level idempotency; the global path-keyed registry is gone. - New Flush() method for callers wanting a clean shutdown (no WAL replay needed on next Open). Close still doesn't auto-Flush. - WAL pinned on explicitly via WriteOptions.DisableWAL(false) in doOpen so the always-on intent is visible at the call site. - Drop placeholder doc.go files in backfill/ and streaming/ — preemptive directory staking out; future slices will create the dirs when they have real code. pkg/rocksdb's package doc inlines into rocksdb.go. - Trim pkg/format to consumed helpers (Bytes, Number, Duration, plus ByteOrder); pkg/memory deleted entirely. Duration uses whole "Xm Ys" form for >= 1 minute. - More test coverage: TestStore_OpsAfterCloseFailWithErrStoreClosed (5 ops), TestStore_CloseLifecycle (double-close + close-on-never-opened), TestStore_IterateCorners (empty prefix / empty CF / unknown CF), TestBatch_ErrorPaths (never-opened / unknown CF in callback / delete-only callback), TestStore_FlushSucceedsOnOpenStore. Drop TestOpen_LogsStateAtOpen (log wording isn't a behavioral contract) and the no-manual-Flush/SyncWAL static guardrail (Flush is now legitimate). - Reformat all multi-sentence doc comments so each new sentence starts on a new line. - Document on *Store why it's a concrete struct, not an interface, and what would justify extracting one later. * format.Bytes: switch to decimal (1000-based) disk-space units KB / MB / GB now mean what disk vendors and `df` mean by them. A WAL size logged by the wrapper can be compared directly against `df -h` output, which is the only reason an operator reads it. * Address PR #733 review: race fix, zero-copy iter, test comment Three issues raised on the PR: - Close-Open race could leak the underlying DB. If Close's CompareAndSwap-on-closed won while doOpen was still constructing the grocksdb instance, Close returned early on \`s.db == nil\` but doOpen later succeeded and set s.db with no one left to free it. Fix: Close calls openOnce.Do(noop) after the CAS to wait for any in-flight Open to settle. If there is no in-flight Open, the noop Do "claims" the once so a later Open returns ErrStoreClosed without touching disk. Open also checks s.closed.Load() after openOnce.Do and returns ErrStoreClosed if Close raced ahead. Real openErr takes precedence so disk-side failures still surface. - Iter.Key() / Value() doc said zero-copy ("valid only between Next-returning-true and the next Next call") but the impl was copying. Switched the impl to zero-copy — return the iterator's internal Slice.Data() directly so the contract matches and Layer-2 facades scanning many keys don't pay a per-call allocation. - Test comment on TestStore_Iterate_SortedPrefixScan said \`chunk:00000005:txhash\` was "not under our scan prefix", but it matches \`chunk:0000000\` (the test's expected output already includes it). Comment fixed; only \`index:00000000:txhash\` is the excluded case. Adds TestStore_ConcurrentOpenAndClose: 20 iterations of Open and Close in parallel goroutines, exercised under \`go test -race\`. * Rewrite comments for clarity (PR review follow-up) Sweep through pkg/rocksdb and pkg/format docstrings, replacing internal-jargon shortcuts with plain-English explanations: - BatchWriter doc gets a worked example showing the typical Store.Batch usage shape, and the lifecycle warning is rewritten in full sentences instead of "lifetime is scoped to the callback" / "captured BatchWriter" jargon. - Put / Delete / Batch method docs explain the queue-then-commit shape explicitly so a reader doesn't need to already know how the callback pattern works. - batchWriter struct + invalidate explain WHY the design works (catch the typo at the line that made it; nil the WriteBatch so stale handles can't corrupt the next batch) instead of name- dropping concepts like "stack-trace-less commit failure". - Open / Close / doOpen / DisableWAL comments rewritten to lead with the user-visible behavior and only mention internals when needed for context. - Package doc dropped the dense comma-list ("CF-aware ..., WAL-on default, flock-protected, auto-mkdir") in favor of a bulleted "what this wrapper handles for every facade" list. - hasPrefix and prefixIter comments cleaned up. No behavior change; all tests pass under -race; lint clean. * Move byte order to pkg/rocksdb; add Encode/Decode helpers - pkg/rocksdb: new encoding.go with unexported byteOrder (single source of truth, BigEndian) plus EncodeUint32 / DecodeUint32 / EncodeUint64 / DecodeUint64. Layer-2 facades go through these helpers and never pick an endianness. - pkg/format: drop the public ByteOrder var (and its encoding/binary import) — endianness for RocksDB-stored integers is no longer a concern of this package. - pkg/rocksdb: add experiment_endianness_test.go (build-tagged experiment, not in the regular suite) demonstrating that swapping BE for LE on uint32 ledger-seq keys turns GetLedgerRange(100, 200) from "5 correct values" into "8 values with 3 polluters, no error flagged" — the single- pass iterator design requires on-disk byte order to match numeric order, which is what BE gives us. * Document BE choice + flag the experimental test as deletable - encoding.go: explain why RocksDB picks big-endian even though packfile (pkg/lfs) picks little-endian — packfile reads are positional so endianness is a free parameter there, while RocksDB iterates byte-lex and so the encoding must match the numeric order we want to scan in. Point to experiment_endianness_test.go for the empirical demonstration. - experiment_endianness_test.go: top-of-file note that this is a build-tagged experiment, excluded from `go test ./...`, kept as the empirical record behind the BE choice, and safe to delete in due course once the rationale is settled. * Flatten BatchWriter from interface to struct The BatchWriter interface had a single implementation (batchWriter) and the wrapper boundary is not a place this codebase mocks, so the interface was paying for nothing concrete. Collapse it. - batch.go: drop the BatchWriter interface; rename the struct batchWriter -> BatchWriter (exported) with unexported fields. Migrate the interface's doc onto the struct; example block now uses func(b *rocksdb.BatchWriter) error. - Store.Batch callback signature: func(BatchWriter) error becomes func(*BatchWriter) error. - Tests (batch_test.go, rocksdb_test.go): update 16 callback signatures and one var declaration to *BatchWriter. Prose references in comments untouched. Iterator (Iter interface + errIter sentinel + prefixIter) left as is. * Adopt go-humanize for byte/number formatting; fix Duration MinInt64 PR review flagged that format.Number and format.Duration stack-overflow when called with math.MinInt64 — `-n` overflows back to MinInt64 (two's complement asymmetry) and the negate-and-recurse branch never terminates. - pkg/format: delete Bytes and Number. Callers now use humanize.Bytes / humanize.Comma directly; both handle MinInt64 correctly. Duration stays — go-humanize has no equivalent compact duration formatter (humanize.Time / RelTime are relative-to-now strings, not "5d 12h 30m"). - pkg/format.Duration: explicit MinInt64 guard before the negate-and-recurse branch. Falls through to MaxInt64; the one-nanosecond asymmetry is invisible at the ~292-year scale. - pkg/format/format_test.go (new): regression test for the bug fix plus a normal-negative formatting check. - pkg/rocksdb/rocksdb.go: import humanize; the open-log line uses humanize.Bytes / humanize.Comma. To keep gosec quiet without //nolint, walDirSize now returns uint64 and a new readUintProperty parses uint64 RocksDB properties — byte sizes flow through as uint64 end-to-end. - go.mod: go-humanize promoted from indirect to direct dependency. * Address PR review: iter.Seq2, lifecycle RWMutex, drop pkg/format PR #733 review (Tamir): - T1 — Drop pkg/format. pkg/format.Duration had a math.MinInt64 stack-overflow that we patched, but its only consumer was the open-log line and stdlib serves that line directly. The open log now uses elapsed.Round(time.Microsecond).String() — preserves the µs precision a fast open needs (Round(ms) would render a 350µs open as "0s") and reads as the standard Go duration format every contributor recognizes. - T4 + T7 — Replace the Iter interface + errIter sentinel + prefixIter struct with a Go 1.23+ range-over-func iterator: Store.Iterate returns iter.Seq2[Entry, error]. The producer closure owns defer it.Close() and yields per-iteration errors, eliminating both the forget-Close footgun and the separate Err() check. Inside the producer, KeySlice() / ValueSlice() return OptimizedSlice (value type) instead of *Slice (heap-allocated) — ~30k transient allocations saved per 10k-key scan. ADR-0008 (added separately in project-data dir) records iter.Seq2 as the iteration shape for everywhere in fullhistory. - T5 — Lifecycle RWMutex on Store, with a long design-decision comment that draws an explicit line between (a) lifecycle / C memory-safety, what this lock IS for — preventing Close from tearing down the C++ DB while a goroutine is mid C call against it — and (b) data consistency, what this lock is NOT for — RocksDB is thread-safe internally and idempotent application data plus the atomic Batch primitive cover ordering at the call site. Every op takes RLock for the duration of its C work; Close takes the exclusive Lock after flipping the closed flag, waiting for in- flight RLock holders to drain. Reads don't block writes; writes don't block reads; only Close serializes. Cost is one atomic CAS per op (under 0.1% CPU overhead at 100k puts/sec), and a single CAS spans an entire Batch. Tests for the new lock: - TestStore_ConcurrentOpsAndCloseRaceFree — 16 worker goroutines (4 each of Put / Get / Iterate / Batch) hammer the store while Close races; -race validates no data race on s.db. - TestStore_CloseWaitsForInflightIterate — deterministic: parks an Iterate goroutine inside its loop body so the RLock stays held, verifies Close blocks until the iteration releases. - T2 — Delete experiment_endianness_test.go. The test was build- tagged out of CI and had no assertions; it only documented why BE is required for numeric RocksDB keys. The same demonstration now lives inline in the byteOrder doc comment in encoding.go — concrete example showing that an LE-encoded ledger-range scan would silently return ledgers 356 / 612 / 65637 alongside the asked-for 100..200 because their LE encodings sort between LE(100) and LE(200). * Store.Get: use GetPinnedCFV2 (zero-copy from block cache) PR #733 review (Tamir, T3). The previous GetCF path allocates a separate C buffer and memcpys the value from the block cache into it; we then make+copy that into a Go-owned slice. Two copies on the read path. GetPinnedCFV2 returns a handle that points directly into the pinned cache page — no separate C buffer. One copy on the read path (cache page -> Go-owned slice). grocksdb's own source comment flags the *V2 variants as the recommended migration target for performance. Semantics unchanged; existing Get tests cover the new code path under -race. * Drop unused context.Context from Store.Batch signature PR #733 review (Tamir, T6). ctx was accepted as `_ context.Context` and never used. grocksdb's db.Write is a blocking CGO call with no cancellation hook, so we couldn't usefully thread it through anyway. Demand-driven port discipline says: remove now, add back when a real consumer needs cancellable batching. - batch.go: Batch signature becomes Batch(fn) instead of Batch(ctx, fn). context import dropped (only use was the ignored parameter). - batch_test.go: 14 call sites lose `context.Background(),`. context import dropped (was only used for those calls). - rocksdb_test.go: two Batch call sites updated. The flock test's exec.CommandContext switches from context.Background() to t.Context() — the spawned child process is now automatically killed if the test panics or times out. context import dropped from the test file entirely.
The LedgerBackend interface in go-stellar-sdk (pulled in by the previous commit) gained a GetLedgerRaw method. The mockLedgerBackend in ledger_reader_test.go no longer satisfies the interface and the compile-time assertion fails, which is what golangci-lint surfaced as a typecheck error on the PR. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…full-history-2026-05-12 Karthik/merge main into feature full history 2026 05 12
* Add Layer-2 storage facades: MetaStore, TxHashStore, LedgerStore pkg/stores: typed interface contracts for the three Layer-2 storage facades. Sealed marker interfaces (MetaStoreEntry / MetaStoreKey) plus concrete ChunkEntry / IndexEntry / ChunkKey / IndexKey for the metastore; TxHashToLedgerSeqEntry value type for the hot txhash store; LedgerEntry for the hot ledger store. Shared ErrNotFound / ErrStoreClosed sentinels. No RocksDB dependency. pkg/rocksdb: concrete impls alongside the Layer-1 wrapper. rocksdb.MetaStore wraps the default CF, type-switches over MetaStoreEntry for AddEntries / DeleteEntries dispatch, exposes MarkTxHashIndexComplete as a typed atomic-transition using pkg/geometry to derive chunk IDs. rocksdb.TxHashStore opens 16 CFs named cf-0..cf-f, hides nibble routing entirely behind AddEntries / RemoveEntries / Get. rocksdb.LedgerStore stores opaque bytes verbatim, maintains in-memory (min, max) bounds via FirstLastKey on Open and on boundary-deletes. Every facade follows the same shape: two-phase New + Open with facade-level sync.Once, atomic.Bool Close with internal Flush, closed-fence at every method, single-vs-batch dispatch by len(slice). pkg/rocksdb: new Layer-1 primitives. IterateFrom(cf, startKey) for seek-and-walk-forward range scans over integer-keyed stores; FirstLastKey(cf) for O(1) bounds via SeekToFirst + SeekToLast. WAL on + per-write Sync on are now wrapper-pinned (DisableWAL and Sync dropped from the Tuning struct); NoCompression universal at the block level. ErrNotFound sentinel added for facade use. pkg/geometry: LedgersPerChunk constant + ChunksInTxIndex helper for the metastore's MarkTxHashIndexComplete; will grow under slice #684. pkg/testutil: ledger and transaction fixture builders ported from the reference commit; MakeRandomTransactions, MakeRandomLedgerCloseMeta, MakeRandomLedgerCloseMetaForSeq. Used by the LedgerStore's xdr round-trip test today; available to every fullhistory test package going forward. Tests: real RocksDB via t.TempDir() across every facade, single + batch + miss + idempotency, race-chaos + deterministic Close-blocks-on-RLock under -race, graceful close + reopen round-trips, xdr round-trip through the LedgerStore. Closes #689 Closes #584 Closes #729 * Inline chunk-range math in metastore; demote klauspost/compress pkg/geometry shrinks to LedgersPerChunk only. The earlier ChunksInTxIndex helper allocated a slice of chunk IDs every call to MarkTxHashIndexComplete just for the function to immediately iterate it; inlining the math at the single call site does the same work without the allocation. Future geometry helpers (ChunkID(ledgerSeq), FirstLedgerInChunk, etc.) land in slice #684 when there are real consumers. go.mod: demote klauspost/compress to // indirect via go mod tidy — nothing under cmd/stellar-rpc/internal/fullhistory imports it since the LedgerStore stopped doing in-store compression. * Consolidate L2 store lifecycle; auto-Flush in Store.Close Move Flush into Store.Close so a graceful shutdown trickles down from the facade Close to the wrapper, drains the memtable, and tears down — no separate Flush call required at any layer. Single source of truth for the closed-state signal: the wrapper's own closed flag, exposed via Store.IsClosed() for the handful of facade paths that short-circuit before reaching a wrapper op (empty-slice writes, invalid-range iteration, GetLedgerRange's in-memory read in the prior implementation). Every other facade method inherits ErrStoreClosed through the wrapper's existing checkOpen. LedgerStore loses the in-memory (minSeq, maxSeq) cache and its supporting fields and helpers (openOnce/openErr/rangeMu/widenRangeBounds /deleteTouchedBoundary/refreshRange). GetLedgerRange now goes straight to FirstLastKey — O(1) SST metadata reads, no disk. All three facades end up structurally identical: one *Store field, one-line Open/Close. Also fix leaks flagged in the architecture sweep: drop the BatchWriter mention and the on-disk encoding detail from pkg/stores doc comments; drop the compression mention from LedgerEntry; remove the ADR pointer from txHashTuning; strip the uncorroborated timing claims from Store.mu and TxHashStore.AddEntries. GetLedgerRange signature gains an error return so a closed-store call surfaces stores.ErrStoreClosed instead of silently returning stale in-memory bounds. * Rename MetaStore Get* methods; aggressive Phase-B comment prune * MetaStore method rename: GetChunkEntry -> GetChunkArtifactState, GetIndexEntry -> GetTxHashIndexState. The returned uint8 is a caller-defined state-machine value; the prior "Entry" naming didn't say what was being returned, and "index" without qualifier was opaque. * Fix wrong chunk-to-ledger example in pkg/geometry doc (the range claim was incorrect). Drop the specifics entirely. * Prune comments across pkg/rocksdb, pkg/stores, pkg/geometry, pkg/testutil: 3-line file preambles; one-line godoc per exported symbol; condensed Store.mu lock-contract doc; Close/Flush rationale lives in exactly one place (Store.Close). Preserved the heavy txHashTuning cross-knob rationale (genuinely non-obvious; metaStoreTuning / ledgerTuning carry a one-line contrast on why their compaction policy differs). * Tests: drop function docstrings throughout. Function names and fixtures carry the intent. * Delete pkg/stores/doc.go. Production code-to-comment ratio: 1.34 -> 0.32. Tests: 0.23 -> 0.06. No behavior changes outside the rename. Verified via diff: zero assertions removed or downgraded; every test-side change is either a mechanical rename, a stricter require.NoError addition, or a docstring deletion. * Cleanup: drop dead wrapper ErrNotFound; move translateError - New pkg/rocksdb/errors.go holds translateError (used by all 3 facades; was misplaced in txhash.go). - Remove unused wrapper ErrNotFound — callers go through stores.ErrNotFound directly. - Tweak metastore schema-block comment. No behavior change. * Drop reference-commit reference from testutil package doc * Address Copilot feedback - txhash: precompute [16]string cf-0..cf-f table; cfNameForTxHash is now an array index, no per-call fmt.Sprintf allocation. - testutil: panic on HashTransactionInEnvelope error instead of silently producing zero hashes. - ledger: document IterateLedgers start>end behavior in the godoc. * Address Tamir's review on PR 738 - Tuning calibration: collapse ledgerTuning + metaStoreTuning to RocksDB defaults; drop MinWriteBufferNumberToMerge=2 pin; drop bloom on LedgerHotStore (callers know their keyspace). - Drop unused methods: LedgerHotStore.DeleteLedgers / GetLedgerRange, TxHashHotStore.RemoveEntries, Store.FirstLastKey, and matching pkg/stores interface methods. - Open failure leak fix: destroy s.cache / s.filter when OpenDbColumnFamilies fails (~512MB cache leak per failed Open with txhash tuning). - DecodeUint32 / DecodeUint64 panic on bad length instead of silent zero. - Merge Open into New: single-phase constructor returns a fully-open Store or error. Drop facade Open() methods, ErrStoreNotOpened sentinel, openOnce / openErr fields. constructAndOpen private method retains leak-fix coverage. - Replace IterateFrom with IterateRange: inclusive [start, end] semantics; upper-bound check moves into the wrapper. - Metastore generic-KV: Get / Put / Delete / Batch / PrefixScan on string keys + string values. Drop typed entries, sealed markers, ChunkArtifactKind, schema-key constants, MarkTxHashIndexComplete. Encoding is caller-driven (fmt.Sprintf / strconv). Exhaustive tests. - LedgerEntry docstring documents the caller-side zstd convention. - Rename LedgerStore -> LedgerHotStore, TxHashStore -> TxHashHotStore; MetaStore unchanged. Cold counterparts will live in pkg/ledgers, pkg/txhash when those land. - Inline pkg/testutil fixtures into ledger_test.go (only one consumer); delete pkg/testutil. - Bare "default" CF literals across tests replaced with the defaultCFName constant. * Restructure: per-domain stores under pkg/stores/{ledger,txhash,metastore} - pkg/rocksdb is now Layer-1 only: Store, BatchWriter, Tuning, iterators, encoding, plus the wrapper-level sentinels ErrInvalidConfig, ErrCFNotFound, ErrStoreClosed. - pkg/stores/ledger holds ledger.HotStore + ledger.Entry; cold ledger reader will land alongside when written. - pkg/stores/txhash holds txhash.HotStore + txhash.Entry + the 16-CF nibble routing (cfNameByNibble); cold txhash reader will land alongside. - pkg/stores/metastore holds metastore.Store + metastore.Entry + the string-keyed metastore.BatchWriter. Single tier; no cold counterpart. - pkg/stores keeps only the cross-cutting ErrNotFound sentinel; per-domain stores reach to rocksdb.ErrStoreClosed directly. - translateError dropped: facades propagate rocksdb sentinels verbatim and emit stores.ErrNotFound at the Get-miss boundary.
…728) (#794) * txhash: cold read assembly — federated getTransaction(byHash) (#728) Implements the read half of #728: resolve a tx hash to its transaction by federating candidate sources across the hot and cold tiers and verifying each candidate against the real ledger. - CandidateSource (Candidates + Exact) is the federation seam, satisfied by the exact hot store (*HotStore) and the fingerprinted cold side (*ColdReader / *ColdReaderSet, which fans out across many index readers). - TxReader partitions sources by exactness at construction and consults the exact tier first; for each candidate ledger it reads the raw LCM via an injected LedgerSource and extracts the transaction with the SDK's ingest.LedgerTransactionViewByHash. That view's found=false result is the downstream rejection of residual MPHF fingerprint false positives. - An exact source whose candidate fails to verify yields ErrInconsistent: the hot index and the ledger store disagree, which is corruption, not a miss. Out of scope (owned by query serving / #770): the concrete federated LedgerSource, cold-index discovery + lifecycle, and wiring into the getTransaction handler. Cold-index build orchestration is deferred per #728. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * txhash: trim verbose comments in cold read assembly Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * txhash: address review — collapse to HashIndex, fall through on transient index errors - Replace CandidateSource/Exact/ColdReaderSet with a one-method HashIndex satisfied by *HotStore and *ColdReader through their existing Get. TxReader now holds explicit hot and cold tiers (both slices — the hot txhash store is chunk-bound, so there is one per active hot chunk), removing the adapter layer and two files. - A transient index error no longer aborts the lookup: it falls through to the remaining indexes and is surfaced only if nothing else resolves, so a hot-store blip can't mask a cold-resident transaction. - NewTxReader validates its ledger source and passphrase. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * txhash: trim doc comments reintroduced during the refactor Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * txhash: surface unavailable cold candidates as incomplete A cold (fingerprinted) candidate whose ledger is unavailable can't be proven a clean miss, so record it as a soft error and surface it on a total miss instead of silently skipping it. Addresses Codex review on #794. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * txhash: keep scanning after cold-candidate ledger/extract errors Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* events,eventstore: single-chunk query engine Implements #663 — the per-chunk query coordinator. Built on the Reader interface so it serves HotStore and ColdReader without tier branching. Filter semantics match the scope bullets: AND within a filter (contract ID + topics 0..3), union across filters, restricted to an optional event-ID window. MaxEvents caps the result ascending or descending. A defensive view-path post-filter drops term-hash collision false positives so result correctness doesn't depend on the xxh3_128 hash being injection-free. The engine's window input is a chunk-relative EventIDRange, not a ledger range. Ledger-bounded queries can't express "strictly after event X" without fragile post-skip work that stalls on Soroban ledgers holding more events than MaxEvents. Adapters that think in ledgers call the EventIDRangeForLedgers helper to translate via the chunk's LedgerOffsets before populating QueryOptions.Range — the rationale lives in the package doc on top of query.go. go test -race / go vet / gofmt clean. 43 tests covering filter combinations, ledger-range clipping, ordering and limit in both directions, post-filter rejection, and HotStore/ColdReader parity. * events,eventstore: address golangci-lint findings - drop unused gocyclo from Query's nolint directive (gocognit+cyclop+funlen are still needed) - switch a for-i loop in the test fixture to range over an integer (intrange linter, Go 1.22+) - drop an unused nolint:gosec directive on the cold-freeze helper No behavior change. All 43 tests still pass under -race; go vet / gofmt clean. * events,eventstore: address codex PR review findings P1 (fixed) — empty leading-ledger windows now stay empty. EventIDRangeForLedgers can legitimately produce EventIDRange{0, 0} when the requested ledger window is an empty prefix of a chunk (e.g. the chunk's first ledger has zero events but later ledgers do). The previous EventIDRange.resolve treated End == 0 as a "whole chunk" sentinel and silently expanded that empty query to return events from later ledgers. Switch QueryOptions.Range from value to *EventIDRange so nil (whole chunk) is distinguishable from an explicit literal range that happens to be {0, 0} (empty). Drop the End == 0 sentinel from resolve; End is now strictly literal (still clipped to EventCount on the upper end). The pointer indirection is one allocation per Query call — negligible against the bitmap and fetch work. Regression test added (TestQuery_EmptyLeadingLedgerRangeStaysEmpty) that reproduces the exact Codex scenario and fails against the old semantics. P2 (documented) — refill-after-collision-dropouts. With MaxEvents set, bitmap-level capping runs before the collision-defense post-filter, so a term-hash collision can consume a cap slot and then get dropped. Probability is ~2^-64 per term (xxh3_128), so effectively unreachable. Added a caveat to QueryOptions.MaxEvents acknowledging the gap; a refill loop is the right fix if a real workload ever exhibits it. * events,eventstore: make Range mandatory with snapshot-isolation contract QueryOptions.Range moves from *EventIDRange to EventIDRange (value). Both fields (Start, End) are now caller-supplied; the engine does NOT invent an upper bound from EventCount. The contract: the caller pins End once at request entry from a snapshot of the chunk's offsets (LedgerOffsets.TotalEvents()) and threads it through every Query call for that request. Events ingested after the snapshot are invisible to the in-flight request. This is the standard pattern for queries over continuously-updated stores (Postgres MVCC, RocksDB snapshots, Lucene IndexReader). The pointer-vs-value question is moot once End is mandatory — there's no "did the caller set Range?" ambiguity to resolve. The zero-value EventIDRange{} is now a legitimate empty range (no events match), not a sentinel for "scan everything." Production callers compute an explicit End; tests use the wholeChunk helper to pin one. Validation: - Range.End < Range.Start now surfaces an explicit error (programmer bug — swapped args, off-by-one in cursor arithmetic). - Range.Start == Range.End remains a legitimate empty range (the Codex P1 case: empty leading-ledger window from EventIDRangeForLedgers). - Range.End > EventCount still clips down silently (defense-in-depth against stale snapshots). Test diff: every test updated to pin its own snapshot via the new wholeChunk(t, r Reader) helper. New TestQuery_InvertedRangeRejected covers the Start > End validation; the empty-leading-ledger regression test still demonstrates the Codex fix. go test -race / go vet / gofmt clean. 45 tests. * events,eventstore: clean up query.go comments Multi-pass comment cleanup, no behavior change. Trims and reorganizes: - Drop the unenforced "caller guarantees ≤15 filters" framing in the package doc, step 1 comment, and indexOfOrAddTerm — no code enforces it; the protocol fan-out can produce more. - Drop the "End == 0 means whole chunk" sentinel reference now that the snapshot-isolation contract makes End mandatory. - Drop the MaxEvents collision caveat (~2^-64 per term — effectively unreachable; not worth weakening the API contract). - Drop the 12-line bitmap-ownership block at the Query header; the invariants are documented inline at the steps where they bite, and a one-line restatement at the header was confirmed redundant. - Move the collision-defense rationale from a 7-line inline comment in Query's body onto postFilter's docstring itself. Function's purpose belongs on the function, not the caller. Query's call site collapses to a one-line trailing comment. - Drop the borrowed-buffer paragraph in fetchAllInRange's docstring — already covered by inline comments at the actual bytes.Clone and end-count assignment lines. - Trim "explains absence" docstring fragments (filterPlan's "no precomputed flag for ContractID," etc.) and unverifiable perf claims (escape analysis, stack residency). - Package doc leads with what the engine does rather than a "two reasons for event-ID range" justification block. Every load-bearing invariant preserved: bitmap-ownership at step 3, race-clip rationale at step 5, descending arithmetic at step 6, snapshot-isolation contract on EventIDRange, collision-defense on postFilter, O(maxTopicIdx) walk rationale on collectTopicViewBytes, borrowed-buffer + bytes.Clone at fetchAllInRange's clone site. Net: ~-100 lines of comments across query.go. * events,eventstore: adapt to SDK ContractEventView API changes feature/full-history bumped go-stellar-sdk past breaking changes in the view extractors. Two adjustments: - ContractIdView.Value() now returns xdr.ContractId (an array) instead of []byte. Take cid[:] to produce the byte slice the post-filter needs. - ContractEventBodyView.V() now returns (int32, error) directly; the old intermediate bodyV.Value() call is gone. No semantic change. * events,eventstore: short-circuit Start == End empty ranges Addresses @chowbao's review nit on PR #796: half-open range [Start, End) with Start == End is a well-formed empty window, and the engine was doing filter validation + EventCount + resolve only to return (nil, nil) at the end. Now we return up-front, before any of that work. Placed above validateFilters intentionally — for empty input the filter contents are irrelevant; a caller's filter bug surfaces on the next non-empty call. ShortContractIDRejected test updated to pin a non-empty Range so it still exercises the validation path. * events,eventstore: tighten Range/Order API Two related cleanups, motivated by @chowbao's PR #796 review: 1. Drop EventIDRange.resolve and the defensive End-clip. Under the snapshot-isolation contract a properly-pinned End is always ≤ the chunk's current EventCount (chunks only grow). The silent clip was masking a real calling bug — wrong chunk's offsets or a stale snapshot. Replace with an explicit error. The clip logic was the helper method's only reason for existing; inline what's left. 2. Replace the Order enum with a Descending bool. `Order` had two values that the engine immediately converted to a `descending bool` for every internal use. The enum was a public- API costume over an internal boolean; the speculative "third order" extensibility never materialized. Drop the type and the two constants; QueryOptions.Descending defaults to false (ascending — the getEvents v1 default). Tests updated: - RangeEndBeyondChunkClips → RangeEndBeyondChunkRejected (tests the new strict error instead of silent clip). - RangeStartAtOrAboveEventCountReturnsEmpty: deleted; same case is now covered by the strict check. - ChunkWithLedgersButZeroEvents: dropped the overshoot-range sub-case (now an error, not a test of clip behavior). - Order: OrderDescending → Descending: true across all call sites. No semantic change for callers that follow the snapshot contract. go test -race / vet / gofmt clean. * events,eventstore: address @tamirms PR #796 review Three doc-drift fixes from the recent Range/Order tightening: - Package doc: {filters, Range, MaxEvents, Order} → Descending. - EventIDRangeForLedgers: clarify that the input ledger window is closed [L1, L2] and the result is half-open [firstID, lastID). - fetchAllInRange: drop the stale reference to resolve(); point at Query's actual preamble checks. Plus the optional readability improvements from the same review: - Cardinality sort: cmp.Compare instead of the four-arm switch. - indexOfOrAddTerm: slices.Index instead of the hand-written loop. - Filter gets isMatchAll() and termKeys() methods. hasMatchAllFilter and Query step 1 collapse onto them. - EventIDRange gets isEmpty() and check() methods. Query's preamble reads as intent rather than inline conditions. Per @tamirms's explicit warning, matchesAnyFilterView and collectTopicViewBytes stay loop-shaped — the lazy cidResolved/topicsWalked cache shared across filters is the whole point of that shape, and methodizing per-filter would re-resolve the view per filter per event. No behavior change. 49 tests still pass under -race; vet / fmt clean. * events,eventstore: trim verbose docstrings on the new helper methods
* rewrite getLedgerRange functions with views * replace RawLedger usage with xdr.LedgerCloseMetaView * modify StreamAllLedger/StreamLedgerRange + callers to take/pass a view * linter/ineffectual err assign * fix shadowing bug in migration * optimize getLedgerRange by NOT fetching full blob * minor port finalizations * add bench test * minor comment adjustment * add human-readable comparison * add explanatory comment * fix missing verb in comment Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * simplify bench test * restore migrations/StreamLedgerFn to operate on LCM rather than views * replace GetLedgerView with loan-shaped WithLedgerRaw on ledgerReader * error optimization on ledgerInfoFromRow * fix parse failure error overreporting * add WithLedgerRaw test * fix getLatestLedger error reporting --------- Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
#910) * packfile: cover record payloads with a widened record CRC32C index.pack was the only cold artifact whose payload bytes had no integrity coverage, and the consequence is worse than a missing check: roaring's UnmarshalBinary accepts a flipped container bit and yields a DIFFERENT posting set, so a corrupt index answers queries wrongly rather than failing. events.pack and the ledgers pack are covered, but only incidentally, by the content checksum in each zstd frame. index.pack is written in passthrough mode, where nothing supplies one. The trailer CRC32C covers the trailer. A multi-item record's tail CRC32C covers only its FOR-encoded item sizes. Neither reaches the payload. Integrity is an axis of its own, not a codec: whether a record carries a checksum is independent of which codec produced its bytes, and the Format value the codec is chosen by cannot express it. So instead of adding a codec that appends a checksum, WriterOptions.RecordChecksum widens the coverage of the CRC32C that is already the last four bytes of every multi-item record, and a trailer flag records which range it covers: flag clear: [payload][FOR sizes][1B W][4B min][4B crc over the FOR region] flag set: [payload][FOR sizes][1B W][4B min][4B crc over the whole record] Same offset, same width. index.pack therefore pays no bytes at all, and the reader needs no configuration, because the flag is on disk: no writer/reader pairing to get wrong, and an older reader meeting a newer file fails loudly through the existing unknown-flags check rather than misreading it. The compressed artifacts leave the flag clear and pay nothing, which matters most on the freeze path, where copying ledger frames verbatim would otherwise have meant a pass over 1.28GB per pubnet chunk (2.3s per stress chunk) to re-cover bytes the frames already cover. Only single-item records, which have no FOR index to share, grow by four. Verification runs before anything parses the record. That also settles an ordering wart in the FOR path, which ran DecodeGroup on unverified bytes and then located the CRC window with the length that parse reported. The widened range follows from the record bounds in the offsets index, which Open has already verified, so nothing the record claims about itself selects the bytes being checked. Cost where it applies: one CRC32C pass at a measured 8.3GB/s, about 1.5% of the I/O that fetched the bytes, and no copy — the reader verifies in place and keeps aliasing its read buffer. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * packfile: checksum the app-data section App data was the one tail section with no integrity coverage. The offsets index carries its own CRC32C and the trailer's covers itself, but the trailer CRC stops before the app-data bytes, and the library's stance was that callers are responsible for their own app-data integrity. No caller took it up, and the data is the kind where that matters: a flipped byte inside events.pack's cumulative ledger offsets can preserve monotonicity and the final total, passing every structural check its decoder makes, and silently shift which ledger a query resolves to. The ledgers pack's firstSeq is four bytes behind nothing but an overflow check. The trailer has four reserved bytes at offset 68, so this costs no space: put a CRC32C over the section there and verify it at open, where the bytes have just been read anyway. A flag bit says the field is populated, so a file written before it meant anything still opens. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * events: translate packfile corruption to the store sentinel stores/errors.go documents stores.ErrCorrupt as the per-domain integrity sentinel that stores are supposed to translate their backing primitive's corruption signal into, and the ledger store has a translateReaderErr doing exactly that. The event store had none, so a packfile integrity failure surfaced wrapped in an events-prefixed message but invisible to errors.Is(err, stores.ErrCorrupt). That was survivable while index.pack reported nothing to translate. Now that its records and the app-data section are checked, the signal exists and needs somewhere to land, so add the ledger store's translation and apply it wherever a packfile error leaves a public method: the two read paths, the range scan, and the metadata load. The two tests corrupt real artifacts and assert on the sentinel, which covers the whole chain: the writer's trailer flag, the reader verifying without being configured to, and the translation. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * packfile: review pass over the record and app-data checksums Cleanups from a /simplify review of the three preceding commits. The on-disk record layout is untouched; the app-data checksum is no longer optional (below). packfile-library.md was the largest item. It owns the format's integrity model and it contradicted the code in six places: it still said app data had no packfile-level check and that the library never wraps payloads, still showed offset 68 as reserved with one flag bit, and in two places recommended implementing integrity as a codec — the pattern codec.go now tells callers not to use. A reader following it would have written a CRC-appending encoder and ended up with two checksums, one of which the library cannot see. The record section now carries both layouts and the ordering property as a table. flagAppDataCRC is gone; app data is verified unconditionally. The writer always set the flag, so the reader's guarded branch could never be false and its test had to hand-forge a trailer no writer emits. The only thing it bought was opening a file built before the field meant anything, and v2 is not live: rebuilding a chunk costs ~100s, while a permanently-true flag bit costs forever. Such a file now fails to open, and the error says it has to be rebuilt rather than implying corruption. Also: - sealRecord takes the payload and the FOR group rather than an assembled record plus the group's length, which both call sites were passing after doing the append themselves. The covered range is then named directly instead of recovered by slice arithmetic. - index.pack's checksum choice moves to cold_format.go beside its format ID and record size. It is part of the artifact's on-disk identity, and every builder has to agree on it; the streaming builder on the p99 branch writes the same artifact and would otherwise have silently produced an unchecked one. - The cold reader now rejects an index.pack built without the checksum. Serving unprotected bitmaps is the silent-wrong-answer case this exists to prevent, and validateMPHF already checks every other pairing invariant right there. - Comments: the "verified before parsing" argument was written four times, so it stays on verifyRecordCRC alone. packfile no longer explains an app-data checksum in terms of events.pack's ledger offsets, and the event store no longer asserts packfile's record tail layout to justify a cost figure. Tests: the RecordChecksum validation case joins TestCreateValidation's table, which also gains it an error-message assertion it did not have standalone; the app-data write sequence is one helper shared with TestAppDataRoundTrip instead of a third copy; a single-use closure factory and a trailer field parsed but never asserted are gone. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * packfile: fixes from a code review of the checksum commits Two independent review passes over the four preceding commits. No correctness bug was found in the record or app-data checksum itself; both reviewers reproduced clean failures across a byte-by-byte corruption sweep, crafted trailers, degenerate records, and the concurrent read paths. These are the real findings. validateMPHF returned index.pack's open error raw, so corruption in one half of that artifact reached the house sentinel and corruption in the other half did not: a flipped byte in a record surfaced as stores.ErrCorrupt through LookupKeys, while a flipped byte in the offsets index, the trailer, or the app-data section surfaced from the same call with no sentinel at all. A consumer keying rebuild off stores.ErrCorrupt, which stores/errors.go promises it can, would have handled one and silently mishandled the other. The stale-build rejection added in the review pass had the same shape and is now the sentinel too, for the same reason: the artifact cannot answer queries. A crafted trailer claiming totalItems > 0 with itemsPerRecord == 0 passed Open and then panicked on the first read, because the guard was gated on recordCount > 0 and the cross-validation on itemsPerRecord > 0, so a trailer setting neither slipped between them. Pre-existing, reproduced on the base branch, one condition to close. Tests: the narrow row of TestRecordChecksumVerifiesBeforeParsing asserted ErrCorrupt, which ErrChecksum wraps, so it passed whether the parser or the checksum rejected the record and could not pin the ordering it names. A new test covers a checksummed file that also carries app data, with an encoder whose output exceeds its input — the one case buildRecord's pre-size cannot absorb, so recordWorker reallocs before sealing. Another covers index.pack corruption at open, the path the sentinel gap was on. And the app-data corruption test regained the zero-size guard that inlining its closure dropped, without which a regression would flip the trailer magic instead and fail as ErrMagic. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * stores: translate a cold reader's metadata errors too The ledger cold reader translated packfile errors to the stores sentinels on its record reads but not in loadHeader, which reaches the packfile through Trailer and AppData. Corruption there surfaced as a raw packfile.ErrCorrupt, so LastSeq, GetLedgerRaw and IterateLedgers all failed without matching stores.ErrCorrupt, against the contract in stores/errors.go that names the trailer explicitly. The gap predates the app-data checksum: magic, version, size and trailer CRC failures all took the same untranslated path. Covering app data with a CRC32C only added another way to reach it, which is how it was noticed. loadHeader has six return points and every one of them reaches callers through the sync.OnceValues init, so the translation goes there rather than at each return. A return added later is covered without anyone remembering, and it costs nothing: init runs once per reader. The helper itself was duplicated byte for byte in the ledger and event stores, which is what let the two drift apart in the first place. It is now one stores.TranslatePackErr. Considered and rejected: having packfile.ErrCorrupt wrap the store sentinel, which would need no call sites at all but contradicts the library's caller-agnostic design and would cover only one of the sentinel's sources — the hot store and the index validator construct it directly. A translating reader wrapper was also tried and measured: it costs 2.77ns per element on ReadRange (1.9ns of interposition, 0.9ns of translation), all of it on the success path, to guard an error that is never hit on the path that pays. The regression test drives all three affected methods over a pack whose app data has one flipped bit, and fails on all three without the change. Also closes a packfile reader the index-offsets corruption test opened inline and left open for the rest of the test. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * stores: translate the close path too packfile.Reader.Close joins the deferred open error into its result, so on a reader that is opened and closed without a read, Close is the first place an open-time failure surfaces — and the only one, since nothing else ran. Both cold readers returned that straight through, so a close-only path reported raw packfile corruption instead of the store sentinel. Close is a public method, so it owes callers the same sentinel as every other. The tests cover the close-only shape specifically, since a reader that has already been read gets its error from the read instead and would hide this. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * stores: make the translation a property of the handle The two preceding commits fixed the two places that forgot to translate a packfile error, one found by review after the other. Two independent misses in one review cycle is not forgetfulness, it is what opt-in translation costs: every call site is a chance to skip it, and skipping it is silent. So the stores no longer hold a *packfile.Reader. They hold a stores.PackReader whose methods translate, which makes the L2 boundary a property of the handle rather than something each caller remembers. There is no way to obtain an untranslated error from it, and a method added later has to translate to compile — which already earned its keep: sizing the type from the existing call sites missed ReadItems, and the compiler caught it where a reviewer would otherwise have had to. This was measured and rejected earlier in review on the strength of a bad benchmark. Wrapping the ReadRange iterator costs ~3ns an element, which was compared against bare packfile iteration at ~16ns an element and read as +17%. The real scan decodes each frame and costs ~470ns an element, so the true figure is near 1% — and it buys the two gaps above plus the next one. The earlier number measured a component and drew a conclusion about the system. Both loadHeader's chokepoint and the Close translation the preceding commits added are now redundant and go away, along with the standalone helper. The regression tests are unchanged: they assert the behaviour, not the mechanism, and all four still fail if the translation is broken. What this does not cover is unchanged. The sentinel has producers that never touch a packfile — the hot store's decode failures and the index validator construct it directly — and rocksdb-backed stores translate their own. This closes the packfile boundary, which is where both defects were. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * packfile, events: pin two guards that no test held Both guards were added by this branch and neither was covered, so either could have been removed by a later simplification without a test noticing. The reader's itemsPerRecord guard rejects a trailer that claims items but no itemsPerRecord, which used to index past the offsets slice and panic on the first read. Reaching it from a real file takes some care: zeroing indexSize trips the index-too-small check first, and keeping the original leaves unconsumed index bytes, so the fixture starts from an EMPTY pack, whose index decodes to zero groups and consumes all of itself, and then claims items in the trailer. The cold reader's stale-build guard rejects an index.pack built without a record checksum. Its fixture clears the trailer flag and re-seals the trailer CRC, so the pack is structurally valid and complete and only the flag is missing, which is what a pre-checksum build looks like. Both tests assert the specific error, not just stores.ErrCorrupt. That is load-bearing: with only the sentinel asserted, both still passed with their guard deleted, because an unrelated check rejects each fixture for its own reasons. A test that cannot fail for the reason it exists is not coverage. index.pack's format and checksum are now validated for every chunk rather than only for one holding terms. An eventless chunk took an early return that skipped them, so it would accept a foreign or unchecked pack. Nothing could be served wrong from a zero-term index, so this buys uniformity rather than a fix: the invariant no longer has a carve-out that has to be remembered. It costs nothing, since the open is already in flight. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Also corrects prose this branch made stale. Comments in both cold readers still described the reader as opening through packfile.Open and holding a packfile.Reader, and WithLedger's comment explained why it captures the callback error by naming translateReaderErr, which this branch deletes. That comment documents a live hazard, not a historical one: the handle translates every error ReadItem returns, so a caller's error routed that way would come back reclassified as a store failure. The check-list comment above the index-pair validation still said "three cheap checks" and named the trailer Format check, which the hoist in this commit moved above the empty-index branch. It is two checks there now. * packfile: commit the record-checksum corruption sweep as a test The byte-by-byte sweep was the PR's headline evidence but lived only in the PR description. It now runs at itemsPerRecord 1 and 4 with masks 0x01, 0xFF and 0x80 over every byte of a small fixture, and asserts that a checksummed file never reads back different bytes without an error. That also gives the single-item widened layout, which has no FOR index, its first file-level corruption test. The unchecked rows are logged, not asserted: they show the gap being closed, which is what makes the checked rows meaningful. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * packfile: a compressing codec does not always supply a checksum The RecordChecksum doc and the design doc both said a compressing codec covers its own bytes. That is true of the default zstd compressor and false of zstd.NewCompressor(zstd.WithoutChecksum()), which this repository exposes. No caller uses it today, but a reader of either doc could pick it and end up with an unprotected artifact while believing it is covered. Both now say which case needs ChecksumCRC32C. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * packfile: say when the format version has to be bumped The comment said to bump on any breaking trailer change, and this branch made one (the reserved bytes at 68:72 are now a mandatory CRC) without bumping. Nothing is released, so no reader has to keep opening older files and every artifact is rebuilt. The comment now says that, so the rule and the code agree. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * design-docs: Verify does check record CRCs, as a side effect Verify streams every item through ReadRange, and every record read runs verifyRecordCRC first, so on a file with a content hash the record CRCs are checked too. The sentence saying Verify does not cover them was half true and could send a reader off to add a second pass. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: Karthik Iyer <karthik.iyer@stellar.org>
…#967) * rpcv2: refuse foreign catalogs at open (the storage-format census) A catalog holding entries outside this binary's exact vocabulary was previously interpreted: unknown keys hard-errored mid-scan, unknown values were raw-cast into lifecycle states, and the resolver would treat a newer binary's artifacts as missing work and overwrite them. Now Open scans every key and value against the exact vocabulary this binary writes (the three state families, the earliest_ledger pin as a canonical round-trip, and the 32-byte meta/catalog-secret) and refuses to start on anything else, naming the offenders with the secret's value redacted and a two-sided message: either written by a newer stellar-rpc, so deploy that version or newer, or corrupted. The census runs inside Open, after the RocksDB open and BEFORE the secret mint, so a refused Open writes nothing into the tree it refuses. Two engine-layer companions: RocksDB open failures matching the known newer-RocksDB signatures are re-labeled with the same deploy-newer hint, since a library bump would otherwise read as corruption on rollback; and every column family now carries an explicit block-based-table format_version pin so a grocksdb upgrade cannot silently change the on-disk table format. Raising the pin is a declared storage-format change. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * rpcv2: finish the cold formats before they freeze at release one Release-one artifact bytes are immutable forever, so every self-description and integrity hook has to be in them from the first artifact. This lands the finishing set: - .bin gains a magic ("SBIN") and version prelude; it was the one cold file with no self-description at all, and a same-width layout change was silently misread. The header scan now rejects foreign and newer files. - The txhash .idx metadata blob and the ledger pack's app-data gain leading version bytes, completing the convention that every app-data blob is self-describing independent of the container's Format id. - index.pack's empty app-data slot gains an 11-byte build stamp: stamp version, term-schema version, and the indexed-field bitmask. The cold reader validates it against the compiled constants, so an index missing a term family becomes distinguishable from one that matched nothing. Freeze and walk write identical stamps (all constants), and decoding ignores trailing bytes as extension room. - The three pack writers enable the packfile content hash. Items reach the hasher in canonical pre-compression form, so the stored hash is independent of the zstd encoder version and audits can content-compare without pinning compressors. - Byte-level golden fixtures pin the term-key derivation and the routing-key blinding: those bytes are on-disk format (every frozen index is built over them), and until now a change to the hash inputs passed the suite because tests compared the functions to themselves. A silent change now fails CI with a demand to bump TermSchemaVersion or revert. - bench-ingest's help warns that it overwrites cold files in place with no catalog trace when pointed at a live deployment's roots. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * design-docs: the format versioning and upgrades section Records the policy the census implements: forward is seamless, backward refuses. The convergence guarantee is re-scoped to states this binary's own protocol can produce, since the census deliberately refuses rather than converges catalog content outside the compiled-in vocabulary. The new section covers where format identity lives (catalog values, with the state@id grammar reserved for the first bump; per-file self-description as witnesses), the bump razor and its non-obvious corollaries (term families bump hot too, RocksDB dependency bumps are format-touching, write-path switches flip walk and freeze together), and the per-tier upgrade shapes: cold is write-new read-old forever, hot discards and re-ingests, capability gaps are loud errors rather than silent empty results. The transactions design's stale .bin and .idx layouts ride along: both predate keyed routing, and the .bin block now shows the magic-and-version prelude, the recorded secret, and the blinded key. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * rpcv2: review pass over the versioning branch Eight adjudicated findings from a high-effort review, applied; one refuted by design (the build stamp's exact-match refusal stays: it fails closed, and per-id read sets with query-time capability gating arrive with the format-id grammar, where this check is revised). - The newer-engine error wrap moves from catalog.Open into rocksdb.New, so hot-chunk opens (the likeliest place to meet a newer-engine DB after a rollback) inherit it too; its text now also names the mispointed-path alternative, since the column-families signature fires for both causes. - geometry exports the state-token registries (AllStates, AllHotStates, IsKnownState, IsKnownHotState) and the census and its tests derive their vocabulary from them, so a token added in geometry can never make the daemon refuse its own catalog. - The census's unknown-key refusal no longer prints the value (length only): under a newer binary an unknown key may hold key material, and the doc promises refusals never print secrets. Test pins it. - Version bytes are checked before lengths in the ledger AppData and the txhash cold metadata decoders, so a longer newer-format blob reports as an upgrade problem instead of a corruption-shaped size mismatch. - The index.pack format and build-stamp checks now run for eventless chunks too (pre-Soroban history is entirely eventless chunks, so the refusal contract must not be data-dependent). - Stale comments fixed: the BBTO install policy above applySharedTableOptions, the appData size in the wrong-size test, and the field-addition doctrine on TermSchemaVersion. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * rpcv2: simplify pass over the versioning branch Findings from four parallel quality reviews (reuse, simplification, efficiency, altitude), deduped and applied; the efficiency angle came back clean. - The leading-version-byte gate becomes one shared helper, stores.CheckBlobVersion, used by the ledger AppData, txhash metadata, and index.pack stamp decoders; the two pre-existing event decoders (LedgerOffsets, index.hash metadata) adopt the same version-before- length order, so the policy and its newer-binary phrasing live once. - ensureSecret's wrong-length branch is deleted: the census runs first on the only call path and owns that validation, and the branch's error had just lost its only test. The secret width now has a single source (catalogSecretLen types the arrays). - TestBlinding_Golden is dropped: stores/blind_test.go already pins BlindKey and DeriveIndexSecret with known-answer vectors, so the event-package copy duplicated a shared byte contract at the wrong level. The term-key goldens, which had no prior pins, stay. - wrapIfEngineTooNew is unexported; its whole design is that only rocksdb.New calls it. - The .bin magic is a plain "SBIN" string constant compared as bytes, instead of a byte-swapped uint32 that spelled NIBS; on-disk bytes are unchanged and the refusal message now prints the magic readably. - Two comment fixes (the Store.bbtos field, one per CF now) and the restored "Related documents" heading the doc edit had swallowed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * rpcv2: second review round over the versioning branch Six of eight round-three findings applied; two rejected (a census key-family registry as speculative generalization the roll-forward doctrine already covers, and the deliberate byte-pinned test parsers). - The newer-engine relabeling now also covers lazily-surfaced errors: with finite max_open_files RocksDB skips table-reader preload, so a newer-format SST first fails at read time; getPinnedWith and the iterator tails wrap those errors too. - scanBinHeader enforces the .bin reserved bytes as zero, making "reserved" a usable version-1-compatible signal instead of dead documentation, and the O_DIRECT merge reader verifies magic and version itself rather than trusting that scanAndValidate ran one file away. - The events Field enum gains an allFields registry: IndexedFieldMask derives from it and the term-key golden iterates it, so an appended field with no pin or mask bit fails tests instead of silently under-reporting in every build stamp (the previous guard was anchored to the last field and could not trip for exactly that case). - DecodeLedgerOffsets and decodeEventsMeta now call stores.CheckBlobVersion instead of hand-rolling the order it names, picking up the newer-binary hint; the LedgerOffsets unknown-version sentinel had no caller and is gone. - The index-pair validation comment states its real guarantee: lookup-path-only, with payload reads independent by design. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * rpcv2: complete the newer-engine wrap and reserved-byte coverage Two review comments on the PR, both valid completions of earlier commits here rather than nitpicks. BatchMultiGet and LastKey were the two remaining read paths returning engine errors raw, so a newer-format SST met lazily through FetchEvents or LastSeq lost the deploy-newer hint the wrap promises; both now route through it. And the merge reader's self-defending prelude check now enforces the reserved bytes as zero, matching scanBinHeader so the two .bin consumers agree on the format. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * catalog: drop the secret-width constant; the field's type is the width catalogSecretLen restated a width the [32]byte array type already expresses, and spreading it into the array spellings churned base lines for nothing. The census now checks len(c.secret), a type property valid before the mint, and secret.go returns to the base spellings. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * rocksdb: drop the newer-engine error relabeling The format_version pin is the mechanism that prevents an engine-format drift; the relabeling only decorated error text in a scenario that requires the pin's own doctrine to have been violated first, and its cost kept growing: a signature list coupled to RocksDB's error strings across library versions, and call sites that multiplied with every review round. RocksDB's own messages already carry the hypothesis ("Unknown Footer version. Maybe this file was created with newer version of RocksDB?"), so the wrapper, its signature list, and all its call sites are gone. The pin, the always-installed BBTO, and their test stay. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * bench: drop the help-text warning; off-topic for the versioning PR Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * stores: negative tests for the stamp gate and the reserved bytes Two review comments, both valid test gaps in this branch's own additions: nothing exercised the schema/mask refusal through the reader (the capability gate a refactor could silently drop), and the .bin reserved-byte rejection had no fixture in either consumer. The new reader test covers both mismatches on eventful and eventless chunks; the .bin refusal test gains a reserved-byte fixture asserted against the pre-scan and the merge reader. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * census: reject cross-window index coverages; two more format pins Round-three review comments, all valid. A syntactically valid coverage key whose endpoints lie outside its own index window is something no binary ever writes, and left accepted a corrupt frozen one would make FrozenIndexCoversRange suppress a legitimate rebuild; the census now validates both endpoints through the index layout, and the acceptance fixture (which itself wrote cross-window keys) is corrected. Two test gaps close alongside: events.pack gains the same content-hash presence and Verify assertion the ledger and index packs already had, and a TermsForBytes golden over a fixed marshaled ContractEvent pins the value encodings feeding the term hash, which the ComputeTermKey golden alone could not see change. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * stores/event: stamp-test fixture carries the record checksum The rebase onto feature/full-history picked up #910, which makes the cold reader refuse any index.pack whose trailer lacks the record-checksum flag. That gate runs before the build-stamp gate, so the pack rewriteIndexPackStamp writes was refused as a stale build and every subtest of TestColdReader_RejectsMismatchedBuildStamp failed with the wrong message. Pass the same RecordChecksum the production writer passes, so the fixture differs from a real index.pack only in its stamp. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * rpcv2: review round three, the two open Copilot threads - IndexedFieldMask becomes an accessor over an unexported var. The loop over allFields stays the single source of truth. No importer can reassign the value the index.pack build stamp records. - The census comment and the design doc no longer claim the refusal is write-free. Open writes no catalog entry of its own. RocksDB may still create housekeeping files and flush a previous binary's WAL on Close. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * rpcv2: review round three, consolidation nits - The index.pack stamp gate moves out of OpenColdReader into checkIndexBuildStamp, beside its decoder. Same checks, same strings. - The .bin prelude check becomes one checkBinPrelude helper, called by scanBinHeader and by the merge reader. The foreign-header test now runs every mutation against both readers. - IsKnownState and IsKnownHotState name their constants in a switch, so a state added to the const block but not here fails the exhaustive linter instead of the next restart. - The census refusal table uses geometry.StateFrozen instead of a literal. - The ledger app-data size test adds the version-byte-only case. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * rpcv2: review round three, comment and doc trims - pinnedTableFormatVersion says that 6 is librocksdb 10.10.1's default and that the pin is a deliberate choice against RocksDB's advice. The repeated "explicit BBTO keeps the format pinned" sentences are cut to one. - TermSchemaVersion's comment drops references to grammar that does not exist in code yet and points to the design doc instead. - appDataSize is written as 1 + 4 with field names. - ParseColdMetadata and DecodeLedgerOffsets wrap the version check in their own sentinel, so errors.Is holds for an empty blob. Tests assert the sentinel again. - Design doc: the format-identity paragraph states the per-blob extension rule, who verifies the content hash, and that a term-schema change ships as a new events format id. One doubled blank line removed. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * rocksdb: the pin comment names the librocksdb version the repo builds, 10.9.1 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: Karthik Iyer <karthik.iyer@stellar.org>
* rpcv1: close captive core before waiting on the ingestion worker daemon.close() waited for the ingestion worker to exit and only then closed captive core. While the daemon is still catching up, that worker sits inside a blocking "stellar-core catchup" command which watches the ledger backend's context, not ingestion's. Nothing could interrupt it except the close that ran afterwards, so shutdown stalled for as long as stellar-core took to exit by itself. Split Service.Close into Stop and Wait, and order shutdown as Stop, core.Close, Wait. Cancelling ingestion before the close matters: it makes the retry loop treat the errors the backend shutdown produces as a clean cancellation rather than a fatal ingestion failure. * ci: report per-test integration timings The job logged only a package total, which cannot distinguish a uniformly slow suite from one slow test. Run with -v and write the twenty slowest tests to the job summary. * ingest: use the American spellings misspell expects * integrationtest: remove the causes of random job failures - Resubmit while Core answers TRY_AGAIN_LATER. The validator still holds the account's previous transaction in its queue when RPC, fed by captive core, already reports it as applied. - Re-arm the protocol upgrade on every poll. An upgrade armed before Core finishes booting is dropped and Core stays on protocol 0. - Pick captive core ports outside the kernel's ephemeral range so a client socket cannot take the port before Core binds it. - Print the last ledger seen when the backfill wait times out. * integrationtest: bound the sendTransaction retry with a plain for loop and use the test context
…on ubuntu-24.04 (#977) * ci: share native-library caches across PRs and run integration tests on ubuntu-24.04 - Run the workflow on push to feature/full-history so librocksdb, libzstd and the Go module cache are saved on the base branch, where every PR against it can restore them. - Key the Go module cache by OS and go.sum only. One copy per workflow, job and matrix leg crowded the small native-library entries out of the 10GB budget. - Move the integration job to ubuntu-24.04 with the noble stellar-core package, so it shares one librocksdb entry with the build and unit jobs. * ci: pin the same stable Core release as main, from the noble repo * ci: fill the Go module cache on a miss so the first job to save stores a complete one * ci: download Go modules after the protected-branch cache reset
Brings the 18 commits main gained since the v28.0.0 release cut into the two-binary tree. Every rpcv1 change lands under internal/rpcv1/. - sqlitedb, ingest, daemon: backfill optimization (buffered inserts, deferred index build via PrepareBulkLoad/FinalizeBulkLoad) - methods: main's GetLedgerRaw dropped; the branch already serves raw ledgers through store.LedgerReader.WithLedgerRaw - event cursors keep the branch's store.StageSentinels as the single definition of the stage sentinels inside main's batching loop - go.mod: main's dependency bumps taken; the branch's newer go-stellar-sdk kept - CI: the branch's noble stellar-core pins kept; main's bench-campaign and socket-scan workflows added - Dockerfile.rpcv2 gets --no-install-recommends like the other two Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Both runtime images have been on ubuntu:24.04 with noble stellar-core packages since the base-image upgrade; the headers still described v1 as jammy and called the 22.04/24.04 split deliberate. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Merge main into feature/full-history (2026-09-04)
…laster enhancements (#933)
* test: run every integration environment in the parallel batch One fake GCS server starts in TestMain, so the datastore tests no longer call t.Setenv and no longer need NoParallel. TestMigrate becomes parallel too. Harness waits are widened because four environments now boot at once. * test: stop paying for the Core settings upgrade where it is not needed Thirteen tests never submit a Soroban transaction, so the raised resource limits do nothing for them. They now skip the upgrade. The upgrade itself polls Core's /sorobaninfo instead of sleeping a fixed five seconds twice. Also fixes a dead branch: the testnet case compared against the formatted file name, so it could never match. * test: give the datastore ledger window a wait that does not depend on setup being slow The 30s window only ever passed because the settings upgrade slept 10s first. Waiting for the network to close about 16 more ledgers needs a window sized for that, not for whatever setup happened to cost. * test: address review findings on the integration-test changes - the health wait no longer reads what its condition goroutine writes, and the condition no longer logs, so a timeout cannot race or panic - the /sorobaninfo poll waits for a number the second upgrade file sets and enable.xdr does not; 65536 was already present and proved nothing - the number must stand alone, so 3500000 does not match 35000000 - waitForCheckpoint, waitForCoreAtLedger and the backfill waits get the same busy-machine budgets their siblings already got - TestGetLedgers waits for 15 ledgers, since 5 is exactly the first page and leaves nothing for the cursor to return - the fake GCS server only starts when integration tests are enabled * test: drop the speculative slack on the core ingestion wait
…row with chunk fill (#973)
# Conflicts: # .github/workflows/integration-tests.yml # .github/workflows/stellar-rpc.yml
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub. |
* update getTransactionByHash to use views * add GetLedgerView to leaderReaderTx interface + implement * update mocks * update getTransactions handler * update mocks * update tests w/ new golden strings * port finalizations, reduce comment verbosity * restore sparseLedgerReader Tx-level GetLedgerView * store: a raw, loan-shaped ledger accessor on the serving read Tx getTransactions costs ~100ms per limit=50 page, and a CPU profile over 40 such requests attributes 1.83s cum to xdr.LedgerEntryChanges.DecodeFrom plus ~2.9s of GC drain/scan on top: the shared handler fully XDR-unmarshals the entire multi-MB LedgerCloseMeta once per ledger it walks, to read a handful of per-transaction fields off it. It has no choice — the serving interface only offers GetLedger, which returns a decoded xdr.LedgerCloseMeta — yet BOTH backends hold the ledger as raw XDR bytes and decode on the way out. Add the seam that lets a handler skip that decode: WithLedgerRaw(ctx, sequence, fn) (found bool, err error) on store.LedgerReaderTx — the Tx/walk variant, because the paging path is the only caller and the one-shot store.LedgerReader has none. Loan-shaped rather than returning bytes: the v2 backend can then hand over its chunk reader's scratch buffer directly, with no copy and no lifetime question, since the bytes' validity ends with fn. The contract is documented on the interface — valid only inside fn, read-only, must not be retained — alongside the walk-cursor rule the two accessors now share. The backends implement it as pass-throughs of what they already hold: - rpcv2 adapter: GetLedger's walk (deadline check, window gate, priming, the two loud contract failures) moves into a shared walkTo, so the decoding and raw accessors advance one cursor and cannot drift on where the walk is. WithLedgerRaw is walkTo plus fn — no decode at all. - rpcv1 sqlitedb: selects the same `meta` column of ledger_close_meta into a []byte instead of an xdr.LedgerCloseMeta, so the row scan never runs UnmarshalBinary. The lent bytes are ours to lend: scanning a BLOB into a *[]byte goes through database/sql's convertAssign, which clones the driver's row buffer (only sql.RawBytes opts out), and BatchGetLedgers already depends on exactly that by retaining the slices it selects. GetLedger stays: every other handler still uses it. No caller uses the new seam yet. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y7opQUW9UZPKzE6B3tz9b5 (cherry picked from commit d8e0797) * store: promote the view-to-Transaction reshape out of the v2 adapter adapters.transactionFromView turns an ingest.LedgerTransactionView into the store.Transaction the serving handlers format. The shared getTransactions handler is about to need exactly that reshape, and a second copy of it in internal/methods would be a copy that can silently disagree with this one. Move it to internal/store as TransactionFromView, next to ParseTransaction — its parsed-path parallel — and have the adapter call it. Behavior is unchanged: same fields, same aliasing, same caller-side rule about only handing it a view whose bytes were already copied out (that note moves to the adapter's call site, which is where the guarantee actually holds). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y7opQUW9UZPKzE6B3tz9b5 (cherry picked from commit ff21f43) * methods: walk getTransactions' page over raw ledger bytes, not a decoded LCM A limit=50 getTransactions page costs ~100ms. Profiling 40 of them attributes 1.83s cum to xdr.LedgerEntryChanges.DecodeFrom and ~2.9s to GC drain/scan on top of it, and neither is work the response needs: the page loop fully XDR-unmarshaled every ledger it touched — LedgerEntryChanges, bucket entries, the lot — to read a handful of per-transaction fields back out and base64 them straight to the client. Read the ledger through the SDK's zero-copy views instead. Each ledger is now borrowed as raw bytes via the new store.LedgerReaderTx.WithLedgerRaw, and ingest.LedgerTransactionViewRange walks its TxProcessing to materialize only the page's worth of transactions — nothing is unmarshaled, and the walk stops at the page limit rather than at the ledger's end. store.TransactionFromView reshapes each one into the store.Transaction the response renderer already takes, so the whole formatting half of the handler is untouched (it moves into transactionInfo, shared verbatim with the differential's reference below). The same precedent already serves getTransaction by hash in the v2 tree (rpcv2/stores/txhash). One field does not survive the move unaided. For a TransactionMeta V3 whose envelope declares Soroban data, the parsed reader always reports exactly one operation slice, even when the meta carries no SorobanMeta at all (GetTransactionEvents: OperationEvents = make([][]xdr.ContractEvent, 1)), while the view extractor leaves it empty — so contractEventsXdr would come back as [] where it used to be [[]]. stellar-core emits exactly that shape for a Soroban transaction charged but never executed, so it is reachable on real protocol 20-22 history. repairV3OperationArity restores it with one union-discriminant read of the meta plus, only for a V3 meta that came back with no operations, one view walk of the envelope for the Soroban flag the SDK computes internally and does not expose. Still no decode. Everything else — envelope, result, meta, diagnostic/transaction/contract events, hash, apply order, fee-bump flag, ledger sequence and close time — comes off the views byte for byte. The correctness argument is the differential: the pre-change extraction is reconstructed in get_transactions_differential_test.go and both paths are run over one sqlite corpus that sweeps LCM V1/V2, TransactionMeta V1/V3/V4, classic and Soroban and fee-bump envelopes, present/absent/empty SorobanMeta, per-operation and top-level and diagnostic events, empty ledgers, five-tx ledgers, both response formats, page limits that land mid-ledger and on boundaries, explicit cursors, and a full cursor round-trip where each path drives its own cursor chain. Every comparison is json.Marshal byte equality of the whole GetTransactionsResponse (not JSONEq — a reordered or vanished field must fail). ~410 subtests; a corpus-vacuity guard and a renderer field-mapping pin keep them from passing for the wrong reason. Every pre-existing getTransactions test passes unmodified; the only edit to one was giving the sparseLedgerReader stub the new interface method. Behavior note: an unreadable ledger's extraction error is now InternalError throughout. Before, a failure inside the transaction reader (only reachable on a corrupt stored ledger) surfaced as InvalidParams while the same corruption caught by the reader's constructor surfaced as InternalError; the view path cannot tell those apart, and a corrupt stored ledger is not the client's fault. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y7opQUW9UZPKzE6B3tz9b5 (cherry picked from commit 2042a38) [cherry-pick resolution: repairV3OperationArity moved from methods into store.ParseTransaction so the v1 and v2 by-hash view reads are repaired too, not only the pager; TransactionFromView folded into the existing view-shaped store.ParseTransaction; the differential's legacy reference extraction is frozen test-locally as legacyParseTransaction since the decode-based store.ParseTransaction no longer exists on this branch.] * drop Tx-level GetLedgerView, restore the divergence-pinning fixtures The pager now borrows raw ledgers through WithLedgerRaw, so LedgerReaderTx's owned-copy GetLedgerView has no callers left: remove it from the interface, both backends, and the mocks/stubs. Revert the V3 fixture edits (and the getLedgers goldens derived from them): a Soroban envelope with absent SorobanMeta is real protocol 20-22 history, and the old fixtures pin the [[]] contractEventsXdr arity that repairV3OperationArity (in store.ParseTransaction) now preserves on the view path. Both fixtures die with that repair at the SDK pin bump for stellar/go-stellar-sdk#5997. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * rename to withLedgerRawFromDB, specify loan in comment * bump SDK past the view-arity fix, delete the interim repair getevents-v2 took main (go-stellar-sdk#5997 included), so the pin moves to 2cc23142 and the SDK now aligns the V3-no-SorobanMeta operation arity itself: delete repairV3OperationArity, envelopeIsSoroban, and their unit test, and return store.ParseTransaction to its plain single-value reshape. The fixture and differential pins for the [[]] shape stay and now pass on the SDK alone. The bump also picks up the protocols/rpc EventInfo.InSuccessfulContractCall deletion (go-stellar-sdk#5995): drop the field from getEvents' response assembly and test expectations. xdr.DiagnosticEvent's field is unaffected. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * bump SDK + remove InSuccessfulContractCall from getEventsV1 protocol struct * update tests w/ new golden strings * restore sparseLedgerReader Tx-level GetLedgerView * store: a raw, loan-shaped ledger accessor on the serving read Tx getTransactions costs ~100ms per limit=50 page, and a CPU profile over 40 such requests attributes 1.83s cum to xdr.LedgerEntryChanges.DecodeFrom plus ~2.9s of GC drain/scan on top: the shared handler fully XDR-unmarshals the entire multi-MB LedgerCloseMeta once per ledger it walks, to read a handful of per-transaction fields off it. It has no choice — the serving interface only offers GetLedger, which returns a decoded xdr.LedgerCloseMeta — yet BOTH backends hold the ledger as raw XDR bytes and decode on the way out. Add the seam that lets a handler skip that decode: WithLedgerRaw(ctx, sequence, fn) (found bool, err error) on store.LedgerReaderTx — the Tx/walk variant, because the paging path is the only caller and the one-shot store.LedgerReader has none. Loan-shaped rather than returning bytes: the v2 backend can then hand over its chunk reader's scratch buffer directly, with no copy and no lifetime question, since the bytes' validity ends with fn. The contract is documented on the interface — valid only inside fn, read-only, must not be retained — alongside the walk-cursor rule the two accessors now share. The backends implement it as pass-throughs of what they already hold: - rpcv2 adapter: GetLedger's walk (deadline check, window gate, priming, the two loud contract failures) moves into a shared walkTo, so the decoding and raw accessors advance one cursor and cannot drift on where the walk is. WithLedgerRaw is walkTo plus fn — no decode at all. - rpcv1 sqlitedb: selects the same `meta` column of ledger_close_meta into a []byte instead of an xdr.LedgerCloseMeta, so the row scan never runs UnmarshalBinary. The lent bytes are ours to lend: scanning a BLOB into a *[]byte goes through database/sql's convertAssign, which clones the driver's row buffer (only sql.RawBytes opts out), and BatchGetLedgers already depends on exactly that by retaining the slices it selects. GetLedger stays: every other handler still uses it. No caller uses the new seam yet. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y7opQUW9UZPKzE6B3tz9b5 (cherry picked from commit d8e0797) * drop Tx-level GetLedgerView, restore the divergence-pinning fixtures The pager now borrows raw ledgers through WithLedgerRaw, so LedgerReaderTx's owned-copy GetLedgerView has no callers left: remove it from the interface, both backends, and the mocks/stubs. Revert the V3 fixture edits (and the getLedgers goldens derived from them): a Soroban envelope with absent SorobanMeta is real protocol 20-22 history, and the old fixtures pin the [[]] contractEventsXdr arity that repairV3OperationArity (in store.ParseTransaction) now preserves on the view path. Both fixtures die with that repair at the SDK pin bump for stellar/go-stellar-sdk#5997. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * undo needless whitespace changes * bump sdk to version with SHA d8c8acf * move getTransactions differential suite to its own PR Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * remove stale reference to differential test suite * Merge branch feature/full-history into getTxByHash-views * add test pinning legacy v0 meta behavior change * update unused mock field * update cursor behavior described in LedgerReaderTx comment * fix err when txnViewRange == 0 * use transactionInfo in GetTransaction * add changelog notes * set cursor before early return with TransactionOrder at 0 * fix transaction.go stale comment * remove double-blank line in CHANGELOG * fix incorrect comment about ledgers being loaned * remove GetLedger from Tx interface --------- Co-authored-by: Tamir Sen <tamir@stellar.org> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…976) * add getTransactions view-walk differential suite Byte-compares the view-walk getTransactions page loop against a test-local copy of the pre-view decode path over a corpus sweeping LCM/meta versions, envelope and event shapes, page boundaries and cursor round-trips. Split out of #962; the suite is @tamirms's work from tamirms/gettransactions-view-walk. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * refactor to-be-shared methods out of differential test * fix positional zipping regression hole * fix unique hash invariant hole in test * split assertSame into self + assertSameError to check error parity * replace deleted ledgerReaderTx.GetLedger usage * fix test suite header comment * fix corpus gaps on four remaining versions * match legacy cursor bytes for client-built cursors + sweep operation orders * add cursor and request-shape error parity cells * add 3 extra cases to getTx cursor test * minor comment fixes in differential test --------- Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
… four CI legs (#1001) * test: run the shared integration suite against rpcv1 or rpcv2, four CI legs Move the daemon-neutral harness and tests to cmd/stellar-rpc/internal/integrationtest, keep the rpcv1-only tests in rpcv1/integrationtest, and add rpcv2/integrationtest. STELLAR_RPC_INTEGRATION_TESTS_DAEMON picks the daemon; CI runs a protocol x daemon matrix. rpcv2 gains an exported options entry point and the process metrics rpcv1 exposes, so TestMetrics runs unchanged on both. * test: exempt delayed-daemon tests from the harness catch-up gate * ci: read the protocol matrix in the dependency sanity checker * test: fix the harness restart race and the nil daemon on the container-RPC path * test: ask for a ledger far beyond the latest in TestGetLedgers * test: submit through rpcv1's own captive core in the harness * test: wait for the daemon to ingest the ledger that applied a limits upgrade * test: strip the sample's prose from the rpcv2 test config * test: restart on a captive-core port collision and spread the port ranges * test: claim a per-process port range with flock; report rpcv1 exits on the exit channel * test: end the goroutine that called Fatal in the rpcv1 exit hook * test: wait for the archive's first checkpoint; point rpcv2 core_url at 127.0.0.1 * rpcv2: report the bound listener addresses; register the process metrics from one shared function * test: bind rpcv2 to port 0, drop the restart, pin the test config to the sample, reject rpcv1-only settings under rpcv2
* bench(perf-eval): add a windowed relay poller for long campaigns Campaigns outlive one GHA job, so a chain of poll jobs each polls one bounded window of the campaign budget. The new relay command reports exactly one of three states per window — ok, fail, or running, where running hands off to the next poll job — and the workflow gates on the state output. The launch job seeds RESULT_KEY with a pending marker, so the object exists for the campaign's whole life and persistent fetch errors read as a fault, not a slow campaign. Relay and the existing gather command now share one resultPoller, so the two S3-protocol consumers cannot drift. Gather gains from the merge: fail-fast after 10 consecutive fetch errors (an AccessDenied-style fault no longer spins for the whole RESULTS_TIMEOUT under a misleading timeout headline) and skipping a pending marker as not-final. 404s stay a healthy answer for legs, which publish only at the end; only the seeded relay key treats a persistent 404 as a fault. Hardening that fell out of making the polling long-running: - One last-chance fetch when the budget is exhausted, so a verdict published between job handoffs or during the final sleep is not lost. - The poll loop never sleeps past its window. - Shared env parsing (RequireEnvInts, requirePositive); zero or negative POLL_INTERVAL, WINDOW_SECONDS, RESULTS_TIMEOUT, or DEBUG_LOG_EVERY_POLLS now fail before any AWS call (the last was a divide-by-zero panic in gather). - VerdictOK/VerdictPending exported next to Result, decoupling the S3 verdict namespace from the relay-state namespace; the coordinator renders a still-pending result through the no-result fallback. - writeNoVerdictComment no longer emits found=false itself; each caller records its own outputs, keeping relay's contract state-only. Tests pin the state rule (only an exhausted budget turns a verdict-less window into a failure), env validation, and the new helpers. * bench(perf-eval): seed the leg result key, fail fast on every absent key The gha-rpc-ci role has no s3:ListBucket on the results bucket, so GetObject on a missing key returns 403 AccessDenied, not 404. Legs never seeded their key, so during a leg's whole run the gatherer read 403 on every poll — and the new consecutive-error fail-fast killed each leg after ten polls, minutes into an hour-long run. Extend the campaign seeding convention to legs: the leg job now writes the pending marker right after assuming the role (s3:PutObject on the bucket is already granted), so every poller key exists from the start. With that invariant uniform, the keySeeded special case is gone: a persistent 403 or 404 is a seeding or config fault everywhere, and the fail-fast is valid for legs and campaigns alike. Seeding also makes a re-run overwrite its predecessor's verdict at launch instead of at publish time. * pr-936: #1 test and bound shared result polling Use fake AWS transports and virtual time to cover polling states, retries, handoff, deadlines, cancellation, and output contracts. Bound result reads and SSM diagnostics, reject invalid protocol data and duration overflow, and count persistent stale reads as faults. Keep retry counts local to each Relay window. RESULTS_FILE defaults remain unchanged; diagnostics follow its directory for isolated tests. Addresses: #936 (comment) * pr-936: #2 describe missing final results accurately Use the same accurate fallback for an absent object and an existing pending seed. Addresses: #936 (comment) * bench(perf-eval): preserve older targets and partial reruns Seed only targets with the shared poller, because historical Gather treats pending as final failure. Keep shared result keys for failed-jobs-only reruns, while rejecting coordinator results from another run or target. Test seeding order and result gates without executing infrastructure steps. * bench(perf-eval): document and verify polling boundaries Cover cross-window retry reset, transient body failures, and cancellation in the final fetch. Document the seed requirement, diagnostics bound, and result states; add the changelog entry. * cleanup and refactor * address PR comments
) * bench(perf-eval): share one pollerConfig between Gather and Relay Gather and Relay read the same nine environment variables, build the same S3 and SSM clients, and construct the same resultPoller. One pollerConfig struct, parsed with caarlos0/env tags, now holds the shared variables; gatherConfig and relayConfig add only their own. newResultPoller builds the AWS clients and the poller for both commands. writeNoVerdictComment is a resultPoller method, so reportGather and relay.reportFault no longer carry the runner, instance ID, and debug line count as parameters. RequireEnv, RequireEnvInts, requirePositive, and requireSeconds are gone along with the time.Duration overflow guard. Positivity checks stay, and every configuration error names the variable. * bench(perf-eval): parse poller env values through typed fields The env library parses a plain int with a 32-bit range, which would have capped DEADLINE_EPOCH at 2038. The seconds, count, and unixTime types parse their variables as 64-bit integers, reject values below one, and keep seconds within a time.Duration, so positivity and the unit conversion live on the type instead of in per-struct validate methods. PollerConfig is exported and embedded so that the standard promoted field lookup can name the environment variable in parse errors. The overflow test cases are back, and tests now cover an unset variable and the aggregate error that names every missing variable. * bench(perf-eval): follow the #936 review rules in the poller config Drop the duration overflow check on seconds, as requested on #936. Read the poll target through a poller method instead of its fields, list PollerConfig in the package comment, and test newResultPoller. * address PR comments
| if err != nil { | ||
| return err | ||
| } | ||
| *c = count(n) |
* migrate initial signatures for views + use view walk in eventInfoForEvents * move events view matcher + v1 filter compiler from rpcv2 into shared/store * wire updated handler to shared matcher * adapt migration_test's GetEvents callback to the view scan signature * use rpcv1 views fns in rpcv2 * fix comments referencing old function names * restore parity with SDK on diagnostic-typed events * render eventInfo in view scanner callback fn * fix number of clauses referenced in comment * update test to account for sorted keys * use sql.RawBytes in getEvents to avoid needless event data copying * move event match test to store + add test * make error messages in shared events file backend neutral * fix stale comments * minor shared renderer optimizations * change handler to check length to see if match found * get event fields in one pass per level * fix scan function error handling * revert to contract/system event type fold * revert event match machinery relocation * error on an event type outside the enum instead of rendering an empty name * fix loan doc: only the event view is borrowed, txHash is a copy
cjonas9
added this pull request to stack #1030
September 18, 2026 22:08
cjonas9
removed this pull request from stack #1030
September 18, 2026 22:51
* remove redundant local newTestDB fn from getEvents test * add getEvents differential test suite * temporary testing: do a perf eval run * route the getEvents cursor differential's past-tip cases through assertSameError The assertSame/assertSameError split merged up from add-differential-test-suite made assertSame reject a reference-side error; the cursors one ledger past the corpus are exactly the error-parity case. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * remove branch push trigger --------- Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
feature/full-historyintomain.Test plan