From 57023715518ed583b977f9184c6dc3376e0c63db Mon Sep 17 00:00:00 2001 From: rymnc <43716372+rymnc@users.noreply.github.com> Date: Fri, 20 Sep 2024 00:52:55 +0530 Subject: [PATCH 01/12] chore(gas_price_service): define port for gas price db lookups --- .../service/adapters/gas_price_adapters.rs | 50 +++++++- .../service/sub_services/algorithm_updater.rs | 119 +++++++++--------- .../src/fuel_gas_price_updater.rs | 20 ++- .../fuel_core_storage_adapter.rs | 52 -------- .../metadata_tests.rs | 61 +++++---- .../src/fuel_gas_price_updater/tests.rs | 17 ++- .../services/gas_price_service/src/ports.rs | 13 ++ 7 files changed, 174 insertions(+), 158 deletions(-) diff --git a/crates/fuel-core/src/service/adapters/gas_price_adapters.rs b/crates/fuel-core/src/service/adapters/gas_price_adapters.rs index b504335f79..06a18a4ccb 100644 --- a/crates/fuel-core/src/service/adapters/gas_price_adapters.rs +++ b/crates/fuel-core/src/service/adapters/gas_price_adapters.rs @@ -5,15 +5,28 @@ use crate::{ use fuel_core_gas_price_service::{ fuel_gas_price_updater::{ fuel_core_storage_adapter::{ + storage::GasPriceMetadata, GasPriceSettings, GasPriceSettingsProvider, }, Error as GasPriceError, Result as GasPriceResult, + UpdaterMetadata, }, - ports::L2Data, + ports::{ + GasPriceData, + L2Data, + }, +}; +use fuel_core_storage::{ + transactional::{ + HistoricalView, + WriteTransaction, + }, + Result as StorageResult, + StorageAsMut, + StorageAsRef, }; -use fuel_core_storage::Result as StorageResult; use fuel_core_types::{ blockchain::{ block::Block, @@ -23,6 +36,11 @@ use fuel_core_types::{ fuel_types::BlockHeight, }; +use crate::database::{ + database_description::gas_price::GasPriceDatabase, + Database, +}; + #[cfg(test)] mod tests; @@ -39,6 +57,34 @@ impl L2Data for OnChainIterableKeyValueView { } } +impl GasPriceData for Database { + fn get_metadata( + &self, + block_height: &BlockHeight, + ) -> StorageResult> { + self.storage::() + .get(block_height) + .map(|metadata| metadata.map(|metadata| metadata.as_ref().clone())) + } + + fn set_metadata(&mut self, metadata: UpdaterMetadata) -> StorageResult<()> { + let height = metadata.l2_block_height(); + let mut tx = self.write_transaction(); + tx.storage_as_mut::() + .insert(&height, &metadata)?; + tx.commit()?; + Ok(()) + } + + fn latest_height(&self) -> Option { + HistoricalView::latest_height(self) + } + + fn rollback_last_block(&self) -> StorageResult<()> { + self.rollback_last_block() + } +} + impl GasPriceSettingsProvider for ConsensusParametersProvider { fn settings( &self, diff --git a/crates/fuel-core/src/service/sub_services/algorithm_updater.rs b/crates/fuel-core/src/service/sub_services/algorithm_updater.rs index f1d8e7308d..cac7bec24b 100644 --- a/crates/fuel-core/src/service/sub_services/algorithm_updater.rs +++ b/crates/fuel-core/src/service/sub_services/algorithm_updater.rs @@ -1,13 +1,6 @@ -use crate::{ - database::{ - database_description::gas_price::GasPriceDatabase, - Database, - RegularStage, - }, - service::{ - adapters::ConsensusParametersProvider, - Config, - }, +use crate::service::{ + adapters::ConsensusParametersProvider, + Config, }; use fuel_core_gas_price_service::{ @@ -19,7 +12,6 @@ use fuel_core_gas_price_service::{ }, fuel_core_storage_adapter::{ get_block_info, - storage::GasPriceMetadata, FuelL2BlockSource, GasPriceSettings, GasPriceSettingsProvider, @@ -29,11 +21,13 @@ use fuel_core_gas_price_service::{ AlgorithmUpdaterV0, BlockInfo, FuelGasPriceUpdater, - MetadataStorage, UpdaterMetadata, V0Metadata, }, - ports::L2Data, + ports::{ + GasPriceData, + L2Data, + }, GasPriceService, SharedGasPriceAlgo, }; @@ -45,51 +39,45 @@ use fuel_core_services::{ }; use fuel_core_storage::{ not_found, - structured_storage::StructuredStorage, - transactional::{ - AtomicView, - HistoricalView, - }, - StorageAsRef, + transactional::AtomicView, }; use fuel_core_types::{ fuel_types::BlockHeight, services::block_importer::SharedImportResult, }; -type Updater = FuelGasPriceUpdater< +type Updater = FuelGasPriceUpdater< FuelL2BlockSource, - MetadataStorageAdapter, + GasPriceStore, DaBlockCostsSharedState, >; -pub struct InitializeTask { +pub struct InitializeTask { pub config: Config, pub genesis_block_height: BlockHeight, pub settings: ConsensusParametersProvider, - pub gas_price_db: Database>, + pub gas_price_db: GasPriceStore, pub on_chain_db: L2DataStoreView, pub block_stream: BoxStream, pub shared_algo: SharedGasPriceAlgo, pub da_block_costs_provider: DaBlockCostsProvider, } -type MetadataStorageAdapter = - StructuredStorage>>; +type Task = GasPriceService>; -type Task = GasPriceService; - -impl InitializeTask +impl + InitializeTask where L2DataStore: L2Data, L2DataStoreView: AtomicView, + GasPriceStore: GasPriceData, { pub fn new( config: Config, genesis_block_height: BlockHeight, settings: ConsensusParametersProvider, block_stream: BoxStream, - gas_price_db: Database>, + gas_price_db: GasPriceStore, on_chain_db: L2DataStoreView, ) -> anyhow::Result { let view = on_chain_db.latest_view()?; @@ -128,16 +116,17 @@ fn get_default_metadata(config: &Config, latest_block_height: u32) -> UpdaterMet }) } -fn get_best_algo( - gas_price_db: &Database>, +fn get_best_algo( + gas_price_db: &GasPriceStore, default_metadata: UpdaterMetadata, -) -> anyhow::Result { +) -> anyhow::Result +where + GasPriceStore: GasPriceData, +{ let best_metadata: UpdaterMetadata = if let Some(height) = gas_price_db.latest_height() { gas_price_db - .storage::() - .get(&height)? - .map(|m| m.into_owned()) + .get_metadata(&height)? .unwrap_or(default_metadata) } else { default_metadata @@ -147,14 +136,16 @@ fn get_best_algo( Ok(algo) } #[async_trait::async_trait] -impl RunnableService for InitializeTask +impl RunnableService + for InitializeTask where L2DataStore: L2Data, L2DataStoreView: AtomicView, + GasPriceStore: GasPriceData, { const NAME: &'static str = "GasPriceUpdater"; type SharedData = SharedGasPriceAlgo; - type Task = Task; + type Task = Task; type TaskParams = (); fn shared_data(&self) -> Self::SharedData { @@ -189,18 +180,19 @@ where } } -pub fn get_synced_gas_price_updater( +pub fn get_synced_gas_price_updater( config: Config, genesis_block_height: BlockHeight, settings: ConsensusParametersProvider, - mut gas_price_db: Database>, + mut gas_price_db: GasPriceStore, on_chain_db: &L2DataStoreView, block_stream: BoxStream, da_block_costs: DaBlockCostsSharedState, -) -> anyhow::Result +) -> anyhow::Result> where L2DataStore: L2Data, L2DataStoreView: AtomicView, + GasPriceStore: GasPriceData, { let mut first_run = false; let latest_block_height: u32 = on_chain_db @@ -228,7 +220,6 @@ where .into(); } - let mut metadata_storage = StructuredStorage::new(gas_price_db); let l2_block_source = FuelL2BlockSource::new(genesis_block_height, settings.clone(), block_stream); @@ -236,15 +227,15 @@ where let updater = FuelGasPriceUpdater::new( default_metadata.into(), l2_block_source, - metadata_storage, + gas_price_db, da_block_costs, ); Ok(updater) } else { if latest_block_height > metadata_height { - sync_metadata_storage_with_on_chain_storage( + sync_gas_price_db_with_on_chain_storage( &settings, - &mut metadata_storage, + &mut gas_price_db, on_chain_db, metadata_height, latest_block_height, @@ -254,7 +245,7 @@ where FuelGasPriceUpdater::init( latest_block_height.into(), l2_block_source, - metadata_storage, + gas_price_db, da_block_costs, config.min_gas_price, config.gas_price_change_percent, @@ -264,11 +255,9 @@ where } } -fn sync_metadata_storage_with_on_chain_storage( +fn sync_gas_price_db_with_on_chain_storage( settings: &ConsensusParametersProvider, - metadata_storage: &mut StructuredStorage< - Database>, - >, + gas_price_db: &mut GasPriceStore, on_chain_db: &L2DataStoreView, metadata_height: u32, latest_block_height: u32, @@ -276,12 +265,14 @@ fn sync_metadata_storage_with_on_chain_storage( where L2DataStore: L2Data, L2DataStoreView: AtomicView, + GasPriceStore: GasPriceData, { - let metadata = metadata_storage - .get_metadata(&metadata_height.into())? - .ok_or(anyhow::anyhow!( - "Expected metadata to exist for height: {metadata_height}" - ))?; + let metadata = + gas_price_db + .get_metadata(&metadata_height.into())? + .ok_or(anyhow::anyhow!( + "Expected metadata to exist for height: {metadata_height}" + ))?; let mut inner: AlgorithmUpdater = metadata.into(); match &mut inner { AlgorithmUpdater::V0(ref mut updater) => { @@ -291,7 +282,7 @@ where metadata_height, latest_block_height, updater, - metadata_storage, + gas_price_db, )?; } AlgorithmUpdater::V1(_) => { @@ -301,19 +292,18 @@ where Ok(()) } -fn sync_v0_metadata( +fn sync_v0_metadata( settings: &ConsensusParametersProvider, on_chain_db: &L2DataStoreView, metadata_height: u32, latest_block_height: u32, updater: &mut AlgorithmUpdaterV0, - metadata_storage: &mut StructuredStorage< - Database>, - >, + gas_price_db: &mut GasPriceStore, ) -> anyhow::Result<()> where L2DataStore: L2Data, L2DataStoreView: AtomicView, + GasPriceStore: GasPriceData, { let first = metadata_height.saturating_add(1); let view = on_chain_db.latest_view()?; @@ -339,16 +329,19 @@ where updater.update_l2_block_data(height, block_gas_used, block_gas_capacity)?; let metadata = AlgorithmUpdater::V0(updater.clone()).into(); - metadata_storage.set_metadata(metadata)?; + gas_price_db.set_metadata(metadata)?; } Ok(()) } -fn revert_gas_price_db_to_height( - gas_price_db: &mut Database>, +fn revert_gas_price_db_to_height( + gas_price_db: &mut GasPriceStore, height: BlockHeight, -) -> anyhow::Result<()> { +) -> anyhow::Result<()> +where + GasPriceStore: GasPriceData, +{ if let Some(gas_price_db_height) = gas_price_db.latest_height() { let gas_price_db_height: u32 = gas_price_db_height.into(); let height: u32 = height.into(); diff --git a/crates/services/gas_price_service/src/fuel_gas_price_updater.rs b/crates/services/gas_price_service/src/fuel_gas_price_updater.rs index 90017c6b9e..327f6d2c84 100644 --- a/crates/services/gas_price_service/src/fuel_gas_price_updater.rs +++ b/crates/services/gas_price_service/src/fuel_gas_price_updater.rs @@ -1,4 +1,5 @@ use crate::{ + ports::GasPriceData, GasPriceAlgorithm, UpdateAlgorithm, }; @@ -200,15 +201,9 @@ impl From for UpdaterMetadata { } } -pub trait MetadataStorage: Send + Sync { - fn get_metadata(&self, block_height: &BlockHeight) - -> Result>; - fn set_metadata(&mut self, metadata: UpdaterMetadata) -> Result<()>; -} - impl FuelGasPriceUpdater where - Metadata: MetadataStorage, + Metadata: GasPriceData, DaBlockCosts: GetDaBlockCosts, { pub fn init( @@ -220,12 +215,13 @@ where exec_gas_price_change_percent: u64, l2_block_fullness_threshold_percent: u64, ) -> Result { - let old_metadata = metadata_storage.get_metadata(&target_block_height)?.ok_or( - Error::CouldNotInitUpdater(anyhow::anyhow!( + let old_metadata = metadata_storage + .get_metadata(&target_block_height) + .map_err(|err| Error::CouldNotInitUpdater(anyhow::anyhow!(err)))? + .ok_or(Error::CouldNotInitUpdater(anyhow::anyhow!( "No metadata found for block height: {:?}", target_block_height - )), - )?; + )))?; let inner = match old_metadata { UpdaterMetadata::V0(old) => { let v0 = AlgorithmUpdaterV0::new( @@ -310,7 +306,7 @@ impl UpdateAlgorithm for FuelGasPriceUpdater where L2: L2BlockSource, - Metadata: MetadataStorage + Send + Sync, + Metadata: GasPriceData, DaBlockCosts: GetDaBlockCosts, { type Algorithm = Algorithm; diff --git a/crates/services/gas_price_service/src/fuel_gas_price_updater/fuel_core_storage_adapter.rs b/crates/services/gas_price_service/src/fuel_gas_price_updater/fuel_core_storage_adapter.rs index a1e4de8ed8..1bf8ce5eae 100644 --- a/crates/services/gas_price_service/src/fuel_gas_price_updater/fuel_core_storage_adapter.rs +++ b/crates/services/gas_price_service/src/fuel_gas_price_updater/fuel_core_storage_adapter.rs @@ -1,29 +1,12 @@ use crate::fuel_gas_price_updater::{ - fuel_core_storage_adapter::storage::{ - GasPriceColumn, - GasPriceMetadata, - }, BlockInfo, - Error, Error as GasPriceError, L2BlockSource, - MetadataStorage, Result, Result as GasPriceResult, - UpdaterMetadata, }; use anyhow::anyhow; use fuel_core_services::stream::BoxStream; -use fuel_core_storage::{ - kv_store::KeyValueInspect, - structured_storage::StructuredStorage, - transactional::{ - Modifiable, - WriteTransaction, - }, - StorageAsMut, - StorageAsRef, -}; use fuel_core_types::fuel_types::BlockHeight; use fuel_core_types::{ @@ -51,41 +34,6 @@ mod l2_source_tests; pub mod storage; -impl MetadataStorage for StructuredStorage -where - Storage: KeyValueInspect + Modifiable, - Storage: Send + Sync, -{ - fn get_metadata( - &self, - block_height: &BlockHeight, - ) -> Result> { - let metadata = self - .storage::() - .get(block_height) - .map_err(|err| Error::CouldNotFetchMetadata { - source_error: err.into(), - })?; - Ok(metadata.map(|inner| inner.into_owned())) - } - - fn set_metadata(&mut self, metadata: UpdaterMetadata) -> Result<()> { - let block_height = metadata.l2_block_height(); - let mut tx = self.write_transaction(); - tx.storage_as_mut::() - .insert(&block_height, &metadata) - .map_err(|err| Error::CouldNotSetMetadata { - block_height, - source_error: err.into(), - })?; - tx.commit().map_err(|err| Error::CouldNotSetMetadata { - block_height, - source_error: err.into(), - })?; - Ok(()) - } -} - pub struct FuelL2BlockSource { genesis_block_height: BlockHeight, gas_price_settings: Settings, diff --git a/crates/services/gas_price_service/src/fuel_gas_price_updater/fuel_core_storage_adapter/metadata_tests.rs b/crates/services/gas_price_service/src/fuel_gas_price_updater/fuel_core_storage_adapter/metadata_tests.rs index 79e6ed748c..371d2f6189 100644 --- a/crates/services/gas_price_service/src/fuel_gas_price_updater/fuel_core_storage_adapter/metadata_tests.rs +++ b/crates/services/gas_price_service/src/fuel_gas_price_updater/fuel_core_storage_adapter/metadata_tests.rs @@ -1,26 +1,30 @@ #![allow(non_snake_case)] -use crate::fuel_gas_price_updater::{ - fuel_core_storage_adapter::storage::GasPriceColumn, - AlgorithmUpdater, +use super::*; +use crate::{ + fuel_gas_price_updater::{ + fuel_core_storage_adapter::storage::{ + GasPriceColumn, + GasPriceMetadata, + }, + AlgorithmUpdater, + UpdaterMetadata, + }, + ports::GasPriceData, }; use fuel_core_storage::{ structured_storage::test::InMemoryStorage, transactional::{ IntoTransaction, + Modifiable, StorageTransaction, }, + Result as StorageResult, StorageAsMut, + StorageAsRef, }; use fuel_gas_price_algorithm::v0::AlgorithmUpdaterV0; -use super::*; - -fn arb_metadata() -> UpdaterMetadata { - let height = 111231u32.into(); - arb_metadata_with_l2_height(height) -} - fn arb_metadata_with_l2_height(l2_height: BlockHeight) -> UpdaterMetadata { let inner = AlgorithmUpdaterV0 { new_exec_price: 100, @@ -36,23 +40,30 @@ fn database() -> StorageTransaction> { InMemoryStorage::default().into_transaction() } -#[tokio::test] -async fn get_metadata__can_get_most_recent_version() { - // given - let mut database = database(); - let block_height: BlockHeight = 1u32.into(); - let metadata = arb_metadata(); - database - .storage_as_mut::() - .insert(&block_height, &metadata) - .unwrap(); +impl GasPriceData for StorageTransaction> { + fn get_metadata( + &self, + block_height: &BlockHeight, + ) -> StorageResult> { + self.storage_as_ref::() + .get(block_height) + .map(|v| v.map(|v| v.as_ref().clone())) + } - // when - let actual = database.get_metadata(&block_height).unwrap(); + fn set_metadata(&mut self, metadata: UpdaterMetadata) -> StorageResult<()> { + self.storage::() + .insert(&metadata.l2_block_height(), &metadata)?; + self.commit_changes(self.changes().clone())?; + Ok(()) + } - // then - let expected = Some(metadata); - assert_eq!(expected, actual); + fn latest_height(&self) -> Option { + todo!() + } + + fn rollback_last_block(&self) -> StorageResult<()> { + todo!() + } } #[tokio::test] diff --git a/crates/services/gas_price_service/src/fuel_gas_price_updater/tests.rs b/crates/services/gas_price_service/src/fuel_gas_price_updater/tests.rs index 6a4a669e05..6ef8b5c699 100644 --- a/crates/services/gas_price_service/src/fuel_gas_price_updater/tests.rs +++ b/crates/services/gas_price_service/src/fuel_gas_price_updater/tests.rs @@ -1,6 +1,7 @@ #![allow(non_snake_case)] use super::*; +use fuel_core_storage::Result as StorageResult; use std::sync::Arc; use tokio::sync::mpsc::Receiver; @@ -37,16 +38,24 @@ impl FakeMetadata { } } -impl MetadataStorage for FakeMetadata { +impl GasPriceData for FakeMetadata { fn get_metadata( &self, _block_height: &BlockHeight, - ) -> Result> { + ) -> StorageResult> { Ok(self.inner.lock().unwrap().clone()) } - fn set_metadata(&mut self, metadata: UpdaterMetadata) -> Result<()> { - let _ = self.inner.lock().unwrap().replace(metadata); + fn set_metadata(&mut self, metadata: UpdaterMetadata) -> StorageResult<()> { + *self.inner.lock().unwrap() = Some(metadata); + Ok(()) + } + + fn latest_height(&self) -> Option { + None + } + + fn rollback_last_block(&self) -> StorageResult<()> { Ok(()) } } diff --git a/crates/services/gas_price_service/src/ports.rs b/crates/services/gas_price_service/src/ports.rs index 7b4169c945..833f20c4e4 100644 --- a/crates/services/gas_price_service/src/ports.rs +++ b/crates/services/gas_price_service/src/ports.rs @@ -1,3 +1,4 @@ +use crate::fuel_gas_price_updater::UpdaterMetadata; use fuel_core_storage::Result as StorageResult; use fuel_core_types::{ blockchain::block::Block, @@ -12,3 +13,15 @@ pub trait L2Data: Send + Sync { height: &BlockHeight, ) -> StorageResult>>; } + +pub trait GasPriceData: Send + Sync { + fn get_metadata( + &self, + block_height: &BlockHeight, + ) -> StorageResult>; + fn set_metadata(&mut self, metadata: UpdaterMetadata) -> StorageResult<()>; + + fn latest_height(&self) -> Option; + + fn rollback_last_block(&self) -> StorageResult<()>; +} From ac699b94cf2f455ad1ac7b7f67c3b93eb1696bc7 Mon Sep 17 00:00:00 2001 From: rymnc <43716372+rymnc@users.noreply.github.com> Date: Fri, 20 Sep 2024 18:15:10 +0530 Subject: [PATCH 02/12] fix: pass metadata by ref --- crates/fuel-core/src/service/adapters/gas_price_adapters.rs | 4 ++-- .../fuel-core/src/service/sub_services/algorithm_updater.rs | 2 +- .../gas_price_service/src/fuel_gas_price_updater.rs | 3 ++- .../fuel_core_storage_adapter/metadata_tests.rs | 6 +++--- .../gas_price_service/src/fuel_gas_price_updater/tests.rs | 4 ++-- crates/services/gas_price_service/src/ports.rs | 2 +- 6 files changed, 11 insertions(+), 10 deletions(-) diff --git a/crates/fuel-core/src/service/adapters/gas_price_adapters.rs b/crates/fuel-core/src/service/adapters/gas_price_adapters.rs index 06a18a4ccb..9e0c7f1800 100644 --- a/crates/fuel-core/src/service/adapters/gas_price_adapters.rs +++ b/crates/fuel-core/src/service/adapters/gas_price_adapters.rs @@ -67,11 +67,11 @@ impl GasPriceData for Database { .map(|metadata| metadata.map(|metadata| metadata.as_ref().clone())) } - fn set_metadata(&mut self, metadata: UpdaterMetadata) -> StorageResult<()> { + fn set_metadata(&mut self, metadata: &UpdaterMetadata) -> StorageResult<()> { let height = metadata.l2_block_height(); let mut tx = self.write_transaction(); tx.storage_as_mut::() - .insert(&height, &metadata)?; + .insert(&height, metadata)?; tx.commit()?; Ok(()) } diff --git a/crates/fuel-core/src/service/sub_services/algorithm_updater.rs b/crates/fuel-core/src/service/sub_services/algorithm_updater.rs index cac7bec24b..324775532e 100644 --- a/crates/fuel-core/src/service/sub_services/algorithm_updater.rs +++ b/crates/fuel-core/src/service/sub_services/algorithm_updater.rs @@ -329,7 +329,7 @@ where updater.update_l2_block_data(height, block_gas_used, block_gas_capacity)?; let metadata = AlgorithmUpdater::V0(updater.clone()).into(); - gas_price_db.set_metadata(metadata)?; + gas_price_db.set_metadata(&metadata)?; } Ok(()) diff --git a/crates/services/gas_price_service/src/fuel_gas_price_updater.rs b/crates/services/gas_price_service/src/fuel_gas_price_updater.rs index 327f6d2c84..b025b0cc06 100644 --- a/crates/services/gas_price_service/src/fuel_gas_price_updater.rs +++ b/crates/services/gas_price_service/src/fuel_gas_price_updater.rs @@ -252,8 +252,9 @@ where } async fn set_metadata(&mut self) -> anyhow::Result<()> { + let metadata = self.inner.clone().into(); self.metadata_storage - .set_metadata(self.inner.clone().into()) + .set_metadata(&metadata) .map_err(|err| anyhow!(err)) } diff --git a/crates/services/gas_price_service/src/fuel_gas_price_updater/fuel_core_storage_adapter/metadata_tests.rs b/crates/services/gas_price_service/src/fuel_gas_price_updater/fuel_core_storage_adapter/metadata_tests.rs index 371d2f6189..649ce292b7 100644 --- a/crates/services/gas_price_service/src/fuel_gas_price_updater/fuel_core_storage_adapter/metadata_tests.rs +++ b/crates/services/gas_price_service/src/fuel_gas_price_updater/fuel_core_storage_adapter/metadata_tests.rs @@ -50,9 +50,9 @@ impl GasPriceData for StorageTransaction> { .map(|v| v.map(|v| v.as_ref().clone())) } - fn set_metadata(&mut self, metadata: UpdaterMetadata) -> StorageResult<()> { + fn set_metadata(&mut self, metadata: &UpdaterMetadata) -> StorageResult<()> { self.storage::() - .insert(&metadata.l2_block_height(), &metadata)?; + .insert(&metadata.l2_block_height(), metadata)?; self.commit_changes(self.changes().clone())?; Ok(()) } @@ -90,7 +90,7 @@ async fn set_metadata__can_set_metadata() { // when let actual = database.get_metadata(&block_height).unwrap(); assert_eq!(None, actual); - database.set_metadata(metadata.clone()).unwrap(); + database.set_metadata(&metadata).unwrap(); let actual = database.get_metadata(&block_height).unwrap(); // then diff --git a/crates/services/gas_price_service/src/fuel_gas_price_updater/tests.rs b/crates/services/gas_price_service/src/fuel_gas_price_updater/tests.rs index 6ef8b5c699..c49f1bb35c 100644 --- a/crates/services/gas_price_service/src/fuel_gas_price_updater/tests.rs +++ b/crates/services/gas_price_service/src/fuel_gas_price_updater/tests.rs @@ -46,8 +46,8 @@ impl GasPriceData for FakeMetadata { Ok(self.inner.lock().unwrap().clone()) } - fn set_metadata(&mut self, metadata: UpdaterMetadata) -> StorageResult<()> { - *self.inner.lock().unwrap() = Some(metadata); + fn set_metadata(&mut self, metadata: &UpdaterMetadata) -> StorageResult<()> { + *self.inner.lock().unwrap() = Some(metadata.clone()); Ok(()) } diff --git a/crates/services/gas_price_service/src/ports.rs b/crates/services/gas_price_service/src/ports.rs index 833f20c4e4..b074840beb 100644 --- a/crates/services/gas_price_service/src/ports.rs +++ b/crates/services/gas_price_service/src/ports.rs @@ -19,7 +19,7 @@ pub trait GasPriceData: Send + Sync { &self, block_height: &BlockHeight, ) -> StorageResult>; - fn set_metadata(&mut self, metadata: UpdaterMetadata) -> StorageResult<()>; + fn set_metadata(&mut self, metadata: &UpdaterMetadata) -> StorageResult<()>; fn latest_height(&self) -> Option; From 100b5b86e092e00e466befb1070d570fa0935f8e Mon Sep 17 00:00:00 2001 From: rymnc <43716372+rymnc@users.noreply.github.com> Date: Mon, 23 Sep 2024 14:53:09 +0530 Subject: [PATCH 03/12] chore: use structured storage strat --- .../service/adapters/gas_price_adapters.rs | 40 +++++++---- .../service/sub_services/algorithm_updater.rs | 70 ++++++++----------- .../src/fuel_gas_price_updater.rs | 6 +- .../fuel_core_storage_adapter.rs | 54 ++++++++++++++ .../metadata_tests.rs | 44 ++---------- .../src/fuel_gas_price_updater/tests.rs | 21 ++---- .../services/gas_price_service/src/ports.rs | 19 ++--- 7 files changed, 130 insertions(+), 124 deletions(-) diff --git a/crates/fuel-core/src/service/adapters/gas_price_adapters.rs b/crates/fuel-core/src/service/adapters/gas_price_adapters.rs index 9e0c7f1800..22689b94f0 100644 --- a/crates/fuel-core/src/service/adapters/gas_price_adapters.rs +++ b/crates/fuel-core/src/service/adapters/gas_price_adapters.rs @@ -16,6 +16,7 @@ use fuel_core_gas_price_service::{ ports::{ GasPriceData, L2Data, + MetadataStorage, }, }; use fuel_core_storage::{ @@ -58,31 +59,40 @@ impl L2Data for OnChainIterableKeyValueView { } impl GasPriceData for Database { + fn latest_height(&self) -> Option { + HistoricalView::latest_height(self) + } +} + +impl MetadataStorage for Database { fn get_metadata( &self, block_height: &BlockHeight, - ) -> StorageResult> { - self.storage::() + ) -> GasPriceResult> { + let metadata = self + .storage::() .get(block_height) - .map(|metadata| metadata.map(|metadata| metadata.as_ref().clone())) + .map_err(|err| GasPriceError::CouldNotFetchMetadata { + source_error: err.into(), + })?; + Ok(metadata.map(|inner| inner.into_owned())) } - fn set_metadata(&mut self, metadata: &UpdaterMetadata) -> StorageResult<()> { - let height = metadata.l2_block_height(); + fn set_metadata(&mut self, metadata: &UpdaterMetadata) -> GasPriceResult<()> { + let block_height = metadata.l2_block_height(); let mut tx = self.write_transaction(); tx.storage_as_mut::() - .insert(&height, metadata)?; - tx.commit()?; + .insert(&block_height, metadata) + .map_err(|err| GasPriceError::CouldNotSetMetadata { + block_height, + source_error: err.into(), + })?; + tx.commit().map_err(|err| GasPriceError::CouldNotSetMetadata { + block_height, + source_error: err.into(), + })?; Ok(()) } - - fn latest_height(&self) -> Option { - HistoricalView::latest_height(self) - } - - fn rollback_last_block(&self) -> StorageResult<()> { - self.rollback_last_block() - } } impl GasPriceSettingsProvider for ConsensusParametersProvider { diff --git a/crates/fuel-core/src/service/sub_services/algorithm_updater.rs b/crates/fuel-core/src/service/sub_services/algorithm_updater.rs index 324775532e..326792f7c9 100644 --- a/crates/fuel-core/src/service/sub_services/algorithm_updater.rs +++ b/crates/fuel-core/src/service/sub_services/algorithm_updater.rs @@ -12,6 +12,7 @@ use fuel_core_gas_price_service::{ }, fuel_core_storage_adapter::{ get_block_info, + storage::GasPriceColumn, FuelL2BlockSource, GasPriceSettings, GasPriceSettingsProvider, @@ -27,6 +28,7 @@ use fuel_core_gas_price_service::{ ports::{ GasPriceData, L2Data, + MetadataStorage, }, GasPriceService, SharedGasPriceAlgo, @@ -38,8 +40,13 @@ use fuel_core_services::{ StateWatcher, }; use fuel_core_storage::{ + kv_store::KeyValueInspect, not_found, - transactional::AtomicView, + structured_storage::StructuredStorage, + transactional::{ + AtomicView, + Modifiable, + }, }; use fuel_core_types::{ fuel_types::BlockHeight, @@ -48,7 +55,7 @@ use fuel_core_types::{ type Updater = FuelGasPriceUpdater< FuelL2BlockSource, - GasPriceStore, + StructuredStorage, DaBlockCostsSharedState, >; @@ -70,7 +77,7 @@ impl where L2DataStore: L2Data, L2DataStoreView: AtomicView, - GasPriceStore: GasPriceData, + GasPriceStore: GasPriceData + MetadataStorage, { pub fn new( config: Config, @@ -121,7 +128,7 @@ fn get_best_algo( default_metadata: UpdaterMetadata, ) -> anyhow::Result where - GasPriceStore: GasPriceData, + GasPriceStore: MetadataStorage + GasPriceData, { let best_metadata: UpdaterMetadata = if let Some(height) = gas_price_db.latest_height() { @@ -141,7 +148,10 @@ impl RunnableService where L2DataStore: L2Data, L2DataStoreView: AtomicView, - GasPriceStore: GasPriceData, + GasPriceStore: GasPriceData + + MetadataStorage + + KeyValueInspect + + Modifiable, { const NAME: &'static str = "GasPriceUpdater"; type SharedData = SharedGasPriceAlgo; @@ -192,7 +202,10 @@ pub fn get_synced_gas_price_updater where L2DataStore: L2Data, L2DataStoreView: AtomicView, - GasPriceStore: GasPriceData, + GasPriceStore: GasPriceData + + MetadataStorage + + KeyValueInspect + + Modifiable, { let mut first_run = false; let latest_block_height: u32 = on_chain_db @@ -202,7 +215,7 @@ where .into(); let maybe_metadata_height = gas_price_db.latest_height(); - let mut metadata_height = if let Some(metadata_height) = maybe_metadata_height { + let metadata_height = if let Some(metadata_height) = maybe_metadata_height { metadata_height.into() } else { first_run = true; @@ -210,24 +223,16 @@ where }; let default_metadata = get_default_metadata(&config, latest_block_height); - if metadata_height > latest_block_height { - revert_gas_price_db_to_height(&mut gas_price_db, latest_block_height.into())?; - metadata_height = gas_price_db - .latest_height() - .ok_or(anyhow::anyhow!( - "Metadata DB height should match the latest block height" - ))? - .into(); - } - let l2_block_source = FuelL2BlockSource::new(genesis_block_height, settings.clone(), block_stream); if BlockHeight::from(latest_block_height) == genesis_block_height || first_run { + let metadata_storage = StructuredStorage::new(gas_price_db); + let updater = FuelGasPriceUpdater::new( default_metadata.into(), l2_block_source, - gas_price_db, + metadata_storage, da_block_costs, ); Ok(updater) @@ -241,11 +246,12 @@ where latest_block_height, )?; } + let metadata_storage = StructuredStorage::new(gas_price_db); FuelGasPriceUpdater::init( latest_block_height.into(), l2_block_source, - gas_price_db, + metadata_storage, da_block_costs, config.min_gas_price, config.gas_price_change_percent, @@ -265,7 +271,7 @@ fn sync_gas_price_db_with_on_chain_storage, - GasPriceStore: GasPriceData, + GasPriceStore: MetadataStorage, { let metadata = gas_price_db @@ -298,12 +304,12 @@ fn sync_v0_metadata( metadata_height: u32, latest_block_height: u32, updater: &mut AlgorithmUpdaterV0, - gas_price_db: &mut GasPriceStore, + metadata_storage: &mut GasPriceStore, ) -> anyhow::Result<()> where L2DataStore: L2Data, L2DataStoreView: AtomicView, - GasPriceStore: GasPriceData, + GasPriceStore: MetadataStorage, { let first = metadata_height.saturating_add(1); let view = on_chain_db.latest_view()?; @@ -329,26 +335,8 @@ where updater.update_l2_block_data(height, block_gas_used, block_gas_capacity)?; let metadata = AlgorithmUpdater::V0(updater.clone()).into(); - gas_price_db.set_metadata(&metadata)?; + metadata_storage.set_metadata(&metadata)?; } Ok(()) } - -fn revert_gas_price_db_to_height( - gas_price_db: &mut GasPriceStore, - height: BlockHeight, -) -> anyhow::Result<()> -where - GasPriceStore: GasPriceData, -{ - if let Some(gas_price_db_height) = gas_price_db.latest_height() { - let gas_price_db_height: u32 = gas_price_db_height.into(); - let height: u32 = height.into(); - let diff = gas_price_db_height.saturating_sub(height); - for _ in 0..diff { - gas_price_db.rollback_last_block()?; - } - } - Ok(()) -} diff --git a/crates/services/gas_price_service/src/fuel_gas_price_updater.rs b/crates/services/gas_price_service/src/fuel_gas_price_updater.rs index b025b0cc06..8cc7c37e26 100644 --- a/crates/services/gas_price_service/src/fuel_gas_price_updater.rs +++ b/crates/services/gas_price_service/src/fuel_gas_price_updater.rs @@ -1,5 +1,5 @@ use crate::{ - ports::GasPriceData, + ports::MetadataStorage, GasPriceAlgorithm, UpdateAlgorithm, }; @@ -203,7 +203,7 @@ impl From for UpdaterMetadata { impl FuelGasPriceUpdater where - Metadata: GasPriceData, + Metadata: MetadataStorage, DaBlockCosts: GetDaBlockCosts, { pub fn init( @@ -307,7 +307,7 @@ impl UpdateAlgorithm for FuelGasPriceUpdater where L2: L2BlockSource, - Metadata: GasPriceData, + Metadata: MetadataStorage, DaBlockCosts: GetDaBlockCosts, { type Algorithm = Algorithm; diff --git a/crates/services/gas_price_service/src/fuel_gas_price_updater/fuel_core_storage_adapter.rs b/crates/services/gas_price_service/src/fuel_gas_price_updater/fuel_core_storage_adapter.rs index 1bf8ce5eae..3b64c17c68 100644 --- a/crates/services/gas_price_service/src/fuel_gas_price_updater/fuel_core_storage_adapter.rs +++ b/crates/services/gas_price_service/src/fuel_gas_price_updater/fuel_core_storage_adapter.rs @@ -1,14 +1,33 @@ use crate::fuel_gas_price_updater::{ BlockInfo, Error as GasPriceError, + Error, L2BlockSource, Result, Result as GasPriceResult, + UpdaterMetadata, }; use anyhow::anyhow; use fuel_core_services::stream::BoxStream; use fuel_core_types::fuel_types::BlockHeight; +use crate::{ + fuel_gas_price_updater::fuel_core_storage_adapter::storage::{ + GasPriceColumn, + GasPriceMetadata, + }, + ports::MetadataStorage, +}; +use fuel_core_storage::{ + kv_store::KeyValueInspect, + structured_storage::StructuredStorage, + transactional::{ + Modifiable, + WriteTransaction, + }, + StorageAsMut, + StorageAsRef, +}; use fuel_core_types::{ blockchain::{ block::Block, @@ -34,6 +53,41 @@ mod l2_source_tests; pub mod storage; +impl MetadataStorage for StructuredStorage +where + Storage: KeyValueInspect + Modifiable, + Storage: Send + Sync, +{ + fn get_metadata( + &self, + block_height: &BlockHeight, + ) -> Result> { + let metadata = self + .storage::() + .get(block_height) + .map_err(|err| Error::CouldNotFetchMetadata { + source_error: err.into(), + })?; + Ok(metadata.map(|inner| inner.into_owned())) + } + + fn set_metadata(&mut self, metadata: &UpdaterMetadata) -> Result<()> { + let block_height = metadata.l2_block_height(); + let mut tx = self.write_transaction(); + tx.storage_as_mut::() + .insert(&block_height, metadata) + .map_err(|err| Error::CouldNotSetMetadata { + block_height, + source_error: err.into(), + })?; + tx.commit().map_err(|err| Error::CouldNotSetMetadata { + block_height, + source_error: err.into(), + })?; + Ok(()) + } +} + pub struct FuelL2BlockSource { genesis_block_height: BlockHeight, gas_price_settings: Settings, diff --git a/crates/services/gas_price_service/src/fuel_gas_price_updater/fuel_core_storage_adapter/metadata_tests.rs b/crates/services/gas_price_service/src/fuel_gas_price_updater/fuel_core_storage_adapter/metadata_tests.rs index 649ce292b7..b0775bcfe4 100644 --- a/crates/services/gas_price_service/src/fuel_gas_price_updater/fuel_core_storage_adapter/metadata_tests.rs +++ b/crates/services/gas_price_service/src/fuel_gas_price_updater/fuel_core_storage_adapter/metadata_tests.rs @@ -1,27 +1,17 @@ #![allow(non_snake_case)] use super::*; -use crate::{ - fuel_gas_price_updater::{ - fuel_core_storage_adapter::storage::{ - GasPriceColumn, - GasPriceMetadata, - }, - AlgorithmUpdater, - UpdaterMetadata, - }, - ports::GasPriceData, +use crate::fuel_gas_price_updater::{ + fuel_core_storage_adapter::storage::GasPriceColumn, + AlgorithmUpdater, + UpdaterMetadata, }; use fuel_core_storage::{ structured_storage::test::InMemoryStorage, transactional::{ IntoTransaction, - Modifiable, StorageTransaction, }, - Result as StorageResult, - StorageAsMut, - StorageAsRef, }; use fuel_gas_price_algorithm::v0::AlgorithmUpdaterV0; @@ -40,32 +30,6 @@ fn database() -> StorageTransaction> { InMemoryStorage::default().into_transaction() } -impl GasPriceData for StorageTransaction> { - fn get_metadata( - &self, - block_height: &BlockHeight, - ) -> StorageResult> { - self.storage_as_ref::() - .get(block_height) - .map(|v| v.map(|v| v.as_ref().clone())) - } - - fn set_metadata(&mut self, metadata: &UpdaterMetadata) -> StorageResult<()> { - self.storage::() - .insert(&metadata.l2_block_height(), metadata)?; - self.commit_changes(self.changes().clone())?; - Ok(()) - } - - fn latest_height(&self) -> Option { - todo!() - } - - fn rollback_last_block(&self) -> StorageResult<()> { - todo!() - } -} - #[tokio::test] async fn get_metadata__returns_none_if_does_not_exist() { // given diff --git a/crates/services/gas_price_service/src/fuel_gas_price_updater/tests.rs b/crates/services/gas_price_service/src/fuel_gas_price_updater/tests.rs index c49f1bb35c..9c4ab7daff 100644 --- a/crates/services/gas_price_service/src/fuel_gas_price_updater/tests.rs +++ b/crates/services/gas_price_service/src/fuel_gas_price_updater/tests.rs @@ -1,7 +1,6 @@ #![allow(non_snake_case)] use super::*; -use fuel_core_storage::Result as StorageResult; use std::sync::Arc; use tokio::sync::mpsc::Receiver; @@ -38,26 +37,16 @@ impl FakeMetadata { } } -impl GasPriceData for FakeMetadata { - fn get_metadata( - &self, - _block_height: &BlockHeight, - ) -> StorageResult> { - Ok(self.inner.lock().unwrap().clone()) +impl MetadataStorage for FakeMetadata { + fn get_metadata(&self, _: &BlockHeight) -> Result> { + let metadata = self.inner.lock().unwrap().clone(); + Ok(metadata) } - fn set_metadata(&mut self, metadata: &UpdaterMetadata) -> StorageResult<()> { + fn set_metadata(&mut self, metadata: &UpdaterMetadata) -> Result<()> { *self.inner.lock().unwrap() = Some(metadata.clone()); Ok(()) } - - fn latest_height(&self) -> Option { - None - } - - fn rollback_last_block(&self) -> StorageResult<()> { - Ok(()) - } } fn arb_metadata() -> UpdaterMetadata { diff --git a/crates/services/gas_price_service/src/ports.rs b/crates/services/gas_price_service/src/ports.rs index b074840beb..868fd8d981 100644 --- a/crates/services/gas_price_service/src/ports.rs +++ b/crates/services/gas_price_service/src/ports.rs @@ -1,4 +1,7 @@ -use crate::fuel_gas_price_updater::UpdaterMetadata; +use crate::fuel_gas_price_updater::{ + Result, + UpdaterMetadata, +}; use fuel_core_storage::Result as StorageResult; use fuel_core_types::{ blockchain::block::Block, @@ -14,14 +17,12 @@ pub trait L2Data: Send + Sync { ) -> StorageResult>>; } -pub trait GasPriceData: Send + Sync { - fn get_metadata( - &self, - block_height: &BlockHeight, - ) -> StorageResult>; - fn set_metadata(&mut self, metadata: &UpdaterMetadata) -> StorageResult<()>; +pub trait MetadataStorage: Send + Sync { + fn get_metadata(&self, block_height: &BlockHeight) + -> Result>; + fn set_metadata(&mut self, metadata: &UpdaterMetadata) -> Result<()>; +} +pub trait GasPriceData: Send + Sync { fn latest_height(&self) -> Option; - - fn rollback_last_block(&self) -> StorageResult<()>; } From 9575a21b48ae46f597bcd5b80e550d8dc207da45 Mon Sep 17 00:00:00 2001 From: rymnc <43716372+rymnc@users.noreply.github.com> Date: Mon, 23 Sep 2024 15:08:24 +0530 Subject: [PATCH 04/12] chore: rollback centrally --- crates/fuel-core/src/combined_database.rs | 12 ++++++++++++ crates/fuel-core/src/service.rs | 1 + .../src/service/adapters/gas_price_adapters.rs | 9 +++++---- 3 files changed, 18 insertions(+), 4 deletions(-) diff --git a/crates/fuel-core/src/combined_database.rs b/crates/fuel-core/src/combined_database.rs index 1cc42680be..b798c7c2d5 100644 --- a/crates/fuel-core/src/combined_database.rs +++ b/crates/fuel-core/src/combined_database.rs @@ -311,6 +311,18 @@ impl CombinedDatabase { Ok(()) } + + pub fn sync_aux_db_heights(&self, shutdown_listener: &mut S) -> anyhow::Result<()> + where + S: ShutdownListener, + { + let on_chain_height = self + .on_chain() + .latest_height_from_metadata()? + .ok_or(anyhow::anyhow!("on-chain database doesn't have height"))?; + + self.rollback_to(on_chain_height, shutdown_listener) + } } /// A trait for listening to shutdown signals. diff --git a/crates/fuel-core/src/service.rs b/crates/fuel-core/src/service.rs index 4894a55f69..b0e4bb0459 100644 --- a/crates/fuel-core/src/service.rs +++ b/crates/fuel-core/src/service.rs @@ -134,6 +134,7 @@ impl FuelService { // initialize sub services tracing::info!("Initializing sub services"); + database.sync_aux_db_heights(shutdown_listener)?; let (services, shared) = sub_services::init_sub_services(&config, database)?; let sub_services = Arc::new(services); diff --git a/crates/fuel-core/src/service/adapters/gas_price_adapters.rs b/crates/fuel-core/src/service/adapters/gas_price_adapters.rs index 22689b94f0..8e499c844b 100644 --- a/crates/fuel-core/src/service/adapters/gas_price_adapters.rs +++ b/crates/fuel-core/src/service/adapters/gas_price_adapters.rs @@ -87,10 +87,11 @@ impl MetadataStorage for Database { block_height, source_error: err.into(), })?; - tx.commit().map_err(|err| GasPriceError::CouldNotSetMetadata { - block_height, - source_error: err.into(), - })?; + tx.commit() + .map_err(|err| GasPriceError::CouldNotSetMetadata { + block_height, + source_error: err.into(), + })?; Ok(()) } } From e5f5d92051757362ece3ab810f42b9d30cfd6f36 Mon Sep 17 00:00:00 2001 From: rymnc <43716372+rymnc@users.noreply.github.com> Date: Mon, 23 Sep 2024 16:35:00 +0530 Subject: [PATCH 05/12] fix: dont sync if nothing in onchain db --- crates/fuel-core/src/combined_database.rs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/crates/fuel-core/src/combined_database.rs b/crates/fuel-core/src/combined_database.rs index b798c7c2d5..08a7ddbd23 100644 --- a/crates/fuel-core/src/combined_database.rs +++ b/crates/fuel-core/src/combined_database.rs @@ -316,12 +316,11 @@ impl CombinedDatabase { where S: ShutdownListener, { - let on_chain_height = self - .on_chain() - .latest_height_from_metadata()? - .ok_or(anyhow::anyhow!("on-chain database doesn't have height"))?; + if let Some(on_chain_height) = self.on_chain().latest_height_from_metadata()? { + self.rollback_to(on_chain_height, shutdown_listener)?; + }; - self.rollback_to(on_chain_height, shutdown_listener) + Ok(()) } } From b2aa337905669638a23a2be336eb28c34142dbbf Mon Sep 17 00:00:00 2001 From: rymnc <43716372+rymnc@users.noreply.github.com> Date: Mon, 23 Sep 2024 17:08:52 +0530 Subject: [PATCH 06/12] test: swallow error for soft rollback --- crates/fuel-core/src/combined_database.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/fuel-core/src/combined_database.rs b/crates/fuel-core/src/combined_database.rs index 08a7ddbd23..b95d70e42c 100644 --- a/crates/fuel-core/src/combined_database.rs +++ b/crates/fuel-core/src/combined_database.rs @@ -317,7 +317,7 @@ impl CombinedDatabase { S: ShutdownListener, { if let Some(on_chain_height) = self.on_chain().latest_height_from_metadata()? { - self.rollback_to(on_chain_height, shutdown_listener)?; + let _ = self.rollback_to(on_chain_height, shutdown_listener); }; Ok(()) From 295301924b7d99a3642a561407e714ba1de12b6f Mon Sep 17 00:00:00 2001 From: rymnc <43716372+rymnc@users.noreply.github.com> Date: Mon, 23 Sep 2024 18:51:44 +0530 Subject: [PATCH 07/12] fix: log warning if rollback failed --- crates/fuel-core/src/combined_database.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/crates/fuel-core/src/combined_database.rs b/crates/fuel-core/src/combined_database.rs index b95d70e42c..a5f527969f 100644 --- a/crates/fuel-core/src/combined_database.rs +++ b/crates/fuel-core/src/combined_database.rs @@ -317,7 +317,11 @@ impl CombinedDatabase { S: ShutdownListener, { if let Some(on_chain_height) = self.on_chain().latest_height_from_metadata()? { - let _ = self.rollback_to(on_chain_height, shutdown_listener); + // todo(https://github.com/FuelLabs/fuel-core/issues/2239): This is a temporary fix + let res = self.rollback_to(on_chain_height, shutdown_listener); + if res.is_err() { + tracing::warn!("Failed to rollback auxiliary databases to on-chain database height: {:?}", res); + } }; Ok(()) From 62343d76a85d007ca67452f4f7cf6503954de447 Mon Sep 17 00:00:00 2001 From: rymnc <43716372+rymnc@users.noreply.github.com> Date: Tue, 24 Sep 2024 18:28:25 +0530 Subject: [PATCH 08/12] fix: retain test --- .../metadata_tests.rs | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/crates/services/gas_price_service/src/fuel_gas_price_updater/fuel_core_storage_adapter/metadata_tests.rs b/crates/services/gas_price_service/src/fuel_gas_price_updater/fuel_core_storage_adapter/metadata_tests.rs index b0775bcfe4..070f7087d9 100644 --- a/crates/services/gas_price_service/src/fuel_gas_price_updater/fuel_core_storage_adapter/metadata_tests.rs +++ b/crates/services/gas_price_service/src/fuel_gas_price_updater/fuel_core_storage_adapter/metadata_tests.rs @@ -15,6 +15,11 @@ use fuel_core_storage::{ }; use fuel_gas_price_algorithm::v0::AlgorithmUpdaterV0; +fn arb_metadata() -> UpdaterMetadata { + let height = 111231u32.into(); + arb_metadata_with_l2_height(height) +} + fn arb_metadata_with_l2_height(l2_height: BlockHeight) -> UpdaterMetadata { let inner = AlgorithmUpdaterV0 { new_exec_price: 100, @@ -30,6 +35,25 @@ fn database() -> StorageTransaction> { InMemoryStorage::default().into_transaction() } +#[tokio::test] +async fn get_metadata__can_get_most_recent_version() { + // given + let mut database = database(); + let block_height: BlockHeight = 1u32.into(); + let metadata = arb_metadata(); + database + .storage_as_mut::() + .insert(&block_height, &metadata) + .unwrap(); + + // when + let actual = database.get_metadata(&block_height).unwrap(); + + // then + let expected = Some(metadata); + assert_eq!(expected, actual); +} + #[tokio::test] async fn get_metadata__returns_none_if_does_not_exist() { // given From d51d494f5b2d6555fa7655c23ea6d14020e686f8 Mon Sep 17 00:00:00 2001 From: rymnc <43716372+rymnc@users.noreply.github.com> Date: Tue, 24 Sep 2024 18:30:43 +0530 Subject: [PATCH 09/12] chore: add comment --- crates/services/gas_price_service/src/ports.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/services/gas_price_service/src/ports.rs b/crates/services/gas_price_service/src/ports.rs index 868fd8d981..597c8f9e8f 100644 --- a/crates/services/gas_price_service/src/ports.rs +++ b/crates/services/gas_price_service/src/ports.rs @@ -23,6 +23,7 @@ pub trait MetadataStorage: Send + Sync { fn set_metadata(&mut self, metadata: &UpdaterMetadata) -> Result<()>; } +/// To be linked with the GasPriceDatabase: Database pub trait GasPriceData: Send + Sync { fn latest_height(&self) -> Option; } From 417459d41525cb6a270188421c148b6e350d31b9 Mon Sep 17 00:00:00 2001 From: rymnc <43716372+rymnc@users.noreply.github.com> Date: Tue, 24 Sep 2024 18:59:50 +0530 Subject: [PATCH 10/12] fix: remove impl for Database --- .../service/adapters/gas_price_adapters.rs | 42 +-------------- .../service/sub_services/algorithm_updater.rs | 53 ++++++++----------- 2 files changed, 24 insertions(+), 71 deletions(-) diff --git a/crates/fuel-core/src/service/adapters/gas_price_adapters.rs b/crates/fuel-core/src/service/adapters/gas_price_adapters.rs index 8e499c844b..80c426f292 100644 --- a/crates/fuel-core/src/service/adapters/gas_price_adapters.rs +++ b/crates/fuel-core/src/service/adapters/gas_price_adapters.rs @@ -5,28 +5,20 @@ use crate::{ use fuel_core_gas_price_service::{ fuel_gas_price_updater::{ fuel_core_storage_adapter::{ - storage::GasPriceMetadata, GasPriceSettings, GasPriceSettingsProvider, }, Error as GasPriceError, Result as GasPriceResult, - UpdaterMetadata, }, ports::{ GasPriceData, L2Data, - MetadataStorage, }, }; use fuel_core_storage::{ - transactional::{ - HistoricalView, - WriteTransaction, - }, + transactional::HistoricalView, Result as StorageResult, - StorageAsMut, - StorageAsRef, }; use fuel_core_types::{ blockchain::{ @@ -64,38 +56,6 @@ impl GasPriceData for Database { } } -impl MetadataStorage for Database { - fn get_metadata( - &self, - block_height: &BlockHeight, - ) -> GasPriceResult> { - let metadata = self - .storage::() - .get(block_height) - .map_err(|err| GasPriceError::CouldNotFetchMetadata { - source_error: err.into(), - })?; - Ok(metadata.map(|inner| inner.into_owned())) - } - - fn set_metadata(&mut self, metadata: &UpdaterMetadata) -> GasPriceResult<()> { - let block_height = metadata.l2_block_height(); - let mut tx = self.write_transaction(); - tx.storage_as_mut::() - .insert(&block_height, metadata) - .map_err(|err| GasPriceError::CouldNotSetMetadata { - block_height, - source_error: err.into(), - })?; - tx.commit() - .map_err(|err| GasPriceError::CouldNotSetMetadata { - block_height, - source_error: err.into(), - })?; - Ok(()) - } -} - impl GasPriceSettingsProvider for ConsensusParametersProvider { fn settings( &self, diff --git a/crates/fuel-core/src/service/sub_services/algorithm_updater.rs b/crates/fuel-core/src/service/sub_services/algorithm_updater.rs index 326792f7c9..df59d8759a 100644 --- a/crates/fuel-core/src/service/sub_services/algorithm_updater.rs +++ b/crates/fuel-core/src/service/sub_services/algorithm_updater.rs @@ -77,21 +77,21 @@ impl where L2DataStore: L2Data, L2DataStoreView: AtomicView, - GasPriceStore: GasPriceData + MetadataStorage, + GasPriceStore: GasPriceData + Modifiable + KeyValueInspect, { pub fn new( config: Config, genesis_block_height: BlockHeight, settings: ConsensusParametersProvider, block_stream: BoxStream, - gas_price_db: GasPriceStore, + mut gas_price_db: GasPriceStore, on_chain_db: L2DataStoreView, ) -> anyhow::Result { let view = on_chain_db.latest_view()?; let latest_block_height = view.latest_height().unwrap_or(genesis_block_height).into(); let default_metadata = get_default_metadata(&config, latest_block_height); - let algo = get_best_algo(&gas_price_db, default_metadata)?; + let algo = get_best_algo(&mut gas_price_db, default_metadata)?; let shared_algo = SharedGasPriceAlgo::new_with_algorithm(algo); // there's no use of this source yet, so we can safely return an error let da_block_costs_source = @@ -124,15 +124,16 @@ fn get_default_metadata(config: &Config, latest_block_height: u32) -> UpdaterMet } fn get_best_algo( - gas_price_db: &GasPriceStore, + gas_price_db: &mut GasPriceStore, default_metadata: UpdaterMetadata, ) -> anyhow::Result where - GasPriceStore: MetadataStorage + GasPriceData, + GasPriceStore: GasPriceData + Modifiable + KeyValueInspect, { let best_metadata: UpdaterMetadata = if let Some(height) = gas_price_db.latest_height() { - gas_price_db + let metadata_storage = StructuredStorage::new(gas_price_db); + metadata_storage .get_metadata(&height)? .unwrap_or(default_metadata) } else { @@ -148,10 +149,7 @@ impl RunnableService where L2DataStore: L2Data, L2DataStoreView: AtomicView, - GasPriceStore: GasPriceData - + MetadataStorage - + KeyValueInspect - + Modifiable, + GasPriceStore: GasPriceData + Modifiable + KeyValueInspect, { const NAME: &'static str = "GasPriceUpdater"; type SharedData = SharedGasPriceAlgo; @@ -194,7 +192,7 @@ pub fn get_synced_gas_price_updater config: Config, genesis_block_height: BlockHeight, settings: ConsensusParametersProvider, - mut gas_price_db: GasPriceStore, + gas_price_db: GasPriceStore, on_chain_db: &L2DataStoreView, block_stream: BoxStream, da_block_costs: DaBlockCostsSharedState, @@ -202,10 +200,7 @@ pub fn get_synced_gas_price_updater where L2DataStore: L2Data, L2DataStoreView: AtomicView, - GasPriceStore: GasPriceData - + MetadataStorage - + KeyValueInspect - + Modifiable, + GasPriceStore: GasPriceData + Modifiable + KeyValueInspect, { let mut first_run = false; let latest_block_height: u32 = on_chain_db @@ -226,9 +221,9 @@ where let l2_block_source = FuelL2BlockSource::new(genesis_block_height, settings.clone(), block_stream); - if BlockHeight::from(latest_block_height) == genesis_block_height || first_run { - let metadata_storage = StructuredStorage::new(gas_price_db); + let mut metadata_storage = StructuredStorage::new(gas_price_db); + if BlockHeight::from(latest_block_height) == genesis_block_height || first_run { let updater = FuelGasPriceUpdater::new( default_metadata.into(), l2_block_source, @@ -240,13 +235,12 @@ where if latest_block_height > metadata_height { sync_gas_price_db_with_on_chain_storage( &settings, - &mut gas_price_db, + &mut metadata_storage, on_chain_db, metadata_height, latest_block_height, )?; } - let metadata_storage = StructuredStorage::new(gas_price_db); FuelGasPriceUpdater::init( latest_block_height.into(), @@ -263,7 +257,7 @@ where fn sync_gas_price_db_with_on_chain_storage( settings: &ConsensusParametersProvider, - gas_price_db: &mut GasPriceStore, + metadata_storage: &mut StructuredStorage, on_chain_db: &L2DataStoreView, metadata_height: u32, latest_block_height: u32, @@ -271,14 +265,13 @@ fn sync_gas_price_db_with_on_chain_storage, - GasPriceStore: MetadataStorage, + GasPriceStore: GasPriceData + Modifiable + KeyValueInspect, { - let metadata = - gas_price_db - .get_metadata(&metadata_height.into())? - .ok_or(anyhow::anyhow!( - "Expected metadata to exist for height: {metadata_height}" - ))?; + let metadata = metadata_storage + .get_metadata(&metadata_height.into())? + .ok_or(anyhow::anyhow!( + "Expected metadata to exist for height: {metadata_height}" + ))?; let mut inner: AlgorithmUpdater = metadata.into(); match &mut inner { AlgorithmUpdater::V0(ref mut updater) => { @@ -288,7 +281,7 @@ where metadata_height, latest_block_height, updater, - gas_price_db, + metadata_storage, )?; } AlgorithmUpdater::V1(_) => { @@ -304,12 +297,12 @@ fn sync_v0_metadata( metadata_height: u32, latest_block_height: u32, updater: &mut AlgorithmUpdaterV0, - metadata_storage: &mut GasPriceStore, + metadata_storage: &mut StructuredStorage, ) -> anyhow::Result<()> where L2DataStore: L2Data, L2DataStoreView: AtomicView, - GasPriceStore: MetadataStorage, + GasPriceStore: GasPriceData + Modifiable + KeyValueInspect, { let first = metadata_height.saturating_add(1); let view = on_chain_db.latest_view()?; From 2e6a9d35648709d93cb5ff7381995eb91b63ebb4 Mon Sep 17 00:00:00 2001 From: rymnc <43716372+rymnc@users.noreply.github.com> Date: Tue, 24 Sep 2024 19:00:45 +0530 Subject: [PATCH 11/12] fix: docstring --- crates/services/gas_price_service/src/ports.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/services/gas_price_service/src/ports.rs b/crates/services/gas_price_service/src/ports.rs index 597c8f9e8f..dbb0d03913 100644 --- a/crates/services/gas_price_service/src/ports.rs +++ b/crates/services/gas_price_service/src/ports.rs @@ -23,7 +23,7 @@ pub trait MetadataStorage: Send + Sync { fn set_metadata(&mut self, metadata: &UpdaterMetadata) -> Result<()>; } -/// To be linked with the GasPriceDatabase: Database +/// To be linked with the GasPriceDatabase: `Database` pub trait GasPriceData: Send + Sync { fn latest_height(&self) -> Option; } From 64818a6c94d15ac84a9c049f674ec4b78fa00939 Mon Sep 17 00:00:00 2001 From: rymnc <43716372+rymnc@users.noreply.github.com> Date: Tue, 24 Sep 2024 19:03:40 +0530 Subject: [PATCH 12/12] fix: improved comment for GasPriceData --- crates/services/gas_price_service/src/ports.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/crates/services/gas_price_service/src/ports.rs b/crates/services/gas_price_service/src/ports.rs index dbb0d03913..2dd56a8d90 100644 --- a/crates/services/gas_price_service/src/ports.rs +++ b/crates/services/gas_price_service/src/ports.rs @@ -23,7 +23,9 @@ pub trait MetadataStorage: Send + Sync { fn set_metadata(&mut self, metadata: &UpdaterMetadata) -> Result<()>; } -/// To be linked with the GasPriceDatabase: `Database` +/// Provides the latest block height. +/// This is used to determine the latest block height that has been processed by the gas price service. +/// We need this to fetch the gas price data for the latest block. pub trait GasPriceData: Send + Sync { fn latest_height(&self) -> Option; }