Protocol 28 (CAP-0083)#196
Conversation
Re-pin go-stellar-sdk to the CAP-0083 XDR regeneration (SPIKE) and bump MaxSupportedProtocolVersion to 28.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e7c6b0ab5e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| // MaxSupportedProtocolVersion defines the maximum supported version of | ||
| // the Stellar protocol. | ||
| MaxSupportedProtocolVersion uint32 = 27 | ||
| MaxSupportedProtocolVersion uint32 = 28 |
There was a problem hiding this comment.
Bumping the default supported protocol to 28 makes integration.NewTest select protocol 28 whenever the env does not cap it lower (or when it is capped at 28), and after Horizon starts it always calls upgradeLimits(). That helper reads testdata/unlimited-config-v%d.xdr for the selected protocol, but the repo only has v26/v27 files, so any protocol-28 integration run with a 28-capable core fails on open testdata/unlimited-config-v28.xdr: no such file or directory before exercising ingestion.
Useful? React with 👍 / 👎.
Matches the reviewed head of stellar/go-stellar-sdk#5952.
Switch the integration matrix to protocol-version [27, 28]. P28 uses the vnext (next-protocol-enabled) core build 27.0.1-3348.ff61f2e6d. The P28 stellar-rpc image is left as a commented placeholder until stellar-rpc#789 publishes one, so that leg stays amber rather than testing against a P27 RPC. Repoint the cache source hash to the P28 (latest) + P27 (previous) images.
…ling the leg The P28 matrix leg had no PROTOCOL_28_STELLAR_RPC_DOCKER_IMG (no published P28-capable stellar-rpc image exists; stellar-rpc#789 is still a draft over protocol-next and the repo only publishes images from main/release/**). The "Pull and set Stellar RPC image" step ran `docker pull ""` -> invalid reference format -> the P28 legs went red and fail-fast cancelled the P27 legs. Leaving the image empty was not a safe "amber": the compose default (stellar/stellar-rpc) is a P27 RPC, so the soroban suites would have run against the wrong RPC. - Guard the RPC pull step so a missing prebuilt image is a no-op. - Build a genuine P28 stellar-rpc image in-CI from stellar-rpc#789 source, fetched by pinned SHA via refs/pull/789/head (there is no p28-cap-0083 branch on stellar/stellar-rpc; the head lives on a fork), plus the pinned P28 captive-core deb, and run all six EnableStellarRPC suites (sac, invokehostfunction, extend_footprint_ttl, txsub, txsub_async, transaction) against it on the P28 leg. - If that build is unavailable, skip those suites loudly (tracked to stellar-rpc#789) rather than silently falling back to a P27 RPC. - fail-fast: false so a new-protocol leg can't mask the previous one. Follow-up: once stellar-rpc#789 finalizes and a P28 image is published, pin PROTOCOL_28_STELLAR_RPC_DOCKER_IMG to it and drop the build step + skip.
CAP-0083 is consensus-layer (skip ledgers); the RPC layer is unaffected, so the P27 stellar-rpc image is functionally adequate for the P28 integration leg. Reverts the from-source-build pipeline added in a9fe4d7 in favor of the simple two-line "use previous protocol's image + TODO". Drops: - PROTOCOL_28_STELLAR_RPC_PR_SHA env var - "Build P28 stellar-rpc image from stellar-rpc#789" step - empty-IMG handling in "Pull and set Stellar RPC image" - PR_SHA reference in the source-hash key Restores the original "Pull and set Stellar RPC image" step. When a real P28 stellar-rpc image is published, replace the PROTOCOL_28_STELLAR_RPC_DOCKER_IMG value with the new tag and drop the TODO.
The P28 integration leg reads testdata/unlimited-config-v28.xdr in upgradeLimits() (internal/test/integration/integration.go:787); it was missed in the P28 bump, so every P28 test going through StartHorizon setup failed with "open testdata/unlimited-config-v28.xdr: no such file or directory". CAP-0083 adds no Soroban config settings, so the v28 ConfigUpgradeSet is byte-identical to v27 under the P28 go-stellar-sdk pin (v26 and v27 are already byte-identical). The protocol matrix is [27, 28], so v26 is now unreferenced; drop it to keep testdata aligned with the matrix, matching the rolling-window precedent from stellar#171/stellar#189.
go-stellar-sdk#5952 (Protocol 28 / CAP-0083) merged into protocol-next at 26fec7c0e891cd88227494e212ecf712e873f039. Drop the sisuresh fork replace directive and pin the require to the upstream merged commit, superseding the review-head pin (4615d5e81367a2e7d42a87c856226d3bc36a3554). The two commits share the same git tree, so no XDR/content change is expected; build and `go mod tidy` verified.
341401c reused the P27 RPC image (stellar/stellar-rpc:27.0.0-preview-190) for the P28 integration leg. That image bundles a P27 captive-core that cannot ingest a protocol-28 network, so every EnableStellarRPC test (TestContractMintToAccount, sac/*, invokehostfunction/*, ...) blocks in waitForStellarRPC() reporting "data stores are not initialized: DB is empty" until the 75m suite timeout panics the entire run. NewTest() already AMBER-skips RPC-dependent tests when HORIZON_INTEGRATION_TESTS_STELLAR_RPC_DOCKER_IMG is empty (tracked: stellar-rpc#789). Restore that path for P28: - set PROTOCOL_28_STELLAR_RPC_DOCKER_IMG empty (no P28 RPC image is published yet; building from source was intentionally dropped), and - restore the empty-image guard in "Pull and set Stellar RPC image" so `docker pull ""` is not attempted. P27 keeps its published RPC image and full Soroban coverage. Re-pin the P28 image to a real tag once stellar-rpc#789 publishes one.
Build 3348 (ff61f2e6d) includes stellar-core#5325 which moved bucket directory creation out of initialize() and into newDB() only. That breaks `stellar-core offline-info` against a fresh captive-core storage dir: BucketManager has no on-disk bucket dir to read and the process exits 1, after which horizon's ingestion state machine falls back to buildFromCheckpoint(checkpointLedger=0) and fails. Pinning back to 3312 (02fa3131b) sidesteps it while the upstream fix is in flight.
…gression" This reverts commit bd94846.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: daf546008d
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| t.Skip("skipping integration test: HORIZON_INTEGRATION_TESTS_ENABLED not set") | ||
| } | ||
|
|
||
| if config.EnableStellarRPC && os.Getenv("HORIZON_INTEGRATION_TESTS_STELLAR_RPC_DOCKER_IMG") == "" { |
There was a problem hiding this comment.
This skip runs before config.ProtocolVersion is defaulted or capped from HORIZON_INTEGRATION_TESTS_CORE_MAX_SUPPORTED_PROTOCOL, so any run with EnableStellarRPC and no RPC image env is skipped even when the effective protocol is a supported non-P28 value, such as an explicit ProtocolVersion: 20/27 or a local run capped at 27. I checked docker/docker-compose.integration-tests.yml, and it still has a default STELLAR_RPC_IMAGE:-stellar/stellar-rpc, so this removes existing RPC test coverage outside the intended P28-no-image case; resolve the effective protocol first and only amber-skip the unsupported protocol leg.
Useful? React with 👍 / 👎.
| id: calculate_source_hash | ||
| run: | | ||
| combined_hash=$(echo "horizon-hash-${{ hashFiles('./') }}-${{ hashFiles('./docker/**') }}-${{ hashFiles('./internal/**') }}-protocol-${{ matrix.protocol-version }}-${{ env.PROTOCOL_27_CORE_DOCKER_IMG }}-${{ env.PROTOCOL_27_STELLAR_RPC_DOCKER_IMG }}-${{ env.PROTOCOL_27_CORE_DEBIAN_PKG_VERSION }}-${{ env.PROTOCOL_26_CORE_DOCKER_IMG }}-${{ env.PREFIX }}" | sha256sum | cut -d ' ' -f 1) | ||
| combined_hash=$(echo "horizon-hash-${{ hashFiles('./') }}-${{ hashFiles('./docker/**') }}-${{ hashFiles('./internal/**') }}-protocol-${{ matrix.protocol-version }}-${{ env.PROTOCOL_28_CORE_DOCKER_IMG }}-${{ env.PROTOCOL_28_STELLAR_RPC_DOCKER_IMG }}-${{ env.PROTOCOL_28_CORE_DEBIAN_PKG_VERSION }}-${{ env.PROTOCOL_27_CORE_DOCKER_IMG }}-${{ env.PREFIX }}" | sha256sum | cut -d ' ' -f 1) |
There was a problem hiding this comment.
For the protocol-27 matrix leg, the steps above still pull PROTOCOL_27_STELLAR_RPC_DOCKER_IMG and install PROTOCOL_27_CORE_DEBIAN_PKG_VERSION, but this cache key no longer includes either value. If the 27 core package or RPC image is repinned without other source changes, actions/cache can hit the previous key and skip go test, leaving the 27 integration suite unrun against the new dependency.
Useful? React with 👍 / 👎.
stellar-core#5329 (merged at 3589a696b) fixes the QueryServer.cpp:124
contiguous-snapshot assertion that fires when captive-core boots fresh
(new-db + catchup) — the bug was introduced by 87f27b30d ('Move
historical ledger management to query server') in the 3312..3348 vnext
window. The new build 3367 is the first vnext deb that contains the
fix.
Docker validator image stays at 3348 vnext since no vnext docker for
3367 is published yet. The bug fires in captive-core, not in the
validator container (which only closes ledgers contiguously).
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 837b19a725
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| // MaxSupportedProtocolVersion defines the maximum supported version of | ||
| // the Stellar protocol. | ||
| MaxSupportedProtocolVersion uint32 = 27 | ||
| MaxSupportedProtocolVersion uint32 = 28 |
There was a problem hiding this comment.
Raising Horizon's supported protocol here also affects the stellar/horizon-verify-range image, but I checked the verify-range workflow and Docker scripts and .github/workflows/horizon.yml still builds that image with STELLAR_CORE_VERSION=27.0.0-3288.7696c069d.noble, which docker/verify-range/dependencies installs as /usr/bin/stellar-core for captive-core verification. Once a verify-range run crosses protocol-28 ledgers, that pushed image will not be able to replay them even though this Horizon build advertises P28 support; the workflow smoke test only verifies an early historical range, so it will not catch the mismatch.
Useful? React with 👍 / 👎.
The plain jammy-vnext docker image for build 3367 was published a few hours after the buildtests/perftests variants, so docker and deb pins can now both reference the same QueryServer-fix build.
There was a problem hiding this comment.
💡 Codex Review
Keep the protocol 26 unlimited-config fixture
Adding the v28 fixture by renaming the v26 file leaves no testdata/unlimited-config-v26.xdr, but NewTest can still select protocol 26 when HORIZON_INTEGRATION_TESTS_CORE_MAX_SUPPORTED_PROTOCOL=26 or a caller passes ProtocolVersion: 26. In that scenario upgradeLimits() runs for protocols >=22 and reads unlimited-config-v%d.xdr, so the p26-capped integration suite now fails on open testdata/unlimited-config-v26.xdr before exercising ingestion; please copy the fixture for v28 instead of removing v26.
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
1 participant
SPIKE — depends on the go-stellar-sdk CAP-0083 XDR regen (#5952). Stays draft until that merges.
Changes
MaxSupportedProtocolVersionto 28 (internal/ingest/main.go).go-stellar-sdkto the #5952 review head4615d5e8viareplace(go.mod/go.sum).[27, 28]; P28 runs against the vnext core build27.0.1-3348.ff61f2e6d(apt…jammy~vnext~buildtests), and repoint the cache source hash to P28+P27.Deferred
stellar-rpcimage: commented placeholder until stellar-rpc#789 publishes one, so the P28 leg stays amber rather than testing against a P27 RPC.replace→ tagged SDK release once #5952 merges, then mark ready.Upstream: stellar/go-stellar-sdk#5952
Downstream: stellar/stellar-rpc#789