Update `evmc` to `12.0.0` and `evmone` to `0.12.0` #15320

rodiazet · 2024-08-08T13:12:29Z

Depends on #15422

github-actions · 2024-08-08T13:12:42Z

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.

nikola-matic

Hey @rodiazet can you please revert all of the dockerfile changes in docker/buildpack-deps? These need to be in a separate PR (which I've already opened), as this is where the artifacts are built; once said artifacts are build in my PR, then you'll need to get the hashes from said new artifacts and apply them in to the CircleCI config.yml (in this PR). See our previous bump PR #14758

r0qs · 2024-08-09T09:19:30Z

.circleci/osx_install_dependencies.sh

@@ -112,7 +112,7 @@ then
  rm -rf "$z3_dir"

  # evmone
-  evmone_version="0.11.0"
+  evmone_version="0.12.0"


The checksum below needs to be updated as well. See line 133: https://github.com/ethereum/solidity/pull/15320/files#diff-95c60f439bb8a5efd25627b6a7c2b37310e42d8bd13e1cdb369fd3af41e88ea7R133

I just checked the evmone repository and it seems that they don't ship darwin-x86_64 anymore, since version 0.12.0. In this case, as they are shipping arm64 now (https://github.com/ethereum/evmone/releases/) we can remove the if below and update the checksum accordingly.

Ah, great catch. @rodiazet can you please update?

How did it even pass CI with the current checksum though? If we're not running CI on intel macs any more and not executing this bit, perhaps it's better to just remove it?

We're ARM only (M1), so it was hitting the first branch and building from sources - the else branch is dead, and should be removed as @r0qs suggested. Or rather, the else branch should become the new standard and simply unpack from the tar.gz, since these are now available for ARM.

What I'm curious about is this. What does b_bytecode_osx_intel-via-ir-optimize even do, since we're running on ARM only? I assume this step should be renamed.

What I'm curious about is this. What does b_bytecode_osx_intel-via-ir-optimize even do, since we're running on ARM only? I assume this step should be renamed.

No, we have a universal binary, which contains both ARM and Intel code in one package. This this step is testing that the Intel part works too. But IIRC it still runs on ARM and just runs under emulation.

Would not hurt to have a comment saying that next to the job though. We should be leaving more of such comments with extra context really.

nikola-matic · 2024-08-16T11:25:45Z

Heya @rodiazet, we finally managed to bump the docker images to evmone12, but unfortunately, not all of them, since the ossfuzz images is causing problems - we therefore decided to leave said images as is (with evmone11), and to bump the rest of them.

You can now bump the hashes in the .circleci/config.yml to the ones produces in my PR #15321. Scroll all the way to the bottom, and update the ones with a 👍 emoji; also note that I've added an Ubuntu 24.04 images, so you'll have to add a separate entry for that one.

Ping if you need help :)

badnewz80

👍

r0qs · 2024-09-12T12:27:23Z

.circleci/osx_install_dependencies.sh

@@ -112,7 +112,7 @@ then
  rm -rf "$z3_dir"

  # evmone
-  evmone_version="0.11.0"
+  evmone_version="0.12.0"


I just checked the evmone repository and it seems that they don't ship darwin-x86_64 anymore, since version 0.12.0. In this case, as they are shipping arm64 now (https://github.com/ethereum/evmone/releases/) we can remove the if below and update the checksum accordingly.

test/Common.h

cameel · 2024-09-12T13:45:57Z

test/evmc/bytes.hpp

I'm assuming the files under test/evmc/ were just straight copied from evmc - or where there any adjustments needed?

BTW, We should really consider replacing this with a submodule soon. We have switched other deps to submodules so this wouldn't even cause the minor hiccups we could expect with such a change in the past.

I'm assuming the files under test/evmc/ were just straight copied from evmc - or where there any adjustments needed?

As always :)

We should really replace this with a submodule soon.

Yes please, but not now.

.circleci/config.yml

cameel · 2024-09-12T14:25:11Z

.circleci/config.yml

