Update stellar-strkey to 0.0.18 by leighmcculloch · Pull Request #547 · stellar/rs-stellar-xdr

leighmcculloch · 2026-06-17T02:55:56Z

What

Bump the stellar-strkey dependency from 0.0.13 to 0.0.18 and adapt the hand-written strkey usage in src/str.rs to the 0.0.17+ API.

Why

0.0.18 is the stellar-strkey release line for protocol 28. The upgrade requires a few source changes because the API differs from 0.0.13: the Strkey::PrivateKeyEd25519 variant was removed, and ed25519::SignedPayload now has private fields exposed via new(ed25519, payload), ed25519(), and payload() accessors instead of public struct fields. src/str.rs was updated to construct and read SignedPayload through these accessors and to drop the removed PrivateKeyEd25519 match arms. The lockfile was updated to pin 0.0.18.

This supersedes the earlier 0.0.17 bump: 0.0.17 contained a CLI bug (Decoded<Strkey> rejected owned JSON keys, breaking strkey encode), fixed in 0.0.18 by stellar/rs-stellar-strkey#125.

Known limitations

N/A

Protocol 28 coordination

⚠️ This change targets versions of software that will ship for Protocol 28, and should not be merged unless this repository is ready to accept Protocol 28 changes.

This is one of a coordinated set of PRs bumping stellar-strkey to 0.0.18 across the Protocol 28 component stack. It is preferred that all Protocol 28 releases of these components depend on the same version of stellar-strkey:

stellar-strkey 0.0.18 raises the minimum supported Rust version to 1.87 and updates/adds transitive dependencies (heapless 0.9, zeroize), so MSRV and supply-chain policy (cargo-deny / cackle) updates may be required in some repositories before CI is green.

0.0.18 includes the strkey CLI fix (accept owned variant keys for Decoded<Strkey>). It is not yet on crates.io, so temporarily patch stellar-strkey to the release/v0.0.18 branch to validate ahead of the release. Remove the patch once 0.0.18 is published.

leighmcculloch · 2026-06-18T02:05:53Z

@@ -202,12 +201,10 @@
    fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
        let SignerKeyEd25519SignedPayload {
            ed25519: Uint256(ed25519),
            payload,
        } = self;
-        let k = stellar_strkey::ed25519::SignedPayload {
-            ed25519: *ed25519,
-            payload: payload.into(),
-        };
+        let k = stellar_strkey::ed25519::SignedPayload::new(*ed25519, payload.as_ref())
+            .map_err(|_| core::fmt::Error)?;


There's a subtle meaningful change here that I'd like to give a heads up and a chance for commentary before this is merged.

The Display::fmt implementation is called by ToString::to_string. Display::fmt can return an error, but ToString::to_string doesn't and so panics if fmt returns an error.

Previously you could construct a SignerKeyEd25519SignedPayload with a zero length payload all the way up to a u32::MAX length payload and it would render a string. It would panic in payloads longer than u32::MAX. However, that string generated might be invalid, since a payload must be 1 to 64 bytes long in stellar-core, we implemented the rs-stellar-strkey parse impl to always require within those lengths. Attempts to then parse the key would error.

The latest rs-stellar-strkey rectified the asymmetry in its own type for two reasons:

The asymmetry was identified as a potential security issue that an application could be impacted by if they used the type naively.

To support no-std no-alloc types a low max length was convenient.

This has a side-effect that it's possible to construct a SignerKeyEd25519SignedPayload xdr type whose Display impl returns an error and its ToString impl panics. It was possible before if the payload was u32::MAX+1+, but now it is possible with 0 and 65+

I think this change is inevitable unless we remove the limits from rs-stellar-strkey, and walked back the no-std no-alloc support for that type.

I don't think this can effect stellar-core, but there is some XDR, albeit invalid, that cannot be represented in string and json forms.

cc @graydon @dmkozh @mootz12

I don't understand why do we need to panic in the Display impl. Can't we just return something like <invalid>? While this may not necessarily affect env today, I can imagine code that logs the invalid payload if it fails some kind of validation - it would be weird for the logging logic to cause panic then.

It will create the same problem that the auditors had with the strkey library, that the type will not roundtrip which can also be a failure point similar to the failure point we are trying to address here. Maybe it's more appropriate to use the Debug impl in that case.

I could look at introducing a validation into the XDR type so that it cannot be constructed with a payload outside the range. We do that today with max lengths on XDR var arrays, but not min lengths.

, that the type will not roundtrip

Why is that an issue, if the input is invalid? IMO it doesn't make sense to require every input to be able to roundtrip. Moreover, this is about XDR types, not strkey types. There is no requirement for XDR types to roundtrip through Display, is there? It's not a serialization format.

I could look at introducing a validation into the XDR type so that it cannot be constructed with a payload outside the range.

That's an option, but I'm still not sold on the roundtrip argument TBH.

0.0.18 is now published to crates.io, so drop the temporary [patch.crates-io] that pointed stellar-strkey at the release/v0.0.18 git branch (and the matching stellar-cli deny.toml allow-git entry). The dependency now resolves from crates.io.

Copilot

✅ Ready to approve

The strkey API updates are localized and consistent with existing parsing/formatting behavior, with only a minor (non-blocking) lint-scoping nit noted.

Note: this review does not count toward required approvals for merging.

Pull request overview

This PR upgrades the optional stellar-strkey dependency to the Protocol 28-compatible 0.0.18 release line and adjusts this crate’s handwritten StrKey parsing/formatting code to match the post-0.0.17 API.

