Suggestions to simplify the V1 storage requirements and initialization (

#2483)

Suggestions for the #2468 PR.

---------

Co-authored-by: Mitchell Turner <james.mitchell.turner@gmail.com>

crates/fuel-core/src/service/sub_services.rs

@@ -182,15 +182,15 @@ pub fn init_sub_services(
     let genesis_block_height = *genesis_block.header().height();
     let settings = consensus_parameters_provider.clone();
     let block_stream = importer_adapter.events_shared_result();
-    let metadata = StructuredStorage::new(database.gas_price().clone());
+    let metadata = database.gas_price().clone();
 
     let gas_price_service_v0 = new_gas_price_service_v0(
         config.clone().into(),
         genesis_block_height,
         settings,
         block_stream,
         database.gas_price().clone(),
-        metadata,
+        StructuredStorage::new(metadata),
         database.on_chain().clone(),
     )?;
 

crates/fuel-gas-price-algorithm/src/v1.rs

@@ -65,18 +65,20 @@ impl AlgorithmV1 {
 
 pub type Height = u32;
 pub type Bytes = u64;
+
 pub trait UnrecordedBlocks {
-    fn insert(&mut self, height: Height, bytes: Bytes) -> Result<(), Error>;
-    fn remove(&mut self, height: &Height) -> Result<Option<Bytes>, Error>;
+    fn insert(&mut self, height: Height, bytes: Bytes) -> Result<(), String>;
+
+    fn remove(&mut self, height: &Height) -> Result<Option<Bytes>, String>;
 }
 
 impl UnrecordedBlocks for BTreeMap<Height, Bytes> {
-    fn insert(&mut self, height: Height, bytes: Bytes) -> Result<(), Error> {
+    fn insert(&mut self, height: Height, bytes: Bytes) -> Result<(), String> {
         self.insert(height, bytes);
         Ok(())
     }
 
-    fn remove(&mut self, height: &Height) -> Result<Option<Bytes>, Error> {
+    fn remove(&mut self, height: &Height) -> Result<Option<Bytes>, String> {
         let value = self.remove(height);
         Ok(value)
     }
@@ -395,7 +397,9 @@ impl AlgorithmUpdaterV1 {
             self.update_da_gas_price();
 
             // metadata
-            unrecorded_blocks.insert(height, block_bytes)?;
+            unrecorded_blocks
+                .insert(height, block_bytes)
+                .map_err(Error::CouldNotInsertUnrecordedBlock)?;
             self.unrecorded_blocks_bytes = self
                 .unrecorded_blocks_bytes
                 .saturating_add(block_bytes as u128);
@@ -563,7 +567,7 @@ impl AlgorithmUpdaterV1 {
         recording_cost: u128,
         unrecorded_blocks: &mut U,
     ) -> Result<(), Error> {
-        self.update_unrecorded_block_bytes(heights, unrecorded_blocks);
+        self.update_unrecorded_block_bytes(heights, unrecorded_blocks)?;
 
         let new_da_block_cost = self
             .latest_known_total_da_cost_excess
@@ -589,26 +593,25 @@ impl AlgorithmUpdaterV1 {
         &mut self,
         heights: &[u32],
         unrecorded_blocks: &mut U,
-    ) {
+    ) -> Result<(), Error> {
         let mut total: u128 = 0;
         for expected_height in heights {
-            let res = unrecorded_blocks.remove(expected_height);
-            match res {
-                Ok(Some(bytes)) => {
-                    total = total.saturating_add(bytes as u128);
-                }
-                Ok(None) => {
-                    tracing::warn!(
-                        "L2 block expected but not found in unrecorded blocks: {}",
-                        expected_height,
-                    );
-                }
-                Err(err) => {
-                    tracing::error!("Could not remove unrecorded block: {}", err);
-                }
+            let maybe_bytes = unrecorded_blocks
+                .remove(expected_height)
+                .map_err(Error::CouldNotRemoveUnrecordedBlock)?;
+
+            if let Some(bytes) = maybe_bytes {
+                total = total.saturating_add(bytes as u128);
+            } else {
+                tracing::warn!(
+                    "L2 block expected but not found in unrecorded blocks: {}",
+                    expected_height,
+                );
             }
         }
         self.unrecorded_blocks_bytes = self.unrecorded_blocks_bytes.saturating_sub(total);
+
+        Ok(())
     }
 
     fn recalculate_projected_cost(&mut self) {

crates/services/gas_price_service/src/common/fuel_core_storage_adapter.rs

@@ -1,29 +1,27 @@
-use crate::common::utils::{
-    BlockInfo,
-    Error as GasPriceError,
-    Result as GasPriceResult,
-};
-use anyhow::anyhow;
-use fuel_core_types::fuel_types::BlockHeight;
-
 use crate::{
     common::{
         fuel_core_storage_adapter::storage::{
             GasPriceColumn,
             GasPriceMetadata,
             SequenceNumberTable,
-            UnrecordedBlocksTable,
         },
         updater_metadata::UpdaterMetadata,
+        utils::{
+            BlockInfo,
+            Error as GasPriceError,
+            Result as GasPriceResult,
+        },
     },
     ports::{
+        GasPriceServiceAtomicStorage,
         GetDaSequenceNumber,
         GetMetadataStorage,
         SetDaSequenceNumber,
         SetMetadataStorage,
-        TransactionableStorage,
     },
 };
+use anyhow::anyhow;
+use core::cmp::min;
 use fuel_core_storage::{
     codec::{
         postcard::Postcard,
@@ -36,29 +34,26 @@ use fuel_core_storage::{
         StorageTransaction,
         WriteTransaction,
     },
+    Error as StorageError,
     StorageAsMut,
     StorageAsRef,
+    StorageInspect,
 };
 use fuel_core_types::{
     blockchain::{
         block::Block,
         header::ConsensusParametersVersion,
     },
+    fuel_merkle::storage::StorageMutate,
     fuel_tx::{
         field::{
             MintAmount,
             MintGasPrice,
         },
         Transaction,
     },
+    fuel_types::BlockHeight,
 };
-use fuel_gas_price_algorithm::v1::{
-    Bytes,
-    Error,
-    Height,
-    UnrecordedBlocks,
-};
-use std::cmp::min;
 
 #[cfg(test)]
 mod metadata_tests;
@@ -67,8 +62,8 @@ pub mod storage;
 
 impl<Storage> SetMetadataStorage for StructuredStorage<Storage>
 where
-    Storage: KeyValueInspect<Column = GasPriceColumn> + Modifiable,
     Storage: Send + Sync,
+    Storage: KeyValueInspect<Column = GasPriceColumn> + Modifiable,
 {
     fn set_metadata(&mut self, metadata: &UpdaterMetadata) -> GasPriceResult<()> {
         let block_height = metadata.l2_block_height();
@@ -84,9 +79,10 @@ where
     }
 }
 
-impl<Storage> GetMetadataStorage for StructuredStorage<Storage>
+impl<Storage> GetMetadataStorage for Storage
 where
-    Storage: KeyValueInspect<Column = GasPriceColumn> + Send + Sync,
+    Storage: Send + Sync,
+    Storage: StorageInspect<GasPriceMetadata, Error = StorageError>,
 {
     fn get_metadata(
         &self,
@@ -102,9 +98,10 @@ where
     }
 }
 
-impl<Storage> GetDaSequenceNumber for StructuredStorage<Storage>
+impl<Storage> GetDaSequenceNumber for Storage
 where
-    Storage: KeyValueInspect<Column = GasPriceColumn> + Send + Sync,
+    Storage: Send + Sync,
+    Storage: StorageInspect<SequenceNumberTable, Error = StorageError>,
 {
     fn get_sequence_number(
         &self,
@@ -119,137 +116,50 @@ where
     }
 }
 
-pub struct NewStorageTransaction<'a, Storage> {
-    inner: StorageTransaction<&'a mut StructuredStorage<Storage>>,
-}
-
-impl<'a, Storage> NewStorageTransaction<'a, Storage> {
-    fn wrap(inner: StorageTransaction<&'a mut StructuredStorage<Storage>>) -> Self {
-        Self { inner }
-    }
-}
-
-impl<'a, Storage> UnrecordedBlocks for NewStorageTransaction<'a, Storage>
+impl<Storage> GasPriceServiceAtomicStorage for Storage
 where
+    Storage: 'static,
+    Storage: GetMetadataStorage + GetDaSequenceNumber,
     Storage: KeyValueInspect<Column = GasPriceColumn> + Modifiable + Send + Sync,
 {
-    fn insert(&mut self, height: Height, bytes: Bytes) -> Result<(), Error> {
-        self.inner
-            .storage_as_mut::<UnrecordedBlocksTable>()
-            .insert(&height, &bytes)
-            .map_err(|err| {
-                Error::CouldNotInsertUnrecordedBlock(format!("Error: {:?}", err))
-            })?;
-        Ok(())
-    }
-
-    fn remove(&mut self, height: &Height) -> Result<Option<Bytes>, Error> {
-        let bytes = self
-            .inner
-            .storage_as_mut::<UnrecordedBlocksTable>()
-            .take(height)
-            .map_err(|err| {
-                Error::CouldNotRemoveUnrecordedBlock(format!("Error: {:?}", err))
-            })?;
-        Ok(bytes)
-    }
-}
-
-impl<Storage> TransactionableStorage for StructuredStorage<Storage>
-where
-    Storage: Modifiable + Send + Sync,
-{
-    type Transaction<'a> = NewStorageTransaction<'a, Storage> where Self: 'a;
+    type Transaction<'a> = StorageTransaction<&'a mut Storage> where Self: 'a;
 
     fn begin_transaction(&mut self) -> GasPriceResult<Self::Transaction<'_>> {
         let tx = self.write_transaction();
-        let wrapped = NewStorageTransaction::wrap(tx);
-        Ok(wrapped)
+        Ok(tx)
     }
 
     fn commit_transaction(transaction: Self::Transaction<'_>) -> GasPriceResult<()> {
         transaction
-            .inner
             .commit()
             .map_err(|err| GasPriceError::CouldNotCommit(err.into()))?;
         Ok(())
     }
 }
 
-impl<'a, Storage> SetMetadataStorage for NewStorageTransaction<'a, Storage>
-where
-    Storage: KeyValueInspect<Column = GasPriceColumn> + Modifiable + Send + Sync,
-{
-    fn set_metadata(&mut self, metadata: &UpdaterMetadata) -> GasPriceResult<()> {
-        let block_height = metadata.l2_block_height();
-        self.inner
-            .storage_as_mut::<GasPriceMetadata>()
-            .insert(&block_height, metadata)
-            .map_err(|err| GasPriceError::CouldNotSetMetadata {
-                block_height,
-                source_error: err.into(),
-            })?;
-        Ok(())
-    }
-}
-impl<'a, Storage> GetMetadataStorage for NewStorageTransaction<'a, Storage>
-where
-    Storage: KeyValueInspect<Column = GasPriceColumn> + Send + Sync,
-{
-    fn get_metadata(
-        &self,
-        block_height: &BlockHeight,
-    ) -> GasPriceResult<Option<UpdaterMetadata>> {
-        let metadata = self
-            .inner
-            .storage::<GasPriceMetadata>()
-            .get(block_height)
-            .map_err(|err| GasPriceError::CouldNotFetchMetadata {
-                source_error: err.into(),
-            })?;
-        Ok(metadata.map(|inner| inner.into_owned()))
-    }
-}
-
-impl<'a, Storage> SetDaSequenceNumber for NewStorageTransaction<'a, Storage>
+impl<Storage> SetDaSequenceNumber for Storage
 where
-    Storage: KeyValueInspect<Column = GasPriceColumn> + Modifiable + Send + Sync,
+    Storage: Send + Sync,
+    Storage: StorageMutate<SequenceNumberTable, Error = StorageError>,
 {
     fn set_sequence_number(
         &mut self,
         block_height: &BlockHeight,
         sequence_number: u32,
     ) -> GasPriceResult<()> {
-        self.inner
-            .storage_as_mut::<SequenceNumberTable>()
+        self.storage_as_mut::<SequenceNumberTable>()
             .insert(block_height, &sequence_number)
             .map_err(|err| GasPriceError::CouldNotFetchDARecord(err.into()))?;
         Ok(())
     }
 }
-impl<'a, Storage> GetDaSequenceNumber for NewStorageTransaction<'a, Storage>
-where
-    Storage: KeyValueInspect<Column = GasPriceColumn> + Modifiable + Send + Sync,
-{
-    fn get_sequence_number(
-        &self,
-        block_height: &BlockHeight,
-    ) -> GasPriceResult<Option<u32>> {
-        let sequence_number = self
-            .inner
-            .storage::<SequenceNumberTable>()
-            .get(block_height)
-            .map_err(|err| GasPriceError::CouldNotFetchDARecord(err.into()))?
-            .map(|no| *no);
-        Ok(sequence_number)
-    }
-}
 
 #[derive(Debug, Clone, PartialEq)]
 pub struct GasPriceSettings {
     pub gas_price_factor: u64,
     pub block_gas_limit: u64,
 }
+
 pub trait GasPriceSettingsProvider: Send + Sync + Clone {
     fn settings(
         &self,

crates/services/gas_price_service/src/ports.rs

@@ -11,7 +11,10 @@ use crate::{
         utils::Result,
     },
     v0::metadata::V0AlgorithmConfig,
-    v1::metadata::V1AlgorithmConfig,
+    v1::{
+        metadata::V1AlgorithmConfig,
+        uninitialized_task::fuel_storage_unrecorded_blocks::AsUnrecordedBlocks,
+    },
 };
 
 pub trait L2Data: Send + Sync {
@@ -43,12 +46,22 @@ pub trait GetDaSequenceNumber: Send + Sync {
     fn get_sequence_number(&self, block_height: &BlockHeight) -> Result<Option<u32>>;
 }
 
-pub trait TransactionableStorage: Send + Sync {
-    type Transaction<'a>
+pub trait GasPriceServiceAtomicStorage
+where
+    Self: 'static,
+    Self: Send + Sync,
+    Self: GetMetadataStorage + GetDaSequenceNumber,
+{
+    type Transaction<'a>: AsUnrecordedBlocks
+        + SetMetadataStorage
+        + GetMetadataStorage
+        + SetDaSequenceNumber
+        + GetDaSequenceNumber
     where
         Self: 'a;
 
     fn begin_transaction(&mut self) -> Result<Self::Transaction<'_>>;
+
     fn commit_transaction(transaction: Self::Transaction<'_>) -> Result<()>;
 }