-    # solbuildpackpusher/solidity-buildpack-deps:ubuntu2004-24
-    default: "solbuildpackpusher/solidity-buildpack-deps@sha256:b8b645fa7ab40d55f2d16eac295d16ca01ec51d32be7d668ae6eaecd47dbd763"
+    # solbuildpackpusher/solidity-buildpack-deps:ubuntu2004-25
+    default: "solbuildpackpusher/solidity-buildpack-deps@sha256:b3f321fb2d8e7a41ca9328672061c1840e5cd3fb5be503aa158d1c508deacf0a"
  ubuntu-2204-docker-image:
    type: string
-    # solbuildpackpusher/solidity-buildpack-deps:ubuntu2204-9
-    default: "solbuildpackpusher/solidity-buildpack-deps@sha256:80247de9655b1f39afd4ac22b14266bc9b9a0d64b283ae8fb9cb5b8250e4e77d"
+    # solbuildpackpusher/solidity-buildpack-deps:ubuntu2204-10
+    default: "solbuildpackpusher/solidity-buildpack-deps@sha256:ba2d878c26d681a7d6c1922258b47c2e5dd61d0e46ab4c6a4862b473e61997b6"


I really liked how @r0qs linked to the PRs hashes came from in #15422. It helps a lot when reviewing. We should always be doing that when the update is spread over more than the usual 2 PRs and you can get lost in them. It would have helped to at least link the PR as a dependency in the description.

In this case:

ubuntu2204-10: Bump evmone to v0.12.0 and add ubuntu2404 image #15321 (comment)

ubuntu2004-25: Bump evmone to v0.12.0 and add ubuntu2404 image #15321 (comment)

ubuntu2204.clang-9: Bump evmone to v0.12.0 and add ubuntu2404 image #15321 (comment)

r0qs

LGTM. Thanks @rodiazet :D

