diff --git a/polkadot/node/core/runtime-api/src/cache.rs b/polkadot/node/core/runtime-api/src/cache.rs index 9c09ea3f22a9e..30463e01a4659 100644 --- a/polkadot/node/core/runtime-api/src/cache.rs +++ b/polkadot/node/core/runtime-api/src/cache.rs @@ -47,7 +47,7 @@ pub(crate) struct RequestResultCache { check_validation_outputs: LruMap<(Hash, ParaId, CandidateCommitments), bool>, session_index_for_child: LruMap, validation_code: LruMap<(Hash, ParaId, OccupiedCoreAssumption), Option>, - validation_code_by_hash: LruMap>, + validation_code_by_hash: LruMap, candidate_pending_availability: LruMap<(Hash, ParaId), Option>, candidates_pending_availability: LruMap<(Hash, ParaId), Vec>, candidate_events: LruMap>, @@ -244,12 +244,15 @@ impl RequestResultCache { self.validation_code.insert(key, value); } - // the actual key is `ValidationCodeHash` (`Hash` is ignored), - // but we keep the interface that way to keep the macro simple + // The actual key is `ValidationCodeHash` (`Hash` is ignored). + // Only `Some` values are cached: validation code may not exist at query time but + // could be added later (e.g. via `force_set_current_code`). Caching `None` would + // produce stale results that prevent approval voting from fetching code that is + // already on-chain, stalling finality. pub(crate) fn validation_code_by_hash( &mut self, key: (Hash, ValidationCodeHash), - ) -> Option<&Option> { + ) -> Option<&ValidationCode> { self.validation_code_by_hash.get(&key.1).map(|v| &*v) } @@ -258,7 +261,9 @@ impl RequestResultCache { key: ValidationCodeHash, value: Option, ) { - self.validation_code_by_hash.insert(key, value); + if let Some(code) = value { + self.validation_code_by_hash.insert(key, code); + } } pub(crate) fn candidate_pending_availability( @@ -683,3 +688,25 @@ pub(crate) enum RequestResult { ParaIds(SessionIndex, Vec), UnappliedSlashesV2(Hash, Vec<(SessionIndex, CandidateHash, slashing::PendingSlashes)>), } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn validation_code_by_hash_does_not_cache_none() { + let mut cache = RequestResultCache::default(); + let relay_parent: Hash = [1u8; 32].into(); + let code = ValidationCode(vec![1, 2, 3]); + let code_hash = code.hash(); + + cache.cache_validation_code_by_hash(code_hash, None); + assert!( + cache.validation_code_by_hash((relay_parent, code_hash)).is_none(), + "None results must not be cached", + ); + + cache.cache_validation_code_by_hash(code_hash, Some(code.clone())); + assert_eq!(cache.validation_code_by_hash((relay_parent, code_hash)), Some(&code),); + } +} diff --git a/polkadot/node/core/runtime-api/src/lib.rs b/polkadot/node/core/runtime-api/src/lib.rs index c047fd46751c7..e12dbf646a35f 100644 --- a/polkadot/node/core/runtime-api/src/lib.rs +++ b/polkadot/node/core/runtime-api/src/lib.rs @@ -291,10 +291,17 @@ where query!(validation_code(para, assumption), sender) .map(|sender| Request::ValidationCode(para, assumption, sender)) }, - Request::ValidationCodeByHash(validation_code_hash, sender) => { - query!(validation_code_by_hash(validation_code_hash), sender) - .map(|sender| Request::ValidationCodeByHash(validation_code_hash, sender)) - }, + Request::ValidationCodeByHash(validation_code_hash, sender) => if let Some(code) = self + .requests_cache + .validation_code_by_hash((relay_parent, validation_code_hash)) + { + self.metrics.on_cached_request(); + let _ = sender.send(Ok(Some(code.clone()))); + None + } else { + Some(sender) + } + .map(|sender| Request::ValidationCodeByHash(validation_code_hash, sender)), Request::CandidatePendingAvailability(para, sender) => { query!(candidate_pending_availability(para), sender) .map(|sender| Request::CandidatePendingAvailability(para, sender)) diff --git a/prdoc/pr_11108.prdoc b/prdoc/pr_11108.prdoc new file mode 100644 index 0000000000000..4f5620fb3c659 --- /dev/null +++ b/prdoc/pr_11108.prdoc @@ -0,0 +1,8 @@ +title: 'polkadot-runtime-api-cache: Only cache validation code that exists' +doc: +- audience: Node Dev + description: |- + Otherwise there is the possibility that nodes cache `None` and some later `force_set_code` enacts the code. Then the nodes that have cached `None` do not know that the validation code now actually exists on chain. +crates: +- name: polkadot-node-core-runtime-api + bump: patch