fix(sdk)!: fetch missing quorum keys instead of banning every node

packages/rs-context-provider/src/error.rs

@@ -21,6 +21,13 @@ pub enum ContextProviderError {
     #[error("invalid quorum: {0}")]
     InvalidQuorum(String),
 
+    /// The provider's trusted quorum source gave no answer about a quorum: it
+    /// was unreachable, answered only in part, or was asked too recently to be
+    /// asked again, or returned an unusable key. Says nothing about whether
+    /// the quorum exists.
+    #[error("quorum source unavailable: {0}")]
+    QuorumSourceUnavailable(String),
+
     /// Core Fork Error
     #[error("activation fork error: {0}")]
     ActivationForkError(String),

packages/rs-context-provider/src/lib.rs

@@ -7,7 +7,7 @@ pub mod error;
 pub mod provider;
 
 pub use error::ContextProviderError;
-pub use provider::{ContextProvider, DataContractProvider};
+pub use provider::{ContextProvider, DataContractProvider, QuorumKeyFuture};
 
 #[cfg(feature = "mocks")]
 pub use provider::MockContextProvider;

packages/rs-context-provider/src/provider.rs

@@ -3,14 +3,22 @@ use dpp::data_contract::TokenConfiguration;
 use dpp::prelude::{CoreBlockHeight, DataContract, Identifier};
 use dpp::version::PlatformVersion;
 use drive::{error::proof::ProofError, query::ContractLookupFn};
-use std::{ops::Deref, sync::Arc};
+use std::{future::Future, ops::Deref, pin::Pin, sync::Arc};
 
 #[cfg(feature = "mocks")]
 use {
     dpp::data_contract::serialized_version::DataContractInSerializationFormat, hex::ToHex,
     std::io::ErrorKind,
 };
 
+/// Future returned by [ContextProvider::fetch_quorum_public_key].
+///
+/// `Send` on every target because the SDK's request futures are `Send`
+/// everywhere, and `'static` so that wrappers such as `Mutex<T>` can forward
+/// the call without holding a guard across an await.
+pub type QuorumKeyFuture =
+    Pin<Box<dyn Future<Output = Result<Option<[u8; 48]>, ContextProviderError>> + Send + 'static>>;
+
 /// Interface between the Sdk and state of the application.
 ///
 /// ContextProvider is called by the [FromProof](crate::FromProof) trait (and similar) to get information about
@@ -91,6 +99,39 @@ pub trait ContextProvider: Send + Sync {
         core_chain_locked_height: u32,
     ) -> Result<[u8; 48], ContextProviderError>; // public key is 48 bytes
 
+    /// Fetches the public key of a quorum this provider does not hold yet.
+    ///
+    /// The SDK calls this from async code when proof verification failed
+    /// because [get_quorum_public_key](Self::get_quorum_public_key) had no key
+    /// for the quorum, and verifies the same response again when a key comes
+    /// back. This lets a provider that caches quorum keys, and cannot block
+    /// inside the synchronous lookup, fetch a key that is newer than its cache.
+    ///
+    /// The arguments come from a response that has not been verified yet. The
+    /// key must come from the provider's own trusted source, never from the
+    /// arguments, and the work done must stay bounded whatever they are.
+    ///
+    /// # Returns
+    ///
+    /// * `None`: an asynchronous fetch would not help, for example because the
+    ///   provider cannot fetch keys or its synchronous lookup already fetched.
+    ///   This is the default.
+    /// * `Some(future)`, resolving to:
+    ///   * `Ok(Some(key))`: the key, which `get_quorum_public_key` now returns
+    ///     as well.
+    ///   * `Ok(None)`: the trusted source, asked after this call was made,
+    ///     answered and does not know this quorum.
+    ///   * `Err(_)`: the provider could not find out, for example because its
+    ///     trusted source was unreachable.
+    fn fetch_quorum_public_key(
+        &self,
+        _quorum_type: u32,
+        _quorum_hash: [u8; 32],
+        _core_chain_locked_height: u32,
+    ) -> Option<QuorumKeyFuture> {
+        None
+    }
+
     /// Gets the platform activation height from core. Once this has happened this can be hardcoded.
     ///
     /// # Returns
@@ -130,6 +171,16 @@ impl<C: AsRef<dyn ContextProvider> + Send + Sync> ContextProvider for C {
             .get_quorum_public_key(quorum_type, quorum_hash, core_chain_locked_height)
     }
 
+    fn fetch_quorum_public_key(
+        &self,
+        quorum_type: u32,
+        quorum_hash: [u8; 32],
+        core_chain_locked_height: u32,
+    ) -> Option<QuorumKeyFuture> {
+        self.as_ref()
+            .fetch_quorum_public_key(quorum_type, quorum_hash, core_chain_locked_height)
+    }
+
     fn get_platform_activation_height(&self) -> Result<CoreBlockHeight, ContextProviderError> {
         self.as_ref().get_platform_activation_height()
     }
@@ -171,6 +222,17 @@ where
         lock.get_quorum_public_key(quorum_type, quorum_hash, core_chain_locked_height)
     }
 
