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

fix(EVM): Additional fixes after review #1196

Open
wants to merge 7 commits into
base: evm-emulator/fixes
Choose a base branch
from
+86 −83
Open

fix(EVM): Additional fixes after review #1196

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
3c77d4a
fix(EVM): Change additional stipends for calls to empty contracts (M-…
0xVolosnikov Jan 14, 2025
875d0a5
fix(EVM): Simplify padding (H-05) (#1195)
0xVolosnikov Jan 14, 2025
0d6fec7
fix(EVM): Increment nonce after EVM force deploy (L-04) (#1197)
0xVolosnikov Jan 14, 2025
16b76a2
fix(EVM): Use stricter checks for calldata (M-06, M-07) (#1200)
0xVolosnikov Jan 14, 2025
0e0a7ac
Add comments for delegate call implementation (L-10)
0xVolosnikov Jan 14, 2025
a9eb273
Apply suggested change
0xVolosnikov Jan 14, 2025
bbfd8e7
Charge additional 32k gas in contract creation from EOA or EraVM cont…
0xVolosnikov Jan 14, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions system-contracts/contracts/ContractDeployer.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@
function createEVM(bytes calldata _initCode) external payable override onlySystemCall returns (uint256, address) {
uint256 senderNonce;
// If the account is an EOA, use the min nonce. If it's a contract, use deployment nonce
if (msg.sender == tx.origin) {

Check warning on line 179 in system-contracts/contracts/ContractDeployer.sol

View workflow job for this annotation

GitHub Actions / lint 

Avoid to use tx.origin

Check warning on line 179 in system-contracts/contracts/ContractDeployer.sol

View workflow job for this annotation

GitHub Actions / lint 

Avoid to use tx.origin

Check warning on line 179 in system-contracts/contracts/ContractDeployer.sol

View workflow job for this annotation

GitHub Actions / lint 

Avoid to use tx.origin
// Subtract 1 for EOA since the nonce has already been incremented for this transaction
senderNonce = NONCE_HOLDER_SYSTEM_CONTRACT.getMinNonce(msg.sender) - 1;
} else {
Expand Down Expand Up @@ -318,6 +318,12 @@
// contract is acceptable.

if (Utils.isCodeHashEVM(_deployment.bytecodeHash)) {
// Note, that for contracts the "nonce" is set as deployment nonce.
uint256 deploymentNonce = NONCE_HOLDER_SYSTEM_CONTRACT.getDeploymentNonce(_deployment.newAddress);
if (deploymentNonce == 0) {
NONCE_HOLDER_SYSTEM_CONTRACT.incrementDeploymentNonce(_deployment.newAddress);
}

// It is not possible to change the AccountInfo for EVM contracts.
_constructEVMContract(_sender, _deployment.newAddress, _deployment.input);
} else {
Expand Down
103 changes: 50 additions & 53 deletions system-contracts/contracts/EvmEmulator.yul
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,12 @@ object "EvmEmulator" {

blobLen := len

if iszero(eq(mod(blobLen, 32), 0)) {
if mod(blobLen, 32) {
blobLen := add(blobLen, sub(32, mod(blobLen, 32)))
}

// Now it is divisible by 32, but we must make sure that the number of 32 byte words is odd
if iszero(eq(mod(blobLen, 64), 32)) {
if iszero(mod(blobLen, 64)) {
blobLen := add(blobLen, 32)
}
}
Expand Down Expand Up @@ -186,6 +186,8 @@ object "EvmEmulator" {

function MAX_UINT32() -> ret { ret := 4294967295 } // 2^32 - 1

function MAX_CALLDATA_OFFSET() -> ret { ret := sub(MAX_UINT32(), 32) } // EraVM will panic if offset + length overflows u32

function EMPTY_KECCAK() -> value { // keccak("")
value := 0xc5d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470
}
Expand Down Expand Up @@ -804,14 +806,16 @@ object "EvmEmulator" {
}

if isCallToEmptyContract {
// In case of a call to the EVM contract that is currently being constructed,
// the DefaultAccount bytecode will be used instead. This is implemented at the virtual machine level.
success := delegatecall(gas(), addr, argsOffset, argsSize, retOffset, retSize)
_saveReturndataAfterZkEVMCall()
}

// We forbid delegatecalls to EraVM native contracts
}
default {
// Precompile. Simlate using staticcall, since EraVM behavior differs here
// Precompile. Simulate using staticcall, since EraVM behavior differs here
success, frameGasLeft := callPrecompile(addr, precompileCost, gasToPass, 0, argsOffset, argsSize, retOffset, retSize, true)
}
}
Expand Down Expand Up @@ -909,25 +913,21 @@ object "EvmEmulator" {
function callZkVmNative(addr, evmGasToPass, value, argsOffset, argsSize, retOffset, retSize, isStatic, rawCodeHash) -> success, frameGasLeft {
let zkEvmGasToPass := mul(evmGasToPass, GAS_DIVISOR()) // convert EVM gas -> ZkVM gas

let additionalStipend := 6000 // should cover first access to empty account
switch value
case 0 {
if gt(addr, 0) { // zero address is always "empty"
if and(shr(224, rawCodeHash), 0xffff) { // if codelen is not zero
additionalStipend := 0
}
let emptyContractExecutionCost := 500 // enough to call "empty" contract
let isEmptyContract := or(iszero(addr), iszero(and(shr(224, rawCodeHash), 0xffff)))
if isEmptyContract {
// we should add some gas to cover overhead of calling EmptyContract or DefaultAccount
// if value isn't zero, MsgValueSimulator will take required gas directly from our frame (as 2300 stipend)
if iszero(value) {
zkEvmGasToPass := add(zkEvmGasToPass, emptyContractExecutionCost)
}
}
default {
additionalStipend := 27000 // Stipend for MsgValueSimulator. Covered by positive_value_cost
}

zkEvmGasToPass := add(zkEvmGasToPass, additionalStipend)

if gt(zkEvmGasToPass, MAX_UINT32()) { // just in case
zkEvmGasToPass := MAX_UINT32()
}

// Please note, that decommitment cost and MsgValueSimulator additional overhead will be charged directly from this frame
let zkEvmGasBefore := gas()
switch isStatic
case 0 {
Expand All @@ -944,16 +944,14 @@ object "EvmEmulator" {
zkEvmGasUsed := 0 // should never happen
}

switch gt(zkEvmGasUsed, additionalStipend)
case 0 {
zkEvmGasUsed := 0
}
default {
zkEvmGasUsed := sub(zkEvmGasUsed, additionalStipend)
if isEmptyContract {
if iszero(value) {
zkEvmGasToPass := sub(zkEvmGasToPass, emptyContractExecutionCost)
}

zkEvmGasUsed := 0 // Calling empty contracts is free from the EVM point of view
}

zkEvmGasToPass := sub(zkEvmGasToPass, additionalStipend)

// refund gas
if gt(zkEvmGasToPass, zkEvmGasUsed) {
frameGasLeft := div(sub(zkEvmGasToPass, zkEvmGasUsed), GAS_DIVISOR())
Expand Down Expand Up @@ -1709,14 +1707,14 @@ object "EvmEmulator" {
dstOffset := add(dstOffset, MEM_OFFSET())

// EraVM will revert if offset + length overflows uint32
if gt(sourceOffset, MAX_UINT32()) {
sourceOffset := MAX_UINT32()
if gt(sourceOffset, MAX_CALLDATA_OFFSET()) {
sourceOffset := MAX_CALLDATA_OFFSET()
}

// Check bytecode out-of-bounds access
let truncatedLen := len
if gt(add(sourceOffset, len), MAX_UINT32()) {
truncatedLen := sub(MAX_UINT32(), sourceOffset) // truncate
if gt(add(sourceOffset, len), MAX_CALLDATA_OFFSET()) { // in theory we could also copy MAX_CALLDATA_OFFSET slot, but it is unreachable
truncatedLen := sub(MAX_CALLDATA_OFFSET(), sourceOffset) // truncate
$llvm_AlwaysInline_llvm$_memsetToZero(add(dstOffset, truncatedLen), sub(len, truncatedLen)) // pad with zeroes any out-of-bounds
}

Expand Down Expand Up @@ -2949,6 +2947,7 @@ object "EvmEmulator" {

if iszero(isCallerEVM) {
evmGasLeft := getEvmGasFromContext()
evmGasLeft := chargeGas(evmGasLeft, 32000)
}

let offset, len, gasToReturn := simulate(isCallerEVM, evmGasLeft, false)
Expand Down Expand Up @@ -3112,6 +3111,8 @@ object "EvmEmulator" {

function MAX_UINT32() -> ret { ret := 4294967295 } // 2^32 - 1

function MAX_CALLDATA_OFFSET() -> ret { ret := sub(MAX_UINT32(), 32) } // EraVM will panic if offset + length overflows u32

function EMPTY_KECCAK() -> value { // keccak("")
value := 0xc5d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470
}
Expand Down Expand Up @@ -3730,14 +3731,16 @@ object "EvmEmulator" {
}

if isCallToEmptyContract {
// In case of a call to the EVM contract that is currently being constructed,
// the DefaultAccount bytecode will be used instead. This is implemented at the virtual machine level.
success := delegatecall(gas(), addr, argsOffset, argsSize, retOffset, retSize)
_saveReturndataAfterZkEVMCall()
}

// We forbid delegatecalls to EraVM native contracts
}
default {
// Precompile. Simlate using staticcall, since EraVM behavior differs here
// Precompile. Simulate using staticcall, since EraVM behavior differs here
success, frameGasLeft := callPrecompile(addr, precompileCost, gasToPass, 0, argsOffset, argsSize, retOffset, retSize, true)
}
}
Expand Down Expand Up @@ -3835,25 +3838,21 @@ object "EvmEmulator" {
function callZkVmNative(addr, evmGasToPass, value, argsOffset, argsSize, retOffset, retSize, isStatic, rawCodeHash) -> success, frameGasLeft {
let zkEvmGasToPass := mul(evmGasToPass, GAS_DIVISOR()) // convert EVM gas -> ZkVM gas

let additionalStipend := 6000 // should cover first access to empty account
switch value
case 0 {
if gt(addr, 0) { // zero address is always "empty"
if and(shr(224, rawCodeHash), 0xffff) { // if codelen is not zero
additionalStipend := 0
}
let emptyContractExecutionCost := 500 // enough to call "empty" contract
let isEmptyContract := or(iszero(addr), iszero(and(shr(224, rawCodeHash), 0xffff)))
if isEmptyContract {
// we should add some gas to cover overhead of calling EmptyContract or DefaultAccount
// if value isn't zero, MsgValueSimulator will take required gas directly from our frame (as 2300 stipend)
if iszero(value) {
zkEvmGasToPass := add(zkEvmGasToPass, emptyContractExecutionCost)
}
}
default {
additionalStipend := 27000 // Stipend for MsgValueSimulator. Covered by positive_value_cost
}

zkEvmGasToPass := add(zkEvmGasToPass, additionalStipend)

if gt(zkEvmGasToPass, MAX_UINT32()) { // just in case
zkEvmGasToPass := MAX_UINT32()
}

// Please note, that decommitment cost and MsgValueSimulator additional overhead will be charged directly from this frame
let zkEvmGasBefore := gas()
switch isStatic
case 0 {
Expand All @@ -3870,16 +3869,14 @@ object "EvmEmulator" {
zkEvmGasUsed := 0 // should never happen
}

switch gt(zkEvmGasUsed, additionalStipend)
case 0 {
zkEvmGasUsed := 0
}
default {
zkEvmGasUsed := sub(zkEvmGasUsed, additionalStipend)
if isEmptyContract {
if iszero(value) {
zkEvmGasToPass := sub(zkEvmGasToPass, emptyContractExecutionCost)
}

zkEvmGasUsed := 0 // Calling empty contracts is free from the EVM point of view
}

zkEvmGasToPass := sub(zkEvmGasToPass, additionalStipend)

// refund gas
if gt(zkEvmGasToPass, zkEvmGasUsed) {
frameGasLeft := div(sub(zkEvmGasToPass, zkEvmGasUsed), GAS_DIVISOR())
Expand Down Expand Up @@ -4623,14 +4620,14 @@ object "EvmEmulator" {
dstOffset := add(dstOffset, MEM_OFFSET())

// EraVM will revert if offset + length overflows uint32
if gt(sourceOffset, MAX_UINT32()) {
sourceOffset := MAX_UINT32()
if gt(sourceOffset, MAX_CALLDATA_OFFSET()) {
sourceOffset := MAX_CALLDATA_OFFSET()
}

// Check bytecode out-of-bounds access
let truncatedLen := len
if gt(add(sourceOffset, len), MAX_UINT32()) {
truncatedLen := sub(MAX_UINT32(), sourceOffset) // truncate
if gt(add(sourceOffset, len), MAX_CALLDATA_OFFSET()) { // in theory we could also copy MAX_CALLDATA_OFFSET slot, but it is unreachable
truncatedLen := sub(MAX_CALLDATA_OFFSET(), sourceOffset) // truncate
$llvm_AlwaysInline_llvm$_memsetToZero(add(dstOffset, truncatedLen), sub(len, truncatedLen)) // pad with zeroes any out-of-bounds
}

Expand Down Expand Up @@ -5856,7 +5853,7 @@ object "EvmEmulator" {

function $llvm_AlwaysInline_llvm$_calldataload(calldataOffset) -> res {
// EraVM will revert if offset + length overflows uint32
if lt(calldataOffset, MAX_UINT32()) {
if lt(calldataOffset, MAX_CALLDATA_OFFSET()) { // in theory we could also copy MAX_CALLDATA_OFFSET slot, but it is unreachable
res := calldataload(calldataOffset)
}
}
Expand Down
4 changes: 2 additions & 2 deletions system-contracts/contracts/precompiles/CodeOracle.yul
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,12 +114,12 @@ object "CodeOracle" {
function paddedBytecodeLen(len) -> blobLen {
blobLen := len

if iszero(eq(mod(blobLen, 32), 0)) {
if mod(blobLen, 32) {
blobLen := add(blobLen, sub(32, mod(blobLen, 32)))
}

// Now it is divisible by 32, but we must make sure that the number of 32 byte words is odd
if iszero(eq(mod(blobLen, 64), 32)) {
if iszero(mod(blobLen, 64)) {
blobLen := add(blobLen, 32)
}
}
Expand Down
6 changes: 4 additions & 2 deletions system-contracts/evm-emulator/EvmEmulator.template.yul
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,12 @@ object "EvmEmulator" {

blobLen := len

if iszero(eq(mod(blobLen, 32), 0)) {
if mod(blobLen, 32) {
blobLen := add(blobLen, sub(32, mod(blobLen, 32)))
}

// Now it is divisible by 32, but we must make sure that the number of 32 byte words is odd
if iszero(eq(mod(blobLen, 64), 32)) {
if iszero(mod(blobLen, 64)) {
blobLen := add(blobLen, 32)
}
}
Expand Down Expand Up @@ -90,6 +90,8 @@ object "EvmEmulator" {

if iszero(isCallerEVM) {
evmGasLeft := getEvmGasFromContext()
// Charge additional creation cost
evmGasLeft := chargeGas(evmGasLeft, 32000)
}

let offset, len, gasToReturn := simulate(isCallerEVM, evmGasLeft, false)
Expand Down
40 changes: 19 additions & 21 deletions system-contracts/evm-emulator/EvmEmulatorFunctions.template.yul
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,8 @@ function OVERHEAD() -> overhead { overhead := 2000 }

function MAX_UINT32() -> ret { ret := 4294967295 } // 2^32 - 1

function MAX_CALLDATA_OFFSET() -> ret { ret := sub(MAX_UINT32(), 32) } // EraVM will panic if offset + length overflows u32

function EMPTY_KECCAK() -> value { // keccak("")
value := 0xc5d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470
}
Expand Down Expand Up @@ -744,14 +746,16 @@ function performDelegateCall(oldSp, evmGasLeft, isStatic, oldStackHead) -> newGa
}

if isCallToEmptyContract {
// In case of a call to the EVM contract that is currently being constructed,
// the DefaultAccount bytecode will be used instead. This is implemented at the virtual machine level.
success := delegatecall(gas(), addr, argsOffset, argsSize, retOffset, retSize)
_saveReturndataAfterZkEVMCall()
}

// We forbid delegatecalls to EraVM native contracts
}
default {
// Precompile. Simlate using staticcall, since EraVM behavior differs here
// Precompile. Simulate using staticcall, since EraVM behavior differs here
success, frameGasLeft := callPrecompile(addr, precompileCost, gasToPass, 0, argsOffset, argsSize, retOffset, retSize, true)
}
}
Expand Down Expand Up @@ -849,25 +853,21 @@ function callPrecompile(addr, precompileCost, gasToPass, value, argsOffset, args
function callZkVmNative(addr, evmGasToPass, value, argsOffset, argsSize, retOffset, retSize, isStatic, rawCodeHash) -> success, frameGasLeft {
let zkEvmGasToPass := mul(evmGasToPass, GAS_DIVISOR()) // convert EVM gas -> ZkVM gas

let additionalStipend := 6000 // should cover first access to empty account
switch value
case 0 {
if gt(addr, 0) { // zero address is always "empty"
if and(shr(224, rawCodeHash), 0xffff) { // if codelen is not zero
additionalStipend := 0
}
let emptyContractExecutionCost := 500 // enough to call "empty" contract
let isEmptyContract := or(iszero(addr), iszero(and(shr(224, rawCodeHash), 0xffff)))
if isEmptyContract {
// we should add some gas to cover overhead of calling EmptyContract or DefaultAccount
// if value isn't zero, MsgValueSimulator will take required gas directly from our frame (as 2300 stipend)
if iszero(value) {
zkEvmGasToPass := add(zkEvmGasToPass, emptyContractExecutionCost)
}
}
default {
additionalStipend := 27000 // Stipend for MsgValueSimulator. Covered by positive_value_cost
}

zkEvmGasToPass := add(zkEvmGasToPass, additionalStipend)

if gt(zkEvmGasToPass, MAX_UINT32()) { // just in case
zkEvmGasToPass := MAX_UINT32()
}

// Please note, that decommitment cost and MsgValueSimulator additional overhead will be charged directly from this frame
let zkEvmGasBefore := gas()
switch isStatic
case 0 {
Expand All @@ -884,16 +884,14 @@ function callZkVmNative(addr, evmGasToPass, value, argsOffset, argsSize, retOffs
zkEvmGasUsed := 0 // should never happen
}

switch gt(zkEvmGasUsed, additionalStipend)
case 0 {
zkEvmGasUsed := 0
}
default {
zkEvmGasUsed := sub(zkEvmGasUsed, additionalStipend)
if isEmptyContract {
if iszero(value) {
zkEvmGasToPass := sub(zkEvmGasToPass, emptyContractExecutionCost)
}

zkEvmGasUsed := 0 // Calling empty contracts is free from the EVM point of view
}

zkEvmGasToPass := sub(zkEvmGasToPass, additionalStipend)

// refund gas
if gt(zkEvmGasToPass, zkEvmGasUsed) {
frameGasLeft := div(sub(zkEvmGasToPass, zkEvmGasUsed), GAS_DIVISOR())
Expand Down
8 changes: 4 additions & 4 deletions system-contracts/evm-emulator/EvmEmulatorLoop.template.yul
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -373,14 +373,14 @@ for { } true { } {
dstOffset := add(dstOffset, MEM_OFFSET())

// EraVM will revert if offset + length overflows uint32
if gt(sourceOffset, MAX_UINT32()) {
sourceOffset := MAX_UINT32()
if gt(sourceOffset, MAX_CALLDATA_OFFSET()) {
sourceOffset := MAX_CALLDATA_OFFSET()
}

// Check bytecode out-of-bounds access
let truncatedLen := len
if gt(add(sourceOffset, len), MAX_UINT32()) {
truncatedLen := sub(MAX_UINT32(), sourceOffset) // truncate
if gt(add(sourceOffset, len), MAX_CALLDATA_OFFSET()) { // in theory we could also copy MAX_CALLDATA_OFFSET slot, but it is unreachable
truncatedLen := sub(MAX_CALLDATA_OFFSET(), sourceOffset) // truncate
$llvm_AlwaysInline_llvm$_memsetToZero(add(dstOffset, truncatedLen), sub(len, truncatedLen)) // pad with zeroes any out-of-bounds
}

Expand Down
Loading
Loading