From 6fb721e8615cb7ed60abcc1a088396e332e64f8e Mon Sep 17 00:00:00 2001
From: Svyatoslav Nikolsky <svyatonik@gmail.com>
Date: Mon, 19 Aug 2019 14:44:16 +0300
Subject: [PATCH] Value lifetime is returned from blockchain cache (#3403)
* value range in blockchain cache
* revert me (testing for spurious failure)
* Revert "revert me (testing for spurious failure)"
This reverts commit 21a4a3cf5ee14e003541b779c41351e4f5e1122a.
---
.../core/client/db/src/cache/list_cache.rs | 19 +++++++++++--------
substrate/core/client/db/src/cache/mod.rs | 17 +++++++++++++++--
substrate/core/client/db/src/light.rs | 18 +++++++++++++-----
substrate/core/client/src/blockchain.rs | 8 +++++++-
substrate/core/consensus/aura/src/lib.rs | 4 ++--
substrate/core/consensus/babe/src/lib.rs | 4 ++--
6 files changed, 50 insertions(+), 20 deletions(-)
diff --git a/substrate/core/client/db/src/cache/list_cache.rs b/substrate/core/client/db/src/cache/list_cache.rs
index 9e54fdbb62e..188012e78f0 100644
--- a/substrate/core/client/db/src/cache/list_cache.rs
+++ b/substrate/core/client/db/src/cache/list_cache.rs
@@ -130,7 +130,10 @@ impl<Block: BlockT, T: CacheItemT, S: Storage<Block, T>> ListCache<Block, T, S>
}
/// Get value valid at block.
- pub fn value_at_block(&self, at: &ComplexBlockId<Block>) -> ClientResult<Option<T>> {
+ pub fn value_at_block(
+ &self,
+ at: &ComplexBlockId<Block>,
+ ) -> ClientResult<Option<(ComplexBlockId<Block>, Option<ComplexBlockId<Block>>, T)>> {
let head = if at.number <= self.best_finalized_block.number {
// if the block is older than the best known finalized block
// => we're should search for the finalized value
@@ -164,7 +167,7 @@ impl<Block: BlockT, T: CacheItemT, S: Storage<Block, T>> ListCache<Block, T, S>
match head {
Some(head) => head.search_best_before(&self.storage, at.number)
- .map(|e| e.map(|e| e.0.value)),
+ .map(|e| e.map(|e| (e.0.valid_from, e.1, e.0.value))),
None => Ok(None),
}
}
@@ -677,7 +680,7 @@ pub mod tests {
.with_entry(test_id(100), StorageEntry { prev_valid_from: Some(test_id(30)), value: 100 })
.with_entry(test_id(30), StorageEntry { prev_valid_from: None, value: 30 }),
1024, test_id(100)
- ).value_at_block(&test_id(50)).unwrap(), Some(30));
+ ).value_at_block(&test_id(50)).unwrap(), Some((test_id(30), Some(test_id(100)), 30)));
// when block is the best finalized block AND value is some
// ---> [100]
assert_eq!(ListCache::new(
@@ -687,7 +690,7 @@ pub mod tests {
.with_entry(test_id(100), StorageEntry { prev_valid_from: Some(test_id(30)), value: 100 })
.with_entry(test_id(30), StorageEntry { prev_valid_from: None, value: 30 }),
1024, test_id(100)
- ).value_at_block(&test_id(100)).unwrap(), Some(100));
+ ).value_at_block(&test_id(100)).unwrap(), Some((test_id(100), None, 100)));
// when block is parallel to the best finalized block
// ---- 100
// ---> [100]
@@ -708,7 +711,7 @@ pub mod tests {
.with_id(50, H256::from_low_u64_be(50))
.with_entry(test_id(100), StorageEntry { prev_valid_from: Some(test_id(30)), value: 100 }),
1024, test_id(100)
- ).value_at_block(&test_id(200)).unwrap(), Some(100));
+ ).value_at_block(&test_id(200)).unwrap(), Some((test_id(100), None, 100)));
// when block is later than last finalized block AND there are no matching forks
// AND block is connected to finalized block AND finalized value is Some
@@ -724,7 +727,7 @@ pub mod tests {
.with_header(test_header(4))
.with_header(fork_header(0, 2, 3)),
1024, test_id(2)
- ).value_at_block(&fork_id(0, 2, 3)).unwrap(), Some(2));
+ ).value_at_block(&fork_id(0, 2, 3)).unwrap(), Some((correct_id(2), None, 2)));
// when block is later than last finalized block AND there are no matching forks
// AND block is not connected to finalized block
// --- 2 --- 3
@@ -754,7 +757,7 @@ pub mod tests {
.with_header(test_header(4))
.with_header(test_header(5)),
1024, test_id(2)
- ).value_at_block(&correct_id(5)).unwrap(), Some(4));
+ ).value_at_block(&correct_id(5)).unwrap(), Some((correct_id(4), None, 4)));
// when block is later than last finalized block AND it does not fits unfinalized fork
// AND it is connected to the finalized block AND finalized value is Some
// ---> [2] ----------> [4]
@@ -769,7 +772,7 @@ pub mod tests {
.with_header(test_header(4))
.with_header(fork_header(0, 2, 3)),
1024, test_id(2)
- ).value_at_block(&fork_id(0, 2, 3)).unwrap(), Some(2));
+ ).value_at_block(&fork_id(0, 2, 3)).unwrap(), Some((correct_id(2), None, 2)));
}
#[test]
diff --git a/substrate/core/client/db/src/cache/mod.rs b/substrate/core/client/db/src/cache/mod.rs
index 4d452b37d96..68245a54eee 100644
--- a/substrate/core/client/db/src/cache/mod.rs
+++ b/substrate/core/client/db/src/cache/mod.rs
@@ -294,7 +294,11 @@ impl<Block: BlockT> BlockchainCache<Block> for DbCacheSync<Block> {
Ok(())
}
- fn get_at(&self, key: &CacheKeyId, at: &BlockId<Block>) -> Option<Vec<u8>> {
+ fn get_at(
+ &self,
+ key: &CacheKeyId,
+ at: &BlockId<Block>,
+ ) -> Option<((NumberFor<Block>, Block::Hash), Option<(NumberFor<Block>, Block::Hash)>, Vec<u8>)> {
let cache = self.0.read();
let storage = cache.cache_at.get(key)?.storage();
let db = storage.db();
@@ -318,7 +322,16 @@ impl<Block: BlockT> BlockchainCache<Block> for DbCacheSync<Block> {
},
};
- cache.cache_at.get(key)?.value_at_block(&at).ok()?
+ cache.cache_at
+ .get(key)?
+ .value_at_block(&at)
+ .map(|block_and_value| block_and_value.map(|(begin_block, end_block, value)|
+ (
+ (begin_block.number, begin_block.hash),
+ end_block.map(|end_block| (end_block.number, end_block.hash)),
+ value,
+ )))
+ .ok()?
}
}
diff --git a/substrate/core/client/db/src/light.rs b/substrate/core/client/db/src/light.rs
index 3e60e9e7a1c..2ee2c2ec545 100644
--- a/substrate/core/client/db/src/light.rs
+++ b/substrate/core/client/db/src/light.rs
@@ -562,10 +562,10 @@ pub(crate) mod tests {
header
}
- pub fn insert_block<F: Fn() -> Header>(
+ pub fn insert_block<F: FnMut() -> Header>(
db: &LightStorage<Block>,
cache: HashMap<well_known_cache_keys::Id, Vec<u8>>,
- header: F,
+ mut header: F,
) -> Hash {
let header = header();
let hash = header.hash();
@@ -850,7 +850,7 @@ pub(crate) mod tests {
fn get_authorities(cache: &dyn BlockchainCache<Block>, at: BlockId<Block>) -> Option<Vec<AuthorityId>> {
cache.get_at(&well_known_cache_keys::AUTHORITIES, &at)
- .and_then(|val| Decode::decode(&mut &val[..]).ok())
+ .and_then(|(_, _, val)| Decode::decode(&mut &val[..]).ok())
}
let auth1 = || AuthorityId::from_raw([1u8; 32]);
@@ -1036,7 +1036,12 @@ pub(crate) mod tests {
assert_eq!(db.cache().get_at(b"test", &BlockId::Number(0)), None);
// insert genesis block (no value for cache is provided)
- insert_block(&db, HashMap::new(), || default_header(&Default::default(), 0));
+ let mut genesis_hash = None;
+ insert_block(&db, HashMap::new(), || {
+ let header = default_header(&Default::default(), 0);
+ genesis_hash = Some(header.hash());
+ header
+ });
// after genesis is inserted => None
assert_eq!(db.cache().get_at(b"test", &BlockId::Number(0)), None);
@@ -1045,6 +1050,9 @@ pub(crate) mod tests {
db.cache().initialize(b"test", vec![42]).unwrap();
// after genesis is inserted + cache is initialized => Some
- assert_eq!(db.cache().get_at(b"test", &BlockId::Number(0)), Some(vec![42]));
+ assert_eq!(
+ db.cache().get_at(b"test", &BlockId::Number(0)),
+ Some(((0, genesis_hash.unwrap()), None, vec![42])),
+ );
}
}
diff --git a/substrate/core/client/src/blockchain.rs b/substrate/core/client/src/blockchain.rs
index 2bf61df704d..f4a4f8a4aa9 100644
--- a/substrate/core/client/src/blockchain.rs
+++ b/substrate/core/client/src/blockchain.rs
@@ -106,7 +106,13 @@ pub trait Cache<Block: BlockT>: Send + Sync {
/// Otherwise cache may end up in inconsistent state.
fn initialize(&self, key: &well_known_cache_keys::Id, value_at_genesis: Vec<u8>) -> Result<()>;
/// Returns cached value by the given key.
- fn get_at(&self, key: &well_known_cache_keys::Id, block: &BlockId<Block>) -> Option<Vec<u8>>;
+ ///
+ /// Returned tuple is the range where value has been active and the value itself.
+ fn get_at(
+ &self,
+ key: &well_known_cache_keys::Id,
+ block: &BlockId<Block>,
+ ) -> Option<((NumberFor<Block>, Block::Hash), Option<(NumberFor<Block>, Block::Hash)>, Vec<u8>)>;
}
/// Blockchain info
diff --git a/substrate/core/consensus/aura/src/lib.rs b/substrate/core/consensus/aura/src/lib.rs
index e2e36fdbfba..3d4148c52df 100644
--- a/substrate/core/consensus/aura/src/lib.rs
+++ b/substrate/core/consensus/aura/src/lib.rs
@@ -582,7 +582,7 @@ fn initialize_authorities_cache<A, B, C>(client: &C) -> Result<(), ConsensusErro
let genesis_id = BlockId::Number(Zero::zero());
let genesis_authorities: Option<Vec<A>> = cache
.get_at(&well_known_cache_keys::AUTHORITIES, &genesis_id)
- .and_then(|v| Decode::decode(&mut &v[..]).ok());
+ .and_then(|(_, _, v)| Decode::decode(&mut &v[..]).ok());
if genesis_authorities.is_some() {
return Ok(());
}
@@ -610,7 +610,7 @@ fn authorities<A, B, C>(client: &C, at: &BlockId<B>) -> Result<Vec<A>, Consensus
.cache()
.and_then(|cache| cache
.get_at(&well_known_cache_keys::AUTHORITIES, at)
- .and_then(|v| Decode::decode(&mut &v[..]).ok())
+ .and_then(|(_, _, v)| Decode::decode(&mut &v[..]).ok())
)
.or_else(|| AuraApi::authorities(&*client.runtime_api(), at).ok())
.ok_or_else(|| consensus_common::Error::InvalidAuthoritiesSet.into())
diff --git a/substrate/core/consensus/babe/src/lib.rs b/substrate/core/consensus/babe/src/lib.rs
index e29de64de2b..6d049dd6be1 100644
--- a/substrate/core/consensus/babe/src/lib.rs
+++ b/substrate/core/consensus/babe/src/lib.rs
@@ -952,7 +952,7 @@ fn epoch_from_cache<B, C>(client: &C, at: &BlockId<B>) -> Option<MaybeSpanEpoch>
client.cache()
.and_then(|cache| cache
.get_at(&well_known_cache_keys::EPOCH, at)
- .and_then(|v| Decode::decode(&mut &v[..]).ok()))
+ .and_then(|(_, _, v)| Decode::decode(&mut &v[..]).ok()))
}
/// Extract current epoch from runtime.
@@ -1202,7 +1202,7 @@ fn initialize_authorities_cache<B, C>(client: &C) -> Result<(), ConsensusError>
let genesis_id = BlockId::Number(Zero::zero());
let genesis_epoch: Option<MaybeSpanEpoch> = cache
.get_at(&well_known_cache_keys::EPOCH, &genesis_id)
- .and_then(|v| Decode::decode(&mut &v[..]).ok());
+ .and_then(|(_, _, v)| Decode::decode(&mut &v[..]).ok());
if genesis_epoch.is_some() {
return Ok(());
}
--
GitLab