extract: update go-stellar-sdk to v0.7.1 - #1
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request upgrades github.com/stellar/go-stellar-sdk to v0.7.1 and updates extractors/tests/CI to correctly handle Protocol 28’s newly-added XDR union arms (plus a Protocol 23-era Soroban rent fee meta version gap), preventing silent column corruption on future protocol bumps.
Changes:
- Update extraction logic for Protocol 28 union arms (
SCV_EXECUTABLE_TAG,CONTRACT_EXECUTABLE_EXTERNAL_REF,STELLAR_VALUE_EMPTY_TX_SET) and fix Sorobanrent_fee_chargedparsing acrossTransactionMetaV3/V4. - Add protocol exhaustiveness guards (
protocol_coverage_test.go) so unhandled discriminants fail CI. - Strengthen repo process/docs: CI gates (gofmt/tidy/vet/race), Dependabot scheduling, and an SDK upgrade checklist.
Reviewed changes
Copilot reviewed 12 out of 16 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| types_soroban.go | Extends ContractCreationData to explicitly record contract executable kind + external-ref metadata. |
| types_core.go | gofmt-only alignment changes to core row structs. |
| types_accounts.go | gofmt-only alignment changes to account snapshot struct. |
| accounts.go | gofmt-only alignment changes in extractAccountData literal. |
| transactions.go | Fixes Soroban rent fee extraction by reading Soroban meta ext from TransactionMeta V3 or V4. |
| soroban.go | Records contract executable explicitly via applyExecutable, including Protocol 28 external-ref details. |
| scval_converter.go | Adds handling for SCV_EXECUTABLE_TAG in ConvertScValToJSON. |
| ledgers.go | Reads validator signature for new STELLAR_VALUE_EMPTY_TX_SET via scpValueSignature. |
| protocol_coverage_test.go | New exhaustiveness/behavior tests intended to fail on future unhandled discriminants/meta versions. |
| go.mod | Bumps go-stellar-sdk to v0.7.1. |
| go.sum | Updates checksums for the new SDK version. |
| README.md | Updates SDK pin mention and links upgrade doc; documents “protocol changes must fail loudly”. |
| docs/SDK_UPGRADES.md | New documented process/checklist for safe SDK upgrades and union-arm auditing. |
| docs/handoffs/2026-08-04-sdk-0.7.1-upgrade.md | New handoff describing the upgrade details, rationale, and follow-ups. |
| .github/workflows/ci.yml | Adds gofmt/tidy drift gates, runs tests with -race, tracks Go via go.mod, adds SDK pin warning job. |
| .github/dependabot.yml | Adds weekly gomod dependency checks grouped for github.com/stellar/*. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+20
to
+30
| // validDiscriminants returns every discriminant value the SDK accepts for an | ||
| // enum, discovered through the generated ValidEnum method. | ||
| func validDiscriminants(e interface{ ValidEnum(int32) bool }) []int32 { | ||
| var valid []int32 | ||
| for v := int32(0); v < 256; v++ { | ||
| if e.ValidEnum(v) { | ||
| valid = append(valid, v) | ||
| } | ||
| } | ||
| return valid | ||
| } |
PR review flagged that validDiscriminants scanned only [0, 256), so a future enum arm at or above 256 would be missed silently -- the exact failure mode the guards exist to prevent. Checking the bound against the SDK showed both ends were already wrong, and the lower one was the bigger miss. Discriminants across xdr_generated.go span [-18, 2000]: the result-code enums run negative (transactionResultCode reaches -18), CryptoKeyType places KEY_TYPE_MUXED_ED25519 at 0x100, and SCSpecType uses a 1000-2000 band. Measured against v0.7.1, the old window found 4 of 5 CryptoKeyType arms -- missing the muxed key type that transactions.go already switches on in getMuxedAddress -- and 2 of 20 TransactionResultCode arms. Widen the window to [-65536, 65536] and state the bound in the doc comment instead of claiming every discriminant is covered. Add TestDiscriminantScanWindowHasHeadroom, which fails when a guarded enum comes within 1024 of an edge, so the bound is monitored rather than assumed. It includes CryptoKeyType even though no other guard uses it: its 0x100 is why the window cannot stop at 255, and asserting it pins that reason to a failing test rather than a comment. Guard runtime is ~15ms for the full suite. Co-Authored-By: Claude Opus 5 (1M context) <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.
This updates
go-stellar-sdkfrom v0.6.0 to v0.7.1 and adds the extraction changes required for Protocol 28.The dependency bump itself compiles cleanly, but Protocol 28 added three XDR union arms that existing switches silently ignored. Without the corresponding extractor changes, executable tags become error objects, external-reference contract creations look like SAC deployments, and empty-tx-set ledgers lose their validator signature.
The audit also found a pre-existing Protocol 23 bug: transaction rent fees were read from
TransactionMetaV3 only, even though current ledgers carry Soroban meta in V4. This leftrent_fee_chargedNULL for Soroban transactions after that upgrade.What changed
SCV_EXECUTABLE_TAG,CONTRACT_EXECUTABLE_EXTERNAL_REF, andSTELLAR_VALUE_EMPTY_TX_SET.The
ContractCreationDatafields are additive, but downstream writers need schema updates before they persist external-reference metadata. Existingtransactions_row_v2data from Protocol 23 onward also needs a backfill ifrent_fee_chargedis required.Testing
CGO_ENABLED=0 go test ./...CGO_ENABLED=0 go vet ./...go mod tidywith no diffgofmt -l .with no outputThe race-enabled suite is also required by CI; the local environment does not have a C compiler available for
-race.