Skip to content

Commit

Permalink
fix(EVM): Remove magic numbers (N-09) (#1175)
Browse files Browse the repository at this point in the history
  • Loading branch information
0xVolosnikov authored Jan 2, 2025
1 parent ddea0fc commit 816c358
Show file tree
Hide file tree
Showing 4 changed files with 43 additions and 37 deletions.
44 changes: 24 additions & 20 deletions system-contracts/contracts/EvmEmulator.yul
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,9 @@ object "EvmEmulator" {
value := 0xc5d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470
}

function ADDRESS_MASK() -> value { // mask for lower 160 bits
value := 0xffffffffffffffffffffffffffffffffffffffff
}

////////////////////////////////////////////////////////////////
// GENERAL FUNCTIONS
Expand Down Expand Up @@ -585,11 +588,12 @@ object "EvmEmulator" {
// EVM GAS MANAGER FUNCTIONALITY
////////////////////////////////////////////////////////////////

// Address higher bytes must be cleaned before
function $llvm_AlwaysInline_llvm$_warmAddress(addr) -> isWarm {
// function warmAccount(address account)
// non-standard selector 0x00
// addr is packed in the same word with selector
mstore(0, and(addr, 0xffffffffffffffffffffffffffffffffffffffff))
mstore(0, addr)

performSystemCall(EVM_GAS_MANAGER_CONTRACT(), 32)

Expand Down Expand Up @@ -824,11 +828,11 @@ object "EvmEmulator" {
}

function _genericPrecallLogic(rawAddr, argsOffset, argsSize, retOffset, retSize) -> addr, gasUsed {
addr := and(rawAddr, 0xffffffffffffffffffffffffffffffffffffffff)

// memory_expansion_cost
gasUsed := expandMemory2(retOffset, retSize, argsOffset, argsSize)

addr := and(rawAddr, ADDRESS_MASK())

let addressAccessCost := 100 // warm address access cost
if iszero($llvm_AlwaysInline_llvm$_warmAddress(addr)) {
addressAccessCost := 2600 // cold address access cost
Expand Down Expand Up @@ -1136,8 +1140,7 @@ object "EvmEmulator" {

if canBeDeployed {
returndatacopy(0, 0, 32)
addr := mload(0)

addr := and(mload(0), ADDRESS_MASK())
pop($llvm_AlwaysInline_llvm$_warmAddress(addr)) // will stay warm even if constructor reverts
// so even if constructor reverts, nonce stays incremented and addr stays warm

Expand Down Expand Up @@ -1631,7 +1634,7 @@ object "EvmEmulator" {
evmGasLeft := chargeGas(evmGasLeft, 100)

let addr := accessStackHead(sp, stackHead)
addr := and(addr, 0xffffffffffffffffffffffffffffffffffffffff)
addr := and(addr, ADDRESS_MASK())

if iszero($llvm_AlwaysInline_llvm$_warmAddress(addr)) {
evmGasLeft := chargeGas(evmGasLeft, 2500)
Expand Down Expand Up @@ -1774,7 +1777,7 @@ object "EvmEmulator" {

let addr := accessStackHead(sp, stackHead)

addr := and(addr, 0xffffffffffffffffffffffffffffffffffffffff)
addr := and(addr, ADDRESS_MASK())
if iszero($llvm_AlwaysInline_llvm$_warmAddress(addr)) {
evmGasLeft := chargeGas(evmGasLeft, 2500)
}
Expand Down Expand Up @@ -1803,15 +1806,14 @@ object "EvmEmulator" {
srcOffset, sp, stackHead := popStackItemWithoutCheck(sp, stackHead)
len, sp, stackHead := popStackItemWithoutCheck(sp, stackHead)

addr := and(addr, 0xffffffffffffffffffffffffffffffffffffffff)

// dynamicGas = 3 * minimum_word_size + memory_expansion_cost + address_access_cost
// minimum_word_size = (size + 31) / 32
let dynamicGas := add(
mul(3, shr(5, add(len, 31))),
expandMemory(dstOffset, len)
)

addr := and(addr, ADDRESS_MASK())
if iszero($llvm_AlwaysInline_llvm$_warmAddress(addr)) {
dynamicGas := add(dynamicGas, 2500)
}
Expand Down Expand Up @@ -1870,7 +1872,7 @@ object "EvmEmulator" {
evmGasLeft := chargeGas(evmGasLeft, 100)

let addr := accessStackHead(sp, stackHead)
addr := and(addr, 0xffffffffffffffffffffffffffffffffffffffff)
addr := and(addr, ADDRESS_MASK())

if iszero($llvm_AlwaysInline_llvm$_warmAddress(addr)) {
evmGasLeft := chargeGas(evmGasLeft, 2500)
Expand Down Expand Up @@ -3104,6 +3106,9 @@ object "EvmEmulator" {
value := 0xc5d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470
}

function ADDRESS_MASK() -> value { // mask for lower 160 bits
value := 0xffffffffffffffffffffffffffffffffffffffff
}

////////////////////////////////////////////////////////////////
// GENERAL FUNCTIONS
Expand Down Expand Up @@ -3501,11 +3506,12 @@ object "EvmEmulator" {
// EVM GAS MANAGER FUNCTIONALITY
////////////////////////////////////////////////////////////////

// Address higher bytes must be cleaned before
function $llvm_AlwaysInline_llvm$_warmAddress(addr) -> isWarm {
// function warmAccount(address account)
// non-standard selector 0x00
// addr is packed in the same word with selector
mstore(0, and(addr, 0xffffffffffffffffffffffffffffffffffffffff))
mstore(0, addr)

performSystemCall(EVM_GAS_MANAGER_CONTRACT(), 32)

Expand Down Expand Up @@ -3740,11 +3746,11 @@ object "EvmEmulator" {
}

function _genericPrecallLogic(rawAddr, argsOffset, argsSize, retOffset, retSize) -> addr, gasUsed {
addr := and(rawAddr, 0xffffffffffffffffffffffffffffffffffffffff)

// memory_expansion_cost
gasUsed := expandMemory2(retOffset, retSize, argsOffset, argsSize)

addr := and(rawAddr, ADDRESS_MASK())

let addressAccessCost := 100 // warm address access cost
if iszero($llvm_AlwaysInline_llvm$_warmAddress(addr)) {
addressAccessCost := 2600 // cold address access cost
Expand Down Expand Up @@ -4052,8 +4058,7 @@ object "EvmEmulator" {

if canBeDeployed {
returndatacopy(0, 0, 32)
addr := mload(0)

addr := and(mload(0), ADDRESS_MASK())
pop($llvm_AlwaysInline_llvm$_warmAddress(addr)) // will stay warm even if constructor reverts
// so even if constructor reverts, nonce stays incremented and addr stays warm

Expand Down Expand Up @@ -4535,7 +4540,7 @@ object "EvmEmulator" {
evmGasLeft := chargeGas(evmGasLeft, 100)

let addr := accessStackHead(sp, stackHead)
addr := and(addr, 0xffffffffffffffffffffffffffffffffffffffff)
addr := and(addr, ADDRESS_MASK())

if iszero($llvm_AlwaysInline_llvm$_warmAddress(addr)) {
evmGasLeft := chargeGas(evmGasLeft, 2500)
Expand Down Expand Up @@ -4678,7 +4683,7 @@ object "EvmEmulator" {

let addr := accessStackHead(sp, stackHead)

addr := and(addr, 0xffffffffffffffffffffffffffffffffffffffff)
addr := and(addr, ADDRESS_MASK())
if iszero($llvm_AlwaysInline_llvm$_warmAddress(addr)) {
evmGasLeft := chargeGas(evmGasLeft, 2500)
}
Expand Down Expand Up @@ -4707,15 +4712,14 @@ object "EvmEmulator" {
srcOffset, sp, stackHead := popStackItemWithoutCheck(sp, stackHead)
len, sp, stackHead := popStackItemWithoutCheck(sp, stackHead)

addr := and(addr, 0xffffffffffffffffffffffffffffffffffffffff)

// dynamicGas = 3 * minimum_word_size + memory_expansion_cost + address_access_cost
// minimum_word_size = (size + 31) / 32
let dynamicGas := add(
mul(3, shr(5, add(len, 31))),
expandMemory(dstOffset, len)
)

addr := and(addr, ADDRESS_MASK())
if iszero($llvm_AlwaysInline_llvm$_warmAddress(addr)) {
dynamicGas := add(dynamicGas, 2500)
}
Expand Down Expand Up @@ -4774,7 +4778,7 @@ object "EvmEmulator" {
evmGasLeft := chargeGas(evmGasLeft, 100)

let addr := accessStackHead(sp, stackHead)
addr := and(addr, 0xffffffffffffffffffffffffffffffffffffffff)
addr := and(addr, ADDRESS_MASK())

if iszero($llvm_AlwaysInline_llvm$_warmAddress(addr)) {
evmGasLeft := chargeGas(evmGasLeft, 2500)
Expand Down
14 changes: 7 additions & 7 deletions system-contracts/contracts/libraries/Utils.sol
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ pragma solidity ^0.8.20;
import {EfficientCall} from "./EfficientCall.sol";
import {RLPEncoder} from "./RLPEncoder.sol";
import {MalformedBytecode, BytecodeError, Overflow} from "../SystemContractErrors.sol";
import {ERA_VM_BYTECODE_FLAG, EVM_BYTECODE_FLAG} from "../Constants.sol";
import {ERA_VM_BYTECODE_FLAG, EVM_BYTECODE_FLAG, CREATE2_EVM_PREFIX} from "../Constants.sol";

/**
* @author Matter Labs
Expand Down Expand Up @@ -103,6 +103,8 @@ library Utils {
return _bytecodeHash & ~IS_CONSTRUCTOR_BYTECODE_HASH_BIT_MASK;
}

uint256 internal constant MAX_BYTECODE_LENGTH = (2 ** 16) - 1;

/// @notice Validate the bytecode format and calculate its hash.
/// @param _bytecode The bytecode to hash.
/// @return hashedBytecode The 32-byte hash of the bytecode.
Expand All @@ -118,7 +120,7 @@ library Utils {

uint256 lengthInWords = _bytecode.length / 32;
// bytecode length must be less than 2^16 words
if (lengthInWords >= 2 ** 16) {
if (lengthInWords > MAX_BYTECODE_LENGTH) {
revert MalformedBytecode(BytecodeError.NumberOfWords);
}
// bytecode length in words must be odd
Expand All @@ -134,9 +136,6 @@ library Utils {
hashedBytecode = hashedBytecode | bytes32(lengthInWords << 224);
}

// the real max supported number is 2^16, but we'll stick to evm convention
uint256 internal constant MAX_EVM_BYTECODE_LENGTH = (2 ** 16) - 1;

/// @notice Validate the bytecode format and calculate its hash.
/// @param _evmBytecodeLen The length of original EVM bytecode in bytes
/// @param _paddedBytecode The padded EVM bytecode to hash.
Expand All @@ -158,7 +157,8 @@ library Utils {
revert MalformedBytecode(BytecodeError.EvmBytecodeLength);
}

if (_evmBytecodeLen > MAX_EVM_BYTECODE_LENGTH) {
// bytecode length must be less than 2^16 bytes
if (_evmBytecodeLen > MAX_BYTECODE_LENGTH) {
revert MalformedBytecode(BytecodeError.EvmBytecodeLengthTooBig);
}

Expand Down Expand Up @@ -187,7 +187,7 @@ library Utils {
bytes32 _salt,
bytes32 _bytecodeHash
) internal pure returns (address newAddress) {
bytes32 hash = keccak256(abi.encodePacked(bytes1(0xff), _sender, _salt, _bytecodeHash));
bytes32 hash = keccak256(abi.encodePacked(bytes1(CREATE2_EVM_PREFIX), _sender, _salt, _bytecodeHash));

newAddress = address(uint160(uint256(hash)));
}
Expand Down
13 changes: 8 additions & 5 deletions system-contracts/evm-emulator/EvmEmulatorFunctions.template.yul
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,9 @@ function EMPTY_KECCAK() -> value { // keccak("")
value := 0xc5d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470
}

function ADDRESS_MASK() -> value { // mask for lower 160 bits
value := 0xffffffffffffffffffffffffffffffffffffffff
}

////////////////////////////////////////////////////////////////
// GENERAL FUNCTIONS
Expand Down Expand Up @@ -525,11 +528,12 @@ function accessStackHead(sp, stackHead) -> value {
// EVM GAS MANAGER FUNCTIONALITY
////////////////////////////////////////////////////////////////

// Address higher bytes must be cleaned before
function $llvm_AlwaysInline_llvm$_warmAddress(addr) -> isWarm {
// function warmAccount(address account)
// non-standard selector 0x00
// addr is packed in the same word with selector
mstore(0, and(addr, 0xffffffffffffffffffffffffffffffffffffffff))
mstore(0, addr)

performSystemCall(EVM_GAS_MANAGER_CONTRACT(), 32)

Expand Down Expand Up @@ -764,11 +768,11 @@ function performDelegateCall(oldSp, evmGasLeft, isStatic, oldStackHead) -> newGa
}

function _genericPrecallLogic(rawAddr, argsOffset, argsSize, retOffset, retSize) -> addr, gasUsed {
addr := and(rawAddr, 0xffffffffffffffffffffffffffffffffffffffff)

// memory_expansion_cost
gasUsed := expandMemory2(retOffset, retSize, argsOffset, argsSize)

addr := and(rawAddr, ADDRESS_MASK())

let addressAccessCost := 100 // warm address access cost
if iszero($llvm_AlwaysInline_llvm$_warmAddress(addr)) {
addressAccessCost := 2600 // cold address access cost
Expand Down Expand Up @@ -1076,8 +1080,7 @@ function _executeCreate(offset, size, value, evmGasLeftOld, isCreate2, salt) ->

if canBeDeployed {
returndatacopy(0, 0, 32)
addr := mload(0)

addr := and(mload(0), ADDRESS_MASK())
pop($llvm_AlwaysInline_llvm$_warmAddress(addr)) // will stay warm even if constructor reverts
// so even if constructor reverts, nonce stays incremented and addr stays warm

Expand Down
9 changes: 4 additions & 5 deletions system-contracts/evm-emulator/EvmEmulatorLoop.template.yul
Original file line number Diff line number Diff line change
Expand Up @@ -309,7 +309,7 @@ for { } true { } {
evmGasLeft := chargeGas(evmGasLeft, 100)

let addr := accessStackHead(sp, stackHead)
addr := and(addr, 0xffffffffffffffffffffffffffffffffffffffff)
addr := and(addr, ADDRESS_MASK())

if iszero($llvm_AlwaysInline_llvm$_warmAddress(addr)) {
evmGasLeft := chargeGas(evmGasLeft, 2500)
Expand Down Expand Up @@ -452,7 +452,7 @@ for { } true { } {

let addr := accessStackHead(sp, stackHead)

addr := and(addr, 0xffffffffffffffffffffffffffffffffffffffff)
addr := and(addr, ADDRESS_MASK())
if iszero($llvm_AlwaysInline_llvm$_warmAddress(addr)) {
evmGasLeft := chargeGas(evmGasLeft, 2500)
}
Expand Down Expand Up @@ -481,15 +481,14 @@ for { } true { } {
srcOffset, sp, stackHead := popStackItemWithoutCheck(sp, stackHead)
len, sp, stackHead := popStackItemWithoutCheck(sp, stackHead)

addr := and(addr, 0xffffffffffffffffffffffffffffffffffffffff)

// dynamicGas = 3 * minimum_word_size + memory_expansion_cost + address_access_cost
// minimum_word_size = (size + 31) / 32
let dynamicGas := add(
mul(3, shr(5, add(len, 31))),
expandMemory(dstOffset, len)
)

addr := and(addr, ADDRESS_MASK())
if iszero($llvm_AlwaysInline_llvm$_warmAddress(addr)) {
dynamicGas := add(dynamicGas, 2500)
}
Expand Down Expand Up @@ -548,7 +547,7 @@ for { } true { } {
evmGasLeft := chargeGas(evmGasLeft, 100)

let addr := accessStackHead(sp, stackHead)
addr := and(addr, 0xffffffffffffffffffffffffffffffffffffffff)
addr := and(addr, ADDRESS_MASK())

if iszero($llvm_AlwaysInline_llvm$_warmAddress(addr)) {
evmGasLeft := chargeGas(evmGasLeft, 2500)
Expand Down

0 comments on commit 816c358

Please sign in to comment.