Adjust nonce validity with blockifier (#839)

* update states to return zero if contract isnt deployed

* remove now meaningless test

* move handle nonce to before apply

* remove nonce init in deploy_contract metthod

Author: SantiagoPittella

src/state/cached_state.rs

@@ -236,9 +236,6 @@ impl<T: StateReader> State for CachedState<T> {
         self.cache
             .class_hash_writes
             .insert(deploy_contract_address.clone(), class_hash);
-        self.cache
-            .nonce_writes_mut()
-            .insert(deploy_contract_address, Felt252::zero());
         Ok(())
     }
 
@@ -347,10 +344,11 @@ impl<T: StateReader> State for CachedState<T> {
                 .nonce_initial_values
                 .insert(contract_address.clone(), nonce);
         }
-        self.cache
+        Ok(self
+            .cache
             .get_nonce(contract_address)
-            .ok_or_else(|| StateError::NoneNonce(contract_address.clone()))
-            .cloned()
+            .unwrap_or(&Felt252::zero())
+            .clone())
     }
 
     fn get_storage_at(&mut self, storage_entry: &StorageEntry) -> Result<Felt252, StateError> {

src/state/in_memory_state_reader.rs

@@ -12,6 +12,7 @@ use crate::{
 };
 use cairo_vm::felt::Felt252;
 use getset::{Getters, MutGetters};
+use num_traits::Zero;
 use std::collections::HashMap;
 
 /// A [StateReader] that holds all the data in memory.
@@ -98,11 +99,12 @@ impl StateReader for InMemoryStateReader {
     }
 
     fn get_nonce_at(&self, contract_address: &Address) -> Result<Felt252, StateError> {
+        let default = Felt252::zero();
         let nonce = self
             .address_to_nonce
             .get(contract_address)
-            .ok_or_else(|| StateError::NoneContractState(contract_address.clone()));
-        nonce.cloned()
+            .unwrap_or(&default);
+        Ok(nonce.clone())
     }
 
     fn get_storage_at(&self, storage_entry: &StorageEntry) -> Result<Felt252, StateError> {

src/state/state_api.rs

@@ -67,6 +67,7 @@ pub trait State {
 
     fn get_class_hash_at(&mut self, contract_address: &Address) -> Result<ClassHash, StateError>;
 
+    /// Default: 0 for an uninitialized contract address.
     fn get_nonce_at(&mut self, contract_address: &Address) -> Result<Felt252, StateError>;
 
     fn get_storage_at(&mut self, storage_entry: &StorageEntry) -> Result<Felt252, StateError>;

src/testing/state.rs

@@ -324,7 +324,7 @@ mod tests {
 
     use super::*;
     use crate::{
-        core::{contract_address::compute_deprecated_class_hash, errors::state_errors::StateError},
+        core::contract_address::compute_deprecated_class_hash,
         definitions::{
             constants::CONSTRUCTOR_ENTRY_POINT_SELECTOR, transaction_type::TransactionType,
         },
@@ -719,17 +719,4 @@ mod tests {
         let err = starknet_state.consume_message_hash(msg_hash).unwrap_err();
         assert_matches!(err, StarknetStateError::InvalidMessageHash);
     }
-
-    #[test]
-    fn test_create_invoke_function_should_fail_with_none_contract_state() {
-        let mut starknet_state = StarknetState::new(None);
-
-        let err = starknet_state
-            .create_invoke_function(Address(0.into()), 0.into(), vec![], 0, None, None, None)
-            .unwrap_err();
-        assert_matches!(
-            err,
-            TransactionError::State(StateError::NoneContractState(_))
-        );
-    }
 }

src/transaction/declare.rs

@@ -267,8 +267,8 @@ impl Declare {
         state: &mut CachedState<S>,
         block_context: &BlockContext,
     ) -> Result<TransactionExecutionInfo, TransactionError> {
-        let mut tx_exec_info = self.apply(state, block_context)?;
         self.handle_nonce(state)?;
+        let mut tx_exec_info = self.apply(state, block_context)?;
 
         let mut tx_execution_context =
             self.get_execution_context(block_context.invoke_tx_max_n_steps);

src/transaction/deploy_account.rs

@@ -155,9 +155,8 @@ impl DeployAccount {
         state: &mut CachedState<S>,
         block_context: &BlockContext,
     ) -> Result<TransactionExecutionInfo, TransactionError> {
-        let mut tx_info = self.apply(state, block_context)?;
-
         self.handle_nonce(state)?;
+        let mut tx_info = self.apply(state, block_context)?;
 
         let mut tx_execution_context =
             self.get_execution_context(block_context.invoke_tx_max_n_steps);
@@ -256,16 +255,15 @@ impl DeployAccount {
             return Ok(());
         }
 
+        // In blockifier, get_nonce_at returns zero if no entry is found.
         let current_nonce = state.get_nonce_at(&self.contract_address)?;
         if current_nonce != self.nonce {
             return Err(TransactionError::InvalidTransactionNonce(
                 current_nonce.to_string(),
                 self.nonce.to_string(),
             ));
         }
-
         state.increment_nonce(&self.contract_address)?;
-
         Ok(())
     }
 

src/transaction/invoke_function.rs

@@ -274,8 +274,8 @@ impl InvokeFunction {
         block_context: &BlockContext,
         remaining_gas: u128,
     ) -> Result<TransactionExecutionInfo, TransactionError> {
-        let mut tx_exec_info = self.apply(state, block_context, remaining_gas)?;
         self.handle_nonce(state)?;
+        let mut tx_exec_info = self.apply(state, block_context, remaining_gas)?;
 
         let mut tx_execution_context =
             self.get_execution_context(block_context.invoke_tx_max_n_steps)?;

tests/internals.rs

@@ -1251,12 +1251,6 @@ fn test_deploy_account() {
 
     assert_eq!(state, state_before);
 
-    // Statement **not** in blockifier.
-    state.cache_mut().nonce_initial_values_mut().insert(
-        deploy_account_tx.contract_address().clone(),
-        Felt252::zero(),
-    );
-
     let tx_info = deploy_account_tx
         .execute(&mut state, &block_context)
         .unwrap();