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.

Sign up for GitHub

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

Jump to bottom

Add block.prevrandao as alias for block.difficulty #13759

Merged
merged 1 commit into from
Jan 25, 2023
Merged

Add block.prevrandao as alias for block.difficulty #13759

merged 1 commit into from
Jan 25, 2023

Conversation

r0qs
Copy link
Member

@r0qs r0qs commented Nov 29, 2022 •
edited
Loading

Supersedes #13531. Fixes #13512.

This PR is an attempt to finish the work started in #13531. It reuses part of the solution proposed there but ignores the renaming of the Instruction enum.

As both opcodes share the same bytecode for different EVMs, the EVM version determine whether the compiler outputs one or the other in the instructionInfo function. The table below illustrate the expected behavior:

EVM Version Changes
EVM < PARIS difficulty: builtin and reserved identifier (not changed). Not allowed as user-defined identifier.
prevrandao: not reserved. Allowed as user-defined identifier.
EVM >= PARIS difficulty: reserved identifier (warning, behave as prevrandao).
prevrandao: builtin and reserved identifier. Not allowed as user-defined identifier.
EVM >= PARIS (0.9.0) difficulty: not reserved. Allowed as user-defined identifier.
prevrandao: builtin and reserved identifier. Not allowed as user-defined identifier.

block.difficulty and block.prevrandao are allowed in all versions with their appropriate warnings.

ekpyron
ekpyron reviewed Nov 29, 2022
View reviewed changes
liblangutil/EVMVersion.cpp Outdated Show resolved Hide resolved
ekpyron
ekpyron reviewed Nov 29, 2022
View reviewed changes
libevmasm/AssemblyItem.cpp Outdated Show resolved Hide resolved
ekpyron
ekpyron reviewed Nov 29, 2022
View reviewed changes
libevmasm/Assembly.cpp Outdated Show resolved Hide resolved
@ekpyron
Copy link
Member

ekpyron commented Nov 29, 2022 •
edited
Loading

I only had a quick look over this so far, but this looks generally good! I think we can still reduce the number of cases, in which we have to drag along the evm version a bit and switch between passing it along and storing it in some cases, but most of it looks good already!

Testing failures seem mostly or entirely SMT related bugs independent of this PR.

ekpyron
ekpyron reviewed Nov 29, 2022
View reviewed changes
libevmasm/AssemblyItem.cpp Outdated Show resolved Hide resolved
@r0qs r0qs force-pushed the prevrandao branch 2 times, most recently from 71d2abf to 553aae5 Compare December 1, 2022 00:20
@ekpyron ekpyron mentioned this pull request Dec 1, 2022
Closed
axic
axic reviewed Dec 2, 2022
View reviewed changes
Changelog.md Outdated Show resolved Hide resolved
axic
axic reviewed Dec 2, 2022
View reviewed changes
docs/cheatsheet.rst Outdated Show resolved Hide resolved
matheusaaguiar
matheusaaguiar reviewed Dec 7, 2022
View reviewed changes
Copy link
Collaborator

@matheusaaguiar matheusaaguiar left a comment

Choose a reason for hiding this comment

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

Some suggestions and questions. Haven't gone through everything yet.

Changelog.md Outdated Show resolved Hide resolved
Changelog.md Outdated Show resolved Hide resolved
libevmasm/PeepholeOptimiser.cpp Outdated Show resolved Hide resolved
libevmasm/PeepholeOptimiser.cpp Outdated Show resolved Hide resolved
libsolidity/analysis/TypeChecker.cpp Outdated Show resolved Hide resolved
@r0qs r0qs force-pushed the prevrandao branch 2 times, most recently from 34c0beb to 7a2f21d Compare December 19, 2022 14:44
@r0qs r0qs force-pushed the prevrandao branch 4 times, most recently from 9e3c982 to 7b99050 Compare January 4, 2023 13:50
ekpyron
ekpyron reviewed Jan 5, 2023
View reviewed changes
libevmasm/AssemblyItem.cpp Outdated Show resolved Hide resolved
@r0qs r0qs force-pushed the prevrandao branch 3 times, most recently from 9c35c73 to 5f349e2 Compare January 9, 2023 00:51
@stackenbotten
Copy link

There was an error when running chk_coding_style for commit ec44edf6fb5429ca873be91391c62fa4ab38477f:

Error: Trailing whitespace found:
test/cmdlineTests/evmasm_difficulty_pos_paris/output:13:PREVRANDAO PUSH1 0x0 SSTORE STOP INVALID 
test/cmdlineTests/evmasm_difficulty_pre_paris/output:13:DIFFICULTY PUSH1 0x0 SSTORE STOP INVALID 
test/cmdlineTests/evmasm_prevrandao_pos_paris/output:13:PREVRANDAO PUSH1 0x0 SSTORE STOP INVALID 
test/cmdlineTests/evmasm_prevrandao_pre_paris/output:13:DIFFICULTY PUSH1 0x0 SSTORE STOP INVALID

Please check that your changes are working as intended.

@r0qs r0qs force-pushed the prevrandao branch 4 times, most recently from 484c674 to 392dad8 Compare January 18, 2023 16:14
@r0qs r0qs mentioned this pull request Jan 18, 2023
Merged
@ekpyron
Copy link
Member

ekpyron commented Jan 18, 2023 •
edited
Loading

Only some very minor comments and a question left. We can probably merge it soon (we should squash the 66+ commits then, though :-))

nikola-matic
nikola-matic reviewed Jan 19, 2023
View reviewed changes
Copy link
Collaborator

@nikola-matic nikola-matic left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Looks like you missed quite a few of the block.difficulty tests in terms of adding EVMVersion: <paris.

