MetaMask / metamask-extension

:globe_with_meridians: :electric_plug: The MetaMask browser extension enables browsing Ethereum blockchain enabled websites
https://metamask.io
Other
11.95k stars 4.89k forks source link

[Bug]: Blocked while sending a transaction, unable to respond #13649

Closed ginlink closed 1 year ago

ginlink commented 2 years ago

Describe the bug

Blocked while sending a transaction, unable to respond in versions greater than 10.9 , but in version 10.8.2, this problem does not occur chainID: 9000

image

Steps to reproduce

1.switch to evmos network(9000) 2.click send 3.It was blocked after about 3 seconds

Error messages or log output

gas price estimation failed due to network error

Version

10.9.3,10.9.2

Build type

No response

Browser

Chrome, Firefox

Operating system

MacOS

Hardware wallet

No response

Additional context

No response

darkwing commented 2 years ago

Thank you for reporting this @ginlink . I've tried getting test PHOTON from the faucet (https://faucet.evmos.org/) but it's currently failing, so I cannot immediately reproduce. I will check again later today.

ginlink commented 2 years ago

@darkwing Thanks for your reply. Just click on PHOTON coin , click send, and wait about 3 seconds, it will be blocked. Through my tests, other network is ok, such as 1, 56, 97.

ginlink commented 2 years ago

@darkwing Yes, there is something wrong with its faucet these days. If you need an account, I can create one for you and give you the private key.

devli13 commented 2 years ago

Hey guys, I am from the Evmos team and just confirming this bug as we have been trying to work to resolve the past couple days. Would appreciate any thoughts as we have isolated the issue to Metamask specific as it does not reproduce on versions earlier than 10.10.0 and when trying curl commands we don't have any issues.

Something else that was found in the error logs is that this url which is called by metamask fails to return for Evmos: Evmos: https://gas-api.metaswap.codefi.network/networks/9000/suggestedGasFees Ethereum: https://gas-api.metaswap.codefi.network/networks/1/suggestedGasFees

Happy to send funds to anyone in this thread if helpful but faucet should be back up

0x4a5e1e4baab commented 2 years ago

Also looking at this issue and see request locking up at the same time as the UI, the request actually finally get initiated and completes in normal time.

image

devli13 commented 2 years ago

Hey all, updating this thread as we diagnosed the issues (two to be exact) and made the appropriate changes to Ethermint to keep compatibility with Metamask. Seems that changes with 10.10.0's fee estimation caused the issues for us and potentially caused issues for other chains as a result.

https://github.com/tharsis/ethermint/pull/970

https://github.com/tharsis/ethermint/pull/968

loredanacirstea commented 2 years ago

@darkwing , The issue (Metamask popup freezing when trying to do a transaction - like clicking on the send ETH button) is reproducible by returning an eth_feeHistory result with a reward array that contains null values. I tracked the problem down to this line return { ...obj, [percentile]: fromHex(priorityFee) }; https://github.com/MetaMask/controllers/blob/dd48bba89f3ae1a39fa5b170686c84bdf9ce0e3d/src/gas/fetchBlockFeeHistory.ts#L220

const priorityFeesByPercentile = percentiles.reduce(
    (obj, percentile, percentileIndex) => {
      const priorityFee = priorityFeesForEachPercentile[percentileIndex];
      return { ...obj, [percentile]: fromHex(priorityFee) };
    },
    {} as Record<Percentile, BN>,
  );

The fromHex function eventually uses this stripHexPrefix function, which throws an Error if the value is not a string: https://github.com/ethereumjs/ethereumjs-monorepo/blob/58e5e0d059d55965b349c922af0ca6177bcb7f26/packages/util/src/internal.ts#L44-L49

I am not sure what happens next, I can only assume it enters some cycles when trying to stop/restart the polling in the GasFeeController. A badly formatted value should not freeze the UI.

Proposed solution: check that priorityFee is of string type, otherwise use 0x0 (before fromHex). I can help with a PR fix if needed.

danjm commented 2 years ago

@loredanacirstea Thanks very much for researching this.

@mcmire What do you think about the solution proposed here by @loredanacirstea?

mcmire commented 2 years ago

Hi @loredanacirstea,

As you and others have pointed out, when we request gas estimates we first attempt to use an API that looks like https://gas-api.metaswap.codefi.network/networks/{networkId}/suggestedGasFees. If that fails we fall back to computing gas fee information based on eth_feeHistory.

In writing this fallback logic I used geth as a reference for how eth_feeHistory behaves. You can see here that it will account for a lack of information and use 0x0 in place of null. Now that I am revisiting this, I see that this spec also exists. The spec requires that all items in the reward array be numbers.

Therefore, I would rather chains be compatible with the spec rather than MetaMask account for deviations from the spec. The fix to ethermint mentioned above seems like it would resolve this issue, is that correct? If so, I am not sure we would need to make a change to the @metamask/controllers package as you suggested.

That said, coming back to the original issue, I 100% agree that MetaMask shouldn't lock up if there are problems fetching gas estimates, and I think there are a few things that we can do (and have already done) to address this:

loredanacirstea commented 2 years ago

@mcmire, about

In addition, the request to https://gas-api.metaswap.codefi.network/networks/9000/suggestedGasFees should work. This API is run by another team and I will work with them to see what we can do to make this work for Evmos.

What is needed from Evmos's side for this to work? If this is an open-source project, can you provide a link to it? Do you have an OpenAPI spec that we can test against?

github-actions[bot] commented 1 year ago

This issue has been automatically marked as stale because it has not had recent activity in the last 90 days. It will be closed in 45 days if there is no further activity. The MetaMask team intends on reviewing this issue before close, and removing the stale label if it is still a bug. We welcome new comments on this issue. We do not intend on closing issues if they report bugs that are still reproducible. Thank you for your contributions.

github-actions[bot] commented 1 year ago

This issue was closed because there has been no follow up activity in the last 45 days. If you feel this was closed in error, please reopen and provide evidence on the latest release of the extension. Thank you for your contributions.