Add unrecorded blocks abstraction to gas price algo (#2468)

## Linked Issues/PRs
Closes #2454

## Description
This PR:

- Adds a trait to the V1 algorithm for storing unrecorded blocks
- Adds `TransactionableStorage` trait to gas price service to allow
abstraction for atomic commits to storage (once per L2 block
- Separates `Get` and `Set` traits for service to specify which need a
transaction to execute
- Add a `GetSequenceNumber` and `SetSequence` number concept for
tracking the DA sequence number the DA service should start on at
startup (I can't remember why this got included in this PR, but it felt
logical at the time...)

We only need a key-value lookup for the unrecorded blocks. Instead of
storing all the data in the metadata, we just moved it to its own
abstraction that can be accessed directly from the algorithm. This will
also help with historical view, since historical view doesn't have a way
to iterate over all values for that height, just do KV lookups. We were
iterating before this change.

Also added the `unrecorded_block_bytes` field to metadata so we can
still track that in the case of historical view.

This exposed a problem with the design, which is that we need changes to
the algorithm to be atomic per l2 block, which is how our Fuel Storage
is designed. This led us to introducing a `TransactionableStorage` trait
for the service, which can be handed to the algorithm updater before
being "commit"ed. The same applies to the `MetadataStorage`, so the
service was redesigned slightly to accomodate this change.

## Checklist
- [x] New behavior is reflected in tests

### Before requesting review
- [ ] I have reviewed the code myself

---------

Co-authored-by: Rafał Chabowski <[email protected]>
Co-authored-by: Green Baneling <[email protected]>

CHANGELOG.md

@@ -40,6 +40,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 - [2485](https://github.com/FuelLabs/fuel-core/pull/2485): Hardcode the timestamp of the genesis block and version of `tai64` to avoid breaking changes for us.
 
 ### Changed
+- [2468](https://github.com/FuelLabs/fuel-core/pull/2468): Abstract unrecorded blocks concept for V1 algorithm, create new storage impl. Introduce `TransactionableStorage` trait to allow atomic changes to the storage.
 - [2295](https://github.com/FuelLabs/fuel-core/pull/2295): `CombinedDb::from_config` now respects `state_rewind_policy` with tmp RocksDB.
 - [2378](https://github.com/FuelLabs/fuel-core/pull/2378): Use cached hash of the topic instead of calculating it on each publishing gossip message.
 - [2429](https://github.com/FuelLabs/fuel-core/pull/2429): Introduce custom enum for representing result of running service tasks

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

@@ -19,6 +19,10 @@ pub enum Error {
     FailedToIncludeL2BlockData(String),
     #[error("L2 block expected but not found in unrecorded blocks: {height}")]
     L2BlockExpectedNotFound { height: u32 },
+    #[error("Could not insert unrecorded block: {0}")]
+    CouldNotInsertUnrecordedBlock(String),
+    #[error("Could not remove unrecorded block: {0}")]
+    CouldNotRemoveUnrecordedBlock(String),
 }
 
 // TODO: separate exec gas price and DA gas price into newtypes for clarity
@@ -59,6 +63,27 @@ impl AlgorithmV1 {
     }
 }
 
+pub type Height = u32;
+pub type Bytes = u64;
+
+pub trait UnrecordedBlocks {
+    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<(), String> {
+        self.insert(height, bytes);
+        Ok(())
+    }
+
+    fn remove(&mut self, height: &Height) -> Result<Option<Bytes>, String> {
+        let value = self.remove(height);
+        Ok(value)
+    }
+}
+
 /// The state of the algorithm used to update the gas price algorithm for each block
 ///
 /// Because there will always be a delay between blocks submitted to the L2 chain and the blocks
@@ -96,8 +121,6 @@ impl AlgorithmV1 {
 /// The DA portion also uses a moving average of the profits over the last `avg_window` blocks
 /// instead of the actual profit. Setting the `avg_window` to 1 will effectively disable the
 /// moving average.
-type Height = u32;
-type Bytes = u64;
 #[derive(serde::Serialize, serde::Deserialize, Debug, Clone, PartialEq)]
 pub struct AlgorithmUpdaterV1 {
     // Execution
@@ -143,8 +166,6 @@ pub struct AlgorithmUpdaterV1 {
     pub latest_da_cost_per_byte: u128,
     /// Activity of L2
     pub l2_activity: L2ActivityTracker,
-    /// The unrecorded blocks that are used to calculate the projected cost of recording blocks
-    pub unrecorded_blocks: BTreeMap<Height, Bytes>,
     /// Total unrecorded block bytes
     pub unrecorded_blocks_bytes: u128,
 }
@@ -269,10 +290,28 @@ impl L2ActivityTracker {
     pub fn current_activity(&self) -> u16 {
         self.chain_activity
     }
+
+    pub fn max_activity(&self) -> u16 {
+        self.max_activity
+    }
+
+    pub fn capped_activity_threshold(&self) -> u16 {
+        self.capped_activity_threshold
+    }
+
+    pub fn decrease_activity_threshold(&self) -> u16 {
+        self.decrease_activity_threshold
+    }
+
+    pub fn block_activity_threshold(&self) -> ClampedPercentage {
+        self.block_activity_threshold
+    }
 }
 
 /// A value that represents a value between 0 and 100. Higher values are clamped to 100
-#[derive(serde::Serialize, serde::Deserialize, Debug, Clone, PartialEq, PartialOrd)]
+#[derive(
+    serde::Serialize, serde::Deserialize, Debug, Copy, Clone, PartialEq, PartialOrd,
+)]
 pub struct ClampedPercentage {
     value: u8,
 }
@@ -300,27 +339,34 @@ impl core::ops::Deref for ClampedPercentage {
 }
 
 impl AlgorithmUpdaterV1 {
-    pub fn update_da_record_data(
+    pub fn update_da_record_data<U: UnrecordedBlocks>(
         &mut self,
         heights: &[u32],
         recorded_bytes: u32,
         recording_cost: u128,
+        unrecorded_blocks: &mut U,
     ) -> Result<(), Error> {
         if !heights.is_empty() {
-            self.da_block_update(heights, recorded_bytes as u128, recording_cost)?;
+            self.da_block_update(
+                heights,
+                recorded_bytes as u128,
+                recording_cost,
+                unrecorded_blocks,
+            )?;
             self.recalculate_projected_cost();
             self.update_da_gas_price();
         }
         Ok(())
     }
 
-    pub fn update_l2_block_data(
+    pub fn update_l2_block_data<U: UnrecordedBlocks>(
         &mut self,
         height: u32,
         used: u64,
         capacity: NonZeroU64,
         block_bytes: u64,
         fee_wei: u128,
+        unrecorded_blocks: &mut U,
     ) -> Result<(), Error> {
         let expected = self.l2_block_height.saturating_add(1);
         if height != expected {
@@ -351,7 +397,9 @@ impl AlgorithmUpdaterV1 {
             self.update_da_gas_price();
 
             // metadata
-            self.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);
@@ -512,13 +560,14 @@ impl AlgorithmUpdaterV1 {
             .saturating_div(100)
     }
 
-    fn da_block_update(
+    fn da_block_update<U: UnrecordedBlocks>(
         &mut self,
         heights: &[u32],
         recorded_bytes: u128,
         recording_cost: u128,
+        unrecorded_blocks: &mut U,
     ) -> Result<(), Error> {
-        self.update_unrecorded_block_bytes(heights);
+        self.update_unrecorded_block_bytes(heights, unrecorded_blocks)?;
 
         let new_da_block_cost = self
             .latest_known_total_da_cost_excess
@@ -540,10 +589,17 @@ impl AlgorithmUpdaterV1 {
 
     // Get the bytes for all specified heights, or get none of them.
     // Always remove the blocks from the unrecorded blocks so they don't build up indefinitely
-    fn update_unrecorded_block_bytes(&mut self, heights: &[u32]) {
+    fn update_unrecorded_block_bytes<U: UnrecordedBlocks>(
+        &mut self,
+        heights: &[u32],
+        unrecorded_blocks: &mut U,
+    ) -> Result<(), Error> {
         let mut total: u128 = 0;
         for expected_height in heights {
-            let maybe_bytes = self.unrecorded_blocks.remove(expected_height);
+            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 {
@@ -554,6 +610,8 @@ impl AlgorithmUpdaterV1 {
             }
         }
         self.unrecorded_blocks_bytes = self.unrecorded_blocks_bytes.saturating_sub(total);
+
+        Ok(())
     }
 
     fn recalculate_projected_cost(&mut self) {

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

@@ -4,8 +4,11 @@
 
 use crate::v1::{
     AlgorithmUpdaterV1,
+    Bytes,
+    Height,
     L2ActivityTracker,
 };
+use std::collections::BTreeMap;
 
 #[cfg(test)]
 mod algorithm_v1_tests;
@@ -38,7 +41,7 @@ pub struct UpdaterBuilder {
     da_cost_per_byte: u128,
     project_total_cost: u128,
     latest_known_total_cost: u128,
-    unrecorded_blocks: Vec<BlockBytes>,
+    unrecorded_blocks_bytes: u64,
     last_profit: i128,
     second_to_last_profit: i128,
     da_gas_price_factor: u64,
@@ -65,7 +68,7 @@ impl UpdaterBuilder {
             da_cost_per_byte: 0,
             project_total_cost: 0,
             latest_known_total_cost: 0,
-            unrecorded_blocks: vec![],
+            unrecorded_blocks_bytes: 0,
             last_profit: 0,
             second_to_last_profit: 0,
             da_gas_price_factor: 1,
@@ -146,8 +149,14 @@ impl UpdaterBuilder {
         self
     }
 
-    fn with_unrecorded_blocks(mut self, unrecorded_blocks: Vec<BlockBytes>) -> Self {
-        self.unrecorded_blocks = unrecorded_blocks;
+    fn with_unrecorded_blocks(
+        mut self,
+        unrecorded_blocks: &BTreeMap<Height, Bytes>,
+    ) -> Self {
+        let unrecorded_block_bytes = unrecorded_blocks
+            .iter()
+            .fold(0u64, |acc, (_, bytes)| acc + bytes);
+        self.unrecorded_blocks_bytes = unrecorded_block_bytes;
         self
     }
 
@@ -180,11 +189,6 @@ impl UpdaterBuilder {
             latest_da_cost_per_byte: self.da_cost_per_byte,
             projected_total_da_cost: self.project_total_cost,
             latest_known_total_da_cost_excess: self.latest_known_total_cost,
-            unrecorded_blocks: self
-                .unrecorded_blocks
-                .iter()
-                .map(|b| (b.height, b.block_bytes))
-                .collect(),
             last_profit: self.last_profit,
             second_to_last_profit: self.second_to_last_profit,
             min_da_gas_price: self.min_da_gas_price,
@@ -193,10 +197,7 @@ impl UpdaterBuilder {
                 .try_into()
                 .expect("Should never be non-zero"),
             l2_activity: self.l2_activity,
-            unrecorded_blocks_bytes: self
-                .unrecorded_blocks
-                .iter()
-                .fold(0u128, |acc, b| acc + u128::from(b.block_bytes)),
+            unrecorded_blocks_bytes: self.unrecorded_blocks_bytes as u128,
         }
     }
 }