Revert "feat: selfdestruct (#554)" (#555)

This reverts commit c53a713.

crates/contracts/src/contract_account.cairo

@@ -34,7 +34,6 @@ trait IContractAccount<TContractState> {
 
     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<u8>) {
+            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,32 +209,26 @@ 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")
             );
             Store::<u64>::write(0, storage_address, new_nonce).expect(NONCE_WRITE_ERROR)
         }
 
 
-        fn increment_nonce(ref self: ContractState) {
-            let storage_address: StorageBaseAddress = storage_base_address_from_felt252(
-                selector!("contract_account_nonce")
-            );
-            let nonce = Store::<u64>::read(0, storage_address).expect(NONCE_READ_ERROR);
-            Store::<u64>::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);
         }
     }
 }

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');
+        }
+    }
 }

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;

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();
 

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);
         };
 

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)
+    }
+}

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);
         }

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)
     }
 }

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