+    fn fetch_quorum_public_key(
+        &self,
+        quorum_type: u32,
+        quorum_hash: [u8; 32],
+        core_chain_locked_height: u32,
+    ) -> Option<QuorumKeyFuture> {
+        // The future is 'static, so the guard is released before it is awaited.
+        let lock = self.lock().expect("lock poisoned");
+        lock.fetch_quorum_public_key(quorum_type, quorum_hash, core_chain_locked_height)
+    }
+
     fn get_platform_activation_height(&self) -> Result<CoreBlockHeight, ContextProviderError> {
         let lock = self.lock().expect("lock poisoned");
         lock.get_platform_activation_height()
@@ -353,3 +415,129 @@ impl<'a, T: ContextProvider + 'a> AsRef<dyn ContextProvider + 'a> for Arc<T> {
         self.deref()
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use std::sync::atomic::{AtomicUsize, Ordering};
+    use std::sync::Mutex;
+
+    /// Provider whose `fetch_quorum_public_key` counts its calls and records
+    /// the quorum it was asked for.
+    #[derive(Default)]
+    struct FetchingProvider {
+        calls: AtomicUsize,
+        asked: Mutex<Option<(u32, [u8; 32], u32)>>,
+    }
+
+    impl ContextProvider for FetchingProvider {
+        fn get_data_contract(
+            &self,
+            _id: &Identifier,
+            _platform_version: &PlatformVersion,
+        ) -> Result<Option<Arc<DataContract>>, ContextProviderError> {
+            Ok(None)
+        }
+
+        fn get_token_configuration(
+            &self,
+            _token_id: &Identifier,
+        ) -> Result<Option<TokenConfiguration>, ContextProviderError> {
+            Ok(None)
+        }
+
+        fn get_quorum_public_key(
+            &self,
+            _quorum_type: u32,
+            _quorum_hash: [u8; 32],
+            _core_chain_locked_height: u32,
+        ) -> Result<[u8; 48], ContextProviderError> {
+            Err(ContextProviderError::InvalidQuorum(
+                "not cached".to_string(),
+            ))
+        }
+
+        fn fetch_quorum_public_key(
+            &self,
+            quorum_type: u32,
+            quorum_hash: [u8; 32],
+            core_chain_locked_height: u32,
+        ) -> Option<QuorumKeyFuture> {
+            self.calls.fetch_add(1, Ordering::SeqCst);
+            *self.asked.lock().unwrap() =
+                Some((quorum_type, quorum_hash, core_chain_locked_height));
+            Some(Box::pin(async { Ok(Some([7u8; 48])) }))
+        }
+
+        fn get_platform_activation_height(&self) -> Result<CoreBlockHeight, ContextProviderError> {
+            Ok(1)
+        }
+    }
+
+    /// The SDK reaches its provider through `Arc`, `Box<dyn ContextProvider>`
+    /// and `Mutex` wrappers. A wrapper that fell back to the default `None`
+    /// would silently turn off fetching of keys newer than the provider's
+    /// cache, so every wrapper must reach the inner provider with the same
+    /// arguments.
+    #[test]
+    fn should_forward_quorum_key_fetches_through_every_wrapper() {
+        fn assert_forwards(provider: &dyn ContextProvider, inner: &FetchingProvider, call: usize) {
+            assert!(provider
+                .fetch_quorum_public_key(6, [0xab; 32], 42)
+                .is_some());
+            assert_eq!(inner.calls.load(Ordering::SeqCst), call);
+            assert_eq!(*inner.asked.lock().unwrap(), Some((6, [0xab; 32], 42)));
+        }
+
+        let inner = Arc::new(FetchingProvider::default());
+
+        let arc: Arc<FetchingProvider> = Arc::clone(&inner);
+        assert_forwards(&arc, &inner, 1);
+
+        let boxed: Arc<Box<dyn ContextProvider>> =
+            Arc::new(Box::new(Arc::clone(&inner)) as Box<dyn ContextProvider>);
+        assert_forwards(&boxed, &inner, 2);
+
+        let mutex = std::sync::Mutex::new(Arc::clone(&inner));
+        assert_forwards(&mutex, &inner, 3);
+    }
+
+    /// Providers that do not override the method keep their behaviour: the
+    /// SDK sees `None` and reports the original verification error.
+    #[test]
+    fn should_not_fetch_quorum_keys_by_default() {
+        struct CacheOnly;
+        impl ContextProvider for CacheOnly {
+            fn get_data_contract(
+                &self,
+                _id: &Identifier,
+                _platform_version: &PlatformVersion,
+            ) -> Result<Option<Arc<DataContract>>, ContextProviderError> {
+                Ok(None)
+            }
+            fn get_token_configuration(
+                &self,
+                _token_id: &Identifier,
+            ) -> Result<Option<TokenConfiguration>, ContextProviderError> {
+                Ok(None)
+            }
+            fn get_quorum_public_key(
+                &self,
+                _quorum_type: u32,
+                _quorum_hash: [u8; 32],
+                _core_chain_locked_height: u32,
+            ) -> Result<[u8; 48], ContextProviderError> {
+                Err(ContextProviderError::InvalidQuorum(
+                    "not cached".to_string(),
+                ))
+            }
+            fn get_platform_activation_height(
+                &self,
+            ) -> Result<CoreBlockHeight, ContextProviderError> {
+                Ok(1)
+            }
+        }
+
+        assert!(CacheOnly.fetch_quorum_public_key(6, [0; 32], 1).is_none());
+    }
+}

