feat(katana-pool): invalidate declare tx if the class already exists (#…

…2564)

crates/katana/pool/src/validation/error.rs

@@ -63,4 +63,9 @@ pub enum InvalidTransactionError {
         /// The nonce that the tx is using.
         tx_nonce: Nonce,
     },
+
+    /// Error when a Declare transaction is trying to declare a class that has already been
+    /// declared.
+    #[error("Class with hash {class_hash:#x} has already been declared.")]
+    ClassAlreadyDeclared { class_hash: ClassHash },
 }

crates/katana/pool/src/validation/mod.rs

@@ -16,13 +16,24 @@ pub struct Error {
     pub error: Box<dyn std::error::Error>,
 }
 
+impl Error {
+    pub fn new(hash: TxHash, error: Box<dyn std::error::Error>) -> Self {
+        Self { hash, error }
+    }
+}
+
 pub type ValidationResult<T> = Result<ValidationOutcome<T>, Error>;
 
 /// A trait for validating transactions before they are added to the transaction pool.
 pub trait Validator {
     type Transaction: PoolTransaction;
 
     /// Validate a transaction.
+    ///
+    /// The `Err` variant of the returned `Result` should only be used to represent unexpected
+    /// errors that occurred during the validation process ie, provider
+    /// [error](katana_provider::error::ProviderError), and not for indicating that the
+    /// transaction is invalid. For that purpose, use the [`ValidationOutcome::Invalid`] enum.
     fn validate(&self, tx: Self::Transaction) -> ValidationResult<Self::Transaction>;
 
     /// Validate a batch of transactions.

crates/katana/pool/src/validation/stateful.rs

@@ -102,6 +102,19 @@ impl Validator for TxValidator {
         let tx_nonce = tx.nonce();
         let address = tx.sender();
 
+        // For declare transactions, perform a static check if there's already an existing class
+        // with the same hash.
+        if let ExecutableTx::Declare(ref declare_tx) = tx.transaction {
+            let class_hash = declare_tx.class_hash();
+            let class = this.state.class(class_hash).map_err(|e| Error::new(tx.hash, e.into()))?;
+
+            // Return an error if the class already exists.
+            if class.is_some() {
+                let error = InvalidTransactionError::ClassAlreadyDeclared { class_hash };
+                return Ok(ValidationOutcome::Invalid { tx, error });
+            }
+        }
+
         // Get the current nonce of the account from the pool or the state
         let current_nonce = if let Some(nonce) = this.pool_nonces.get(&address) {
             *nonce
@@ -125,7 +138,8 @@ impl Validator for TxValidator {
             _ => tx.nonce() == Nonce::ONE && current_nonce == Nonce::ZERO,
         };
 
-        // prepare a stateful validator and validate the transaction
+        // prepare a stateful validator and run the account validation logic (ie __validate__
+        // entrypoint)
         let result = validate(
             this.prepare(),
             tx,

crates/katana/rpc/rpc-types/src/error/starknet.rs

@@ -182,6 +182,7 @@ impl From<Box<InvalidTransactionError>> for StarknetApiError {
     fn from(error: Box<InvalidTransactionError>) -> Self {
         match error.as_ref() {
             InvalidTransactionError::InsufficientFunds { .. } => Self::InsufficientAccountBalance,
+            InvalidTransactionError::ClassAlreadyDeclared { .. } => Self::ClassAlreadyDeclared,
             InvalidTransactionError::IntrinsicFeeTooLow { .. } => Self::InsufficientMaxFee,
             InvalidTransactionError::NonAccount { .. } => Self::NonAccount,
             InvalidTransactionError::InvalidNonce { .. } => {

crates/katana/rpc/rpc/tests/starknet.rs

@@ -34,6 +34,13 @@ use tokio::sync::Mutex;
 
 mod common;
 
+/// Macro used to assert that the given error is a Starknet error.
+macro_rules! assert_starknet_err {
+    ($err:expr, $api_err:pat) => {
+        assert_matches!($err, AccountError::Provider(ProviderError::StarknetError($api_err)))
+    };
+}
+
 #[tokio::test]
 async fn declare_and_deploy_contract() -> Result<()> {
     let sequencer =
@@ -143,6 +150,41 @@ async fn declare_and_deploy_legacy_contract() -> Result<()> {
     Ok(())
 }
 
+#[tokio::test]
+async fn declaring_already_existing_class() -> Result<()> {
+    let config = get_default_test_config(SequencingConfig::default());
+    let sequencer = TestSequencer::start(config).await;
+
+    let account = sequencer.account();
+    let provider = sequencer.provider();
+
+    let path = PathBuf::from("tests/test_data/cairo1_contract.json");
+    let (contract, compiled_hash) = common::prepare_contract_declaration_params(&path)?;
+    let class_hash = contract.class_hash();
+
+    // Declare the class for the first time.
+    let res = account.declare_v2(contract.clone().into(), compiled_hash).send().await?;
+
+    // check that the tx is executed successfully and return the correct receipt
+    let _ = dojo_utils::TransactionWaiter::new(res.transaction_hash, &provider).await?;
+    // check that the class is actually declared
+    assert!(provider.get_class(BlockId::Tag(BlockTag::Pending), class_hash).await.is_ok());
+
+    // -----------------------------------------------------------------------
+    // Declaring the same class again should fail with a ClassAlreadyDeclared error
+
+    // We set max fee manually to avoid perfoming fee estimation as we just want to test that the
+    // pool validation will reject the tx.
+    //
+    // The value of the max fee is also irrelevant here, as the validator will only perform static
+    // checks and will not run the account's validation.
+
+    let result = account.declare_v2(contract.into(), compiled_hash).max_fee(Felt::ONE).send().await;
+    assert_starknet_err!(result.unwrap_err(), StarknetError::ClassAlreadyDeclared);
+
+    Ok(())
+}
+
 #[rstest::rstest]
 #[tokio::test]
 async fn deploy_account(
@@ -343,13 +385,6 @@ async fn concurrent_transactions_submissions(
     Ok(())
 }
 
-/// Macro used to assert that the given error is a Starknet error.
-macro_rules! assert_starknet_err {
-    ($err:expr, $api_err:pat) => {
-        assert_matches!($err, AccountError::Provider(ProviderError::StarknetError($api_err)))
-    };
-}
-
 #[rstest::rstest]
 #[tokio::test]
 async fn ensure_validator_have_valid_state(