Skip to content

Commit

Permalink
feat: call value transfer (kkrt-labs#539)
Browse files Browse the repository at this point in the history
<!--- Please provide a general summary of your changes in the title
above -->
Update the CALL opcode to include native token value transfer.

<!-- Give an estimate of the time you spent on this PR in terms of work
days. Did you spend 0.5 days on this PR or rather 2 days? -->

Time spent on this PR: 0.5 days

## Pull request type

<!-- Please try to limit your pull request to one type, submit multiple
pull requests if needed. -->

Please check the type of change your PR introduces:

- [x] Bugfix
- [x] Feature
- [ ] Code style update (formatting, renaming)
- [ ] Refactoring (no functional changes, no api changes)
- [ ] Build related changes
- [ ] Documentation content changes
- [ ] Other (please describe):

## What is the current behavior?
CALL opcode does not include native token value transfer.

<!-- Please describe the current behavior that you are modifying, or
link to a relevant issue. -->

Resolves kkrt-labs#465 

## What is the new behavior?

<!-- Please describe the behavior or changes that are being added by
this PR. -->

- Native token transfer is possible with the CALL opcode
- Unit testing
- Fix on SELFDESTRUCT opcode (update `transfer` to `transferFrom`).
  • Loading branch information
greged93 authored Mar 9, 2023
1 parent 85a6fca commit ba253c4
Show file tree
Hide file tree
Showing 5 changed files with 153 additions and 36 deletions.
30 changes: 27 additions & 3 deletions src/kakarot/instructions/system_operations.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,25 @@ namespace SystemOperations {
assert ctx.read_only * sub_ctx.call_context.value = FALSE;
}

if (sub_ctx.call_context.value == 0) {
return sub_ctx;
}

let (native_token_address_) = native_token_address.read();
let amount_u256 = Helpers.to_uint256(sub_ctx.call_context.value);
let sender = ctx.starknet_contract_address;
let recipient = sub_ctx.starknet_contract_address;
let (success) = IEth.transferFrom(
contract_address=native_token_address_,
sender=sender,
recipient=recipient,
amount=amount_u256,
);
with_attr error_message(
"Kakarot: 0xF1: failed to transfer token from {sender} to {recipient}") {
assert success = TRUE;
}

return sub_ctx;
}

