From 702936c08d4737c08136a6df48d57f6de989195b Mon Sep 17 00:00:00 2001 From: Heorhii Azarov Date: Tue, 1 Aug 2023 14:38:07 +0300 Subject: [PATCH 1/4] Init storage with version --- chainstate/launcher/src/lib.rs | 4 +- .../launcher/src/storage_compatibility.rs | 96 ++++++++----------- chainstate/src/detail/error.rs | 13 ++- chainstate/storage/src/internal/mod.rs | 33 ++++++- chainstate/storage/src/internal/store_tx.rs | 4 +- chainstate/storage/src/internal/test.rs | 26 ++--- chainstate/storage/src/internal/version.rs | 32 +++++++ chainstate/storage/src/lib.rs | 4 +- chainstate/storage/src/mock/mock_impl.rs | 6 +- chainstate/storage/src/mock/tests.rs | 4 +- 10 files changed, 136 insertions(+), 86 deletions(-) create mode 100644 chainstate/storage/src/internal/version.rs diff --git a/chainstate/launcher/src/lib.rs b/chainstate/launcher/src/lib.rs index b2eb32011f..7bf5842082 100644 --- a/chainstate/launcher/src/lib.rs +++ b/chainstate/launcher/src/lib.rs @@ -39,10 +39,10 @@ fn make_chainstate_and_storage_impl( chain_config: Arc, chainstate_config: ChainstateConfig, ) -> Result, Error> { - let mut storage = chainstate_storage::Store::new(storage_backend) + let storage = chainstate_storage::Store::new(storage_backend, &chain_config) .map_err(|e| Error::FailedToInitializeChainstate(e.into()))?; - storage_compatibility::check_storage_compatibility(&mut storage, chain_config.as_ref()) + storage_compatibility::check_storage_compatibility(&storage, chain_config.as_ref()) .map_err(InitializationError::StorageCompatibilityCheckError)?; let chainstate = chainstate::make_chainstate( diff --git a/chainstate/launcher/src/storage_compatibility.rs b/chainstate/launcher/src/storage_compatibility.rs index 9ecbfa08ae..2f207d0d24 100644 --- a/chainstate/launcher/src/storage_compatibility.rs +++ b/chainstate/launcher/src/storage_compatibility.rs @@ -14,16 +14,12 @@ // limitations under the License. use chainstate::StorageCompatibilityCheckError; -use chainstate_storage::{BlockchainStorageRead, BlockchainStorageWrite}; +use chainstate_storage::{BlockchainStorageRead, ChainstateStorageVersion}; use common::chain::ChainConfig; use utils::ensure; -const CHAINSTATE_STORAGE_VERSION_UNINITIALIZED: u32 = 0; -const CHAINSTATE_STORAGE_VERSION_V1: u32 = 1; -const CURRENT_CHAINSTATE_STORAGE_VERSION: u32 = CHAINSTATE_STORAGE_VERSION_V1; - -pub fn check_storage_compatibility( - storage: &mut chainstate_storage::Store, +pub fn check_storage_compatibility( + storage: &impl BlockchainStorageRead, chain_config: &ChainConfig, ) -> Result<(), StorageCompatibilityCheckError> { check_storage_version(storage)?; @@ -33,74 +29,64 @@ pub fn check_storage_compatibility( Ok(()) } -fn check_storage_version( - storage: &mut chainstate_storage::Store, +fn check_storage_version( + storage: &impl BlockchainStorageRead, ) -> Result<(), StorageCompatibilityCheckError> { let storage_version = storage .get_storage_version() - .map_err(StorageCompatibilityCheckError::StorageError)?; + .map_err(StorageCompatibilityCheckError::StorageError)? + .ok_or(StorageCompatibilityCheckError::StorageVersionMissing)?; + let storage_version = ChainstateStorageVersion::new(storage_version) + .ok_or(StorageCompatibilityCheckError::StorageVersionConversionError)?; + + ensure!( + storage_version == ChainstateStorageVersion::CURRENT, + StorageCompatibilityCheckError::ChainstateStorageVersionMismatch( + storage_version, + ChainstateStorageVersion::CURRENT + ) + ); - if storage_version == CHAINSTATE_STORAGE_VERSION_UNINITIALIZED { - storage - .set_storage_version(CURRENT_CHAINSTATE_STORAGE_VERSION) - .map_err(StorageCompatibilityCheckError::StorageError)?; - } else { - ensure!( - storage_version == CURRENT_CHAINSTATE_STORAGE_VERSION, - StorageCompatibilityCheckError::ChainstateStorageVersionMismatch( - storage_version, - CURRENT_CHAINSTATE_STORAGE_VERSION - ) - ); - } Ok(()) } -fn check_magic_bytes( - storage: &mut chainstate_storage::Store, +fn check_magic_bytes( + storage: &impl BlockchainStorageRead, chain_config: &ChainConfig, ) -> Result<(), StorageCompatibilityCheckError> { let storage_magic_bytes = storage .get_magic_bytes() - .map_err(StorageCompatibilityCheckError::StorageError)?; - let chain_config_magic_bytes = chain_config.magic_bytes(); + .map_err(StorageCompatibilityCheckError::StorageError)? + .ok_or(StorageCompatibilityCheckError::MagicBytesMissing)?; - match storage_magic_bytes { - Some(storage_magic_bytes) => ensure!( - &storage_magic_bytes == chain_config_magic_bytes, - StorageCompatibilityCheckError::ChainConfigMagicBytesMismatch( - storage_magic_bytes, - chain_config_magic_bytes.to_owned() - ) - ), - None => storage - .set_magic_bytes(chain_config_magic_bytes) - .map_err(StorageCompatibilityCheckError::StorageError)?, - }; + ensure!( + &storage_magic_bytes == chain_config.magic_bytes(), + StorageCompatibilityCheckError::ChainConfigMagicBytesMismatch( + storage_magic_bytes, + chain_config.magic_bytes().to_owned() + ) + ); Ok(()) } -fn check_chain_type( - storage: &mut chainstate_storage::Store, +fn check_chain_type( + storage: &impl BlockchainStorageRead, chain_config: &ChainConfig, ) -> Result<(), StorageCompatibilityCheckError> { - let storage_chain_type = - storage.get_chain_type().map_err(StorageCompatibilityCheckError::StorageError)?; + let storage_chain_type = storage + .get_chain_type() + .map_err(StorageCompatibilityCheckError::StorageError)? + .ok_or(StorageCompatibilityCheckError::ChainTypeMissing)?; let chain_config_type = chain_config.chain_type().name(); - match storage_chain_type { - Some(storage_chain_type) => ensure!( - storage_chain_type == chain_config_type, - StorageCompatibilityCheckError::ChainTypeMismatch( - storage_chain_type, - chain_config_type.to_owned() - ) - ), - None => storage - .set_chain_type(chain_config_type) - .map_err(StorageCompatibilityCheckError::StorageError)?, - }; + ensure!( + storage_chain_type == chain_config_type, + StorageCompatibilityCheckError::ChainTypeMismatch( + storage_chain_type, + chain_config_type.to_owned() + ) + ); Ok(()) } diff --git a/chainstate/src/detail/error.rs b/chainstate/src/detail/error.rs index 3860ed9229..c5527382fd 100644 --- a/chainstate/src/detail/error.rs +++ b/chainstate/src/detail/error.rs @@ -21,6 +21,7 @@ use super::{ storage::TransactionVerifierStorageError, }, }; +use chainstate_storage::ChainstateStorageVersion; use chainstate_types::{GetAncestorError, PropertyQueryError}; use common::{ chain::{ @@ -214,10 +215,18 @@ pub enum InitializationError { pub enum StorageCompatibilityCheckError { #[error("Block storage error: `{0}`")] StorageError(#[from] chainstate_storage::Error), + #[error("Storage version is missing in the db")] + StorageVersionMissing, + #[error("Failed to convert storage error")] + StorageVersionConversionError, + #[error("Magic bytes are missing in the db")] + MagicBytesMissing, + #[error("Chain type is missing in the db")] + ChainTypeMissing, #[error( - "Node cannot load chainstate database because the versions mismatch: db `{0}`, app `{1}`" + "Node cannot load chainstate database because the versions mismatch: db `{0:?}`, app `{1:?}`" )] - ChainstateStorageVersionMismatch(u32, u32), + ChainstateStorageVersionMismatch(ChainstateStorageVersion, ChainstateStorageVersion), #[error( "Chain's config magic bytes do not match the one from database : expected `{0:?}`, actual `{1:?}`" )] diff --git a/chainstate/storage/src/internal/mod.rs b/chainstate/storage/src/internal/mod.rs index f92dec4a22..48ca171f97 100644 --- a/chainstate/storage/src/internal/mod.rs +++ b/chainstate/storage/src/internal/mod.rs @@ -22,8 +22,8 @@ use common::{ config::EpochIndex, tokens::{TokenAuxiliaryData, TokenId}, transaction::{Transaction, TxMainChainIndex, TxMainChainPosition}, - AccountNonce, AccountType, Block, DelegationId, GenBlock, OutPointSourceId, PoolId, - UtxoOutPoint, + AccountNonce, AccountType, Block, ChainConfig, DelegationId, GenBlock, OutPointSourceId, + PoolId, UtxoOutPoint, }, primitives::{Amount, BlockHeight, Id}, }; @@ -42,12 +42,35 @@ use crate::{ mod store_tx; pub use store_tx::{StoreTxRo, StoreTxRw}; +mod version; +pub use version::ChainstateStorageVersion; + /// Store for blockchain data, parametrized over the backend B pub struct Store(storage::Storage); impl Store { /// Create a new chainstate storage - pub fn new(backend: B) -> crate::Result { + pub fn new(backend: B, chain_config: &ChainConfig) -> crate::Result { + let mut storage = Self::from_backend(backend)?; + + // Set defaults if missing + + if storage.get_storage_version()?.is_none() { + storage.set_storage_version(ChainstateStorageVersion::CURRENT as u32)?; + } + + if storage.get_magic_bytes()?.is_none() { + storage.set_magic_bytes(chain_config.magic_bytes())?; + } + + if storage.get_chain_type()?.is_none() { + storage.set_chain_type(chain_config.chain_type().name())?; + } + + Ok(storage) + } + + fn from_backend(backend: B) -> crate::Result { let storage = Self(storage::Storage::new(backend).map_err(crate::Error::from)?); Ok(storage) } @@ -146,7 +169,7 @@ impl Store { impl Store { /// Create a default storage (mostly for testing, may want to remove this later) pub fn new_empty() -> crate::Result { - Self::new(B::default()) + Self::from_backend(B::default()) } } @@ -201,7 +224,7 @@ macro_rules! delegate_to_transaction { impl BlockchainStorageRead for Store { delegate_to_transaction! { - fn get_storage_version(&self) -> crate::Result; + fn get_storage_version(&self) -> crate::Result>; fn get_magic_bytes(&self) -> crate::Result>; fn get_chain_type(&self) -> crate::Result>; fn get_best_block_id(&self) -> crate::Result>>; diff --git a/chainstate/storage/src/internal/store_tx.rs b/chainstate/storage/src/internal/store_tx.rs index 68a42a1961..50ba57b93d 100644 --- a/chainstate/storage/src/internal/store_tx.rs +++ b/chainstate/storage/src/internal/store_tx.rs @@ -79,8 +79,8 @@ macro_rules! impl_read_ops { ($TxType:ident) => { /// Blockchain data storage transaction impl<'st, B: storage::Backend> BlockchainStorageRead for $TxType<'st, B> { - fn get_storage_version(&self) -> crate::Result { - self.read_value::().map(|v| v.unwrap_or_default()) + fn get_storage_version(&self) -> crate::Result> { + self.read_value::() } fn get_magic_bytes(&self) -> crate::Result> { diff --git a/chainstate/storage/src/internal/test.rs b/chainstate/storage/src/internal/test.rs index 8f29470b0e..69b3fc334d 100644 --- a/chainstate/storage/src/internal/test.rs +++ b/chainstate/storage/src/internal/test.rs @@ -33,7 +33,7 @@ fn test_storage_get_default_version_in_tx() { let store = TestStore::new_empty().unwrap(); let vtx = store.transaction_ro().unwrap().get_storage_version().unwrap(); let vst = store.get_storage_version().unwrap(); - assert_eq!(vtx, 0, "Default storage version wrong"); + assert_eq!(vtx, None, "Default storage version wrong"); assert_eq!(vtx, vst, "Transaction and non-transaction inconsistency"); }) } @@ -73,9 +73,9 @@ fn test_storage_manipulation() { let mut store = TestStore::new_empty().unwrap(); // Storage version manipulation - assert_eq!(store.get_storage_version(), Ok(0)); + assert_eq!(store.get_storage_version(), Ok(None)); assert_eq!(store.set_storage_version(2), Ok(())); - assert_eq!(store.get_storage_version(), Ok(2)); + assert_eq!(store.get_storage_version(), Ok(Some(2))); // Store is now empty, the block is not there assert_eq!(store.get_block(block0.get_id()), Ok(None)); @@ -159,7 +159,7 @@ fn get_set_transactions() { let store = Store::clone(&store); utils::thread::spawn(move || { let mut tx = store.transaction_rw(None).unwrap(); - let v = tx.get_storage_version().unwrap(); + let v = tx.get_storage_version().unwrap().unwrap(); tx.set_storage_version(v + 1).unwrap(); tx.commit().unwrap(); }) @@ -168,8 +168,8 @@ fn get_set_transactions() { let store = Store::clone(&store); utils::thread::spawn(move || { let tx = store.transaction_ro().unwrap(); - let v1 = tx.get_storage_version().unwrap(); - let v2 = tx.get_storage_version().unwrap(); + let v1 = tx.get_storage_version().unwrap().unwrap(); + let v2 = tx.get_storage_version().unwrap().unwrap(); assert!([2, 3].contains(&v1)); assert_eq!(v1, v2, "Version query in a transaction inconsistent"); }) @@ -177,7 +177,7 @@ fn get_set_transactions() { let _ = thr0.join(); let _ = thr1.join(); - assert_eq!(store.get_storage_version(), Ok(3)); + assert_eq!(store.get_storage_version(), Ok(Some(3))); }) } @@ -193,7 +193,7 @@ fn test_storage_transactions() { let store = Store::clone(&store); utils::thread::spawn(move || { let mut tx = store.transaction_rw(None).unwrap(); - let v = tx.get_storage_version().unwrap(); + let v = tx.get_storage_version().unwrap().unwrap(); tx.set_storage_version(v + 3).unwrap(); tx.commit().unwrap(); }) @@ -202,7 +202,7 @@ fn test_storage_transactions() { let store = Store::clone(&store); utils::thread::spawn(move || { let mut tx = store.transaction_rw(None).unwrap(); - let v = tx.get_storage_version().unwrap(); + let v = tx.get_storage_version().unwrap().unwrap(); tx.set_storage_version(v + 5).unwrap(); tx.commit().unwrap(); }) @@ -210,7 +210,7 @@ fn test_storage_transactions() { let _ = thr0.join(); let _ = thr1.join(); - assert_eq!(store.get_storage_version(), Ok(10)); + assert_eq!(store.get_storage_version(), Ok(Some(10))); }) } @@ -226,7 +226,7 @@ fn test_storage_transactions_with_result_check() { let store = Store::clone(&store); utils::thread::spawn(move || { let mut tx = store.transaction_rw(None).unwrap(); - let v = tx.get_storage_version().unwrap(); + let v = tx.get_storage_version().unwrap().unwrap(); assert!(tx.set_storage_version(v + 3).is_ok()); assert!(tx.commit().is_ok()); }) @@ -235,7 +235,7 @@ fn test_storage_transactions_with_result_check() { let store = Store::clone(&store); utils::thread::spawn(move || { let mut tx = store.transaction_rw(None).unwrap(); - let v = tx.get_storage_version().unwrap(); + let v = tx.get_storage_version().unwrap().unwrap(); assert!(tx.set_storage_version(v + 5).is_ok()); assert!(tx.commit().is_ok()); }) @@ -243,7 +243,7 @@ fn test_storage_transactions_with_result_check() { let _ = thr0.join(); let _ = thr1.join(); - assert_eq!(store.get_storage_version(), Ok(10)); + assert_eq!(store.get_storage_version(), Ok(Some(10))); }) } diff --git a/chainstate/storage/src/internal/version.rs b/chainstate/storage/src/internal/version.rs new file mode 100644 index 0000000000..8eb780cb85 --- /dev/null +++ b/chainstate/storage/src/internal/version.rs @@ -0,0 +1,32 @@ +// Copyright (c) 2023 RBB S.r.l +// opensource@mintlayer.org +// SPDX-License-Identifier: MIT +// Licensed under the MIT License; +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://github.com/mintlayer/mintlayer-core/blob/master/LICENSE +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#[derive(Debug, Clone, Copy, Eq, PartialEq)] +#[repr(u32)] +pub enum ChainstateStorageVersion { + // skipping 0 for historical reasons + V1 = 1, +} + +impl ChainstateStorageVersion { + pub const CURRENT: Self = Self::V1; + + pub fn new(value: u32) -> Option { + match value { + 1 => Some(ChainstateStorageVersion::V1), + _ => None, + } + } +} diff --git a/chainstate/storage/src/lib.rs b/chainstate/storage/src/lib.rs index 86a148832b..2e2a369755 100644 --- a/chainstate/storage/src/lib.rs +++ b/chainstate/storage/src/lib.rs @@ -24,7 +24,7 @@ pub mod schema; use std::collections::BTreeMap; use common::chain::block::signed_block_header::SignedBlockHeader; -pub use internal::Store; +pub use internal::{ChainstateStorageVersion, Store}; use chainstate_types::{BlockIndex, EpochStorageRead, EpochStorageWrite}; use common::chain::block::BlockReward; @@ -61,7 +61,7 @@ pub trait BlockchainStorageRead: + EpochStorageRead { /// Get storage version - fn get_storage_version(&self) -> crate::Result; + fn get_storage_version(&self) -> crate::Result>; /// Get magic bytes fn get_magic_bytes(&self) -> crate::Result>; diff --git a/chainstate/storage/src/mock/mock_impl.rs b/chainstate/storage/src/mock/mock_impl.rs index 975081618d..10c3282a25 100644 --- a/chainstate/storage/src/mock/mock_impl.rs +++ b/chainstate/storage/src/mock/mock_impl.rs @@ -44,7 +44,7 @@ mockall::mock! { pub Store {} impl crate::BlockchainStorageRead for Store { - fn get_storage_version(&self) -> crate::Result; + fn get_storage_version(&self) -> crate::Result>; fn get_magic_bytes(&self) -> crate::Result>; fn get_chain_type(&self) -> crate::Result>; fn get_best_block_id(&self) -> crate::Result>>; @@ -300,7 +300,7 @@ mockall::mock! { pub StoreTxRo {} impl crate::BlockchainStorageRead for StoreTxRo { - fn get_storage_version(&self) -> crate::Result; + fn get_storage_version(&self) -> crate::Result>; fn get_magic_bytes(&self) -> crate::Result>; fn get_chain_type(&self) -> crate::Result>; fn get_best_block_id(&self) -> crate::Result>>; @@ -414,7 +414,7 @@ mockall::mock! { pub StoreTxRw {} impl crate::BlockchainStorageRead for StoreTxRw { - fn get_storage_version(&self) -> crate::Result; + fn get_storage_version(&self) -> crate::Result>; fn get_magic_bytes(&self) -> crate::Result>; fn get_chain_type(&self) -> crate::Result>; fn get_best_block_id(&self) -> crate::Result>>; diff --git a/chainstate/storage/src/mock/tests.rs b/chainstate/storage/src/mock/tests.rs index 3de9d6b43d..6ff06eb601 100644 --- a/chainstate/storage/src/mock/tests.rs +++ b/chainstate/storage/src/mock/tests.rs @@ -92,7 +92,7 @@ fn mock_transaction() { let mut store = MockStore::new(); store.expect_transaction_rw().returning(|_| { let mut mock_tx = MockStoreTxRw::new(); - mock_tx.expect_get_storage_version().return_const(Ok(3)); + mock_tx.expect_get_storage_version().return_const(Ok(Some(3))); mock_tx .expect_set_storage_version() .with(mockall::predicate::eq(4)) @@ -103,7 +103,7 @@ fn mock_transaction() { // Test some code against the mock let mut tx = store.transaction_rw(None).unwrap(); - let v = tx.get_storage_version().unwrap(); + let v = tx.get_storage_version().unwrap().unwrap(); tx.set_storage_version(v + 1).unwrap(); tx.commit().unwrap(); } From db2e40a42a371c05a76a2b0b753de81f16073b3e Mon Sep 17 00:00:00 2001 From: Heorhii Azarov Date: Tue, 1 Aug 2023 18:37:42 +0300 Subject: [PATCH 2/4] Store ChainstateStorageVersion --- Cargo.lock | 1 + .../launcher/src/storage_compatibility.rs | 2 - chainstate/src/detail/error.rs | 2 - chainstate/storage/Cargo.toml | 2 + chainstate/storage/src/internal/mod.rs | 6 +- chainstate/storage/src/internal/store_tx.rs | 11 ++- chainstate/storage/src/internal/test.rs | 93 ++++++++++++------- chainstate/storage/src/internal/version.rs | 16 +--- chainstate/storage/src/lib.rs | 4 +- chainstate/storage/src/mock/mock_impl.rs | 12 ++- chainstate/storage/src/mock/tests.rs | 16 ++-- 11 files changed, 95 insertions(+), 70 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 5ab2905589..b47ca7cfb8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -872,6 +872,7 @@ dependencies = [ "itertools", "mockall", "num-traits", + "parity-scale-codec", "pos_accounting", "rstest", "serialization", diff --git a/chainstate/launcher/src/storage_compatibility.rs b/chainstate/launcher/src/storage_compatibility.rs index 2f207d0d24..f8449a42c9 100644 --- a/chainstate/launcher/src/storage_compatibility.rs +++ b/chainstate/launcher/src/storage_compatibility.rs @@ -36,8 +36,6 @@ fn check_storage_version( .get_storage_version() .map_err(StorageCompatibilityCheckError::StorageError)? .ok_or(StorageCompatibilityCheckError::StorageVersionMissing)?; - let storage_version = ChainstateStorageVersion::new(storage_version) - .ok_or(StorageCompatibilityCheckError::StorageVersionConversionError)?; ensure!( storage_version == ChainstateStorageVersion::CURRENT, diff --git a/chainstate/src/detail/error.rs b/chainstate/src/detail/error.rs index c5527382fd..67ce8ff569 100644 --- a/chainstate/src/detail/error.rs +++ b/chainstate/src/detail/error.rs @@ -217,8 +217,6 @@ pub enum StorageCompatibilityCheckError { StorageError(#[from] chainstate_storage::Error), #[error("Storage version is missing in the db")] StorageVersionMissing, - #[error("Failed to convert storage error")] - StorageVersionConversionError, #[error("Magic bytes are missing in the db")] MagicBytesMissing, #[error("Chain type is missing in the db")] diff --git a/chainstate/storage/Cargo.toml b/chainstate/storage/Cargo.toml index 41b99b2474..81faf833b5 100644 --- a/chainstate/storage/Cargo.toml +++ b/chainstate/storage/Cargo.toml @@ -17,6 +17,8 @@ utxo = { path = '../../utxo' } mockall = { workspace = true, optional = true } +parity-scale-codec.workspace = true + [dev-dependencies] crypto = { path = '../../crypto' } test-utils = {path = '../../test-utils'} diff --git a/chainstate/storage/src/internal/mod.rs b/chainstate/storage/src/internal/mod.rs index 48ca171f97..a3094b608b 100644 --- a/chainstate/storage/src/internal/mod.rs +++ b/chainstate/storage/src/internal/mod.rs @@ -56,7 +56,7 @@ impl Store { // Set defaults if missing if storage.get_storage_version()?.is_none() { - storage.set_storage_version(ChainstateStorageVersion::CURRENT as u32)?; + storage.set_storage_version(ChainstateStorageVersion::CURRENT)?; } if storage.get_magic_bytes()?.is_none() { @@ -224,7 +224,7 @@ macro_rules! delegate_to_transaction { impl BlockchainStorageRead for Store { delegate_to_transaction! { - fn get_storage_version(&self) -> crate::Result>; + fn get_storage_version(&self) -> crate::Result>; fn get_magic_bytes(&self) -> crate::Result>; fn get_chain_type(&self) -> crate::Result>; fn get_best_block_id(&self) -> crate::Result>>; @@ -379,7 +379,7 @@ impl PoSAccountingStorageRead for Store BlockchainStorageWrite for Store { delegate_to_transaction! { - fn set_storage_version(&mut self, version: u32) -> crate::Result<()>; + fn set_storage_version(&mut self, version: ChainstateStorageVersion) -> crate::Result<()>; fn set_magic_bytes(&mut self, bytes: &[u8; 4]) -> crate::Result<()>; fn set_chain_type(&mut self, chain: &str) -> crate::Result<()>; fn set_best_block_id(&mut self, id: &Id) -> crate::Result<()>; diff --git a/chainstate/storage/src/internal/store_tx.rs b/chainstate/storage/src/internal/store_tx.rs index 50ba57b93d..6d46d15e3c 100644 --- a/chainstate/storage/src/internal/store_tx.rs +++ b/chainstate/storage/src/internal/store_tx.rs @@ -37,11 +37,12 @@ use utxo::{Utxo, UtxosBlockUndo, UtxosStorageRead, UtxosStorageWrite}; use crate::{ schema::{self as db, Schema}, - BlockchainStorageRead, BlockchainStorageWrite, SealedStorageTag, TipStorageTag, + BlockchainStorageRead, BlockchainStorageWrite, ChainstateStorageVersion, SealedStorageTag, + TipStorageTag, }; mod well_known { - use super::{Codec, GenBlock, Id}; + use super::{ChainstateStorageVersion, Codec, GenBlock, Id}; /// Pre-defined database keys pub trait Entry { @@ -61,7 +62,7 @@ mod well_known { }; } - declare_entry!(StoreVersion: u32); + declare_entry!(StoreVersion: ChainstateStorageVersion); declare_entry!(BestBlockId: Id); declare_entry!(UtxosBestBlockId: Id); declare_entry!(TxIndexEnabled: bool); @@ -79,7 +80,7 @@ macro_rules! impl_read_ops { ($TxType:ident) => { /// Blockchain data storage transaction impl<'st, B: storage::Backend> BlockchainStorageRead for $TxType<'st, B> { - fn get_storage_version(&self) -> crate::Result> { + fn get_storage_version(&self) -> crate::Result> { self.read_value::() } @@ -394,7 +395,7 @@ impl_read_ops!(StoreTxRo); impl_read_ops!(StoreTxRw); impl<'st, B: storage::Backend> BlockchainStorageWrite for StoreTxRw<'st, B> { - fn set_storage_version(&mut self, version: u32) -> crate::Result<()> { + fn set_storage_version(&mut self, version: ChainstateStorageVersion) -> crate::Result<()> { self.write_value::(&version) } diff --git a/chainstate/storage/src/internal/test.rs b/chainstate/storage/src/internal/test.rs index 69b3fc334d..a417e2cca8 100644 --- a/chainstate/storage/src/internal/test.rs +++ b/chainstate/storage/src/internal/test.rs @@ -74,8 +74,14 @@ fn test_storage_manipulation() { // Storage version manipulation assert_eq!(store.get_storage_version(), Ok(None)); - assert_eq!(store.set_storage_version(2), Ok(())); - assert_eq!(store.get_storage_version(), Ok(Some(2))); + assert_eq!( + store.set_storage_version(ChainstateStorageVersion::V0), + Ok(()) + ); + assert_eq!( + store.get_storage_version(), + Ok(Some(ChainstateStorageVersion::V0)) + ); // Store is now empty, the block is not there assert_eq!(store.get_block(block0.get_id()), Ok(None)); @@ -150,17 +156,19 @@ fn test_storage_manipulation() { #[test] fn get_set_transactions() { utils::concurrency::model(|| { - // Set up the store and initialize the version to 2 + // Set up the store and initialize the version to 0 let mut store = TestStore::new_empty().unwrap(); - assert_eq!(store.set_storage_version(2), Ok(())); + assert_eq!( + store.set_storage_version(ChainstateStorageVersion::V0), + Ok(()) + ); // Concurrently bump version and run a transaction that reads the version twice. let thr1 = { let store = Store::clone(&store); utils::thread::spawn(move || { let mut tx = store.transaction_rw(None).unwrap(); - let v = tx.get_storage_version().unwrap().unwrap(); - tx.set_storage_version(v + 1).unwrap(); + tx.set_storage_version(ChainstateStorageVersion::V1).unwrap(); tx.commit().unwrap(); }) }; @@ -170,31 +178,44 @@ fn get_set_transactions() { let tx = store.transaction_ro().unwrap(); let v1 = tx.get_storage_version().unwrap().unwrap(); let v2 = tx.get_storage_version().unwrap().unwrap(); - assert!([2, 3].contains(&v1)); + assert!([ChainstateStorageVersion::V0, ChainstateStorageVersion::V1].contains(&v1)); assert_eq!(v1, v2, "Version query in a transaction inconsistent"); }) }; let _ = thr0.join(); let _ = thr1.join(); - assert_eq!(store.get_storage_version(), Ok(Some(3))); + assert_eq!( + store.get_storage_version(), + Ok(Some(ChainstateStorageVersion::V1)) + ); }) } -#[test] -fn test_storage_transactions() { - utils::concurrency::model(|| { - // Set up the store and initialize the version to 2 - let mut store = TestStore::new_empty().unwrap(); - assert_eq!(store.set_storage_version(2), Ok(())); +#[rstest] +#[trace] +#[case(Seed::from_entropy())] +fn test_storage_transactions(#[case] seed: Seed) { + utils::concurrency::model(move || { + // Set up the store with empty utxo set + let mut rng = make_seedable_rng(seed); + let store = TestStore::new_empty().unwrap(); + assert!(store.read_utxo_set().unwrap().is_empty()); - // Concurrently bump version by 3 and 5 in two separate threads + let (utxo1, outpoint1) = create_rand_utxo(&mut rng, 1); + let (utxo2, outpoint2) = create_rand_utxo(&mut rng, 1); + + let expected_utxo_set = BTreeMap::from_iter([ + (outpoint1.clone(), utxo1.clone()), + (outpoint2.clone(), utxo2.clone()), + ]); + + // Concurrently insert 2 utxo in two separate threads let thr0 = { let store = Store::clone(&store); utils::thread::spawn(move || { let mut tx = store.transaction_rw(None).unwrap(); - let v = tx.get_storage_version().unwrap().unwrap(); - tx.set_storage_version(v + 3).unwrap(); + tx.set_utxo(&outpoint1, utxo1).unwrap(); tx.commit().unwrap(); }) }; @@ -202,32 +223,41 @@ fn test_storage_transactions() { let store = Store::clone(&store); utils::thread::spawn(move || { let mut tx = store.transaction_rw(None).unwrap(); - let v = tx.get_storage_version().unwrap().unwrap(); - tx.set_storage_version(v + 5).unwrap(); + tx.set_utxo(&outpoint2, utxo2).unwrap(); tx.commit().unwrap(); }) }; let _ = thr0.join(); let _ = thr1.join(); - assert_eq!(store.get_storage_version(), Ok(Some(10))); + assert_eq!(store.read_utxo_set(), Ok(expected_utxo_set)); }) } -#[test] -fn test_storage_transactions_with_result_check() { - utils::concurrency::model(|| { - // Set up the store and initialize the version to 2 - let mut store = TestStore::new_empty().unwrap(); - assert_eq!(store.set_storage_version(2), Ok(())); +#[rstest] +#[trace] +#[case(Seed::from_entropy())] +fn test_storage_transactions_with_result_check(#[case] seed: Seed) { + utils::concurrency::model(move || { + // Set up the store with empty utxo set + let mut rng = make_seedable_rng(seed); + let store = TestStore::new_empty().unwrap(); + assert!(store.read_utxo_set().unwrap().is_empty()); + + let (utxo1, outpoint1) = create_rand_utxo(&mut rng, 1); + let (utxo2, outpoint2) = create_rand_utxo(&mut rng, 1); + + let expected_utxo_set = BTreeMap::from_iter([ + (outpoint1.clone(), utxo1.clone()), + (outpoint2.clone(), utxo2.clone()), + ]); - // Concurrently bump version by 3 and 5 in two separate threads + // Concurrently insert 2 utxo in two separate threads let thr0 = { let store = Store::clone(&store); utils::thread::spawn(move || { let mut tx = store.transaction_rw(None).unwrap(); - let v = tx.get_storage_version().unwrap().unwrap(); - assert!(tx.set_storage_version(v + 3).is_ok()); + assert!(tx.set_utxo(&outpoint1, utxo1).is_ok()); assert!(tx.commit().is_ok()); }) }; @@ -235,15 +265,14 @@ fn test_storage_transactions_with_result_check() { let store = Store::clone(&store); utils::thread::spawn(move || { let mut tx = store.transaction_rw(None).unwrap(); - let v = tx.get_storage_version().unwrap().unwrap(); - assert!(tx.set_storage_version(v + 5).is_ok()); + assert!(tx.set_utxo(&outpoint2, utxo2).is_ok()); assert!(tx.commit().is_ok()); }) }; let _ = thr0.join(); let _ = thr1.join(); - assert_eq!(store.get_storage_version(), Ok(Some(10))); + assert_eq!(store.read_utxo_set(), Ok(expected_utxo_set)); }) } diff --git a/chainstate/storage/src/internal/version.rs b/chainstate/storage/src/internal/version.rs index 8eb780cb85..47aef5e70d 100644 --- a/chainstate/storage/src/internal/version.rs +++ b/chainstate/storage/src/internal/version.rs @@ -13,20 +13,14 @@ // See the License for the specific language governing permissions and // limitations under the License. -#[derive(Debug, Clone, Copy, Eq, PartialEq)] -#[repr(u32)] +use serialization::{Decode, Encode}; + +#[derive(Debug, Encode, Decode, Clone, Copy, Eq, PartialEq)] pub enum ChainstateStorageVersion { - // skipping 0 for historical reasons - V1 = 1, + V0, + V1, } impl ChainstateStorageVersion { pub const CURRENT: Self = Self::V1; - - pub fn new(value: u32) -> Option { - match value { - 1 => Some(ChainstateStorageVersion::V1), - _ => None, - } - } } diff --git a/chainstate/storage/src/lib.rs b/chainstate/storage/src/lib.rs index 2e2a369755..1ffa025794 100644 --- a/chainstate/storage/src/lib.rs +++ b/chainstate/storage/src/lib.rs @@ -61,7 +61,7 @@ pub trait BlockchainStorageRead: + EpochStorageRead { /// Get storage version - fn get_storage_version(&self) -> crate::Result>; + fn get_storage_version(&self) -> crate::Result>; /// Get magic bytes fn get_magic_bytes(&self) -> crate::Result>; @@ -138,7 +138,7 @@ pub trait BlockchainStorageWrite: + EpochStorageWrite { /// Set storage version - fn set_storage_version(&mut self, version: u32) -> Result<()>; + fn set_storage_version(&mut self, version: ChainstateStorageVersion) -> Result<()>; /// Set magic bytes fn set_magic_bytes(&mut self, bytes: &[u8; 4]) -> Result<()>; diff --git a/chainstate/storage/src/mock/mock_impl.rs b/chainstate/storage/src/mock/mock_impl.rs index 10c3282a25..48a73f222b 100644 --- a/chainstate/storage/src/mock/mock_impl.rs +++ b/chainstate/storage/src/mock/mock_impl.rs @@ -39,12 +39,14 @@ use super::mock_impl_accounting::{ PoSAccountingStorageWriteTip, }; +use crate::ChainstateStorageVersion; + mockall::mock! { /// A mock object for blockchain storage pub Store {} impl crate::BlockchainStorageRead for Store { - fn get_storage_version(&self) -> crate::Result>; + fn get_storage_version(&self) -> crate::Result>; fn get_magic_bytes(&self) -> crate::Result>; fn get_chain_type(&self) -> crate::Result>; fn get_best_block_id(&self) -> crate::Result>>; @@ -149,7 +151,7 @@ mockall::mock! { } impl crate::BlockchainStorageWrite for Store { - fn set_storage_version(&mut self, version: u32) -> crate::Result<()>; + fn set_storage_version(&mut self, version: ChainstateStorageVersion) -> crate::Result<()>; fn set_magic_bytes(&mut self, bytes: &[u8; 4]) -> crate::Result<()>; fn set_chain_type(&mut self, chain: &str) -> crate::Result<()>; fn set_best_block_id(&mut self, id: &Id) -> crate::Result<()>; @@ -300,7 +302,7 @@ mockall::mock! { pub StoreTxRo {} impl crate::BlockchainStorageRead for StoreTxRo { - fn get_storage_version(&self) -> crate::Result>; + fn get_storage_version(&self) -> crate::Result>; fn get_magic_bytes(&self) -> crate::Result>; fn get_chain_type(&self) -> crate::Result>; fn get_best_block_id(&self) -> crate::Result>>; @@ -414,7 +416,7 @@ mockall::mock! { pub StoreTxRw {} impl crate::BlockchainStorageRead for StoreTxRw { - fn get_storage_version(&self) -> crate::Result>; + fn get_storage_version(&self) -> crate::Result>; fn get_magic_bytes(&self) -> crate::Result>; fn get_chain_type(&self) -> crate::Result>; fn get_best_block_id(&self) -> crate::Result>>; @@ -518,7 +520,7 @@ mockall::mock! { } impl crate::BlockchainStorageWrite for StoreTxRw { - fn set_storage_version(&mut self, version: u32) -> crate::Result<()>; + fn set_storage_version(&mut self, version: ChainstateStorageVersion) -> crate::Result<()>; fn set_magic_bytes(&mut self, bytes: &[u8; 4]) -> crate::Result<()>; fn set_chain_type(&mut self, chain: &str) -> crate::Result<()>; fn set_best_block_id(&mut self, id: &Id) -> crate::Result<()>; diff --git a/chainstate/storage/src/mock/tests.rs b/chainstate/storage/src/mock/tests.rs index 6ff06eb601..9270838eba 100644 --- a/chainstate/storage/src/mock/tests.rs +++ b/chainstate/storage/src/mock/tests.rs @@ -14,8 +14,10 @@ // limitations under the License. use super::*; -use crate::{BlockchainStorageRead, BlockchainStorageWrite, Transactional}; -use crate::{TransactionRo, TransactionRw}; +use crate::{ + BlockchainStorageRead, BlockchainStorageWrite, ChainstateStorageVersion, TransactionRo, + TransactionRw, Transactional, +}; use common::{ chain::block::{timestamp::BlockTimestamp, BlockReward, ConsensusData}, chain::{signed_transaction::SignedTransaction, transaction::Transaction, Block, GenBlock}, @@ -33,7 +35,7 @@ fn basic_mock() { let mut mock = MockStore::new(); mock.expect_set_storage_version().times(1).return_const(Ok(())); - let r = mock.set_storage_version(5); + let r = mock.set_storage_version(ChainstateStorageVersion::V0); assert_eq!(r, Ok(())); } @@ -42,7 +44,7 @@ fn basic_fail() { let mut mock = MockStore::new(); mock.expect_set_storage_version().times(1).return_const(Err(TXFAIL)); - let r = mock.set_storage_version(5); + let r = mock.set_storage_version(ChainstateStorageVersion::V0); assert_eq!(r, Err(TXFAIL)); } @@ -92,10 +94,9 @@ fn mock_transaction() { let mut store = MockStore::new(); store.expect_transaction_rw().returning(|_| { let mut mock_tx = MockStoreTxRw::new(); - mock_tx.expect_get_storage_version().return_const(Ok(Some(3))); mock_tx .expect_set_storage_version() - .with(mockall::predicate::eq(4)) + .with(mockall::predicate::eq(ChainstateStorageVersion::V1)) .return_const(Ok(())); mock_tx.expect_commit().times(1).return_const(Ok(())); Ok(mock_tx) @@ -103,8 +104,7 @@ fn mock_transaction() { // Test some code against the mock let mut tx = store.transaction_rw(None).unwrap(); - let v = tx.get_storage_version().unwrap().unwrap(); - tx.set_storage_version(v + 1).unwrap(); + tx.set_storage_version(ChainstateStorageVersion::V1).unwrap(); tx.commit().unwrap(); } From 552e5da49039fde8f821297f0eb5e796e13a4bd1 Mon Sep 17 00:00:00 2001 From: Heorhii Azarov Date: Tue, 1 Aug 2023 18:47:25 +0300 Subject: [PATCH 3/4] Use transaction to init storage --- chainstate/storage/src/internal/mod.rs | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/chainstate/storage/src/internal/mod.rs b/chainstate/storage/src/internal/mod.rs index a3094b608b..b28f7ccb9a 100644 --- a/chainstate/storage/src/internal/mod.rs +++ b/chainstate/storage/src/internal/mod.rs @@ -51,22 +51,25 @@ pub struct Store(storage::Storage); impl Store { /// Create a new chainstate storage pub fn new(backend: B, chain_config: &ChainConfig) -> crate::Result { - let mut storage = Self::from_backend(backend)?; + let storage = Self::from_backend(backend)?; // Set defaults if missing + let mut db_tx = storage.transaction_rw(None)?; - if storage.get_storage_version()?.is_none() { - storage.set_storage_version(ChainstateStorageVersion::CURRENT)?; + if db_tx.get_storage_version()?.is_none() { + db_tx.set_storage_version(ChainstateStorageVersion::CURRENT)?; } - if storage.get_magic_bytes()?.is_none() { - storage.set_magic_bytes(chain_config.magic_bytes())?; + if db_tx.get_magic_bytes()?.is_none() { + db_tx.set_magic_bytes(chain_config.magic_bytes())?; } - if storage.get_chain_type()?.is_none() { - storage.set_chain_type(chain_config.chain_type().name())?; + if db_tx.get_chain_type()?.is_none() { + db_tx.set_chain_type(chain_config.chain_type().name())?; } + db_tx.commit()?; + Ok(storage) } From ce1569b4acafbb43388e11270b626b173e9d07fd Mon Sep 17 00:00:00 2001 From: Heorhii Azarov Date: Wed, 2 Aug 2023 13:39:12 +0300 Subject: [PATCH 4/4] Make ChainstateStorageVersion a struct --- chainstate/storage/src/internal/test.rs | 15 +++++++++------ chainstate/storage/src/internal/version.rs | 11 ++++++----- chainstate/storage/src/mock/tests.rs | 8 ++++---- 3 files changed, 19 insertions(+), 15 deletions(-) diff --git a/chainstate/storage/src/internal/test.rs b/chainstate/storage/src/internal/test.rs index a417e2cca8..87c283d87e 100644 --- a/chainstate/storage/src/internal/test.rs +++ b/chainstate/storage/src/internal/test.rs @@ -75,12 +75,12 @@ fn test_storage_manipulation() { // Storage version manipulation assert_eq!(store.get_storage_version(), Ok(None)); assert_eq!( - store.set_storage_version(ChainstateStorageVersion::V0), + store.set_storage_version(ChainstateStorageVersion::new(0)), Ok(()) ); assert_eq!( store.get_storage_version(), - Ok(Some(ChainstateStorageVersion::V0)) + Ok(Some(ChainstateStorageVersion::new(0))) ); // Store is now empty, the block is not there @@ -159,7 +159,7 @@ fn get_set_transactions() { // Set up the store and initialize the version to 0 let mut store = TestStore::new_empty().unwrap(); assert_eq!( - store.set_storage_version(ChainstateStorageVersion::V0), + store.set_storage_version(ChainstateStorageVersion::new(0)), Ok(()) ); @@ -168,7 +168,7 @@ fn get_set_transactions() { let store = Store::clone(&store); utils::thread::spawn(move || { let mut tx = store.transaction_rw(None).unwrap(); - tx.set_storage_version(ChainstateStorageVersion::V1).unwrap(); + tx.set_storage_version(ChainstateStorageVersion::new(1)).unwrap(); tx.commit().unwrap(); }) }; @@ -178,7 +178,10 @@ fn get_set_transactions() { let tx = store.transaction_ro().unwrap(); let v1 = tx.get_storage_version().unwrap().unwrap(); let v2 = tx.get_storage_version().unwrap().unwrap(); - assert!([ChainstateStorageVersion::V0, ChainstateStorageVersion::V1].contains(&v1)); + assert!( + [ChainstateStorageVersion::new(0), ChainstateStorageVersion::new(1)] + .contains(&v1) + ); assert_eq!(v1, v2, "Version query in a transaction inconsistent"); }) }; @@ -187,7 +190,7 @@ fn get_set_transactions() { let _ = thr1.join(); assert_eq!( store.get_storage_version(), - Ok(Some(ChainstateStorageVersion::V1)) + Ok(Some(ChainstateStorageVersion::new(1))) ); }) } diff --git a/chainstate/storage/src/internal/version.rs b/chainstate/storage/src/internal/version.rs index 47aef5e70d..9c63d2cad0 100644 --- a/chainstate/storage/src/internal/version.rs +++ b/chainstate/storage/src/internal/version.rs @@ -16,11 +16,12 @@ use serialization::{Decode, Encode}; #[derive(Debug, Encode, Decode, Clone, Copy, Eq, PartialEq)] -pub enum ChainstateStorageVersion { - V0, - V1, -} +pub struct ChainstateStorageVersion(u32); impl ChainstateStorageVersion { - pub const CURRENT: Self = Self::V1; + pub const CURRENT: Self = Self(1); + + pub fn new(value: u32) -> Self { + Self(value) + } } diff --git a/chainstate/storage/src/mock/tests.rs b/chainstate/storage/src/mock/tests.rs index 9270838eba..5a1d7bdf29 100644 --- a/chainstate/storage/src/mock/tests.rs +++ b/chainstate/storage/src/mock/tests.rs @@ -35,7 +35,7 @@ fn basic_mock() { let mut mock = MockStore::new(); mock.expect_set_storage_version().times(1).return_const(Ok(())); - let r = mock.set_storage_version(ChainstateStorageVersion::V0); + let r = mock.set_storage_version(ChainstateStorageVersion::new(0)); assert_eq!(r, Ok(())); } @@ -44,7 +44,7 @@ fn basic_fail() { let mut mock = MockStore::new(); mock.expect_set_storage_version().times(1).return_const(Err(TXFAIL)); - let r = mock.set_storage_version(ChainstateStorageVersion::V0); + let r = mock.set_storage_version(ChainstateStorageVersion::new(0)); assert_eq!(r, Err(TXFAIL)); } @@ -96,7 +96,7 @@ fn mock_transaction() { let mut mock_tx = MockStoreTxRw::new(); mock_tx .expect_set_storage_version() - .with(mockall::predicate::eq(ChainstateStorageVersion::V1)) + .with(mockall::predicate::eq(ChainstateStorageVersion::new(1))) .return_const(Ok(())); mock_tx.expect_commit().times(1).return_const(Ok(())); Ok(mock_tx) @@ -104,7 +104,7 @@ fn mock_transaction() { // Test some code against the mock let mut tx = store.transaction_rw(None).unwrap(); - tx.set_storage_version(ChainstateStorageVersion::V1).unwrap(); + tx.set_storage_version(ChainstateStorageVersion::new(1)).unwrap(); tx.commit().unwrap(); }