packages/rs-drive-proof-verifier/src/error.rs

@@ -109,6 +109,26 @@ pub enum Error {
     /// Context provider error
     #[error("context provider error: {0}")]
     ContextProviderError(#[from] dash_context_provider::ContextProviderError),
+
+    /// The context provider could not supply the public key of the quorum
+    /// that signed the proof. The quorum fields come from the response, which
+    /// is not verified yet; they say which key to look for, not that the
+    /// quorum exists.
+    #[error(
+        "quorum key unavailable for quorum {quorum_type}:{}: {error}",
+        hex::encode(.quorum_hash)
+    )]
+    QuorumKeyUnavailable {
+        /// Quorum type named by the proof
+        quorum_type: u32,
+        /// Quorum hash named by the proof
+        quorum_hash: [u8; 32],
+        /// Core chain locked height from the response metadata
+        core_chain_locked_height: u32,
+        /// Why the provider could not supply the key
+        #[source]
+        error: dash_context_provider::ContextProviderError,
+    },
 }
 
 impl From<drive::error::Error> for Error {

packages/rs-drive-proof-verifier/src/lib.rs

@@ -95,7 +95,9 @@ pub use proof::{FromProof, Length};
 // Re-export context provider types from dash-context-provider
 #[cfg(feature = "mocks")]
 pub use dash_context_provider::MockContextProvider;
-pub use dash_context_provider::{ContextProvider, ContextProviderError, DataContractProvider};
+pub use dash_context_provider::{
+    ContextProvider, ContextProviderError, DataContractProvider, QuorumKeyFuture,
+};
 
 /// From Request
 pub mod from_request;

packages/rs-drive-proof-verifier/src/verify.rs

@@ -85,8 +85,14 @@ pub(crate) fn verify_tenderdash_signature(
     // Now, lookup quorum details
     let chain_id = mtd.chain_id.clone();
     let quorum_type = proof.quorum_type;
-    let pubkey_bytes =
-        provider.get_quorum_public_key(quorum_type, quorum_hash, core_locked_height)?;
+    let pubkey_bytes = provider
+        .get_quorum_public_key(quorum_type, quorum_hash, core_locked_height)
+        .map_err(|error| Error::QuorumKeyUnavailable {
+            quorum_type,
+            quorum_hash,
+            core_chain_locked_height: core_locked_height,
+            error,
+        })?;
 
     let state_id = StateId {
         app_version: version,
@@ -534,19 +540,24 @@ mod tests {
     }
 
     /// When the provider fails to return a quorum public key, the error must
-    /// propagate (wrapped into Error::ContextProviderError) rather than panic.
+    /// name the quorum the proof asked for, so the SDK can fetch that key and
+    /// verify the same response again, and keep the provider's message for
+    /// callers that classify errors by their text.
     #[test]
-    fn test_verify_tenderdash_proof_provider_error_propagates() {
+    fn should_report_the_proofs_quorum_when_the_provider_has_no_key() {
         let proof = Proof {
             grovedb_proof: grovedb_proof_bytes(1),
-            quorum_hash: vec![0u8; 32],
+            quorum_hash: vec![0xab; 32],
             signature: vec![0u8; 96],
             round: 1,
             block_id_hash: vec![0u8; 32],
-            quorum_type: 1,
+            quorum_type: 6,
         };
 
-        let metadata = test_metadata();
+        let metadata = ResponseMetadata {
+            core_chain_locked_height: 4321,
+            ..test_metadata()
+        };
         let provider = ErroringProvider;
 
         let result = verify_tenderdash_proof(&proof, &metadata, &[0u8; 32], &provider);
@@ -556,6 +567,19 @@ mod tests {
             err_msg.contains("simulated provider failure"),
             "error should contain provider message, got: {err_msg}"
         );
+        match err {
+            Error::QuorumKeyUnavailable {
+                quorum_type,
+                quorum_hash,
+                core_chain_locked_height,
+                error: ContextProviderError::InvalidQuorum(_),
+            } => {
+                assert_eq!(quorum_type, 6);
+                assert_eq!(quorum_hash, [0xab; 32]);
+                assert_eq!(core_chain_locked_height, metadata.core_chain_locked_height);
+            }
+            other => panic!("expected QuorumKeyUnavailable, got {other:?}"),
+        }
     }
 
     /// A provider that returns 48 zero-bytes as the "public key" causes the