Add support for signed payload signer keys to StrKey
#511
Conversation
Shaptic
requested review from
acharb,
piyalbasu,
paulbellamy,
quietbits and
leighmcculloch
| @@ -236,4 +238,30 @@ export class Keypair { | |||
|
|
|||
| return new xdr.DecoratedSignature({ hint, signature }); | |||
| } | |||
|
|
|||
| /** | |||
| * Returns the signature hint for a signed payload signer. | |||
There was a problem hiding this comment.
💬 Minor discrepancy here, the comment says this function returns the signature hint, but it returns a decorated signature.
| * This is defined as the last 4 bytes of the signer key XORed with last 4 | ||
| * bytes of the payload (zero-left-padded if necessary). | ||
| * | ||
| * @param {Buffer} data data to both sign and treat as the payload | ||
| * @return {xdr.DecoratedSignature} | ||
| * | ||
| * @see https://github.com/stellar/stellar-protocol/blob/master/core/cap-0040.md#signature-hint | ||
| */ | ||
| signPayloadDecorated(data) { | ||
| const signature = this.sign(data); | ||
| const keyHint = this.signatureHint(); | ||
|
|
||
| let hint = Buffer.from(data.slice(-4)); | ||
| if (hint.length < 4) { | ||
| // front-pad with zeroes if necessary | ||
| hint = Buffer.concat([Buffer.alloc(4 - data.length, 0), hint]); |
There was a problem hiding this comment.
❗ The function comment states that the last 4 bytes are zero-left-padded if less than 4 bytes, but it is zero-right-padded in CAP-40:
If the payload has a length less than 4 bytes, then 1 to 4 zero bytes are appended to the payload such that it has a length of 4 bytes, for calculating the hint.
Ref: https://github.com/stellar/stellar-protocol/blob/f13f159/core/cap-0040.md#signature-hint
There was a problem hiding this comment.
Yep, caught this later in #523. Wish GH had a better notification system! Def worth a ping in my case if you add comments post-merge 👍
| * @param {string} address data to decode | ||
| * @returns {Buffer} | ||
| */ | ||
| static decodeEd25519SecretSeed(data) { | ||
| return decodeCheck('ed25519SecretSeed', data); | ||
| static decodeEd25519SecretSeed(address) { | ||
| return decodeCheck('ed25519SecretSeed', address); |
There was a problem hiding this comment.
💡 Fwiw the secret seed strkey is not an address, it's a seed encoded in a strkey. Using the term encoded instead of data may be more accurate if you're wanting to get away from the generic data.
There was a problem hiding this comment.
Yeah that's a pretty good point... I wanted to get away from the generic data to indicate the type. Calling it encoded doesn't help, since that's implied by decodeEd25.... I find myself checking the docstring every single time I use this set of helpers, because "am I encoding to strkey or am I decoding from strkey or am I decoding from binary or am I ..." etc.
There was a problem hiding this comment.
Naming it str may help then.
| * @param {string} address data to decode | ||
| * @returns {Buffer} | ||
| */ | ||
| static decodePreAuthTx(data) { | ||
| return decodeCheck('preAuthTx', data); | ||
| static decodePreAuthTx(address) { | ||
| return decodeCheck('preAuthTx', address); |
There was a problem hiding this comment.
💡 Same here, this is also not an address, it's a tx hash encoded in a strkey.
| * @param {string} address data to decode | ||
| * @returns {Buffer} | ||
| */ | ||
| static decodeSha256Hash(data) { | ||
| return decodeCheck('sha256Hash', data); | ||
| static decodeSha256Hash(address) { | ||
| return decodeCheck('sha256Hash', address); |
| * @param {string} address address to decode | ||
| * @returns {Buffer} | ||
| */ | ||
| static decodeSignedPayload(address) { | ||
| return decodeCheck('signedPayload', address); |
* Update XDR definitions and relevant code for Protocol 19 (#509) * Add support for signed payload signer keys to `StrKey` (#511) * Support new preconditions when using building transactions (#513) * Add conversion layer between `xdr.SignerKey`s and `StrKey`s (#520, #523) Co-authored-by: Paul Bellamy <[email protected]>
This adds support for encode signed payload signers (CAP-40) to their StrKey representation (SEP-23):
Closes #507.