From 5e7716849a11fe33a8d03236c80b68c7f6fda62b Mon Sep 17 00:00:00 2001 From: arkpar Date: Wed, 19 Jan 2022 17:17:22 +0100 Subject: [PATCH 1/7] Refactored tx storage database scheme --- bin/node/testing/src/bench.rs | 1 - client/cli/src/config.rs | 11 +- client/cli/src/params/database_params.rs | 19 - client/db/src/lib.rs | 423 ++++++++++++----------- client/db/src/parity_db.rs | 18 +- client/db/src/upgrade.rs | 40 ++- client/db/src/utils.rs | 19 +- client/service/src/builder.rs | 14 +- client/service/src/config.rs | 4 +- client/service/src/lib.rs | 1 - client/service/test/src/client/mod.rs | 4 +- client/service/test/src/lib.rs | 3 +- test-utils/client/src/lib.rs | 1 - 13 files changed, 292 insertions(+), 266 deletions(-) diff --git a/bin/node/testing/src/bench.rs b/bin/node/testing/src/bench.rs index 18333280bb8a4..8227582d88a4b 100644 --- a/bin/node/testing/src/bench.rs +++ b/bin/node/testing/src/bench.rs @@ -393,7 +393,6 @@ impl BenchDb { state_pruning: PruningMode::ArchiveAll, source: database_type.into_settings(dir.into()), keep_blocks: sc_client_db::KeepBlocks::All, - transaction_storage: sc_client_db::TransactionStorageMode::BlockBody, }; let task_executor = TaskExecutor::new(); diff --git a/client/cli/src/config.rs b/client/cli/src/config.rs index b5470db91db3a..a40be77f65aa0 100644 --- a/client/cli/src/config.rs +++ b/client/cli/src/config.rs @@ -31,7 +31,7 @@ use sc_service::{ NodeKeyConfig, OffchainWorkerConfig, PrometheusConfig, PruningMode, Role, RpcMethods, TelemetryEndpoints, TransactionPoolOptions, WasmExecutionMethod, }, - ChainSpec, KeepBlocks, TracingReceiver, TransactionStorageMode, + ChainSpec, KeepBlocks, TracingReceiver, }; use sc_tracing::logging::LoggerBuilder; use std::{net::SocketAddr, path::PathBuf}; @@ -198,14 +198,6 @@ pub trait CliConfiguration: Sized { Ok(self.database_params().map(|x| x.database_cache_size()).unwrap_or_default()) } - /// Get the database transaction storage scheme. - fn database_transaction_storage(&self) -> Result { - Ok(self - .database_params() - .map(|x| x.transaction_storage()) - .unwrap_or(TransactionStorageMode::BlockBody)) - } - /// Get the database backend variant. /// /// By default this is retrieved from `DatabaseParams` if it is available. Otherwise its `None`. @@ -519,7 +511,6 @@ pub trait CliConfiguration: Sized { state_cache_child_ratio: self.state_cache_child_ratio()?, state_pruning: self.state_pruning(unsafe_pruning, &role)?, keep_blocks: self.keep_blocks()?, - transaction_storage: self.database_transaction_storage()?, wasm_method: self.wasm_method()?, wasm_runtime_overrides: self.wasm_runtime_overrides(), execution_strategies: self.execution_strategies(is_dev, is_validator)?, diff --git a/client/cli/src/params/database_params.rs b/client/cli/src/params/database_params.rs index dd11c21f432b1..599dded44c61c 100644 --- a/client/cli/src/params/database_params.rs +++ b/client/cli/src/params/database_params.rs @@ -18,7 +18,6 @@ use crate::arg_enums::Database; use clap::Args; -use sc_service::TransactionStorageMode; /// Parameters for block import. #[derive(Debug, Clone, Args)] @@ -36,15 +35,6 @@ pub struct DatabaseParams { /// Limit the memory the database cache can use. #[clap(long = "db-cache", value_name = "MiB")] pub database_cache_size: Option, - - /// Enable storage chain mode - /// - /// This changes the storage format for blocks bodies. - /// If this is enabled, each transaction is stored separately in the - /// transaction database column and is only referenced by hash - /// in the block body column. - #[clap(long)] - pub storage_chain: bool, } impl DatabaseParams { @@ -57,13 +47,4 @@ impl DatabaseParams { pub fn database_cache_size(&self) -> Option { self.database_cache_size } - - /// Transaction storage scheme. - pub fn transaction_storage(&self) -> TransactionStorageMode { - if self.storage_chain { - TransactionStorageMode::StorageChain - } else { - TransactionStorageMode::BlockBody - } - } } diff --git a/client/db/src/lib.rs b/client/db/src/lib.rs index d7550ff9aea17..b291cc059c658 100644 --- a/client/db/src/lib.rs +++ b/client/db/src/lib.rs @@ -112,11 +112,16 @@ pub type DbHash = sp_core::H256; /// This is used as block body when storage-chain mode is enabled. #[derive(Debug, Encode, Decode)] -struct ExtrinsicHeader { - /// Hash of the indexed part - indexed_hash: DbHash, // Zero hash if there's no indexed data - /// The rest of the data. - data: Vec, +enum DbExtrinsic { + /// Extrinsic that contains indexed data. + Indexed { + /// Hash of the indexed part. + hash: DbHash, + /// Extrinsic header. + header: Vec, + }, + /// Complete extrinsic data. + Full(B::Extrinsic), } /// A reference tracking state. @@ -294,8 +299,6 @@ pub struct DatabaseSettings { pub source: DatabaseSource, /// Block pruning mode. pub keep_blocks: KeepBlocks, - /// Block body/Transaction storage scheme. - pub transaction_storage: TransactionStorageMode, } /// Block pruning settings. @@ -307,16 +310,6 @@ pub enum KeepBlocks { Some(u32), } -/// Block body storage scheme. -#[derive(Debug, Clone, Copy)] -pub enum TransactionStorageMode { - /// Store block body as an encoded list of full transactions in the BODY column - BlockBody, - /// Store a list of hashes in the BODY column and each transaction individually - /// in the TRANSACTION column. - StorageChain, -} - /// Where to find the database.. #[derive(Debug, Clone)] pub enum DatabaseSource { @@ -405,6 +398,7 @@ pub(crate) mod columns { pub const OFFCHAIN: u32 = 9; /// Transactions pub const TRANSACTION: u32 = 11; + pub const BODY_INDEX: u32 = 12; } struct PendingBlock { @@ -452,13 +446,11 @@ pub struct BlockchainDb { leaves: RwLock>>, header_metadata_cache: Arc>, header_cache: Mutex>>, - transaction_storage: TransactionStorageMode, } impl BlockchainDb { fn new( db: Arc>, - transaction_storage: TransactionStorageMode, ) -> ClientResult { let meta = read_meta::(&*db, columns::HEADER)?; let leaves = LeafSet::read_from_db(&*db, columns::META, meta_keys::LEAF_PREFIX)?; @@ -468,7 +460,6 @@ impl BlockchainDb { meta: Arc::new(RwLock::new(meta)), header_metadata_cache: Arc::new(HeaderMetadataCache::default()), header_cache: Default::default(), - transaction_storage, }) } @@ -557,59 +548,58 @@ impl sc_client_api::blockchain::HeaderBackend for Blockcha impl sc_client_api::blockchain::Backend for BlockchainDb { fn body(&self, id: BlockId) -> ClientResult>> { - let body = match read_db(&*self.db, columns::KEY_LOOKUP, columns::BODY, id)? { - Some(body) => body, - None => return Ok(None), - }; - match self.transaction_storage { - TransactionStorageMode::BlockBody => match Decode::decode(&mut &body[..]) { - Ok(body) => Ok(Some(body)), + if let Some(body) = read_db(&*self.db, columns::KEY_LOOKUP, columns::BODY, id)? { + // Plain body + match Decode::decode(&mut &body[..]) { + Ok(body) => return Ok(Some(body)), Err(err) => return Err(sp_blockchain::Error::Backend(format!( "Error decoding body: {}", err ))), - }, - TransactionStorageMode::StorageChain => { - match Vec::::decode(&mut &body[..]) { - Ok(index) => { - let extrinsics: ClientResult> = index - .into_iter() - .map(|ExtrinsicHeader { indexed_hash, data }| { - let decode_result = if indexed_hash != Default::default() { - match self.db.get(columns::TRANSACTION, indexed_hash.as_ref()) { - Some(t) => { - let mut input = - utils::join_input(data.as_ref(), t.as_ref()); - Block::Extrinsic::decode(&mut input) - }, - None => - return Err(sp_blockchain::Error::Backend(format!( - "Missing indexed transaction {:?}", - indexed_hash - ))), - } - } else { - Block::Extrinsic::decode(&mut data.as_ref()) + } + } + + if let Some(index) = read_db(&*self.db, columns::KEY_LOOKUP, columns::BODY_INDEX, id)? { + match Vec::>::decode(&mut &index[..]) { + Ok(index) => { + let mut body = Vec::new(); + for ex in index { + match ex { + DbExtrinsic::Indexed { hash, header } => { + match self.db.get(columns::TRANSACTION, hash.as_ref()) { + Some(t) => { + let mut input = + utils::join_input(header.as_ref(), t.as_ref()); + let ex = Block::Extrinsic::decode(&mut input).map_err(|err| + sp_blockchain::Error::Backend(format!( + "Error decoding indexed extrinsic: {}", + err + )))?; + body.push(ex); + }, + None => + return Err(sp_blockchain::Error::Backend(format!( + "Missing indexed transaction {:?}", + hash + ))), }; - decode_result.map_err(|err| { - sp_blockchain::Error::Backend(format!( - "Error decoding extrinsic: {}", - err - )) - }) - }) - .collect(); - Ok(Some(extrinsics?)) - }, - Err(err) => - return Err(sp_blockchain::Error::Backend(format!( - "Error decoding body list: {}", - err - ))), - } - }, + }, + DbExtrinsic::Full(ex) => { + body.push(ex); + } + } + } + return Ok(Some(body)) + }, + Err(err) => + return Err(sp_blockchain::Error::Backend(format!( + "Error decoding body list: {}", + err + ))) + } } + Ok(None) } fn justifications(&self, id: BlockId) -> ClientResult> { @@ -647,37 +637,32 @@ impl sc_client_api::blockchain::Backend for BlockchainDb) -> ClientResult>>> { - match self.transaction_storage { - TransactionStorageMode::BlockBody => Ok(None), - TransactionStorageMode::StorageChain => { - let body = match read_db(&*self.db, columns::KEY_LOOKUP, columns::BODY, id)? { - Some(body) => body, - None => return Ok(None), - }; - match Vec::::decode(&mut &body[..]) { - Ok(index) => { - let mut transactions = Vec::new(); - for ExtrinsicHeader { indexed_hash, .. } in index.into_iter() { - if indexed_hash != Default::default() { - match self.db.get(columns::TRANSACTION, indexed_hash.as_ref()) { - Some(t) => transactions.push(t), - None => - return Err(sp_blockchain::Error::Backend(format!( - "Missing indexed transaction {:?}", - indexed_hash - ))), - } - } + let body = match read_db(&*self.db, columns::KEY_LOOKUP, columns::BODY_INDEX, id)? { + Some(body) => body, + None => return Ok(None), + }; + match Vec::>::decode(&mut &body[..]) { + Ok(index) => { + let mut transactions = Vec::new(); + for ex in index.into_iter() { + if let DbExtrinsic::Indexed { hash, .. } = ex { + match self.db.get(columns::TRANSACTION, hash.as_ref()) { + Some(t) => transactions.push(t), + None => + return Err(sp_blockchain::Error::Backend(format!( + "Missing indexed transaction {:?}", + hash + ))), } - Ok(Some(transactions)) - }, - Err(err) => - return Err(sp_blockchain::Error::Backend(format!( - "Error decoding body list: {}", - err - ))), + } } + Ok(Some(transactions)) }, + Err(err) => + return Err(sp_blockchain::Error::Backend(format!( + "Error decoding body list: {}", + err + ))), } } } @@ -1004,7 +989,6 @@ pub struct Backend { import_lock: Arc>, is_archive: bool, keep_blocks: KeepBlocks, - transaction_storage: TransactionStorageMode, io_stats: FrozenForDuration<(kvdb::IoStats, StateUsageInfo)>, state_usage: Arc, genesis_state: RwLock>>>, @@ -1025,7 +1009,6 @@ impl Backend { Self::new_test_with_tx_storage( keep_blocks, canonicalization_delay, - TransactionStorageMode::BlockBody, ) } @@ -1034,7 +1017,6 @@ impl Backend { pub fn new_test_with_tx_storage( keep_blocks: u32, canonicalization_delay: u64, - transaction_storage: TransactionStorageMode, ) -> Self { let db = kvdb_memorydb::create(crate::utils::NUM_COLUMNS); let db = sp_database::as_database(db); @@ -1044,7 +1026,6 @@ impl Backend { state_pruning: PruningMode::keep_blocks(keep_blocks), source: DatabaseSource::Custom(db), keep_blocks: KeepBlocks::Some(keep_blocks), - transaction_storage, }; Self::new(db_setting, canonicalization_delay).expect("failed to create test-db") @@ -1056,7 +1037,7 @@ impl Backend { config: &DatabaseSettings, ) -> ClientResult { let is_archive_pruning = config.state_pruning.is_archive(); - let blockchain = BlockchainDb::new(db.clone(), config.transaction_storage.clone())?; + let blockchain = BlockchainDb::new(db.clone())?; let map_e = |e: sc_state_db::Error| sp_blockchain::Error::from_state_db(e); let state_db: StateDb<_, _> = StateDb::new( config.state_pruning.clone(), @@ -1082,7 +1063,6 @@ impl Backend { io_stats: FrozenForDuration::new(std::time::Duration::from_secs(1)), state_usage: Arc::new(StateUsageStats::new()), keep_blocks: config.keep_blocks.clone(), - transaction_storage: config.transaction_storage.clone(), genesis_state: RwLock::new(None), }; @@ -1316,26 +1296,18 @@ impl Backend { transaction.set_from_vec(columns::HEADER, &lookup_key, pending_block.header.encode()); if let Some(body) = pending_block.body { - match self.transaction_storage { - TransactionStorageMode::BlockBody => { - transaction.set_from_vec(columns::BODY, &lookup_key, body.encode()); - }, - TransactionStorageMode::StorageChain => { - let body = - apply_index_ops::(&mut transaction, body, operation.index_ops); - transaction.set_from_vec(columns::BODY, &lookup_key, body); - }, + // If we have any index operations we save block in the new format with indexed extrinsic headers + // Otherwise we save the body as a single blob. + if operation.index_ops.is_empty() { + transaction.set_from_vec(columns::BODY, &lookup_key, body.encode()); + } else { + let body = + apply_index_ops::(&mut transaction, body, operation.index_ops)?; + transaction.set_from_vec(columns::BODY_INDEX, &lookup_key, body); } } if let Some(body) = pending_block.indexed_body { - match self.transaction_storage { - TransactionStorageMode::BlockBody => { - debug!(target: "db", "Commit: ignored indexed block body"); - }, - TransactionStorageMode::StorageChain => { - apply_indexed_body::(&mut transaction, body); - }, - } + apply_indexed_body::(&mut transaction, body); } if let Some(justifications) = pending_block.justifications { transaction.set_from_vec( @@ -1672,7 +1644,7 @@ impl Backend { let mut hash = h.clone(); // Follow displaced chains back until we reach a finalized block. // Since leaves are discarded due to finality, they can't have parents - // that are canonical, but not yet finalized. So we stop deletig as soon as + // that are canonical, but not yet finalized. So we stop deleting as soon as // we reach canonical chain. while self.blockchain.hash(number)? != Some(hash.clone()) { let id = BlockId::::hash(hash.clone()); @@ -1695,36 +1667,36 @@ impl Backend { transaction: &mut Transaction, id: BlockId, ) -> ClientResult<()> { - match read_db(&*self.storage.db, columns::KEY_LOOKUP, columns::BODY, id)? { - Some(body) => { - debug!(target: "db", "Removing block #{}", id); - utils::remove_from_db( - transaction, - &*self.storage.db, - columns::KEY_LOOKUP, - columns::BODY, - id, - )?; - match self.transaction_storage { - TransactionStorageMode::BlockBody => {}, - TransactionStorageMode::StorageChain => { - match Vec::::decode(&mut &body[..]) { - Ok(body) => - for ExtrinsicHeader { indexed_hash, .. } in body { - if indexed_hash != Default::default() { - transaction.release(columns::TRANSACTION, indexed_hash); - } - }, - Err(err) => - return Err(sp_blockchain::Error::Backend(format!( - "Error decoding body list: {}", - err - ))), + debug!(target: "db", "Removing block #{}", id); + utils::remove_from_db( + transaction, + &*self.storage.db, + columns::KEY_LOOKUP, + columns::BODY, + id, + )?; + if let Some(index) = read_db(&*self.storage.db, columns::KEY_LOOKUP, columns::BODY_INDEX, id)? { + utils::remove_from_db( + transaction, + &*self.storage.db, + columns::KEY_LOOKUP, + columns::BODY_INDEX, + id, + )?; + match Vec::>::decode(&mut &index[..]) { + Ok(index) => { + for ex in index { + if let DbExtrinsic::Indexed { hash, .. } = ex { + transaction.release(columns::TRANSACTION, hash); } - }, + } } - }, - None => return Ok(()), + Err(err) => + return Err(sp_blockchain::Error::Backend(format!( + "Error decoding body list: {}", + err + ))), + } } Ok(()) } @@ -1765,8 +1737,8 @@ fn apply_index_ops( transaction: &mut Transaction, body: Vec, ops: Vec, -) -> Vec { - let mut extrinsic_headers: Vec = Vec::with_capacity(body.len()); +) -> ClientResult> { + let mut extrinsic_index: Vec> = Vec::with_capacity(body.len()); let mut index_map = HashMap::new(); let mut renewed_map = HashMap::new(); for op in ops { @@ -1780,37 +1752,46 @@ fn apply_index_ops( } } for (index, extrinsic) in body.into_iter().enumerate() { - let extrinsic = extrinsic.encode(); - let extrinsic_header = if let Some(hash) = renewed_map.get(&(index as u32)) { + let db_extrinsic = if let Some(hash) = renewed_map.get(&(index as u32)) { // Bump ref counter + let extrinsic = extrinsic.encode(); transaction.reference(columns::TRANSACTION, DbHash::from_slice(hash.as_ref())); - ExtrinsicHeader { indexed_hash: hash.clone(), data: extrinsic } + DbExtrinsic::Indexed { hash: hash.clone(), header: extrinsic } } else { match index_map.get(&(index as u32)) { - Some((hash, size)) if *size as usize <= extrinsic.len() => { - let offset = extrinsic.len() - *size as usize; - transaction.store( - columns::TRANSACTION, - DbHash::from_slice(hash.as_ref()), - extrinsic[offset..].to_vec(), - ); - ExtrinsicHeader { - indexed_hash: DbHash::from_slice(hash.as_ref()), - data: extrinsic[..offset].to_vec(), + Some((hash, size)) => { + let extrinsic = extrinsic.encode(); + if *size as usize <= extrinsic.len() { + let offset = extrinsic.len() - *size as usize; + transaction.store( + columns::TRANSACTION, + DbHash::from_slice(hash.as_ref()), + extrinsic[offset..].to_vec(), + ); + DbExtrinsic::Indexed { + hash: DbHash::from_slice(hash.as_ref()), + header: extrinsic[..offset].to_vec(), + } + } else { + return Err(sp_blockchain::Error::Backend(format!( + "Error indexing extrinsic {}, invalid size. {} > {}", + DbHash::from_slice(hash.as_ref()), size, extrinsic.len(), + ))); } }, - _ => ExtrinsicHeader { indexed_hash: Default::default(), data: extrinsic }, + _ => DbExtrinsic::Full(extrinsic), } }; - extrinsic_headers.push(extrinsic_header); + extrinsic_index.push(db_extrinsic); } debug!( target: "db", - "DB transaction index: {} inserted, {} renewed", + "DB transaction index: {} inserted, {} renewed, {} full", index_map.len(), - renewed_map.len() + renewed_map.len(), + extrinsic_index.len() - index_map.len() - renewed_map.len(), ); - extrinsic_headers.encode() + Ok(extrinsic_index.encode()) } fn apply_indexed_body(transaction: &mut Transaction, body: Vec>) { @@ -2375,7 +2356,6 @@ pub(crate) mod tests { state_pruning: PruningMode::keep_blocks(1), source: DatabaseSource::Custom(backing), keep_blocks: KeepBlocks::All, - transaction_storage: TransactionStorageMode::BlockBody, }, 0, ) @@ -3045,46 +3025,44 @@ pub(crate) mod tests { #[test] fn prune_blocks_on_finalize() { - for storage in &[TransactionStorageMode::BlockBody, TransactionStorageMode::StorageChain] { - let backend = Backend::::new_test_with_tx_storage(2, 0, *storage); - let mut blocks = Vec::new(); - let mut prev_hash = Default::default(); - for i in 0..5 { - let hash = insert_block( - &backend, - i, - prev_hash, - None, - Default::default(), - vec![i.into()], - None, - ) - .unwrap(); - blocks.push(hash); - prev_hash = hash; - } + let backend = Backend::::new_test_with_tx_storage(2, 0); + let mut blocks = Vec::new(); + let mut prev_hash = Default::default(); + for i in 0..5 { + let hash = insert_block( + &backend, + i, + prev_hash, + None, + Default::default(), + vec![i.into()], + None, + ) + .unwrap(); + blocks.push(hash); + prev_hash = hash; + } - { - let mut op = backend.begin_operation().unwrap(); - backend.begin_state_operation(&mut op, BlockId::Hash(blocks[4])).unwrap(); - for i in 1..5 { - op.mark_finalized(BlockId::Hash(blocks[i]), None).unwrap(); - } - backend.commit_operation(op).unwrap(); + { + let mut op = backend.begin_operation().unwrap(); + backend.begin_state_operation(&mut op, BlockId::Hash(blocks[4])).unwrap(); + for i in 1..5 { + op.mark_finalized(BlockId::Hash(blocks[i]), None).unwrap(); } - let bc = backend.blockchain(); - assert_eq!(None, bc.body(BlockId::hash(blocks[0])).unwrap()); - assert_eq!(None, bc.body(BlockId::hash(blocks[1])).unwrap()); - assert_eq!(None, bc.body(BlockId::hash(blocks[2])).unwrap()); - assert_eq!(Some(vec![3.into()]), bc.body(BlockId::hash(blocks[3])).unwrap()); - assert_eq!(Some(vec![4.into()]), bc.body(BlockId::hash(blocks[4])).unwrap()); + backend.commit_operation(op).unwrap(); } + let bc = backend.blockchain(); + assert_eq!(None, bc.body(BlockId::hash(blocks[0])).unwrap()); + assert_eq!(None, bc.body(BlockId::hash(blocks[1])).unwrap()); + assert_eq!(None, bc.body(BlockId::hash(blocks[2])).unwrap()); + assert_eq!(Some(vec![3.into()]), bc.body(BlockId::hash(blocks[3])).unwrap()); + assert_eq!(Some(vec![4.into()]), bc.body(BlockId::hash(blocks[4])).unwrap()); } #[test] fn prune_blocks_on_finalize_with_fork() { let backend = - Backend::::new_test_with_tx_storage(2, 10, TransactionStorageMode::StorageChain); + Backend::::new_test_with_tx_storage(2, 10); let mut blocks = Vec::new(); let mut prev_hash = Default::default(); for i in 0..5 { @@ -3143,10 +3121,53 @@ pub(crate) mod tests { assert_eq!(Some(vec![4.into()]), bc.body(BlockId::hash(blocks[4])).unwrap()); } + #[test] + fn indexed_data_block_body() { + let backend = + Backend::::new_test_with_tx_storage(1, 10); + + let x0 = ExtrinsicWrapper::from(0u64).encode(); + let x1 = ExtrinsicWrapper::from(1u64).encode(); + let x0_hash = as sp_core::Hasher>::hash(&x0[1..]); + let x1_hash = as sp_core::Hasher>::hash(&x1[1..]); + let index = vec![ + IndexOperation::Insert { + extrinsic: 0, + hash: x0_hash.as_ref().to_vec(), + size: (x0.len() - 1) as u32, + }, + IndexOperation::Insert { + extrinsic: 1, + hash: x1_hash.as_ref().to_vec(), + size: (x1.len() - 1) as u32, + }, + ]; + let hash = insert_block( + &backend, + 0, + Default::default(), + None, + Default::default(), + vec![0u64.into(), 1u64.into()], + Some(index), + ) + .unwrap(); + let bc = backend.blockchain(); + assert_eq!(bc.indexed_transaction(&x0_hash).unwrap().unwrap(), &x0[1..]); + assert_eq!(bc.indexed_transaction(&x1_hash).unwrap().unwrap(), &x1[1..]); + + // Push one more blocks and make sure block is pruned and transaction index is cleared. + insert_block(&backend, 1, hash, None, Default::default(), vec![], None).unwrap(); + backend.finalize_block(BlockId::Number(1), None).unwrap(); + assert_eq!(bc.body(BlockId::Number(0)).unwrap(), None); + assert_eq!(bc.indexed_transaction(&x0_hash).unwrap(), None); + assert_eq!(bc.indexed_transaction(&x1_hash).unwrap(), None); + } + #[test] fn renew_transaction_storage() { let backend = - Backend::::new_test_with_tx_storage(2, 10, TransactionStorageMode::StorageChain); + Backend::::new_test_with_tx_storage(2, 10); let mut blocks = Vec::new(); let mut prev_hash = Default::default(); let x1 = ExtrinsicWrapper::from(0u64).encode(); @@ -3194,7 +3215,7 @@ pub(crate) mod tests { #[test] fn remove_leaf_block_works() { let backend = - Backend::::new_test_with_tx_storage(2, 10, TransactionStorageMode::StorageChain); + Backend::::new_test_with_tx_storage(2, 10); let mut blocks = Vec::new(); let mut prev_hash = Default::default(); for i in 0..2 { diff --git a/client/db/src/parity_db.rs b/client/db/src/parity_db.rs index ebe2988e27f04..1ea40679b4e44 100644 --- a/client/db/src/parity_db.rs +++ b/client/db/src/parity_db.rs @@ -38,20 +38,22 @@ pub fn open>( path: &std::path::Path, db_type: DatabaseType, create: bool, + upgrade: bool, ) -> parity_db::Result>> { let mut config = parity_db::Options::with_columns(path, NUM_COLUMNS as u8); match db_type { DatabaseType::Full => { - let indexes = [ + let compressed = [ columns::STATE, columns::HEADER, columns::BODY, + columns::BODY_INDEX, columns::TRANSACTION, columns::JUSTIFICATIONS, ]; - for i in indexes { + for i in compressed { let mut column = &mut config.columns[i as usize]; column.compression = parity_db::CompressionType::Lz4; } @@ -60,9 +62,21 @@ pub fn open>( state_col.ref_counted = true; state_col.preimage = true; state_col.uniform = true; + + let mut tx_col = &mut config.columns[columns::TRANSACTION as usize]; + tx_col.ref_counted = true; + tx_col.preimage = true; + tx_col.uniform = true; }, } + if upgrade { + log::info!("Upgrading metadata"); + if let Some(meta) = parity_db::Options::load_metadata(path)? { + config.write_metadata(path, &meta.salt)?; + } + } + let db = if create { parity_db::Db::open_or_create(&config)? } else { diff --git a/client/db/src/upgrade.rs b/client/db/src/upgrade.rs index a42a1960a57c3..b39d6291eee4e 100644 --- a/client/db/src/upgrade.rs +++ b/client/db/src/upgrade.rs @@ -33,11 +33,12 @@ use sp_runtime::traits::Block as BlockT; const VERSION_FILE_NAME: &'static str = "db_version"; /// Current db version. -const CURRENT_VERSION: u32 = 3; +const CURRENT_VERSION: u32 = 4; /// Number of columns in v1. const V1_NUM_COLUMNS: u32 = 11; const V2_NUM_COLUMNS: u32 = 12; +const V3_NUM_COLUMNS: u32 = 12; /// Database upgrade errors. #[derive(Debug)] @@ -68,7 +69,7 @@ impl fmt::Display for UpgradeError { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { UpgradeError::UnknownDatabaseVersion => { - write!(f, "Database version cannot be read from exisiting db_version file") + write!(f, "Database version cannot be read from existing db_version file") }, UpgradeError::MissingDatabaseVersionFile => write!(f, "Missing database version file"), UpgradeError::UnsupportedVersion(version) => { @@ -92,9 +93,16 @@ pub fn upgrade_db(db_path: &Path, db_type: DatabaseType) -> Upgra 0 => return Err(UpgradeError::UnsupportedVersion(db_version)), 1 => { migrate_1_to_2::(db_path, db_type)?; - migrate_2_to_3::(db_path, db_type)? + migrate_2_to_3::(db_path, db_type)?; + migrate_3_to_4::(db_path, db_type)?; }, - 2 => migrate_2_to_3::(db_path, db_type)?, + 2 => { + migrate_2_to_3::(db_path, db_type)?; + migrate_3_to_4::(db_path, db_type)?; + }, + 3 => { + migrate_3_to_4::(db_path, db_type)?; + } CURRENT_VERSION => (), _ => return Err(UpgradeError::FutureDatabaseVersion(db_version)), } @@ -139,6 +147,15 @@ fn migrate_2_to_3(db_path: &Path, _db_type: DatabaseType) -> Upgr Ok(()) } +/// Migration from version3 to version4: +/// 1) the number of columns has changed from 12 to 13; +/// 2) BODY_INDEX column is added; +fn migrate_3_to_4(db_path: &Path, _db_type: DatabaseType) -> UpgradeResult<()> { + let db_cfg = DatabaseConfig::with_columns(V3_NUM_COLUMNS); + let db = Database::open(&db_cfg, db_path)?; + db.add_column().map_err(Into::into) +} + /// Reads current database version from the file at given path. /// If the file does not exist returns 0. fn current_version(path: &Path) -> UpgradeResult { @@ -174,7 +191,7 @@ fn version_file_path(path: &Path) -> PathBuf { mod tests { use super::*; use crate::{ - tests::Block, DatabaseSettings, DatabaseSource, KeepBlocks, TransactionStorageMode, + tests::Block, DatabaseSettings, DatabaseSource, KeepBlocks, }; use sc_state_db::PruningMode; @@ -194,7 +211,6 @@ mod tests { state_pruning: PruningMode::ArchiveAll, source: DatabaseSource::RocksDb { path: db_path.to_owned(), cache_size: 128 }, keep_blocks: KeepBlocks::All, - transaction_storage: TransactionStorageMode::BlockBody, }, db_type, ) @@ -229,4 +245,16 @@ mod tests { assert_eq!(current_version(&db_path).unwrap(), CURRENT_VERSION); } } + + #[test] + fn upgrade_to_4_works() { + let db_type = DatabaseType::Full; + for version_from_file in &[None, Some(1), Some(2), Some(3)] { + let db_dir = tempfile::TempDir::new().unwrap(); + let db_path = db_dir.path().join(db_type.as_str()); + create_db(&db_path, *version_from_file); + open_database(&db_path, db_type).unwrap(); + assert_eq!(current_version(&db_path).unwrap(), CURRENT_VERSION); + } + } } diff --git a/client/db/src/utils.rs b/client/db/src/utils.rs index c3a60962a67da..b118970c042c4 100644 --- a/client/db/src/utils.rs +++ b/client/db/src/utils.rs @@ -40,7 +40,7 @@ use sp_trie::DBValue; feature = "test-helpers", test ))] -pub const NUM_COLUMNS: u32 = 12; +pub const NUM_COLUMNS: u32 = 13; /// Meta column. The set of keys in the column is shared by full && light storages. pub const COLUMN_META: u32 = 0; @@ -252,7 +252,7 @@ impl From for sp_blockchain::Error { #[cfg(feature = "with-parity-db")] impl From for OpenDbError { fn from(err: parity_db::Error) -> Self { - if err.to_string().contains("use open_or_create") { + if matches!(err, parity_db::Error::DatabaseNotFound) { OpenDbError::DoesNotExist } else { OpenDbError::Internal(err.to_string()) @@ -272,8 +272,16 @@ impl From for OpenDbError { #[cfg(feature = "with-parity-db")] fn open_parity_db(path: &Path, db_type: DatabaseType, create: bool) -> OpenDbResult { - let db = crate::parity_db::open(path, db_type, create)?; - Ok(db) + match crate::parity_db::open(path, db_type, create, false) { + Ok(db) => Ok(db), + Err(parity_db::Error::InvalidConfiguration(_)) => { + // Try to update the database with the new config + Ok(crate::parity_db::open(path, db_type, create, true)?) + } + Err(e) => { + Err(e.into()) + }, + } } #[cfg(not(feature = "with-parity-db"))] @@ -573,7 +581,7 @@ impl<'a, 'b> codec::Input for JoinInput<'a, 'b> { #[cfg(test)] mod tests { use super::*; - use crate::{KeepBlocks, TransactionStorageMode}; + use crate::KeepBlocks; use codec::Input; use sc_state_db::PruningMode; use sp_runtime::testing::{Block as RawBlock, ExtrinsicWrapper}; @@ -689,7 +697,6 @@ mod tests { state_pruning: PruningMode::ArchiveAll, source, keep_blocks: KeepBlocks::All, - transaction_storage: TransactionStorageMode::BlockBody, } } diff --git a/client/service/src/builder.rs b/client/service/src/builder.rs index bf681aec94c7d..e9c1691107c71 100644 --- a/client/service/src/builder.rs +++ b/client/service/src/builder.rs @@ -19,7 +19,7 @@ use crate::{ build_network_future, client::{Client, ClientConfig}, - config::{Configuration, KeystoreConfig, PrometheusConfig, TransactionStorageMode}, + config::{Configuration, KeystoreConfig, PrometheusConfig}, error::Error, metrics::MetricsService, start_rpc_servers, RpcHandlers, SpawnTaskHandle, TaskManager, TransactionPoolAdapter, @@ -39,7 +39,7 @@ use sc_executor::RuntimeVersionOf; use sc_keystore::LocalKeystore; use sc_network::{ block_request_handler::{self, BlockRequestHandler}, - config::{Role, SyncMode}, + config::Role, light_client_requests::{self, handler::LightClientRequestHandler}, state_request_handler::{self, StateRequestHandler}, warp_request_handler::{self, RequestHandler as WarpSyncRequestHandler, WarpSyncProvider}, @@ -264,7 +264,6 @@ where state_pruning: config.state_pruning.clone(), source: config.database.clone(), keep_blocks: config.keep_blocks.clone(), - transaction_storage: config.transaction_storage.clone(), }; let backend = new_db_backend(db_config)?; @@ -842,7 +841,7 @@ where } }; - let mut network_params = sc_network::config::Params { + let network_params = sc_network::config::Params { role: config.role.clone(), executor: { let spawn_handle = Clone::clone(&spawn_handle); @@ -869,13 +868,6 @@ where light_client_request_protocol_config, }; - // Storage chains don't keep full block history and can't be synced in full mode. - // Force fast sync when storage chain mode is enabled. - if matches!(config.transaction_storage, TransactionStorageMode::StorageChain) { - network_params.network_config.sync_mode = - SyncMode::Fast { storage_chain_mode: true, skip_proofs: false }; - } - let has_bootnodes = !network_params.network_config.boot_nodes.is_empty(); let network_mut = sc_network::NetworkWorker::new(network_params)?; let network = network_mut.service().clone(); diff --git a/client/service/src/config.rs b/client/service/src/config.rs index 45a6f832f8ee1..fd32aebdebdd8 100644 --- a/client/service/src/config.rs +++ b/client/service/src/config.rs @@ -19,7 +19,7 @@ //! Service configuration. pub use sc_client_api::execution_extensions::{ExecutionStrategies, ExecutionStrategy}; -pub use sc_client_db::{Database, DatabaseSource, KeepBlocks, PruningMode, TransactionStorageMode}; +pub use sc_client_db::{Database, DatabaseSource, KeepBlocks, PruningMode}; pub use sc_executor::WasmExecutionMethod; pub use sc_network::{ config::{ @@ -71,8 +71,6 @@ pub struct Configuration { pub state_pruning: PruningMode, /// Number of blocks to keep in the db. pub keep_blocks: KeepBlocks, - /// Transaction storage scheme. - pub transaction_storage: TransactionStorageMode, /// Chain configuration. pub chain_spec: Box, /// Wasm execution method. diff --git a/client/service/src/lib.rs b/client/service/src/lib.rs index 430a818c0f47c..eb8dc64aeafac 100644 --- a/client/service/src/lib.rs +++ b/client/service/src/lib.rs @@ -58,7 +58,6 @@ pub use self::{ }; pub use config::{ BasePath, Configuration, DatabaseSource, KeepBlocks, PruningMode, Role, RpcMethods, TaskType, - TransactionStorageMode, }; pub use sc_chain_spec::{ ChainSpec, ChainType, Extension as ChainSpecExtension, GenericChainSpec, NoExtension, diff --git a/client/service/test/src/client/mod.rs b/client/service/test/src/client/mod.rs index 2b0ea460c4dd3..553c20f2e2f03 100644 --- a/client/service/test/src/client/mod.rs +++ b/client/service/test/src/client/mod.rs @@ -24,7 +24,7 @@ use sc_client_api::{ in_mem, BlockBackend, BlockchainEvents, FinalityNotifications, StorageProvider, }; use sc_client_db::{ - Backend, DatabaseSettings, DatabaseSource, KeepBlocks, PruningMode, TransactionStorageMode, + Backend, DatabaseSettings, DatabaseSource, KeepBlocks, PruningMode, }; use sc_consensus::{ BlockCheckParams, BlockImport, BlockImportParams, ForkChoiceStrategy, ImportResult, @@ -1203,7 +1203,6 @@ fn doesnt_import_blocks_that_revert_finality() { state_cache_child_ratio: None, state_pruning: PruningMode::ArchiveAll, keep_blocks: KeepBlocks::All, - transaction_storage: TransactionStorageMode::BlockBody, source: DatabaseSource::RocksDb { path: tmp.path().into(), cache_size: 1024 }, }, u64::MAX, @@ -1419,7 +1418,6 @@ fn returns_status_for_pruned_blocks() { state_cache_child_ratio: None, state_pruning: PruningMode::keep_blocks(1), keep_blocks: KeepBlocks::All, - transaction_storage: TransactionStorageMode::BlockBody, source: DatabaseSource::RocksDb { path: tmp.path().into(), cache_size: 1024 }, }, u64::MAX, diff --git a/client/service/test/src/lib.rs b/client/service/test/src/lib.rs index 67b33dfd55d13..c492ed30f7d91 100644 --- a/client/service/test/src/lib.rs +++ b/client/service/test/src/lib.rs @@ -30,7 +30,7 @@ use sc_service::{ client::Client, config::{BasePath, DatabaseSource, KeystoreConfig}, ChainSpecExtension, Configuration, Error, GenericChainSpec, KeepBlocks, Role, RuntimeGenesis, - SpawnTaskHandle, TaskManager, TransactionStorageMode, + SpawnTaskHandle, TaskManager, }; use sc_transaction_pool_api::TransactionPool; use sp_api::BlockId; @@ -235,7 +235,6 @@ fn node_config< state_cache_child_ratio: None, state_pruning: Default::default(), keep_blocks: KeepBlocks::All, - transaction_storage: TransactionStorageMode::BlockBody, chain_spec: Box::new((*spec).clone()), wasm_method: sc_service::config::WasmExecutionMethod::Interpreted, wasm_runtime_overrides: Default::default(), diff --git a/test-utils/client/src/lib.rs b/test-utils/client/src/lib.rs index 2779656558826..4d4ed8f1e4ca4 100644 --- a/test-utils/client/src/lib.rs +++ b/test-utils/client/src/lib.rs @@ -108,7 +108,6 @@ impl let backend = Arc::new(Backend::new_test_with_tx_storage( keep_blocks, 0, - sc_client_db::TransactionStorageMode::StorageChain, )); Self::with_backend(backend) } From 60a9a69d22223f41565a4eadb22f1b24fe964fdf Mon Sep 17 00:00:00 2001 From: arkpar Date: Wed, 2 Feb 2022 16:04:56 +0100 Subject: [PATCH 2/7] Bump parity-db --- Cargo.lock | 10 +++++----- client/db/Cargo.toml | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a45b3bf3289cc..c1d6695c87f43 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6462,9 +6462,9 @@ dependencies = [ [[package]] name = "parity-db" -version = "0.3.5" +version = "0.3.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "78a95abf24f1097c6e3181abbbbfc3630b3b5e681470940f719b69acb4911c7f" +checksum = "68de01cff53da5574397233383dd7f5c15ee958c348245765ea8cb09f2571e6b" dependencies = [ "blake2-rfc", "crc32fast", @@ -10935,7 +10935,7 @@ dependencies = [ "chrono", "lazy_static", "matchers", - "parking_lot 0.11.2", + "parking_lot 0.9.0", "regex", "serde", "serde_json", @@ -11108,8 +11108,8 @@ version = "1.6.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "4ee73e6e4924fe940354b8d4d98cad5231175d615cd855b758adc658c0aac6a0" dependencies = [ - "cfg-if 1.0.0", - "rand 0.8.4", + "cfg-if 0.1.10", + "rand 0.6.5", "static_assertions", ] diff --git a/client/db/Cargo.toml b/client/db/Cargo.toml index 0db0f8309e2eb..35d802dd2b35d 100644 --- a/client/db/Cargo.toml +++ b/client/db/Cargo.toml @@ -33,7 +33,7 @@ sc-state-db = { version = "0.10.0-dev", path = "../state-db" } sp-trie = { version = "4.0.0", path = "../../primitives/trie" } sp-blockchain = { version = "4.0.0-dev", path = "../../primitives/blockchain" } sp-database = { version = "4.0.0-dev", path = "../../primitives/database" } -parity-db = { version = "0.3.5", optional = true } +parity-db = { version = "0.3.6", optional = true } [dev-dependencies] sp-tracing = { version = "4.0.0", path = "../../primitives/tracing" } From 5e8bfa5d2f7dc072d825db7d4d0695f03d7dabdb Mon Sep 17 00:00:00 2001 From: arkpar Date: Wed, 2 Feb 2022 16:33:17 +0100 Subject: [PATCH 3/7] fmt --- bin/node/cli/benches/block_production.rs | 3 +- bin/node/cli/benches/transaction_pool.rs | 3 +- client/db/src/lib.rs | 72 +++++++++++------------- client/db/src/upgrade.rs | 6 +- client/db/src/utils.rs | 4 +- client/service/test/src/client/mod.rs | 4 +- test-utils/client/src/lib.rs | 5 +- 7 files changed, 40 insertions(+), 57 deletions(-) diff --git a/bin/node/cli/benches/block_production.rs b/bin/node/cli/benches/block_production.rs index 69e9e0076a165..77b51fa28dd1a 100644 --- a/bin/node/cli/benches/block_production.rs +++ b/bin/node/cli/benches/block_production.rs @@ -29,7 +29,7 @@ use sc_consensus::{ use sc_service::{ config::{ DatabaseSource, KeepBlocks, KeystoreConfig, NetworkConfiguration, OffchainWorkerConfig, - PruningMode, TransactionStorageMode, WasmExecutionMethod, + PruningMode, WasmExecutionMethod, }, BasePath, Configuration, Role, }; @@ -76,7 +76,6 @@ fn new_node(tokio_handle: Handle) -> node_cli::service::NewFullBase { state_cache_child_ratio: None, state_pruning: PruningMode::ArchiveAll, keep_blocks: KeepBlocks::All, - transaction_storage: TransactionStorageMode::BlockBody, chain_spec: spec, wasm_method: WasmExecutionMethod::Compiled, execution_strategies: ExecutionStrategies { diff --git a/bin/node/cli/benches/transaction_pool.rs b/bin/node/cli/benches/transaction_pool.rs index 9baa3e7fc117d..e89527f2333a2 100644 --- a/bin/node/cli/benches/transaction_pool.rs +++ b/bin/node/cli/benches/transaction_pool.rs @@ -25,7 +25,7 @@ use sc_client_api::execution_extensions::ExecutionStrategies; use sc_service::{ config::{ DatabaseSource, KeepBlocks, KeystoreConfig, NetworkConfiguration, OffchainWorkerConfig, - PruningMode, TransactionPoolOptions, TransactionStorageMode, WasmExecutionMethod, + PruningMode, TransactionPoolOptions, WasmExecutionMethod, }, BasePath, Configuration, Role, }; @@ -67,7 +67,6 @@ fn new_node(tokio_handle: Handle) -> node_cli::service::NewFullBase { state_cache_child_ratio: None, state_pruning: PruningMode::ArchiveAll, keep_blocks: KeepBlocks::All, - transaction_storage: TransactionStorageMode::BlockBody, chain_spec: spec, wasm_method: WasmExecutionMethod::Interpreted, // NOTE: we enforce the use of the native runtime to make the errors more debuggable diff --git a/client/db/src/lib.rs b/client/db/src/lib.rs index b291cc059c658..e23505fd089a8 100644 --- a/client/db/src/lib.rs +++ b/client/db/src/lib.rs @@ -449,9 +449,7 @@ pub struct BlockchainDb { } impl BlockchainDb { - fn new( - db: Arc>, - ) -> ClientResult { + fn new(db: Arc>) -> ClientResult { let meta = read_meta::(&*db, columns::HEADER)?; let leaves = LeafSet::read_from_db(&*db, columns::META, meta_keys::LEAF_PREFIX)?; Ok(BlockchainDb { @@ -571,32 +569,35 @@ impl sc_client_api::blockchain::Backend for BlockchainDb { let mut input = utils::join_input(header.as_ref(), t.as_ref()); - let ex = Block::Extrinsic::decode(&mut input).map_err(|err| - sp_blockchain::Error::Backend(format!( - "Error decoding indexed extrinsic: {}", - err - )))?; + let ex = Block::Extrinsic::decode(&mut input).map_err( + |err| { + sp_blockchain::Error::Backend(format!( + "Error decoding indexed extrinsic: {}", + err + )) + }, + )?; body.push(ex); }, None => return Err(sp_blockchain::Error::Backend(format!( - "Missing indexed transaction {:?}", - hash - ))), + "Missing indexed transaction {:?}", + hash + ))), }; }, DbExtrinsic::Full(ex) => { body.push(ex); - } + }, } } return Ok(Some(body)) }, Err(err) => return Err(sp_blockchain::Error::Backend(format!( - "Error decoding body list: {}", - err - ))) + "Error decoding body list: {}", + err + ))), } } Ok(None) @@ -1006,18 +1007,12 @@ impl Backend { /// Create new memory-backed client backend for tests. #[cfg(any(test, feature = "test-helpers"))] pub fn new_test(keep_blocks: u32, canonicalization_delay: u64) -> Self { - Self::new_test_with_tx_storage( - keep_blocks, - canonicalization_delay, - ) + Self::new_test_with_tx_storage(keep_blocks, canonicalization_delay) } /// Create new memory-backed client backend for tests. #[cfg(any(test, feature = "test-helpers"))] - pub fn new_test_with_tx_storage( - keep_blocks: u32, - canonicalization_delay: u64, - ) -> Self { + pub fn new_test_with_tx_storage(keep_blocks: u32, canonicalization_delay: u64) -> Self { let db = kvdb_memorydb::create(crate::utils::NUM_COLUMNS); let db = sp_database::as_database(db); let db_setting = DatabaseSettings { @@ -1296,8 +1291,8 @@ impl Backend { transaction.set_from_vec(columns::HEADER, &lookup_key, pending_block.header.encode()); if let Some(body) = pending_block.body { - // If we have any index operations we save block in the new format with indexed extrinsic headers - // Otherwise we save the body as a single blob. + // If we have any index operations we save block in the new format with indexed + // extrinsic headers Otherwise we save the body as a single blob. if operation.index_ops.is_empty() { transaction.set_from_vec(columns::BODY, &lookup_key, body.encode()); } else { @@ -1675,7 +1670,9 @@ impl Backend { columns::BODY, id, )?; - if let Some(index) = read_db(&*self.storage.db, columns::KEY_LOOKUP, columns::BODY_INDEX, id)? { + if let Some(index) = + read_db(&*self.storage.db, columns::KEY_LOOKUP, columns::BODY_INDEX, id)? + { utils::remove_from_db( transaction, &*self.storage.db, @@ -1684,13 +1681,12 @@ impl Backend { id, )?; match Vec::>::decode(&mut &index[..]) { - Ok(index) => { + Ok(index) => for ex in index { if let DbExtrinsic::Indexed { hash, .. } = ex { transaction.release(columns::TRANSACTION, hash); } - } - } + }, Err(err) => return Err(sp_blockchain::Error::Backend(format!( "Error decoding body list: {}", @@ -1775,8 +1771,10 @@ fn apply_index_ops( } else { return Err(sp_blockchain::Error::Backend(format!( "Error indexing extrinsic {}, invalid size. {} > {}", - DbHash::from_slice(hash.as_ref()), size, extrinsic.len(), - ))); + DbHash::from_slice(hash.as_ref()), + size, + extrinsic.len(), + ))) } }, _ => DbExtrinsic::Full(extrinsic), @@ -3061,8 +3059,7 @@ pub(crate) mod tests { #[test] fn prune_blocks_on_finalize_with_fork() { - let backend = - Backend::::new_test_with_tx_storage(2, 10); + let backend = Backend::::new_test_with_tx_storage(2, 10); let mut blocks = Vec::new(); let mut prev_hash = Default::default(); for i in 0..5 { @@ -3123,8 +3120,7 @@ pub(crate) mod tests { #[test] fn indexed_data_block_body() { - let backend = - Backend::::new_test_with_tx_storage(1, 10); + let backend = Backend::::new_test_with_tx_storage(1, 10); let x0 = ExtrinsicWrapper::from(0u64).encode(); let x1 = ExtrinsicWrapper::from(1u64).encode(); @@ -3166,8 +3162,7 @@ pub(crate) mod tests { #[test] fn renew_transaction_storage() { - let backend = - Backend::::new_test_with_tx_storage(2, 10); + let backend = Backend::::new_test_with_tx_storage(2, 10); let mut blocks = Vec::new(); let mut prev_hash = Default::default(); let x1 = ExtrinsicWrapper::from(0u64).encode(); @@ -3214,8 +3209,7 @@ pub(crate) mod tests { #[test] fn remove_leaf_block_works() { - let backend = - Backend::::new_test_with_tx_storage(2, 10); + let backend = Backend::::new_test_with_tx_storage(2, 10); let mut blocks = Vec::new(); let mut prev_hash = Default::default(); for i in 0..2 { diff --git a/client/db/src/upgrade.rs b/client/db/src/upgrade.rs index b39d6291eee4e..ec91a753ed870 100644 --- a/client/db/src/upgrade.rs +++ b/client/db/src/upgrade.rs @@ -102,7 +102,7 @@ pub fn upgrade_db(db_path: &Path, db_type: DatabaseType) -> Upgra }, 3 => { migrate_3_to_4::(db_path, db_type)?; - } + }, CURRENT_VERSION => (), _ => return Err(UpgradeError::FutureDatabaseVersion(db_version)), } @@ -190,9 +190,7 @@ fn version_file_path(path: &Path) -> PathBuf { #[cfg(test)] mod tests { use super::*; - use crate::{ - tests::Block, DatabaseSettings, DatabaseSource, KeepBlocks, - }; + use crate::{tests::Block, DatabaseSettings, DatabaseSource, KeepBlocks}; use sc_state_db::PruningMode; fn create_db(db_path: &Path, version: Option) { diff --git a/client/db/src/utils.rs b/client/db/src/utils.rs index b118970c042c4..7dcb6676a1750 100644 --- a/client/db/src/utils.rs +++ b/client/db/src/utils.rs @@ -277,10 +277,8 @@ fn open_parity_db(path: &Path, db_type: DatabaseType, create: boo Err(parity_db::Error::InvalidConfiguration(_)) => { // Try to update the database with the new config Ok(crate::parity_db::open(path, db_type, create, true)?) - } - Err(e) => { - Err(e.into()) }, + Err(e) => Err(e.into()), } } diff --git a/client/service/test/src/client/mod.rs b/client/service/test/src/client/mod.rs index 553c20f2e2f03..c86b23347d417 100644 --- a/client/service/test/src/client/mod.rs +++ b/client/service/test/src/client/mod.rs @@ -23,9 +23,7 @@ use sc_block_builder::BlockBuilderProvider; use sc_client_api::{ in_mem, BlockBackend, BlockchainEvents, FinalityNotifications, StorageProvider, }; -use sc_client_db::{ - Backend, DatabaseSettings, DatabaseSource, KeepBlocks, PruningMode, -}; +use sc_client_db::{Backend, DatabaseSettings, DatabaseSource, KeepBlocks, PruningMode}; use sc_consensus::{ BlockCheckParams, BlockImport, BlockImportParams, ForkChoiceStrategy, ImportResult, }; diff --git a/test-utils/client/src/lib.rs b/test-utils/client/src/lib.rs index 4d4ed8f1e4ca4..94a350b5f5df3 100644 --- a/test-utils/client/src/lib.rs +++ b/test-utils/client/src/lib.rs @@ -105,10 +105,7 @@ impl /// Create new `TestClientBuilder` with default backend and storage chain mode pub fn with_tx_storage(keep_blocks: u32) -> Self { - let backend = Arc::new(Backend::new_test_with_tx_storage( - keep_blocks, - 0, - )); + let backend = Arc::new(Backend::new_test_with_tx_storage(keep_blocks, 0)); Self::with_backend(backend) } } From d12335546adbebec1d254e2893ac19f72ca17cdf Mon Sep 17 00:00:00 2001 From: arkpar Date: Thu, 3 Feb 2022 14:53:27 +0100 Subject: [PATCH 4/7] Fix handling invalid index size + test --- client/db/src/lib.rs | 55 ++++++++++++++++++++++++++++++++++---------- 1 file changed, 43 insertions(+), 12 deletions(-) diff --git a/client/db/src/lib.rs b/client/db/src/lib.rs index e23505fd089a8..ffb1f99818859 100644 --- a/client/db/src/lib.rs +++ b/client/db/src/lib.rs @@ -110,7 +110,7 @@ const DB_HASH_LEN: usize = 32; /// Hash type that this backend uses for the database. pub type DbHash = sp_core::H256; -/// This is used as block body when storage-chain mode is enabled. +/// An extrinsic entry in the database. #[derive(Debug, Encode, Decode)] enum DbExtrinsic { /// Extrinsic that contains indexed data. @@ -1756,25 +1756,21 @@ fn apply_index_ops( } else { match index_map.get(&(index as u32)) { Some((hash, size)) => { - let extrinsic = extrinsic.encode(); - if *size as usize <= extrinsic.len() { - let offset = extrinsic.len() - *size as usize; + let encoded = extrinsic.encode(); + if *size as usize <= encoded.len() { + let offset = encoded.len() - *size as usize; transaction.store( columns::TRANSACTION, DbHash::from_slice(hash.as_ref()), - extrinsic[offset..].to_vec(), + encoded[offset..].to_vec(), ); DbExtrinsic::Indexed { hash: DbHash::from_slice(hash.as_ref()), - header: extrinsic[..offset].to_vec(), + header: encoded[..offset].to_vec(), } } else { - return Err(sp_blockchain::Error::Backend(format!( - "Error indexing extrinsic {}, invalid size. {} > {}", - DbHash::from_slice(hash.as_ref()), - size, - extrinsic.len(), - ))) + // Invalid indexed slice. Just store full data and don't index anything. + DbExtrinsic::Full(extrinsic) } }, _ => DbExtrinsic::Full(extrinsic), @@ -3160,6 +3156,41 @@ pub(crate) mod tests { assert_eq!(bc.indexed_transaction(&x1_hash).unwrap(), None); } + #[test] + fn index_invalid_size() { + let backend = Backend::::new_test_with_tx_storage(1, 10); + + let x0 = ExtrinsicWrapper::from(0u64).encode(); + let x1 = ExtrinsicWrapper::from(1u64).encode(); + let x0_hash = as sp_core::Hasher>::hash(&x0[..]); + let x1_hash = as sp_core::Hasher>::hash(&x1[..]); + let index = vec![ + IndexOperation::Insert { + extrinsic: 0, + hash: x0_hash.as_ref().to_vec(), + size: (x0.len()) as u32, + }, + IndexOperation::Insert { + extrinsic: 1, + hash: x1_hash.as_ref().to_vec(), + size: (x1.len() + 1) as u32, + }, + ]; + insert_block( + &backend, + 0, + Default::default(), + None, + Default::default(), + vec![0u64.into(), 1u64.into()], + Some(index), + ) + .unwrap(); + let bc = backend.blockchain(); + assert_eq!(bc.indexed_transaction(&x0_hash).unwrap().unwrap(), &x0[..]); + assert_eq!(bc.indexed_transaction(&x1_hash).unwrap(), None); + } + #[test] fn renew_transaction_storage() { let backend = Backend::::new_test_with_tx_storage(2, 10); From ef13720d419368a9b18909221156e13cf078c0c6 Mon Sep 17 00:00:00 2001 From: arkpar Date: Thu, 3 Feb 2022 16:18:23 +0100 Subject: [PATCH 5/7] Removed superflous result --- client/db/src/lib.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/client/db/src/lib.rs b/client/db/src/lib.rs index ffb1f99818859..7894b50bac5a5 100644 --- a/client/db/src/lib.rs +++ b/client/db/src/lib.rs @@ -1297,7 +1297,7 @@ impl Backend { transaction.set_from_vec(columns::BODY, &lookup_key, body.encode()); } else { let body = - apply_index_ops::(&mut transaction, body, operation.index_ops)?; + apply_index_ops::(&mut transaction, body, operation.index_ops); transaction.set_from_vec(columns::BODY_INDEX, &lookup_key, body); } } @@ -1733,7 +1733,7 @@ fn apply_index_ops( transaction: &mut Transaction, body: Vec, ops: Vec, -) -> ClientResult> { +) -> Vec { let mut extrinsic_index: Vec> = Vec::with_capacity(body.len()); let mut index_map = HashMap::new(); let mut renewed_map = HashMap::new(); @@ -1785,7 +1785,7 @@ fn apply_index_ops( renewed_map.len(), extrinsic_index.len() - index_map.len() - renewed_map.len(), ); - Ok(extrinsic_index.encode()) + extrinsic_index.encode() } fn apply_indexed_body(transaction: &mut Transaction, body: Vec>) { From 8e0961b93c3659e7e7f9a17cf9f6513fe9838cc7 Mon Sep 17 00:00:00 2001 From: arkpar Date: Fri, 4 Mar 2022 10:18:40 +0100 Subject: [PATCH 6/7] Minor changes --- client/db/src/lib.rs | 2 +- client/db/src/parity_db.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/client/db/src/lib.rs b/client/db/src/lib.rs index d9f9446f6fa93..c08a677fe4b13 100644 --- a/client/db/src/lib.rs +++ b/client/db/src/lib.rs @@ -661,7 +661,7 @@ impl sc_client_api::blockchain::Backend for BlockchainDb - return Err(sp_blockchain::Error::Backend(format!( + Err(sp_blockchain::Error::Backend(format!( "Error decoding body list: {}", err ))), diff --git a/client/db/src/parity_db.rs b/client/db/src/parity_db.rs index 1ea40679b4e44..c27d7b0d4b4db 100644 --- a/client/db/src/parity_db.rs +++ b/client/db/src/parity_db.rs @@ -71,7 +71,7 @@ pub fn open>( } if upgrade { - log::info!("Upgrading metadata"); + log::info!("Upgrading database metadata."); if let Some(meta) = parity_db::Options::load_metadata(path)? { config.write_metadata(path, &meta.salt)?; } From 7c8e0d5615699e0967afcdc295478879188479c9 Mon Sep 17 00:00:00 2001 From: arkpar Date: Fri, 4 Mar 2022 10:42:25 +0100 Subject: [PATCH 7/7] fmt --- client/db/src/lib.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/client/db/src/lib.rs b/client/db/src/lib.rs index c08a677fe4b13..f451062de33ab 100644 --- a/client/db/src/lib.rs +++ b/client/db/src/lib.rs @@ -661,10 +661,7 @@ impl sc_client_api::blockchain::Backend for BlockchainDb - Err(sp_blockchain::Error::Backend(format!( - "Error decoding body list: {}", - err - ))), + Err(sp_blockchain::Error::Backend(format!("Error decoding body list: {}", err))), } } }