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

Enable EIP- 2831 Signer transaction.wait() style rejections. #1477

Closed
ricmoo opened this issue Apr 16, 2021 · 30 comments
Closed

Enable EIP- 2831 Signer transaction.wait() style rejections. #1477

ricmoo opened this issue Apr 16, 2021 · 30 comments
Labels
enhancement New feature or improvement. fixed/complete This Bug is fixed or Enhancement is complete and published. minor-bump Planned for the next minor version bump.

Comments

@ricmoo
Copy link
Member

ricmoo commented Apr 16, 2021

When a Signer sends a transaction, the TransactionResponse object has a .wait() method. Currently that method rejects a CALL_EXCEPTION if the transaction reverts, otherwise it resolves with the TransactionReceipt, once the transaction is mined, based on its hash.

This feature will enable an additional rejection case, along with a new error in the Logger,, TRANSACTION_REPLACED. This will occur if the transaction is replaced (i.e. the from and nonce match the transaction but the hash is different), with a field indicating the reason for the replacement:

  • repriced; the data and to match, but the gas price has changed
  • cancelled; the data has been changed to 0x and the to changed to the from
  • replaced; any other change

This must occur in a minor version bump.

In v6 we may change repriced to also resolve (instead of reject), but this requires more thought and would not be backwards compatible within v5.

@ricmoo ricmoo added enhancement New feature or improvement. on-deck This Enhancement or Bug is currently being worked on. minor-bump Planned for the next minor version bump. labels Apr 16, 2021
@ricmoo
Copy link
Member Author

ricmoo commented May 20, 2021

This is now available in 5.2.0. Please check out this article for more details. :)

@ricmoo ricmoo added fixed/complete This Bug is fixed or Enhancement is complete and published. and removed on-deck This Enhancement or Bug is currently being worked on. labels May 20, 2021
@ricmoo ricmoo closed this as completed May 30, 2021
pull bot pushed a commit to shapeshift/ethers.js that referenced this issue Jun 4, 2021
pull bot pushed a commit to shapeshift/ethers.js that referenced this issue Jun 4, 2021
pull bot pushed a commit to shapeshift/ethers.js that referenced this issue Jun 4, 2021
pull bot pushed a commit to shapeshift/ethers.js that referenced this issue Jun 4, 2021
@mwamedacen
Copy link

mwamedacen commented Jun 5, 2021

Thanks you @ricmoo, exciting feature out here and very much need !

Any plan to support it for external providers (through Web3Provider)?

Indeed, current implementation of JsonRpcSigner#sendTransaction doesn't pass startBlock

    return this.provider._wrapTransaction(tx, hash);

