Skip to content
This repository has been archived by the owner on Jan 9, 2025. It is now read-only.

Commit

Permalink
[KGA-19] dev: use Cairo1Helpers class for 'call_contract' for future-…
Browse files Browse the repository at this point in the history
…proofness (#1625)

Uses a call to the Cairo1Helpers class to access the "new" call_contract
syscall, that will __in the future__ have the ability to gracefully
handle failed contract calls.

NOTE: The change implemented in the following PR does not fix [KGA-19]
(code-423n4/2024-09-kakarot-findings#49)
because the Starknet network does not allow handling failed calls yet.

Attention to reviewer: The change to the Cairo1Helpers class needs to be
merged in kakarot-ssj first. Discussion can be made here
  • Loading branch information
enitrat authored Nov 25, 2024
1 parent ae13d79 commit 0e61ea9
Show file tree
Hide file tree
Showing 8 changed files with 358 additions and 275 deletions.
4 changes: 2 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ endif

.PHONY: build test coverage clean

# 176384150 corresponds to release v0.1.13 of Kakarot SSJ.
KKRT_SSJ_RELEASE_ID = 176384150
# 176384150 corresponds to release v0.1.16 of Kakarot SSJ.
KKRT_SSJ_RELEASE_ID = 186371784
# Kakarot SSJ artifacts for precompiles.
KKRT_SSJ_BUILD_ARTIFACT_URL = $(shell curl -L https://api.github.com/repos/kkrt-labs/kakarot-ssj/releases/${KKRT_SSJ_RELEASE_ID} | jq -r '.assets[0].browser_download_url')
ROOT_DIR:=$(shell dirname $(realpath $(lastword $(MAKEFILE_LIST))))
Expand Down
43 changes: 40 additions & 3 deletions cairo/kakarot-ssj/crates/contracts/src/cairo1_helpers.cairo
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use core::starknet::{EthAddress, secp256_trait::Signature};
use core::starknet::{EthAddress, secp256_trait::Signature, ContractAddress};

#[starknet::interface]
pub trait IPrecompiles<T> {
Expand Down Expand Up @@ -103,20 +103,41 @@ pub trait IHelpers<T> {
fn verify_signature_secp256r1(
self: @T, msg_hash: u256, r: u256, s: u256, x: u256, y: u256
) -> bool;


/// Calls a contract using the new Cairo call_contract_syscall.
///
/// Meant to be used from Cairo Zero classes that want to be able to manage the return value of
/// the call.
/// Only applicable from Starknet v0.13.4 and onwwards.
///
/// # Arguments
///
/// * `to` - The address of the contract to call.
/// * `selector` - The selector of the function to call.
/// * `calldata` - The calldata to pass to the function.
///
/// # Returns
/// A tuple containing:
/// * A boolean indicating whether the call was successful.
/// * The output of the call.
fn new_call_contract_syscall(
ref self: T, to: ContractAddress, selector: felt252, calldata: Span<felt252>
) -> (bool, Span<felt252>);
}


pub mod embeddable_impls {
use core::keccak::{cairo_keccak, keccak_u256s_be_inputs};
use core::num::traits::Zero;
use core::starknet::EthAddress;
use core::starknet::eth_signature::{verify_eth_signature};
use core::starknet::secp256_trait::{
Signature, recover_public_key, Secp256PointTrait, is_valid_signature
};
use core::starknet::secp256_trait::{Secp256Trait};
use core::starknet::secp256k1::Secp256k1Point;
use core::starknet::secp256r1::{Secp256r1Point};
use core::starknet::{ContractAddress, EthAddress};
use core::traits::Into;
use core::{starknet, starknet::SyscallResultTrait};
use evm::errors::EVMError;
Expand Down Expand Up @@ -149,7 +170,7 @@ pub mod embeddable_impls {
}

#[starknet::embeddable]
pub impl Helpers<TContractState> of super::IHelpers<TContractState> {
pub impl Helpers<TContractState, +Drop<TContractState>> of super::IHelpers<TContractState> {
fn get_block_hash(self: @TContractState, block_number: u64) -> felt252 {
starknet::syscalls::get_block_hash_syscall(block_number).unwrap_syscall()
}
Expand Down Expand Up @@ -217,6 +238,22 @@ pub mod embeddable_impls {

return is_valid_signature(msg_hash, r, s, public_key);
}

fn new_call_contract_syscall(
ref self: TContractState,
to: ContractAddress,
selector: felt252,
calldata: Span<felt252>
) -> (bool, Span<felt252>) {
// Note: until Starknet v0.13.4, the transaction will fail if the call reverted.
let result = starknet::syscalls::call_contract_syscall(to, selector, calldata);
match result {
Result::Ok(output) => { return (true, output); },
// This will need to be manually tested and enabled once contract calls can be
// handled.
Result::Err(error) => { return panic(error); }
}
}
}
}

Expand Down
517 changes: 265 additions & 252 deletions cairo/kakarot-ssj/crates/evm/src/precompiles/modexp.cairo

Large diffs are not rendered by default.

3 changes: 1 addition & 2 deletions cairo_zero/kakarot/accounts/account_contract.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -337,6 +337,5 @@ func execute_starknet_call{syscall_ptr: felt*, pedersen_ptr: HashBuiltin*, range
to: felt, function_selector: felt, calldata_len: felt, calldata: felt*
) -> (retdata_len: felt, retdata: felt*, success: felt) {
Ownable.assert_only_owner();
let (retdata_len, retdata) = call_contract(to, function_selector, calldata_len, calldata);
return (retdata_len, retdata, TRUE);
return AccountContract.execute_starknet_call(to, function_selector, calldata_len, calldata);
}
29 changes: 29 additions & 0 deletions cairo_zero/kakarot/accounts/library.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -374,6 +374,35 @@ namespace AccountContract {
Account_code_hash.write(code_hash);
return ();
}

// @notice Execute a starknet call.
// @dev Used when executing a Cairo Precompile. Used to preserve the caller address.
// @param to The address to call.
// @param function_selector The function selector to call.
// @param calldata_len The length of the calldata array.
// @param calldata The calldata for the call.
// @return retdata_len The length of the return data array.
// @return retdata The return data from the call.
// @return success 1 if the call was successful, 0 otherwise.
func execute_starknet_call{syscall_ptr: felt*, pedersen_ptr: HashBuiltin*, range_check_ptr}(
to: felt, function_selector: felt, calldata_len: felt, calldata: felt*
) -> (retdata_len: felt, retdata: felt*, success: felt) {
alloc_locals;
let (kakarot_address) = Ownable_owner.read();
let (helpers_class) = IKakarot.get_cairo1_helpers_class_hash(kakarot_address);
// Note: until Starknet v0.13.4, the transaction will always fail if the call reverted.
// Post starknet v0.13.4, it will be possible to handle failures in contract calls.
// Currently, `new_call_contract_syscall` is implemented to panic if the call reverted.
// Manual changes to the implementation in the Cairo1Helpers class will be needed to handle call reverts.
let (success, retdata_len, retdata) = ICairo1Helpers.library_call_new_call_contract_syscall(
class_hash=helpers_class,
to=to,
selector=function_selector,
calldata_len=calldata_len,
calldata=calldata,
);
return (retdata_len, retdata, success);
}
}

namespace Internals {
Expand Down
5 changes: 5 additions & 0 deletions cairo_zero/kakarot/interfaces/interfaces.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -243,4 +243,9 @@ namespace ICairo1Helpers {
input_len: felt, input: felt*, last_input_word: felt, last_input_num_bytes: felt
) -> (sha256_u32_array_len: felt, sha256_u32_array: felt*) {
}

func new_call_contract_syscall(
to: felt, selector: felt, calldata_len: felt, calldata: felt*
) -> (success: felt, retdata_len: felt, retdata: felt*) {
}
}
31 changes: 15 additions & 16 deletions cairo_zero/tests/src/kakarot/accounts/test_account_contract.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

from kakarot_scripts.data.pre_eip155_txs import PRE_EIP155_TX
from kakarot_scripts.utils.uint256 import int_to_uint256
from tests.utils.constants import CHAIN_ID, TRANSACTIONS
from tests.utils.constants import CAIRO1_HELPERS_CLASS_HASH, CHAIN_ID, TRANSACTIONS
from tests.utils.errors import cairo_error
from tests.utils.helpers import generate_random_private_key, rlp_encode_signed_data
from tests.utils.hints import patch_hint
Expand Down Expand Up @@ -263,28 +263,27 @@ def test_should_assert_only_owner(self, cairo_run):

@SyscallHandler.patch("Ownable_owner", SyscallHandler.caller_address)
def test_should_execute_starknet_call(self, cairo_run):
called_address = 0xABCDEF1234567890
function_selector = 0x0987654321FEDCBA
calldata = [random.randint(0, 255) for _ in range(32)]
expected_return_data = [random.randint(0, 255) for _ in range(32)] + [
int(True)
]
call_to = 0xABCDEF1234567890
call_selector = 0x0987654321FEDCBA
call_calldata = [random.randint(0, 255) for _ in range(32)]
call_retdata = [random.randint(0, 255) for _ in range(32)]
with SyscallHandler.patch(
function_selector, lambda *_: expected_return_data
"new_call_contract_syscall",
lambda *_: [int(True), len(call_retdata), *call_retdata],
):
return_data, success = cairo_run(
"test__execute_starknet_call",
called_address=called_address,
function_selector=function_selector,
calldata=calldata,
called_address=call_to,
function_selector=call_selector,
calldata=call_calldata,
)

assert return_data == expected_return_data
assert return_data == call_retdata
assert success == 1
SyscallHandler.mock_call.assert_any_call(
contract_address=called_address,
function_selector=function_selector,
calldata=calldata,
SyscallHandler.mock_library_call.assert_any_call(
class_hash=CAIRO1_HELPERS_CLASS_HASH,
function_selector=get_selector_from_name("new_call_contract_syscall"),
calldata=[call_to, call_selector, len(call_calldata), *call_calldata],
)

class TestExecuteFromOutside:
Expand Down
1 change: 1 addition & 0 deletions tests/utils/serde.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ def serialize_list(self, segment_ptr, item_scope=None, list_len=None):
list_len * item_size
if list_len is not None
else self.runner.segments.get_segment_size(segment_ptr.segment_index)
- segment_ptr.offset
)
output = []
for i in range(0, list_len, item_size):
Expand Down

0 comments on commit 0e61ea9

Please sign in to comment.