Introduce `Shanghai` version and `push0` support #14107

hrkrshnn · 2023-04-10T10:04:25Z

Partially implements #14073. The remaining will be done in future PRs.

Introduces version Shanghai.
Assembler supports use of push0 from EVM versions starting at Shanghai.
Basic tests and docs.

See https://eips.ethereum.org/EIPS/eip-3855 Note: `push0` costs 2 gas, i.e., `Tier::Base` unlike other pushes.

github-actions · 2023-04-10T10:04:44Z

Thank you for your contribution to the Solidity compiler! A team member will follow up shortly.

If you haven't read our contributing guidelines and our review checklist before, please do it now, this makes the reviewing process and accepting your contribution smoother.

If you have any questions or need our help, feel free to post them in the PR or talk to us directly on the #solidity-dev channel on Matrix.

ekpyron · 2023-04-11T10:00:22Z

libevmasm/Assembly.cpp

@@ -544,6 +544,7 @@ LinkerObject const& Assembly::assemble() const
 		bytesRequiredIncludingData += static_cast<unsigned>(sub->assemble().bytecode.size());

 	unsigned bytesPerDataRef = numberEncodingSize(bytesRequiredIncludingData);
+	// TODO can this return push0?


Nope - this would only ever return zero, if there was neither code nor data, in which case we'd not do anything anyways - but since bytesRequiredIncludingData is always strictly positive, bytesPerDataRef will be so as well, so dataRefPush will always be at least a PUSH1.

ekpyron · 2023-04-11T10:01:22Z

liblangutil/EVMVersion.h


 	EVMVersion(Version _version): m_version(_version) {}

+	// FIXME update the default EVM version in a future PR


We don't really need the comment - we'll do it once Shanghai goes live, s.t. the first release after defaults to Shanghai.

ekpyron

The main "issue" I'm seeing with this is that AssemblyItem::bytesRequired, resp. evmasm::bytesRequiredreturn wrong values for PUSH0 - which is a bit annoying, since they will now depend on the EVM version.

It's not that big of a deal for now, though, since at worst this will result in reserving too much space for tags, if push1 0 vs push0 let the codesize estimate drop sufficiently. It will/would be more of a problem for EOF, for which it's nicer if we can accurately predict code size. So this should be adjusted, even if this means passing down evm versions even more - but I'm fine with doing it separately.

Not entirely sure about some of the syntax tests, but fine.

So I'm fine with merging this - we could still remove the comments I'm commenting about, but apart from that, this is good enough for a first version and we can do the rest separately.

invocamanman · 2023-04-11T15:54:17Z

Since this uses a new opcode, and it's used "underneath", so the user does not have fully control over it, it totally breaks compatibility with previous forks. For instance if you try to deploy any contract compiled with the 0.8.20 in any fork previous to Shanghai it will break the contract ( since the push0 will be processed as an INVALID).
This could break contracts on others EVM compatible chains therefore I suggest to apply this change in a major version change such as 0.9

Thank you^^

ekpyron · 2023-04-11T16:23:20Z

Since this uses a new opcode, and it's used "underneath", so the user does not have fully control over it, it totally breaks compatibility with previous forks. For instance if you try to deploy any contract compiled with the 0.8.20 in any fork previous to Shanghai it will break the contract ( since the push0 will be processed as an INVALID). This could break contracts on others EVM compatible chains therefore I suggest to apply this change in a major version change such as 0.9

Thank you^^

That's true for any previous hard-forks as well - the compiler can be set up to the EVM version you want and is backwards-compatible up until homestead. It has generally always followed EVM mainnet in the default version, so in the first solc release after the hardfork we will switch the default (note that this PR does not switch the default, but merely adds support upon explicit request for now).

Overriding this default especially in the connection with other EVM-compatible chains is a job for the framework involved, for the compiler I strongly tend towards sticking to the traditional convention.

ekpyron · 2023-04-11T16:46:56Z

@hrkrshnn Can you squash the commits and remove the comments? Then I'd probably just merge this for now.
We can then try to raise some more input on switching the default before the hard-fork, to give people the chance to sway my mind on just switching the default with the fork :-).

hrkrshnn · 2023-04-11T17:25:23Z

Can you squash the commits and remove the comments?

Ah come on, I painfully separated all the commits 😢 Will fix the pending comments today.

ekpyron · 2023-04-11T17:38:54Z

Can you squash the commits and remove the comments?

Ah come on, I painfully separated all the commits cry Will fix the pending comments today.

Haha, ok, just removing the comments is also just fine :-).

ekpyron · 2023-04-11T17:43:18Z

We can then try to raise some more input on switching the default before the hard-fork, to give people the chance to sway my mind on just switching the default with the fork

Not that there's that much time left before Shanghai to decide this :-). But we can also leave it as paris as default for the next release and ultimately decide for the release after, we've been lagging behind slightly in the past already anyways. Or we still take some time to get input on this before releasing.

Note: Paris is still the default

1. `push0_disallowed.yul`: checks if `push0()` is a valid builtin in strict Yul 2. `push0_disallowed.sol`: checks if `push0()` is a valid builtin in inline assembly 3. `push0.sol`: simple semantic test that returns 0 4. `evmone_support.sol`: tests if push0 works properly in evmone 5. Updated some bytecode too large tests to use `shanghai` as version 6. Updated various tests where `push1 0` was hardcoded in different forms / expectations on bytecode size (`Assembler.cpp`, `GasCosts.cpp`, `SolidityCompiler.cpp`, `SolidityExpressionCompiler.cpp`)

hrkrshnn · 2023-04-11T22:13:06Z

Just removed both the comments (the todo and fixme).

invocamanman · 2023-04-12T08:26:50Z

Thank you so much for your responses and also for taking into account the suggestion^^ 😊

axic · 2023-04-12T08:55:02Z

Changelog.md

@@ -4,6 +4,8 @@ Language Features:


 Compiler Features:
+ * Assembler: Use ``push0`` for placing ``0`` in the stack for EVM versions starting from "Shanghai". This decreases the deployment costs.


This decreases runtime cost too, 3 gas vs 2 gas.

Will add it.

liblangutil/EVMVersion.h

Fixes #14107 (comment)

Fixes ethereum#14107 (comment)

feat: add support for push0 in libevmasm

8757e0a

See https://eips.ethereum.org/EIPS/eip-3855 Note: `push0` costs 2 gas, i.e., `Tier::Base` unlike other pushes.

github-actions bot added the external contribution ⭐ label Apr 10, 2023

hrkrshnn force-pushed the push0 branch from be8e860 to 1a7760e Compare April 10, 2023 10:06

NunoFilipeSantos assigned hrkrshnn Apr 11, 2023

ekpyron reviewed Apr 11, 2023

View reviewed changes

ekpyron previously approved these changes Apr 11, 2023

View reviewed changes

NunoFilipeSantos added this to the 0.8.20 milestone Apr 11, 2023

hrkrshnn added 4 commits April 12, 2023 00:10

feat: change generation of bytecode for pushes with zero-value

0bfcdaf

feat: introduce EVM Version Shanghai

802f895

Note: Paris is still the default

docs: On supporting shanghai EVM version and changelog

b30f89b

hrkrshnn dismissed ekpyron’s stale review via b30f89b April 11, 2023 22:11

hrkrshnn force-pushed the push0 branch from 1a7760e to b30f89b Compare April 11, 2023 22:11

ekpyron approved these changes Apr 12, 2023

View reviewed changes

ekpyron merged commit 82bde40 into develop Apr 12, 2023

ekpyron deleted the push0 branch April 12, 2023 08:10

axic reviewed Apr 12, 2023

View reviewed changes

aarlt reviewed Apr 12, 2023

View reviewed changes

liblangutil/EVMVersion.h Show resolved Hide resolved

axic mentioned this pull request Apr 13, 2023

Change the constant optimizer to make use of PUSH0 #14117

Merged

ekpyron mentioned this pull request Apr 17, 2023

Emit PUSH0 as junk in evm code transform, if available. #14133

Merged

DaniPopes mentioned this pull request Apr 29, 2023

feat(solc): add support for Paris and Shanghai EVM versions gakonst/ethers-rs#2385

Merged

3 tasks

hrkrshnn added a commit that referenced this pull request May 8, 2023

docs: mention that push0 also decreases the runtime costs

cbd6495

Fixes #14107 (comment)

hrkrshnn mentioned this pull request May 8, 2023

docs: mention that push0 also decreases the runtime costs #14195

Closed

Ruko97 pushed a commit to Ruko97/solidity that referenced this pull request Apr 7, 2024

docs: mention that push0 also decreases the runtime costs

6a18ee3

Fixes ethereum#14107 (comment)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introduce `Shanghai` version and `push0` support #14107

Introduce `Shanghai` version and `push0` support #14107

hrkrshnn commented Apr 10, 2023

github-actions bot commented Apr 10, 2023

ekpyron Apr 11, 2023

ekpyron Apr 11, 2023

ekpyron left a comment •

edited

Loading

invocamanman commented Apr 11, 2023 •

edited

Loading

ekpyron commented Apr 11, 2023 •

edited

Loading

ekpyron commented Apr 11, 2023

hrkrshnn commented Apr 11, 2023 •

edited

Loading

ekpyron commented Apr 11, 2023

ekpyron commented Apr 11, 2023 •

edited

Loading

hrkrshnn commented Apr 11, 2023

invocamanman commented Apr 12, 2023

axic Apr 12, 2023

hrkrshnn Apr 12, 2023


		EVMVersion(Version _version): m_version(_version) {}

		// FIXME update the default EVM version in a future PR

		@@ -4,6 +4,8 @@ Language Features:


		Compiler Features:
		* Assembler: Use ``push0`` for placing ``0`` in the stack for EVM versions starting from "Shanghai". This decreases the deployment costs.

Introduce Shanghai version and push0 support #14107

Introduce Shanghai version and push0 support #14107

Conversation

hrkrshnn commented Apr 10, 2023

github-actions bot commented Apr 10, 2023

ekpyron Apr 11, 2023

Choose a reason for hiding this comment

ekpyron Apr 11, 2023

Choose a reason for hiding this comment

ekpyron left a comment • edited Loading

Choose a reason for hiding this comment

invocamanman commented Apr 11, 2023 • edited Loading

ekpyron commented Apr 11, 2023 • edited Loading

ekpyron commented Apr 11, 2023

hrkrshnn commented Apr 11, 2023 • edited Loading

ekpyron commented Apr 11, 2023

ekpyron commented Apr 11, 2023 • edited Loading

hrkrshnn commented Apr 11, 2023

invocamanman commented Apr 12, 2023

axic Apr 12, 2023

Choose a reason for hiding this comment

hrkrshnn Apr 12, 2023

Choose a reason for hiding this comment

Introduce `Shanghai` version and `push0` support #14107

Introduce `Shanghai` version and `push0` support #14107

ekpyron left a comment •

edited

Loading

invocamanman commented Apr 11, 2023 •

edited

Loading

ekpyron commented Apr 11, 2023 •

edited

Loading

hrkrshnn commented Apr 11, 2023 •

edited

Loading

ekpyron commented Apr 11, 2023 •

edited

Loading