which results in replacement always being undefined in BaseProvider#wrapTransaction

    if (confirms !== 0 && startBlock != null) {
        replacement = {

which ultimately results into opting out of tx replacement watching in BaseProvider#_waitForTransaction

    if (replaceable) {
        let lastBlockNumber = replaceable.startBlock;

I can think of multiple of solutions (calling _getInternalBlockNumber in JsonRpcSigner#sendTransaction, pass blockNumber to _wrapTransaction in BaseProvider#getTransaction...) but I'm not sure of the potential side effects or if these are ubiquitous enough.

Would love to hear from you back.

@ricmoo
Copy link
Member Author

ricmoo commented Jun 7, 2021

@mwamedacen Thanks! Yes, it was completely an oversight. I will add that to the next release. :)

@mwamedacen
Copy link

Thanks again @ricmoo, do you think it would be possible to opt-in for transaction replacement detection through BaseProvider#waitForTransaction() ? Thank you

@ricmoo
Copy link
Member Author

ricmoo commented Jun 22, 2021

@mwamedacen What do you mean? You want to not throw on replacement and receive no notice and stall indefinitely?

You can just use const receipt = await tx.wait().catch(() => new Promise((r)=>{}) to discard any replacement and stall the program from every progressing in that block. If the transaction works, once mined the receipt is set and moves forward. If the transaction is replaced the code will never progress past that line.

@mwamedacen
Copy link

@mwamedacen What do you mean? You want to not throw on replacement and receive no notice and stall indefinitely?

You can just use const receipt = await tx.wait().catch(() => new Promise((r)=>{}) to discard any replacement and stall the program from every progressing in that block. If the transaction works, once mined the receipt is set and moves forward. If the transaction is replaced the code will never progress past that line.

Sorry I wasn't clear, I meant: if we deal with transaction hash, waiting for transaction never throws on replacement (might be wrong though).

@ricmoo
Copy link
Member Author

ricmoo commented Jun 22, 2021

If you wait on a transaction hash it will eventually resolve to the receipt or throw an exception which includes the receipt of the replacement transaction. tx.wait() will eventually finish, one way or the other.

Prior to this feature, tx.wait() would never proceed if the transaction was replaced. If you want to replicate this functionality, you can use the above .catch(), but there are probably very few cases you would want to do that.

Does that make sense?

@mwamedacen
Copy link

If you wait on a transaction hash it will eventually resolve to the receipt or throw an exception which includes the receipt of the replacement transaction. tx.wait() will eventually finish, one way or the other.

Prior to this feature, tx.wait() would never proceed if the transaction was replaced. If you want to replicate this functionality, you can use the above .catch(), but there are probably very few cases you would want to do that.

Does that make sense?

Sorry I'm a bit confused, my case is when I don't have access to the transactionResponse object but only the hash.

So I'm waiting on the transaction via the provider, say

await web3Provider.waitForTransaction(transactionHash)

In this case, replaceable param is always passed down to _waitFortransaction() as null

    async waitForTransaction(transactionHash: string, confirmations?: number, timeout?: number): Promise<TransactionReceipt> {
        return this._waitForTransaction(transactionHash, (confirmations == null) ? 1: confirmations, timeout || 0, null);
    }

@devdomsos
Copy link

devdomsos commented Feb 21, 2022

Hey Ricmoo, @ricmoo

thanks for an amazing support for the speeded-up transaction.

I am right now testing the functionality on Polygon. Maybe I am doing sth wrong but I have followed your explanation here:
https://blog.ricmoo.com/highlights-ethers-js-may-2021-2826e858277d

Updated the version of ethers.js to 5.5.4 to have the support for replaced transactions.

I have the following pattern:

I have a handleDepositCallback function that calls the contract. That function returns response object that has the wait method. Then on the front-end, I have the following

const handleModalConfirm = useCallback(async () => {
   
    if (handleDepositCallback) {
      const tx = await handleDepositCallback()
      try {
        // Wait for the transaction to be mined
        const hashish = await tx?.hash

        console.debug('deposited', { hashish })
        onSetDepositHash(hashish, sourceChainId)

        const result = await tx?.wait();
     
        return result

      } catch (error) {

        console.debug(error, 'error in the first catch wait await')

        if (error.code === Logger.errors.TRANSACTION_REPLACED) {
          if (error.cancelled) {
            // The transaction was replaced  :'(
            console.debug(error.replacement, 'The transaction was replaced  ')
        
          } else {
            // The user used "speed up" or something similar
            // in their client, but we now have the updated info
            console.debug(error.replacement, 'error.replacement');
            console.debug(error.receipt, 'error.receipt')
         
          }
        }
      }

Then using MetaMask, I submit a transaction:

  1. First transaction goes with Low gas fee
  2. After a second or 2 I use MM feature to speed up the transaction using custom fee.
  3. the first transaction does not go through : https://polygonscan.com/tx/0x0b52f9b3c1e25916ea23b86118266dccfbc30dddaa2fbcc0d8fed1c7eecd9182
  4. the 2nd gets mined: https://polygonscan.com/tx/0xd673066a971ed4b383d6ef88b19fc8e017ce6568809877d60e4629b6f5c4f196

In such case, should we get to the catch() block?

@ricmoo
Copy link
Member Author

ricmoo commented Feb 21, 2022

Yes, you should get the catch block. Are you not?

@devdomsos
Copy link

devdomsos commented Feb 21, 2022

No, it never gets to the } catch (error) {. I have tested it with a single transaction (not speeded-up), and the code properly goes to:

const result = await tx?.wait();

and returns the transaction details with receipt.

It could be that I am doing sth wrong, therefore I prefer to check with you.

I have only tested it on Polygon. I will try also on BSC.

@ricmoo
Copy link
Member Author

ricmoo commented Feb 21, 2022

There are definitely problems with both Polygon and BSC when it comes to receipts, that are still being tracked down; they seem to emit data before they complete indexing (and therefore responding to requests).

So, the result.transactionHash != tx.hash? Are either left undefined? Do the nonce and from match?

@Velenir
Copy link

Velenir commented Feb 21, 2022

When submitting a low-priced transaction on Polygon I often encounter that resulting TransactionResponse.from === "0x000... , zero address. Polygonscan shows the same zero address.

In that case replacement tx won't be detected.
In my experience this issue only happens with public rpc, private nodes are more reliable.

@devdomsos may this be your issue?

@devdomsos
Copy link

devdomsos commented Feb 21, 2022

There are definitely problems with both Polygon and BSC when it comes to receipts, that are still being tracked down; they seem to emit data before they complete indexing (and therefore responding to requests).

So, the result.transactionHash != tx.hash? Are either left undefined? Do the nonce and from match?

Yes, result.transactionHash != tx.hash
In both cases hashes are defined. The hash is generated immediately for both txs (not mined and mined).
But if I try to resolve a promise from an un-mined transactions hash (so the first transaction) I get undefined.
Do the nonce and from match? --> I have the problem with nonce and from. On Polygon, so far I have been getting the mined transaction via polygonscan API. There I get an object like this:

{
blockNumber: '25056381', 
timeStamp: '1645120639', 
hash: '0x80ba412a261313a74b96a93c16a529c6b625c2942343464da354d436a1c49e3b', 
nonce: '122', 
blockHash: '0xc47eb987ac11b076962a85f840cd09352c5de30f7e221baed37167b52388e712', …}
blockHash: "0xc47eb987ac11b076962a85f840cd09352c5de30f7e221baed37167b52388e712"
blockNumber: "25056381"
confirmations: "21873"
contractAddress: ""
cumulativeGasUsed: "10154133"
from: "0xc33c31ff0b6c13c643b80d6c63c8a44e507dd273"
gas: "105644"
gasPrice: "34578977251"
gasUsed: "90014"
hash:"0x80ba412a261313a74b96a93c16a529c6b625c2942343464da354d436a1c49e3b"
input: "0x178a8a9800000000000000000000000092868a5255c628da08f550a858a802f5351c522300000000000000000000000000000000000000000000000000005af3107a4000000000000000000000000000c33c31ff0b6c13c643b80d6c63c8a44e507dd2730000000000000000000000000000000000000000000000000000000000000038"isError: "0"
nonce: "122"
timeStamp: "1645120639"
to: "0xcbce172d7af2616804ab5b2494102daec47b2635"
transactionIndex: "36"
txreceipt_status: "1"
value: "0"[[Prototype]]:
}

That is a mined transaction.

The not-mined transaction object looks like this:

{hash: '0x773c58c16ce4be464fae08a266910e54919824d7bd49903094f5a28360321f07',
chainId: null

data: null

from: null

gasLimit: null

gasPrice: null

hash: "0x773c58c16ce4be464fae08a266910e54919824d7bd49903094f5a28360321f07"

nonce: null

value: null

wait: confirmations => {…}
}

As you can see both nonce and from are null from the 1st (not-mined) tx.
And so I cannot compare nonce and from of the 1st transactionObject and then the 2nd (final) transactionObject that I get from polygonscan API.

@Velenir, I have null for the 'from' key on Polygon. I have tried 2 RPC urls, both were public. Which nodes would you recommend I use?

@Velenir
Copy link

Velenir commented Feb 22, 2022

Which nodes would you recommend I use?

I had good results with Infura, quicknode is another good candidate

@devdomsos
Copy link

@Velenir thank you. I will have to use these then.

@Velenir However, I would be interested in the following: is the new support for replaced transactions provided by ethers library working for you?

@Velenir
Copy link

Velenir commented Feb 22, 2022

However, I would be interested in the following: is the new support for replaced transactions provided by ethers library working for you?

Yes, all in all it's working. Not working only in the case of:

  1. On polygon with public rpc
  2. And I purposefully send a low-priced transaction

@devdomsos
Copy link

However, I would be interested in the following: is the new support for replaced transactions provided by ethers library working for you?

Yes, all in all it's working. Not working only in the case of:

  1. On polygon with public rpc
  2. And I purposefully send a low-priced transaction

@Velenir thank you for the info. How would you recommend I test then the transaction replacement on Polygon? I would then have to wait for the network to be congested to test this? On BSC I am not even able to in time upspeed the transaction. It goes through right away.

@Velenir
Copy link

Velenir commented Feb 23, 2022

How would you recommend I test then the transaction replacement on Polygon?

If you use a private node, then you should be able to send an intentionally underpriced transaction and it would be detected alright.
If you use a public rpc, you need to be fast and get lucky.

@devdomsos
Copy link

devdomsos commented Feb 23, 2022

How would you recommend I test then the transaction replacement on Polygon?

If you use a private node, then you should be able to send an intentionally underpriced transaction and it would be detected alright. If you use a public rpc, you need to be fast and get lucky.

For POLYGON
I changed the node to a private one from Infura for Polygon - getting the same result as with using public rpc. Meaning I never get to the catch block.

For BSC
Well, then it is what it is. :D

Could you share with us the way you call the smart contract and how you await for the wait() promise? Only if you are ok with sharing this of course. Maybe I am doing something wrong on my end.

@GregTheGreek
Copy link

Can we standardize this as EIP-2831?

@aysuio
Copy link

aysuio commented Mar 3, 2022

Hey @devdomsos, thanks for the sample code. I'm using typescript and struggling to handle the error from tx.wait(). Is there way to assign error message type to the error so that I know what I'm dealing with during coding? Here is an example:

 try{
  await txResp.wait(1)
} catch ( error: CertainErrorType ) {
  // do sth
}

What the "CertainErrorType " should be?

@devdomsos
Copy link

devdomsos commented Mar 11, 2022

@aysuio, not sure if you found an answer to it, but generally I was using template code from this update:
https://blog.ricmoo.com/highlights-ethers-js-may-2021-2826e858277d

 } catch (error) {

        console.debug(error, 'error in the first catch wait await')

        if (error.code === Logger.errors.TRANSACTION_REPLACED) {
          if (error.cancelled) {
            // The transaction was replaced  :'(
            console.debug(error.replacement, 'The transaction was replaced  ')
        
          } else {
            // The user used "speed up" or something similar
            // in their client, but we now have the updated info
            console.debug(error.replacement, 'error.replacement');
            console.debug(error.receipt, 'error.receipt')
         
          }
        }

Let me know if you managed to test this feature on your end. :) I still cannot get it to work because I cannot test it.

@aysuio
Copy link

aysuio commented Mar 29, 2022

@aysuio, not sure if you found an answer to it, but generally I was using template code from this update: https://blog.ricmoo.com/highlights-ethers-js-may-2021-2826e858277d

 } catch (error) {

        console.debug(error, 'error in the first catch wait await')

        if (error.code === Logger.errors.TRANSACTION_REPLACED) {
          if (error.cancelled) {
            // The transaction was replaced  :'(
            console.debug(error.replacement, 'The transaction was replaced  ')
        
          } else {
            // The user used "speed up" or something similar
            // in their client, but we now have the updated info
            console.debug(error.replacement, 'error.replacement');
            console.debug(error.receipt, 'error.receipt')
         
          }
        }

Let me know if you managed to test this feature on your end. :) I still cannot get it to work because I cannot test it.

tried TRANSACTION_REPLACED, it generated error params as stated in ethers.js enum ErrorCode

There are bunch of other codes like UNPREDICTABLE_GAS_LIMIT, which are well defined in the ErrorCode.

@devdomsos
Copy link

@aysuio, not sure if you found an answer to it, but generally I was using template code from this update: https://blog.ricmoo.com/highlights-ethers-js-may-2021-2826e858277d

 } catch (error) {

        console.debug(error, 'error in the first catch wait await')

        if (error.code === Logger.errors.TRANSACTION_REPLACED) {
          if (error.cancelled) {
            // The transaction was replaced  :'(
            console.debug(error.replacement, 'The transaction was replaced  ')
        
          } else {
            // The user used "speed up" or something similar
            // in their client, but we now have the updated info
            console.debug(error.replacement, 'error.replacement');
            console.debug(error.receipt, 'error.receipt')
         
          }
        }

Let me know if you managed to test this feature on your end. :) I still cannot get it to work because I cannot test it.

tried TRANSACTION_REPLACED, it generated error params as stated in ethers.js enum ErrorCode

There are bunch of other codes like UNPREDICTABLE_GAS_LIMIT, which are well defined in the ErrorCode.

@aysuio, how did you manage to re-produce the replaced transaction? Which blockchain were you testing it on? Thanks!

@tiff0000
Copy link

tiff0000 commented Nov 2, 2022

When you get TRANSACTION_REPLACED, how can you get the new transaction hash that of the replacement?

@ricmoo
Copy link
Member Author

ricmoo commented Nov 2, 2022

@tiff0000

The replacement property on the error has the entire tx including the hash. See https://docs.ethers.io/v5/api/utils/logger/#errors--transaction-replaced . :)

@GregTheGreek
Copy link

GregTheGreek commented Nov 2, 2022

@ricmoo Is ethers just polling the node under the hood to grab all the information?

@ricmoo
Copy link
Member Author

ricmoo commented Nov 2, 2022

@GregTheGreek Depending on the backend, it uses the block listener to then get the nonce; if the nonce has changed, then it searches the block for the tx matching the nonce and from, to derive the replacement reason (replaced, cancelled or repriced).

@bilaljawed
Copy link

@ricmoo Question:

I dont have a TransactionReceipt object as i am getting the hash from metamask from frontend to backend, If i go another way by passing transaction hash to waitForTransaction(hash) is it going to act the same way as tx.wait() ?

Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or improvement. fixed/complete This Bug is fixed or Enhancement is complete and published. minor-bump Planned for the next minor version bump.
Projects
None yet
Development

No branches or pull requests

8 participants