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

Feat: refactore gas charge logic form EVM exit reason #935

Merged
merged 26 commits into from
Aug 27, 2024

Conversation

mrLSD
Copy link
Member

@mrLSD mrLSD commented Jul 5, 2024

Description

To be fully compatible with EVM, we should:
➡️ Success| Revert
➡️ ExitError - Execution errors should charge gas from users
➡️ ExitFatal - shouldn't charge user gas
➡️ Transactions validation errors should not charge user gas

This PR changed the logic for EVM exit reason gas charge for users, which includes EVM execution errors gas charge. Previously transaction just panicked, and user's gas was not charged.

EVM

Related to EVM changes: aurora-is-near/sputnikvm/pull/52

@mrLSD mrLSD self-assigned this Jul 5, 2024
@mrLSD mrLSD requested a review from aleksuss August 13, 2024 15:36
@mrLSD mrLSD added A-testing Area: If something has added tests, or changed them. A-evm-compatibility Area: EVM compatibility changes or fixes. labels Aug 13, 2024
engine-sdk/src/caching.rs Outdated Show resolved Hide resolved
engine/src/engine.rs Outdated Show resolved Hide resolved
engine-tests/src/utils/mod.rs Show resolved Hide resolved
engine-types/src/parameters/engine.rs Outdated Show resolved Hide resolved
## Description

The PR proposes adding some code refactoring to the #935 

## Performance / NEAR gas cost considerations

No performance changes.

## Testing

A regression test for the `TransactionStatus` has been added.
@aleksuss aleksuss requested a review from birchmd August 27, 2024 09:32
@aleksuss aleksuss added this pull request to the merge queue Aug 27, 2024
Merged via the queue into develop with commit c505f58 Aug 27, 2024
24 checks passed
@aleksuss aleksuss deleted the feat/charge-gas-refactoring branch August 27, 2024 14:25
aleksuss added a commit that referenced this pull request Oct 10, 2024
### Description

To be fully compatible with EVM, we should:
:arrow_right:  Success| Revert
:arrow_right:  ExitError - Execution errors should charge gas from users
:arrow_right:  ExitFatal - shouldn't charge user gas
:arrow_right:  Transactions validation errors should not charge user gas

This PR changed the logic for EVM exit reason gas charge for users,
which includes EVM execution errors gas charge. Previously transaction
just panicked, and user's gas was not charged.

#### EVM
Related to EVM changes:  aurora-is-near/sputnikvm/pull/52

---------

Co-authored-by: Oleksandr Anyshchenko <oleksandr.anyshchenko@aurora.dev>
@aleksuss aleksuss mentioned this pull request Oct 10, 2024
aleksuss added a commit that referenced this pull request Oct 10, 2024
### Additions