rodiazet changed the title ~~Update evmc to 12.0.0 and evmone to 0.12.0`~~ Update evmc to 12.0.0 and evmone to 0.12.0 Aug 8, 2024

github-actions bot added the external contribution ⭐ label Aug 8, 2024

rodiazet force-pushed the evmc-evmone-12 branch from d0c2549 to 7e91b78 Compare August 8, 2024 13:13

nikola-matic requested changes Aug 9, 2024

View reviewed changes

rodiazet force-pushed the evmc-evmone-12 branch from 7e91b78 to c06af9f Compare August 9, 2024 09:09

nikola-matic marked this pull request as draft August 9, 2024 09:11

r0qs reviewed Aug 9, 2024

View reviewed changes

ekpyron added the 🟡 PR review label label Aug 15, 2024

rodiazet force-pushed the evmc-evmone-12 branch 2 times, most recently from 008bdd7 to 9286908 Compare August 20, 2024 16:26

badnewz80 reviewed Sep 8, 2024

View reviewed changes

cameel added the build system 🏗️ label Sep 9, 2024

rodiazet changed the base branch from develop to run-jobs-on-ubuntu-2404 September 9, 2024 15:18

rodiazet force-pushed the evmc-evmone-12 branch from 9286908 to 1006992 Compare September 9, 2024 15:23

nikola-matic force-pushed the run-jobs-on-ubuntu-2404 branch 2 times, most recently from 3c67d5f to 6f548be Compare September 10, 2024 07:02

nikola-matic added the has dependencies The PR depends on other PRs that must be merged first label Sep 10, 2024

nikola-matic mentioned this pull request Sep 10, 2024

Add ubuntu 2404 images to CI config #15422

Merged

nikola-matic deleted the branch ethereum:develop September 11, 2024 13:13

nikola-matic closed this Sep 11, 2024

cameel reopened this Sep 11, 2024

rodiazet changed the base branch from run-jobs-on-ubuntu-2404 to develop September 11, 2024 13:58

rodiazet force-pushed the evmc-evmone-12 branch from 1006992 to d130d64 Compare September 11, 2024 14:07

nikola-matic marked this pull request as ready for review September 12, 2024 12:19

nikola-matic previously approved these changes Sep 12, 2024

View reviewed changes

r0qs requested changes Sep 12, 2024

View reviewed changes

ethereum deleted a comment from badnewz80 Sep 12, 2024

cameel reviewed Sep 12, 2024

View reviewed changes

test/Common.h Show resolved Hide resolved

cameel reviewed Sep 12, 2024

View reviewed changes

rodiazet dismissed nikola-matic’s stale review via fee6b35 September 12, 2024 13:49

rodiazet force-pushed the evmc-evmone-12 branch from d130d64 to fee6b35 Compare September 12, 2024 13:49

cameel reviewed Sep 12, 2024

View reviewed changes

.circleci/config.yml Outdated Show resolved Hide resolved

cameel reviewed Sep 12, 2024

View reviewed changes

rodiazet force-pushed the evmc-evmone-12 branch 2 times, most recently from 6a6de79 to 890ad92 Compare September 13, 2024 08:11

nikola-matic and others added 2 commits September 13, 2024 10:13

Upgrade jobs to run on Ubuntu 2404

16b2447

Update evmc to 12.0.0 and evmone to 0.12.0

1d58415

rodiazet force-pushed the evmc-evmone-12 branch from 890ad92 to 1d58415 Compare September 13, 2024 08:13

r0qs approved these changes Sep 13, 2024

View reviewed changes

ekpyron merged commit a92a51b into ethereum:develop Sep 17, 2024
72 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update `evmc` to `12.0.0` and `evmone` to `0.12.0` #15320

Update `evmc` to `12.0.0` and `evmone` to `0.12.0` #15320

rodiazet commented Aug 8, 2024 •

edited by nikola-matic

Loading

github-actions bot commented Aug 8, 2024

nikola-matic left a comment

r0qs Aug 9, 2024 •

edited

Loading

r0qs Sep 12, 2024

nikola-matic Sep 12, 2024

cameel Sep 12, 2024 •

edited

Loading

nikola-matic Sep 12, 2024

rodiazet Sep 12, 2024

cameel Sep 12, 2024 •

edited

Loading

nikola-matic commented Aug 16, 2024

badnewz80 left a comment •

edited

Loading

r0qs Sep 12, 2024

cameel Sep 12, 2024 •

edited

Loading

nikola-matic Sep 12, 2024

cameel Sep 12, 2024

r0qs left a comment

Update evmc to 12.0.0 and evmone to 0.12.0 #15320

Update evmc to 12.0.0 and evmone to 0.12.0 #15320

Conversation

rodiazet commented Aug 8, 2024 • edited by nikola-matic Loading

github-actions bot commented Aug 8, 2024

nikola-matic left a comment

Choose a reason for hiding this comment

r0qs Aug 9, 2024 • edited Loading

Choose a reason for hiding this comment

r0qs Sep 12, 2024

Choose a reason for hiding this comment

nikola-matic Sep 12, 2024

Choose a reason for hiding this comment

cameel Sep 12, 2024 • edited Loading

Choose a reason for hiding this comment

nikola-matic Sep 12, 2024

Choose a reason for hiding this comment

rodiazet Sep 12, 2024

Choose a reason for hiding this comment

cameel Sep 12, 2024 • edited Loading

Choose a reason for hiding this comment

nikola-matic commented Aug 16, 2024

badnewz80 left a comment • edited Loading

Choose a reason for hiding this comment

r0qs Sep 12, 2024

Choose a reason for hiding this comment

cameel Sep 12, 2024 • edited Loading

Choose a reason for hiding this comment

nikola-matic Sep 12, 2024

Choose a reason for hiding this comment

cameel Sep 12, 2024

Choose a reason for hiding this comment

r0qs left a comment

Choose a reason for hiding this comment

Update `evmc` to `12.0.0` and `evmone` to `0.12.0` #15320

Update `evmc` to `12.0.0` and `evmone` to `0.12.0` #15320

rodiazet commented Aug 8, 2024 •

edited by nikola-matic

Loading

r0qs Aug 9, 2024 •

edited

Loading

cameel Sep 12, 2024 •

edited

Loading

cameel Sep 12, 2024 •

edited

Loading

badnewz80 left a comment •

edited

Loading

cameel Sep 12, 2024 •

edited

Loading