wevm / wagmi

Reactive primitives for Ethereum apps
https://wagmi.sh
MIT License
5.97k stars 1.05k forks source link

bug: waitForTransaction doesn't account for safe's `safeTxHash` #2461

Closed TateB closed 9 months ago

TateB commented 1 year ago

Is there an existing issue for this?

Package Version

1.0.6

Current Behavior

when using useWaitForTransaction, providing a hash from a transaction initiated with SafeConnector will never return a receipt. this is because eth_sendTransaction through the connector returns a safeTxHash, which isn't equivalent to the onchain hash.

Expected Behavior

not really sure of the best way to implement a solution to this, but there should probably be a way to use useWaitForTransaction with a tx hash from safe, and within the returned receipt the onchain tx hash. there should also be a way to know that the tx hash returned from useSendTransaction is a safe tx hash and not an onchain one.

Steps To Reproduce

(for use with repro repo)

  1. create a safe on goerli here
  2. clone + install example repo
  3. run pnpm build and pnpm start (dev doesn't work with safe browser)
  4. visit the running app in the safe browser https://app.safe.global/apps/open?safe=gor:[your-safe-address]&appUrl=http%3A%2F%2Flocalhost%3A3000
  5. click "Send Transaction" button in the example app (if the popup doesn't load properly, click cancel and try again)
  6. in the transaction prompt, untick the "Execute transaction" box
  7. click submit and sign in wallet
  8. take notice of "Safe Transaction Hash" and "Safe Transaction Receipt" -- essentially the current native behaviour in wagmi.
  9. in the "Transaction queue" at the bottom of the screen, select the new transaction and click "Execute", and accept in wallet
  10. once the transaction is finalised, the "Safe Transaction Hash" should be mismatching with the "Real Transaction Hash". additionally, the real transaction receipt will be a defined object, but the safe transaction receipt will stay undefined.

Link to Minimal Reproducible Example (StackBlitz, CodeSandbox, GitHub repo etc.)

https://github.com/TateB/wagmi-safe-repro

Anything else?

stackblitz doesn't work properly with the safe browser, which is why the example is a github repo :( the example includes a hacked together NewSafeConnector to help demonstrate the difference between the current returned data and the ideal returned data, it's far from the best way to implement a fix for this

razgraf commented 1 year ago

Have just stumbled over a similar behavior with wagmi/actions writeContract + waitForTransaction. The hash returned by the former is not the correct hash if the Safe connector is being used.

Side note: This is happening with wagmi <1.0 on my side, although the example repo @TateB provided shows that the problem persists in v1.0+ as well.

yivlad commented 1 year ago

Having similar issue. Finding a quick and reasonably elegant fix without modifying code inside wagmi or viem turned out to be quite challenging, so I came up with a temporary helper that works a bit better than default useWaitForTransaction:

import { useState } from "react";
import { Address, parseAbi } from "viem";
import {
  useAccount,
  useContractEvent,
  usePublicClient,
  useQuery,
  useWaitForTransaction,
} from "wagmi";

const useIsWalletContract = (address: Address | undefined) => {
  const publicClient = usePublicClient();

  return useQuery(["isWalletContract", address], async () => {
    if (!address) {
      return undefined;
    }
    const bytecode = await publicClient.getBytecode({
      address,
    });
    return bytecode !== undefined;
  });
};

const useGnosisTransaction = (
  ...args: Parameters<typeof useWaitForTransaction>
) => {
  const publicClient = usePublicClient();
  const hash = args[0]?.hash;

  return useQuery(
    ["gnosisTransaction", hash],
    async () => {
      if (!hash) {
        throw new Error("Hash not found");
      }

      const receipt = await publicClient.getTransactionReceipt({
        hash: hash,
      });

      return receipt;
    },
    {
      ...args[0],
      enabled: !!hash && args[0]?.enabled !== false,
    }
  );
};

type useWaitForTransactionWrappedType = (
  ...args: Parameters<typeof useWaitForTransaction>
) => ReturnType<typeof useWaitForTransaction> & {
  txHash: `0x${string}` | undefined;
};

const gnosisAbi = parseAbi([
  "event ExecutionSuccess(bytes32 txHash, uint256 payment)",
]);

export const useWaitForTransactionWrapped: useWaitForTransactionWrappedType = (
  args
) => {
  const { address } = useAccount();
  const { data: isWalletContract } = useIsWalletContract(address);
  const [gnosisTxHash, setGnosisTxHash] = useState<`0x${string}` | undefined>();
  const unwatch = useContractEvent({
    abi: gnosisAbi,
    address,
    eventName: "ExecutionSuccess",
    listener: (logs) => {
      logs.forEach((log) => {
        if (log.args.txHash === args?.hash) {
          if (!log.transactionHash) {
            throw new Error("Transaction hash not found");
          }

          setGnosisTxHash(log.transactionHash);
          unwatch?.();
        }
      });
    },
  });

  const plain = useWaitForTransaction({
    ...args,
    hash: isWalletContract === false ? args?.hash : undefined,
  });
  const gnosis = useGnosisTransaction({
    ...args,
    hash: gnosisTxHash,
  });

  if (isWalletContract) {
    return {
      ...gnosis,
      status:
        args?.hash && gnosis.status === "idle" ? "loading" : gnosis.status,
      txHash: gnosisTxHash,
    };
  }

  return {
    ...plain,
    txHash: isWalletContract === false ? args?.hash : undefined,
  };
};

There are few limitations - for example, the returned status is idle until the transaction is signed. I guess to fix this one would need to put some kind of a callback between prepareWriteContract and writeContract here. Moreover this doesn't work well with Rainbowkit as Rainbowkit uses wagmi hooks internally.

Hope this helps someone.

EDIT: refactored code a bit

razgraf commented 1 year ago

As an alternative (without hooks) one can also make use of @safe-global/safe-apps-sdk to resolve a safeHash to an onChainHash. A bit hack-ish but it seems to be working for now.


import SafeAppsSDK, { TransactionStatus } from "@safe-global/safe-apps-sdk";
...

const transaction = await writeContract(...);

if(!isSafe) {
   /** The usual case, outside of a Safe environment */
   const receipt = await waitForTransaction({hash: transaction.hash});
}
else {
   /** The hash will be a safeHash, which needs to be resolved to an on chain one */
   const sdk = new SafeAppsSDK(...);
   while (true) {
       /** The SDK will be pinged until a txHash is available and the txStatus is in an end-state */
       const queued = await sdk.txs.getBySafeTxHash(hash);
       if (
        queued.txStatus === TransactionStatus.AWAITING_CONFIRMATIONS ||
        queued.txStatus === TransactionStatus.AWAITING_EXECUTION
       ) {
         /** Mimic a status watcher by checking once every 5 seconds */
        await sleep(5000);
       }
       else {
         /** The txStatus is in an end-state (e.g. success) so we probably have a valid, on chain txHash*/
         const receipt = await waitForTransaction({ hash: queued.txHash });
         return receipt;
       }

   }
}
cleaver86 commented 1 year ago

Would love to see this get fixed in the near future. We have several Safe users that are having difficulty interacting with our app because of this issue.

jejebl commented 1 year ago

I've tried your answer razgraf but it doesn't work for me. The result of writeContract() is the safeTxHash, but we have to wait for the transaction hash to succeed. How can we obtain the transaction hash from the safeTxHash? Has anyone solved this problem?

razgraf commented 1 year ago

I've tried your answer razgraf but it doesn't work for me. The result of writeContract() is the safeTxHash, but we have to wait for the transaction hash to succeed. How can we obtain the transaction hash from the safeTxHash? Has anyone solved this problem?

That's what the while is for. As soon as you get a safeTxHash from writeContract you start polling, as the Safe SDK will now have answers for the status of that Safe TX. When the status of that Safe TX is concrete (not AWAITING_...) the SDK will provide you with a real on-chain tx hash (see queued.txHash).

I suggest watching how the result of that sdk.txs.getBySafeTxHash evolves to understand the format better. Disclaimer: the Safe SDK sometimes fails to provide correct status (it happens once every 20 tries or so) but that's unfortunately on them to fix.

jejebl commented 1 year ago

razgraf The hash in getBySafeTxHash(hash) is the transaction value from const transaction = await writeContract(...); ? My code waits indefinitely when I call the getBySafeTxHash function. This is my code:

   const { hash: approveSent } = await writeContract(config)
   console.log(approveSent)
    const sdk = new SafeAppsSDK({
      allowedDomains: [/app.safe.global$/],
      debug: false,
    });

    let bool = true;
    while (bool) {
        // The SDK will be pinged until a txHash is available and the txStatus is in an end-state
        const queued = await sdk.txs.getBySafeTxHash(approveSent);
        console.log(queued)
        if (
         queued.txStatus === TransactionStatus.AWAITING_CONFIRMATIONS ||
         queued.txStatus === TransactionStatus.AWAITING_EXECUTION
        ) {
          // Mimic a status watcher by checking once every 5 seconds
          console.log('1')
          window.setTimeout(() => {
            console.log("Delayed for 5 seconds.");
          }, 5000);
        }
        else {
          // The txStatus is in an end-state (e.g. success) so we probably have a valid, on chain txHash
          console.log('2')
          const receipt = await waitForTransaction({ hash: queued.txHash});
          bool = false;
          return receipt;
        }
    }
razgraf commented 1 year ago

Can you try with {debug: true} and no allowedDomains when you initiate the SDK?

Also, not sure window.setTimeout keeps things sync there, which may lead to a broken while loop. I'd refactor the code to use full async/await. Make sure to await this sleep/delay method.

function sleep(ms = 1000): Promise<void> {
  return new Promise((resolve) => setTimeout(resolve, ms));
}
jejebl commented 1 year ago

razgraf I tried but it's doesn't work.

tmm commented 9 months ago

Moving to discussion as this isn't the intent of waitForTransaction right now. At some point, Viem might have a nice API for this and Wagmi will inherit it downstream.