feat(drive-abci)!: remove unnecessary validation from check tx and re-check (#1647)

Co-authored-by: Ivan Shumkov <ivan@shumkov.ru>

Author: QuantumExplorer

packages/rs-dpp/src/identity/state_transition/mod.rs

@@ -16,3 +16,11 @@ pub trait AssetLockProved {
     /// Get asset lock proof
     fn asset_lock_proof(&self) -> &AssetLockProof;
 }
+
+/// Only state transitions funded with Asset Lock Transactions have Asset Lock Proofs
+pub trait OptionallyAssetLockProved {
+    /// Returns asset lock proof if state transition implements it
+    fn optional_asset_lock_proof(&self) -> Option<&AssetLockProof> {
+        None
+    }
+}

packages/rs-dpp/src/state_transition/mod.rs

@@ -36,7 +36,9 @@ use crate::consensus::ConsensusError;
 
 use crate::identity::identity_public_key::accessors::v0::IdentityPublicKeyGettersV0;
 use crate::identity::signer::Signer;
+use crate::identity::state_transition::OptionallyAssetLockProved;
 use crate::identity::{IdentityPublicKey, KeyID, KeyType, Purpose, SecurityLevel};
+use crate::prelude::AssetLockProof;
 pub use state_transitions::*;
 
 use crate::serialization::Signable;
@@ -243,16 +245,13 @@ pub enum StateTransition {
     IdentityCreditTransfer(IdentityCreditTransferTransition),
 }
 
-impl StateTransition {
-    /// This means we should transform into the action before validation of the structure
-    pub fn requires_state_to_validate_structure(&self) -> bool {
-        matches!(self, StateTransition::DocumentsBatch(_))
-    }
-    /// This means we should transform into the action before validation of the identity and signatures
-    pub fn requires_state_to_validate_identity_and_signatures(&self) -> bool {
-        matches!(self, StateTransition::DocumentsBatch(_))
+impl OptionallyAssetLockProved for StateTransition {
+    fn optional_asset_lock_proof(&self) -> Option<&AssetLockProof> {
+        call_method!(self, optional_asset_lock_proof)
     }
+}
 
+impl StateTransition {
     pub fn is_identity_signed(&self) -> bool {
         !matches!(
             self,

packages/rs-dpp/src/state_transition/state_transitions/contract/data_contract_create_transition/mod.rs

@@ -27,6 +27,7 @@ use platform_versioning::PlatformVersioned;
 use serde::{Deserialize, Serialize};
 
 use crate::data_contract::created_data_contract::CreatedDataContract;
+use crate::identity::state_transition::OptionallyAssetLockProved;
 pub use v0::*;
 
 pub type DataContractCreateTransitionLatest = DataContractCreateTransitionV0;
@@ -146,6 +147,8 @@ impl DataContractCreateTransition {
     }
 }
 
+impl OptionallyAssetLockProved for DataContractCreateTransition {}
+
 #[cfg(test)]
 mod test {
     use crate::data_contract::conversion::json::DataContractJsonConversionMethodsV0;
@@ -155,7 +158,6 @@ mod test {
     use crate::data_contract::accessors::v0::DataContractV0Getters;
     use crate::data_contract::conversion::value::v0::DataContractValueConversionMethodsV0;
     use crate::state_transition::data_contract_create_transition::accessors::DataContractCreateTransitionAccessorsV0;
-    use crate::state_transition::state_transitions::common_fields::property_names;
     use crate::state_transition::traits::StateTransitionLike;
     use crate::state_transition::{StateTransitionType, StateTransitionValueConvert};
     use crate::tests::fixtures::get_data_contract_fixture;
@@ -185,7 +187,7 @@ mod test {
                     ),
                 ),
                 (
-                    property_names::ENTROPY,
+                    ENTROPY,
                     Value::Bytes32(created_data_contract.entropy_used().to_buffer()),
                 ),
                 (

packages/rs-dpp/src/state_transition/state_transitions/contract/data_contract_update_transition/mod.rs

@@ -27,6 +27,7 @@ use platform_version::{TryFromPlatformVersioned, TryIntoPlatformVersioned};
 
 use crate::data_contract::DataContract;
 
+use crate::identity::state_transition::OptionallyAssetLockProved;
 pub use v0::*;
 
 pub type DataContractUpdateTransitionLatest = DataContractUpdateTransitionV0;
@@ -99,6 +100,8 @@ impl StateTransitionFieldTypes for DataContractUpdateTransition {
     }
 }
 
+impl OptionallyAssetLockProved for DataContractUpdateTransition {}
+
 #[cfg(test)]
 mod test {
     use crate::data_contract::DataContract;

packages/rs-dpp/src/state_transition/state_transitions/document/documents_batch_transition/mod.rs

@@ -41,6 +41,7 @@ use crate::state_transition::data_contract_update_transition::{
 
 use crate::state_transition::documents_batch_transition::fields::property_names;
 
+use crate::identity::state_transition::OptionallyAssetLockProved;
 pub use v0::*;
 
 #[derive(
@@ -593,3 +594,4 @@ pub fn get_security_level_requirement(v: &Value, default: SecurityLevel) -> Secu
 //         assert_eq!(hex::encode(expected_bytes), hex::encode(bytes));
 //     }
 // }
+impl OptionallyAssetLockProved for DocumentsBatchTransition {}

packages/rs-dpp/src/state_transition/state_transitions/identity/identity_create_transition/proved.rs

@@ -1,8 +1,14 @@
-use crate::identity::state_transition::AssetLockProved;
+use crate::identity::state_transition::{AssetLockProved, OptionallyAssetLockProved};
 use crate::prelude::AssetLockProof;
 use crate::state_transition::identity_create_transition::IdentityCreateTransition;
 use crate::ProtocolError;
 
+impl OptionallyAssetLockProved for IdentityCreateTransition {
+    fn optional_asset_lock_proof(&self) -> Option<&AssetLockProof> {
+        Some(self.asset_lock_proof())
+    }
+}
+
 impl AssetLockProved for IdentityCreateTransition {
     fn set_asset_lock_proof(
         &mut self,

packages/rs-dpp/src/state_transition/state_transitions/identity/identity_credit_transfer_transition/mod.rs

@@ -15,6 +15,7 @@ use crate::state_transition::identity_credit_transfer_transition::v0::IdentityCr
 use crate::state_transition::identity_credit_transfer_transition::v0::IdentityCreditTransferTransitionV0Signable;
 use crate::state_transition::StateTransitionFieldTypes;
 
+use crate::identity::state_transition::OptionallyAssetLockProved;
 use crate::ProtocolError;
 use bincode::{Decode, Encode};
 use derive_more::From;
@@ -71,6 +72,8 @@ impl IdentityCreditTransferTransition {
     }
 }
 
+impl OptionallyAssetLockProved for IdentityCreditTransferTransition {}
+
 impl StateTransitionFieldTypes for IdentityCreditTransferTransition {
     fn signature_property_paths() -> Vec<&'static str> {
         vec![SIGNATURE]

packages/rs-dpp/src/state_transition/state_transitions/identity/identity_credit_withdrawal_transition/mod.rs

@@ -15,6 +15,7 @@ mod version;
 use crate::state_transition::identity_credit_withdrawal_transition::v0::IdentityCreditWithdrawalTransitionV0Signable;
 use crate::state_transition::StateTransitionFieldTypes;
 
+use crate::identity::state_transition::OptionallyAssetLockProved;
 use crate::ProtocolError;
 use bincode::{Decode, Encode};
 use data_contracts::withdrawals_contract::document_types::withdrawal::properties::OUTPUT_SCRIPT;
@@ -65,3 +66,5 @@ impl StateTransitionFieldTypes for IdentityCreditWithdrawalTransition {
         vec![SIGNATURE, OUTPUT_SCRIPT]
     }
 }
+
+impl OptionallyAssetLockProved for IdentityCreditWithdrawalTransition {}

packages/rs-dpp/src/state_transition/state_transitions/identity/identity_topup_transition/proved.rs

@@ -1,8 +1,14 @@
-use crate::identity::state_transition::AssetLockProved;
+use crate::identity::state_transition::{AssetLockProved, OptionallyAssetLockProved};
 use crate::prelude::AssetLockProof;
 use crate::state_transition::identity_topup_transition::IdentityTopUpTransition;
 use crate::ProtocolError;
 
+impl OptionallyAssetLockProved for IdentityTopUpTransition {
+    fn optional_asset_lock_proof(&self) -> Option<&AssetLockProof> {
+        Some(self.asset_lock_proof())
+    }
+}
+
 impl AssetLockProved for IdentityTopUpTransition {
     fn set_asset_lock_proof(
         &mut self,

packages/rs-dpp/src/state_transition/state_transitions/identity/identity_update_transition/mod.rs

@@ -17,6 +17,7 @@ use crate::state_transition::identity_update_transition::v0::IdentityUpdateTrans
 use crate::state_transition::StateTransitionFieldTypes;
 use fields::*;
 
+use crate::identity::state_transition::OptionallyAssetLockProved;
 use crate::ProtocolError;
 use bincode::{Decode, Encode};
 use derive_more::From;
@@ -70,6 +71,8 @@ impl IdentityUpdateTransition {
     }
 }
 
+impl OptionallyAssetLockProved for IdentityUpdateTransition {}
+
 impl StateTransitionFieldTypes for IdentityUpdateTransition {
     fn binary_property_paths() -> Vec<&'static str> {
         vec![SIGNATURE, ADD_PUBLIC_KEYS_SIGNATURE]

packages/rs-drive-abci/src/abci/handler/mod.rs

@@ -753,8 +753,8 @@ where
     ) -> Result<ResponseCheckTx, proto::ResponseException> {
         let _timer = crate::metrics::abci_request_duration("check_tx");
 
-        let RequestCheckTx { tx, .. } = request;
-        match self.platform.check_tx(tx.as_slice()) {
+        let RequestCheckTx { tx, r#type } = request;
+        match self.platform.check_tx(tx.as_slice(), r#type.try_into()?) {
             Ok(validation_result) => {
                 let platform_state = self.platform.state.read().unwrap();
                 let platform_version = platform_state.current_platform_version()?;
@@ -774,7 +774,11 @@ where
 
                 let gas_wanted = validation_result
                     .data
-                    .map(|fee_result| fee_result.total_base_fee())
+                    .map(|fee_result| {
+                        fee_result
+                            .map(|fee_result| fee_result.total_base_fee())
+                            .unwrap_or_default()
+                    })
                     .unwrap_or_default();
 
                 Ok(ResponseCheckTx {

packages/rs-drive-abci/src/execution/check_tx/mod.rs

@@ -2,13 +2,52 @@ use crate::error::execution::ExecutionError;
 use crate::error::Error;
 use crate::platform_types::platform::Platform;
 
+use crate::abci::AbciError;
 use crate::rpc::core::CoreRPCLike;
 use dpp::consensus::ConsensusError;
 use dpp::fee::fee_result::FeeResult;
 use dpp::validation::ValidationResult;
 
 mod v0;
 
+// @append_only
+#[repr(u8)]
+#[derive(Copy, Clone, Debug)]
+pub enum CheckTxLevel {
+    FirstTimeCheck,
+    Recheck,
+}
+
+impl TryFrom<u8> for CheckTxLevel {
+    type Error = Error;
+
+    fn try_from(value: u8) -> Result<Self, Self::Error> {
+        match value {
+            0 => Ok(CheckTxLevel::FirstTimeCheck),
+            1 => Ok(CheckTxLevel::Recheck),
+            value => Err(Error::Abci(AbciError::BadRequest(format!(
+                "Invalid value for CheckTxLevel {}",
+                value
+            )))),
+        }
+    }
+}
+
+impl TryFrom<i32> for CheckTxLevel {
+    type Error = Error;
+
+    fn try_from(value: i32) -> Result<Self, Self::Error> {
+        match value {
+            0 => Ok(CheckTxLevel::FirstTimeCheck),
+            1 => Ok(CheckTxLevel::Recheck),
+            value => Err(Error::Abci(AbciError::BadRequest(format!(
+                "Invalid value for CheckTxLevel {}",
+                value
+            )))),
+        }
+    }
+}
+
 impl<C> Platform<C>
 where
     C: CoreRPCLike,
@@ -30,11 +69,12 @@ where
     pub fn check_tx(
         &self,
         raw_tx: &[u8],
-    ) -> Result<ValidationResult<FeeResult, ConsensusError>, Error> {
+        check_tx_level: CheckTxLevel,
+    ) -> Result<ValidationResult<Option<FeeResult>, ConsensusError>, Error> {
         let state = self.state.read().expect("expected to get state");
         let platform_version = state.current_platform_version()?;
         match platform_version.drive_abci.methods.engine.check_tx {
-            0 => self.check_tx_v0(raw_tx),
+            0 => self.check_tx_v0(raw_tx, check_tx_level),
             version => Err(Error::Execution(ExecutionError::UnknownVersionMismatch {
                 method: "check_tx".to_string(),
                 known_versions: vec![0],