SEP-23: Add strkey type for CAP-40 signed payload signers

[leighmcculloch]

What

Introduces a new strkey type to represent ed25519 signed payload signers introduced by CAP-40.

Why

CAP-40 introduces a new signer for disclosing transaction signatures in transactions. Some ecosystem services, such as Horizon, only refer to signers in string form. Stellar SDKs also use strings to pass around signers. A string form for the signer introduced by CAP-40 is required for these services.

The leading P prefix for the new strkey was chosen to indicate that the signer is a signed payload because the word payload begins with p.

The 4-byte length prefix is to ensure that a truncated signed payload that has a valid CRC is not considered valid.

The 4-byte length prefix and the payload zero padding to a multiple of 4 bytes makes the raw bytes encoded in the strkey identical to the signer key when XDR encoded, a property of existing strkeys, excluding muxed accounts that match their XDR encoding with fields reversed.

Close stellar-deprecated/starlight#191

Merging

This is intended for merging if CAP-40 is accepted.

[stanford-scs]

This looks good except for the one nit I put in an annotation of the diff.

[github-actions]

This pull request is stale because it has been open for 30 days with no activity. It will be closed in 30 days unless the stale label is removed.

[Shaptic]

@leighmcculloch Will this get merged soon? I'd like to start the relevant SDK work but don't want something to change in the middle of it.

[leighmcculloch]

@Shaptic This is unlikely to be merged until the core implementation is complete, otherwise we'll have no room to make breaking changes if necessary. For the same reason it would be ideal if one SDK and Horizon is near complete too.

However, I don't think this will change substantially as strkeys are opaque so any change is likely to require only tweaks in the SDK and not changes downstream to Horizon.

Also, at the moment the PR has only one approval. It would be great if @stellar/horizon-committers, and anyone else, could review also.

[Shaptic]

Some minor optional comments ⬇️, but LGTM as-is 👍

[Lippertt]

Não tô te entendendo fala tu mano Em qui, 10 de mar de 2022 2:29 PM, George ***@***.***> escreveu:

…

***@***.**** commented on this pull request. ------------------------------ In ecosystem/sep-0023.md <#1014 (comment)> : > @@ -77,14 +79,20 @@ The following steps transform a binary key into a strkey: 3. If we are encoding a multiplexed address, append an 8-byte memo ID in network byte order (most significant byte first). -4. Compute a 16-bit CRC16 checksum of the combined version byte, - optional memo ID, and binary key (using polynomial - x¹⁶ + x¹² + x⁵ + 1). Append the - two-byte checksum to the result of the previous step (e.g., - producing a 35-byte quantity for a non-multiplexed ED25519 public - key, or 43 byte quantity for a multiplexed one). +4. If we are encoding a signed payload, append a 4-byte length in I'll be honest, the only reason why I even noticed this is because I saw the AAAA characters in the middle of the strkey and that seemed odd to me. So reducing the length has an aesthetic benefit as well 😆 but again I don't have a strong opinion here, especially if we want adherence to XDR, as it'd make SDK code simpler. — Reply to this email directly, view it on GitHub <#1014 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AWJCMJYRB46RNMTUJPW43KDU7IWO3ANCNFSM5A47SGFQ> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>. You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

[stanford-scs]

Looks good to me.

[leighmcculloch]

@Shaptic @sisuresh Are you okay with us merging this at this point? Merging is locking it in.