Skip to content

Experiment with ifdef-guarded changes to xdr#5898

Closed
leighmcculloch wants to merge 8 commits intomainfrom
xdrifdef
Closed

Experiment with ifdef-guarded changes to xdr#5898
leighmcculloch wants to merge 8 commits intomainfrom
xdrifdef

Conversation

@leighmcculloch
Copy link
Copy Markdown
Member

@leighmcculloch leighmcculloch commented Feb 11, 2026

What

Experiment with ifdef-guarded changes to xdr.

Why

Close https://github.com/stellar/pod-fsr/issues/13

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR experiments with ifdef-guarded changes to XDR definitions by introducing conditional compilation support for a new SCV_SPARSE_MAP type variant. The implementation uses Go build tags to generate two versions of the ScVal type definitions that can be selected at compile time.

Changes:

  • Added SPARSE_MAP ifdef guards to the Stellar-contract.x XDR schema to conditionally include SCV_SPARSE_MAP = 22 enum value and corresponding union case
  • Generated two separate Go files with build tags: xdr_generated_sparse_map.go (with sparse_map tag) and xdr_generated_no_sparse_map.go (with !sparse_map tag)
  • Removed ScValType enum and ScVal union definitions from the main xdr_generated.go file (now defined only in the build-tagged files)
  • Updated Makefile to format all xdr_generated*.go files and fixed a spelling error in a comment
  • Updated XDR and XDRGEN commit references

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
xdr/Stellar-contract.x Added ifdef guards around SCV_SPARSE_MAP enum value and union case
xdr/xdr_generated_sparse_map.go New generated file with sparse_map build tag including SCV_SPARSE_MAP support
xdr/xdr_generated_no_sparse_map.go New generated file with !sparse_map build tag excluding SCV_SPARSE_MAP support
xdr/xdr_generated.go Removed ScValType and ScVal definitions (now in build-tagged files), updated XDR hash
xdr/xdr_commit_generated.txt Updated XDR commit reference
Makefile Updated to format all xdr_generated*.go files and fixed comment spelling

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +72 to +74
// SCV_LEDGER_KEY_NONCE = 21,
// SCV_SPARSE_MAP = 22
//
Copy link

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The XDR comment in the generated code contains duplicated and conflicting lines. Lines 72-75 show:

SCV_LEDGER_KEY_NONCE = 21,
SCV_SPARSE_MAP = 22

SCV_LEDGER_KEY_NONCE = 21

This appears to be a malformed representation of the ifdef-guarded XDR definition. The comment should accurately reflect the conditional compilation structure from the source XDR file.

Suggested change
// SCV_LEDGER_KEY_NONCE = 21,
// SCV_SPARSE_MAP = 22
//

Copilot uses AI. Check for mistakes.
Comment on lines +261 to +288
//
// case SCV_SPARSE_MAP:
// SCMap *sparseMap;
//
// };
type ScVal struct {
Type ScValType
B *bool
Error *ScError
U32 *Uint32
I32 *Int32
U64 *Uint64
I64 *Int64
Timepoint *TimePoint
Duration *Duration
U128 *UInt128Parts
I128 *Int128Parts
U256 *UInt256Parts
I256 *Int256Parts
Bytes *ScBytes
Str *ScString
Sym *ScSymbol
Vec **ScVec
Map **ScMap
Address *ScAddress
Instance *ScContractInstance
NonceKey *ScNonceKey
}
Copy link

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The XDR comment documentation in the !sparse_map build includes references to the SCV_SPARSE_MAP case (lines 262-263), but the actual ScVal struct (line 288) does not include the corresponding SparseMap field. This creates a discrepancy between the documentation and the actual implementation. The comment should be consistent with the build configuration - either the SCV_SPARSE_MAP case should be removed from the comment, or clearly marked as not available in this build.

Copilot uses AI. Check for mistakes.
Comment on lines +71 to +76
//
// SCV_LEDGER_KEY_NONCE = 21,
// SCV_SPARSE_MAP = 22
//
// SCV_LEDGER_KEY_NONCE = 21
//
Copy link

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The XDR comment in the generated code contains duplicated and conflicting lines. Lines 72-75 show:

SCV_LEDGER_KEY_NONCE = 21,
SCV_SPARSE_MAP = 22

SCV_LEDGER_KEY_NONCE = 21

This appears to be a malformed representation of the ifdef-guarded XDR definition. The comment should accurately reflect the conditional compilation structure from the source XDR file.

Suggested change
//
// SCV_LEDGER_KEY_NONCE = 21,
// SCV_SPARSE_MAP = 22
//
// SCV_LEDGER_KEY_NONCE = 21
//
// SCV_LEDGER_KEY_NONCE = 21,
// SCV_SPARSE_MAP = 22
//

Copilot uses AI. Check for mistakes.
@leighmcculloch leighmcculloch deleted the xdrifdef branch March 5, 2026 05:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants