Add support for signed payloads to the setOptions operation.
#534
Conversation
src/signerkey.js
Outdated
| * | ||
| * @returns {string} the StrKey-encoded signed payload address (P...) | ||
| */ | ||
| static encodeSignedPayloadFromAddress(address, payload) { |
There was a problem hiding this comment.
Any reason this can't have a shorter name?
| static encodeSignedPayloadFromAddress(address, payload) { | |
| static encodeSignedPayload(address, payload) { |
There was a problem hiding this comment.
probably not 🤷 that clashes with StrKey.encodeSignedPayload, but separate namespaces should be enough of a hint that they're different
There was a problem hiding this comment.
ooh how about composeSignedPayload 🎶
There was a problem hiding this comment.
What is StrKey.encodeSignedPayload for? It seems like a very low-level thing to expose to developers. We don't expose it in other SDKs.
There was a problem hiding this comment.
All of the StrKey functions in the SDK convert between raw buffers and strkeys. I'm not a fan and it deserves a refactor to line up more w/ the other SDK implementations, but that's historical.
There was a problem hiding this comment.
I think the historic reason was fine because once-upon-a-time all strkeys were just a single 32byte buffer.
It's unintuitive to me that a user must use a SignerKey object to generate a strkey. Maybe we should do soemthing like what we did with MuxedAccounts, where you create a SignedPayloadSigner which is an object that sits apart, and then you can generate a SignerKey from it.
| * | ||
| * @returns {string} the StrKey-encoded signed payload address (P...) | ||
| */ | ||
| static composeSignedPayload(address, payload) { |
There was a problem hiding this comment.
Can we name this function encodeSignedPayload? It fits existing terms used in the SignerKey type and doesn't clash with any existing function. Using a new term compose seems unnecessary.
src/signerkey.js
Outdated
| * | ||
| * @returns {string} the StrKey-encoded signed payload address (P...) | ||
| */ | ||
| static encodeSignedPayloadFromAddress(address, payload) { |
There was a problem hiding this comment.
I think the historic reason was fine because once-upon-a-time all strkeys were just a single 32byte buffer.
It's unintuitive to me that a user must use a SignerKey object to generate a strkey. Maybe we should do soemthing like what we did with MuxedAccounts, where you create a SignedPayloadSigner which is an object that sits apart, and then you can generate a SignerKey from it.
|
Just realized that my PR #542 was intended to solve the same problem. Wish I'd done more comprehensive search before writing the code. There were no mentions of this PR in the issue tracker. Is there any ETA? Quite a high-priority issue for us. Can we go at least with a simple MVP that allows to build a transaction plus correctly parse tx XDRs, and implement additional tooling later given that there's no consensus on methods naming? |
|
Looks like this PR fell through the cracks, it was intended to go out when protocol 19 was released. cc @Shaptic |
|
I think we can close this in lieu of the merged-and-released #542. If usability and ergonomics become a concern, we can revisit this design. |
While many parts of the SDK have been updated to enable the new signer type from CAP-40, the
SetOptionsoperation has not.As part of this, new helpers exist:
which should make it easy to create a P-address from a G-address and a payload and vice-versa.