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

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

Closed
ginlink opened this issue Feb 16, 2022 · 12 comments
Closed

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

ginlink opened this issue Feb 16, 2022 · 12 comments
Labels
area-gas stale issues and PRs marked as stale type-bug

Comments

@ginlink
Copy link

ginlink commented Feb 16, 2022

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
Copy link
Contributor

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
Copy link
Author

ginlink commented Feb 18, 2022

@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
Copy link
Author

ginlink commented Feb 18, 2022

@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
Copy link

devli13 commented Mar 2, 2022

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
Copy link

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
Copy link

devli13 commented Mar 8, 2022

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.

evmos/ethermint#970

evmos/ethermint#968

@loredanacirstea
Copy link

@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
Copy link
Contributor

danjm commented Mar 18, 2022

@loredanacirstea Thanks very much for researching this.

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

@mcmire
Copy link
Contributor

mcmire commented Mar 18, 2022

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:

  • Part of the reason this is probably happening is that if the fallback gas estimate logic (as I mentioned above) is triggered, it will retrieve a massive amount of block information via eth_feeHistory. It is likely that the UI froze because the network took a long time to respond with this data. We are about to issue another release will remove these extra network calls and this should make the UI more responsive.
  • 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. That will prevent the fallback logic from even being triggered and should result in a snappy UI.
  • We should also figure out a way to not block the UI if all of our attempts to fetch gas estimates in various ways fail. This is something we've discussed internally, and there are probably some designs we will need to implement to truly address this, but at the very least, we should probably just allow users to enter a custom max base fee and priority fee and continue.

@loredanacirstea
Copy link

loredanacirstea commented Mar 21, 2022

@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
Copy link
Contributor

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 github-actions bot added the stale issues and PRs marked as stale label Jul 21, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Sep 4, 2023

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.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Sep 4, 2023
@github-project-automation github-project-automation bot moved this to To be fixed in Bugs by severity Apr 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-gas stale issues and PRs marked as stale type-bug
Projects
None yet
Development

No branches or pull requests

8 participants