- Added support of CANCUN hardfork by [@mrLSD]. ([#926])
- Added support of EIP-3607 by [@mrLSD]. ([#930])
- Removed restrictions from funding XCC sub-accounts by [@birchmd].
([#931])

### Changes

- Made some EVM gas costs optimisations by [@mrLSD]. ([#934])
- Refactored the gas charge logic form EVM exit reasons by [@mrLSD].
([#935])
- Updated some dependencies and rust-toolchain by [@mrLSD]. ([#936])
- Removed unused `bytes_to_hex` function by [@dwiekawki]. ([#942])
- Added building of actual version of the `near-sandbox` in the
scheduled CI job by [@aleksuss] ([#950])

### Fixes

- Removed duplicated `test` task in the `README.md` by [@dwiekawki].
([#943])
- Fixed some typos in the `README.md` and `Cargo.toml` by [@DemoYeti].
([#945]) ([#946])
- Fixed exceeded prepaid gas error in the `mirror_erc20_token`
transaction by [@aleksuss] ([#951])
- Modified `hardhat.config.js` to support contract verification by
[@spilin] ([#958])

[#926]: #926
[#930]: #930
[#931]: #931
[#934]: #934
[#935]: #935
[#936]: #936
[#942]: #942
[#943]: #943
[#945]: #945
[#946]: #946
[#950]: #950
[#951]: #951
[#958]: #958

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: Michael Birch <michael.birch@aurora.dev>
Co-authored-by: Evgeny Ukhanov <evgeny@aurora.dev>
Co-authored-by: dwiekawki <176287097+dwiekawki@users.noreply.github.com>
Co-authored-by: dwiekawki <dwiekawki@users.noreply.github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: DemoYeti <164791169+DemoYeti@users.noreply.github.com>
Co-authored-by: spilin <LyoshaKR@gmail.com>
aleksuss added a commit that referenced this pull request Oct 10, 2024
### Additions

- Added support of CANCUN hardfork by [@mrLSD]. ([#926])
- Added support of EIP-3607 by [@mrLSD]. ([#930])
- Removed restrictions from funding XCC sub-accounts by [@birchmd].
([#931])

### Changes

- Made some EVM gas costs optimisations by [@mrLSD]. ([#934])
- Refactored the gas charge logic form EVM exit reasons by [@mrLSD].
([#935])
- Updated some dependencies and rust-toolchain by [@mrLSD]. ([#936])
- Removed unused `bytes_to_hex` function by [@dwiekawki]. ([#942])
- Added building of actual version of the `near-sandbox` in the
scheduled CI job by [@aleksuss] ([#950])

### Fixes

- Removed duplicated `test` task in the `README.md` by [@dwiekawki].
([#943])
- Fixed some typos in the `README.md` and `Cargo.toml` by [@DemoYeti].
([#945]) ([#946])
- Fixed exceeded prepaid gas error in the `mirror_erc20_token`
transaction by [@aleksuss] ([#951])
- Modified `hardhat.config.js` to support contract verification by
[@spilin] ([#958])

[#926]: #926
[#930]: #930
[#931]: #931
[#934]: #934
[#935]: #935
[#936]: #936
[#942]: #942
[#943]: #943
[#945]: #945
[#946]: #946
[#950]: #950
[#951]: #951
[#958]: #958

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: Michael Birch <michael.birch@aurora.dev>
Co-authored-by: Evgeny Ukhanov <evgeny@aurora.dev>
Co-authored-by: dwiekawki <176287097+dwiekawki@users.noreply.github.com>
Co-authored-by: dwiekawki <dwiekawki@users.noreply.github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: DemoYeti <164791169+DemoYeti@users.noreply.github.com>
Co-authored-by: spilin <LyoshaKR@gmail.com>
aleksuss added a commit that referenced this pull request Oct 10, 2024
### Additions

- Added support of CANCUN hardfork by [@mrLSD]. ([#926])
- Added support of EIP-3607 by [@mrLSD]. ([#930])
- Removed restrictions from funding XCC sub-accounts by [@birchmd].
([#931])

### Changes

- Made some EVM gas costs optimisations by [@mrLSD]. ([#934])
- Refactored the gas charge logic form EVM exit reasons by [@mrLSD].
([#935])
- Updated some dependencies and rust-toolchain by [@mrLSD]. ([#936])
- Removed unused `bytes_to_hex` function by [@dwiekawki]. ([#942])
- Added building of actual version of the `near-sandbox` in the
scheduled CI job by [@aleksuss] ([#950])

### Fixes

- Removed duplicated `test` task in the `README.md` by [@dwiekawki].
([#943])
- Fixed some typos in the `README.md` and `Cargo.toml` by [@DemoYeti].
([#945]) ([#946])
- Fixed exceeded prepaid gas error in the `mirror_erc20_token`
transaction by [@aleksuss] ([#951])
- Modified `hardhat.config.js` to support contract verification by
[@spilin] ([#958])

[#926]: #926
[#930]: #930
[#931]: #931
[#934]: #934
[#935]: #935
[#936]: #936
[#942]: #942
[#943]: #943
[#945]: #945
[#946]: #946
[#950]: #950
[#951]: #951
[#958]: #958

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: Michael Birch <michael.birch@aurora.dev>
Co-authored-by: Evgeny Ukhanov <evgeny@aurora.dev>
Co-authored-by: dwiekawki <176287097+dwiekawki@users.noreply.github.com>
Co-authored-by: dwiekawki <dwiekawki@users.noreply.github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: DemoYeti <164791169+DemoYeti@users.noreply.github.com>
Co-authored-by: spilin <LyoshaKR@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-evm-compatibility Area: EVM compatibility changes or fixes. A-testing Area: If something has added tests, or changed them.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants