Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[spec] v2.2 - action chaining #18
[spec] v2.2 - action chaining #18
Changes from 2 commits
d624aa1
5e3fb0f
56bf013
30eb754
c5ce032
23663cf
3a89e76
c947953
ba3c360
2641cf4
3655457
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
ActionPostResponse
should be as follows, allowing for form-like use-cases where tx approval is not required for each action and maybe just the last action in the chain required the approval.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Form-like is needed. From a UX point of view, you don't want to sign more than once. So ideally, at every new screen, the developer gets new information and packages a final instruction for the user to sign. If you want to make users sign every screen, then you can too.
I would even make every chain/screen transaction optional. Maybe you sign on the first screen and the last screen is an optional survey. Maybe there is no transaction at all, simply collecting info, e.g. pubkeys
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, that's why the transaction can be either
string
ornull
, making the transaction optional.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While this is a useful thing to support form-like experiences, actions are still currently required to return a transaction, since the user is required to sign a transaction.
Implementing something like this is planned in general, but not within this spec change / proposal. When sign message support is added, maybe in there?
@thearyanag maybe you could post a more complete sRFC with details about a proposal to support some sort of "non signing action" support? My gut is that something like this should not be allowed on the root action, so the user is still required to "initiate a session" within the blink by signing a transaction (or eventually a message), then they could complete form-like experiences
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is the user required to sign transaction?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Blink will become inconvenient if you have to sign more than once. Unless it is a feature of that blink to sign multiple times, I believe requiring more than one signature is UX misstep.
Fill form, sign, fill new form, sign again. Devs stuck with partial signs to handle. Find the Blink again, load the partial properly.
At which point you might as well redirect the user to an external site to provide a streamline experience.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with you on form based things. I agree that the idea of the user not being always required to sign something in every single action is a good one. It can unlock some interesting new experiences within blinks.
I'm saying it is out of scope for this specific PR/spec change. This spec change has already gone through much discussion on the Solana forum for several weeks now and is effectively finalized.
Hence my original request:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Within the upcoming spec proposal for sign message, we are planning to suggest a session token that can be passed back and forth between the action api server and the blink client. Think JWT passed between server and client, but within a blink.
Something like this would also make the idea of non-signing actions even more powerful and useful. The user initiates an authenticated session (by signing a message via their wallet in the first action in a chain), the api server verifies the signature in their backend, the api server provides their
token
for the client interactions.Off the top of my head:
Going with a pure "this is a form and does not require the user to sign anything" approach you seem to be suggesting is easily abused since a bot can spam the action api server to submit the form data. But never truly connect or verify a wallet. Ripe for abuse.
Hence my suggestion of:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fair. I'm late to the discussion and this is a needed step anyhow.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
agreed, I'll post a more detailed sRFC
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also proposing to explicitly highlight the behaviour on tx failed during confirmation - my proposal is to always send the tx signature to API regardless of confirmation status to enable basic error handling in all cases
Thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the transaction failed to confirm, I would assume the action could be called again via the blink UI to attempt to repeat it. Is this not the case?
Having the ability to achieve a callback based on each of these solana transaction lifecycle events could be useful though. But it does feel like scope creep...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we generally have 2 options
a) As you proposing, assume the action should be called again via the blink UI to attempt to repeat it. I think it will work fine, but can be a bit annoying if you have a long chain and want to repeat only last action.
b) If next action with
POST
type is defined, always pass the tx signature to backend regardless of confirmation result - backend can check signature status and make a decision about the next step, e.g. retrying last action, rather than to repeat the entire chain from the beginningSo, blink client in (b) just needs to always send tx signature to callback, backend then will return the next state that should be shown to user
No need for any special behaviour, just always pass tx signature to callback after tx confirmation stage finished
Just render it, as it would do it for the any other next action
I think single would be already good for the start - developers can just check signature status on their backend to decide what should be done next
So (b) just gives a bit more options to developer to handle potential tx errors, we just need to always send tx sig to callback. Does it still feels like scope creep to you?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would not expect the blink-client to attempt to repeat the entire action chain from the start if the current action failed to confirm. That would be foolish lol. I would only the client to reattempt the current action.
there would have to be some way to denote each of the lifecycle status you suggested ones though:
tx confirmed
,tx failed
, andtx timed out
.A transaction can fail for many reason on the client side. "tx failed" would be a generic failure vs "tx timeout" would be a specific one like an expired blockhash. it could fail preflight checks performed by the blink-client and or wallet.
many causes of a transaction failing of which would result in no tx id existing on chain. Therefore not giving much useful info to the action api to actually handle various error states. there would not always be a tx id to give to process on-chain errored transaction
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Long story short: I believe it's good enough to go with just reattempting the last action, we can improve error handling later as a separate sRFC.
Ahh, ok, then I've misunderstood. Reattempting the last action instead of the entire chain is good for the start
In the future could still be a single callback, but with different payloads. Generally errors should have an error code + error message, that is useful info to pass to API together with the signature if it's available. E.g. see phantom deeplink callback https://docs.phantom.app/phantom-deeplinks/provider-methods/signandsendtransaction#reject. Not advocating to use the same structures, but this just gives an idea of data we could include.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To summarize, imo gtg with reattempting the last action
Let's skip error handling callback improvements and make them as a separate spec update, seems like we need broader discussion here