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

Fix revert handling #48

Merged
merged 8 commits into from
Oct 18, 2024
Merged

Fix revert handling #48

merged 8 commits into from
Oct 18, 2024

Conversation

fadeev
Copy link
Member

@fadeev fadeev commented Oct 15, 2024

  • Revert context now contains correct data
  • Revert amount on EVM should take into account the revert call gas costs
npx hardhat localnet

Using the Hello example:

yarn deploy

The Hello contract expects a string, so we're sending a uint256 to simulate a revert. Passing 1*10^18 ERC-20, and getting 999999999992957809, which is the amount minus the withdraw gas costs

npx hardhat evm-deposit-and-call --amount 1 --receiver 0xE6E340D132b5f46d1e472DebcD681B2aBc16e57E --network localhost --revert-address 0xc3e53F4d16Ae77Db1c982e75a937B9f60FE63690 --erc20 0x0DCd1Bf9A1b36cE34237eEaFef220932846BCD82 --call-on-revert --types '["uint256"]' 42

With gas we're getting back 999999999993000000:

npx hardhat evm-deposit-and-call --amount 1 --receiver 0xE6E340D132b5f46d1e472DebcD681B2aBc16e57E --network localhost --revert-address 0xc3e53F4d16Ae77Db1c982e75a937B9f60FE63690 --call-on-revert --types '["uint256"]' 42
Transaction hash: 0x61b006f1294e5b8a02b86bda86b99b3379766d4f9058d3070a0e020c418c7214

The actual gas costs is 7000000, but with ERC-20 we're getting less, because we're swapping, even though the exchange is 1:1 we're getting slightly less.

@fadeev fadeev changed the title Populate revert context with correct data Fix revert handling Oct 17, 2024
@fadeev fadeev marked this pull request as ready for review October 18, 2024 08:26
@fadeev fadeev merged commit e2ec956 into main Oct 18, 2024
4 checks passed
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

Successfully merging this pull request may close these issues.

2 participants