diff --git a/lib/src/executor/runtime_call.rs b/lib/src/executor/runtime_call.rs index 77703e0af7..3e88217618 100644 --- a/lib/src/executor/runtime_call.rs +++ b/lib/src/executor/runtime_call.rs @@ -1355,8 +1355,6 @@ enum PendingStorageChangesTrieNode { /// Writing and reading keys the main trie under this prefix obeys special rules. const CHILD_STORAGE_SPECIAL_PREFIX: &[u8] = b":child_storage:"; -/// Writing and reading keys the main trie under this prefix obeys special rules. -const DEFAULT_CHILD_STORAGE_SPECIAL_PREFIX: &[u8] = b":child_storage:default:"; impl Inner { /// Continues the execution. @@ -1442,11 +1440,7 @@ impl Inner { // If we've finished calculating a child trie, update its entry in the // main trie. if let Some(child_trie) = &trie { - let mut main_trie_key = Vec::with_capacity( - DEFAULT_CHILD_STORAGE_SPECIAL_PREFIX.len() + child_trie.len(), - ); - main_trie_key.extend_from_slice(DEFAULT_CHILD_STORAGE_SPECIAL_PREFIX); - main_trie_key.extend_from_slice(child_trie); + let main_trie_key = trie::default_child_trie_root_key(child_trie); if trie_root_hash != trie::EMPTY_BLAKE2_TRIE_MERKLE_VALUE { self.pending_storage_changes diff --git a/lib/src/network/codec/state_request.rs b/lib/src/network/codec/state_request.rs index f596cdc3d5..808c9d4ff4 100644 --- a/lib/src/network/codec/state_request.rs +++ b/lib/src/network/codec/state_request.rs @@ -87,14 +87,10 @@ pub fn build_state_request(config: StateRequest) -> impl Iterator either::Right( - protobuf::bytes_tag_encode(2, { - let mut vec = b":child_storage:default:".to_vec(); - vec.extend(child_trie); - vec - }) - .map(either::Left) - .chain(protobuf::bytes_tag_encode(2, key).map(either::Right)) - .map(either::Right), + protobuf::bytes_tag_encode(2, crate::trie::default_child_trie_root_key(&child_trie)) + .map(either::Left) + .chain(protobuf::bytes_tag_encode(2, key).map(either::Right)) + .map(either::Right), ), }; diff --git a/lib/src/network/codec/storage_call_proof.rs b/lib/src/network/codec/storage_call_proof.rs index c15abafc99..b2a542507c 100644 --- a/lib/src/network/codec/storage_call_proof.rs +++ b/lib/src/network/codec/storage_call_proof.rs @@ -168,16 +168,23 @@ pub fn build_child_storage_proof_request<'a>( impl Iterator + Clone + 'a> + 'a, >, ) -> impl Iterator> { + // The remote expects the child storage key in field 3 to be the full prefixed key + // (`:child_storage:default:`). It strips the prefix via + // `ChildType::from_prefixed_key` and rejects a bare child trie name with + // `InvalidChildStorageKey`. `ChildStorageProofRequestConfig::child_trie` is the bare name, + // so the prefix is prepended here. + let prefixed_child_trie = crate::trie::default_child_trie_root_key(config.child_trie.as_ref()); + // Message format for RemoteReadChildRequest (tag 4 in Request oneof): // - Field 2: block hash - // - Field 3: child storage key (child trie name) + // - Field 3: child storage key (prefixed child trie key) // - Field 6: keys to fetch protobuf::message_tag_encode( 4, protobuf::bytes_tag_encode(2, config.block_hash) .map(either::Left) .chain( - protobuf::bytes_tag_encode(3, config.child_trie) + protobuf::bytes_tag_encode(3, prefixed_child_trie) .map(either::Left) .map(either::Right), ) diff --git a/lib/src/trie.rs b/lib/src/trie.rs index 72623ffe92..7bcd47aa4a 100644 --- a/lib/src/trie.rs +++ b/lib/src/trie.rs @@ -177,6 +177,22 @@ pub const EMPTY_KECCAK256_TRIE_MERKLE_VALUE: [u8; 32] = [ 214, 101, 145, 255, 150, 169, 224, 100, 188, 201, 138, ]; +/// Prefix that identifies a default child trie within the main trie's storage. +/// +/// The root of a default child trie identified by `trie_id` is stored in the main trie under +/// the key `concat(`[`DEFAULT_CHILD_STORAGE_PREFIX`]`, trie_id)`. Use +/// [`default_child_trie_root_key`] to build that key. +pub const DEFAULT_CHILD_STORAGE_PREFIX: &[u8] = b":child_storage:default:"; + +/// Returns the main-trie storage key under which the root of the default child trie identified +/// by `trie_id` is stored: `concat(`[`DEFAULT_CHILD_STORAGE_PREFIX`]`, trie_id)`. +pub fn default_child_trie_root_key(trie_id: &[u8]) -> Vec { + let mut k = Vec::with_capacity(DEFAULT_CHILD_STORAGE_PREFIX.len() + trie_id.len()); + k.extend_from_slice(DEFAULT_CHILD_STORAGE_PREFIX); + k.extend_from_slice(trie_id); + k +} + /// Returns the Merkle value of a trie containing the entries passed as parameter. The entries /// passed as parameter are `(key, value)`. /// diff --git a/light-base/src/json_rpc_service/background.rs b/light-base/src/json_rpc_service/background.rs index befc5ebb74..6302f503ae 100644 --- a/light-base/src/json_rpc_service/background.rs +++ b/light-base/src/json_rpc_service/background.rs @@ -2270,26 +2270,29 @@ pub(super) async fn run( } }; - if child_trie.is_some() { - // TODO: implement this + // Child-trie queries only support value and hash reads. The descendants + // and merkle-value variants resolve against a trie root that, for a child + // trie, isn't known until its proof arrives. + if child_trie.is_some() + && items.iter().any(|item| { + !matches!( + item.ty, + methods::ChainHeadStorageType::Value + | methods::ChainHeadStorageType::Hash + ) + }) + { let _ = me .responses_tx .send(parse::build_error_response( request_id_json, parse::ErrorResponse::ServerError( -32000, - "Child key storage queries not supported yet", + "child-trie storage queries only support value and hash reads", ), None, )) .await; - log!( - &me.platform, - Warn, - &me.log_target, - "chainHead_v1_storage has been called with a non-null childTrie. \ - This isn't supported by smoldot yet." - ); continue; } @@ -2341,15 +2344,28 @@ pub(super) async fn run( } // Initialize the storage query operation. - let fetch_operation = me.sync_service.clone().storage_query( - block_number, - hash.0, - block_state_trie_root, - storage_operations.into_iter(), - 3, - Duration::from_secs(20), - NonZero::::new(2).unwrap(), - ); + let fetch_operation = if let Some(child_trie) = child_trie { + me.sync_service.clone().child_storage_query( + block_number, + hash.0, + block_state_trie_root, + child_trie.0, + storage_operations.into_iter(), + 3, + Duration::from_secs(20), + NonZero::::new(2).unwrap(), + ) + } else { + me.sync_service.clone().storage_query( + block_number, + hash.0, + block_state_trie_root, + storage_operations.into_iter(), + 3, + Duration::from_secs(20), + NonZero::::new(2).unwrap(), + ) + }; let operation_id = { let mut operation_id = [0u8; 32]; diff --git a/light-base/src/runtime_service.rs b/light-base/src/runtime_service.rs index 4f94740606..7ab36f2b1e 100644 --- a/light-base/src/runtime_service.rs +++ b/light-base/src/runtime_service.rs @@ -3411,10 +3411,7 @@ async fn runtime_call_single_attempt( let proof_access_duration_before = platform.now(); let trie_root = if let Some(child_trie) = child_trie { // TODO: allocation here, but probably not problematic - const PREFIX: &[u8] = b":child_storage:default:"; - let mut key = Vec::with_capacity(PREFIX.len() + child_trie.len()); - key.extend_from_slice(PREFIX); - key.extend_from_slice(child_trie.as_ref()); + let key = smoldot::trie::default_child_trie_root_key(child_trie.as_ref()); match call_proof.storage_value(block_state_trie_root_hash, &key) { Err(_) => { return ( @@ -3755,10 +3752,7 @@ fn get_trie_root_for_child_or_main<'a>( child_trie: Option<&[u8]>, ) -> Result, ()> { if let Some(child_trie) = child_trie { - const PREFIX: &[u8] = b":child_storage:default:"; - let mut key = Vec::with_capacity(PREFIX.len() + child_trie.len()); - key.extend_from_slice(PREFIX); - key.extend_from_slice(child_trie); + let key = smoldot::trie::default_child_trie_root_key(child_trie); match proof.storage_value(block_state_trie_root_hash, &key) { Err(_) => Err(()), Ok(None) => Ok(None), diff --git a/light-base/src/sync_service.rs b/light-base/src/sync_service.rs index ac17af618e..d5be5d1c62 100644 --- a/light-base/src/sync_service.rs +++ b/light-base/src/sync_service.rs @@ -424,34 +424,101 @@ impl SyncService { total_attempts: u32, timeout_per_request: Duration, max_parallel: NonZero, + ) -> StorageQuery { + self.storage_query_inner( + block_number, + block_hash, + main_trie_root_hash, + None, + requests, + total_attempts, + timeout_per_request, + max_parallel, + ) + } + + /// Like [`SyncService::storage_query`], but reads from the child trie named `child_trie` + /// (the bytes after the `:child_storage:default:` prefix) instead of the main trie. + /// + /// The proof is fetched with a child-storage-proof network request. Verification is two + /// levels deep: the child trie's root is first resolved from `main_trie_root_hash` at + /// `:child_storage:default:`, then each requested key is verified against that + /// child root. Only [`StorageRequestItemTy::Value`] and [`StorageRequestItemTy::Hash`] are + /// supported here. The descendants and merkle-value variants are not, as they resolve + /// against a fixed root that isn't known for a child trie until the proof arrives. + pub fn child_storage_query( + self: Arc, + block_number: u64, + block_hash: [u8; 32], + main_trie_root_hash: [u8; 32], + child_trie: Vec, + requests: impl Iterator, + total_attempts: u32, + timeout_per_request: Duration, + max_parallel: NonZero, + ) -> StorageQuery { + self.storage_query_inner( + block_number, + block_hash, + main_trie_root_hash, + Some(child_trie), + requests, + total_attempts, + timeout_per_request, + max_parallel, + ) + } + + fn storage_query_inner( + self: Arc, + block_number: u64, + block_hash: [u8; 32], + main_trie_root_hash: [u8; 32], + child_trie: Option>, + requests: impl Iterator, + total_attempts: u32, + timeout_per_request: Duration, + max_parallel: NonZero, ) -> StorageQuery { let total_attempts = usize::try_from(total_attempts).unwrap_or(usize::MAX); let requests = requests - .map(|request| match request.ty { - StorageRequestItemTy::DescendantsHashes - | StorageRequestItemTy::DescendantsValues => RequestImpl::PrefixScan { - scan: prefix_proof::prefix_scan(prefix_proof::Config { - prefix: &request.key, - trie_root_hash: main_trie_root_hash, - full_storage_values_required: matches!( + .map(|request| { + debug_assert!( + child_trie.is_none() + || matches!( request.ty, - StorageRequestItemTy::DescendantsValues + StorageRequestItemTy::Value | StorageRequestItemTy::Hash ), - }), - requested_key: request.key, - }, - StorageRequestItemTy::Value => RequestImpl::ValueOrHash { - key: request.key, - hash: false, - }, - StorageRequestItemTy::Hash => RequestImpl::ValueOrHash { - key: request.key, - hash: true, - }, - StorageRequestItemTy::MerkleProof => RequestImpl::MerkleProof { key: request.key }, - StorageRequestItemTy::ClosestDescendantMerkleValue => { - RequestImpl::ClosestDescendantMerkleValue { key: request.key } + "child-trie queries only support `Value` and `Hash` request types" + ); + match request.ty { + StorageRequestItemTy::DescendantsHashes + | StorageRequestItemTy::DescendantsValues => RequestImpl::PrefixScan { + scan: prefix_proof::prefix_scan(prefix_proof::Config { + prefix: &request.key, + trie_root_hash: main_trie_root_hash, + full_storage_values_required: matches!( + request.ty, + StorageRequestItemTy::DescendantsValues + ), + }), + requested_key: request.key, + }, + StorageRequestItemTy::Value => RequestImpl::ValueOrHash { + key: request.key, + hash: false, + }, + StorageRequestItemTy::Hash => RequestImpl::ValueOrHash { + key: request.key, + hash: true, + }, + StorageRequestItemTy::MerkleProof => { + RequestImpl::MerkleProof { key: request.key } + } + StorageRequestItemTy::ClosestDescendantMerkleValue => { + RequestImpl::ClosestDescendantMerkleValue { key: request.key } + } } }) .enumerate() @@ -461,6 +528,7 @@ impl SyncService { block_number, block_hash, main_trie_root_hash, + child_trie, total_attempts, timeout_per_request, _max_parallel: max_parallel, @@ -578,6 +646,10 @@ pub struct StorageQuery { block_number: u64, block_hash: [u8; 32], main_trie_root_hash: [u8; 32], + /// `Some` for a child-trie query (the bytes after `:child_storage:default:`). When set, + /// requests are fetched via child-storage-proof requests and each key is verified against + /// the child trie root resolved from `main_trie_root_hash`, not against the main root. + child_trie: Option>, /// Requests that haven't been fulfilled yet. /// The `usize` is the index of the request in the original list of requests that the API user /// provided. @@ -717,38 +789,52 @@ impl StorageQuery { keys }; - let result = self - .sync_service - .network_service - .clone() - .storage_proof_request( - target.clone(), - codec::StorageProofRequestConfig { - block_hash: self.block_hash, - keys: keys_to_request.into_iter(), - }, - self.timeout_per_request, - ) - .await; + let result: Result<_, StorageQueryNetworkError> = + if let Some(child_trie) = &self.child_trie { + self.sync_service + .network_service + .clone() + .child_storage_proof_request( + target.clone(), + codec::ChildStorageProofRequestConfig { + block_hash: self.block_hash, + child_trie: &child_trie[..], + keys: keys_to_request.into_iter(), + }, + self.timeout_per_request, + ) + .await + .map_err(StorageQueryNetworkError::ChildStorageProof) + } else { + self.sync_service + .network_service + .clone() + .storage_proof_request( + target.clone(), + codec::StorageProofRequestConfig { + block_hash: self.block_hash, + keys: keys_to_request.into_iter(), + }, + self.timeout_per_request, + ) + .await + .map_err(StorageQueryNetworkError::StorageProof) + }; let proof = match result { Ok(r) => r, Err(err) => { // In case of error that isn't a protocol error, we reduce the number of // trie node items to request. - let reduce_max = match &err { - network_service::StorageProofRequestError::RequestTooLarge => true, - network_service::StorageProofRequestError::Request( - service::StorageProofRequestError::Request(err), - ) => !err.is_protocol_error(), - _ => false, - }; - - if !matches!( - err, - network_service::StorageProofRequestError::RequestTooLarge - ) || self.response_nodes_cap == 1 - { + let reduce_max = err.is_request_too_large() + || match err.protocol_request_error() { + Some(service::StorageProofRequestError::Request(inner)) => { + !inner.is_protocol_error() + } + _ => false, + }; + + if !err.is_request_too_large() || self.response_nodes_cap == 1 { self.sync_service .network_service .ban_and_disconnect( @@ -789,6 +875,44 @@ impl StorageQuery { } }; + // Resolve the trie root that the requested keys are verified against. For a main-trie + // query this is `main_trie_root_hash`. For a child-trie query the child root is first + // read from the main trie at `:child_storage:default:`. `None` means the + // child trie doesn't exist, in which case every key has no value. + let effective_root: Option<[u8; 32]> = if let Some(child_trie) = &self.child_trie { + let child_root_key = trie::default_child_trie_root_key(child_trie); + match decoded_proof.storage_value(&self.main_trie_root_hash, &child_root_key) { + Ok(Some((value, _))) => match <&[u8; 32]>::try_from(value) { + Ok(hash) => Some(*hash), + Err(_) => { + // The stored child root isn't a 32-byte hash, which means a corrupt + // proof. Ban the peer and count the failure. + self.sync_service + .network_service + .ban_and_disconnect( + target, + network_service::BanSeverity::High, + "bad-child-trie-root", + ) + .await; + self.outcome_errors + .push(StorageQueryErrorDetail::MissingProofEntry); + continue; + } + }, + Ok(None) => None, + Err(_) => { + // The main-trie path to the child root is absent from the proof. Retry + // against another peer. + self.outcome_errors + .push(StorageQueryErrorDetail::MissingProofEntry); + continue; + } + } + } else { + Some(self.main_trie_root_hash) + }; + let mut proof_has_advanced_verification = false; for (request_index, request) in mem::take(&mut self.requests_remaining) { @@ -867,8 +991,21 @@ impl StorageQuery { } } RequestImpl::ValueOrHash { key, hash } => { + let Some(lookup_root) = effective_root.as_ref() else { + // Child trie doesn't exist, so the key has no value. + proof_has_advanced_verification = true; + self.available_results.push_back(( + request_index, + if hash { + StorageResultItem::Hash { key, hash: None } + } else { + StorageResultItem::Value { key, value: None } + }, + )); + continue; + }; match decoded_proof.trie_node_info( - &self.main_trie_root_hash, + lookup_root, trie::bytes_to_nibbles(key.iter().copied()), ) { Ok(node_info) => match node_info.storage_value { @@ -1046,21 +1183,22 @@ impl StorageQueryError { /// issue. pub fn is_network_problem(&self) -> bool { self.errors.iter().all(|err| match err { - StorageQueryErrorDetail::Network( - network_service::StorageProofRequestError::Request( - service::StorageProofRequestError::Request(_) - | service::StorageProofRequestError::RemoteCouldntAnswer, - ), - ) - | StorageQueryErrorDetail::Network( - network_service::StorageProofRequestError::NoConnection, - ) => true, - StorageQueryErrorDetail::Network( - network_service::StorageProofRequestError::Request( - service::StorageProofRequestError::Decode(_), - ) - | network_service::StorageProofRequestError::RequestTooLarge, - ) => false, + StorageQueryErrorDetail::Network(net) => { + if net.is_no_connection() { + return true; + } + if net.is_request_too_large() { + return false; + } + match net.protocol_request_error() { + Some( + service::StorageProofRequestError::Request(_) + | service::StorageProofRequestError::RemoteCouldntAnswer, + ) => true, + Some(service::StorageProofRequestError::Decode(_)) => false, + None => false, + } + } StorageQueryErrorDetail::ProofVerification(_) | StorageQueryErrorDetail::MissingProofEntry => false, }) @@ -1086,7 +1224,7 @@ impl fmt::Display for StorageQueryError { pub enum StorageQueryErrorDetail { /// Error during the network request. #[display("{_0}")] - Network(network_service::StorageProofRequestError), + Network(StorageQueryNetworkError), /// Error verifying the proof. #[display("{_0}")] ProofVerification(proof_decode::Error), @@ -1094,6 +1232,57 @@ pub enum StorageQueryErrorDetail { MissingProofEntry, } +/// Network-level error returned by a storage query. Distinguishes a main-trie request from a +/// child-trie request so callers can tell them apart for logging or retry policy. +#[derive(Debug, derive_more::Display, derive_more::Error, Clone)] +pub enum StorageQueryNetworkError { + /// Error during a main-trie storage proof request. + #[display("storage proof request: {_0}")] + StorageProof(network_service::StorageProofRequestError), + /// Error during a child-trie storage proof request. + #[display("child storage proof request: {_0}")] + ChildStorageProof(network_service::ChildStorageProofRequestError), +} + +impl StorageQueryNetworkError { + /// Returns `true` if this is a `NoConnection` error from either variant. + fn is_no_connection(&self) -> bool { + matches!( + self, + StorageQueryNetworkError::StorageProof( + network_service::StorageProofRequestError::NoConnection + ) | StorageQueryNetworkError::ChildStorageProof( + network_service::ChildStorageProofRequestError::NoConnection + ) + ) + } + + /// Returns `true` if the request was rejected for being too large. + fn is_request_too_large(&self) -> bool { + matches!( + self, + StorageQueryNetworkError::StorageProof( + network_service::StorageProofRequestError::RequestTooLarge + ) | StorageQueryNetworkError::ChildStorageProof( + network_service::ChildStorageProofRequestError::RequestTooLarge + ) + ) + } + + /// Returns the protocol-level request error if this wraps a `Request(_)`. + fn protocol_request_error(&self) -> Option<&service::StorageProofRequestError> { + match self { + StorageQueryNetworkError::StorageProof( + network_service::StorageProofRequestError::Request(e), + ) + | StorageQueryNetworkError::ChildStorageProof( + network_service::ChildStorageProofRequestError::Request(e), + ) => Some(e), + _ => None, + } + } +} + /// Return value of [`SyncService::subscribe_all`]. pub struct SubscribeAll { /// SCALE-encoded header of the finalized block at the time of the subscription. diff --git a/light-base/src/sync_service/parachain.rs b/light-base/src/sync_service/parachain.rs index 65a3a40270..e74d520b0d 100644 --- a/light-base/src/sync_service/parachain.rs +++ b/light-base/src/sync_service/parachain.rs @@ -1527,10 +1527,7 @@ fn run_single_runtime_call( executor::runtime_call::RuntimeCall::StorageGet(get) => { let child_trie = get.child_trie().map(|c| c.as_ref().to_vec()); let trie_root = if let Some(child_trie) = &child_trie { - const PREFIX: &[u8] = b":child_storage:default:"; - let mut key = Vec::with_capacity(PREFIX.len() + child_trie.len()); - key.extend_from_slice(PREFIX); - key.extend_from_slice(child_trie); + let key = smoldot::trie::default_child_trie_root_key(child_trie); match proof.storage_value(state_root, &key) { Ok(Some((value, _))) => match <&[u8; 32]>::try_from(value) { Ok(hash) => Some(*hash),