From 86d2bf5b95338711bd901a3fa87bb43119063eb7 Mon Sep 17 00:00:00 2001 From: Mathieu <60658558+enitrat@users.noreply.github.com> Date: Fri, 17 Nov 2023 14:33:56 +0300 Subject: [PATCH 1/9] fix: delegatecall caller (#543) * fix: delegatecall caller * dev: add caller to CallArgs struct --- .trunk/trunk.yaml | 4 ++-- crates/evm/src/call_helpers.cairo | 24 ++++++++++++++----- .../src/instructions/system_operations.cairo | 6 ++--- 3 files changed, 23 insertions(+), 11 deletions(-) diff --git a/.trunk/trunk.yaml b/.trunk/trunk.yaml index 73b154b27..fe91f5b54 100644 --- a/.trunk/trunk.yaml +++ b/.trunk/trunk.yaml @@ -32,12 +32,12 @@ lint: - actionlint@1.6.26 - bandit@1.7.5 - black@23.9.1 - - checkov@3.0.32 + - checkov@3.0.37 - git-diff-check - isort@5.12.0 - markdownlint@0.37.0 - oxipng@9.0.0 - - prettier@3.0.3 + - prettier@3.1.0 - ruff@0.1.5 - shellcheck@0.9.0 - shfmt@3.6.0 diff --git a/crates/evm/src/call_helpers.cairo b/crates/evm/src/call_helpers.cairo index 8d954a7a4..956ab213a 100644 --- a/crates/evm/src/call_helpers.cairo +++ b/crates/evm/src/call_helpers.cairo @@ -21,6 +21,7 @@ use utils::traits::{BoolIntoNumeric, U256TryIntoResult}; /// Created in order to simplify setting up the call opcodes #[derive(Drop)] struct CallArgs { + caller: Address, to: Address, gas: u128, value: u256, @@ -42,7 +43,7 @@ enum CallType { impl MachineCallHelpersImpl of MachineCallHelpers { /// Prepare the initialization of a new child or so-called sub-context /// As part of the CALL family of opcodes. - fn prepare_call(ref self: Machine, call_type: CallType) -> Result { + fn prepare_call(ref self: Machine, call_type: @CallType) -> Result { let gas = self.stack.pop_u128()?; let to = self.stack.pop_eth_address()?; @@ -56,6 +57,13 @@ impl MachineCallHelpersImpl of MachineCallHelpers { CallType::StaticCall => (0, false), }; + let caller = match call_type { + CallType::Call => self.address(), + CallType::DelegateCall => self.call_ctx().caller, + CallType::CallCode => self.address(), + CallType::StaticCall => self.address(), + }; + let args_offset = self.stack.pop_usize()?; let args_size = self.stack.pop_usize()?; @@ -67,7 +75,14 @@ impl MachineCallHelpersImpl of MachineCallHelpers { Result::Ok( CallArgs { - to, value, gas, calldata: calldata.span(), ret_offset, ret_size, should_transfer + caller, + to, + value, + gas, + calldata: calldata.span(), + ret_offset, + ret_size, + should_transfer } ) } @@ -100,11 +115,8 @@ impl MachineCallHelpersImpl of MachineCallHelpers { // We enter the standard flow let bytecode = self.state.get_account(call_args.to.evm)?.code; - // The caller in the subcontext is the current context's current address - let caller = self.address(); - let call_ctx = CallContextTrait::new( - caller, + call_args.caller, bytecode, call_args.calldata, call_args.value, diff --git a/crates/evm/src/instructions/system_operations.cairo b/crates/evm/src/instructions/system_operations.cairo index 2d5ff86c7..904efd79c 100644 --- a/crates/evm/src/instructions/system_operations.cairo +++ b/crates/evm/src/instructions/system_operations.cairo @@ -78,7 +78,7 @@ impl SystemOperations of SystemOperationsTrait { /// CALL /// # Specification: https://www.evm.codes/#f1?fork=shanghai fn exec_call(ref self: Machine) -> Result<(), EVMError> { - let call_args = self.prepare_call(CallType::Call)?; + let call_args = self.prepare_call(@CallType::Call)?; let read_only = self.read_only(); let value = call_args.value; @@ -102,7 +102,7 @@ impl SystemOperations of SystemOperationsTrait { /// STATICCALL /// # Specification: https://www.evm.codes/#fa?fork=shanghai fn exec_staticcall(ref self: Machine) -> Result<(), EVMError> { - let call_args = self.prepare_call(CallType::StaticCall)?; + let call_args = self.prepare_call(@CallType::StaticCall)?; let read_only = self.read_only(); // Initialize the sub context. @@ -118,7 +118,7 @@ impl SystemOperations of SystemOperationsTrait { /// DELEGATECALL /// # Specification: https://www.evm.codes/#f4?fork=shanghai fn exec_delegatecall(ref self: Machine) -> Result<(), EVMError> { - let call_args = self.prepare_call(CallType::DelegateCall)?; + let call_args = self.prepare_call(@CallType::DelegateCall)?; let read_only = self.read_only(); // Initialize the sub context. From b7b379e81751d16f51c1fb3c056b0ef1cc411671 Mon Sep 17 00:00:00 2001 From: Charlotte <49371958+chachaleo@users.noreply.github.com> Date: Mon, 20 Nov 2023 11:50:53 +0200 Subject: [PATCH 2/9] feat: chain_id in eao (#547) * feat: chain_id in eao * Update crates/contracts/src/tests/test_eoa.cairo Co-authored-by: Mathieu <60658558+enitrat@users.noreply.github.com> * Update crates/contracts/src/tests/test_eoa.cairo Co-authored-by: Mathieu <60658558+enitrat@users.noreply.github.com> * modif from review * modif from review * modif from review2 --------- Co-authored-by: Mathieu <60658558+enitrat@users.noreply.github.com> --- crates/contracts/src/eoa.cairo | 21 ++++--- crates/contracts/src/tests/test_eoa.cairo | 61 +++++++++++++------ crates/evm/src/model/eoa.cairo | 3 + .../evm/src/tests/test_model/test_eoa.cairo | 5 ++ 4 files changed, 63 insertions(+), 27 deletions(-) diff --git a/crates/contracts/src/eoa.cairo b/crates/contracts/src/eoa.cairo index 0d444199d..178213fa7 100644 --- a/crates/contracts/src/eoa.cairo +++ b/crates/contracts/src/eoa.cairo @@ -5,10 +5,12 @@ use starknet::{ContractAddress, EthAddress, ClassHash}; trait IExternallyOwnedAccount { fn kakarot_core_address(self: @TContractState) -> ContractAddress; fn evm_address(self: @TContractState) -> EthAddress; + fn chain_id(self: @TContractState) -> u128; /// Upgrade the ExternallyOwnedAccount smart contract /// Using replace_class_syscall fn upgrade(ref self: TContractState, new_class_hash: ClassHash); + fn set_chain_id(ref self: TContractState, chain_id: u128); } #[starknet::contract] @@ -29,6 +31,7 @@ mod ExternallyOwnedAccount { struct Storage { evm_address: EthAddress, kakarot_core_address: ContractAddress, + chain_id: u128, #[substorage(v0)] upgradeable: upgradeable_component::Storage, } @@ -40,13 +43,6 @@ mod ExternallyOwnedAccount { UpgradeableEvent: upgradeable_component::Event, } - #[constructor] - fn constructor( - ref self: ContractState, kakarot_address: ContractAddress, evm_address: EthAddress - ) { - self.kakarot_core_address.write(kakarot_address); - self.evm_address.write(evm_address); - } #[abi(embed_v0)] impl ExternallyOwnedAccount of super::IExternallyOwnedAccount { @@ -57,12 +53,23 @@ mod ExternallyOwnedAccount { self.evm_address.read() } + fn chain_id(self: @ContractState) -> u128 { + self.chain_id.read() + } // TODO: make this function reachable from an external invoke call // TODO: add some security methods to make sure that only some specific upgrades can be made ( low priority ) fn upgrade(ref self: ContractState, new_class_hash: ClassHash) { assert(get_caller_address() == get_contract_address(), 'Caller not contract address'); self.upgradeable.upgrade_contract(new_class_hash); } + + fn set_chain_id(ref self: ContractState, chain_id: u128) { + assert( + get_caller_address() == self.kakarot_core_address.read(), + 'Caller not kakarot address' + ); + self.chain_id.write(chain_id); + } } #[abi(embed_v0)] diff --git a/crates/contracts/src/tests/test_eoa.cairo b/crates/contracts/src/tests/test_eoa.cairo index d0ff4a8a7..7b925e6fc 100644 --- a/crates/contracts/src/tests/test_eoa.cairo +++ b/crates/contracts/src/tests/test_eoa.cairo @@ -4,11 +4,19 @@ mod test_external_owned_account { IExternallyOwnedAccount, ExternallyOwnedAccount, IExternallyOwnedAccountDispatcher, IExternallyOwnedAccountDispatcherTrait }; + use contracts::kakarot_core::kakarot::StoredAccountType; + use contracts::kakarot_core::{IKakarotCore, KakarotCore, KakarotCore::KakarotCoreInternal}; use contracts::tests::test_upgradeable::{ IMockContractUpgradeableDispatcher, IMockContractUpgradeableDispatcherTrait, MockContractUpgradeableV1 }; - use evm::tests::test_utils::{kakarot_address, eoa_address}; + use contracts::tests::test_utils::setup_contracts_for_testing; + use contracts::uninitialized_account::{ + IUninitializedAccountDispatcher, IUninitializedAccountDispatcherTrait, UninitializedAccount, + IUninitializedAccount + }; + use evm::model::{Address, AddressTrait}; + use evm::tests::test_utils::{kakarot_address, evm_address, eoa_address, chain_id}; use starknet::class_hash::Felt252TryIntoClassHash; use starknet::testing::{set_caller_address, set_contract_address}; use starknet::{ @@ -16,38 +24,46 @@ mod test_external_owned_account { EthAddress }; - fn deploy_eoa() -> IExternallyOwnedAccountDispatcher { - let calldata: Span = array![kakarot_address().into(), eoa_address().into()].span(); - - let maybe_address = deploy_syscall( - ExternallyOwnedAccount::TEST_CLASS_HASH.try_into().unwrap(), 0, calldata, false - ); - match maybe_address { - Result::Ok(( - contract_address, _ - )) => { IExternallyOwnedAccountDispatcher { contract_address } }, - Result::Err(err) => panic(err) - } + fn deploy_eoa(eoa_address: EthAddress) -> IExternallyOwnedAccountDispatcher { + let kakarot_address = get_contract_address(); + let calldata: Span = array![kakarot_address.into(), eoa_address.into()].span(); + + let (starknet_address, _) = deploy_syscall( + UninitializedAccount::TEST_CLASS_HASH.try_into().unwrap(), + evm_address().into(), + calldata, + false + ) + .expect('failed to deploy EOA'); + + let account = IUninitializedAccountDispatcher { contract_address: starknet_address }; + + account.initialize(ExternallyOwnedAccount::TEST_CLASS_HASH.try_into().unwrap()); + let eoa = IExternallyOwnedAccountDispatcher { contract_address: starknet_address }; + eoa.set_chain_id(chain_id()); + eoa } #[test] #[available_gas(2000000000)] fn test_kakarot_address() { - let expected_address = kakarot_address(); + let (_, kakarot) = setup_contracts_for_testing(); + let kakarot_address = kakarot.contract_address; - let eoa_contract = deploy_eoa(); + let eoa_contract = deploy_eoa(eoa_address()); - assert(eoa_contract.kakarot_core_address() == expected_address, 'wrong kakarot_address'); + assert(eoa_contract.kakarot_core_address() == kakarot_address, 'wrong kakarot_address'); } #[test] #[available_gas(2000000000)] fn test_evm_address() { let owner = contract_address_const::<1>(); - let expected_address: EthAddress = eoa_address(); + let (_, kakarot) = setup_contracts_for_testing(); + let kakarot_address = kakarot.contract_address; - let eoa_contract = deploy_eoa(); + let eoa_contract = deploy_eoa(eoa_address()); assert(eoa_contract.evm_address() == expected_address, 'wrong evm_address'); } @@ -55,7 +71,10 @@ mod test_external_owned_account { #[test] #[available_gas(2000000000)] fn test_eoa_upgrade() { - let eoa_contract = deploy_eoa(); + let (_, kakarot) = setup_contracts_for_testing(); + let kakarot_address = kakarot.contract_address; + let eoa_contract = deploy_eoa(eoa_address()); + let new_class_hash: ClassHash = MockContractUpgradeableV1::TEST_CLASS_HASH .try_into() .unwrap(); @@ -75,7 +94,9 @@ mod test_external_owned_account { #[available_gas(2000000000)] #[should_panic(expected: ('Caller not contract address', 'ENTRYPOINT_FAILED'))] fn test_eoa_upgrade_from_noncontractaddress() { - let eoa_contract = deploy_eoa(); + let (_, kakarot) = setup_contracts_for_testing(); + let kakarot_address = kakarot.contract_address; + let eoa_contract = deploy_eoa(eoa_address()); let new_class_hash: ClassHash = MockContractUpgradeableV1::TEST_CLASS_HASH .try_into() .unwrap(); diff --git a/crates/evm/src/model/eoa.cairo b/crates/evm/src/model/eoa.cairo index d23593767..5ef0f9852 100644 --- a/crates/evm/src/model/eoa.cairo +++ b/crates/evm/src/model/eoa.cairo @@ -1,3 +1,4 @@ +use contracts::eoa::{IExternallyOwnedAccountDispatcher, IExternallyOwnedAccountDispatcherTrait}; use contracts::kakarot_core::kakarot::KakarotCore::{ContractStateEventEmitter, EOADeployed}; use contracts::kakarot_core::kakarot::StoredAccountType; use contracts::kakarot_core::{IKakarotCore, KakarotCore, KakarotCore::KakarotCoreInternal}; @@ -38,6 +39,8 @@ impl EOAImpl of EOATrait { contract_address: starknet_address }; account.initialize(kakarot_state.eoa_class_hash()); + let eoa = IExternallyOwnedAccountDispatcher { contract_address: starknet_address }; + eoa.set_chain_id(kakarot_state.chain_id()); kakarot_state .set_address_registry(evm_address, StoredAccountType::EOA(starknet_address)); kakarot_state.emit(EOADeployed { evm_address, starknet_address }); diff --git a/crates/evm/src/tests/test_model/test_eoa.cairo b/crates/evm/src/tests/test_model/test_eoa.cairo index b432c8a95..14b0ba778 100644 --- a/crates/evm/src/tests/test_model/test_eoa.cairo +++ b/crates/evm/src/tests/test_model/test_eoa.cairo @@ -1,3 +1,4 @@ +use contracts::eoa::{IExternallyOwnedAccountDispatcher, IExternallyOwnedAccountDispatcherTrait}; use contracts::kakarot_core::KakarotCore; use contracts::kakarot_core::interface::IExtendedKakarotCoreDispatcherTrait; use contracts::tests::test_utils as contract_utils; @@ -23,4 +24,8 @@ fn test_eoa_deploy() { assert(event.evm_address == test_utils::evm_address(), 'wrong evm address'); assert(event.starknet_address.into() == eoa_address.starknet, 'wrong starknet address'); + + let kakarot_chain_id = kakarot_core.chain_id(); + let eoa = IExternallyOwnedAccountDispatcher { contract_address: eoa_address.starknet }; + assert(eoa.chain_id() == kakarot_chain_id, 'wrong chain id'); } From 5c1739a3c753fd2e379ee47c139e9fae2785fe51 Mon Sep 17 00:00:00 2001 From: Mathieu <60658558+enitrat@users.noreply.github.com> Date: Mon, 20 Nov 2023 13:40:46 +0300 Subject: [PATCH 3/9] feat: implement some remaining todos (#549) * feat: address TODOs * detail blockhash error --- crates/contracts/src/contract_account.cairo | 24 +++--- crates/contracts/src/eoa.cairo | 10 ++- crates/contracts/src/tests/test_eoa.cairo | 2 +- .../src/tests/test_kakarot_core.cairo | 81 ++++++++++++++++++- crates/evm/src/execution.cairo | 36 +++------ .../src/tests/test_execution_context.cairo | 5 -- .../test_block_information.cairo | 11 ++- .../test_memory_operations.cairo | 18 +++-- 8 files changed, 132 insertions(+), 55 deletions(-) diff --git a/crates/contracts/src/contract_account.cairo b/crates/contracts/src/contract_account.cairo index 346cccc4a..f5a36343b 100644 --- a/crates/contracts/src/contract_account.cairo +++ b/crates/contracts/src/contract_account.cairo @@ -34,7 +34,6 @@ trait IContractAccount { fn nonce(self: @TContractState) -> u64; fn set_nonce(ref self: TContractState, new_nonce: u64); - fn increment_nonce(ref self: TContractState); // *** // JUMP // Records of valid jumps in the context of jump opcodes @@ -144,6 +143,7 @@ mod ContractAccount { } fn set_bytecode(ref self: ContractState, bytecode: Span) { + self.assert_only_kakarot_core(); let packed_bytecode: ByteArray = ByteArrayExTrait::from_bytes(bytecode); // data_address is h(h(sn_keccak("contract_account_bytecode")), evm_address) let data_address = storage_base_address_from_felt252( @@ -192,6 +192,7 @@ mod ContractAccount { } fn set_storage_at(ref self: ContractState, key: u256, value: u256) { + self.assert_only_kakarot_core(); let storage_address = compute_storage_base_address( selector!("contract_account_storage_keys"), array![key.low.into(), key.high.into()].span() @@ -208,6 +209,7 @@ mod ContractAccount { fn set_nonce(ref self: ContractState, new_nonce: u64) { + self.assert_only_kakarot_core(); let storage_address: StorageBaseAddress = storage_base_address_from_felt252( selector!("contract_account_nonce") ); @@ -215,25 +217,18 @@ mod ContractAccount { } - fn increment_nonce(ref self: ContractState) { - let storage_address: StorageBaseAddress = storage_base_address_from_felt252( - selector!("contract_account_nonce") - ); - let nonce = Store::::read(0, storage_address).expect(NONCE_READ_ERROR); - Store::::write(0, storage_address, nonce + 1).expect(NONCE_WRITE_ERROR) - } - fn is_false_positive_jumpdest(self: @ContractState, offset: usize) -> bool { panic_with_felt252('unimplemented') } fn set_false_positive_jumpdest(ref self: ContractState, offset: usize) { + self.assert_only_kakarot_core(); panic_with_felt252('unimplemented') } fn selfdestruct(ref self: ContractState) { - //TODO add access control + self.assert_only_kakarot_core(); self.set_nonce(0); self.evm_address.write(0.try_into().unwrap()); self.set_bytecode(array![].span()); @@ -243,11 +238,18 @@ mod ContractAccount { fn upgrade(ref self: ContractState, new_class_hash: ClassHash) { + self.assert_only_kakarot_core(); + self.upgradeable.upgrade_contract(new_class_hash); + } + } + + #[generate_trait] + impl PrivateImpl of PrivateTrait { + fn assert_only_kakarot_core(self: @ContractState) { assert( get_caller_address() == self.kakarot_core_address.read(), 'Caller not Kakarot Core address' ); - self.upgradeable.upgrade_contract(new_class_hash); } } } diff --git a/crates/contracts/src/eoa.cairo b/crates/contracts/src/eoa.cairo index 178213fa7..97a99073e 100644 --- a/crates/contracts/src/eoa.cairo +++ b/crates/contracts/src/eoa.cairo @@ -56,10 +56,9 @@ mod ExternallyOwnedAccount { fn chain_id(self: @ContractState) -> u128 { self.chain_id.read() } - // TODO: make this function reachable from an external invoke call // TODO: add some security methods to make sure that only some specific upgrades can be made ( low priority ) fn upgrade(ref self: ContractState, new_class_hash: ClassHash) { - assert(get_caller_address() == get_contract_address(), 'Caller not contract address'); + self.assert_only_self(); self.upgradeable.upgrade_contract(new_class_hash); } @@ -111,4 +110,11 @@ mod ExternallyOwnedAccount { array![] } } + + #[generate_trait] + impl PrivateImpl of PrivateTrait { + fn assert_only_self(self: @ContractState) { + assert(get_caller_address() == get_contract_address(), 'Caller not self'); + } + } } diff --git a/crates/contracts/src/tests/test_eoa.cairo b/crates/contracts/src/tests/test_eoa.cairo index 7b925e6fc..add866219 100644 --- a/crates/contracts/src/tests/test_eoa.cairo +++ b/crates/contracts/src/tests/test_eoa.cairo @@ -92,7 +92,7 @@ mod test_external_owned_account { #[test] #[available_gas(2000000000)] - #[should_panic(expected: ('Caller not contract address', 'ENTRYPOINT_FAILED'))] + #[should_panic(expected: ('Caller not self', 'ENTRYPOINT_FAILED'))] fn test_eoa_upgrade_from_noncontractaddress() { let (_, kakarot) = setup_contracts_for_testing(); let kakarot_address = kakarot.contract_address; diff --git a/crates/contracts/src/tests/test_kakarot_core.cairo b/crates/contracts/src/tests/test_kakarot_core.cairo index 016505813..ad065e941 100644 --- a/crates/contracts/src/tests/test_kakarot_core.cairo +++ b/crates/contracts/src/tests/test_kakarot_core.cairo @@ -1,5 +1,7 @@ use contracts::contract_account::ContractAccount::TEST_CLASS_HASH as ContractAccountTestClassHash; +use contracts::contract_account::{IContractAccountDispatcher, IContractAccountDispatcherTrait}; use contracts::eoa::ExternallyOwnedAccount; +use contracts::kakarot_core::interface::IExtendedKakarotCoreDispatcherTrait; use contracts::kakarot_core::kakarot::StoredAccountType; use contracts::kakarot_core::{ interface::IExtendedKakarotCoreDispatcherImpl, KakarotCore, KakarotCore::{KakarotCoreInternal}, @@ -171,11 +173,84 @@ fn test_kakarot_core_upgrade_contract() { assert(version == 1, 'version is not 1'); } -// TODO add tests related to contract accounts once they can be deployed. -#[ignore] #[test] #[available_gas(20000000)] -fn test_kakarot_contract_account() {} +fn test_kakarot_contract_account_nonce() { + // Given + let (native_token, kakarot_core) = contract_utils::setup_contracts_for_testing(); + let address = ContractAccountTrait::deploy( + test_utils::other_evm_address(), Default::default().span() + ) + .unwrap(); + + // When + let nonce = kakarot_core.contract_account_nonce(address.evm); + + // Then + assert(nonce == 1, 'wrong nonce'); +} + + +#[test] +#[available_gas(20000000)] +fn test_kakarot_contract_account_storage_at() { + // Given + let (native_token, kakarot_core) = contract_utils::setup_contracts_for_testing(); + let address = ContractAccountTrait::deploy( + test_utils::other_evm_address(), Default::default().span() + ) + .unwrap(); + let ca = IContractAccountDispatcher { contract_address: address.starknet }; + let expected_value = 420; + let key = 69; + ca.set_storage_at(69, expected_value); + + // When + let value = kakarot_core.contract_account_storage_at(address.evm, key); + + // Then + assert(value == expected_value, 'wrong storage value'); +} + +#[test] +#[available_gas(2000000000)] +fn test_kakarot_contract_account_bytecode() { + // Given + let (native_token, kakarot_core) = contract_utils::setup_contracts_for_testing(); + let address = ContractAccountTrait::deploy( + test_utils::other_evm_address(), counter_evm_bytecode() + ) + .unwrap(); + + // When + let bytecode = kakarot_core.contract_account_bytecode(address.evm); + + // Then + assert(bytecode == counter_evm_bytecode(), 'wrong bytecode'); +} + + +#[test] +#[available_gas(20000000)] +#[should_panic(expected: ('unimplemented', 'ENTRYPOINT_FAILED'))] +fn test_kakarot_contract_account_false_positive_jumpdest() { + // Given + let (native_token, kakarot_core) = contract_utils::setup_contracts_for_testing(); + let address = ContractAccountTrait::deploy( + test_utils::other_evm_address(), Default::default().span() + ) + .unwrap(); + let ca = IContractAccountDispatcher { contract_address: address.starknet }; + let offset = 1337; + ca.set_false_positive_jumpdest(offset); + + // When + let is_false_jumpdest = kakarot_core + .contract_account_false_positive_jumpdest(address.evm, offset); + + // Then + assert(is_false_jumpdest, 'should be false jumpdest'); +} #[test] #[available_gas(2000000000000)] diff --git a/crates/evm/src/execution.cairo b/crates/evm/src/execution.cairo index 2f5bdabb1..d74cc8d51 100644 --- a/crates/evm/src/execution.cairo +++ b/crates/evm/src/execution.cairo @@ -65,35 +65,14 @@ fn execute( let transfer = Transfer { sender: origin, recipient: target, amount: value }; match machine.state.add_transfer(transfer) { Result::Ok(x) => {}, - Result::Err(err) => { - return ExecutionResult { - status: Status::Reverted, - return_data: Default::default().span(), - destroyed_contracts: Default::default().span(), - create_addresses: Default::default().span(), - events: Default::default().span(), - state: machine.state, - error: Option::Some(err) - }; - } + Result::Err(err) => { return reverted_with_err(machine, err); } } // cache the EOA of origin inside the state, and set its nonce to get_tx_info().unbox().nonce. // This allows us to call create with the correct nonce as the salt. let mut origin_account = match machine.state.get_account(origin.evm) { Result::Ok(account) => { account }, - //TODO create helper method to avoid this boilerplace - Result::Err(err) => { - return ExecutionResult { - status: Status::Reverted, - return_data: Default::default().span(), - destroyed_contracts: Default::default().span(), - create_addresses: Default::default().span(), - events: Default::default().span(), - state: machine.state, - error: Option::Some(err) - }; - } + Result::Err(err) => { return reverted_with_err(machine, err); } }; origin_account.nonce = starknet::get_tx_info().unbox().nonce.try_into().unwrap(); machine.state.set_account(origin_account); @@ -118,3 +97,14 @@ fn execute( } } +fn reverted_with_err(self: Machine, error: EVMError) -> ExecutionResult { + ExecutionResult { + status: Status::Reverted, + return_data: Default::default().span(), + destroyed_contracts: Default::default().span(), + create_addresses: Default::default().span(), + events: Default::default().span(), + state: self.state, + error: Option::Some(error) + } +} diff --git a/crates/evm/src/tests/test_execution_context.cairo b/crates/evm/src/tests/test_execution_context.cairo index e4ab6df86..190067a52 100644 --- a/crates/evm/src/tests/test_execution_context.cairo +++ b/crates/evm/src/tests/test_execution_context.cairo @@ -18,11 +18,6 @@ use starknet::{EthAddress, ContractAddress}; use test_utils::{callvalue, test_address}; use traits::PartialEq; -// TODO remove once no longer required (see https://github.com/starkware-libs/cairo/issues/3863) -#[inline(never)] -fn no_op() {} - - #[test] #[available_gas(1000000)] fn test_call_context_new() { diff --git a/crates/evm/src/tests/test_instructions/test_block_information.cairo b/crates/evm/src/tests/test_instructions/test_block_information.cairo index 55300f4f5..983192e09 100644 --- a/crates/evm/src/tests/test_instructions/test_block_information.cairo +++ b/crates/evm/src/tests/test_instructions/test_block_information.cairo @@ -45,7 +45,6 @@ fn test_exec_blockhash_above_bounds() { // TODO: implement exec_blockhash testing for block number within bounds // https://github.com/starkware-libs/cairo/blob/77a7e7bc36aa1c317bb8dd5f6f7a7e6eef0ab4f3/crates/cairo-lang-starknet/cairo_level_tests/interoperability.cairo#L173 -#[ignore] #[test] #[available_gas(20000000)] fn test_exec_blockhash_within_bounds() { @@ -56,9 +55,13 @@ fn test_exec_blockhash_within_bounds() { // When machine.stack.push(244).unwrap(); - machine.exec_blockhash(); - // Then - assert(machine.stack.peek().unwrap() == 0xF, 'stack top should be 0xF'); + + //TODO the CASM runner used in tests doesn't implement + //`get_block_hash_syscall` yet. As such, this test should fail no if the + //queried block is within bounds + assert(machine.exec_blockhash().is_err(), 'CASM Runner cant blockhash'); +// Then +// assert(machine.stack.peek().unwrap() == 0xF, 'stack top should be 0xF'); } diff --git a/crates/evm/src/tests/test_instructions/test_memory_operations.cairo b/crates/evm/src/tests/test_instructions/test_memory_operations.cairo index 197cac42a..28d0fbd30 100644 --- a/crates/evm/src/tests/test_instructions/test_memory_operations.cairo +++ b/crates/evm/src/tests/test_instructions/test_memory_operations.cairo @@ -355,7 +355,7 @@ fn test_exec_jump_out_of_bounds() { // Remove ignore once its handled #[test] #[available_gas(20000000)] -#[ignore] +#[should_panic(expected: ('exec_jump should throw error',))] fn test_exec_jump_inside_pushn() { // Given let bytecode: Span = array![0x60, 0x5B, 0x60, 0x00].span(); @@ -367,8 +367,11 @@ fn test_exec_jump_inside_pushn() { let result = machine.exec_jump(); // Then - assert(result.is_err(), 'invalid jump dest'); - assert(result.unwrap_err() == EVMError::JumpError(INVALID_DESTINATION), 'invalid jump dest'); + assert(result.is_err(), 'exec_jump should throw error'); + assert( + result.unwrap_err() == EVMError::JumpError(INVALID_DESTINATION), + 'jump dest should be invalid' + ); } #[test] @@ -481,7 +484,7 @@ fn test_exec_jumpi_invalid_zero() { // Remove ignore once its handled #[test] #[available_gas(20000000)] -#[ignore] +#[should_panic(expected: ('exec_jump should throw error',))] fn test_exec_jumpi_inside_pushn() { // Given let bytecode: Span = array![0x60, 0x5B, 0x60, 0x00].span(); @@ -495,8 +498,11 @@ fn test_exec_jumpi_inside_pushn() { let result = machine.exec_jumpi(); // Then - assert(result.is_err(), 'invalid jump dest'); - assert(result.unwrap_err() == EVMError::JumpError(INVALID_DESTINATION), 'invalid jump dest'); + assert(result.is_err(), 'exec_jump should throw error'); + assert( + result.unwrap_err() == EVMError::JumpError(INVALID_DESTINATION), + 'jump dest should be invalid' + ); } #[test] From c53a713e99f1dafc45a4cbb4e81c1333e71276d1 Mon Sep 17 00:00:00 2001 From: Mathieu <60658558+enitrat@users.noreply.github.com> Date: Mon, 20 Nov 2023 15:03:19 +0300 Subject: [PATCH 4/9] feat: selfdestruct (#554) --- crates/contracts/src/contract_account.cairo | 24 ++-- crates/contracts/src/eoa.cairo | 10 +- crates/contracts/src/tests/test_eoa.cairo | 2 +- .../src/tests/test_kakarot_core.cairo | 85 +------------- crates/evm/src/create_helpers.cairo | 2 +- crates/evm/src/execution.cairo | 36 +++--- .../environmental_information.cairo | 1 + .../src/instructions/system_operations.cairo | 36 +++++- crates/evm/src/model.cairo | 4 +- crates/evm/src/model/account.cairo | 41 ++++--- crates/evm/src/model/contract_account.cairo | 6 +- crates/evm/src/model/eoa.cairo | 2 +- .../src/tests/test_execution_context.cairo | 5 + .../test_block_information.cairo | 13 +-- .../test_environment_information.cairo | 16 +-- .../test_memory_operations.cairo | 23 ++-- .../test_system_operations.cairo | 109 +++++++++++++++++- crates/evm/src/tests/test_model.cairo | 39 +++---- .../test_model/test_contract_account.cairo | 4 +- crates/evm/src/tests/test_state.cairo | 5 +- crates/evm/src/tests/test_utils.cairo | 28 ++++- 21 files changed, 285 insertions(+), 206 deletions(-) diff --git a/crates/contracts/src/contract_account.cairo b/crates/contracts/src/contract_account.cairo index f5a36343b..346cccc4a 100644 --- a/crates/contracts/src/contract_account.cairo +++ b/crates/contracts/src/contract_account.cairo @@ -34,6 +34,7 @@ trait IContractAccount { fn nonce(self: @TContractState) -> u64; fn set_nonce(ref self: TContractState, new_nonce: u64); + fn increment_nonce(ref self: TContractState); // *** // JUMP // Records of valid jumps in the context of jump opcodes @@ -143,7 +144,6 @@ mod ContractAccount { } fn set_bytecode(ref self: ContractState, bytecode: Span) { - self.assert_only_kakarot_core(); let packed_bytecode: ByteArray = ByteArrayExTrait::from_bytes(bytecode); // data_address is h(h(sn_keccak("contract_account_bytecode")), evm_address) let data_address = storage_base_address_from_felt252( @@ -192,7 +192,6 @@ mod ContractAccount { } fn set_storage_at(ref self: ContractState, key: u256, value: u256) { - self.assert_only_kakarot_core(); let storage_address = compute_storage_base_address( selector!("contract_account_storage_keys"), array![key.low.into(), key.high.into()].span() @@ -209,7 +208,6 @@ mod ContractAccount { fn set_nonce(ref self: ContractState, new_nonce: u64) { - self.assert_only_kakarot_core(); let storage_address: StorageBaseAddress = storage_base_address_from_felt252( selector!("contract_account_nonce") ); @@ -217,18 +215,25 @@ mod ContractAccount { } + fn increment_nonce(ref self: ContractState) { + let storage_address: StorageBaseAddress = storage_base_address_from_felt252( + selector!("contract_account_nonce") + ); + let nonce = Store::::read(0, storage_address).expect(NONCE_READ_ERROR); + Store::::write(0, storage_address, nonce + 1).expect(NONCE_WRITE_ERROR) + } + fn is_false_positive_jumpdest(self: @ContractState, offset: usize) -> bool { panic_with_felt252('unimplemented') } fn set_false_positive_jumpdest(ref self: ContractState, offset: usize) { - self.assert_only_kakarot_core(); panic_with_felt252('unimplemented') } fn selfdestruct(ref self: ContractState) { - self.assert_only_kakarot_core(); + //TODO add access control self.set_nonce(0); self.evm_address.write(0.try_into().unwrap()); self.set_bytecode(array![].span()); @@ -238,18 +243,11 @@ mod ContractAccount { fn upgrade(ref self: ContractState, new_class_hash: ClassHash) { - self.assert_only_kakarot_core(); - self.upgradeable.upgrade_contract(new_class_hash); - } - } - - #[generate_trait] - impl PrivateImpl of PrivateTrait { - fn assert_only_kakarot_core(self: @ContractState) { assert( get_caller_address() == self.kakarot_core_address.read(), 'Caller not Kakarot Core address' ); + self.upgradeable.upgrade_contract(new_class_hash); } } } diff --git a/crates/contracts/src/eoa.cairo b/crates/contracts/src/eoa.cairo index 97a99073e..178213fa7 100644 --- a/crates/contracts/src/eoa.cairo +++ b/crates/contracts/src/eoa.cairo @@ -56,9 +56,10 @@ mod ExternallyOwnedAccount { fn chain_id(self: @ContractState) -> u128 { self.chain_id.read() } + // TODO: make this function reachable from an external invoke call // TODO: add some security methods to make sure that only some specific upgrades can be made ( low priority ) fn upgrade(ref self: ContractState, new_class_hash: ClassHash) { - self.assert_only_self(); + assert(get_caller_address() == get_contract_address(), 'Caller not contract address'); self.upgradeable.upgrade_contract(new_class_hash); } @@ -110,11 +111,4 @@ mod ExternallyOwnedAccount { array![] } } - - #[generate_trait] - impl PrivateImpl of PrivateTrait { - fn assert_only_self(self: @ContractState) { - assert(get_caller_address() == get_contract_address(), 'Caller not self'); - } - } } diff --git a/crates/contracts/src/tests/test_eoa.cairo b/crates/contracts/src/tests/test_eoa.cairo index add866219..7b925e6fc 100644 --- a/crates/contracts/src/tests/test_eoa.cairo +++ b/crates/contracts/src/tests/test_eoa.cairo @@ -92,7 +92,7 @@ mod test_external_owned_account { #[test] #[available_gas(2000000000)] - #[should_panic(expected: ('Caller not self', 'ENTRYPOINT_FAILED'))] + #[should_panic(expected: ('Caller not contract address', 'ENTRYPOINT_FAILED'))] fn test_eoa_upgrade_from_noncontractaddress() { let (_, kakarot) = setup_contracts_for_testing(); let kakarot_address = kakarot.contract_address; diff --git a/crates/contracts/src/tests/test_kakarot_core.cairo b/crates/contracts/src/tests/test_kakarot_core.cairo index ad065e941..f6165de59 100644 --- a/crates/contracts/src/tests/test_kakarot_core.cairo +++ b/crates/contracts/src/tests/test_kakarot_core.cairo @@ -1,7 +1,5 @@ use contracts::contract_account::ContractAccount::TEST_CLASS_HASH as ContractAccountTestClassHash; -use contracts::contract_account::{IContractAccountDispatcher, IContractAccountDispatcherTrait}; use contracts::eoa::ExternallyOwnedAccount; -use contracts::kakarot_core::interface::IExtendedKakarotCoreDispatcherTrait; use contracts::kakarot_core::kakarot::StoredAccountType; use contracts::kakarot_core::{ interface::IExtendedKakarotCoreDispatcherImpl, KakarotCore, KakarotCore::{KakarotCoreInternal}, @@ -173,84 +171,11 @@ fn test_kakarot_core_upgrade_contract() { assert(version == 1, 'version is not 1'); } +// TODO add tests related to contract accounts once they can be deployed. +#[ignore] #[test] #[available_gas(20000000)] -fn test_kakarot_contract_account_nonce() { - // Given - let (native_token, kakarot_core) = contract_utils::setup_contracts_for_testing(); - let address = ContractAccountTrait::deploy( - test_utils::other_evm_address(), Default::default().span() - ) - .unwrap(); - - // When - let nonce = kakarot_core.contract_account_nonce(address.evm); - - // Then - assert(nonce == 1, 'wrong nonce'); -} - - -#[test] -#[available_gas(20000000)] -fn test_kakarot_contract_account_storage_at() { - // Given - let (native_token, kakarot_core) = contract_utils::setup_contracts_for_testing(); - let address = ContractAccountTrait::deploy( - test_utils::other_evm_address(), Default::default().span() - ) - .unwrap(); - let ca = IContractAccountDispatcher { contract_address: address.starknet }; - let expected_value = 420; - let key = 69; - ca.set_storage_at(69, expected_value); - - // When - let value = kakarot_core.contract_account_storage_at(address.evm, key); - - // Then - assert(value == expected_value, 'wrong storage value'); -} - -#[test] -#[available_gas(2000000000)] -fn test_kakarot_contract_account_bytecode() { - // Given - let (native_token, kakarot_core) = contract_utils::setup_contracts_for_testing(); - let address = ContractAccountTrait::deploy( - test_utils::other_evm_address(), counter_evm_bytecode() - ) - .unwrap(); - - // When - let bytecode = kakarot_core.contract_account_bytecode(address.evm); - - // Then - assert(bytecode == counter_evm_bytecode(), 'wrong bytecode'); -} - - -#[test] -#[available_gas(20000000)] -#[should_panic(expected: ('unimplemented', 'ENTRYPOINT_FAILED'))] -fn test_kakarot_contract_account_false_positive_jumpdest() { - // Given - let (native_token, kakarot_core) = contract_utils::setup_contracts_for_testing(); - let address = ContractAccountTrait::deploy( - test_utils::other_evm_address(), Default::default().span() - ) - .unwrap(); - let ca = IContractAccountDispatcher { contract_address: address.starknet }; - let offset = 1337; - ca.set_false_positive_jumpdest(offset); - - // When - let is_false_jumpdest = kakarot_core - .contract_account_false_positive_jumpdest(address.evm, offset); - - // Then - assert(is_false_jumpdest, 'should be false jumpdest'); -} +fn test_kakarot_contract_account() {} #[test] #[available_gas(2000000000000)] @@ -262,7 +187,7 @@ fn test_eth_call() { let eoa = kakarot_core.deploy_eoa(evm_address); let account = ContractAccountTrait::deploy( - test_utils::other_evm_address(), counter_evm_bytecode() + test_utils::other_evm_address(), 1, counter_evm_bytecode() ) .unwrap(); @@ -292,7 +217,7 @@ fn test_handle_call() { let evm_address = test_utils::evm_address(); let eoa = kakarot_core.deploy_eoa(evm_address); let account = ContractAccountTrait::deploy( - test_utils::other_evm_address(), counter_evm_bytecode() + test_utils::other_evm_address(), 1, counter_evm_bytecode() ) .unwrap(); diff --git a/crates/evm/src/create_helpers.cairo b/crates/evm/src/create_helpers.cairo index 9db799152..271afc9c3 100644 --- a/crates/evm/src/create_helpers.cairo +++ b/crates/evm/src/create_helpers.cairo @@ -98,7 +98,7 @@ impl MachineCreateHelpersImpl of MachineCreateHelpers { // - contract is already deployed at this location (type fetched from storage) // - Contract has been scheduled for deployment (type set in cache) // If the AccountType is unknown, then there's no collision. - if target_account.is_deployed() { + if target_account.exists() { return self.stack.push(0); }; diff --git a/crates/evm/src/execution.cairo b/crates/evm/src/execution.cairo index d74cc8d51..2f5bdabb1 100644 --- a/crates/evm/src/execution.cairo +++ b/crates/evm/src/execution.cairo @@ -65,14 +65,35 @@ fn execute( let transfer = Transfer { sender: origin, recipient: target, amount: value }; match machine.state.add_transfer(transfer) { Result::Ok(x) => {}, - Result::Err(err) => { return reverted_with_err(machine, err); } + Result::Err(err) => { + return ExecutionResult { + status: Status::Reverted, + return_data: Default::default().span(), + destroyed_contracts: Default::default().span(), + create_addresses: Default::default().span(), + events: Default::default().span(), + state: machine.state, + error: Option::Some(err) + }; + } } // cache the EOA of origin inside the state, and set its nonce to get_tx_info().unbox().nonce. // This allows us to call create with the correct nonce as the salt. let mut origin_account = match machine.state.get_account(origin.evm) { Result::Ok(account) => { account }, - Result::Err(err) => { return reverted_with_err(machine, err); } + //TODO create helper method to avoid this boilerplace + Result::Err(err) => { + return ExecutionResult { + status: Status::Reverted, + return_data: Default::default().span(), + destroyed_contracts: Default::default().span(), + create_addresses: Default::default().span(), + events: Default::default().span(), + state: machine.state, + error: Option::Some(err) + }; + } }; origin_account.nonce = starknet::get_tx_info().unbox().nonce.try_into().unwrap(); machine.state.set_account(origin_account); @@ -97,14 +118,3 @@ fn execute( } } -fn reverted_with_err(self: Machine, error: EVMError) -> ExecutionResult { - ExecutionResult { - status: Status::Reverted, - return_data: Default::default().span(), - destroyed_contracts: Default::default().span(), - create_addresses: Default::default().span(), - events: Default::default().span(), - state: self.state, - error: Option::Some(error) - } -} diff --git a/crates/evm/src/instructions/environmental_information.cairo b/crates/evm/src/instructions/environmental_information.cairo index 0dd292a7c..4bd0b47c0 100644 --- a/crates/evm/src/instructions/environmental_information.cairo +++ b/crates/evm/src/instructions/environmental_information.cairo @@ -244,6 +244,7 @@ impl EnvironmentInformationImpl of EnvironmentInformationTrait { // -> Undeployed CAs that might be deployed later, but currently don't // exist and have only been touched for value transfers // -> Undeployed EOAs + // Selfdestructed CAs still exist until the end of the TX. if account.is_precompile() || (account.account_type == AccountType::Unknown) { return self.stack.push(0); } diff --git a/crates/evm/src/instructions/system_operations.cairo b/crates/evm/src/instructions/system_operations.cairo index 904efd79c..3fab70a75 100644 --- a/crates/evm/src/instructions/system_operations.cairo +++ b/crates/evm/src/instructions/system_operations.cairo @@ -1,12 +1,14 @@ //! System operations. use box::BoxTrait; +use contracts::kakarot_core::{KakarotCore, IKakarotCore}; use evm::call_helpers::{MachineCallHelpers, CallType}; use evm::create_helpers::{MachineCreateHelpers, CreateType}; use evm::errors::{EVMError, VALUE_TRANSFER_IN_STATIC_CALL, WRITE_IN_STATIC_CONTEXT}; use evm::machine::{Machine, MachineCurrentContextTrait}; use evm::memory::MemoryTrait; use evm::model::account::{AccountTrait}; +use evm::model::{Address, Transfer}; use evm::stack::StackTrait; use evm::state::StateTrait; use utils::math::Exponentiation; @@ -131,6 +133,38 @@ impl SystemOperations of SystemOperationsTrait { if self.read_only() { return Result::Err(EVMError::WriteInStaticContext(WRITE_IN_STATIC_CONTEXT)); } - Result::Err(EVMError::NotImplemented) + let kakarot_state = KakarotCore::unsafe_new_contract_state(); + let stack_address = self.stack.pop_eth_address()?; + + //TODO Remove this when https://eips.ethereum.org/EIPS/eip-6780 is validated + let recipient_evm_address = if (stack_address == self.address().evm) { + 0.try_into().unwrap() + } else { + stack_address + }; + let recipient_starknet_address = kakarot_state + .compute_starknet_address(recipient_evm_address); + let mut account = self.state.get_account(self.address().evm)?; + + let recipient = Address { + evm: recipient_evm_address, starknet: recipient_starknet_address + }; + + // Transfer balance + self + .state + .add_transfer( + Transfer { + sender: account.address(), + recipient, + amount: self.state.read_balance(account.address().evm)? + } + ); + + // Register for selfdestruct + account.selfdestruct(); + self.state.set_account(account); + self.set_stopped(); + Result::Ok(()) } } diff --git a/crates/evm/src/model.cairo b/crates/evm/src/model.cairo index f564d345e..989c9278a 100644 --- a/crates/evm/src/model.cairo +++ b/crates/evm/src/model.cairo @@ -31,9 +31,9 @@ struct Address { #[generate_trait] impl AddressImpl of AddressTrait { - fn is_registered(evm_address: EthAddress) -> bool { + fn is_deployed(self: EthAddress) -> bool { let mut kakarot_state = KakarotCore::unsafe_new_contract_state(); - let maybe_account = kakarot_state.address_registry(evm_address); + let maybe_account = kakarot_state.address_registry(self); match maybe_account { Option::Some(_) => true, Option::None => false diff --git a/crates/evm/src/model/account.cairo b/crates/evm/src/model/account.cairo index c8f3bf7e1..a8c19eb62 100644 --- a/crates/evm/src/model/account.cairo +++ b/crates/evm/src/model/account.cairo @@ -97,10 +97,8 @@ impl AccountImpl of AccountTrait { let starknet_address = kakarot_state.compute_starknet_address(evm_address); Result::Ok( // If no account exists at `address`, then we are trying to - // access an undeployed contract account. Simple value - // transfers between EOAs don't call this function - - // Therefore, we're sure that only contract accounts are - // undeployed. + // access an undeployed account (CA or EOA). We create an + // empty account with the correct address and return it. Account { account_type: AccountType::Unknown, address: Address { starknet: starknet_address, evm: evm_address, }, @@ -161,8 +159,8 @@ impl AccountImpl of AccountTrait { /// be registered already, and the nonce must not be 0 or the code must not /// be empty #[inline(always)] - fn should_deploy(self: @Account, is_registered: bool) -> bool { - if !is_registered && self.is_ca() && (*self.nonce != 0 || !(*self.code).is_empty()) { + fn should_deploy(self: @Account) -> bool { + if self.is_ca() && (*self.nonce != 0 || !(*self.code).is_empty()) { return true; }; false @@ -180,10 +178,10 @@ impl AccountImpl of AccountTrait { /// /// `Ok(())` if the commit was successful, otherwise an `EVMError`. fn commit(self: @Account) -> Result<(), EVMError> { - // Case account exists and is already on chain - let is_registered = AddressTrait::is_registered(self.address().evm); + // Case account is already deployed onchain + let is_deployed = self.address().evm.is_deployed(); - if is_registered { + if is_deployed { match self.account_type { AccountType::EOA(eoa) => { // no - op @@ -196,21 +194,22 @@ impl AccountImpl of AccountTrait { .set_address_registry( self.address().evm, StoredAccountType::UnexistingAccount ); - return self.selfdestruct(); + return ContractAccountTrait::selfdestruct(self); } self.store_nonce(*self.nonce) //Storage is handled outside of the account and must be commited after all accounts are commited. }, AccountType::Unknown => { Result::Ok(()) } } - } else if self.should_deploy(is_registered) { - //Case new account - // If SELFDESTRUCT, just do nothing - if (*self.selfdestruct == true) { - return Result::Ok(()); + } else if self.should_deploy() { + // If SELFDESTRUCT, deploy empty SN account + let (initial_nonce, initial_code) = if (*self.selfdestruct == true) { + (0, Default::default().span()) + } else { + (*self.nonce, *self.code) }; - let mut ca_address = ContractAccountTrait::deploy(self.address().evm, *self.code)?; - self.store_nonce(*self.nonce) + ContractAccountTrait::deploy(self.address().evm, initial_nonce, initial_code)?; + Result::Ok(()) //Storage is handled outside of the account and must be commited after all accounts are commited. } else { Result::Ok(()) @@ -231,19 +230,19 @@ impl AccountImpl of AccountTrait { false } - /// Returns whether an accound is deployed at the given address. + /// Returns whether an account exists at the given address. /// /// Based on the state of the account in the cache - the account can - /// not be commited on-chain, but already be deployed in the KakarotState. + /// not be deployed on-chain, but already exist in the KakarotState. /// # Arguments /// /// * `address` - The Ethereum address to look up. /// /// # Returns /// - /// `true` if an account is deployed at this address, `false` otherwise. + /// `true` if an account exists at this address, `false` otherwise. #[inline(always)] - fn is_deployed(self: @Account) -> bool { + fn exists(self: @Account) -> bool { let is_known = *self.account_type != AccountType::Unknown; //TODO(account) verify whether is_known is a sufficient condition diff --git a/crates/evm/src/model/contract_account.cairo b/crates/evm/src/model/contract_account.cairo index 4f8a0d2b1..5c09a7f96 100644 --- a/crates/evm/src/model/contract_account.cairo +++ b/crates/evm/src/model/contract_account.cairo @@ -53,7 +53,9 @@ impl ContractAccountImpl of ContractAccountTrait { /// * The evm_address and starknet_address the CA is deployed at - which is KakarotCore /// # Errors /// * `ACCOUNT_EXISTS` - If a contract account already exists at the given `evm_address` - fn deploy(evm_address: EthAddress, bytecode: Span) -> Result { + fn deploy( + evm_address: EthAddress, nonce: u64, bytecode: Span + ) -> Result { let mut kakarot_state = KakarotCore::unsafe_new_contract_state(); let account_class_hash = kakarot_state.account_class_hash(); let kakarot_address = get_contract_address(); @@ -71,7 +73,7 @@ impl ContractAccountImpl of ContractAccountTrait { // Initialize the account let account = IContractAccountDispatcher { contract_address: starknet_address }; - account.set_nonce(1); + account.set_nonce(nonce); account.set_bytecode(bytecode); // Kakarot Core logic diff --git a/crates/evm/src/model/eoa.cairo b/crates/evm/src/model/eoa.cairo index 5ef0f9852..3e7519915 100644 --- a/crates/evm/src/model/eoa.cairo +++ b/crates/evm/src/model/eoa.cairo @@ -18,7 +18,7 @@ impl EOAImpl of EOATrait { /// * `evm_address` - The EVM address of the EOA to deploy. fn deploy(evm_address: EthAddress) -> Result { // Unlike CAs, there is not check for the existence of an EOA prealably to calling `EOATrait::deploy` - therefore, we need to check that there is no collision. - let mut is_deployed = AddressTrait::is_registered(evm_address); + let mut is_deployed = evm_address.is_deployed(); if is_deployed { return Result::Err(EVMError::DeployError(EOA_EXISTS)); } diff --git a/crates/evm/src/tests/test_execution_context.cairo b/crates/evm/src/tests/test_execution_context.cairo index 190067a52..e4ab6df86 100644 --- a/crates/evm/src/tests/test_execution_context.cairo +++ b/crates/evm/src/tests/test_execution_context.cairo @@ -18,6 +18,11 @@ use starknet::{EthAddress, ContractAddress}; use test_utils::{callvalue, test_address}; use traits::PartialEq; +// TODO remove once no longer required (see https://github.com/starkware-libs/cairo/issues/3863) +#[inline(never)] +fn no_op() {} + + #[test] #[available_gas(1000000)] fn test_call_context_new() { diff --git a/crates/evm/src/tests/test_instructions/test_block_information.cairo b/crates/evm/src/tests/test_instructions/test_block_information.cairo index 983192e09..27aa3d73e 100644 --- a/crates/evm/src/tests/test_instructions/test_block_information.cairo +++ b/crates/evm/src/tests/test_instructions/test_block_information.cairo @@ -45,6 +45,7 @@ fn test_exec_blockhash_above_bounds() { // TODO: implement exec_blockhash testing for block number within bounds // https://github.com/starkware-libs/cairo/blob/77a7e7bc36aa1c317bb8dd5f6f7a7e6eef0ab4f3/crates/cairo-lang-starknet/cairo_level_tests/interoperability.cairo#L173 +#[ignore] #[test] #[available_gas(20000000)] fn test_exec_blockhash_within_bounds() { @@ -55,13 +56,9 @@ fn test_exec_blockhash_within_bounds() { // When machine.stack.push(244).unwrap(); - - //TODO the CASM runner used in tests doesn't implement - //`get_block_hash_syscall` yet. As such, this test should fail no if the - //queried block is within bounds - assert(machine.exec_blockhash().is_err(), 'CASM Runner cant blockhash'); -// Then -// assert(machine.stack.peek().unwrap() == 0xF, 'stack top should be 0xF'); + machine.exec_blockhash(); + // Then + assert(machine.stack.peek().unwrap() == 0xF, 'stack top should be 0xF'); } @@ -155,7 +152,7 @@ fn test_exec_selfbalance_zero() { fn test_exec_selfbalance_contract_account() { // Given let (native_token, kakarot_core) = setup_contracts_for_testing(); - let mut ca_address = ContractAccountTrait::deploy(evm_address(), array![].span()) + let mut ca_address = ContractAccountTrait::deploy(evm_address(), 1, array![].span()) .expect('failed deploy contract account',); fund_account_with_native_token(ca_address.starknet, native_token, 0x1); diff --git a/crates/evm/src/tests/test_instructions/test_environment_information.cairo b/crates/evm/src/tests/test_instructions/test_environment_information.cairo index a3787094b..2027f2e90 100644 --- a/crates/evm/src/tests/test_instructions/test_environment_information.cairo +++ b/crates/evm/src/tests/test_instructions/test_environment_information.cairo @@ -93,7 +93,7 @@ fn test_exec_balance_zero() { fn test_exec_balance_contract_account() { // Given let (native_token, kakarot_core) = setup_contracts_for_testing(); - let mut ca_address = ContractAccountTrait::deploy(evm_address(), array![].span()) + let mut ca_address = ContractAccountTrait::deploy(evm_address(), 1, array![].span()) .expect('failed deploy contract account',); fund_account_with_native_token(ca_address.starknet, native_token, 0x1); @@ -579,7 +579,7 @@ fn test_exec_extcodesize_ca_empty() { let (native_token, kakarot_core) = setup_contracts_for_testing(); // The bytecode remains empty, and we expect the empty hash in return - let mut ca_address = ContractAccountTrait::deploy(evm_address(), array![].span()); + let mut ca_address = ContractAccountTrait::deploy(evm_address(), 1, array![].span()); machine.stack.push(evm_address.into()); @@ -600,7 +600,7 @@ fn test_exec_extcodesize_ca_with_bytecode() { let (native_token, kakarot_core) = setup_contracts_for_testing(); // The bytecode stored is the bytecode of a Counter.sol smart contract - ContractAccountTrait::deploy(evm_address(), counter_evm_bytecode()); + ContractAccountTrait::deploy(evm_address(), 1, counter_evm_bytecode()); machine.stack.push(evm_address.into()); // When @@ -624,7 +624,7 @@ fn test_exec_extcodecopy_ca() { let (native_token, kakarot_core) = setup_contracts_for_testing(); // The bytecode stored is the bytecode of a Counter.sol smart contract - ContractAccountTrait::deploy(evm_address(), counter_evm_bytecode()); + ContractAccountTrait::deploy(evm_address(), 1, counter_evm_bytecode()); // size machine.stack.push(50).unwrap(); @@ -655,7 +655,7 @@ fn test_exec_extcodecopy_ca_offset_out_of_bounds() { let (native_token, kakarot_core) = setup_contracts_for_testing(); // The bytecode stored is the bytecode of a Counter.sol smart contract - ContractAccountTrait::deploy(evm_address(), counter_evm_bytecode()); + ContractAccountTrait::deploy(evm_address(), 1, counter_evm_bytecode()); // size machine.stack.push(5); @@ -925,7 +925,7 @@ fn test_exec_extcodehash_selfdestructed() { let (native_token, kakarot_core) = setup_contracts_for_testing(); // The bytecode remains empty, and we expect the empty hash in return - let mut ca_address = ContractAccountTrait::deploy(evm_address, array![].span()) + let mut ca_address = ContractAccountTrait::deploy(evm_address, 1, array![].span()) .expect('CA deployment failed'); let account = Account { account_type: AccountType::ContractAccount, @@ -983,7 +983,7 @@ fn test_exec_extcodehash_ca_empty() { let mut machine = setup_machine(); let (native_token, kakarot_core) = setup_contracts_for_testing(); // The bytecode remains empty, and we expect the empty hash in return - let mut ca_address = ContractAccountTrait::deploy(evm_address(), array![].span()); + let mut ca_address = ContractAccountTrait::deploy(evm_address(), 1, array![].span()); machine.stack.push(evm_address.into()); @@ -1027,7 +1027,7 @@ fn test_exec_extcodehash_ca_with_bytecode() { let (native_token, kakarot_core) = setup_contracts_for_testing(); // The bytecode stored is the bytecode of a Counter.sol smart contract - let mut ca_address = ContractAccountTrait::deploy(evm_address(), counter_evm_bytecode()); + let mut ca_address = ContractAccountTrait::deploy(evm_address(), 1, counter_evm_bytecode()); machine.stack.push(evm_address.into()); // When diff --git a/crates/evm/src/tests/test_instructions/test_memory_operations.cairo b/crates/evm/src/tests/test_instructions/test_memory_operations.cairo index 28d0fbd30..cd6075564 100644 --- a/crates/evm/src/tests/test_instructions/test_memory_operations.cairo +++ b/crates/evm/src/tests/test_instructions/test_memory_operations.cairo @@ -355,7 +355,7 @@ fn test_exec_jump_out_of_bounds() { // Remove ignore once its handled #[test] #[available_gas(20000000)] -#[should_panic(expected: ('exec_jump should throw error',))] +#[ignore] fn test_exec_jump_inside_pushn() { // Given let bytecode: Span = array![0x60, 0x5B, 0x60, 0x00].span(); @@ -367,11 +367,8 @@ fn test_exec_jump_inside_pushn() { let result = machine.exec_jump(); // Then - assert(result.is_err(), 'exec_jump should throw error'); - assert( - result.unwrap_err() == EVMError::JumpError(INVALID_DESTINATION), - 'jump dest should be invalid' - ); + assert(result.is_err(), 'invalid jump dest'); + assert(result.unwrap_err() == EVMError::JumpError(INVALID_DESTINATION), 'invalid jump dest'); } #[test] @@ -484,7 +481,7 @@ fn test_exec_jumpi_invalid_zero() { // Remove ignore once its handled #[test] #[available_gas(20000000)] -#[should_panic(expected: ('exec_jump should throw error',))] +#[ignore] fn test_exec_jumpi_inside_pushn() { // Given let bytecode: Span = array![0x60, 0x5B, 0x60, 0x00].span(); @@ -498,11 +495,8 @@ fn test_exec_jumpi_inside_pushn() { let result = machine.exec_jumpi(); // Then - assert(result.is_err(), 'exec_jump should throw error'); - assert( - result.unwrap_err() == EVMError::JumpError(INVALID_DESTINATION), - 'jump dest should be invalid' - ); + assert(result.is_err(), 'invalid jump dest'); + assert(result.unwrap_err() == EVMError::JumpError(INVALID_DESTINATION), 'invalid jump dest'); } #[test] @@ -532,7 +526,7 @@ fn test_exec_sload_from_storage() { // Given let (native_token, kakarot_core) = setup_contracts_for_testing(); let mut machine = setup_machine(); - let mut ca_address = ContractAccountTrait::deploy(machine.address().evm, array![].span()) + let mut ca_address = ContractAccountTrait::deploy(machine.address().evm, 1, array![].span()) .unwrap(); let account = Account { account_type: AccountType::ContractAccount, @@ -603,7 +597,8 @@ fn test_exec_sstore_finalized() { let (native_token, kakarot_core) = setup_contracts_for_testing(); let mut machine = setup_machine(); // Deploys the contract account to be able to commit storage changes. - let ca_address = ContractAccountTrait::deploy(machine.address().evm, array![].span()).unwrap(); + let ca_address = ContractAccountTrait::deploy(machine.address().evm, 1, array![].span()) + .unwrap(); let account = Account { account_type: AccountType::ContractAccount, address: ca_address, diff --git a/crates/evm/src/tests/test_instructions/test_system_operations.cairo b/crates/evm/src/tests/test_instructions/test_system_operations.cairo index ab3bb25d0..afea1f5c5 100644 --- a/crates/evm/src/tests/test_instructions/test_system_operations.cairo +++ b/crates/evm/src/tests/test_instructions/test_system_operations.cairo @@ -1,22 +1,29 @@ use contracts::kakarot_core::interface::IExtendedKakarotCoreDispatcherTrait; use contracts::tests::test_data::{storage_evm_bytecode, storage_evm_initcode}; -use contracts::tests::test_utils::setup_contracts_for_testing; +use contracts::tests::test_utils::{fund_account_with_native_token, setup_contracts_for_testing}; +use debug::PrintTrait; use evm::call_helpers::{MachineCallHelpers, MachineCallHelpersImpl}; use evm::context::{ExecutionContext, ExecutionContextTrait, ExecutionContextType}; +use evm::errors::EVMErrorTrait; use evm::instructions::MemoryOperationTrait; use evm::instructions::SystemOperationsTrait; use evm::interpreter::EVMInterpreterTrait; use evm::machine::{Machine, MachineCurrentContextTrait}; use evm::memory::MemoryTrait; +use evm::model::account::{Account}; use evm::model::contract_account::ContractAccountTrait; -use evm::model::{AccountTrait, Address}; +use evm::model::eoa::EOATrait; +use evm::model::{AccountTrait, Address, AccountType, Transfer}; use evm::stack::StackTrait; -use evm::state::StateTrait; +use evm::state::{State, StateTrait}; use evm::tests::test_utils::{ setup_machine_with_nested_execution_context, setup_machine, setup_machine_with_bytecode, - initialize_contract_account, native_token, evm_address + test_address, initialize_contract_account, native_token, evm_address, other_evm_address, + setup_machine_with_target }; +use openzeppelin::token::erc20::interface::{IERC20CamelDispatcher, IERC20CamelDispatcherTrait}; use starknet::EthAddress; +use starknet::testing::set_contract_address; use utils::helpers::load_word; use utils::traits::EthAddressIntoU256; @@ -397,7 +404,7 @@ fn test_exec_create2() { let deployed_bytecode = array![0xff].span(); let eth_address: EthAddress = 0x00000000000000000075766d5f61646472657373_u256.into(); - let contract_address = ContractAccountTrait::deploy(eth_address, deployed_bytecode) + let contract_address = ContractAccountTrait::deploy(eth_address, 1, deployed_bytecode) .expect('failed deploying CA'); let mut ctx = machine.current_ctx.unbox(); @@ -438,3 +445,95 @@ fn test_exec_create2() { assert(account.nonce() == 1, 'wrong nonce'); assert(account.code == storage_evm_bytecode(), 'wrong bytecode'); } + +#[test] +#[available_gas(200000000)] +fn test_exec_selfdestruct_existing_ca() { + // Given + let (native_token, kakarot_core) = setup_contracts_for_testing(); + let destroyed_address = test_address().evm; // address in machine call context + let ca_address = ContractAccountTrait::deploy( + destroyed_address, 1, array![0x1, 0x2, 0x3].span() + ) + .expect('failed deploying CA'); + fund_account_with_native_token(ca_address.starknet, native_token, 1000); + let recipient = EOATrait::deploy(other_evm_address()).expect('failed deploying eoa'); + let mut machine = setup_machine_with_target(ca_address); + + // When + machine.stack.push(recipient.evm.into()); + machine.exec_selfdestruct().expect('selfdestruct failed'); + machine.state.commit_context(); + machine.state.commit_state(); + machine.state = Default::default(); //empty state to force re-fetch from SN + // Then + let destructed = machine.state.get_account(ca_address.evm).expect('couldnt fetch destructed'); + + assert(destructed.nonce() == 0, 'destructed nonce should be 0'); + assert( + destructed.balance().expect('couldnt get balance') == 0, 'destructed balance should be 0' + ); + assert(destructed.bytecode().len() == 0, 'bytecode should be empty'); + + let recipient = machine.state.get_account(recipient.evm).expect('couldnt fetch recipient'); +//TODO this assertion fails because of deterministic address calculations. +// Once addressed in the compiler code, this test should be fixed. +// in selfdestruct, we execute: +// let recipient_starknet_address = kakarot_state +// .compute_starknet_address(recipient_evm_address); +// assert(recipient.balance().expect('couldnt get balance') == 1000, 'wrong recipient balance'); +} + +#[test] +#[available_gas(200000000)] +fn test_selfdestruct_undeployed_ca() { //TODO +// for now we can't fund an undeployed CA as the SN address is not deterministically calculated in the runner, so no way of funding a SN address by calculating it from an EVM address +// This test should +// - deploy kkt and token, deploy an EOA +// - call `get_account` on an undeployed account, set its type to CA, its nonce to 1, its code to something +// - selfdestruct it +// verify that the value transfer has succeeded +// + +} + +#[test] +#[available_gas(200000000)] +fn test_exec_selfdestruct_add_transfer_post_selfdestruct() { + // Given + let (native_token, kakarot_core) = setup_contracts_for_testing(); + + // Deploy sender and recipiens EOAs, and CA that will be selfdestructed and funded with 100 tokens + let sender = EOATrait::deploy('sender'.try_into().unwrap()).expect('failed deploy EOA',); + let recipient = EOATrait::deploy('recipient'.try_into().unwrap()).expect('failed deploy EOA',); + let ca_address = ContractAccountTrait::deploy( + 'contract'.try_into().unwrap(), 1, array![].span() + ) + .expect('failed deploy CA'); + fund_account_with_native_token(sender.starknet, native_token, 150); + fund_account_with_native_token(ca_address.starknet, native_token, 100); + let mut machine = setup_machine_with_target(ca_address); + + // Cache the CA into state + let mut ca = machine.state.get_account('contract'.try_into().unwrap()).expect('couldnt get CA'); + + // When + machine.stack.push(recipient.evm.into()); + machine.exec_selfdestruct().expect('selfdestruct failed'); + // Add a transfer from sender to CA - after it was selfdestructed in local state. This transfer should go through. + let transfer = Transfer { sender, recipient: ca_address, amount: 150 }; + machine.state.add_transfer(transfer).unwrap(); + machine.state.commit_context(); + machine.state.commit_state(); + machine.state = Default::default(); //empty state to force re-fetch from SN + + // Then + let recipient_balance = native_token.balanceOf(recipient.starknet); + let sender_balance = native_token.balanceOf(sender.starknet); + let ca_balance = native_token.balanceOf(ca_address.starknet); + //TODO this assert fails because of deterministic address calculations. + //FIXME when addressed in the compiler code, this test should be fixed. + // assert(recipient_balance == 100, 'recipient wrong balance'); + assert(sender_balance == 0, 'sender wrong balance'); + assert(ca_balance == 150, 'ca wrong balance'); +} diff --git a/crates/evm/src/tests/test_model.cairo b/crates/evm/src/tests/test_model.cairo index c08c8cd5c..d0d67f850 100644 --- a/crates/evm/src/tests/test_model.cairo +++ b/crates/evm/src/tests/test_model.cairo @@ -10,7 +10,7 @@ use starknet::testing::set_contract_address; #[test] #[available_gas(20000000)] -fn test_is_registered_eoa_exists() { +fn test_is_deployed_eoa_exists() { // Given let (native_token, kakarot_core) = setup_contracts_for_testing(); let eoa_address = EOATrait::deploy(evm_address()).expect('failed deploy contract account',); @@ -19,39 +19,38 @@ fn test_is_registered_eoa_exists() { // When set_contract_address(kakarot_core.contract_address); - let is_registered = AddressTrait::is_registered(evm_address()); + let is_deployed = evm_address().is_deployed(); // Then - assert(is_registered, 'account should be deployed'); + assert(is_deployed, 'account should be deployed'); } #[test] #[available_gas(20000000)] -fn test_is_registered_ca_exists() { +fn test_is_deployed_ca_exists() { // Given let (native_token, kakarot_core) = setup_contracts_for_testing(); - ContractAccountTrait::deploy(evm_address(), array![].span()) + ContractAccountTrait::deploy(evm_address(), 1, array![].span()) .expect('failed deploy contract account',); // When - let is_registered = AddressTrait::is_registered(evm_address()); - + let is_deployed = evm_address().is_deployed(); // Then - assert(is_registered, 'account should be deployed'); + assert(is_deployed, 'account should be deployed'); } #[test] #[available_gas(20000000)] -fn test_is_registered_undeployed() { +fn test_is_deployed_undeployed() { // Given let (native_token, kakarot_core) = setup_contracts_for_testing(); // When set_contract_address(kakarot_core.contract_address); - let is_registered = AddressTrait::is_registered(evm_address()); + let is_deployed = evm_address().is_deployed(); // Then - assert(!is_registered, 'account should be undeployed'); + assert(!is_deployed, 'account shouldnt be deployed'); } @@ -94,7 +93,7 @@ fn test_address_balance_eoa() { #[test] #[available_gas(5000000)] -fn test_account_is_deployed_eoa() { +fn test_account_exists_eoa() { // Given let (native_token, kakarot_core) = setup_contracts_for_testing(); let mut eoa_address = EOATrait::deploy(evm_address()).expect('failed deploy eoa',); @@ -103,29 +102,29 @@ fn test_account_is_deployed_eoa() { let account = AccountTrait::fetch(evm_address()).unwrap().unwrap(); // Then - assert(account.is_deployed() == true, 'account should be deployed'); + assert(account.exists() == true, 'account should be deployed'); } #[test] #[available_gas(5000000)] -fn test_account_is_deployed_contract_account() { +fn test_account_exists_contract_account() { // Given let (native_token, kakarot_core) = setup_contracts_for_testing(); - let mut ca_address = ContractAccountTrait::deploy(evm_address(), array![].span()) + let mut ca_address = ContractAccountTrait::deploy(evm_address(), 1, array![].span()) .expect('failed deploy contract account',); // When let account = AccountTrait::fetch(evm_address()).unwrap().unwrap(); // Then - assert(account.is_deployed() == true, 'account should be deployed'); + assert(account.exists() == true, 'account should be deployed'); } #[test] #[available_gas(5000000)] -fn test_account_is_deployed_undeployed() { +fn test_account_exists_undeployed() { // Given let (native_token, kakarot_core) = setup_contracts_for_testing(); @@ -133,7 +132,7 @@ fn test_account_is_deployed_undeployed() { let account = AccountTrait::fetch_or_create(evm_address()).unwrap(); // Then - assert(account.is_deployed() == false, 'account should be deployed'); + assert(account.exists() == false, 'account should be deployed'); } @@ -142,7 +141,7 @@ fn test_account_is_deployed_undeployed() { fn test_account_balance_contract_account() { // Given let (native_token, kakarot_core) = setup_contracts_for_testing(); - let mut ca_address = ContractAccountTrait::deploy(evm_address(), array![].span()) + let mut ca_address = ContractAccountTrait::deploy(evm_address(), 1, array![].span()) .expect('failed deploy contract account',); fund_account_with_native_token(ca_address.starknet, native_token, 0x1); @@ -160,7 +159,7 @@ fn test_account_balance_contract_account() { fn test_address_balance_contract_account() { // Given let (native_token, kakarot_core) = setup_contracts_for_testing(); - let mut ca_address = ContractAccountTrait::deploy(evm_address(), array![].span()) + let mut ca_address = ContractAccountTrait::deploy(evm_address(), 1, array![].span()) .expect('failed deploy contract account',); fund_account_with_native_token(ca_address.starknet, native_token, 0x1); diff --git a/crates/evm/src/tests/test_model/test_contract_account.cairo b/crates/evm/src/tests/test_model/test_contract_account.cairo index f86897a13..aecfee533 100644 --- a/crates/evm/src/tests/test_model/test_contract_account.cairo +++ b/crates/evm/src/tests/test_model/test_contract_account.cairo @@ -23,7 +23,7 @@ fn test_contract_account_deploy() { set_contract_address(kakarot_core.contract_address); let bytecode = counter_evm_bytecode(); - let ca_address = ContractAccountTrait::deploy(test_utils::evm_address(), bytecode) + let ca_address = ContractAccountTrait::deploy(test_utils::evm_address(), 1, bytecode) .expect('CA deployment failed'); let account = ContractAccountBuilderTrait::new(ca_address) .fetch_nonce() @@ -45,7 +45,7 @@ fn test_at_contract_account_deployed() { let evm_address = test_utils::evm_address(); let (native_token, kakarot_core) = contract_utils::setup_contracts_for_testing(); - let ca = ContractAccountTrait::deploy(evm_address, Default::default().span()).unwrap(); + let ca = ContractAccountTrait::deploy(evm_address, 1, Default::default().span()).unwrap(); let ca_address = ContractAccountTrait::at(evm_address) .unwrap() diff --git a/crates/evm/src/tests/test_state.cairo b/crates/evm/src/tests/test_state.cairo index 22620577d..0a2d5db15 100644 --- a/crates/evm/src/tests/test_state.cairo +++ b/crates/evm/src/tests/test_state.cairo @@ -194,7 +194,7 @@ mod test_simple_log { mod test_state { use contracts::tests::test_utils as contract_utils; use contracts::uninitialized_account::UninitializedAccount; - use evm::model::account::{Account, AccountType}; + use evm::model::account::{Account, AccountType, AccountTrait}; use evm::model::contract_account::{ContractAccountTrait}; use evm::model::eoa::EOATrait; use evm::model::{Event, Transfer, Address}; @@ -280,7 +280,7 @@ mod test_state { // Transfer native tokens to sender let (native_token, kakarot_core) = contract_utils::setup_contracts_for_testing(); let evm_address: EthAddress = test_utils::evm_address(); - let mut ca_address = ContractAccountTrait::deploy(evm_address, array![].span()) + let mut ca_address = ContractAccountTrait::deploy(evm_address, 1, array![].span()) .expect('sender deploy failed'); let mut state: State = Default::default(); @@ -352,6 +352,7 @@ mod test_state { assert(state.read_balance(recipient.evm).unwrap() == 100, 'Recipient balance mismatch'); } + #[test] #[available_gas(200000000)] fn test_read_balance_cached() { diff --git a/crates/evm/src/tests/test_utils.cairo b/crates/evm/src/tests/test_utils.cairo index ab5ae9ada..bb85c074c 100644 --- a/crates/evm/src/tests/test_utils.cairo +++ b/crates/evm/src/tests/test_utils.cairo @@ -24,7 +24,7 @@ fn test_address() -> Address { } fn other_evm_address() -> EthAddress { - 0xabde1.try_into().unwrap() + 'other_evm_address'.try_into().unwrap() } fn other_starknet_address() -> ContractAddress { @@ -90,7 +90,7 @@ fn setup_call_context() -> CallContext { let bytecode: Span = array![0x00].span(); let calldata: Span = array![4, 5, 6].span(); let value: u256 = callvalue(); - let address = test_address(); + let caller = test_address(); let read_only = false; let gas_price = 0xaaaaaa; let gas_limit = 0xffffff; @@ -98,7 +98,7 @@ fn setup_call_context() -> CallContext { let output_size = 0; CallContextTrait::new( - address, + caller, bytecode, calldata, value, @@ -143,6 +143,15 @@ fn setup_execution_context() -> ExecutionContext { ExecutionContextTrait::new(context_id, address, call_ctx, Default::default(), return_data,) } + +fn setup_execution_context_with_target(target: Address) -> ExecutionContext { + let context_id = ExecutionContextType::Root; + let call_ctx = setup_call_context(); + let return_data = array![1, 2, 3].span(); + + ExecutionContextTrait::new(context_id, target, call_ctx, Default::default(), return_data,) +} + fn setup_static_execution_context() -> ExecutionContext { let context_id = ExecutionContextType::Root; let call_ctx = setup_static_call_context(); @@ -252,6 +261,17 @@ fn setup_machine() -> Machine { } } +fn setup_machine_with_target(target: Address) -> Machine { + Machine { + current_ctx: BoxTrait::new(setup_execution_context_with_target(target)), + ctx_count: 1, + stack: Default::default(), + memory: Default::default(), + state: Default::default(), + error: Option::None + } +} + fn setup_static_machine() -> Machine { Machine { @@ -347,7 +367,7 @@ fn parent_ctx_return_data(ref self: Machine) -> Span { fn initialize_contract_account( eth_address: EthAddress, bytecode: Span, storage: Span<(u256, u256)> ) -> Result { - let mut ca_address = ContractAccountTrait::deploy(eth_address, bytecode) + let mut ca_address = ContractAccountTrait::deploy(eth_address, 1, bytecode) .expect('failed deploying CA'); // Set the storage of the contract account let account = Account { From 0c45d33b2088f42590c60fac6968965c1dddc2c9 Mon Sep 17 00:00:00 2001 From: Elias Tazartes <66871571+Eikix@users.noreply.github.com> Date: Mon, 20 Nov 2023 13:04:04 +0100 Subject: [PATCH 5/9] Revert "feat: selfdestruct (#554)" (#555) This reverts commit c53a713e99f1dafc45a4cbb4e81c1333e71276d1. --- crates/contracts/src/contract_account.cairo | 24 ++-- crates/contracts/src/eoa.cairo | 10 +- crates/contracts/src/tests/test_eoa.cairo | 2 +- .../src/tests/test_kakarot_core.cairo | 85 +++++++++++++- crates/evm/src/create_helpers.cairo | 2 +- crates/evm/src/execution.cairo | 36 +++--- .../environmental_information.cairo | 1 - .../src/instructions/system_operations.cairo | 36 +----- crates/evm/src/model.cairo | 4 +- crates/evm/src/model/account.cairo | 41 +++---- crates/evm/src/model/contract_account.cairo | 6 +- crates/evm/src/model/eoa.cairo | 2 +- .../src/tests/test_execution_context.cairo | 5 - .../test_block_information.cairo | 13 ++- .../test_environment_information.cairo | 16 +-- .../test_memory_operations.cairo | 23 ++-- .../test_system_operations.cairo | 109 +----------------- crates/evm/src/tests/test_model.cairo | 39 ++++--- .../test_model/test_contract_account.cairo | 4 +- crates/evm/src/tests/test_state.cairo | 5 +- crates/evm/src/tests/test_utils.cairo | 28 +---- 21 files changed, 206 insertions(+), 285 deletions(-) diff --git a/crates/contracts/src/contract_account.cairo b/crates/contracts/src/contract_account.cairo index 346cccc4a..f5a36343b 100644 --- a/crates/contracts/src/contract_account.cairo +++ b/crates/contracts/src/contract_account.cairo @@ -34,7 +34,6 @@ trait IContractAccount { fn nonce(self: @TContractState) -> u64; fn set_nonce(ref self: TContractState, new_nonce: u64); - fn increment_nonce(ref self: TContractState); // *** // JUMP // Records of valid jumps in the context of jump opcodes @@ -144,6 +143,7 @@ mod ContractAccount { } fn set_bytecode(ref self: ContractState, bytecode: Span) { + self.assert_only_kakarot_core(); let packed_bytecode: ByteArray = ByteArrayExTrait::from_bytes(bytecode); // data_address is h(h(sn_keccak("contract_account_bytecode")), evm_address) let data_address = storage_base_address_from_felt252( @@ -192,6 +192,7 @@ mod ContractAccount { } fn set_storage_at(ref self: ContractState, key: u256, value: u256) { + self.assert_only_kakarot_core(); let storage_address = compute_storage_base_address( selector!("contract_account_storage_keys"), array![key.low.into(), key.high.into()].span() @@ -208,6 +209,7 @@ mod ContractAccount { fn set_nonce(ref self: ContractState, new_nonce: u64) { + self.assert_only_kakarot_core(); let storage_address: StorageBaseAddress = storage_base_address_from_felt252( selector!("contract_account_nonce") ); @@ -215,25 +217,18 @@ mod ContractAccount { } - fn increment_nonce(ref self: ContractState) { - let storage_address: StorageBaseAddress = storage_base_address_from_felt252( - selector!("contract_account_nonce") - ); - let nonce = Store::::read(0, storage_address).expect(NONCE_READ_ERROR); - Store::::write(0, storage_address, nonce + 1).expect(NONCE_WRITE_ERROR) - } - fn is_false_positive_jumpdest(self: @ContractState, offset: usize) -> bool { panic_with_felt252('unimplemented') } fn set_false_positive_jumpdest(ref self: ContractState, offset: usize) { + self.assert_only_kakarot_core(); panic_with_felt252('unimplemented') } fn selfdestruct(ref self: ContractState) { - //TODO add access control + self.assert_only_kakarot_core(); self.set_nonce(0); self.evm_address.write(0.try_into().unwrap()); self.set_bytecode(array![].span()); @@ -243,11 +238,18 @@ mod ContractAccount { fn upgrade(ref self: ContractState, new_class_hash: ClassHash) { + self.assert_only_kakarot_core(); + self.upgradeable.upgrade_contract(new_class_hash); + } + } + + #[generate_trait] + impl PrivateImpl of PrivateTrait { + fn assert_only_kakarot_core(self: @ContractState) { assert( get_caller_address() == self.kakarot_core_address.read(), 'Caller not Kakarot Core address' ); - self.upgradeable.upgrade_contract(new_class_hash); } } } diff --git a/crates/contracts/src/eoa.cairo b/crates/contracts/src/eoa.cairo index 178213fa7..97a99073e 100644 --- a/crates/contracts/src/eoa.cairo +++ b/crates/contracts/src/eoa.cairo @@ -56,10 +56,9 @@ mod ExternallyOwnedAccount { fn chain_id(self: @ContractState) -> u128 { self.chain_id.read() } - // TODO: make this function reachable from an external invoke call // TODO: add some security methods to make sure that only some specific upgrades can be made ( low priority ) fn upgrade(ref self: ContractState, new_class_hash: ClassHash) { - assert(get_caller_address() == get_contract_address(), 'Caller not contract address'); + self.assert_only_self(); self.upgradeable.upgrade_contract(new_class_hash); } @@ -111,4 +110,11 @@ mod ExternallyOwnedAccount { array![] } } + + #[generate_trait] + impl PrivateImpl of PrivateTrait { + fn assert_only_self(self: @ContractState) { + assert(get_caller_address() == get_contract_address(), 'Caller not self'); + } + } } diff --git a/crates/contracts/src/tests/test_eoa.cairo b/crates/contracts/src/tests/test_eoa.cairo index 7b925e6fc..add866219 100644 --- a/crates/contracts/src/tests/test_eoa.cairo +++ b/crates/contracts/src/tests/test_eoa.cairo @@ -92,7 +92,7 @@ mod test_external_owned_account { #[test] #[available_gas(2000000000)] - #[should_panic(expected: ('Caller not contract address', 'ENTRYPOINT_FAILED'))] + #[should_panic(expected: ('Caller not self', 'ENTRYPOINT_FAILED'))] fn test_eoa_upgrade_from_noncontractaddress() { let (_, kakarot) = setup_contracts_for_testing(); let kakarot_address = kakarot.contract_address; diff --git a/crates/contracts/src/tests/test_kakarot_core.cairo b/crates/contracts/src/tests/test_kakarot_core.cairo index f6165de59..ad065e941 100644 --- a/crates/contracts/src/tests/test_kakarot_core.cairo +++ b/crates/contracts/src/tests/test_kakarot_core.cairo @@ -1,5 +1,7 @@ use contracts::contract_account::ContractAccount::TEST_CLASS_HASH as ContractAccountTestClassHash; +use contracts::contract_account::{IContractAccountDispatcher, IContractAccountDispatcherTrait}; use contracts::eoa::ExternallyOwnedAccount; +use contracts::kakarot_core::interface::IExtendedKakarotCoreDispatcherTrait; use contracts::kakarot_core::kakarot::StoredAccountType; use contracts::kakarot_core::{ interface::IExtendedKakarotCoreDispatcherImpl, KakarotCore, KakarotCore::{KakarotCoreInternal}, @@ -171,11 +173,84 @@ fn test_kakarot_core_upgrade_contract() { assert(version == 1, 'version is not 1'); } -// TODO add tests related to contract accounts once they can be deployed. -#[ignore] #[test] #[available_gas(20000000)] -fn test_kakarot_contract_account() {} +fn test_kakarot_contract_account_nonce() { + // Given + let (native_token, kakarot_core) = contract_utils::setup_contracts_for_testing(); + let address = ContractAccountTrait::deploy( + test_utils::other_evm_address(), Default::default().span() + ) + .unwrap(); + + // When + let nonce = kakarot_core.contract_account_nonce(address.evm); + + // Then + assert(nonce == 1, 'wrong nonce'); +} + + +#[test] +#[available_gas(20000000)] +fn test_kakarot_contract_account_storage_at() { + // Given + let (native_token, kakarot_core) = contract_utils::setup_contracts_for_testing(); + let address = ContractAccountTrait::deploy( + test_utils::other_evm_address(), Default::default().span() + ) + .unwrap(); + let ca = IContractAccountDispatcher { contract_address: address.starknet }; + let expected_value = 420; + let key = 69; + ca.set_storage_at(69, expected_value); + + // When + let value = kakarot_core.contract_account_storage_at(address.evm, key); + + // Then + assert(value == expected_value, 'wrong storage value'); +} + +#[test] +#[available_gas(2000000000)] +fn test_kakarot_contract_account_bytecode() { + // Given + let (native_token, kakarot_core) = contract_utils::setup_contracts_for_testing(); + let address = ContractAccountTrait::deploy( + test_utils::other_evm_address(), counter_evm_bytecode() + ) + .unwrap(); + + // When + let bytecode = kakarot_core.contract_account_bytecode(address.evm); + + // Then + assert(bytecode == counter_evm_bytecode(), 'wrong bytecode'); +} + + +#[test] +#[available_gas(20000000)] +#[should_panic(expected: ('unimplemented', 'ENTRYPOINT_FAILED'))] +fn test_kakarot_contract_account_false_positive_jumpdest() { + // Given + let (native_token, kakarot_core) = contract_utils::setup_contracts_for_testing(); + let address = ContractAccountTrait::deploy( + test_utils::other_evm_address(), Default::default().span() + ) + .unwrap(); + let ca = IContractAccountDispatcher { contract_address: address.starknet }; + let offset = 1337; + ca.set_false_positive_jumpdest(offset); + + // When + let is_false_jumpdest = kakarot_core + .contract_account_false_positive_jumpdest(address.evm, offset); + + // Then + assert(is_false_jumpdest, 'should be false jumpdest'); +} #[test] #[available_gas(2000000000000)] @@ -187,7 +262,7 @@ fn test_eth_call() { let eoa = kakarot_core.deploy_eoa(evm_address); let account = ContractAccountTrait::deploy( - test_utils::other_evm_address(), 1, counter_evm_bytecode() + test_utils::other_evm_address(), counter_evm_bytecode() ) .unwrap(); @@ -217,7 +292,7 @@ fn test_handle_call() { let evm_address = test_utils::evm_address(); let eoa = kakarot_core.deploy_eoa(evm_address); let account = ContractAccountTrait::deploy( - test_utils::other_evm_address(), 1, counter_evm_bytecode() + test_utils::other_evm_address(), counter_evm_bytecode() ) .unwrap(); diff --git a/crates/evm/src/create_helpers.cairo b/crates/evm/src/create_helpers.cairo index 271afc9c3..9db799152 100644 --- a/crates/evm/src/create_helpers.cairo +++ b/crates/evm/src/create_helpers.cairo @@ -98,7 +98,7 @@ impl MachineCreateHelpersImpl of MachineCreateHelpers { // - contract is already deployed at this location (type fetched from storage) // - Contract has been scheduled for deployment (type set in cache) // If the AccountType is unknown, then there's no collision. - if target_account.exists() { + if target_account.is_deployed() { return self.stack.push(0); }; diff --git a/crates/evm/src/execution.cairo b/crates/evm/src/execution.cairo index 2f5bdabb1..d74cc8d51 100644 --- a/crates/evm/src/execution.cairo +++ b/crates/evm/src/execution.cairo @@ -65,35 +65,14 @@ fn execute( let transfer = Transfer { sender: origin, recipient: target, amount: value }; match machine.state.add_transfer(transfer) { Result::Ok(x) => {}, - Result::Err(err) => { - return ExecutionResult { - status: Status::Reverted, - return_data: Default::default().span(), - destroyed_contracts: Default::default().span(), - create_addresses: Default::default().span(), - events: Default::default().span(), - state: machine.state, - error: Option::Some(err) - }; - } + Result::Err(err) => { return reverted_with_err(machine, err); } } // cache the EOA of origin inside the state, and set its nonce to get_tx_info().unbox().nonce. // This allows us to call create with the correct nonce as the salt. let mut origin_account = match machine.state.get_account(origin.evm) { Result::Ok(account) => { account }, - //TODO create helper method to avoid this boilerplace - Result::Err(err) => { - return ExecutionResult { - status: Status::Reverted, - return_data: Default::default().span(), - destroyed_contracts: Default::default().span(), - create_addresses: Default::default().span(), - events: Default::default().span(), - state: machine.state, - error: Option::Some(err) - }; - } + Result::Err(err) => { return reverted_with_err(machine, err); } }; origin_account.nonce = starknet::get_tx_info().unbox().nonce.try_into().unwrap(); machine.state.set_account(origin_account); @@ -118,3 +97,14 @@ fn execute( } } +fn reverted_with_err(self: Machine, error: EVMError) -> ExecutionResult { + ExecutionResult { + status: Status::Reverted, + return_data: Default::default().span(), + destroyed_contracts: Default::default().span(), + create_addresses: Default::default().span(), + events: Default::default().span(), + state: self.state, + error: Option::Some(error) + } +} diff --git a/crates/evm/src/instructions/environmental_information.cairo b/crates/evm/src/instructions/environmental_information.cairo index 4bd0b47c0..0dd292a7c 100644 --- a/crates/evm/src/instructions/environmental_information.cairo +++ b/crates/evm/src/instructions/environmental_information.cairo @@ -244,7 +244,6 @@ impl EnvironmentInformationImpl of EnvironmentInformationTrait { // -> Undeployed CAs that might be deployed later, but currently don't // exist and have only been touched for value transfers // -> Undeployed EOAs - // Selfdestructed CAs still exist until the end of the TX. if account.is_precompile() || (account.account_type == AccountType::Unknown) { return self.stack.push(0); } diff --git a/crates/evm/src/instructions/system_operations.cairo b/crates/evm/src/instructions/system_operations.cairo index 3fab70a75..904efd79c 100644 --- a/crates/evm/src/instructions/system_operations.cairo +++ b/crates/evm/src/instructions/system_operations.cairo @@ -1,14 +1,12 @@ //! System operations. use box::BoxTrait; -use contracts::kakarot_core::{KakarotCore, IKakarotCore}; use evm::call_helpers::{MachineCallHelpers, CallType}; use evm::create_helpers::{MachineCreateHelpers, CreateType}; use evm::errors::{EVMError, VALUE_TRANSFER_IN_STATIC_CALL, WRITE_IN_STATIC_CONTEXT}; use evm::machine::{Machine, MachineCurrentContextTrait}; use evm::memory::MemoryTrait; use evm::model::account::{AccountTrait}; -use evm::model::{Address, Transfer}; use evm::stack::StackTrait; use evm::state::StateTrait; use utils::math::Exponentiation; @@ -133,38 +131,6 @@ impl SystemOperations of SystemOperationsTrait { if self.read_only() { return Result::Err(EVMError::WriteInStaticContext(WRITE_IN_STATIC_CONTEXT)); } - let kakarot_state = KakarotCore::unsafe_new_contract_state(); - let stack_address = self.stack.pop_eth_address()?; - - //TODO Remove this when https://eips.ethereum.org/EIPS/eip-6780 is validated - let recipient_evm_address = if (stack_address == self.address().evm) { - 0.try_into().unwrap() - } else { - stack_address - }; - let recipient_starknet_address = kakarot_state - .compute_starknet_address(recipient_evm_address); - let mut account = self.state.get_account(self.address().evm)?; - - let recipient = Address { - evm: recipient_evm_address, starknet: recipient_starknet_address - }; - - // Transfer balance - self - .state - .add_transfer( - Transfer { - sender: account.address(), - recipient, - amount: self.state.read_balance(account.address().evm)? - } - ); - - // Register for selfdestruct - account.selfdestruct(); - self.state.set_account(account); - self.set_stopped(); - Result::Ok(()) + Result::Err(EVMError::NotImplemented) } } diff --git a/crates/evm/src/model.cairo b/crates/evm/src/model.cairo index 989c9278a..f564d345e 100644 --- a/crates/evm/src/model.cairo +++ b/crates/evm/src/model.cairo @@ -31,9 +31,9 @@ struct Address { #[generate_trait] impl AddressImpl of AddressTrait { - fn is_deployed(self: EthAddress) -> bool { + fn is_registered(evm_address: EthAddress) -> bool { let mut kakarot_state = KakarotCore::unsafe_new_contract_state(); - let maybe_account = kakarot_state.address_registry(self); + let maybe_account = kakarot_state.address_registry(evm_address); match maybe_account { Option::Some(_) => true, Option::None => false diff --git a/crates/evm/src/model/account.cairo b/crates/evm/src/model/account.cairo index a8c19eb62..c8f3bf7e1 100644 --- a/crates/evm/src/model/account.cairo +++ b/crates/evm/src/model/account.cairo @@ -97,8 +97,10 @@ impl AccountImpl of AccountTrait { let starknet_address = kakarot_state.compute_starknet_address(evm_address); Result::Ok( // If no account exists at `address`, then we are trying to - // access an undeployed account (CA or EOA). We create an - // empty account with the correct address and return it. + // access an undeployed contract account. Simple value + // transfers between EOAs don't call this function - + // Therefore, we're sure that only contract accounts are + // undeployed. Account { account_type: AccountType::Unknown, address: Address { starknet: starknet_address, evm: evm_address, }, @@ -159,8 +161,8 @@ impl AccountImpl of AccountTrait { /// be registered already, and the nonce must not be 0 or the code must not /// be empty #[inline(always)] - fn should_deploy(self: @Account) -> bool { - if self.is_ca() && (*self.nonce != 0 || !(*self.code).is_empty()) { + fn should_deploy(self: @Account, is_registered: bool) -> bool { + if !is_registered && self.is_ca() && (*self.nonce != 0 || !(*self.code).is_empty()) { return true; }; false @@ -178,10 +180,10 @@ impl AccountImpl of AccountTrait { /// /// `Ok(())` if the commit was successful, otherwise an `EVMError`. fn commit(self: @Account) -> Result<(), EVMError> { - // Case account is already deployed onchain - let is_deployed = self.address().evm.is_deployed(); + // Case account exists and is already on chain + let is_registered = AddressTrait::is_registered(self.address().evm); - if is_deployed { + if is_registered { match self.account_type { AccountType::EOA(eoa) => { // no - op @@ -194,22 +196,21 @@ impl AccountImpl of AccountTrait { .set_address_registry( self.address().evm, StoredAccountType::UnexistingAccount ); - return ContractAccountTrait::selfdestruct(self); + return self.selfdestruct(); } self.store_nonce(*self.nonce) //Storage is handled outside of the account and must be commited after all accounts are commited. }, AccountType::Unknown => { Result::Ok(()) } } - } else if self.should_deploy() { - // If SELFDESTRUCT, deploy empty SN account - let (initial_nonce, initial_code) = if (*self.selfdestruct == true) { - (0, Default::default().span()) - } else { - (*self.nonce, *self.code) + } else if self.should_deploy(is_registered) { + //Case new account + // If SELFDESTRUCT, just do nothing + if (*self.selfdestruct == true) { + return Result::Ok(()); }; - ContractAccountTrait::deploy(self.address().evm, initial_nonce, initial_code)?; - Result::Ok(()) + let mut ca_address = ContractAccountTrait::deploy(self.address().evm, *self.code)?; + self.store_nonce(*self.nonce) //Storage is handled outside of the account and must be commited after all accounts are commited. } else { Result::Ok(()) @@ -230,19 +231,19 @@ impl AccountImpl of AccountTrait { false } - /// Returns whether an account exists at the given address. + /// Returns whether an accound is deployed at the given address. /// /// Based on the state of the account in the cache - the account can - /// not be deployed on-chain, but already exist in the KakarotState. + /// not be commited on-chain, but already be deployed in the KakarotState. /// # Arguments /// /// * `address` - The Ethereum address to look up. /// /// # Returns /// - /// `true` if an account exists at this address, `false` otherwise. + /// `true` if an account is deployed at this address, `false` otherwise. #[inline(always)] - fn exists(self: @Account) -> bool { + fn is_deployed(self: @Account) -> bool { let is_known = *self.account_type != AccountType::Unknown; //TODO(account) verify whether is_known is a sufficient condition diff --git a/crates/evm/src/model/contract_account.cairo b/crates/evm/src/model/contract_account.cairo index 5c09a7f96..4f8a0d2b1 100644 --- a/crates/evm/src/model/contract_account.cairo +++ b/crates/evm/src/model/contract_account.cairo @@ -53,9 +53,7 @@ impl ContractAccountImpl of ContractAccountTrait { /// * The evm_address and starknet_address the CA is deployed at - which is KakarotCore /// # Errors /// * `ACCOUNT_EXISTS` - If a contract account already exists at the given `evm_address` - fn deploy( - evm_address: EthAddress, nonce: u64, bytecode: Span - ) -> Result { + fn deploy(evm_address: EthAddress, bytecode: Span) -> Result { let mut kakarot_state = KakarotCore::unsafe_new_contract_state(); let account_class_hash = kakarot_state.account_class_hash(); let kakarot_address = get_contract_address(); @@ -73,7 +71,7 @@ impl ContractAccountImpl of ContractAccountTrait { // Initialize the account let account = IContractAccountDispatcher { contract_address: starknet_address }; - account.set_nonce(nonce); + account.set_nonce(1); account.set_bytecode(bytecode); // Kakarot Core logic diff --git a/crates/evm/src/model/eoa.cairo b/crates/evm/src/model/eoa.cairo index 3e7519915..5ef0f9852 100644 --- a/crates/evm/src/model/eoa.cairo +++ b/crates/evm/src/model/eoa.cairo @@ -18,7 +18,7 @@ impl EOAImpl of EOATrait { /// * `evm_address` - The EVM address of the EOA to deploy. fn deploy(evm_address: EthAddress) -> Result { // Unlike CAs, there is not check for the existence of an EOA prealably to calling `EOATrait::deploy` - therefore, we need to check that there is no collision. - let mut is_deployed = evm_address.is_deployed(); + let mut is_deployed = AddressTrait::is_registered(evm_address); if is_deployed { return Result::Err(EVMError::DeployError(EOA_EXISTS)); } diff --git a/crates/evm/src/tests/test_execution_context.cairo b/crates/evm/src/tests/test_execution_context.cairo index e4ab6df86..190067a52 100644 --- a/crates/evm/src/tests/test_execution_context.cairo +++ b/crates/evm/src/tests/test_execution_context.cairo @@ -18,11 +18,6 @@ use starknet::{EthAddress, ContractAddress}; use test_utils::{callvalue, test_address}; use traits::PartialEq; -// TODO remove once no longer required (see https://github.com/starkware-libs/cairo/issues/3863) -#[inline(never)] -fn no_op() {} - - #[test] #[available_gas(1000000)] fn test_call_context_new() { diff --git a/crates/evm/src/tests/test_instructions/test_block_information.cairo b/crates/evm/src/tests/test_instructions/test_block_information.cairo index 27aa3d73e..983192e09 100644 --- a/crates/evm/src/tests/test_instructions/test_block_information.cairo +++ b/crates/evm/src/tests/test_instructions/test_block_information.cairo @@ -45,7 +45,6 @@ fn test_exec_blockhash_above_bounds() { // TODO: implement exec_blockhash testing for block number within bounds // https://github.com/starkware-libs/cairo/blob/77a7e7bc36aa1c317bb8dd5f6f7a7e6eef0ab4f3/crates/cairo-lang-starknet/cairo_level_tests/interoperability.cairo#L173 -#[ignore] #[test] #[available_gas(20000000)] fn test_exec_blockhash_within_bounds() { @@ -56,9 +55,13 @@ fn test_exec_blockhash_within_bounds() { // When machine.stack.push(244).unwrap(); - machine.exec_blockhash(); - // Then - assert(machine.stack.peek().unwrap() == 0xF, 'stack top should be 0xF'); + + //TODO the CASM runner used in tests doesn't implement + //`get_block_hash_syscall` yet. As such, this test should fail no if the + //queried block is within bounds + assert(machine.exec_blockhash().is_err(), 'CASM Runner cant blockhash'); +// Then +// assert(machine.stack.peek().unwrap() == 0xF, 'stack top should be 0xF'); } @@ -152,7 +155,7 @@ fn test_exec_selfbalance_zero() { fn test_exec_selfbalance_contract_account() { // Given let (native_token, kakarot_core) = setup_contracts_for_testing(); - let mut ca_address = ContractAccountTrait::deploy(evm_address(), 1, array![].span()) + let mut ca_address = ContractAccountTrait::deploy(evm_address(), array![].span()) .expect('failed deploy contract account',); fund_account_with_native_token(ca_address.starknet, native_token, 0x1); diff --git a/crates/evm/src/tests/test_instructions/test_environment_information.cairo b/crates/evm/src/tests/test_instructions/test_environment_information.cairo index 2027f2e90..a3787094b 100644 --- a/crates/evm/src/tests/test_instructions/test_environment_information.cairo +++ b/crates/evm/src/tests/test_instructions/test_environment_information.cairo @@ -93,7 +93,7 @@ fn test_exec_balance_zero() { fn test_exec_balance_contract_account() { // Given let (native_token, kakarot_core) = setup_contracts_for_testing(); - let mut ca_address = ContractAccountTrait::deploy(evm_address(), 1, array![].span()) + let mut ca_address = ContractAccountTrait::deploy(evm_address(), array![].span()) .expect('failed deploy contract account',); fund_account_with_native_token(ca_address.starknet, native_token, 0x1); @@ -579,7 +579,7 @@ fn test_exec_extcodesize_ca_empty() { let (native_token, kakarot_core) = setup_contracts_for_testing(); // The bytecode remains empty, and we expect the empty hash in return - let mut ca_address = ContractAccountTrait::deploy(evm_address(), 1, array![].span()); + let mut ca_address = ContractAccountTrait::deploy(evm_address(), array![].span()); machine.stack.push(evm_address.into()); @@ -600,7 +600,7 @@ fn test_exec_extcodesize_ca_with_bytecode() { let (native_token, kakarot_core) = setup_contracts_for_testing(); // The bytecode stored is the bytecode of a Counter.sol smart contract - ContractAccountTrait::deploy(evm_address(), 1, counter_evm_bytecode()); + ContractAccountTrait::deploy(evm_address(), counter_evm_bytecode()); machine.stack.push(evm_address.into()); // When @@ -624,7 +624,7 @@ fn test_exec_extcodecopy_ca() { let (native_token, kakarot_core) = setup_contracts_for_testing(); // The bytecode stored is the bytecode of a Counter.sol smart contract - ContractAccountTrait::deploy(evm_address(), 1, counter_evm_bytecode()); + ContractAccountTrait::deploy(evm_address(), counter_evm_bytecode()); // size machine.stack.push(50).unwrap(); @@ -655,7 +655,7 @@ fn test_exec_extcodecopy_ca_offset_out_of_bounds() { let (native_token, kakarot_core) = setup_contracts_for_testing(); // The bytecode stored is the bytecode of a Counter.sol smart contract - ContractAccountTrait::deploy(evm_address(), 1, counter_evm_bytecode()); + ContractAccountTrait::deploy(evm_address(), counter_evm_bytecode()); // size machine.stack.push(5); @@ -925,7 +925,7 @@ fn test_exec_extcodehash_selfdestructed() { let (native_token, kakarot_core) = setup_contracts_for_testing(); // The bytecode remains empty, and we expect the empty hash in return - let mut ca_address = ContractAccountTrait::deploy(evm_address, 1, array![].span()) + let mut ca_address = ContractAccountTrait::deploy(evm_address, array![].span()) .expect('CA deployment failed'); let account = Account { account_type: AccountType::ContractAccount, @@ -983,7 +983,7 @@ fn test_exec_extcodehash_ca_empty() { let mut machine = setup_machine(); let (native_token, kakarot_core) = setup_contracts_for_testing(); // The bytecode remains empty, and we expect the empty hash in return - let mut ca_address = ContractAccountTrait::deploy(evm_address(), 1, array![].span()); + let mut ca_address = ContractAccountTrait::deploy(evm_address(), array![].span()); machine.stack.push(evm_address.into()); @@ -1027,7 +1027,7 @@ fn test_exec_extcodehash_ca_with_bytecode() { let (native_token, kakarot_core) = setup_contracts_for_testing(); // The bytecode stored is the bytecode of a Counter.sol smart contract - let mut ca_address = ContractAccountTrait::deploy(evm_address(), 1, counter_evm_bytecode()); + let mut ca_address = ContractAccountTrait::deploy(evm_address(), counter_evm_bytecode()); machine.stack.push(evm_address.into()); // When diff --git a/crates/evm/src/tests/test_instructions/test_memory_operations.cairo b/crates/evm/src/tests/test_instructions/test_memory_operations.cairo index cd6075564..28d0fbd30 100644 --- a/crates/evm/src/tests/test_instructions/test_memory_operations.cairo +++ b/crates/evm/src/tests/test_instructions/test_memory_operations.cairo @@ -355,7 +355,7 @@ fn test_exec_jump_out_of_bounds() { // Remove ignore once its handled #[test] #[available_gas(20000000)] -#[ignore] +#[should_panic(expected: ('exec_jump should throw error',))] fn test_exec_jump_inside_pushn() { // Given let bytecode: Span = array![0x60, 0x5B, 0x60, 0x00].span(); @@ -367,8 +367,11 @@ fn test_exec_jump_inside_pushn() { let result = machine.exec_jump(); // Then - assert(result.is_err(), 'invalid jump dest'); - assert(result.unwrap_err() == EVMError::JumpError(INVALID_DESTINATION), 'invalid jump dest'); + assert(result.is_err(), 'exec_jump should throw error'); + assert( + result.unwrap_err() == EVMError::JumpError(INVALID_DESTINATION), + 'jump dest should be invalid' + ); } #[test] @@ -481,7 +484,7 @@ fn test_exec_jumpi_invalid_zero() { // Remove ignore once its handled #[test] #[available_gas(20000000)] -#[ignore] +#[should_panic(expected: ('exec_jump should throw error',))] fn test_exec_jumpi_inside_pushn() { // Given let bytecode: Span = array![0x60, 0x5B, 0x60, 0x00].span(); @@ -495,8 +498,11 @@ fn test_exec_jumpi_inside_pushn() { let result = machine.exec_jumpi(); // Then - assert(result.is_err(), 'invalid jump dest'); - assert(result.unwrap_err() == EVMError::JumpError(INVALID_DESTINATION), 'invalid jump dest'); + assert(result.is_err(), 'exec_jump should throw error'); + assert( + result.unwrap_err() == EVMError::JumpError(INVALID_DESTINATION), + 'jump dest should be invalid' + ); } #[test] @@ -526,7 +532,7 @@ fn test_exec_sload_from_storage() { // Given let (native_token, kakarot_core) = setup_contracts_for_testing(); let mut machine = setup_machine(); - let mut ca_address = ContractAccountTrait::deploy(machine.address().evm, 1, array![].span()) + let mut ca_address = ContractAccountTrait::deploy(machine.address().evm, array![].span()) .unwrap(); let account = Account { account_type: AccountType::ContractAccount, @@ -597,8 +603,7 @@ fn test_exec_sstore_finalized() { let (native_token, kakarot_core) = setup_contracts_for_testing(); let mut machine = setup_machine(); // Deploys the contract account to be able to commit storage changes. - let ca_address = ContractAccountTrait::deploy(machine.address().evm, 1, array![].span()) - .unwrap(); + let ca_address = ContractAccountTrait::deploy(machine.address().evm, array![].span()).unwrap(); let account = Account { account_type: AccountType::ContractAccount, address: ca_address, diff --git a/crates/evm/src/tests/test_instructions/test_system_operations.cairo b/crates/evm/src/tests/test_instructions/test_system_operations.cairo index afea1f5c5..ab3bb25d0 100644 --- a/crates/evm/src/tests/test_instructions/test_system_operations.cairo +++ b/crates/evm/src/tests/test_instructions/test_system_operations.cairo @@ -1,29 +1,22 @@ use contracts::kakarot_core::interface::IExtendedKakarotCoreDispatcherTrait; use contracts::tests::test_data::{storage_evm_bytecode, storage_evm_initcode}; -use contracts::tests::test_utils::{fund_account_with_native_token, setup_contracts_for_testing}; -use debug::PrintTrait; +use contracts::tests::test_utils::setup_contracts_for_testing; use evm::call_helpers::{MachineCallHelpers, MachineCallHelpersImpl}; use evm::context::{ExecutionContext, ExecutionContextTrait, ExecutionContextType}; -use evm::errors::EVMErrorTrait; use evm::instructions::MemoryOperationTrait; use evm::instructions::SystemOperationsTrait; use evm::interpreter::EVMInterpreterTrait; use evm::machine::{Machine, MachineCurrentContextTrait}; use evm::memory::MemoryTrait; -use evm::model::account::{Account}; use evm::model::contract_account::ContractAccountTrait; -use evm::model::eoa::EOATrait; -use evm::model::{AccountTrait, Address, AccountType, Transfer}; +use evm::model::{AccountTrait, Address}; use evm::stack::StackTrait; -use evm::state::{State, StateTrait}; +use evm::state::StateTrait; use evm::tests::test_utils::{ setup_machine_with_nested_execution_context, setup_machine, setup_machine_with_bytecode, - test_address, initialize_contract_account, native_token, evm_address, other_evm_address, - setup_machine_with_target + initialize_contract_account, native_token, evm_address }; -use openzeppelin::token::erc20::interface::{IERC20CamelDispatcher, IERC20CamelDispatcherTrait}; use starknet::EthAddress; -use starknet::testing::set_contract_address; use utils::helpers::load_word; use utils::traits::EthAddressIntoU256; @@ -404,7 +397,7 @@ fn test_exec_create2() { let deployed_bytecode = array![0xff].span(); let eth_address: EthAddress = 0x00000000000000000075766d5f61646472657373_u256.into(); - let contract_address = ContractAccountTrait::deploy(eth_address, 1, deployed_bytecode) + let contract_address = ContractAccountTrait::deploy(eth_address, deployed_bytecode) .expect('failed deploying CA'); let mut ctx = machine.current_ctx.unbox(); @@ -445,95 +438,3 @@ fn test_exec_create2() { assert(account.nonce() == 1, 'wrong nonce'); assert(account.code == storage_evm_bytecode(), 'wrong bytecode'); } - -#[test] -#[available_gas(200000000)] -fn test_exec_selfdestruct_existing_ca() { - // Given - let (native_token, kakarot_core) = setup_contracts_for_testing(); - let destroyed_address = test_address().evm; // address in machine call context - let ca_address = ContractAccountTrait::deploy( - destroyed_address, 1, array![0x1, 0x2, 0x3].span() - ) - .expect('failed deploying CA'); - fund_account_with_native_token(ca_address.starknet, native_token, 1000); - let recipient = EOATrait::deploy(other_evm_address()).expect('failed deploying eoa'); - let mut machine = setup_machine_with_target(ca_address); - - // When - machine.stack.push(recipient.evm.into()); - machine.exec_selfdestruct().expect('selfdestruct failed'); - machine.state.commit_context(); - machine.state.commit_state(); - machine.state = Default::default(); //empty state to force re-fetch from SN - // Then - let destructed = machine.state.get_account(ca_address.evm).expect('couldnt fetch destructed'); - - assert(destructed.nonce() == 0, 'destructed nonce should be 0'); - assert( - destructed.balance().expect('couldnt get balance') == 0, 'destructed balance should be 0' - ); - assert(destructed.bytecode().len() == 0, 'bytecode should be empty'); - - let recipient = machine.state.get_account(recipient.evm).expect('couldnt fetch recipient'); -//TODO this assertion fails because of deterministic address calculations. -// Once addressed in the compiler code, this test should be fixed. -// in selfdestruct, we execute: -// let recipient_starknet_address = kakarot_state -// .compute_starknet_address(recipient_evm_address); -// assert(recipient.balance().expect('couldnt get balance') == 1000, 'wrong recipient balance'); -} - -#[test] -#[available_gas(200000000)] -fn test_selfdestruct_undeployed_ca() { //TODO -// for now we can't fund an undeployed CA as the SN address is not deterministically calculated in the runner, so no way of funding a SN address by calculating it from an EVM address -// This test should -// - deploy kkt and token, deploy an EOA -// - call `get_account` on an undeployed account, set its type to CA, its nonce to 1, its code to something -// - selfdestruct it -// verify that the value transfer has succeeded -// - -} - -#[test] -#[available_gas(200000000)] -fn test_exec_selfdestruct_add_transfer_post_selfdestruct() { - // Given - let (native_token, kakarot_core) = setup_contracts_for_testing(); - - // Deploy sender and recipiens EOAs, and CA that will be selfdestructed and funded with 100 tokens - let sender = EOATrait::deploy('sender'.try_into().unwrap()).expect('failed deploy EOA',); - let recipient = EOATrait::deploy('recipient'.try_into().unwrap()).expect('failed deploy EOA',); - let ca_address = ContractAccountTrait::deploy( - 'contract'.try_into().unwrap(), 1, array![].span() - ) - .expect('failed deploy CA'); - fund_account_with_native_token(sender.starknet, native_token, 150); - fund_account_with_native_token(ca_address.starknet, native_token, 100); - let mut machine = setup_machine_with_target(ca_address); - - // Cache the CA into state - let mut ca = machine.state.get_account('contract'.try_into().unwrap()).expect('couldnt get CA'); - - // When - machine.stack.push(recipient.evm.into()); - machine.exec_selfdestruct().expect('selfdestruct failed'); - // Add a transfer from sender to CA - after it was selfdestructed in local state. This transfer should go through. - let transfer = Transfer { sender, recipient: ca_address, amount: 150 }; - machine.state.add_transfer(transfer).unwrap(); - machine.state.commit_context(); - machine.state.commit_state(); - machine.state = Default::default(); //empty state to force re-fetch from SN - - // Then - let recipient_balance = native_token.balanceOf(recipient.starknet); - let sender_balance = native_token.balanceOf(sender.starknet); - let ca_balance = native_token.balanceOf(ca_address.starknet); - //TODO this assert fails because of deterministic address calculations. - //FIXME when addressed in the compiler code, this test should be fixed. - // assert(recipient_balance == 100, 'recipient wrong balance'); - assert(sender_balance == 0, 'sender wrong balance'); - assert(ca_balance == 150, 'ca wrong balance'); -} diff --git a/crates/evm/src/tests/test_model.cairo b/crates/evm/src/tests/test_model.cairo index d0d67f850..c08c8cd5c 100644 --- a/crates/evm/src/tests/test_model.cairo +++ b/crates/evm/src/tests/test_model.cairo @@ -10,7 +10,7 @@ use starknet::testing::set_contract_address; #[test] #[available_gas(20000000)] -fn test_is_deployed_eoa_exists() { +fn test_is_registered_eoa_exists() { // Given let (native_token, kakarot_core) = setup_contracts_for_testing(); let eoa_address = EOATrait::deploy(evm_address()).expect('failed deploy contract account',); @@ -19,38 +19,39 @@ fn test_is_deployed_eoa_exists() { // When set_contract_address(kakarot_core.contract_address); - let is_deployed = evm_address().is_deployed(); + let is_registered = AddressTrait::is_registered(evm_address()); // Then - assert(is_deployed, 'account should be deployed'); + assert(is_registered, 'account should be deployed'); } #[test] #[available_gas(20000000)] -fn test_is_deployed_ca_exists() { +fn test_is_registered_ca_exists() { // Given let (native_token, kakarot_core) = setup_contracts_for_testing(); - ContractAccountTrait::deploy(evm_address(), 1, array![].span()) + ContractAccountTrait::deploy(evm_address(), array![].span()) .expect('failed deploy contract account',); // When - let is_deployed = evm_address().is_deployed(); + let is_registered = AddressTrait::is_registered(evm_address()); + // Then - assert(is_deployed, 'account should be deployed'); + assert(is_registered, 'account should be deployed'); } #[test] #[available_gas(20000000)] -fn test_is_deployed_undeployed() { +fn test_is_registered_undeployed() { // Given let (native_token, kakarot_core) = setup_contracts_for_testing(); // When set_contract_address(kakarot_core.contract_address); - let is_deployed = evm_address().is_deployed(); + let is_registered = AddressTrait::is_registered(evm_address()); // Then - assert(!is_deployed, 'account shouldnt be deployed'); + assert(!is_registered, 'account should be undeployed'); } @@ -93,7 +94,7 @@ fn test_address_balance_eoa() { #[test] #[available_gas(5000000)] -fn test_account_exists_eoa() { +fn test_account_is_deployed_eoa() { // Given let (native_token, kakarot_core) = setup_contracts_for_testing(); let mut eoa_address = EOATrait::deploy(evm_address()).expect('failed deploy eoa',); @@ -102,29 +103,29 @@ fn test_account_exists_eoa() { let account = AccountTrait::fetch(evm_address()).unwrap().unwrap(); // Then - assert(account.exists() == true, 'account should be deployed'); + assert(account.is_deployed() == true, 'account should be deployed'); } #[test] #[available_gas(5000000)] -fn test_account_exists_contract_account() { +fn test_account_is_deployed_contract_account() { // Given let (native_token, kakarot_core) = setup_contracts_for_testing(); - let mut ca_address = ContractAccountTrait::deploy(evm_address(), 1, array![].span()) + let mut ca_address = ContractAccountTrait::deploy(evm_address(), array![].span()) .expect('failed deploy contract account',); // When let account = AccountTrait::fetch(evm_address()).unwrap().unwrap(); // Then - assert(account.exists() == true, 'account should be deployed'); + assert(account.is_deployed() == true, 'account should be deployed'); } #[test] #[available_gas(5000000)] -fn test_account_exists_undeployed() { +fn test_account_is_deployed_undeployed() { // Given let (native_token, kakarot_core) = setup_contracts_for_testing(); @@ -132,7 +133,7 @@ fn test_account_exists_undeployed() { let account = AccountTrait::fetch_or_create(evm_address()).unwrap(); // Then - assert(account.exists() == false, 'account should be deployed'); + assert(account.is_deployed() == false, 'account should be deployed'); } @@ -141,7 +142,7 @@ fn test_account_exists_undeployed() { fn test_account_balance_contract_account() { // Given let (native_token, kakarot_core) = setup_contracts_for_testing(); - let mut ca_address = ContractAccountTrait::deploy(evm_address(), 1, array![].span()) + let mut ca_address = ContractAccountTrait::deploy(evm_address(), array![].span()) .expect('failed deploy contract account',); fund_account_with_native_token(ca_address.starknet, native_token, 0x1); @@ -159,7 +160,7 @@ fn test_account_balance_contract_account() { fn test_address_balance_contract_account() { // Given let (native_token, kakarot_core) = setup_contracts_for_testing(); - let mut ca_address = ContractAccountTrait::deploy(evm_address(), 1, array![].span()) + let mut ca_address = ContractAccountTrait::deploy(evm_address(), array![].span()) .expect('failed deploy contract account',); fund_account_with_native_token(ca_address.starknet, native_token, 0x1); diff --git a/crates/evm/src/tests/test_model/test_contract_account.cairo b/crates/evm/src/tests/test_model/test_contract_account.cairo index aecfee533..f86897a13 100644 --- a/crates/evm/src/tests/test_model/test_contract_account.cairo +++ b/crates/evm/src/tests/test_model/test_contract_account.cairo @@ -23,7 +23,7 @@ fn test_contract_account_deploy() { set_contract_address(kakarot_core.contract_address); let bytecode = counter_evm_bytecode(); - let ca_address = ContractAccountTrait::deploy(test_utils::evm_address(), 1, bytecode) + let ca_address = ContractAccountTrait::deploy(test_utils::evm_address(), bytecode) .expect('CA deployment failed'); let account = ContractAccountBuilderTrait::new(ca_address) .fetch_nonce() @@ -45,7 +45,7 @@ fn test_at_contract_account_deployed() { let evm_address = test_utils::evm_address(); let (native_token, kakarot_core) = contract_utils::setup_contracts_for_testing(); - let ca = ContractAccountTrait::deploy(evm_address, 1, Default::default().span()).unwrap(); + let ca = ContractAccountTrait::deploy(evm_address, Default::default().span()).unwrap(); let ca_address = ContractAccountTrait::at(evm_address) .unwrap() diff --git a/crates/evm/src/tests/test_state.cairo b/crates/evm/src/tests/test_state.cairo index 0a2d5db15..22620577d 100644 --- a/crates/evm/src/tests/test_state.cairo +++ b/crates/evm/src/tests/test_state.cairo @@ -194,7 +194,7 @@ mod test_simple_log { mod test_state { use contracts::tests::test_utils as contract_utils; use contracts::uninitialized_account::UninitializedAccount; - use evm::model::account::{Account, AccountType, AccountTrait}; + use evm::model::account::{Account, AccountType}; use evm::model::contract_account::{ContractAccountTrait}; use evm::model::eoa::EOATrait; use evm::model::{Event, Transfer, Address}; @@ -280,7 +280,7 @@ mod test_state { // Transfer native tokens to sender let (native_token, kakarot_core) = contract_utils::setup_contracts_for_testing(); let evm_address: EthAddress = test_utils::evm_address(); - let mut ca_address = ContractAccountTrait::deploy(evm_address, 1, array![].span()) + let mut ca_address = ContractAccountTrait::deploy(evm_address, array![].span()) .expect('sender deploy failed'); let mut state: State = Default::default(); @@ -352,7 +352,6 @@ mod test_state { assert(state.read_balance(recipient.evm).unwrap() == 100, 'Recipient balance mismatch'); } - #[test] #[available_gas(200000000)] fn test_read_balance_cached() { diff --git a/crates/evm/src/tests/test_utils.cairo b/crates/evm/src/tests/test_utils.cairo index bb85c074c..ab5ae9ada 100644 --- a/crates/evm/src/tests/test_utils.cairo +++ b/crates/evm/src/tests/test_utils.cairo @@ -24,7 +24,7 @@ fn test_address() -> Address { } fn other_evm_address() -> EthAddress { - 'other_evm_address'.try_into().unwrap() + 0xabde1.try_into().unwrap() } fn other_starknet_address() -> ContractAddress { @@ -90,7 +90,7 @@ fn setup_call_context() -> CallContext { let bytecode: Span = array![0x00].span(); let calldata: Span = array![4, 5, 6].span(); let value: u256 = callvalue(); - let caller = test_address(); + let address = test_address(); let read_only = false; let gas_price = 0xaaaaaa; let gas_limit = 0xffffff; @@ -98,7 +98,7 @@ fn setup_call_context() -> CallContext { let output_size = 0; CallContextTrait::new( - caller, + address, bytecode, calldata, value, @@ -143,15 +143,6 @@ fn setup_execution_context() -> ExecutionContext { ExecutionContextTrait::new(context_id, address, call_ctx, Default::default(), return_data,) } - -fn setup_execution_context_with_target(target: Address) -> ExecutionContext { - let context_id = ExecutionContextType::Root; - let call_ctx = setup_call_context(); - let return_data = array![1, 2, 3].span(); - - ExecutionContextTrait::new(context_id, target, call_ctx, Default::default(), return_data,) -} - fn setup_static_execution_context() -> ExecutionContext { let context_id = ExecutionContextType::Root; let call_ctx = setup_static_call_context(); @@ -261,17 +252,6 @@ fn setup_machine() -> Machine { } } -fn setup_machine_with_target(target: Address) -> Machine { - Machine { - current_ctx: BoxTrait::new(setup_execution_context_with_target(target)), - ctx_count: 1, - stack: Default::default(), - memory: Default::default(), - state: Default::default(), - error: Option::None - } -} - fn setup_static_machine() -> Machine { Machine { @@ -367,7 +347,7 @@ fn parent_ctx_return_data(ref self: Machine) -> Span { fn initialize_contract_account( eth_address: EthAddress, bytecode: Span, storage: Span<(u256, u256)> ) -> Result { - let mut ca_address = ContractAccountTrait::deploy(eth_address, 1, bytecode) + let mut ca_address = ContractAccountTrait::deploy(eth_address, bytecode) .expect('failed deploying CA'); // Set the storage of the contract account let account = Account { From 8597c6c8768c433778a819bf3ff20cf52b6b0e04 Mon Sep 17 00:00:00 2001 From: Harsh Bajpai Date: Mon, 20 Nov 2023 20:32:33 +0530 Subject: [PATCH 6/9] feat: seperate target address and the code address (#553) * feat: seperate target and code_address * fix: fix callcode caller address --------- Co-authored-by: Harsh Bajpai Co-authored-by: Harsh Bajpai --- crates/evm/src/call_helpers.cairo | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/crates/evm/src/call_helpers.cairo b/crates/evm/src/call_helpers.cairo index 956ab213a..6e1dd5611 100644 --- a/crates/evm/src/call_helpers.cairo +++ b/crates/evm/src/call_helpers.cairo @@ -22,6 +22,7 @@ use utils::traits::{BoolIntoNumeric, U256TryIntoResult}; #[derive(Drop)] struct CallArgs { caller: Address, + code_address: Address, to: Address, gas: u128, value: u256, @@ -45,10 +46,22 @@ impl MachineCallHelpersImpl of MachineCallHelpers { /// As part of the CALL family of opcodes. fn prepare_call(ref self: Machine, call_type: @CallType) -> Result { let gas = self.stack.pop_u128()?; - let to = self.stack.pop_eth_address()?; + + let code_address = self.stack.pop_eth_address()?; + let to = match call_type { + CallType::Call => code_address, + CallType::DelegateCall => self.address().evm, + CallType::CallCode => self.address().evm, + CallType::StaticCall => code_address + }; let kakarot_core = KakarotCore::unsafe_new_contract_state(); - let to = Address { evm: to, starknet: kakarot_core.compute_starknet_address(to), }; + + let code_address = Address { + evm: code_address, starknet: kakarot_core.compute_starknet_address(code_address) + }; + + let to = Address { evm: to, starknet: kakarot_core.compute_starknet_address(to) }; let (value, should_transfer) = match call_type { CallType::Call => (self.stack.pop()?, true), @@ -76,6 +89,7 @@ impl MachineCallHelpersImpl of MachineCallHelpers { Result::Ok( CallArgs { caller, + code_address, to, value, gas, @@ -113,7 +127,7 @@ impl MachineCallHelpersImpl of MachineCallHelpers { // Case 2: `to` address is not a precompile // We enter the standard flow - let bytecode = self.state.get_account(call_args.to.evm)?.code; + let bytecode = self.state.get_account(call_args.code_address.evm)?.code; let call_ctx = CallContextTrait::new( call_args.caller, From e3d6ac0c55bb8dce4dce5558be29805f922f4c0b Mon Sep 17 00:00:00 2001 From: Harsh Bajpai Date: Mon, 20 Nov 2023 21:58:08 +0530 Subject: [PATCH 7/9] feat: delegate call test (#557) Co-authored-by: Harsh Bajpai --- .../test_system_operations.cairo | 28 ++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/crates/evm/src/tests/test_instructions/test_system_operations.cairo b/crates/evm/src/tests/test_instructions/test_system_operations.cairo index ab3bb25d0..5ab56f397 100644 --- a/crates/evm/src/tests/test_instructions/test_system_operations.cairo +++ b/crates/evm/src/tests/test_instructions/test_system_operations.cairo @@ -1,6 +1,8 @@ use contracts::kakarot_core::interface::IExtendedKakarotCoreDispatcherTrait; use contracts::tests::test_data::{storage_evm_bytecode, storage_evm_initcode}; use contracts::tests::test_utils::setup_contracts_for_testing; +use core::result::ResultTrait; + use evm::call_helpers::{MachineCallHelpers, MachineCallHelpersImpl}; use evm::context::{ExecutionContext, ExecutionContextTrait, ExecutionContextType}; use evm::instructions::MemoryOperationTrait; @@ -369,7 +371,24 @@ fn test_exec_delegatecall() { // Deploy bytecode at 0x100 // ret (+ 0x1 0x1) let deployed_bytecode = array![ - 0x60, 0x01, 0x60, 0x01, 0x01, 0x60, 0x00, 0x53, 0x60, 0x20, 0x60, 0x00, 0xf3 + 0x60, + 0x01, + 0x60, + 0x01, + 0x01, + 0x60, + 0x00, + 0x53, + 0x60, + 0x42, + 0x60, + 0x42, + 0x55, + 0x60, + 0x20, + 0x60, + 0x00, + 0xf3 ] .span(); let eth_address: EthAddress = 0x100_u256.into(); @@ -383,6 +402,13 @@ fn test_exec_delegatecall() { assert(machine.error.is_none(), 'run should be success'); assert(2 == load_word(1, machine.return_data()), 'Wrong return_data'); assert(machine.stopped(), 'machine should be stopped'); + + let storage_val = machine + .state + .read_state(evm_address, 0x42) + .expect('failed reading storage slot'); + + assert(storage_val == 0x42, 'storage value is not 0x42'); } From b5644865ebd8303f5ba249ee2623c1d24618a1ae Mon Sep 17 00:00:00 2001 From: Harsh Bajpai Date: Tue, 21 Nov 2023 15:25:23 +0530 Subject: [PATCH 8/9] feat: add callcode (#559) * feat: add callcode * fix: fix wrong enum being provided to `prepare_call` --------- Co-authored-by: Harsh Bajpai --- .../src/instructions/system_operations.cairo | 6 +- .../test_system_operations.cairo | 81 +++++++++++++++++++ 2 files changed, 86 insertions(+), 1 deletion(-) diff --git a/crates/evm/src/instructions/system_operations.cairo b/crates/evm/src/instructions/system_operations.cairo index 904efd79c..8eebe842c 100644 --- a/crates/evm/src/instructions/system_operations.cairo +++ b/crates/evm/src/instructions/system_operations.cairo @@ -112,7 +112,11 @@ impl SystemOperations of SystemOperationsTrait { /// CALLCODE /// # Specification: https://www.evm.codes/#f2?fork=shanghai fn exec_callcode(ref self: Machine) -> Result<(), EVMError> { - Result::Err(EVMError::NotImplemented) + let call_args = self.prepare_call(@CallType::CallCode)?; + let read_only = self.read_only(); + + // Initialize the sub context. + self.init_call_sub_ctx(call_args, read_only) } /// DELEGATECALL diff --git a/crates/evm/src/tests/test_instructions/test_system_operations.cairo b/crates/evm/src/tests/test_instructions/test_system_operations.cairo index 5ab56f397..6d33c882b 100644 --- a/crates/evm/src/tests/test_instructions/test_system_operations.cairo +++ b/crates/evm/src/tests/test_instructions/test_system_operations.cairo @@ -331,6 +331,87 @@ fn test_exec_staticcall_no_return() { assert(machine.stopped(), 'machine should be stopped') } +#[test] +#[available_gas(4_000_000_000)] +fn test_exec_call_code() { + // Given + let mut interpreter = EVMInterpreterTrait::new(); + let (native_token, kakarot_core) = setup_contracts_for_testing(); + + let evm_address = evm_address(); + let eoa = kakarot_core.deploy_eoa(evm_address); + + // Set machine bytecode + // (call 0xffffff 0x100 0 0 0 0 1) + let bytecode = array![ + 0x60, + 0x01, + 0x60, + 0x00, + 0x60, + 0x00, + 0x60, + 0x00, + 0x60, + 0x00, + 0x60, + 0x00, + 0x61, + 0x01, + 0x00, + 0x62, + 0xff, + 0xff, + 0xff, + // CALLCODE + 0xf2, + 0x00 + ] + .span(); + let mut machine = setup_machine_with_bytecode(bytecode); + // Deploy bytecode at 0x100 + // ret (+ 0x1 0x1) + let deployed_bytecode = array![ + 0x60, + 0x01, + 0x60, + 0x01, + 0x01, + 0x60, + 0x00, + 0x53, + 0x60, + 0x42, + 0x60, + 0x42, + 0x55, + 0x60, + 0x20, + 0x60, + 0x00, + 0xf3 + ] + .span(); + let eth_address: EthAddress = 0x100_u256.into(); + initialize_contract_account(eth_address, deployed_bytecode, Default::default().span()) + .expect('set code failed'); + + // When + interpreter.run(ref machine); + + // Then + assert(machine.error.is_none(), 'run should be success'); + assert(2 == load_word(1, machine.return_data()), 'Wrong return_data'); + assert(machine.stopped(), 'machine should be stopped'); + + let storage_val = machine + .state + .read_state(evm_address, 0x42) + .expect('failed reading storage slot'); + + assert(storage_val == 0x42, 'storage value is not 0x42'); +} + #[test] #[available_gas(4_000_000_000)] From 9c481e675a93070b81e1dbcdad89af97d51dc114 Mon Sep 17 00:00:00 2001 From: Mathieu <60658558+enitrat@users.noreply.github.com> Date: Tue, 21 Nov 2023 13:55:29 +0100 Subject: [PATCH 9/9] chore: bump cairo (#548) * chore: bump cairo * restore alexandria storage * tests: remove bytearray serde test (in corelib now) * fix: deleted import --------- Co-authored-by: Elias Tazartes <66871571+Eikix@users.noreply.github.com> --- .github/workflows/gas_reports.yml | 2 +- .github/workflows/gas_snapshot.yml | 2 +- .github/workflows/test.yml | 2 +- .tool-versions | 2 +- Scarb.lock | 4 +- Scarb.toml | 4 +- crates/contracts/Scarb.toml | 2 +- crates/contracts/src/contract_account.cairo | 32 +++------ .../src/kakarot_core/interface.cairo | 1 - .../contracts/src/kakarot_core/kakarot.cairo | 1 - crates/evm/src/model/contract_account.cairo | 2 +- crates/utils/src/helpers.cairo | 9 --- crates/utils/src/tests/test_helpers.cairo | 30 -------- crates/utils/src/traits.cairo | 68 ------------------- 14 files changed, 18 insertions(+), 143 deletions(-) diff --git a/.github/workflows/gas_reports.yml b/.github/workflows/gas_reports.yml index bdc27119a..c75c68869 100644 --- a/.github/workflows/gas_reports.yml +++ b/.github/workflows/gas_reports.yml @@ -27,7 +27,7 @@ jobs: - name: Set up Scarb uses: software-mansion/setup-scarb@v1 with: - scarb-version: 2.3.1 + scarb-version: 2.4.0-rc2 - name: Run compare_snapshot script id: run-script diff --git a/.github/workflows/gas_snapshot.yml b/.github/workflows/gas_snapshot.yml index bad2c1539..4e7d49ba0 100644 --- a/.github/workflows/gas_snapshot.yml +++ b/.github/workflows/gas_snapshot.yml @@ -23,7 +23,7 @@ jobs: - name: Set up Scarb uses: software-mansion/setup-scarb@v1 with: - scarb-version: 2.3.1 + scarb-version: 2.4.0-rc2 - name: Generate gas snapshot run: python scripts/gen_snapshot.py diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 1cf9d76a1..75033b677 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -16,7 +16,7 @@ jobs: - uses: actions/checkout@v3 - uses: software-mansion/setup-scarb@v1 with: - scarb-version: 2.3.1 + scarb-version: 2.4.0-rc2 - run: scarb fmt --check - run: scarb build - run: scarb test diff --git a/.tool-versions b/.tool-versions index 697917e57..cff9618ee 100644 --- a/.tool-versions +++ b/.tool-versions @@ -1 +1 @@ -scarb 2.3.1 +scarb 2.4.0-rc2 diff --git a/Scarb.lock b/Scarb.lock index ee6e45ab2..1b118f632 100644 --- a/Scarb.lock +++ b/Scarb.lock @@ -3,8 +3,8 @@ version = 1 [[package]] name = "alexandria_storage" -version = "0.2.0" -source = "git+https://github.com/keep-starknet-strange/alexandria.git?rev=0682b1e096fe40b2bbab69863e63f2c88ca06c77#0682b1e096fe40b2bbab69863e63f2c88ca06c77" +version = "0.3.0" +source = "git+https://github.com/keep-starknet-strange/alexandria.git?rev=01a7690dc25d19a086f525b8ce66aa505c8e7527#01a7690dc25d19a086f525b8ce66aa505c8e7527" [[package]] name = "contracts" diff --git a/Scarb.toml b/Scarb.toml index aec96db9d..780661873 100644 --- a/Scarb.toml +++ b/Scarb.toml @@ -4,14 +4,14 @@ members = ["crates/*"] [workspace.package] description = "Kakarot is an (zk)-Ethereum Virtual Machine implementation written in Cairo." documentation = "https://www.kakarot.org/" -cairo-version = "2.3.1" +cairo-version = "2.4.0-rc2" version = "0.1.0" readme = "README.md" repository = "https://github.com/kkrt-labs/kakarot-ssj/" license-file = "LICENSE" [workspace.dependencies] -starknet = "2.3.1" +starknet = "2.4.0-rc2" [workspace.tool.fmt] sort-module-level-items = true diff --git a/crates/contracts/Scarb.toml b/crates/contracts/Scarb.toml index b7d38d1e4..ce42c717e 100644 --- a/crates/contracts/Scarb.toml +++ b/crates/contracts/Scarb.toml @@ -9,7 +9,7 @@ starknet.workspace = true evm = { path = "../evm" } openzeppelin = { path = "../openzeppelin" } utils = { path = "../utils" } -alexandria_storage = { git = "https://github.com/keep-starknet-strange/alexandria.git", rev = "0682b1e096fe40b2bbab69863e63f2c88ca06c77" } +alexandria_storage = { git = "https://github.com/keep-starknet-strange/alexandria.git", rev = "01a7690dc25d19a086f525b8ce66aa505c8e7527" } [tool] fmt.workspace = true diff --git a/crates/contracts/src/contract_account.cairo b/crates/contracts/src/contract_account.cairo index f5a36343b..66109351d 100644 --- a/crates/contracts/src/contract_account.cairo +++ b/crates/contracts/src/contract_account.cairo @@ -75,7 +75,7 @@ mod ContractAccount { use super::IContractAccount; use utils::helpers::{ByteArrayExTrait, ResultExTrait}; use utils::storage::{compute_storage_base_address}; - use utils::traits::{StorageBaseAddressIntoFelt252, StoreBytes31}; + use utils::traits::{StorageBaseAddressIntoFelt252}; component!(path: upgradeable_component, storage: upgradeable, event: UpgradeableEvent); @@ -88,6 +88,7 @@ mod ContractAccount { // evm_address, kakarot_core_address will be set by account/account.cairo::constructor evm_address: EthAddress, kakarot_core_address: ContractAddress, + contract_account_bytecode: List, #[substorage(v0)] upgradeable: upgradeable_component::Storage, } @@ -115,21 +116,14 @@ mod ContractAccount { // We start loading the full 31-byte words of bytecode data at address // `data_address`. The `pending_word` and `pending_word_len` are stored at // address `data_address-2` and `data_address-1` respectively. - //TODO(eni) replace with ListTrait::new() once merged in alexandria - let list_len = Store::::read(0, data_address).expect(BYTECODE_READ_ERROR); - let mut stored_list: List = List { - address_domain: 0, - base: data_address, - len: list_len, - storage_size: Store::::size() - }; + let mut stored_list: List = ListTrait::fetch(0, data_address) + .expect('failed to fetch bytecode'); let pending_word_addr: felt252 = data_address.into() - 2; let pending_word_len_addr: felt252 = pending_word_addr + 1; // Read the `ByteArray` in the contract storage. let bytecode = ByteArray { - //TODO(eni) PR alexandria to make List methods return SyscallResult - data: stored_list.array(), + data: stored_list.array().expect('failed to fetch bytecode'), pending_word: Store::< felt252 >::read(0, storage_base_address_from_felt252(pending_word_addr)) @@ -145,20 +139,10 @@ mod ContractAccount { fn set_bytecode(ref self: ContractState, bytecode: Span) { self.assert_only_kakarot_core(); let packed_bytecode: ByteArray = ByteArrayExTrait::from_bytes(bytecode); - // data_address is h(h(sn_keccak("contract_account_bytecode")), evm_address) - let data_address = storage_base_address_from_felt252( - selector!("contract_account_bytecode") - ); + let data_address = self.contract_account_bytecode.address(); // We start storing the full 31-byte words of bytecode data at address // `data_address`. The `pending_word` and `pending_word_len` are stored at // address `data_address-2` and `data_address-1` respectively. - //TODO(eni) replace with ListTrait::new() once merged in alexandria - let mut stored_list: List = List { - address_domain: 0, - base: data_address, - len: 0, - storage_size: Store::::size() - }; let pending_word_addr: felt252 = data_address.into() - 2; let pending_word_len_addr: felt252 = pending_word_addr + 1; @@ -179,8 +163,8 @@ mod ContractAccount { packed_bytecode.pending_word_len ) .expect(BYTECODE_WRITE_ERROR); - //TODO(eni) PR Alexandria so that from_span returns SyscallResult - stored_list.from_span(packed_bytecode.data.span()); + let mut stored_list: List = ListTrait::new(0, data_address); + stored_list.append_span(packed_bytecode.data.span()); } fn storage_at(self: @ContractState, key: u256) -> u256 { diff --git a/crates/contracts/src/kakarot_core/interface.cairo b/crates/contracts/src/kakarot_core/interface.cairo index 0a717ca17..9e253ba57 100644 --- a/crates/contracts/src/kakarot_core/interface.cairo +++ b/crates/contracts/src/kakarot_core/interface.cairo @@ -1,7 +1,6 @@ use contracts::kakarot_core::kakarot::StoredAccountType; use evm::model::AccountType; use starknet::{ContractAddress, EthAddress, ClassHash}; -use utils::traits::ByteArraySerde; #[starknet::interface] trait IKakarotCore { diff --git a/crates/contracts/src/kakarot_core/kakarot.cairo b/crates/contracts/src/kakarot_core/kakarot.cairo index bf91cf21e..04f4bdc08 100644 --- a/crates/contracts/src/kakarot_core/kakarot.cairo +++ b/crates/contracts/src/kakarot_core/kakarot.cairo @@ -36,7 +36,6 @@ mod KakarotCore { use super::{INVOKE_ETH_CALL_FORBIDDEN}; use super::{StoredAccountType}; use utils::helpers::{compute_starknet_address}; - use utils::traits::{ByteArraySerde}; component!(path: ownable_component, storage: ownable, event: OwnableEvent); component!(path: upgradeable_component, storage: upgradeable, event: UpgradeableEvent); diff --git a/crates/evm/src/model/contract_account.cairo b/crates/evm/src/model/contract_account.cairo index 4f8a0d2b1..b7d37eb59 100644 --- a/crates/evm/src/model/contract_account.cairo +++ b/crates/evm/src/model/contract_account.cairo @@ -33,7 +33,7 @@ use starknet::{ }; use utils::helpers::{ByteArrayExTrait, ResultExTrait}; use utils::storage::{compute_storage_base_address}; -use utils::traits::{StorageBaseAddressIntoFelt252, StoreBytes31}; +use utils::traits::{StorageBaseAddressIntoFelt252}; #[generate_trait] diff --git a/crates/utils/src/helpers.cairo b/crates/utils/src/helpers.cairo index 55d9b6aa2..ed09f5ea5 100644 --- a/crates/utils/src/helpers.cairo +++ b/crates/utils/src/helpers.cairo @@ -494,15 +494,6 @@ impl ArrayExtension> of ArrayExtTrait { } self.append(value); } - - fn append_span<+Clone>(ref self: Array, mut span: Span) { - loop { - match span.pop_front() { - Option::Some(current) => { self.append(current.clone()); }, - Option::None => { break; } - }; - } - } } #[generate_trait] diff --git a/crates/utils/src/tests/test_helpers.cairo b/crates/utils/src/tests/test_helpers.cairo index 1c48b285c..d3e80e536 100644 --- a/crates/utils/src/tests/test_helpers.cairo +++ b/crates/utils/src/tests/test_helpers.cairo @@ -3,7 +3,6 @@ use utils::helpers::{ }; use utils::helpers::{ByteArrayExTrait}; use utils::helpers; -use utils::traits::{ByteArraySerde}; #[test] #[available_gas(2000000000)] @@ -409,35 +408,6 @@ fn test_u32_bytes_used_leading_zeroes() { assert(bytes_count == 2, 'wrong bytes count'); } -#[test] -#[available_gas(2000000000)] -fn test_bytearray_deserialize() { - let mut serialized: Span = array![ - 0x03, 0xabcdef, 0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff, - ] - .span(); - - let deserialized: ByteArray = Serde::::deserialize(ref serialized).unwrap(); - - let expected = ByteArray { - data: array![ - 0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff.try_into().unwrap() - ], - pending_word_len: 3, - pending_word: 0xabcdef - }; - assert(expected.len() == deserialized.len(), 'len mismatch'); - let mut i = 0; - loop { - if i == deserialized.len() { - break; - } - - assert(expected[i] == deserialized[i], 'item mismatch'); - i += 1; - }; -} - #[test] #[available_gas(20000000)] fn test_compute_msg_hash() { diff --git a/crates/utils/src/traits.cairo b/crates/utils/src/traits.cairo index e0016c581..d0b5c59e2 100644 --- a/crates/utils/src/traits.cairo +++ b/crates/utils/src/traits.cairo @@ -129,71 +129,3 @@ impl U256TryIntoResult> of TryIntoResult { } } } - - -impl StoreBytes31 of Store { - fn read(address_domain: u32, base: StorageBaseAddress) -> SyscallResult { - Result::Ok( - Store::::read(address_domain, base)? - .try_into() - .expect('StoreBytes31 - non bytes31') - ) - } - #[inline(always)] - fn write(address_domain: u32, base: StorageBaseAddress, value: bytes31) -> SyscallResult<()> { - Store::::write(address_domain, base, value.into()) - } - #[inline(always)] - fn read_at_offset( - address_domain: u32, base: StorageBaseAddress, offset: u8 - ) -> SyscallResult { - Result::Ok( - Store::::read_at_offset(address_domain, base, offset)? - .try_into() - .expect('StoreBytes31 - non bytes31') - ) - } - #[inline(always)] - fn write_at_offset( - address_domain: u32, base: StorageBaseAddress, offset: u8, value: bytes31 - ) -> SyscallResult<()> { - Store::::write_at_offset(address_domain, base, offset, value.into()) - } - #[inline(always)] - fn size() -> u8 { - 1_u8 - } -} - -impl ByteArraySerde of Serde { - fn serialize(self: @ByteArray, ref output: Array) { - // First felt is number of bytes used in the last felt - // Second felt is the pending word - // Subsequent felts are the full 31-byte words - output.append((*self.pending_word_len).into()); - output.append((*self.pending_word).into()); - let mut i = 0; - loop { - if i == self.data.len() { - break; - } - output.append((*self.data[i]).into()); - i += 1; - }; - } - - fn deserialize(ref serialized: Span) -> Option { - let pending_word_len: u32 = (*serialized.pop_front()?).try_into()?; - let pending_word = *serialized.pop_front()?; - let mut data: Array = Default::default(); - loop { - match serialized.pop_front() { - Option::Some(val) => { data.append((*val).try_into().unwrap()); }, - Option::None => { break; } - } - }; - Option::Some( - ByteArray { data: data, pending_word: pending_word, pending_word_len: pending_word_len } - ) - } -}