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

Contract methods /w Metamask do not resolve Tx object until mined #372

Closed
sudoryan opened this issue Dec 11, 2018 · 26 comments
Closed

Contract methods /w Metamask do not resolve Tx object until mined #372

sudoryan opened this issue Dec 11, 2018 · 26 comments
Assignees

Comments

@sudoryan
Copy link

According to the docs, contract methods sends are supposed to resolve to a tx object when the transaction is sent. This isn't the case when using Metamask web3 provider where the contract method only resolves once mined.

const provider = new ethers.providers.Web3Provider(window.web3.currentProvider);
const signer = provider.getSigner()
const contractWithSigner = contract.connect(signer);

contractWithSigner.fn().then(tx => console.log(tx.hash)); // not resolved until mined
@sudoryan sudoryan reopened this Dec 12, 2018
@ricmoo
Copy link
Member

ricmoo commented Dec 12, 2018

This is, I think, an issue with MetaMask and a delay between submitting a transaction to the network and making it available for fetching, but it is something that would be nice to properly support.

I will investigate and see if introducing a short delay (a fast-check) to the polling helps. It may make sense for there to be a MetaMaskProvider, with the little quirks ironed out too.

@ricmoo ricmoo self-assigned this Dec 12, 2018
@ricmoo ricmoo added investigate Under investigation and may be a bug. on-deck This Enhancement or Bug is currently being worked on. labels Dec 12, 2018
@sudoryan
Copy link
Author

Sometimes it resolves the tx object right away and sometimes it needs to be mined.

@ricmoo
Copy link
Member

ricmoo commented Dec 14, 2018

@sudoryan I'm trying to reproduce this now. What percentage of the time are you seeing it not work?

@sudoryan
Copy link
Author

It's weird, when it works - it works. When it doesn't - it doesn't. Right now I can't get it working I've sent 10+ transactions and they don't return the hash. I tried restarting browser, relogging metamask, clearing cache unable to get it working.

@ricmoo
Copy link
Member

ricmoo commented Dec 14, 2018

LOL! I have the opposite problem, I can't get it to fail...

@sudoryan
Copy link
Author

I had to reinstall Metamask! Sorry about that, thanks @ricmoo.

@ricmoo
Copy link
Member

ricmoo commented Dec 21, 2018

Good to know though. I guess MetaMask can just get in a weird state, but now I know to recommend that in the future.

Thanks! :)

@ricmoo ricmoo removed the on-deck This Enhancement or Bug is currently being worked on. label Feb 11, 2019
@lnbc1QWFyb24
Copy link

Hey @ricmoo , first thanks for a fantastic library! I am getting the same issue above consistently. I have tried re-installing MetaMask and have tested on other machines with other users. Any ideas on what could be causing this? I have not been able to get a transaction to resolve as soon as it is approved within MetaMask, it always resolves after 1 confirmation.

@ricmoo
Copy link
Member

ricmoo commented May 13, 2019

Hmmm.. This may be new behaviour? What version are you using? Can you include the code you are using?

Basically, the way ethers handles this (only for JSON-RPC API based providers), is once a transaction is sent, it gets back a transaction hash, which it then polls using getTransaction, which should return the pending transaction. Some backends do not return a transaction to getTransaction until it is mined, which will cause this issue. If you do not care about the contents of the transaction, you can see this solution for a way to get by faster. In v5 this is exposed by the built-in library for those using a JsonRpcSigner when they just want a fast result, and don't care what was actually sent.

@lnbc1QWFyb24
Copy link

I have tried v4 via the CDN and I have also tried building from the repo.

const provider = new ethers.providers.Web3Provider(window.ethereum) // have also tried window.web3.currentProvider
const contract = new ethers.Contract(address, abi, provider.getSigner())

contract.myMethod(someParameter).then(result => console.log(result)) // doesn't log until tx has 1 confirmation

I swear this worked a month or so ago, so I am not sure if it is a recent change with MM?

@ricmoo
Copy link
Member

ricmoo commented May 13, 2019

I could be either a change in MetaMask, or possibly a change in INFURA. I will reach out to them this week if this persists. I'll put together a short sample too. I don't really use MetaMask very often, so I don't see these problems very often. :s

@lnbc1QWFyb24
Copy link

Awesome, thanks mate. I will try running some older builds of MetaMask and see if I can find out if it was a change on their end.

@lnbc1QWFyb24
Copy link

@ricmoo I did some digging on this. The transaction hash is getting returned correctly from the sendTransaction call via the MetaMask provider:

Screen Shot 2019-05-24 at 1 48 01 pm

