thirdweb-dev / js

Best in class web3 SDKs for Browser, Node and Mobile apps
https://thirdweb.com
Apache License 2.0
430 stars 328 forks source link

Fixing Popup Handling Issue in Coinbase Smart Wallet Integration #3320

Closed bablu573 closed 3 months ago

bablu573 commented 3 months ago

When users attempt to process transactions, they get stuck on the "waiting for confirmation" screen due to popup handling issues. This primarily occurs when popups are blocked or not managed properly.

This was not an issue when using RainbowKit. Here are two websites to test:

Please check using Safari with popups disabled.

ThirdWeb SDK version: 5.29.1

joaquim-verges commented 3 months ago

@bablu573 thanks for reporting this! I think I know what's going on, will fix asap

bablu573 commented 3 months ago

Hey @joaquim-verges when to expect the release

joaquim-verges commented 3 months ago

@bablu573 PR #3330 fixes the issue.

You can test it right now by installing the dev version from the PR

npm i thirdweb@0.0.0-dev-906c9785e323ac0b10f42d8888e28188f8e7353b-20240615094612

worked well for me, but let me know if it works for you and we'll get it released to stable asap.

bablu573 commented 3 months ago

@joaquim-verges Its not working, can you try it on safari mobile and after connecting wallet getting error here are images and git repo https://github.com/AvinashNayak27/thirdweb-demo https://thirdweb-demo-liard.vercel.app/

Screenshot 2024-06-15 at 3 35 05 PM Screenshot 2024-06-15 at 3 35 23 PM
jnsdls commented 3 months ago

yep, confirming I see the same.

we're actively tackling this, however it might take a day or two, we'll keep updating this issue as we have builds to try

jnsdls commented 3 months ago

https://github.com/thirdweb-dev/js/pull/3334 will solve the failed connection screen issue

https://github.com/thirdweb-dev/js/pull/3336 is the start of adding a usePrepareTransaction hook which (when tested and finished) will resolve the transaction issue

bablu573 commented 3 months ago

How can I test this and will this solve the pop up issue,as we are heading towards launch soon

joaquim-verges commented 3 months ago

@bablu573 can you try this new version?

npm i thirdweb@0.0.0-dev-1c5f23e90e7330b6208b944cdd1ae332f7df19eb-20240616061727

Let us know if everything works with this one

bablu573 commented 3 months ago

@joaquim-verges It didn't fix,looks like it fixed connecting wallet but not sending transactions

joaquim-verges commented 3 months ago

@joaquim-verges It didn't fix,looks like it fixed connecting wallet but not sending transactions

Can you try removing this line https://github.com/AvinashNayak27/thirdweb-demo/blob/main/app%2Fpage.js#L361

You don't need this, and its delaying your transaction

bablu573 commented 3 months ago

Still not fixed

Screenshot 2024-06-17 at 7 18 13 AM
joaquim-verges commented 3 months ago

@bablu573 checked out the repo, updated to thirdweb@5.29.3

and changed the TransactionButton code to this:

<TransactionButton
          transaction={async () => {
            return prepareContractCall({
              contract,
              method: "function batchMintNFT(address recipient, uint256 numberOfNFTs, string[] svgNames, string[] emotions)",
              params: [recipient, svgName.length, svgName, svgName],
            });
          }}
          onTransactionSent={(result) => {
            console.log("Transaction submitted", result.transactionHash);
          }}
          onTransactionConfirmed={(receipt) => {
            console.log("Transaction confirmed", receipt.transactionHash);
            setTxHash(receipt.transactionHash);
          }}
          onError={(error) => {
            console.error("Transaction error", error);
            setError(error.message);
          }}
          style={{ width: "200px", marginTop: "10px" }}
        >
          Confirm Transaction
        </TransactionButton>

more optimal to use the function signature rather than resolveMethod - see reference for your contract here https://thirdweb.com/base-sepolia-testnet/0xe9A518fEC8E3c756BC44B47351D5780C4b74f200/explorer?name=batchMintNFT

i ran this on safari and it works for me - can you try on your end? (can't push to your repo)

bablu573 commented 3 months ago

Not fixed 🥲 https://thirdweb-demo-liard.vercel.app

Screenshot 2024-06-17 at 9 14 40 AM
jnsdls commented 3 months ago

very interesting, it fails for me on the first "click" but when clicked a second time it works... we'll have to go deeper it seems

joaquim-verges commented 3 months ago

ok @bablu573 one last change in your repo that will fix your issue, add this prop to the TransactionButton

<TransactionButton
   payModal={false} // <-- add this
  ...
 >

the issue is that we do a check the first time you click that helps users pay with fiat/other currency if needed.

Those checks are causing that delay between the click and sending the transaction, causing safari to block it.

If you set payModal={false} it skips all the checks and directly sends the transaction. Your can do the same thing with the useSendTransaction hook.

While this should unblock you for the time being, we're working on a proper solution for this that wont require disabling this.

Let us know if that finally fixes it, and thank you for reporting the issue and bearing with us!

bablu573 commented 3 months ago

Hey thanks it fixed the issue

bablu573 commented 3 months ago

Hey @joaquim-verges didn't want to spam with new issue

 createWallet("com.coinbase.wallet",{
    walletConfig:{
      options: "smartWalletOnly"
    }
  }),

is not working i.e dapp is picking coinbase wallet extension over coinbase smart wallet

jnsdls commented 3 months ago

@bablu573 appreciate the sentiment RE not spamming, I opened a new issue as this seems to be a (related but) separate problem, closing this one instead