Skip to content

Document ToXdr and FromXdr traits#1767

Merged
leighmcculloch merged 2 commits intomainfrom
improve-xdr-trait-docs
Mar 17, 2026
Merged

Document ToXdr and FromXdr traits#1767
leighmcculloch merged 2 commits intomainfrom
improve-xdr-trait-docs

Conversation

@leighmcculloch
Copy link
Copy Markdown
Member

What

Added documentation to the ToXdr and FromXdr traits explaining the two-step serialization process: values are first converted to a Val, then serialized to XDR in their ScVal form. Added Errors and Panics sections to FromXdr and from_xdr, doc examples on both traits, and doc comments on the to_xdr, from_xdr methods and the Error associated type.

Why

FromXdr::from_xdr panics if the provided bytes are not valid XDR for an ScVal, but returns an error if the Val cannot be converted into the target type. This distinction was undocumented. The panic originates from the host side, making it non-obvious even when reading the implementation.

Close #1766

@leighmcculloch leighmcculloch requested a review from a team March 17, 2026 10:30
@leighmcculloch leighmcculloch marked this pull request as ready for review March 17, 2026 10:31
Copilot AI review requested due to automatic review settings March 17, 2026 10:31
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

Improves the Soroban SDK’s XDR conversion API documentation by expanding the rustdoc for ToXdr and FromXdr, clarifying how values are serialized/deserialized via Val/ScVal and what error/panic behavior to expect.

Changes:

  • Clarified ToXdr semantics (conversion to Val then serialization as ScVal XDR) and added a short usage example.
  • Clarified FromXdr semantics (deserialize ScVal XDR to Val, then convert) and documented error/panic behavior with an example.

@mootz12
Copy link
Copy Markdown
Contributor

mootz12 commented Mar 17, 2026

Approving - but it might be worth exploring having from_xdr surface the env::Error thrown by deserialize_from_bytes instead of unwrap_infallible(). I don't really have a use case in mind here, but this might make it simpler to take in raw XDR in contracts.

Leaving as an extra comment to get your thoughts.

@leighmcculloch
Copy link
Copy Markdown
Member Author

it might be worth exploring having from_xdr surface the env::Error thrown by deserialize_from_bytes instead of unwrap_infallible()

Yeah, but that's a protocol change, so not going to do that here. Take that to a GitHub Discussion to pursue.

@leighmcculloch leighmcculloch added this pull request to the merge queue Mar 17, 2026
Merged via the queue into main with commit 7847eae Mar 17, 2026
178 of 180 checks passed
@leighmcculloch leighmcculloch deleted the improve-xdr-trait-docs branch March 17, 2026 20:44
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.

Document ToXdr and FromXdr traits

3 participants