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.

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

[KGA-29] [KGA-136] fix: cases with Cairo precompiles and delegatecalls #1567

Merged
merged 11 commits into from
Nov 15, 2024
17 changes: 7 additions & 10 deletions cairo_zero/kakarot/interpreter.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -67,20 +67,11 @@
let is_precompile = PrecompilesHelpers.is_precompile(evm.message.code_address.evm);
if (is_precompile != FALSE) {
let parent_context = evm.message.parent;
let is_parent_zero = Helpers.is_zero(cast(parent_context, felt));
if (is_parent_zero != FALSE) {
// Case A: The precompile is called straight from an EOA
tempvar caller_code_address = evm.message.caller;
} else {
// Case B: The precompile is called from a contract
tempvar caller_code_address = parent_context.evm.message.code_address.evm;
}
tempvar caller_address = evm.message.caller;
let (output_len, output, gas_used, revert_code) = Precompiles.exec_precompile(
evm.message.code_address.evm,
evm.message.calldata_len,
evm.message.calldata,
caller_code_address,
caller_address,
evm.message.address.evm,
);
Expand All @@ -101,9 +92,15 @@
}
let range_check_ptr = [ap - 2];
let evm = cast([ap - 1], model.EVM*);

// Only count the cairo precompile if it was executed and did not revert.
// If it did revert, we're ensured no state changes were made in the cairo call.
let is_cairo_precompile_called = PrecompilesHelpers.is_kakarot_precompile(
evm.message.code_address.evm
);
let is_cairo_precompile_executed = is_cairo_precompile_called * (
1 - precompile_reverted
);

Check warning on line 103 in cairo_zero/kakarot/interpreter.cairo

View check run for this annotation

Codecov / codecov/patch

cairo_zero/kakarot/interpreter.cairo#L101-L103