test/libsolidity/semanticTests/state/block_difficulty.sol
test/libsolidity/semanticTests/state/block_difficulty_post_paris.sol
test/libsolidity/smtCheckerTests/bmc_coverage/range_check.sol
test/libsolidity/smtCheckerTests/special/block_vars_bmc_internal.sol
test/libsolidity/smtCheckerTests/special/block_vars_chc_internal.sol
test/libsolidity/smtCheckerTests/special/difficulty.sol
test/libsolidity/smtCheckerTests/special/many.sol
test/libsolidity/smtCheckerTests/special/many_internal.sol
test/libsolidity/smtCheckerTests/special/range_check.sol
test/libsolidity/smtCheckerTests/special/tx_data_immutable.sol
test/libsolidity/smtCheckerTests/special/tx_data_immutable_fail.sol
test/libsolidity/syntaxTests/types/magic_block.sol
test/libsolidity/syntaxTests/types/magic_block_istanbul.sol

Unless you wanna fix those up in #13883?

@r0qs
Copy link
Member Author

r0qs commented Jan 19, 2023 •
edited
Loading

Looks like you missed quite a few of the block.difficulty tests in terms of adding EVMVersion: <paris.

test/libsolidity/semanticTests/state/block_difficulty.sol
test/libsolidity/semanticTests/state/block_difficulty_post_paris.sol
test/libsolidity/smtCheckerTests/bmc_coverage/range_check.sol
test/libsolidity/smtCheckerTests/special/block_vars_bmc_internal.sol
test/libsolidity/smtCheckerTests/special/block_vars_chc_internal.sol
test/libsolidity/smtCheckerTests/special/difficulty.sol
test/libsolidity/smtCheckerTests/special/many.sol
test/libsolidity/smtCheckerTests/special/many_internal.sol
test/libsolidity/smtCheckerTests/special/range_check.sol
test/libsolidity/smtCheckerTests/special/tx_data_immutable.sol
test/libsolidity/smtCheckerTests/special/tx_data_immutable_fail.sol
test/libsolidity/syntaxTests/types/magic_block.sol
test/libsolidity/syntaxTests/types/magic_block_istanbul.sol

Unless you wanna fix those up in #13883?

Yes, related with the smtCheckerTests, I'm adding them right now in #13883 because SMT tests only run for the default EVM version.

For the test/libsolidity/semanticTests/state/block_difficulty.sol it already run for EVM < paris and there is the block_difficulty_post_paris.sol to test difficulty in EVM > paris as well the block_prevrandao_pre_paris.sol and block_prevrandao.sol for the prevrandao state checks.

For the syntaxTests in questions indeed it is missing the magic_block for prevrandao and EVM >= paris. I will add it.

ekpyron
ekpyron reviewed Jan 23, 2023
View reviewed changes
libyul/AsmAnalysis.cpp Outdated Show resolved Hide resolved
ekpyron
ekpyron previously approved these changes Jan 23, 2023
View reviewed changes
Copy link
Member

@ekpyron ekpyron left a comment

Choose a reason for hiding this comment

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

I'm approving this - but we should squash the commits before merging. A single commit should suffice :-).

@r0qs r0qs requested a review from ekpyron January 23, 2023 18:48
@r0qs @axic
@ekpyron @matheusaaguiar @nikola-matic
Adds support for the EVM version "Paris".
ef6ff2f 
Deprecates `block.difficulty` and disallow `difficulty()` in inline assembly for EVM versions >= paris.
The change is due to the renaming introduced by EIP-4399 (see: https://eips.ethereum.org/EIPS/eip-4399).
Introduces `block.prevrandao` in Solidity and `prevrandao()` in inline assembly for EVM versions >= paris.

Co-authored-by: Alex Beregszaszi <alex@rtfs.hu>
Co-authored-by: Daniel <daniel@ekpyron.org>
Co-authored-by: matheusaaguiar <95899911+matheusaaguiar@users.noreply.github.com>
Co-authored-by: Nikola Matić <nikola.matic@ethereum.org>
@r0qs r0qs mentioned this pull request Jan 23, 2023
Open
aarlt
aarlt reviewed Jan 25, 2023
View reviewed changes
libevmasm/GasMeter.h
@@ -221,7 +221,7 @@ class GasMeter

/// @returns gas costs for simple instructions with constant gas costs (that do not
Copy link
Member

Choose a reason for hiding this comment

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

Is this comment still correct? I mean we have now the EVM version as a parameter here - so something may be different based on that information, right?

Copy link
Member

Choose a reason for hiding this comment

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

Ah ok, maybe not - gas costs seem to be still the same

@ekpyron ekpyron merged commit fd9ac9a into develop Jan 25, 2023
@ekpyron ekpyron deleted the prevrandao branch January 25, 2023 15:37
@Stasian007

This comment was marked as spam.

Sign in to view
@aarlt aarlt mentioned this pull request Jan 27, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@axic axic axic left review comments

@nikola-matic nikola-matic nikola-matic left review comments

@aarlt aarlt aarlt left review comments

@stackenbotten stackenbotten stackenbotten left review comments

@matheusaaguiar matheusaaguiar matheusaaguiar left review comments

@ekpyron ekpyron ekpyron approved these changes

@chriseth chriseth Awaiting requested review from chriseth

@cameel cameel Awaiting requested review from cameel

Assignees

@r0qs r0qs

Labels
None yet
Projects
Solidity Focus Board
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

feat: introduce block.prevrandao as alias for block.difficulty
9 participants
@r0qs @ekpyron @stackenbotten @Stasian007 @axic @cameel @nikola-matic @aarlt @matheusaaguiar