This log consistently logs the hash immediately after the transaction is approved within MetaMask.

Should sendTransaction be returning a object that has the hash and the wait method on it here instead?

Currently it is returning the result of a call to getTransaction which takes the hash and returns tx receipt that has been wrapped with this._wrapTransaction if I understand correctly and this is why it is only resolving with the receipt and not with the hash and then the receipt after calling wait.

@lnbc1QWFyb24
Copy link

@ricmoo I have a possibly naive solution (I am not super familiar with Typescript) for this that is working for me nicely. It is just a small change to the sendTransaction method in json-rpc-provider.ts:

sendTransaction(transaction: TransactionRequest): Promise<any> {
        return this.sendUncheckedTransaction(transaction).then((hash) => {
            return {
              hash,
              wait: (confirmations?: number) => {    
                return this.provider.waitForTransaction(hash, confirmations)
              }
            }
        })
    }

Happy to put together a PR if you think this is a good path to take or if you want to point me in a better direction?

@ricmoo
Copy link
Member

ricmoo commented May 24, 2019

I think you are trying to implement the UncheckedJsonRpcSigner? If so, here is the full implementation from v5:

https://github.com/ethers-io/ethers.js/blob/ethers-v5-beta/packages/providers/src.ts/json-rpc-provider.ts#L187

It cannot be put into the normal JsonRpcSigner though, since non-MetaMask JSON-RPC backends work fine with the normal JsonRpcSigner...

@lnbc1QWFyb24
Copy link

Yes that would work nicely. Although the problem is that I am trying to find a solution that will be merged in to ethers.js in a future release so that a library I am working on can support ethers.js . Our library requires the transactionHash after the user has confirmed the transaction so that our transaction notifications work correctly.
Is there any chance that you will modify the web3Provider to use the UncheckedJsonRpcSigner in the next release?

@ricmoo
Copy link
Member

ricmoo commented May 27, 2019

Yes, it is already supported in v5 beta. :)

There are 2 ways to get an UncheckedJsonRpcSigner. If you have a JsonRpcSigner already, then signer.connectUnchecked() is what you want. If you have a JsonRpcProvider, then provider.getUncheckedSigner(addressOrIndex) is for you. :)

Of course, addressOrIndex is optional and defaults to 0. You can include the same solution in v4 using the similar code included in the issue.

Keep in mind the Web3Provider is just the JsonRpcProvider extended to override send, so it has all the properties of a JsonRpcProvider.

@NoahZinsmeister
Copy link

Just wanted to jump in and say that this same issue (with MM + ethers) also hit me recently, and I'm 95% sure that nothing changed on my end, so I guess it's likely to be a MM/Infura issue. Going to look into the unchecked signer stuff.

@lnbc1QWFyb24
Copy link

@ricmoo Thanks I will see if I can the unchecked signer working 👍

@lnbc1QWFyb24
Copy link

@ricmoo Do you have a rough estimate of when you are planning on releasing v5?

@ricmoo
Copy link
Member

ricmoo commented May 29, 2019

The public beta is available now. Basically once I stop getting complaints and bug reports, it will get promoted from next to latest. :)

I’ll be making a less-soft, more public announcement for it soon, so I expect more bug reports to flow in after that. :)

@lnbc1QWFyb24
Copy link

Awesome thanks mate! 👌

@Destiner
Copy link

Destiner commented Jun 13, 2019

Adding my 2 cents. I'm having the same problem, where signer.sendTransaction doesn't return anything until it's mined (and according to the TransactionResponse, it waits for 2 confirmations) on Metamask. Was blaming changes in my code, but then tried old version that were live for days, which had the same issue.

Seems like we now should expect that behavior for both Web3Provider and JsonRpcProvider, so hopefully there will be an easy way in ethers.js to get tx hash quickly.

UPD: can't sendTransaction just return the hash by default? You can use waitForTransaction to get data about tx anyway. Seems like a nice dev UX and a neat separation of concerns. I believe most of the folks use sendTransaction simply to get hash anyway, and that "time to mine" delay degrades application quality and sometimes even breaks part of the functionality.

UPD 2: This seems relevant.

@deacix
Copy link

deacix commented Jun 18, 2019

Hi everyone! We have now the same problem!
How can we solve it?

@ricmoo
Copy link
Member

ricmoo commented Jun 19, 2019

Please help out by following this issue instead. :)

#544

@deacix
Copy link

deacix commented Jun 22, 2019

Hi ricmoo,

4.0.31 is still not workin

@ricmoo ricmoo removed the investigate Under investigation and may be a bug. label Oct 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants