fix(levm): create to an existing address (#1523)

**Motivation**

The `CREATE` transaction logic did not handle the case where the address
where we the contract will be deploy already exists.

**Description**

- Add a check in `VM::new()` where the transaction is `CREATE`. If
`new_contract_address` exists in the db and it has nonce or bytecode, we
don't create the new account and we don't add it in the cache. We know
this is an error and the revert will be handled in `transact()`
- Something similar in `transact()`, we check if the transaction is
`CREATE` and if `new_address_acc` has code or nonce. In that case we
return a `TransactionReport` with `TxResult::Revert`.
- Support the creation of a contract even if the account has balance.

Related #1517

---------

Co-authored-by: Tomas Fabrizio Orsi <[email protected]>
Co-authored-by: Jeremías Salomón <[email protected]>
Co-authored-by: JereSalo <[email protected]>

crates/vm/levm/src/opcode_handlers/system.rs

@@ -528,14 +528,22 @@ impl VM {
         };
 
         // 3. Account has nonce or code.
-        if self.get_account(new_address).has_code_or_nonce() {
+        let new_account = self.get_account(new_address);
+        if new_account.has_code_or_nonce() {
+            self.increment_account_nonce(deployer_address)?;
             current_call_frame.stack.push(CREATE_DEPLOYMENT_FAIL)?;
             return Ok(OpcodeSuccess::Continue);
         }
 
         // THIRD: Changes to the state
         // 1. Creating contract.
-        let new_account = Account::new(value_in_wei_to_send, Bytes::new(), 1, Default::default());
+
+        // If the address has balance but there is no account associated with it, we need to add the value to it
+        let new_balance = value_in_wei_to_send
+            .checked_add(new_account.info.balance)
+            .ok_or(VMError::BalanceOverflow)?;
+
+        let new_account = Account::new(new_balance, Bytes::new(), 1, Default::default());
         cache::insert_account(&mut self.cache, new_address, new_account);
 
         // 2. Increment sender's nonce.

crates/vm/levm/src/vm.rs

@@ -209,9 +209,17 @@ impl VM {
 
                 default_touched_accounts.insert(new_contract_address);
 
-                let created_contract = Account::new(value, Bytes::new(), 1, HashMap::new());
-
-                cache::insert_account(&mut cache, new_contract_address, created_contract);
+                // Since we are in a CREATE transaction, we need to check if the address is already occupied.
+                // If it is, we should not continue with the transaction. We will handle the revert in the next step.
+                let new_account = db.get_account_info(new_contract_address);
+                let balance = value
+                    .checked_add(new_account.balance)
+                    .ok_or(VMError::BalanceOverflow)?;
+
+                if !new_account.has_code() && !new_account.has_nonce() {
+                    let created_contract = Account::new(balance, Bytes::new(), 1, HashMap::new());
+                    cache::insert_account(&mut cache, new_contract_address, created_contract);
+                }
 
                 let initial_call_frame = CallFrame::new(
                     env.origin,
@@ -616,14 +624,6 @@ impl VM {
     /// - It calculates and adds intrinsic gas to the 'gas used' of callframe and environment.
     ///   See 'docs' for more information about validations.
     fn prepare_execution(&mut self, initial_call_frame: &mut CallFrame) -> Result<(), VMError> {
-        //TODO: This should revert the transaction, not throw an error. And I don't know if it should be done here...
-        // if self.is_create() {
-        //     // If address is already in db, there's an error
-        //     let new_address_acc = self.db.get_account_info(call_frame.to);
-        //     if !new_address_acc.is_empty() {
-        //         return Err(VMError::AddressAlreadyOccupied);
-        //     }
-        // }
         let sender_address = self.env.origin;
         let sender_account = self.get_account(sender_address);
 
@@ -880,6 +880,17 @@ impl VM {
 
         self.prepare_execution(&mut initial_call_frame)?;
 
+        // The transaction should be reverted if:
+        // - The transaction is a CREATE transaction and
+        // - The address is already in the database and
+        // - The address is not empty
+        if self.is_create() {
+            let new_address_acc = self.db.get_account_info(initial_call_frame.to);
+            if new_address_acc.has_code() || new_address_acc.has_nonce() {
+                return self.handle_create_non_empty_account(&initial_call_frame);
+            }
+        }
+
         let mut report = self.execute(&mut initial_call_frame)?;
         if self.is_create() && !report.is_success() {
             remove_account(&mut self.cache, &initial_call_frame.to);
@@ -1142,6 +1153,26 @@ impl VM {
             }
         }
     }
+
+    fn handle_create_non_empty_account(
+        &mut self,
+        initial_call_frame: &CallFrame,
+    ) -> Result<TransactionReport, VMError> {
+        let mut report = TransactionReport {
+            result: TxResult::Revert(VMError::AddressAlreadyOccupied),
+            gas_used: self.env.gas_limit.low_u64(),
+            gas_refunded: 0,
+            logs: vec![],
+            new_state: self.cache.clone(),
+            output: Bytes::new(),
+            created_address: None,
+        };
+
+        self.post_execution_changes(initial_call_frame, &mut report)?;
+        report.new_state.clone_from(&self.cache);
+
+        Ok(report)
+    }
 }
 
 fn get_n_value(op: Opcode, base_opcode: Opcode) -> Result<usize, VMError> {