Changes:

Bump stellar-strkey from 0.0.13 to 0.0.18 and update Cargo.lock accordingly.
Update src/str.rs to use ed25519::SignedPayload::{new, ed25519(), payload()} and remove match arms for the removed Strkey::PrivateKeyEd25519 variant.
Raise crate MSRV to Rust 1.87.0.

File summaries

File	Description
`src/str.rs`	Adapts StrKey parsing/formatting to the new `SignedPayload` constructor/accessors and removes deprecated enum handling.
`src/lib.rs`	Adds crate-level clippy allow attributes (likely to keep MSRV clippy green).
`Cargo.toml`	Updates MSRV and bumps the `stellar-strkey` dependency version.
`Cargo.lock`	Locks updated dependency graph for `stellar-strkey 0.0.18` (incl. new transitive deps).

Copilot's findings

Files reviewed: 3/4 changed files
Comments generated: 1

Note

Your feedback helps us improve the quality of this feature.
Please use 👍 or 👎 to tell us whether this assessment is correct.

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

+#![allow(clippy::unnecessary_semicolon)]
+#![allow(clippy::non_std_lazy_statics)]
 #![allow(unused_attributes)]


chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 355ac1d409

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-06-18T03:37:48Z

+        let k = stellar_strkey::ed25519::SignedPayload::new(*ed25519, payload.as_ref())
+            .map_err(|_| core::fmt::Error)?;


Handle empty signed-payloads without panicking

When a SignerKeyEd25519SignedPayload is decoded or constructed with an empty payload (the generated field is BytesM<64>, which has no non-empty invariant), this new call to stellar_strkey::ed25519::SignedPayload::new returns InvalidPayloadLength. Mapping that to fmt::Error makes Display fail, so callers using .to_string() panic and serde SerializeDisplay can no longer serialize a value that the XDR reader accepts. Either preserve the old encoding path for empty payloads or reject that value before it can be represented as this XDR type.

Useful? React with 👍 / 👎.

update stellar-strkey to 0.0.17

c7796c1

leighmcculloch added 2 commits June 17, 2026 05:20

raise msrv to 1.87 for strkey 0.0.17

6f3e5ca

allow new pedantic lints from 1.87 clippy

e69fdd3

leighmcculloch commented Jun 17, 2026

View reviewed changes

Comment thread src/lib.rs Outdated

leighmcculloch added 2 commits June 17, 2026 16:07

Apply suggestion from @leighmcculloch

150246b

update stellar-strkey to 0.0.18

5c69faa

0.0.18 includes the strkey CLI fix (accept owned variant keys for Decoded<Strkey>). It is not yet on crates.io, so temporarily patch stellar-strkey to the release/v0.0.18 branch to validate ahead of the release. Remove the patch once 0.0.18 is published.

leighmcculloch changed the title ~~Update stellar-strkey to 0.0.17~~ Update stellar-strkey to 0.0.18 Jun 17, 2026

leighmcculloch commented Jun 18, 2026

View reviewed changes

depend on published stellar-strkey 0.0.18

355ac1d

0.0.18 is now published to crates.io, so drop the temporary [patch.crates-io] that pointed stellar-strkey at the release/v0.0.18 git branch (and the matching stellar-cli deny.toml allow-git entry). The dependency now resolves from crates.io.

leighmcculloch marked this pull request as ready for review June 18, 2026 03:32

Copilot AI review requested due to automatic review settings June 18, 2026 03:32

Copilot started reviewing on behalf of leighmcculloch June 18, 2026 03:32 View session

Copilot AI approved these changes Jun 18, 2026

View reviewed changes

Comment thread src/lib.rs

Comment on lines +9 to 11

#![allow(clippy::unnecessary_semicolon)]

#![allow(clippy::non_std_lazy_statics)]

#![allow(unused_attributes)]

chatgpt-codex-connector Bot reviewed Jun 18, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Update stellar-strkey to 0.0.18#547

Update stellar-strkey to 0.0.18#547
leighmcculloch wants to merge 6 commits into
mainfrom
update-stellar-strkey-0-0-17

leighmcculloch commented Jun 17, 2026 •

edited

Loading

Uh oh!

Uh oh!

leighmcculloch Jun 18, 2026

Uh oh!

dmkozh Jun 18, 2026

Uh oh!

leighmcculloch Jun 18, 2026

Uh oh!

leighmcculloch Jun 19, 2026

Uh oh!

dmkozh Jun 19, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

chatgpt-codex-connector Bot Jun 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		let k = stellar_strkey::ed25519::SignedPayload::new(*ed25519, payload.as_ref())
		.map_err(\|_\| core::fmt::Error)?;

Uh oh!

Conversation

leighmcculloch commented Jun 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

Why

Known limitations

Protocol 28 coordination

Uh oh!

Uh oh!

leighmcculloch Jun 18, 2026

Choose a reason for hiding this comment

Uh oh!

dmkozh Jun 18, 2026

Choose a reason for hiding this comment

Uh oh!

leighmcculloch Jun 18, 2026

Choose a reason for hiding this comment

Uh oh!

leighmcculloch Jun 19, 2026

Choose a reason for hiding this comment

Uh oh!

dmkozh Jun 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

✅ Ready to approve

Copilot's findings

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Jun 18, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

leighmcculloch commented Jun 17, 2026 •

edited

Loading