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

CANCUN hard fork impelemtation #39

Merged
merged 22 commits into from
Jun 11, 2024
Merged

CANCUN hard fork impelemtation #39

merged 22 commits into from
Jun 11, 2024

Conversation

mrLSD
Copy link
Member

@mrLSD mrLSD commented Mar 29, 2024

Description

📚 Deneb/Cancun Ethereum specification: https://eips.ethereum.org/EIPS/eip-7569

This PR includes implementation:
➡️ EIP-1153: Transient storage opcodes
➡️ EIP-4844: Shard Blob Transactions
➡️ EIP-5656: MCOPY - Memory copying instruction
➡️ EIP-6780: SELFDESTRUCT only in same transaction
➡️ EIP-7516: BLOBBASEFEE opcode

Improvements

➡️ Refactored evm-tests to 100% of ethereum-jsob-tests pass for:

  • Shanghai hardfork
  • Cancun hardfork

➡️ Fixed bugs in evm-tests
➡️ Fixed bugs in evm

Incudes

@mrLSD mrLSD self-assigned this Mar 29, 2024
@mrLSD mrLSD added hard-fork enhancement New feature or request labels Mar 29, 2024
@mrLSD mrLSD marked this pull request as ready for review April 5, 2024 11:09
///
/// This function panics if `denominator` is zero.
#[inline]
pub fn fake_exponential(factor: u64, numerator: u64, denominator: u64) -> u128 {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I worry about overflow in this function.

The implementation in the EIP is written in Python which uses arbitrary precision integers. The geth implementation uses arbitrary precision integers as well.

The Erigon implementation uses 256-bit integers, but still checks for overflow and returns an error in that case. I think we should follow this example and also use U256 with overflow-checked arithmetic.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The source is here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But I agree, it seems like should be refactored.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually... this function can be removed. As it's for test KZG-precompiles

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure if it's not needed then please remove.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done: 12b1dd0

src/executor/stack/memory.rs Outdated Show resolved Hide resolved
runtime/src/eval/system.rs Outdated Show resolved Hide resolved
src/executor/stack/executor.rs Outdated Show resolved Hide resolved
runtime/src/handler.rs Outdated Show resolved Hide resolved
core/src/utils.rs Outdated Show resolved Hide resolved
core/src/utils.rs Outdated Show resolved Hide resolved
@mrLSD mrLSD mentioned this pull request Apr 10, 2024
@mrLSD mrLSD mentioned this pull request May 16, 2024
@mrLSD mrLSD mentioned this pull request May 27, 2024
mrLSD added 2 commits May 28, 2024 12:04
* Extend tests output and refactore tests runner

* Refactored CLI for recursive tests and extended information

* Set Shanghai spec as default

* Extend output for tests

* evm-tests: extend state statistics for failed results

* evm-tests: extended debug info

* Extend gas cost analyzer

* Extend tests and fixes for Cancun hard fork

* Fix: Apply CREATE storage reset - changed logic. GAS cost for MCOPY - improved calculation

* EIP-3607 implementation. DeepCall bug fixes. KZG-boilerplate

* KZG precompile

* Removev debug info

* Fix: tests for EIP-3860 (#41)

undefined

* ForkSpec string error. Refactored usage as constantn USIZE_MAX

* Remove already fixed tests from skipping list

* Added KzgInput
* Extend tests output and refactore tests runner

* Refactored CLI for recursive tests and extended information

* Set Shanghai spec as default

* Extend output for tests

* evm-tests: extend state statistics for failed results

* evm-tests: extended debug info

* Extend gas cost analyzer

* Extend tests and fixes for Cancun hard fork

* Fix: Apply CREATE storage reset - changed logic. GAS cost for MCOPY - improved calculation

* EIP-3607 implementation. DeepCall bug fixes. KZG-boilerplate

* KZG precompile

* EIP-3860 test flow fixes

* Fix Shanghai tests

* Removev debug info

* Separate check_exit_reason func

* Remove printing

* Added `as_deref`  to  check_create_exit_reason

* Fix: tests for EIP-3860 (#41)

undefined

* ForkSpec string error. Refactored usage as constantn USIZE_MAX

* Remove already fixed tests from skipping list

* Refactore blob-hash logic and tests

* Added KzgInput

* Gas price fix and investigations for blob-transactions

* Refactored skipped-match and clippy

* Refactored gas price and should-skip logic

* Fix tests for KZG-precompiles and SSTORE gas cost

* Added print-debug feature

* Fix randomness validation

* Refactore transactions validation

* Extend expected check for call-transaction and empty-create assertion

* Edit doc comments
birchmd
birchmd previously approved these changes Jun 5, 2024
Copy link
Member

@birchmd birchmd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for all your work on this large feature!

///
/// This function panics if `denominator` is zero.
#[inline]
pub fn fake_exponential(factor: u64, numerator: u64, denominator: u64) -> u128 {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure if it's not needed then please remove.

aleksuss
aleksuss previously approved these changes Jun 10, 2024
Copy link
Member

@aleksuss aleksuss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall looks good 👍🏻

core/src/memory.rs Outdated Show resolved Hide resolved
core/src/memory.rs Outdated Show resolved Hide resolved
evm-tests/ethjson/src/spec/builtin.rs Show resolved Hide resolved
evm-tests/jsontests/src/main.rs Show resolved Hide resolved
evm-tests/jsontests/src/main.rs Outdated Show resolved Hide resolved
src/backend/memory.rs Show resolved Hide resolved
@mrLSD mrLSD merged commit f737dd8 into master Jun 11, 2024
3 checks passed
@mrLSD mrLSD deleted the hard-fork/cancun branch June 11, 2024 16:27
github-merge-queue bot pushed a commit to aurora-is-near/aurora-engine that referenced this pull request Jun 13, 2024
## Description

➡️ Added **EVM CANCUN** hard fork support. Based on SputnikVM release
[v0.42.0-aurora](https://github.com/aurora-is-near/sputnikvm/releases/tag/v0.42.0-aurora)
➡️  Changed `engine-tests` that related to **CACUN** changes
➡️  Solidity: updated to `0.8.25` which support **CANCUN** Features
➡️  Extended Transactions fields for **CANCUN** hard fork requirements

### Cancun hard fork

➡️ EVM impelentation of Cancun hard fork:
aurora-is-near/sputnikvm/pull/39

### Breaking changes

➡️ Transaction extended according to
[EIP-4844](https://eips.ethereum.org/EIPS/eip-4844) requirements

### Gas cost
Gas cost changed as expected insignificantly.
- 2 tests: gas increased to `1 TGas`
- 1 test: gas decreased to `1 TGas`
aleksuss pushed a commit to aurora-is-near/aurora-engine that referenced this pull request Oct 10, 2024
## Description

➡️ Added **EVM CANCUN** hard fork support. Based on SputnikVM release
[v0.42.0-aurora](https://github.com/aurora-is-near/sputnikvm/releases/tag/v0.42.0-aurora)
➡️  Changed `engine-tests` that related to **CACUN** changes
➡️  Solidity: updated to `0.8.25` which support **CANCUN** Features
➡️  Extended Transactions fields for **CANCUN** hard fork requirements

### Cancun hard fork

➡️ EVM impelentation of Cancun hard fork:
aurora-is-near/sputnikvm/pull/39

### Breaking changes

➡️ Transaction extended according to
[EIP-4844](https://eips.ethereum.org/EIPS/eip-4844) requirements

### Gas cost
Gas cost changed as expected insignificantly.
- 2 tests: gas increased to `1 TGas`
- 1 test: gas decreased to `1 TGas`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request hard-fork
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants