Experiment with ifdef-guarded changes to xdr#910
Experiment with ifdef-guarded changes to xdr#910leighmcculloch wants to merge 3 commits intomainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR modifies the Stellar Quickstart Docker image configuration to experiment with ifdef-guarded XDR changes for transaction metadata v5. However, it makes these changes by modifying the stable "latest" image configuration rather than adding a new experimental configuration entry.
Changes:
- Changed the image tag from "latest" to "xdrifdef"
- Updated stellar-core reference from stable version tag v25.1.1 to commit hash 2e45f3df2b07f8b53a02178c0fe57f3652199535
- Added experimental configure flag "--xdr-transaction-meta-v5" to stellar-core build options
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -1,6 +1,6 @@ | |||
| [ | |||
| { | |||
There was a problem hiding this comment.
The tag name has been changed from "latest" to "xdrifdef". According to the README (lines 68-72), "latest" is documented as "stable software, compatible with mainnet" and is one of the main published DockerHub tags. Changing this tag name would break compatibility for users depending on the "latest" tag.
For an experimental feature, consider either:
- Adding a new image configuration entry in images.json instead of modifying the existing "latest" entry
- Using a clearly experimental tag name and keeping the "latest" configuration intact
| { | |
| { | |
| "tag": "latest", | |
| "events": [ | |
| "pull_request", | |
| "push" | |
| ], | |
| "config": { | |
| "protocol_version_default": 25 | |
| }, | |
| "deps": [ | |
| { | |
| "name": "xdr", | |
| "repo": "stellar/rs-stellar-xdr", | |
| "ref": "v25.0.0" | |
| }, | |
| { | |
| "name": "core", | |
| "repo": "stellar/stellar-core", | |
| "ref": "2e45f3df2b07f8b53a02178c0fe57f3652199535", | |
| "options": { | |
| "configure_flags": "--disable-tests --xdr-transaction-meta-v5" | |
| } | |
| }, | |
| { | |
| "name": "rpc", | |
| "repo": "stellar/stellar-rpc", | |
| "ref": "v25.0.1" | |
| }, | |
| { | |
| "name": "horizon", | |
| "repo": "stellar/stellar-horizon", | |
| "ref": "v25.0.1", | |
| "options": { | |
| "pkg": "github.com/stellar/stellar-horizon" | |
| } | |
| }, | |
| { | |
| "name": "friendbot", | |
| "repo": "stellar/friendbot", | |
| "ref": "4a6a1715d5b81c640dd9f887dd37b26499e95e72", | |
| "options": { | |
| "pkg": "github.com/stellar/friendbot" | |
| } | |
| }, | |
| { | |
| "name": "lab", | |
| "repo": "stellar/laboratory", | |
| "ref": "main" | |
| }, | |
| { | |
| "name": "galexie", | |
| "repo": "stellar/stellar-galexie", | |
| "ref": "galexie-v25.1.0" | |
| } | |
| ], | |
| "tests": { | |
| "continue-on-error": false, | |
| "additional-tests": [ | |
| { | |
| "arch": "amd64", | |
| "network": "pubnet", | |
| "enable": "core,rpc,horizon" | |
| } | |
| ] | |
| } | |
| }, | |
| { |
| "name": "core", | ||
| "repo": "stellar/stellar-core", | ||
| "ref": "v25.1.1", | ||
| "ref": "2e45f3df2b07f8b53a02178c0fe57f3652199535", |
There was a problem hiding this comment.
The stellar-core reference has been changed from a stable version tag "v25.1.1" to a commit hash "2e45f3df2b07f8b53a02178c0fe57f3652199535". While using commit hashes is acceptable for experimental configurations (as seen in the "future" tag at line 166), this change is being applied to what was the "latest" tag, which is documented as containing "stable software, compatible with mainnet".
If this is meant to be an experimental build, ensure this configuration is in a separate experimental tag entry rather than replacing the stable "latest" configuration.
| "ref": "2e45f3df2b07f8b53a02178c0fe57f3652199535", | |
| "ref": "v25.1.1", |
images.json
Outdated
| "ref": "2e45f3df2b07f8b53a02178c0fe57f3652199535", | ||
| "options": { | ||
| "configure_flags": "--disable-tests" | ||
| "configure_flags": "--disable-tests --xdr-transaction-meta-v5" |
There was a problem hiding this comment.
The configure flag "--xdr-transaction-meta-v5" has been added for this experimental build. This appears to be an ifdef-guarded feature flag (as mentioned in the PR description). However, this change is being applied to what was the "latest" tag configuration.
Since this is an experimental feature, this configuration should be in a separate experimental image entry (similar to how "nightly" and "nightly-next" are separate entries) rather than modifying the stable "latest" configuration.
images.json
Outdated
| [ | ||
| { | ||
| "tag": "latest", | ||
| "tag": "xdrifdef", |
There was a problem hiding this comment.
The tag name "xdrifdef" is unclear and doesn't follow the naming conventions used for other experimental tags in this file. Other experimental/developmental tags use more descriptive names:
- "testing" (line 69) - for release candidates
- "future" (line 140) - for future network compatible builds
- "nightly" (line 201) - for nightly builds
- "nightly-next" (line 261) - for next protocol nightlies
Consider using a more descriptive name like "xdr-meta-v5-experiment" or "experimental-transaction-meta-v5" that clearly indicates this is an experimental build and what feature is being tested.
| "tag": "xdrifdef", | |
| "tag": "experimental-transaction-meta-v5", |
What
Experiment with ifdef-guarded changes to xdr
Why
For: