Skip to content
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

Sending transactions on iOS shows a deep link prompt after first attempt #1165

Closed
markdalgleish opened this issue Jun 16, 2022 · 13 comments
Closed
Labels
type: bug Something isn't working

Comments

@markdalgleish
Copy link

Describe the bug

Attempting to send a transaction on iOS via @walletconnect/ethereum-provider + ethers shows a prompt to the user to open in the app rather than opening the app automatically after the first attempt.

It seems to be caused by the fact that the deep link navigation is triggered some time after the user interaction has occurred. The browser then intercepts the navigation, giving users a chance to back out of it.

I pushed a minimal repro here: https://github.com/markdalgleish/walletconnnect-deeplink-prompt-issue
It's deployed here: https://walletconnnect-deeplink-prompt-issue.vercel.app

I was also able to reproduce this same issue by simply navigating to the deep link URL after a 1s timeout. To help with debugging, I also included a button for doing this in my minimal repro. Note this doesn't happen if you navigate immediately, so there's also a button for this too for comparison.

I'm not sure if this is something that can be fixed or not, short of pre-emptively navigating to the deep link URL, but I realise that might also cause issues with requests that are still in flight. Either way, I figured it was worth raising since it's not a great user experience.

SDK Version

To Reproduce
Steps to reproduce the behavior:

  1. Go to https://walletconnnect-deeplink-prompt-issue.vercel.app on iOS.
  2. Connect wallet.
  3. Tap "Send transaction".
  4. Cancel request and go back to browser.
  5. Tap "Send transaction" again.
  6. See prompt to Open this page in "App name"?

Expected behavior
Immediately navigates to the app, the same way it did on the first attempt.

Screenshots

Deep link prompt

Smartphone:

  • Device: iPhone 12 Pro
  • OS: iOS 15.5
  • Browser: Safari
@markdalgleish
Copy link
Author

Surprisingly I'm not seeing this issue on https://example.walletconnect.org, but looking at the code I can't see why it wouldn't run into this too: https://github.com/WalletConnect/walletconnect-example-dapp/blob/8a3216597474b09a7dcadc84efaf7e393c8eb05d/src/App.tsx#L284-L357

In my repro app, I've tried making the same API calls as the WC example by hard coding multiple await fetch(...) calls before navigating and I still get prompted. I can't figure out why the WC example works without issue. Any ideas?

@intergalacticspacehighway
Copy link

intergalacticspacehighway commented Jun 23, 2022

@markdalgleish i think it all comes down to how long does it take before setting window.location.href (WalletConnect uses this instead of window.open). If it happens quickly after the user action it works as expected or else shows the popup. I was able to reproduce the issue by below change in example.

Screenshot 2022-06-24 at 12 25 09 AM

RPReplay_Final1656010925.MP4

@intergalacticspacehighway
Copy link

intergalacticspacehighway commented Jun 23, 2022

i think there might not be a solution to this (wishing i am wrong!). I think acceptable UX would be if you're doing "some stuff" before signing/sending transaction, show a check your wallet button that a user can tap and redirected, instead of auto redirection. Also, wallet notifications might help in most cases

@markdalgleish
Copy link
Author

markdalgleish commented Jun 23, 2022

I was also able to introduce this issue to the WalletConnect example by adding the following line after the 2nd network request when sending a transaction, forcing it to wait until the next tick before continuing:

WC example

If I add this line any earlier it doesn't work. It seems like it's not simply a timing issue and that the browser is using some more advanced heuristics to decide whether a navigation is triggered by a user action or not.

@intergalacticspacehighway
Copy link

intergalacticspacehighway commented Jun 24, 2022

wow. had some more observations.

This works as expected

Screenshot 2022-06-24 at 10 05 52 AM

This doesn't

Screenshot 2022-06-24 at 9 56 09 AM

  • sometimes it works during first try but fails on subsequent attempts.

Extreme, but even this works

Screenshot 2022-06-24 at 9 57 26 AM

I think it might be allowing multiple fetch (under certain timeout) under user action. Here's the server if you want to try https://my-vercel-functions-six.vercel.app/api?delay=1000

@intergalacticspacehighway
Copy link

intergalacticspacehighway commented Jun 24, 2022

I think it might be allowing multiple fetch (under certain timeout) under user action.

if this is true, WalletConnect might have to move window.location.href assignment to fetch API success instead of call_request_sent socket listener.

Correction: looking into source code call_request_sent appears to be triggered synchronously (not a socket listener). uff. will have to dig more 🤦

@intergalacticspacehighway

okay, i have one more theory.
in your repro example, it's using provider from ethers which does a bunch of things before calling sendTransaction on actual WalletConnect's connector instance. I think the ethers provider does gasEstimation and it might be using polling or setTimeout. Will have to dig more, I have a hunch that this might be causing it.

@markdalgleish
Copy link
Author

markdalgleish commented Jun 28, 2022

I've been digging into JsonRpcSigner in Ethers to understand this a bit more, and here's what I've found.

Calling signer.sendTransaction(tx) results in two blocks of async work happening before the deep link fires:

  • It calls this.provider._getInternalBlockNumber (source)
  • It then calls this.sendUncheckedTransaction which itself performs a couple of async calls:
    • If the transaction's gasLimit property is nullish, it calls this.provider.estimateGas (source)
    • If the transaction's to property exists, it calls this.provider.resolveName (source)

Knowing this, I've been able to work around the timing issues by doing the following:

  • Resolve the gasLimit and to properties up front by calling signer.populateTransaction (source) rather than doing it as part of the sendTransaction call.
  • Call signer.sendUncheckedTransaction directly so that it doesn't call this.provider._getInternalBlockNumber.

I've expanded my repro to include a demo of this. There's now an additional row of actions where you can populate a transaction, then—when it's ready—pass it to either sendTransaction or sendUncheckedTransaction.

Populate transaction

I've found that tapping "Send transaction" still works most of the time, but occasionally prompts me to open in the app instead of navigating immediately. If I tap "Send unchecked transaction", it takes me directly to the app every time without a prompt.

@intergalacticspacehighway
Copy link

intergalacticspacehighway commented Jun 29, 2022

Thanks for digging in, really makes sense. I have some questions.

Why does it wait for _getInternalBlockNumber call before calling sendUncheckedTransaction, it doesn't seem to be using it's return value in sendUncheckedTransaction (unless it mutates stuff or is waiting purposefully).

Also, I don't get the reason why should we "estimate" gas on dapp side (unless we specify it explicitly), shouldn't it be handled at the wallet level as it'll eventually calculate it if I am not wrong.

@markdalgleish
Copy link
Author

markdalgleish commented Jun 30, 2022

Some additional findings in terms of a workaround:

  • Calling signer.populateTransaction worked with WalletConnect but was throwing errors for me when using MetaMask, so I had to use provider.estimateGas and provider.resolveName directly. Transactions also support promises for any their values so you can use the resolveProperties function from ethers/lib/utils (source) to resolve these ahead of time.
  • An alternative to using signer.sendUncheckedTransaction is to use UncheckedJsonRpcSigner instead (source). This gives you a signer with the same interface, but calling sendTransaction always uses sendUncheckedTransaction internally. You can get an instance of this from JsonRpcSigner by calling signer.connectUnchecked() (source).

EDIT: As per my first point, I've updated my repro to avoid signer.populateTransaction and to populate it manually using resolveProperties, provider.resolveName and provider.estimateGas.

@jxom
Copy link

jxom commented Jul 4, 2022

Update: we are starting to work on "eager" hooks in wagmi (using @markdalgleish's eager populating of txn request) to help with deeplinking issues on iOS.

wevm/wagmi#658

@xaun
Copy link

xaun commented Jul 4, 2022

Maybe #444 (comment) is related to this? I experience this alert previously but since making the mentioned change in approach, this alert does not show up anymore.

@markdalgleish
Copy link
Author

I opened a PR adding documentation to the WalletConnect site explaining the cause of these iOS app link issues and how to avoid them which recently got merged. The documentation is now live: https://walletconnect-docs-git-fork-markdalgleish-b4d402-walletconnect1.vercel.app/mobile-linking#ios-app-link-constraints

I'm closing this issue since there's not much else that can be done from a WalletConnect perspective. I've started a GitHub discussion against Ethers to see if anything can be done there to help WalletConnect consumers avoid these issues: ethers-io/ethers.js#3178

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants