fix(sdk)!: don't allow duplicate mock expectations

packages/rs-dapi-client/src/dapi_client.rs

@@ -25,10 +25,11 @@ pub enum DapiClientError<TE> {
     /// [AddressListError] errors
     #[error("address list error: {0}")]
     AddressList(AddressListError),
+
     #[cfg(feature = "mocks")]
-    /// Expectation not found
-    #[error("mock expectation not found for request: {0}")]
-    MockExpectationNotFound(String),
+    #[error("mock error: {0}")]
+    /// Error happened in mock client
+    Mock(#[from] crate::mock::MockError),
 }
 
 impl<TE: CanRetry> CanRetry for DapiClientError<TE> {
@@ -39,7 +40,7 @@ impl<TE: CanRetry> CanRetry for DapiClientError<TE> {
             Transport(transport_error, _) => transport_error.is_node_failure(),
             AddressList(_) => false,
             #[cfg(feature = "mocks")]
-            MockExpectationNotFound(_) => false,
+            Mock(_) => false,
         }
     }
 }

packages/rs-dapi-client/src/mock.rs

@@ -42,12 +42,12 @@ impl MockDapiClient {
     }
 
     /// Add a new expectation for a request
-    pub fn expect<R>(&mut self, request: &R, response: &R::Response) -> &mut Self
+    pub fn expect<R>(&mut self, request: &R, response: &R::Response) -> Result<&mut Self, MockError>
     where
         R: TransportRequest + Mockable,
         R::Response: Mockable,
     {
-        let key = self.expectations.add(request, response);
+        let key = self.expectations.add(request, response)?;
 
         tracing::trace!(
             %key,
@@ -56,7 +56,7 @@ impl MockDapiClient {
             "mock added expectation"
         );
 
-        self
+        Ok(self)
     }
 
     /// Load expectation from file.
@@ -84,7 +84,12 @@ impl MockDapiClient {
         })?;
 
         let (request, response) = data.deserialize();
-        self.expect(&request, &response);
+        self.expect(&request, &response).map_err(|e| {
+            std::io::Error::new(
+                std::io::ErrorKind::InvalidInput,
+                format!("unable to add expectation: {}", e),
+            )
+        })?;
         Ok((request, response))
     }
 }
@@ -113,10 +118,11 @@ impl DapiRequestExecutor for MockDapiClient {
         return if let Some(response) = response {
             Ok(response)
         } else {
-            Err(DapiClientError::MockExpectationNotFound(format!(
+            Err(MockError::MockExpectationNotFound(format!(
                 "unexpected mock request with key {}, use MockDapiClient::expect(): {:?}",
                 key, request
-            )))
+            ))
+            .into())
         };
     }
 }
@@ -149,12 +155,7 @@ impl Key {
 
         let mut e = sha2::Sha256::new();
         e.update(encoded);
-        let key = e.finalize().try_into().map_err(|e| {
-            std::io::Error::new(
-                std::io::ErrorKind::InvalidData,
-                format!("invalid key generated: {}", e),
-            )
-        })?;
+        let key = e.finalize().into();
 
         Ok(Self(key))
     }
@@ -175,7 +176,20 @@ impl Display for Key {
     }
 }
 
+#[derive(Debug, thiserror::Error)]
+/// Mock errors
+pub enum MockError {
+    #[error("mock expectation not found for request: {0}")]
+    /// Expectation not found
+    MockExpectationNotFound(String),
+
+    #[error("expectation already defined for request: {0}")]
+    /// Expectation already defined for request
+    MockExpectationConflict(String),
+}
+
 #[derive(Debug)]