Added lines #L101 - L103 were not covered by tests
tempvar message = new model.Message(
bytecode=evm.message.bytecode,
bytecode_len=evm.message.bytecode_len,
Expand All @@ -120,7 +117,7 @@
is_create=evm.message.is_create,
depth=evm.message.depth,
env=evm.message.env,
cairo_precompile_called=is_cairo_precompile_called,
cairo_precompile_called=is_cairo_precompile_executed,

Check warning on line 120 in cairo_zero/kakarot/interpreter.cairo

View check run for this annotation

Codecov / codecov/patch

cairo_zero/kakarot/interpreter.cairo#L120

Added line #L120 was not covered by tests
);
tempvar evm = new model.EVM(
message=message,
Expand Down
4 changes: 1 addition & 3 deletions cairo_zero/kakarot/precompiles/precompiles.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ namespace Precompiles {
// @param precompile_address The precompile evm_address.
// @param input_len The length of the input array.
// @param input The input array.
// @param caller_code_address The address of the code of the contract that calls the precompile.
// @param caller_address The address of the caller of the precompile. Delegatecall rules apply.
// @param message_address The address being executed in the current message.
// @return output_len The output length.
Expand All @@ -48,7 +47,6 @@ namespace Precompiles {
precompile_address: felt,
input_len: felt,
input: felt*,
caller_code_address: felt,
caller_address: felt,
message_address: felt,
) -> (output_len: felt, output: felt*, gas_used: felt, reverted: felt) {
Expand Down Expand Up @@ -135,7 +133,7 @@ namespace Precompiles {

kakarot_precompile:
let is_call_authorized_ = PrecompilesHelpers.is_call_authorized(
precompile_address, caller_code_address, caller_address, message_address
precompile_address, caller_address, message_address
);
tempvar is_not_authorized = 1 - is_call_authorized_;
tempvar syscall_ptr = syscall_ptr;
Expand Down
22 changes: 13 additions & 9 deletions cairo_zero/kakarot/precompiles/precompiles_helpers.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -75,25 +75,30 @@

// @notice Returns whether the call to the precompile is authorized.
// @dev A call is authorized if:
// a. The precompile requires a whitelist AND the CODE_ADDRESS of the caller is whitelisted
// a. The precompile requires a whitelist AND the ADDRESS of the caller is whitelisted
// b. The precompile is CAIRO_MULTICALL_PRECOMPILE and the precompile address is the same as the message address (NOT a DELEGATECALL / CALLCODE).
// @param precompile_address The address of the precompile.
// @param caller_code_address The code_address of the precompile caller.
// @param caller_address The address of the caller.
// @param message_address The address being executed in the current message.
// @return Whether the call is authorized.
func is_call_authorized{syscall_ptr: felt*, pedersen_ptr: HashBuiltin*, range_check_ptr}(
precompile_address: felt,
caller_code_address: felt,
caller_address: felt,
message_address: felt,
precompile_address: felt, caller_address: felt, message_address: felt
) -> felt {
alloc_locals;
let precompile_requires_whitelist = requires_whitelist(precompile_address);

tempvar is_not_delegatecall: felt = Helpers.is_zero(message_address - precompile_address);

// Ensure that calls to precompiles that require a whitelist are properly authorized.
if (precompile_requires_whitelist == TRUE) {
let is_whitelisted = is_caller_whitelisted(caller_code_address);
// If the call is not a DELEGATECALL / CALLCODE, the actual caller is the caller_address
// Otherwise, the actual caller is the message_address.
if (is_not_delegatecall != FALSE) {
tempvar actual_caller_address = caller_address;
} else {
tempvar actual_caller_address = message_address;

Check warning on line 99 in cairo_zero/kakarot/precompiles/precompiles_helpers.cairo

View check run for this annotation

Codecov / codecov/patch

cairo_zero/kakarot/precompiles/precompiles_helpers.cairo#L99

Added line #L99 was not covered by tests
}
let is_whitelisted = is_caller_whitelisted(actual_caller_address);
ClementWalter marked this conversation as resolved.
Show resolved Hide resolved
tempvar syscall_ptr = syscall_ptr;
tempvar pedersen_ptr = pedersen_ptr;
tempvar range_check_ptr = range_check_ptr;
Expand All @@ -109,11 +114,10 @@
let range_check_ptr = [ap - 2];
let authorized = [ap - 1];

// Ensure that calls to CAIRO_CALL_PRECOMPILE or CAIRO_CALL_PRECOMPILE are not made through
// Ensure that calls to CAIRO_CALL_PRECOMPILE or CAIRO_MULTICALL_PRECOMPILE are not made through
// a delegatecall / callcode.
let is_delegatecall_protected_ = is_delegatecall_protected(precompile_address);
if (is_delegatecall_protected_ != FALSE) {
let is_not_delegatecall = Helpers.is_zero(message_address - precompile_address);
tempvar authorized = authorized * is_not_delegatecall;
} else {
tempvar authorized = authorized;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,15 +25,13 @@ func test__precompiles_run{
// Given
local address;
local input_len;
local caller_code_address;
local caller_address;
local message_address;
let (local input) = alloc();
%{
ids.address = program_input["address"]
ids.input_len = len(program_input["input"])
segments.write_arg(ids.input, program_input["input"])
ids.caller_code_address = program_input.get("caller_code_address", 0)
ids.caller_address = program_input.get("caller_address", 0)
ids.message_address = program_input.get("message_address", 0)
%}
Expand All @@ -43,7 +41,6 @@ func test__precompiles_run{
precompile_address=address,
input_len=input_len,
input=input,
caller_code_address=caller_code_address,
caller_address=caller_address,
message_address=message_address,
);
Expand Down
47 changes: 22 additions & 25 deletions cairo_zero/tests/src/kakarot/precompiles/test_precompiles.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@
)
from tests.utils.syscall_handler import SyscallHandler

AUTHORIZED_CALLER_CODE = 0xA7071ED
UNAUTHORIZED_CALLER_CODE = 0xC0C0C0
AUTHORIZED_CALLER_ADDRESS = 0xA7071ED
UNAUTHORIZED_CALLER_ADDRESS = 0xC0C0C0
CALLER_ADDRESS = 0x123ABC432


Expand Down Expand Up @@ -80,7 +80,7 @@ def test__p256_verify_precompile(
class TestKakarotPrecompiles:
@SyscallHandler.patch(
"Kakarot_authorized_cairo_precompiles_callers",
AUTHORIZED_CALLER_CODE,
AUTHORIZED_CALLER_ADDRESS,
1,
)
@SyscallHandler.patch("deploy", lambda *_: [0])
Expand All @@ -103,38 +103,37 @@ def test_should_deploy_account_when_sender_starknet_address_zero(
+ f"{0x60:064x}" # data_offset
+ f"{0x00:064x}" # data_len
),
caller_code_address=AUTHORIZED_CALLER_CODE,
caller_address=CALLER_ADDRESS,
caller_address=AUTHORIZED_CALLER_ADDRESS,
message_address=0x75001,
)
assert not bool(reverted)
assert bytes(return_data) == b""
assert gas_used == CAIRO_PRECOMPILE_GAS

SyscallHandler.mock_deploy.assert_called_once()
return
assert SyscallHandler.mock_deploy.call_count == 1

@SyscallHandler.patch(
"Kakarot_authorized_cairo_precompiles_callers",
AUTHORIZED_CALLER_CODE,
AUTHORIZED_CALLER_ADDRESS,
1,
)
@SyscallHandler.patch(
"Kakarot_evm_to_starknet_address", CALLER_ADDRESS, 0x1234
)
@SyscallHandler.patch("deploy", lambda *_: [0])
@pytest.mark.parametrize(
"address, caller_code_address, input_data, expected_return_data, expected_reverted",
"address, caller_address, input_data, expected_return_data, expected_reverted",
[
(
0x75001,
AUTHORIZED_CALLER_CODE,
AUTHORIZED_CALLER_ADDRESS,
bytes.fromhex("0abcdef0"),
b"Kakarot: OutOfBoundsRead",
True,
), # invalid input
(
0x75001,
AUTHORIZED_CALLER_CODE,
AUTHORIZED_CALLER_ADDRESS,
bytes.fromhex(
f"{0xc0de:064x}"
+ f"{get_selector_from_name('inc'):064x}"
Expand All @@ -146,7 +145,7 @@ def test_should_deploy_account_when_sender_starknet_address_zero(
), # call_contract
(
0x75001,
AUTHORIZED_CALLER_CODE,
AUTHORIZED_CALLER_ADDRESS,
bytes.fromhex(
f"{0xc0de:064x}"
+ f"{get_selector_from_name('get'):064x}"
Expand All @@ -159,7 +158,7 @@ def test_should_deploy_account_when_sender_starknet_address_zero(
), # call_contract with return data
(
0x75001,
UNAUTHORIZED_CALLER_CODE,
UNAUTHORIZED_CALLER_ADDRESS,
bytes.fromhex("0abcdef0"),
b"Kakarot: unauthorizedPrecompile",
True,
Expand All @@ -176,7 +175,7 @@ def test__cairo_precompiles(
self,
cairo_run,
address,
caller_code_address,
caller_address,
input_data,
expected_return_data,
expected_reverted,
Expand All @@ -197,35 +196,34 @@ def test__cairo_precompiles(
"test__precompiles_run",
address=address,
input=input_data,
caller_code_address=caller_code_address,
caller_address=CALLER_ADDRESS,
caller_address=caller_address,
message_address=address,
)
assert bool(reverted) == expected_reverted
assert bytes(return_data) == expected_return_data
assert gas_used == (
CAIRO_PRECOMPILE_GAS
if caller_code_address == AUTHORIZED_CALLER_CODE
if caller_address == AUTHORIZED_CALLER_ADDRESS
else 0
)
return

class TestKakarotMessaging:
@SyscallHandler.patch(
"Kakarot_authorized_cairo_precompiles_callers",
AUTHORIZED_CALLER_CODE,
AUTHORIZED_CALLER_ADDRESS,
1,
)
@SyscallHandler.patch(
"Kakarot_l1_messaging_contract_address",
0xC0DE,
)
@pytest.mark.parametrize(
"address, caller_code_address, input_data, to_address, expected_reverted_return_data, expected_reverted",
"address, caller_address, input_data, to_address, expected_reverted_return_data, expected_reverted",
[
(
0x75002,
AUTHORIZED_CALLER_CODE,
AUTHORIZED_CALLER_ADDRESS,
encode(
["uint160", "bytes"], [0xC0DE, encode(["uint128"], [0x2A])]
),
Expand All @@ -235,15 +233,15 @@ class TestKakarotMessaging:
),
(
0x75002,
AUTHORIZED_CALLER_CODE,
AUTHORIZED_CALLER_ADDRESS,
encode(["uint160", "bytes"], [0xC0DE, 0x2A.to_bytes(1, "big")]),
0xC0DE,
b"",
False,
),
(
0x75002,
UNAUTHORIZED_CALLER_CODE,
UNAUTHORIZED_CALLER_ADDRESS,
bytes.fromhex("0abcdef0"),
0xC0DE,
b"Kakarot: unauthorizedPrecompile",
Expand All @@ -258,7 +256,7 @@ class TestKakarotMessaging:
)
def test__cairo_message(
self,
caller_code_address,
caller_address,
cairo_run,
address,
input_data,
Expand All @@ -271,8 +269,7 @@ def test__cairo_message(
"test__precompiles_run",
address=address,
input=input_data,
caller_code_address=caller_code_address,
caller_address=CALLER_ADDRESS,
caller_address=caller_address,
message_address=address,
)
if expected_reverted:
Expand Down
3 changes: 1 addition & 2 deletions kakarot_scripts/utils/kakarot.py
Original file line number Diff line number Diff line change
Expand Up @@ -712,8 +712,7 @@ async def eth_send_transaction(
typed_transaction = TypedTransaction.from_dict(payload)

evm_tx = EvmAccount.sign_transaction(
typed_transaction.as_dict(),
hex(evm_account.signer.private_key),
typed_transaction.as_dict(), f"{evm_account.signer.private_key:064x}"
)

if WEB3.is_connected():
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ library Internals {
mstore(0x40, add(result, add(0x20, size)))
}

require(success, string(abi.encodePacked("CairoLib: call_contract failed with: ", result)));
require(success, string(abi.encodePacked("CairoLib: cairo call failed with: ", result)));

return result;
}
Expand Down
Loading
Loading