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.

Sign up for GitHub

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

Fix storage items being silently discarded from requests #1216

Merged
merged 2 commits into from
Oct 12, 2023
Merged

Fix storage items being silently discarded from requests #1216

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
701b2f4
Fix storage items being silently discarded from requests
tomaka Oct 12, 2023
6b78a6c
PR link
tomaka Oct 12, 2023
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
38 changes: 28 additions & 10 deletions light-base/src/sync_service.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -626,6 +626,8 @@ impl<TPlat: PlatformRef> SyncService<TPlat> {
}
};

let mut proof_has_advanced_verification = false;

for request in mem::take(&mut requests_remaining) {
match request {
RequestImpl::PrefixScan {
Expand All @@ -634,6 +636,7 @@ impl<TPlat: PlatformRef> SyncService<TPlat> {
} => {
match scan.resume(proof.decode()) {
Ok(prefix_proof::ResumeOutcome::InProgress(scan)) => {
proof_has_advanced_verification = true;
requests_remaining.push(RequestImpl::PrefixScan {
scan,
requested_key,
Expand All @@ -643,6 +646,7 @@ impl<TPlat: PlatformRef> SyncService<TPlat> {
entries,
full_storage_values_required,
}) => {
proof_has_advanced_verification = true;
// The value of `full_storage_values_required` determines whether
// we wanted full values (`true`) or hashes (`false`).
for (key, value) in entries {
Expand Down Expand Up @@ -681,15 +685,16 @@ impl<TPlat: PlatformRef> SyncService<TPlat> {
}
}
}
Err((_, prefix_proof::Error::InvalidProof(err))) => {
Err((_, prefix_proof::Error::InvalidProof(_))) => {
// Since we decode the proof above, this is never supposed to
// be reachable.
debug_assert!(false);
outcome_errors
.push(StorageQueryErrorDetail::ProofVerification(err));
unreachable!()
}
Err((_, prefix_proof::Error::MissingProofEntry)) => {
outcome_errors.push(StorageQueryErrorDetail::MissingProofEntry);
Err((scan, prefix_proof::Error::MissingProofEntry)) => {
requests_remaining.push(RequestImpl::PrefixScan {
requested_key,
scan,
});
}
}
}
Expand All @@ -701,15 +706,17 @@ impl<TPlat: PlatformRef> SyncService<TPlat> {
) {
Ok(node_info) => match node_info.storage_value {
proof_decode::StorageValue::HashKnownValueMissing(h) if hash => {
proof_has_advanced_verification = true;
final_results.push(StorageResultItem::Hash {
key,
hash: Some(*h),
});
}
proof_decode::StorageValue::HashKnownValueMissing(_) => {
outcome_errors.push(StorageQueryErrorDetail::MissingProofEntry);
requests_remaining.push(RequestImpl::ValueOrHash { key, hash });
}
proof_decode::StorageValue::Known { value, .. } => {
proof_has_advanced_verification = true;
if hash {
let hashed_value =
blake2_rfc::blake2b::blake2b(32, &[], value);
Expand All @@ -728,6 +735,7 @@ impl<TPlat: PlatformRef> SyncService<TPlat> {
}
}
proof_decode::StorageValue::None => {
proof_has_advanced_verification = true;
if hash {
final_results
.push(StorageResultItem::Hash { key, hash: None });
Expand All @@ -738,7 +746,7 @@ impl<TPlat: PlatformRef> SyncService<TPlat> {
}
},
Err(proof_decode::IncompleteProofError { .. }) => {
outcome_errors.push(StorageQueryErrorDetail::MissingProofEntry);
requests_remaining.push(RequestImpl::ValueOrHash { key, hash });
}
}
}
Expand All @@ -752,7 +760,8 @@ impl<TPlat: PlatformRef> SyncService<TPlat> {
Ok(Some(merkle_value)) => Some(merkle_value.as_ref().to_vec()),
Ok(None) => None,
Err(proof_decode::IncompleteProofError { .. }) => {
outcome_errors.push(StorageQueryErrorDetail::MissingProofEntry);
requests_remaining
.push(RequestImpl::ClosestDescendantMerkleValue { key });
continue;
}
};
Expand All @@ -763,11 +772,14 @@ impl<TPlat: PlatformRef> SyncService<TPlat> {
Ok(Some(ancestor)) => Some(ancestor.to_vec()),
Ok(None) => None,
Err(proof_decode::IncompleteProofError { .. }) => {
outcome_errors.push(StorageQueryErrorDetail::MissingProofEntry);
requests_remaining
.push(RequestImpl::ClosestDescendantMerkleValue { key });
continue;
}
};

proof_has_advanced_verification = true;

final_results.push(StorageResultItem::ClosestDescendantMerkleValue {
requested_key: key,
closest_descendant_merkle_value,
Expand All @@ -776,6 +788,12 @@ impl<TPlat: PlatformRef> SyncService<TPlat> {
}
}
}

// If the proof doesn't contain any item that reduces the number of things to request,
// then we push an error.
if !proof_has_advanced_verification {
outcome_errors.push(StorageQueryErrorDetail::MissingProofEntry);
}
}
}

Expand Down
4 changes: 4 additions & 0 deletions wasm-node/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

## Unreleased

### Fixed

- Fix storage items requested through JSON-RPC functions not being sent to the JSON-RPC client when the full node doesn't send it back in the Merkle proof. ([#1216](https://github.com/smol-dot/smoldot/pull/1216))

## 2.0.4 - 2023-10-11

### Changed
Expand Down
Loading