Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Backport changes about the light client protocol #3018

Merged
merged 2 commits into from
Nov 22, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions bin/light-base/src/runtime_service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down
7 changes: 2 additions & 5 deletions bin/light-base/src/sync_service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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,
})
Expand Down
4 changes: 4 additions & 0 deletions bin/wasm-node/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
19 changes: 12 additions & 7 deletions src/network/protocol/storage_call_proof.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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`].
Expand Down
35 changes: 21 additions & 14 deletions src/network/service/requests_responses.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
))
)),
}
});

Expand All @@ -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,
Expand Down Expand Up @@ -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()
}
}

Expand Down Expand Up @@ -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`].
Expand All @@ -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 {
Expand All @@ -1058,6 +1064,7 @@ impl CallProofRequestError {
match self {
CallProofRequestError::Request(_) => true,
CallProofRequestError::Decode(_) => false,
CallProofRequestError::RemoteCouldntAnswer => true,
}
}
}
Expand Down