+/// Wrapper that encapsulated serialized form of expected response for a request
 struct ExpectedResponse(Vec<u8>);
 
 impl ExpectedResponse {
@@ -199,14 +213,26 @@ struct Expectations {
 impl Expectations {
     /// Add expected request and provide given response.
     ///
-    /// If the expectation already exists, it will be silently replaced.
-    pub fn add<I: Mockable, O: Mockable>(&mut self, request: &I, response: &O) -> Key {
+    /// If the expectation already exists, error is returned.
+    pub fn add<I: Mockable + Debug, O: Mockable>(
+        &mut self,
+        request: &I,
+        response: &O,
+    ) -> Result<Key, MockError> {
         let key = Key::new(request);
         let value = ExpectedResponse::serialize(response);
 
+        if self.expectations.contains_key(&key) {
+            return Err(MockError::MockExpectationConflict(format!(
+                "expectation with key {} already defined for {} request",
+                key,
+                std::any::type_name::<I>(),
+            )));
+        }
+
         self.expectations.insert(key.clone(), value);
 
-        key
+        Ok(key)
     }
 
     /// Get the response for a given request.

packages/rs-dapi-client/tests/mock_dapi_client.rs

@@ -19,7 +19,7 @@ async fn test_mock_get_identity_dapi_client() {
         }))
     };
 
-    dapi.expect(&request, &response);
+    dapi.expect(&request, &response).expect("expectation added");
 
     let settings = RequestSettings::default();
 

packages/rs-sdk/Cargo.toml

@@ -45,6 +45,7 @@ dpp = { path = "../rs-dpp", features = [
 data-contracts = { path = "../data-contracts" }
 tokio-test = { version = "0.4.3" }
 clap = { version = "4.4.18", features = ["derive"] }
+sanitize-filename = { version = "0.5.0" }
 
 [features]
 default = ["mocks", "offline-testing"]

packages/rs-sdk/src/error.rs

@@ -30,6 +30,10 @@ pub enum Error {
     /// DAPI client error, for example, connection error
     #[error("Dapi client error: {0}")]
     DapiClientError(String),
+    #[cfg(feature = "mocks")]
+    /// DAPI mocks error
+    #[error("Dapi mocks error: {0}")]
+    DapiMocksError(#[from] rs_dapi_client::mock::MockError),
     /// Dash core error
     #[error("Dash core error: {0}")]
     CoreError(#[from] dpp::dashcore::Error),

packages/rs-sdk/src/mock/sdk.rs

@@ -8,6 +8,7 @@ use dapi_grpc::{
 };
 use dpp::version::PlatformVersion;
 use drive_proof_verifier::{error::ContextProviderError, FromProof, MockContextProvider};
+use rs_dapi_client::mock::MockError;
 use rs_dapi_client::{
     mock::{Key, MockDapiClient},
     transport::TransportRequest,
@@ -175,7 +176,7 @@ impl MockDashPlatformSdk {
             })?
             .deserialize();
 
-        self.dapi.lock().await.expect(&data.0, &data.1);
+        self.dapi.lock().await.expect(&data.0, &data.1)?;
         Ok(())
     }
 
@@ -195,8 +196,8 @@ impl MockDashPlatformSdk {
     ///
     /// ## Returns
     ///
-    /// * Some(O): If the object is expected to exist.
-    /// * None: If the object is expected to not exist.
+    /// * Reference to self on success, to allow chaining
+    /// * Error when expectation cannot be set or is already defined for this request
     ///
     /// ## Panics
     ///
@@ -217,7 +218,7 @@ impl MockDashPlatformSdk {
     ///     // Define query that will be sent
     ///     let query = expected.id();
     ///     // Expect that in response to `query`, `expected` will be returned
-    ///     api.mock().expect_fetch(query, Some(expected.clone())).await;
+    ///     api.mock().expect_fetch(query, Some(expected.clone())).await.unwrap();
     ///
     ///     // Fetch the identity
     ///     let retrieved = dpp::prelude::Identity::fetch(&api, query)
@@ -233,14 +234,14 @@ impl MockDashPlatformSdk {
         &mut self,
         query: Q,
         object: Option<O>,
-    ) -> &mut Self
+    ) -> Result<&mut Self, Error>
     where
         <<O as Fetch>::Request as TransportRequest>::Response: Default,
     {
         let grpc_request = query.query(self.prove).expect("query must be correct");
-        self.expect(grpc_request, object).await;
+        self.expect(grpc_request, object).await?;
 
-        self
+        Ok(self)
     }
 
     /// Expect a [FetchMany] request and return provided object.
@@ -260,8 +261,8 @@ impl MockDashPlatformSdk {
     ///
     /// ## Returns
     ///
-    /// * `Some(Vec<O>)`: If the objects are expected to exist.
-    /// * `None`: If the objects are expected to not exist.
+    /// * Reference to self on success, to allow chaining
+    /// * Error when expectation cannot be set or is already defined for this request
     ///
     /// ## Panics
     ///
@@ -280,7 +281,7 @@ impl MockDashPlatformSdk {
         &mut self,
         query: Q,
         objects: Option<BTreeMap<K, Option<O>>>,
-    ) -> &mut Self
+    ) -> Result<&mut Self, Error>
     where
         BTreeMap<K, Option<O>>: MockResponse,
         <<O as FetchMany<K>>::Request as TransportRequest>::Response: Default,
@@ -291,59 +292,43 @@ impl MockDashPlatformSdk {
             > + Sync,
     {
         let grpc_request = query.query(self.prove).expect("query must be correct");
-        self.expect(grpc_request, objects).await;
+        self.expect(grpc_request, objects).await?;
 
-        self
+        Ok(self)
     }
 
     /// Save expectations for a request.
     async fn expect<I: TransportRequest, O: MockResponse>(
         &mut self,
         grpc_request: I,
         returned_object: Option<O>,
-    ) where
+    ) -> Result<(), Error>
+    where
         I::Response: Default,
     {
         let key = Key::new(&grpc_request);
 
+        // detect duplicates
+        if self.from_proof_expectations.contains_key(&key) {
+            return Err(MockError::MockExpectationConflict(format!(
+                "proof expectation key {} already defined for {} request: {:?}",
+                key,
+                std::any::type_name::<I>(),
+                grpc_request
+            ))
+            .into());
+        }
+
         // This expectation will work for from_proof
         self.from_proof_expectations
             .insert(key, returned_object.mock_serialize(self));
 
         // This expectation will work for execute
         let mut dapi_guard = self.dapi.lock().await;
-        // We don't really care about the response, as it will be mocked by from_proof
-        dapi_guard.expect(&grpc_request, &Default::default());
-    }
+        // We don't really care about the response, as it will be mocked by from_proof, so we provide default()
+        dapi_guard.expect(&grpc_request, &Default::default())?;
 
-    /// Wrapper around [FromProof] that uses mock expectations instead of executing [FromProof] trait.
-    #[allow(dead_code)]
-    #[deprecated(note = "This function is marked as unused.")]
-    pub(crate) fn parse_proof<I, O: FromProof<I>>(
-        &self,
-        request: O::Request,
-        response: O::Response,
-    ) -> Result<Option<O>, drive_proof_verifier::Error>
-    where
-        O::Request: Mockable,
-        Option<O>: MockResponse,
-        // O: FromProof<<O as FromProof<I>>::Request>,
-    {
-        let key = Key::new(&request);
-
-        let data = match self.from_proof_expectations.get(&key) {
-            Some(d) => Option::<O>::mock_deserialize(self, d),
-            None => {
-                let version = self.version();
-                let provider = self.quorum_provider.as_ref()
-                    .ok_or(ContextProviderError::InvalidQuorum(
-                     "expectation not found and quorum info provider not initialized with sdk.mock().quorum_info_dir()".to_string()
-                    ))?;
-                O::maybe_from_proof(request, response, version, provider)?
-            }
-        };
-
-        Ok(data)
+        Ok(())
     }
 
     /// Wrapper around [FromProof] that uses mock expectations instead of executing [FromProof] trait.

packages/rs-sdk/tests/fetch/broadcast.rs

@@ -19,7 +19,7 @@ mod online {
         const TIMEOUT: std::time::Duration = std::time::Duration::from_millis(400);
 
         let cfg = Config::new();
-        let sdk = cfg.setup_api().await;
+        let sdk = cfg.setup_api("test_wait_timeout").await;
         let sdk_ref: &Sdk = sdk.as_ref();
 
         let request: WaitForStateTransitionResultRequest = WaitForStateTransitionResultRequestV0 {