search

5afe / safe-react

Deprecated! New repo – https://github.com/safe-global/web-core
MIT License
332 stars 363 forks source link

Gas limit estimation on xDAI is not working #2999

Closed anxolin closed 2 years ago

anxolin commented 2 years ago

Description

Part of #2998

Implement a workaround for the problem with gas estimations for xDAI. This error affects to all operations in xDAO, both for Dapps using Wallet Connect, and Gnosis Safe Apps.

More context on the issue: https://gnosisinc.slack.com/archives/CQB729LLF/p1635940014445400

🚨 NOTE: This issue is very important to us, so it is a blocker to launch the Gnosis Safe in xDAI network. If Gnosis Safe team solves this issue, we could release as planned next week this new integration. If not, we would need to invest some time in disabling Gnosis Safe for xDAI for CowSwap users.

Additionally, this is somewhat related to #3002

Environment

Steps to reproduce

  1. Connect to xDAI
  2. Open SushiSwap App
  3. Try to wrap ETH, when the last signer tries to execute the transaction, they will see a message saying something like: "this transaction will revert, please reject it to save some gas costs". However, if you try to manually edit the gas limit, then it will say it doesn't revert anymore.

Expected result

I believe the issue is because Ethermine has issues estimating the required gas (https://github.com/NethermindEth/nethermind/issues/3577), however I believe while they don't fix the issue, the web could try to overestimate the gas if the web observes that it reverts, and see if it doesn't revert with that overestimation.

This work-around could be reverted after Ethermine solution, and be even be only triggered in xDAI (so its more scoped).

Obtained result

Users think the transaction will revert, and they are encouraged to reject the transactions.

Screenshots

francovenica commented 2 years ago

Tried a few tx and they worked with the gas estimated by default: Tx builder Contract interaction Removing/adding owner Send funds Create a spending limit

Issue: The safe creation message shows that you will need a certain amount of funds to create the safe, but in MM is like 1/3 of what it was shown in the message image

image

katspaugh commented 2 years ago

Fixed ✅

Apparently, it's a long-standing bug: we weren't passing the estimated gas price to the wallet on Safe Creation. We didn't notice before because the wallet's own and our estimations were similar.

francovenica commented 2 years ago

Looks good now.

anxolin commented 2 years ago

Nice! is there an URL where i can test this? Is there an approximated date to release this to production? Just to coordinate on our end, we will need to re-enable xDAI on our end when that happens.

mmv08 commented 2 years ago

Nice! is there an URL where i can test this? Is there an approximated date to release this to production? Just to coordinate on our end, we will need to re-enable xDAI on our end when that happens.

https://safe-team.dev.gnosisdev.com/app

katspaugh commented 2 years ago

@anxolin we're planning to go live with it on Dec 1st.

anxolin commented 2 years ago

Okay, cool, we went then live with xDAI disabled for Gnosis Safe, only Rinkeby and Mainnet are available: https://cowswap.exchange/#/

I'll set the date in the calendar then to enable xDAI. Let us know if there's change of plans. Thanks @katspaugh!