Backport changes about the light client protocol (#3018)

Fix #2975

Backports paritytech/substrate#8046
See also paritytech/substrate#12732

bin/light-base/src/runtime_service.rs

@@ -961,8 +961,6 @@ impl RuntimeCallError {
     pub fn is_network_problem(&self) -> bool {
         match self {
             RuntimeCallError::InvalidRuntime(_) => false,
-            // TODO: as a temporary hack, we consider `TrieRootNotFound` as the remote not knowing about the requested block; see https://github.com/paritytech/substrate/pull/8046
-            RuntimeCallError::StorageRetrieval(proof_decode::Error::TrieRootNotFound) => true,
             RuntimeCallError::StorageRetrieval(_) => false,
             RuntimeCallError::MissingProofEntry => false,
             RuntimeCallError::CallProof(err) => err.is_network_problem(),

bin/light-base/src/sync_service.rs

@@ -613,7 +613,8 @@ impl StorageQueryError {
         self.errors.iter().all(|err| match err {
             StorageQueryErrorDetail::Network(
                 network_service::StorageProofRequestError::Request(
-                    service::StorageProofRequestError::Request(_),
+                    service::StorageProofRequestError::Request(_)
+                    | service::StorageProofRequestError::RemoteCouldntAnswer,
                 ),
             )
             | StorageQueryErrorDetail::Network(
@@ -624,10 +625,6 @@ impl StorageQueryError {
                     service::StorageProofRequestError::Decode(_),
                 ),
             ) => false,
-            // TODO: as a temporary hack, we consider `TrieRootNotFound` as the remote not knowing about the requested block; see https://github.com/paritytech/substrate/pull/8046
-            StorageQueryErrorDetail::ProofVerification(proof_decode::Error::TrieRootNotFound) => {
-                true
-            }
             StorageQueryErrorDetail::ProofVerification(_)
             | StorageQueryErrorDetail::MissingProofEntry => false,
         })

bin/wasm-node/CHANGELOG.md

@@ -7,6 +7,10 @@
 - In earlier versions of smoldot, `setTimeout(callback, 0)` was frequently used in order split execution of CPU-intensive tasks in multiple smaller ones while still giving back control to the execution environment (such as NodeJS or the browser). Unfortunately, when a web page is in the background, browsers set a minimum delay of one second for `setTimeout`. For this reason, the usage of ̀`setTimeout` has now been reduced to the strict minimum, except when the environment is browser and `document.visibilityState` is equal to `visible`. ([#2999](https://github.com/paritytech/smoldot/pull/2999))
 - Optimize the Merkle proof verification. The complexity has been reduced from `O(n^2)` to `O(n * log n)`. ([#3013](https://github.com/paritytech/smoldot/pull/3013))
 
+### Fixed
+
+- Fix `ProtobufDecode` errors appearing while the Grandpa warp syncing is still in progress. ([#3018](https://github.com/paritytech/smoldot/pull/3018))
+
 ## 0.7.7 - 2022-11-11
 
 ### Added

src/network/protocol/storage_call_proof.rs

@@ -94,29 +94,34 @@ pub fn build_call_proof_request<'a>(
 
 /// Decodes a response to a storage proof request or a call proof request.
 ///
-/// On success, contains a list of Merkle proof entries.
+/// On success, contains a list of Merkle proof entries, or `None` if the remote couldn't answer
+/// the request.
 pub fn decode_storage_or_call_proof_response(
     ty: StorageOrCallProof,
     response_bytes: &[u8],
-) -> Result<Vec<&[u8]>, DecodeStorageCallProofResponseError> {
+) -> Result<Option<Vec<&[u8]>>, DecodeStorageCallProofResponseError> {
     let field_num = match ty {
         StorageOrCallProof::CallProof => 1,
         StorageOrCallProof::StorageProof => 2,
     };
 
+    // TODO: while the `proof` field is correctly optional, the `response` field isn't supposed to be optional; make it `#[required]` again once https://github.com/paritytech/substrate/pull/12732 has been merged and released
+
     let mut parser = nom::combinator::all_consuming::<_, _, nom::error::Error<&[u8]>, _>(
         nom::combinator::complete(protobuf::message_decode! {
-            #[required] response = field_num => protobuf::message_tag_decode(protobuf::message_decode!{
-                #[required] proof = 2 => protobuf::bytes_tag_decode
+            #[optional] response = field_num => protobuf::message_tag_decode(protobuf::message_decode!{
+                #[optional] proof = 2 => protobuf::bytes_tag_decode
             }),
         }),
     );
 
-    let proof: &[u8] = match nom::Finish::finish(parser(response_bytes)) {
-        Ok((_, out)) => out.response.proof,
+    let proof = match nom::Finish::finish(parser(response_bytes)) {
+        Ok((_, out)) => out.response.and_then(|r| r.proof),
         Err(_) => return Err(DecodeStorageCallProofResponseError::ProtobufDecode),
     };
 
+    let Some(proof) = proof else { return Ok(None) };
+
     // The proof itself is a SCALE-encoded `Vec<Vec<u8>>`.
     let (_, decoded) = nom::combinator::all_consuming(nom::combinator::flat_map(
         crate::util::nom_scale_compact_usize,
@@ -126,7 +131,7 @@ pub fn decode_storage_or_call_proof_response(
         DecodeStorageCallProofResponseError::ProofDecodeError
     })?;
 
-    Ok(decoded)
+    Ok(Some(decoded))
 }
 
 /// Error potentially returned by [`decode_storage_or_call_proof_response`].

src/network/service/requests_responses.rs

@@ -186,16 +186,16 @@ where
                 let response = response
                     .map_err(StorageProofRequestError::Request)
                     .and_then(|payload| {
-                        if let Err(err) = protocol::decode_storage_or_call_proof_response(
+                        match protocol::decode_storage_or_call_proof_response(
                             protocol::StorageOrCallProof::StorageProof,
                             &payload,
                         ) {
-                            Err(StorageProofRequestError::Decode(err))
-                        } else {
-                            Ok(EncodedMerkleProof(
+                            Err(err) => Err(StorageProofRequestError::Decode(err)),
+                            Ok(None) => Err(StorageProofRequestError::RemoteCouldntAnswer),
+                            Ok(Some(_)) => Ok(EncodedMerkleProof(
                                 payload,
                                 protocol::StorageOrCallProof::StorageProof,
-                            ))
+                            )),
                         }
                     });
 
@@ -208,19 +208,19 @@ where
                 let response =
                     response
                         .map_err(CallProofRequestError::Request)
-                        .and_then(|payload| {
-                            if let Err(err) = protocol::decode_storage_or_call_proof_response(
+                        .and_then(
+                            |payload| match protocol::decode_storage_or_call_proof_response(
                                 protocol::StorageOrCallProof::CallProof,
                                 &payload,
                             ) {
-                                Err(CallProofRequestError::Decode(err))
-                            } else {
-                                Ok(EncodedMerkleProof(
+                                Err(err) => Err(CallProofRequestError::Decode(err)),
+                                Ok(None) => Err(CallProofRequestError::RemoteCouldntAnswer),
+                                Ok(Some(_)) => Ok(EncodedMerkleProof(
                                     payload,
                                     protocol::StorageOrCallProof::CallProof,
-                                ))
-                            }
-                        });
+                                )),
+                            },
+                        );
 
                 Event::RequestResult {
                     request_id,
@@ -909,7 +909,9 @@ pub struct EncodedMerkleProof(Vec<u8>, protocol::StorageOrCallProof);
 impl EncodedMerkleProof {
     /// Returns the decoded version of the proof.
     pub fn decode(&self) -> Vec<&[u8]> {
-        protocol::decode_storage_or_call_proof_response(self.1, &self.0).unwrap()
+        protocol::decode_storage_or_call_proof_response(self.1, &self.0)
+            .unwrap()
+            .unwrap()
     }
 }
 
@@ -1040,6 +1042,8 @@ pub enum StorageProofRequestError {
     Request(peers::RequestError),
     #[display(fmt = "Response decoding error: {}", _0)]
     Decode(protocol::DecodeStorageCallProofResponseError),
+    /// The remote is incapable of answering this specific request.
+    RemoteCouldntAnswer,
 }
 
 /// Error returned by [`ChainNetwork::start_call_proof_request`].
@@ -1049,6 +1053,8 @@ pub enum CallProofRequestError {
     Request(peers::RequestError),
     #[display(fmt = "Response decoding error: {}", _0)]
     Decode(protocol::DecodeStorageCallProofResponseError),
+    /// The remote is incapable of answering this specific request.
+    RemoteCouldntAnswer,
 }
 
 impl CallProofRequestError {
@@ -1058,6 +1064,7 @@ impl CallProofRequestError {
         match self {
             CallProofRequestError::Request(_) => true,
             CallProofRequestError::Decode(_) => false,
+            CallProofRequestError::RemoteCouldntAnswer => true,
         }
     }
 }