Add Captive Core block fetcher#9
Open
GabrielCartier wants to merge 28 commits into
Open
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a new block-fetch backend that uses an embedded Captive Stellar Core subprocess, while also correcting RPC fetcher hash handling and generalizing network selection to support non-(mainnet|testnet) passphrases.
Changes:
- Introduces
fetch captive-corecommand (and dev script) to stream ledgers vialedgerbackend.CaptiveStellarCore. - Fixes RPC fetcher hash decoding (ledger hash is hex, removes incorrect previous-hash decoding) and switches bstream block IDs to hex for filesystem-safe one-block filenames.
- Refactors RPC fetcher initialization from
isMainnet boolto an explicitnetworkPassphrase string, updating call sites and tests.
Reviewed changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| rpc/init_test.go | Adds helper to map test RPC endpoint → network passphrase. |
| rpc/fetcher.go | Fixes ledger hash decoding and switches block ID encoding to hex; uses passphrase string for tx extraction. |
| rpc/fetcher_test.go | Updates tests to construct the fetcher with the correct network passphrase. |
| devel/poller.sh | Updates dev polling start block and fetch args. |
| devel/captive-core.sh | Adds dev script to run reader-node using the captive-core fetcher. |
| cmd/firestellar/tool_compare_fetcher_blocks.go | Fixes compare tool to honor --network by selecting the right passphrase. |
| cmd/firestellar/main.go | Registers new fetch rpc and fetch captive-core subcommands. |
| cmd/firestellar/fetcher.go | Renames/updates RPC fetch command; adds passphrase resolution logic and deprecates --is-mainnet. |
| cmd/firestellar/fetcher_captive_core.go | Implements captive-core fetch loop and block conversion to bstream blocks. |
| .gitignore | Ignores runtime data directories created by dev scripts. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 10 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
cmd/firestellar/fetcher.go:30
- The
--state-dirflag help text is incorrect (currently says "interval between fetch"), which can confuse operators and tooling. Update the description to reflect that this directory is used for poller state/checkpoint storage.
cmd.Flags().StringArray("endpoints", []string{}, "List of endpoints to use to fetch different method calls")
cmd.Flags().String("state-dir", "/data/poller", "interval between fetch")
cmd.Flags().Duration("interval-between-fetch", 0, "interval between fetch")
cmd.Flags().Duration("latest-block-retry-interval", time.Second, "interval between fetch")
- stellar/client.go: route FundAccount through configured httpClient (timeout); SubmitOpsExpectFail now propagates non-horizon errors instead of swallowing all errors. - stellar/ops.go: ChangeTrust takes txnbuild.CreditAsset (compile-time guard) instead of asserting on txnbuild.Asset at runtime. - xdr/decode.go: corrected doc on ContractId normalization (encoded as C... strkey, not placeholder). - devstack/devstack.go: clarified DataRoot default points at <testRoot>/.data.
InProcessRPCFetcher.logger / .networkPassphrase and InProcessCaptiveCoreFetcher.logger were stored but never read.
- stellar/client.go: drop extra slash in FundAccount URL (/friendbot?addr=) - Dockerfile-rpc-fetcher: bump build image to golang:1.26 (matches go.mod) - README + doc comments: correct paths (test/scenarios, test/scripts) and Go version (1.26+)
- tool_compare_merged_blocks: nil-check stellar Header before recovering PreviousLedgerHash - devstack: rename misnamed repoRoot field to testRoot (actually <repo>/test/) - xdr/decode: correct doc on what drives per-run drift (deterministic keypairs via AccountSeedScope) - scenarios/main_test: drop dead '_ = exitCode' from defer
- fix-block-hashes: switch stopBlock comparisons to >= so the range stays exclusive on the upper bound, matching the merged-block compare tool. - tool-compare-merged-blocks: wire logger.Debug per-bundle and drop the unused `_ = logger` placeholder. - xdr/decode.go: clarify normalizeDynamicFields determinism comment; SAC IDs and addresses are stable when runner.Config.AccountSeedScope is set (the default in the suite). - README: split fetcher docs into RPC vs captive-core sections; document --state-dir / --ignore-cursor resume semantics shared across both. - .gitignore: cover the data-cc-mainnet runtime dir produced by the captive-core dev script.
JSON unmarshal into any/map[string]any coerces all numbers to float64,
losing precision above 2^53. Stellar amounts use stroops (1 XLM = 1e7)
and total supply ~5e17 stroops sits well above that threshold, and
sequence numbers can grow into the same range — so snapshot diffs and
cross-backend equality checks could silently miss real differences.
Switch all four JSON round-trips to json.Decoder.UseNumber():
- xdr/decode.go: roundtrip (driver for all event decode paths).
- snapshot/snapshot.go: Snapshot.normalize (actual side) and Load
(expected side, so DeepEqual compares like-to-like json.Number).
- snapshot/diff.go: toGeneric (used by DiffViews for backend diffs).
Also stop swallowing roundtrip errors in decode{Diagnostic,Tx,Contract}Events;
surface them via the same {_decodeError: ...} sentinel used for
UnmarshalBinary failures so SDK shape regressions fail loud instead of
dropping fields silently.
…visory) SDF disclosed a critical network vulnerability in stellar-core; the fix lives in 26.1.0-3210.427aa3978. Pre-fix builds must not ship. - Dockerfile: post-install dpkg --compare-versions check fails the build when stellar-core is older than STELLAR_CORE_MIN_VERSION (26.1.0 default). The apt install line still pulls from SDF's stable channel, which now ships the patched build. - test/scripts/dev/docker-compose.yml: pull_policy: always on stellar/quickstart so each devstack up grabs the latest image (the testing tag floats forward and now bundles the patched stellar-core). Override via QUICKSTART_PULL_POLICY=missing for air-gapped runs. - test/README.md: bump documented minimum stellar-core version and point operators at brew/apt upgrade commands.
GabrielCartier
force-pushed
the
feature/captive-core
branch
from
627389f to
a9e9db2
Compare
The roundtrip()/Load()/toGeneric() change to UseNumber means real decoded payloads now carry json.Number where they previously held float64. The dynamic-field / strkey / account-id normalizers were still type-switching on float64, so SeqNum/BumpTo/OfferId placeholders and 32-byte strkey collapse silently no-op'd on real input, drifting snapshots away from their expected shape. - Add numericByte() and isNumericValue() helpers that accept both json.Number and float64 (the latter kept for hand-built fixtures and defensive coding). - Rewrite dynamicFieldPlaceholder, isThirtyTwoByteArray, tryEncodeStrkey, and the Type/byte-array assertions in tryEncodeAccountID to use the helpers. - Update decode_test.go fixtures to use json.Number to match the actual runtime representation; jsonNumberSlice now lives up to its name. The SeqNum case uses 9007199254740993 (2^53 + 1) so the test would have failed under the old float64-coerced path — locking in the precision fix.
tool-compare-merged-blocks only iterated refStore.Walk, so any bundle that lived exclusively in the current store was never visited. The tool reported a clean diff even when the current side had extra blocks (or an entire range main was missing). Run a second walk over curStore after the reference pass, skipping filenames already visited, and report each current-only block as missing-in-reference. Honors stop-on-first-diff and shares the existing stopErr sentinel. go.mod: the rebase onto main inadvertently kept go-stellar-sdk v0.4.0 from the feature branch instead of main's v0.5.0 (dependabot bump in PR #3). Restore v0.5.0 and update the inline comment to reflect that this version tracks main.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
maoueh
reviewed
May 14, 2026
| uses: actions/setup-go@v4 | ||
| with: | ||
| go-version: ${{ matrix.go-version }} | ||
| go-version: 1.26.x |
Contributor
There was a problem hiding this comment.
There is a config to switch to pull from go.mod defined version, I think it's a better.
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.
Adds a second fetch backend (
fetch captive-core) that streams Stellar blocks via an embedded Captive Core subprocess, as an alternative to the existing RPC fetcher. Also folds in a one-shot fix tool to rewrite v1-fetcher merged blocks and a battlefield-style integration test harness that exercises both backends.What changed
New:
fetch captive-corecommand (cmd/firestellar/fetcher_captive_core.go)stellar-corein-process via the Stellar SDK'sledgerbackend.CaptiveStellarCoremainnet,testnet, andcustomnetworks via--stellar-core-network--stellar-core-network-passphraseand--stellar-core-history-archive-urls; passphrase/archive flags also override the defaults for mainnet/testnet--stellar-core-confpath{state-dir}/cursor.jsonafter each successful emission; restarts resume atlast_fired_block + 1instead of replaying from<first-streamable-block>. Pass--ignore-cursorto force a fresh start (e.g. when running under a supervisor that already tracks downstream state).devel/captive-core.shFix: hash encoding in RPC fetcher (
rpc/fetcher.go)ledger[0].Hashfrom stellar-rpc is hex, not base64 — switched tohex.DecodeStringPreviousLedgerHashis already raw bytes on the XDR struct — removed the incorrectbase64.DecodeString(HexString())round-trip, now uses[:]directly/characters acting as path separatorsRefactor:
isMainnet bool→networkPassphrase stringFetchernow holds the full passphrase string instead of a boolean flag, allowing the RPC fetcher to also work against arbitrary networksNew:
fix-block-hashestool (cmd/tools/fix/tools_fix_block_hashes.go)base64.Decode(hex_string)as the ledger hash, which is a reversible identity round-trip on 64-char base64-alphabet input.ConvertBrokenHashre-encodes the garbage bytes back to base64 (yielding the original hex string) and hex-decodes that to the original 32-byte hash — no external source needed.pbbstream.Block.Id/ParentId(which were accidentally already correct hex thanks to the same round-trip identity); bails on any mismatch.New:
tool-compare-merged-blocks(cmd/firestellar/tool_compare_merged_blocks.go)--sanitize-reference/--sanitize-currentapplies the sameConvertBrokenHashrecovery asfix-block-hashesto either side before comparing, so legacy v1 stores can be diffed against correctly-hashed v2 stores.New: battlefield-style integration test harness (
test/)test/lib/devstack/— launches a local Stellar devstack (validator + follower via docker-compose undertest/scripts/dev/)test/lib/firehose/— in-process fetchers for both backends (inprocess_rpc.go,inprocess_cc.go) plus a sharedFetcherinterfacetest/lib/snapshot/— golden-file snapshot diffing againsttest/snapshots/**/*.expected.jsontest/lib/stellar/+test/lib/xdr/— Horizon client helpers + XDR decoders used by the scenariostest/scenarios/— payment / account / asset / offer / multisig / soroban / fee-bump scenarios driving both backends and asserting equivalent outputtest/README.mddocuments how to bring up the stack and run the suiteTooling note — go-json-experiment pin downgrade
go.modpinsgithub.com/go-json-experiment/jsonto the Oct 2023 snapshot to match firehose-core's internalfirehose-core/json/marshallers.go(old API namesNewMarshalers/MarshalFuncV2)JoinMarshalers/MarshalToFunc;cmd/firestellar/tool_decode_block.gowas using the new names and has been reverted to the old ones to compile against the downgraded depgo.modandtool_decode_block.goflag this for a flip-back once firehose-core upstream bumps its own pinNotable side effects
rpc.NewFetchersignature change: callers must passnetworkPassphrase stringinstead ofisMainnet boolfix-block-hashesagainst the historical range before swapping the reader over to the v2 captive-core pathTest plan
go build ./...cleango vet ./...cleanfetch captive-coreagainst mainnet + testnet streams blocks end-to-endfix-block-hashesrewrites a sample range and the resulting block hashes matchfetch captive-coreoutput for the same ledgerstest/scenarios/...pass against both RPC and captive-core in-process fetchers via the devstack