Closed Pessina closed 3 months ago
vercel[bot]
commented
5 months ago The latest updates on your projects. Learn more about Vercel for Git ↗︎
| Name | Status | Preview | Comments | Updated (UTC) |
|---|---|---|---|---|
| fast-auth-signer | ✅ Ready (Inspect) | Visit Preview | 💬 Add feedback | Jun 11, 2024 9:55am |
hcho112
commented
5 months ago While I was looking at the code, it comes to my mind that, would it be nicer if:
TDLR, I don't think just assume that user wants to transaction with their account balance is a good thing. It is possible that either they may not want this feature nor not being aware of this will happen. (Some user may even think this is a scam because they may expect using fast-auth + relayer would pay the gas but it ends up using user's balance)
Ideally, we should implement both where:
allowToUseUserBalance or some sort of boolean flag. and need to rebase and fix yarn lock file
Pessina
commented
5 months ago @hcho112 Good point!
For the developer part, I agree with you, and it would be beneficial to allow developers to configure whether they want users to pay for the transaction in case the relayer refuses the transaction. We can bring this up for discussion. The issue here is that we would need to change the wallet interface. From what I understand, this requires a NEP, right? I propose we handle this later as a separate ticket.
However, for the user part, I believe they are already aware of the fees as it's already stated on the iFrame of the fast-auth-signer.
As you can see, the total already includes the fees. Users assume they will pay the gas, but sometimes they won't. It's a positive surprise in that case.
What do you think?
Pessina
commented
5 months ago @hcho112 regarding the multi-chain, this PR doesn't touched multi-chain code, it's only for the /sign method.
hcho112
commented
5 months ago @Pessina Personally I think this is a good change, I would rather have something that works even if I need to pay transaction with my account. However I would rather confirm with @diogoortega and @nall-near for further feedback. Can we get your feedback?
In terms of making change on near-fastauth-wallet, I don't think it needs to be mentioned in NEP. We can make the config changes by updating FastAuthWalletParams type with adding extra prop and pass down from there?
diogoortega
commented
5 months ago Agree that it's great to have a fallback mechanism instead of just having the transaction fail. However, we should inform the user. When will we know if the transaction will be "sponsored" (via relayer) or not? Can we show that info in the confirmation dialog linked above or would we only know after the transaction is confirmed by the user?
If we don't know beforehand, maybe we could add a checkbox (default ON) that says something like "Use my balance for this transaction if sponsoring is not available". Similar to this toggle on iOS that tries to send your text via SMS when iMessage isn't available:
What do you think @nall-near?
nall-near
commented
4 months ago I think the context of why the transaction fails is important. For example, in the case of un-allowed actions, or non-whitelisted receiver ID, these hurdles could be intentional to protect the user from carelessly signing transactions.
Similar to Dio's question above,
When will we know if the transaction will be "sponsored" (via relayer) or not? Will we know that the proposed transaction does not meet the appropriate criteria (whitelist, allowed, etc.) before signing?
Regardless, I agree with @diogoortega that we need to inform the user if something is different/unexpected about signing, and try to be as explicit as possible. A. Txn fails because no more gas allotted | Inform user that they will need to pay for the fees B. Txn fails because the receiver is not whitelisted | Inform the user this is not inherently trusted and they should proceed with caution C. Txn fails because the action is not allowed | similar to B., inform the user that this action is not automatic and to confirm before continuing
Let me know if I'm overthinking this, we don't need unique messaging for each use case, but I do want to alert the user with some caution if they are trying to do something outside the allowed context of the relayer as well as when they may need to pay for the fees themselves.
I also like Dio's suggestion to have a toggle within the dapp context settings that allows users to confirm automatically.
nall-near
commented
4 months ago We could use something like this for the signer app, it could also be a little less scary if that makes sense to y'all.
Pessina
commented
4 months ago @diogoortega and @nall-near, thank you for all the input. Let me try to answer your questions, and please let me know if anything is unclear.
When will we know if the transaction will be "sponsored" (via relayer) or not?
We will only know if the transaction will be "sponsored" after sending it via the relayer and checking the response.
Regarding the custom error messages: yes, it's possible. The relayer returns a custom message with some clues about why it failed. I can double-check the messages so we can customize them accordingly.
Will we know that the proposed transaction does not meet the appropriate criteria (whitelist, allowed, etc.) before signing?
No, we only find out after calling the relayer.
We can set up a call with the relayer team to discuss approaches for obtaining this information before calling the send transaction endpoint on the relayer. However, I would advise doing this in a separate PR as it may take time.
Note: User confirmation is only possible when the transaction requires the FullAccess Key. For the FunctionCall Key, the dApp holds the key and can sign transactions on behalf of the user without confirmation.
Currently, we do not have a UI for the FunctionCall Key. We can develop one, but the dApp would still be able to bypass this confirmation if desired.
Let me provide more details about the flow:
1 - FullAccess Key:
2 - FunctionCall Key:
The main difference is that with the FullAccess Key, we display the iFrame for user confirmation. In the second scenario, we do not have a UI to ask the user to confirm if they want to use their own balance as the dApp holds the FunctionCall Key and can sign transaction on behalf of the user without asking.
Pessina
commented
4 months ago I just had a chat with @DavidM-D about this issue, and he came up with a nice suggestion that we think may simplify the flow.
The dApp probably knows if the transaction can or cannot be relayed, so we could expose two endpoints for the transaction signature:
By doing this, we can create two distinct user interfaces to communicate the transaction details to the user, rather than having a single interface that attempts to handle both scenarios with a toggle box or by prompting the iFrame twice for confirmation.
What are your thoughts on this approach?
Also, I asked him how hard it would be to implement the feature to check if the relayer will accept/refuse the transaction before sending it. He said it will be complicated to do that, so it would be better if we avoid this path.
esaminu
commented
4 months ago The solution to expose 2 endpoints is the most technically sound and makes the most sense. We should move the current implementation into a new signAndSendDelegateAction in near-fastauth-wallet and the new signing of transactions and sending directly to rpc should be done in the current signAndSendTransaction.
We will have to make changes to https://github.com/NearSocial/VM and also https://github.com/near/near-discovery-components to call the signAndSendDelegateAction when a component wants to use the relayer (Everything currently always calls signAndSendTransaction). This also means that all third party components will by default use the user's funds which also makes sense IMO.
As far as UX we can now just use different copy (e.g. "using my funds" vs "using the relayer" or similar) for the 2 different routes unless there are other suggestions
nall-near
commented
4 months ago Hey @esaminu @Pessina, I agree with the two end points. we will keep the UX simple. always displaying the fee number but in the relayer instance mentioning that they are paid or sponsored and maybe crossing out the fee amount.
I'm still curious about the other cases regarding failed transactions and if we need to mitigate those eventually, but fine to move forward with your suggestion for now.
diogoortega
commented
4 months ago Also agree with the two endpoints solution. Thank you for the additional details.
hcho112
commented
4 months ago One thing to note, would be nicer if we start writing test for two different route? @esaminu created PR for signTransaction so would be nicer if we create test once this PR is in master
Pessina
commented
3 months ago @hcho112 I believe the error you are facing it's because you didn't linked the local near-fastauth-wallet package, that's why it's not finding the methods.
In order to do that you need to run pnpm link [path to your near-fastauth-wallet build] on near-discovery root folder, then you can paste the test code I shared with you on Slack and conduct the testing.
Those are the recordings:
https://github.com/near/fast-auth-signer/assets/22358004/6d2f0316-6187-4e17-9d73-cfc93925ab3b
https://github.com/near/fast-auth-signer/assets/22358004/0737a34d-5f73-45ae-836d-d447113f7d9a
If you still face issues, please let me know and we can work on this together.
This PR introduces a fallback mechanism for transactions when the relayer service fails to relay them due to specific issues such as actions not being allowed or the sender/receiver not being whitelisted. In such cases, the transaction will be retried using the user's own balance, ensuring that the transaction can still be processed without dependency on the relayer's current state or configuration.