-
Notifications
You must be signed in to change notification settings - Fork 5.7k
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
Update evmc
to 12.0.0
and evmone
to 0.12.0
#15320
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -112,26 +112,10 @@ then | |
rm -rf "$z3_dir" | ||
|
||
# evmone | ||
evmone_version="0.11.0" | ||
if [[ $(uname -m) == 'arm64' ]] | ||
then | ||
# evmone does not provide any builds for apple silicon yet. so lets just build it locally. | ||
# be aware that we are only building the arm version here, we don't build a universal binary. | ||
git clone https://github.com/ethereum/evmone.git | ||
cd evmone | ||
git checkout "v${evmone_version}" | ||
git submodule update --init | ||
cmake -S . -B build | ||
cmake --build build | ||
cd build | ||
sudo make install | ||
cd ../.. | ||
rm -rf evmone | ||
else | ||
evmone_package="evmone-${evmone_version}-darwin-x86_64.tar.gz" | ||
wget "https://github.com/ethereum/evmone/releases/download/v${evmone_version}/${evmone_package}" | ||
validate_checksum "$evmone_package" 83ed20676681d9a31bd30cac399ab7c615ccab8adb8087cc2c7e9cd22b4d2efc | ||
tar xzpf "$evmone_package" -C /usr/local | ||
rm "$evmone_package" | ||
fi | ||
evmone_version="0.12.0" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The checksum below needs to be updated as well. See line 133: https://github.com/ethereum/solidity/pull/15320/files#diff-95c60f439bb8a5efd25627b6a7c2b37310e42d8bd13e1cdb369fd3af41e88ea7R133 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I just checked the evmone repository and it seems that they don't ship There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, great catch. @rodiazet can you please update? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
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. |
||
evmone_package="evmone-${evmone_version}-darwin-arm64.tar.gz" | ||
wget "https://github.com/ethereum/evmone/releases/download/v${evmone_version}/${evmone_package}" | ||
validate_checksum "$evmone_package" e164e0d2b985cc1cca07b501538b2e804bf872d1d8d531f9241d518a886234a6 | ||
sudo tar xzpf "$evmone_package" -C /usr/local | ||
rm "$evmone_package" | ||
fi |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,93 @@ | ||
// EVMC: Ethereum Client-VM Connector API. | ||
// Copyright 2024 The EVMC Authors. | ||
// Licensed under the Apache License, Version 2.0. | ||
#pragma once | ||
|
||
#include <algorithm> | ||
#include <cstring> | ||
#include <string> | ||
#include <string_view> | ||
|
||
namespace evmc | ||
{ | ||
/// The char traits for byte-like types. | ||
/// | ||
/// See: https://en.cppreference.com/w/cpp/string/char_traits. | ||
template <typename T> | ||
struct byte_traits : std::char_traits<char> | ||
{ | ||
static_assert(sizeof(T) == 1, "type must be a byte"); | ||
|
||
using char_type = T; ///< The byte type. | ||
|
||
/// Assigns c2 to c1. | ||
static constexpr void assign(char_type& c1, const char_type& c2) { c1 = c2; } | ||
|
||
/// Assigns value to each byte in [ptr, ptr+count). | ||
static constexpr char_type* assign(char_type* ptr, std::size_t count, char_type value) | ||
{ | ||
std::fill_n(ptr, count, value); | ||
return ptr; | ||
} | ||
|
||
/// Returns true if bytes are equal. | ||
static constexpr bool eq(char_type a, char_type b) { return a == b; } | ||
|
||
/// Returns true if byte a is less than byte b. | ||
static constexpr bool lt(char_type a, char_type b) { return a < b; } | ||
|
||
/// Copies count bytes from src to dest. Performs correctly even if ranges overlap. | ||
static constexpr char_type* move(char_type* dest, const char_type* src, std::size_t count) | ||
{ | ||
if (dest < src) | ||
std::copy_n(src, count, dest); | ||
else if (src < dest) | ||
std::copy_backward(src, src + count, dest + count); | ||
return dest; | ||
} | ||
|
||
/// Copies count bytes from src to dest. The ranges must not overlap. | ||
static constexpr char_type* copy(char_type* dest, const char_type* src, std::size_t count) | ||
{ | ||
std::copy_n(src, count, dest); | ||
return dest; | ||
} | ||
|
||
/// Compares lexicographically the bytes in two ranges of equal length. | ||
static constexpr int compare(const char_type* a, const char_type* b, std::size_t count) | ||
{ | ||
for (; count != 0; --count, ++a, ++b) | ||
{ | ||
if (lt(*a, *b)) | ||
return -1; | ||
if (lt(*b, *a)) | ||
return 1; | ||
} | ||
return 0; | ||
} | ||
|
||
/// Returns the length of a null-terminated byte string. | ||
// TODO: Not constexpr | ||
static std::size_t length(const char_type* s) | ||
{ | ||
return std::strlen(reinterpret_cast<const char*>(s)); | ||
} | ||
|
||
/// Finds the value in the range of bytes and returns the pointer to the first occurrence | ||
/// or nullptr if not found. | ||
static constexpr const char_type* find(const char_type* s, | ||
std::size_t count, | ||
const char_type& value) | ||
{ | ||
const auto end = s + count; | ||
const auto p = std::find(s, end, value); | ||
return p != end ? p : nullptr; | ||
} | ||
}; | ||
|
||
/// String of unsigned chars representing bytes. | ||
using bytes = std::basic_string<unsigned char, byte_traits<unsigned char>>; | ||
|
||
/// String view of unsigned chars representing bytes. | ||
using bytes_view = std::basic_string_view<unsigned char, byte_traits<unsigned char>>; | ||
} // namespace evmc |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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)