Expand Down Expand Up @@ -355,10 +374,15 @@ namespace SystemOperations {
let (balance: Uint256) = IEth.balanceOf(
contract_address=native_token_address_, account=ctx.starknet_contract_address
);
let (success) = IEth.transfer(
contract_address=native_token_address_, recipient=address_felt, amount=balance
let sender = ctx.starknet_contract_address;
let (success) = IEth.transferFrom(
contract_address=native_token_address_,
sender=sender,
recipient=address_felt,
amount=balance
);
with_attr error_message("Kakarot: Transfer failed") {
with_attr error_message(
"Kakarot: 0xFF: failed to transfer token from {sender} to {address_felt}") {
assert success = TRUE;
}

Expand Down
3 changes: 3 additions & 0 deletions src/kakarot/interfaces/interfaces.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ namespace IEth {
func transfer(recipient: felt, amount: Uint256) -> (success: felt) {
}

func transferFrom(sender: felt, recipient: felt, amount: Uint256) -> (success: felt) {
}

func approve(spender: felt, amount: Uint256) -> (success: felt) {
}
}
Expand Down
4 changes: 3 additions & 1 deletion tests/unit/src/kakarot/accounts/eoa/test_library.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,9 @@ async def test_should_transfer_value_to_destination_address(
expected_balances = Counter()

# Storing initial balance, as eth storage persists across tests.
initial_balance = (await eth.balanceOf(mock_externally_owned_account.contract_address).call()).result.balance.low
initial_balance = (
await eth.balanceOf(mock_externally_owned_account.contract_address).call()
).result.balance.low

# Mint tokens to the EOA
await eth.mint(
Expand Down
113 changes: 88 additions & 25 deletions tests/unit/src/kakarot/instructions/test_system_operations.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
from starkware.cairo.common.alloc import alloc
from starkware.cairo.common.cairo_builtins import HashBuiltin, BitwiseBuiltin
from starkware.cairo.common.default_dict import default_dict_new
from starkware.cairo.common.uint256 import Uint256
from starkware.cairo.common.uint256 import Uint256, uint256_sub
from starkware.starknet.common.syscalls import deploy, get_contract_address
from starkware.cairo.common.math import split_felt, assert_not_zero, assert_le

Expand All @@ -28,7 +28,7 @@ from kakarot.instructions.system_operations import (
CreateHelper,
SelfDestructHelper,
)
from kakarot.interfaces.interfaces import IContractAccount, IKakarot, IAccount
from kakarot.interfaces.interfaces import IContractAccount, IKakarot, IAccount, IEth
from kakarot.library import Kakarot
from kakarot.accounts.library import Accounts
from kakarot.model import model
Expand All @@ -40,11 +40,10 @@ from utils.utils import Helpers
@constructor
func constructor{
syscall_ptr: felt*, pedersen_ptr: HashBuiltin*, range_check_ptr, bitwise_ptr: BitwiseBuiltin*
}(contract_account_class_hash_: felt, account_proxy_class_hash_) {
}(native_token_address_: felt, contract_account_class_hash_: felt, account_proxy_class_hash_) {
native_token_address.write(native_token_address_);
account_proxy_class_hash.write(account_proxy_class_hash_);
contract_account_class_hash.write(contract_account_class_hash_);
let (contract_address: felt) = get_contract_address();
native_token_address.write(contract_address);
return ();
}

Expand All @@ -67,17 +66,13 @@ func get_native_token{syscall_ptr: felt*, pedersen_ptr: HashBuiltin*, range_chec
return Kakarot.get_native_token();
}

//
// ETH ERC20
//

// @dev The contract account initialization includes a call to an ERC20 contract to set an infitite transfer allowance to Kakarot.
// As the ERC20 contract is not deployed within this test, we make a call to this contract instead.
// @dev mock function that returns the computed starknet address from an evm address
@external
func approve{syscall_ptr: felt*, pedersen_ptr: HashBuiltin*, range_check_ptr}(
spender: felt, amount: Uint256
) -> (success: felt) {
return ERC20.approve(spender, amount);
func compute_starknet_address{syscall_ptr: felt*, pedersen_ptr: HashBuiltin*, range_check_ptr}(
evm_address: felt
) -> (contract_address: felt) {
let (contract_address_) = Accounts.compute_starknet_address(evm_address);
return (contract_address=contract_address_);
}

//
Expand Down Expand Up @@ -173,19 +168,23 @@ func test__exec_revert{
func test__exec_call__should_return_a_new_context_based_on_calling_ctx_stack{
syscall_ptr: felt*, pedersen_ptr: HashBuiltin*, range_check_ptr, bitwise_ptr: BitwiseBuiltin*
}() {
// Deploy an empty contract
// Deploy two empty contract
alloc_locals;

let (contract_account_class_hash_) = contract_account_class_hash.read();
let (evm_contract_address) = CreateHelper.get_create_address(0, 0);
let (local starknet_contract_address) = Accounts.create(
contract_account_class_hash_, evm_contract_address
let (caller_evm_contract_address) = CreateHelper.get_create_address(0, 0);
let (caller_starknet_contract_address) = Accounts.create(
contract_account_class_hash_, caller_evm_contract_address
);
let (callee_evm_contract_address) = CreateHelper.get_create_address(1, 0);
let (callee_starknet_contract_address) = Accounts.create(
contract_account_class_hash_, callee_evm_contract_address
);

// Fill the stack with input data
let stack: model.Stack* = Stack.init();
let gas = Helpers.to_uint256(Constants.TRANSACTION_GAS_LIMIT);
let (address_high, address_low) = split_felt(evm_contract_address);
let (address_high, address_low) = split_felt(callee_evm_contract_address);
let address = Uint256(address_low, address_high);
tempvar value = Uint256(2, 0);
let args_offset = Uint256(3, 0);
Expand All @@ -208,7 +207,9 @@ func test__exec_call__should_return_a_new_context_based_on_calling_ctx_stack{
let stack = Stack.push(stack, memory_offset);
let (bytecode) = alloc();
local bytecode_len = 0;
let ctx = TestHelpers.init_context_with_stack(bytecode_len, bytecode, stack);
let ctx = TestHelpers.init_context_at_address_with_stack_and_caller_address(
caller_evm_contract_address, bytecode_len, bytecode, stack, caller_starknet_contract_address
);
let ctx = MemoryOperations.exec_mstore(ctx);

// When
Expand All @@ -230,8 +231,8 @@ func test__exec_call__should_return_a_new_context_based_on_calling_ctx_stack{
let (gas_felt, _) = Helpers.div_rem(Constants.TRANSACTION_GAS_LIMIT, 64);
assert_le(sub_ctx.gas_limit, gas_felt);
assert sub_ctx.gas_price = 0;
assert sub_ctx.starknet_contract_address = starknet_contract_address;
assert sub_ctx.evm_contract_address = evm_contract_address;
assert sub_ctx.starknet_contract_address = callee_starknet_contract_address;
assert sub_ctx.evm_contract_address = callee_evm_contract_address;
TestHelpers.assert_execution_context_equal(sub_ctx.calling_context, ctx);

// Fake a RETURN in sub_ctx then teardow, see note in evm.codes:
Expand All @@ -249,6 +250,69 @@ func test__exec_call__should_return_a_new_context_based_on_calling_ctx_stack{
return ();
}

@external
func test__exec_call__should_transfer_value{
syscall_ptr: felt*, pedersen_ptr: HashBuiltin*, range_check_ptr, bitwise_ptr: BitwiseBuiltin*
}() {
// Deploy two empty contract
alloc_locals;

let (contract_account_class_hash_) = contract_account_class_hash.read();
let (caller_evm_contract_address) = CreateHelper.get_create_address(0, 0);
let (caller_starknet_contract_address) = Accounts.create(
contract_account_class_hash_, caller_evm_contract_address
);
let (callee_evm_contract_address) = CreateHelper.get_create_address(1, 0);
let (callee_starknet_contract_address) = Accounts.create(
contract_account_class_hash_, callee_evm_contract_address
);

// Get the balance of caller pre-call
let (native_token_address_) = native_token_address.read();
let (caller_pre_balance) = IEth.balanceOf(contract_address=native_token_address_, account=caller_starknet_contract_address);

// Fill the stack with input data
let stack: model.Stack* = Stack.init();
let gas = Helpers.to_uint256(Constants.TRANSACTION_GAS_LIMIT);
let (address_high, address_low) = split_felt(callee_evm_contract_address);
let address = Uint256(address_low, address_high);
tempvar value = Uint256(2, 0);
let args_offset = Uint256(3, 0);
let args_size = Uint256(4, 0);
tempvar ret_offset = Uint256(5, 0);
tempvar ret_size = Uint256(6, 0);
let stack = Stack.push(stack, ret_size);
let stack = Stack.push(stack, ret_offset);
let stack = Stack.push(stack, args_size);
let stack = Stack.push(stack, args_offset);
let stack = Stack.push(stack, value);
let stack = Stack.push(stack, address);
let stack = Stack.push(stack, gas);
let memory_word = Uint256(low=0, high=22774453838368691922685013100469420032);
let memory_offset = Uint256(0, 0);
let stack = Stack.push(stack, memory_word);
let stack = Stack.push(stack, memory_offset);
let (bytecode) = alloc();
local bytecode_len = 0;
let ctx = TestHelpers.init_context_at_address_with_stack_and_caller_address(
caller_evm_contract_address, bytecode_len, bytecode, stack, caller_starknet_contract_address
);
let ctx = MemoryOperations.exec_mstore(ctx);

// When
let sub_ctx = SystemOperations.exec_call(ctx);

// Then
// get balances of caller and callee post-call
let (callee_balance) = IEth.balanceOf(contract_address=native_token_address_, account=callee_starknet_contract_address);
let (caller_post_balance) = IEth.balanceOf(contract_address=native_token_address_, account=caller_starknet_contract_address);
let (caller_diff_balance) = uint256_sub(caller_pre_balance, caller_post_balance);

assert callee_balance = Uint256(2, 0);
assert caller_diff_balance = Uint256(2, 0);
return();
}

@external
func test__exec_callcode__should_return_a_new_context_based_on_calling_ctx_stack{
syscall_ptr: felt*, pedersen_ptr: HashBuiltin*, range_check_ptr, bitwise_ptr: BitwiseBuiltin*
Expand Down Expand Up @@ -664,9 +728,8 @@ func test__exec_create2__should_return_a_new_context_with_bytecode_from_memory_a
@external
func test__exec_selfdestruct__should_delete_account_bytecode{
syscall_ptr: felt*, pedersen_ptr: HashBuiltin*, range_check_ptr, bitwise_ptr: BitwiseBuiltin*
}(evm_address: felt) {
}() {
alloc_locals;
native_token_address.write(evm_address);

// Create sub_ctx writing directly in memory because need to update calling_context
let (bytecode) = alloc();
Expand Down
39 changes: 32 additions & 7 deletions tests/unit/src/kakarot/instructions/test_system_operations.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,22 +10,38 @@
from tests.utils.helpers import get_create2_address, get_create_address
from tests.utils.uint256 import int_to_uint256

ZERO_ACCOUNT = "0x0000000000000000000000000000000000000000"


@pytest_asyncio.fixture(scope="module")
async def system_operations(
starknet: Starknet, contract_account_class, account_proxy_class
starknet: Starknet, eth, contract_account_class, account_proxy_class
):
return await starknet.deploy(
source="./tests/unit/src/kakarot/instructions/test_system_operations.cairo",
cairo_path=["src"],
constructor_calldata=[
eth.contract_address,
contract_account_class.class_hash,
account_proxy_class.class_hash,
],
disable_hint_validation=True,
)


@pytest_asyncio.fixture(scope="module")
async def mint(system_operations, eth):
async def _factory(evm_address: str, value: int):
# mint tokens to the provided evm address
sender = int(get_create_address(evm_address, 0), 16)
starket_contract_address = (
await system_operations.compute_starknet_address(sender).call()
).result.contract_address
await eth.mint(starket_contract_address, int_to_uint256(value)).execute()

return _factory


@pytest.mark.asyncio
class TestSystemOperations:
@pytest.mark.parametrize("size", range(34, 65))
Expand All @@ -44,15 +60,24 @@ async def test_return(self, system_operations):
1000
).call()

async def test_call(self, system_operations):
await system_operations.test__exec_call__should_return_a_new_context_based_on_calling_ctx_stack().call()
async def test_call(self, system_operations, mint):
await mint(ZERO_ACCOUNT, 2)
await system_operations.test__exec_call__should_return_a_new_context_based_on_calling_ctx_stack().call(
system_operations.contract_address
)

await system_operations.test__exec_callcode__should_return_a_new_context_based_on_calling_ctx_stack().call()

await system_operations.test__exec_staticcall__should_return_a_new_context_based_on_calling_ctx_stack().call()

await system_operations.test__exec_delegatecall__should_return_a_new_context_based_on_calling_ctx_stack().call()

async def test_call__should_transfer_value(self, system_operations, mint):
await mint(ZERO_ACCOUNT, 2)
await system_operations.test__exec_call__should_transfer_value().call(
system_operations.contract_address
)

@pytest.mark.parametrize("salt", [127, 256, 2**55 - 1])
async def test_create(self, system_operations, salt):
evm_caller_address_int = 15
Expand Down Expand Up @@ -95,7 +120,7 @@ async def test_create2(self, system_operations):
from_bytes(decode_hex(expected_create2_addr)),
).call()

async def test_selfdestruct(self, system_operations, eth):
await system_operations.test__exec_selfdestruct__should_delete_account_bytecode(
eth.contract_address,
).call()
async def test_selfdestruct(self, system_operations):
await system_operations.test__exec_selfdestruct__should_delete_account_bytecode().call(
system_operations.contract_address
)

0 comments on commit ba253c4

Please sign in to comment.