[v3 ETH Mainnet] Fill or Kill Order failing due to insufficient gas

[wizardlabsxyz]

Why would this contract function run out of gas?

Example of a transaction that failed from my application https://etherscan.io/tx/0x628c7c14d69e838a05ac7dbd8e5b4e6f968eae810c7940ca42ddeb4e02947428

[johnrjj]

I'm guessing it has to do with one of the NFTs requiring a bit more gas than usual to do a transfer (sometimes there are custom transfer hooks and methods)

I confirmed it is indeed a gas issue -- https://dashboard.tenderly.co/tx/mainnet/0x628c7c14d69e838a05ac7dbd8e5b4e6f968eae810c7940ca42ddeb4e02947428

I re-simulated your transaction with a 250k gas limit and it went through successfully --

From there I profiled the gas and it looks like the first NFT transferred takes extra gas in the transfer method of the contract:

I would just pad this transaction with a bit more gas.

[wizardlabsxyz]

Interesting, thanks for the quick investigation. The site we are building will handle swaps of all different combinations and counts of NFTs and ERC20s. If metamask is not able to calculate the gas limit properly, how can we be confident in the gas limits that we arbitrarily set (or pad)?

[jinsley8]

I'm having the same issue, can we set a higher gas limit like this?

https://github.com/trader-xyz/nft-swap-sdk/blob/5782d110b8882f7490f33b80acf59cd6f2ea7301/test/v3/swap.test.ts#L81

[wizardlabsxyz]

I solved this by adding a gas buffer and this essentially doubles the gas limit. I don't love this because orders will likely fail again due to insufficient gas but this at least solved the problem for the transaction I was testing. Perhaps is someone wants to trade many tokens at the same time doubling the limit will not be sufficient. I am uncertain if there is a better way.

const fillTx = await nftSwapSdk.fillSignedOrder(signedOrder, { gasAmountBufferMultiple: 2 });

[jinsley8]

Default fillSignedOrder seems to run out of gas for certain NFT collections for single swaps ERC721<>ERC721 plus fees:

const fillTx = await swapSdk.fillSignedOrder(normalizedOrder);

This gets over the limit for the same transaction:

const fillTx = await swapSdk.fillSignedOrder(normalizedOrder, undefined, {
    gasLimit: '250000',
});

However, if I try swapping 3 ERC721 for 1 ERC721 the gas is like 420,000.

I'll test gasAmountBufferMultiple, otherwise I may try calculating gas based on number of assets in the swap. 250,000 base for 2, plus 80,000-100,000 for each additional asset.

Until I ran into this issue, I had been thinking if the txn hash is returned then it was successful. I think you be better to wait for a couple of block confirmations to determine if the txn is a success or failed.

I'm thinking of using wagmi waitForTransaction to wait for a couple block confirmations before continuing with the rest of my code.

https://wagmi.sh/react/hooks/useWaitForTransaction

import { useWaitForTransaction } from 'wagmi'

const fillTx = await swapSdk.fillSignedOrder(normalizedOrder);
const fillTxReceipt = await swapSdk.awaitTransactionHash(fillTx.hash);
console.log(`useHandleApproveTrade: 🎉 🥳 Order filled. TxHash: ${fillTxReceipt.transactionHash}`);

const waitForTransaction = useWaitForTransaction({
    chainId: 1,
    confirmations: 2,
    hash: fillTxReceipt.transactionHash,
    onSuccess(data) {
      console.log('Success', data)
    },
    onError(error) {
      console.log('Error', error)
    },
})

[wizardlabsxyz]

does the result of awaitTransactionHash not indicate whether the transaction failed or not?

I've used useWaitForTransaction before and my assumption was that awaitTransactionHash was the same thing. It sounds like you're saying that awaitTransactionHash actually resolves before the transaction is actually completed in some cases. Is that what you mean?

[jinsley8]

It returns TransactionReceipt as:

const fillTxReceipt = {
    accessList: null
    blockHash: null
    blockNumber: null
    chainId: 0
    confirmations: 0
    creates: null
    data: "0xe14b5....00000000000000000000000000000000000000000000000000"
    from: "0x0A...A0"
    to: "0x0...94"
    hash: "0x9eb...29"
    gasLimit: BigNumber {_hex: '0x0493e0', _isBigNumber: true}
    gasPrice: BigNumber {_hex: '0x2439710915', _isBigNumber: true}
    maxFeePerGas: BigNumber {_hex: '0x2439710915', _isBigNumber: true}
    maxPriorityFeePerGas: BigNumber {_hex: '0x094d4705eb', _isBigNumber: true}
    nonce: 248
    r: "0x213...a66"
    s: "0x7f7...b9bba3"
    transactionIndex: null
    type: 2
    v: 0
    value: BigNumber {_hex: '0x00', _isBigNumber: true}
    wait: (confirmations) => {…}
}

useWaitForTransaction returns that as data plus state: https://wagmi.sh/react/hooks/useWaitForTransaction#return-value

[wizardlabsxyz]

it looks like fillTxReceipt has a function called wait that takes an integer as the number of confirmations you want to wait for. Perhaps using fillTxReceipt.wait(2) is the same as using useWaitForTransaction as you illustrated above.

[jinsley8]

Yes, possibly, I'll test it out. It looks like it returns errors.

https://docs.ethers.org/v5/api/providers/types/#providers-TransactionResponse

[jinsley8]

Okay this works:

const fillTx = await swapSdk.fillSignedOrder(normalizedOrder, { gasAmountBufferMultiple: 1.4 });
const fillTxReceipt = await fillTx.wait(2);
const filledTxnHash = fillTxReceipt.transactionHash;
const txnSuccess = fillTxReceipt?.status ?? 0;
// now I make sure the txnSuccess === 1 and a hash exists to consider it a successful txn

// .wait() returns the same TransactionReceipt as .awaitTransactionHash() but the latter only waits 0-1 confirms
const fillTxReceipt = await swapSdk.awaitTransactionHash(fillTx.hash);

It returns TransactionReceipt if successful otherwise it errors.

awaitTransactionHash could be changed to allow an optional confirms parameter: https://docs.ethers.org/v5/api/providers/provider/#Provider-waitForTransaction