Small suggestions and simplification to the balances indexation PR (#…

…2465)

Suggestions and simplifications for the
#2383.

CHANGELOG.md

@@ -43,8 +43,6 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 
 #### Breaking
 - [2389](https://github.com/FuelLabs/fuel-core/pull/2258): Updated the `messageProof` GraphQL schema to return a non-nullable `MessageProof`.
-
-#### Breaking
 - [2383](https://github.com/FuelLabs/fuel-core/pull/2383): Asset balance queries now return U128 instead of U64. 
 - [2154](https://github.com/FuelLabs/fuel-core/pull/2154): Transaction graphql endpoints use `TransactionType` instead of `fuel_tx::Transaction`.
 - [2446](https://github.com/FuelLabs/fuel-core/pull/2446): Use graphiql instead of graphql-playground due to known vulnerability and stale development.

crates/fuel-core/src/combined_database.rs

@@ -7,11 +7,7 @@ use crate::{
             off_chain::OffChain,
             on_chain::OnChain,
             relayer::Relayer,
-            DatabaseDescription,
-            DatabaseMetadata,
-            IndexationKind,
         },
-        metadata::MetadataTable,
         Database,
         GenesisDatabase,
         Result as DatabaseResult,
@@ -32,12 +28,7 @@ use fuel_core_storage::tables::{
     ContractsState,
     Messages,
 };
-use fuel_core_storage::{
-    transactional::ReadTransaction,
-    Result as StorageResult,
-    StorageAsMut,
-};
-use fuel_core_txpool::ports::AtomicView;
+use fuel_core_storage::Result as StorageResult;
 use fuel_core_types::fuel_types::BlockHeight;
 use std::path::PathBuf;
 
@@ -178,40 +169,6 @@ impl CombinedDatabase {
         Ok(())
     }
 
-    pub fn initialize(&self) -> StorageResult<()> {
-        self.initialize_indexation()?;
-        Ok(())
-    }
-
-    fn initialize_indexation(&self) -> StorageResult<()> {
-        // When genesis is missing write to the database that balances cache should be used.
-        let on_chain_view = self.on_chain().latest_view()?;
-        if on_chain_view.get_genesis().is_err() {
-            let all_indexations = IndexationKind::all().collect();
-            tracing::info!(
-                "No genesis, initializing metadata with all supported indexations: {:?}",
-                all_indexations
-            );
-            let off_chain_view = self.off_chain().latest_view()?;
-            let mut database_tx = off_chain_view.read_transaction();
-            database_tx
-                .storage_as_mut::<MetadataTable<OffChain>>()
-                .insert(
-                    &(),
-                    &DatabaseMetadata::V2 {
-                        version: <OffChain as DatabaseDescription>::version(),
-                        height: Default::default(),
-                        indexation_availability: all_indexations,
-                    },
-                )?;
-            self.off_chain()
-                .data
-                .commit_changes(None, database_tx.into_changes())?;
-        };
-
-        Ok(())
-    }
-
     pub fn on_chain(&self) -> &Database<OnChain> {
         &self.on_chain
     }

crates/fuel-core/src/database.rs

@@ -24,7 +24,6 @@ use crate::{
         KeyValueView,
     },
 };
-use database_description::IndexationKind;
 use fuel_core_chain_config::TableEntry;
 use fuel_core_gas_price_service::common::fuel_core_storage_adapter::storage::GasPriceMetadata;
 use fuel_core_services::SharedMutex;
@@ -68,7 +67,10 @@ pub use fuel_core_database::Error;
 pub type Result<T> = core::result::Result<T, Error>;
 
 // TODO: Extract `Database` and all belongs into `fuel-core-database`.
-use crate::database::database_description::gas_price::GasPriceDatabase;
+use crate::database::database_description::{
+    gas_price::GasPriceDatabase,
+    indexation_availability,
+};
 #[cfg(feature = "rocksdb")]
 use crate::state::{
     historical_rocksdb::{
@@ -536,7 +538,7 @@ where
         None => DatabaseMetadata::V2 {
             version: Description::version(),
             height: new_height,
-            indexation_availability: IndexationKind::all().collect(),
+            indexation_availability: indexation_availability::<Description>(None),
         },
     };
     updated_metadata
@@ -1115,20 +1117,19 @@ mod tests {
     }
 
     mod metadata {
+        use crate::database::database_description::IndexationKind;
+        use fuel_core_storage::kv_store::StorageColumn;
         use std::{
             borrow::Cow,
             collections::HashSet,
         };
-
-        use fuel_core_storage::kv_store::StorageColumn;
         use strum::EnumCount;
 
         use super::{
             database_description::DatabaseDescription,
             update_metadata,
             DatabaseHeight,
             DatabaseMetadata,
-            IndexationKind,
         };
 
         #[derive(Debug, Copy, Clone, Default, PartialEq, Eq)]

crates/fuel-core/src/database/database_description.rs

@@ -132,3 +132,22 @@ impl<Height> DatabaseMetadata<Height> {
         }
     }
 }
+
+/// Gets the indexation availability from the metadata.
+pub fn indexation_availability<D>(
+    metadata: Option<DatabaseMetadata<D::Height>>,
+) -> HashSet<IndexationKind>
+where
+    D: DatabaseDescription,
+{
+    match metadata {
+        Some(DatabaseMetadata::V1 { .. }) => HashSet::new(),
+        Some(DatabaseMetadata::V2 {
+            indexation_availability,
+            ..
+        }) => indexation_availability.clone(),
+        // If the metadata doesn't exist, it is a new database,
+        // and we should set all indexation kinds to available.
+        None => IndexationKind::all().collect(),
+    }
+}

crates/fuel-core/src/database/metadata.rs

@@ -1,3 +1,7 @@
+use super::database_description::{
+    indexation_availability,
+    IndexationKind,
+};
 use crate::database::{
     database_description::{
         DatabaseDescription,
@@ -17,8 +21,6 @@ use fuel_core_storage::{
     StorageInspect,
 };
 
-use super::database_description::IndexationKind;
-
 /// The table that stores all metadata about the database.
 pub struct MetadataTable<Description>(core::marker::PhantomData<Description>);
 
@@ -78,10 +80,12 @@ where
     }
 
     pub fn indexation_available(&self, kind: IndexationKind) -> StorageResult<bool> {
-        let Some(metadata) = self.storage::<MetadataTable<Description>>().get(&())?
-        else {
-            return Ok(false)
-        };
-        Ok(metadata.indexation_available(kind))
+        let metadata = self
+            .storage::<MetadataTable<Description>>()
+            .get(&())?
+            .map(|metadata| metadata.into_owned());
+
+        let indexation_availability = indexation_availability::<Description>(metadata);
+        Ok(indexation_availability.contains(&kind))
     }
 }

crates/fuel-core/src/graphql_api.rs

@@ -80,7 +80,10 @@ impl Default for Costs {
 }
 
 pub const DEFAULT_QUERY_COSTS: Costs = Costs {
-    balance_query: 40001, /* Cost will depend on whether balances index is available or not, but let's keep the default high to be on the safe side */
+    // TODO: The cost of the `balance` and `balances` query should depend on the
+    //  `OffChainDatabase::balances_enabled` value. If additional indexation is enabled,
+    //  the cost should be cheaper.
+    balance_query: 40001,
     coins_to_spend: 40001,
     get_peers: 40001,
     estimate_predicates: 40001,

crates/fuel-core/src/graphql_api/indexation.rs

@@ -65,12 +65,12 @@ where
 {
     let key = message.recipient();
     let storage = block_st_transaction.storage::<MessageBalances>();
-    let current_balance = storage.get(key)?.unwrap_or_default();
+    let current_balance = storage.get(key)?.unwrap_or_default().into_owned();
     let MessageBalance {
         mut retryable,
         mut non_retryable,
-    } = *current_balance;
-    if message.has_retryable_amount() {
+    } = current_balance;
+    if message.is_retryable_message() {
         retryable = retryable.saturating_add(message.amount() as u128);
     } else {
         non_retryable = non_retryable.saturating_add(message.amount() as u128);
@@ -80,8 +80,10 @@ where
         non_retryable,
     };
 
-    let storage = block_st_transaction.storage::<MessageBalances>();
-    Ok(storage.insert(key, &new_balance)?)
+    block_st_transaction
+        .storage::<MessageBalances>()
+        .insert(key, &new_balance)
+        .map_err(Into::into)
 }
 
 fn decrease_message_balance<T>(
@@ -96,36 +98,37 @@ where
     let MessageBalance {
         retryable,
         non_retryable,
-    } = *storage.get(key)?.unwrap_or_default();
-    let current_balance = if message.has_retryable_amount() {
+    } = storage.get(key)?.unwrap_or_default().into_owned();
+    let current_balance = if message.is_retryable_message() {
         retryable
     } else {
         non_retryable
     };
 
-    current_balance
+    let new_amount = current_balance
         .checked_sub(message.amount() as u128)
         .ok_or_else(|| IndexationError::MessageBalanceWouldUnderflow {
             owner: *message.recipient(),
             current_amount: current_balance,
             requested_deduction: message.amount() as u128,
-            retryable: message.has_retryable_amount(),
-        })
-        .and_then(|new_amount| {
-            let storage = block_st_transaction.storage::<MessageBalances>();
-            let new_balance = if message.has_retryable_amount() {
-                MessageBalance {
-                    retryable: new_amount,
-                    non_retryable,
-                }
-            } else {
-                MessageBalance {
-                    retryable,
-                    non_retryable: new_amount,
-                }
-            };
-            storage.insert(key, &new_balance).map_err(Into::into)
-        })
+            retryable: message.is_retryable_message(),
+        })?;
+
+    let new_balance = if message.is_retryable_message() {
+        MessageBalance {
+            retryable: new_amount,
+            non_retryable,
+        }
+    } else {
+        MessageBalance {
+            retryable,
+            non_retryable: new_amount,
+        }
+    };
+    block_st_transaction
+        .storage::<MessageBalances>()
+        .insert(key, &new_balance)
+        .map_err(Into::into)
 }
 
 fn increase_coin_balance<T>(
@@ -137,11 +140,13 @@ where
 {
     let key = CoinBalancesKey::new(&coin.owner, &coin.asset_id);
     let storage = block_st_transaction.storage::<CoinBalances>();
-    let current_amount = *storage.get(&key)?.unwrap_or_default();
+    let current_amount = storage.get(&key)?.unwrap_or_default().into_owned();
     let new_amount = current_amount.saturating_add(coin.amount as u128);
 
-    let storage = block_st_transaction.storage::<CoinBalances>();
-    Ok(storage.insert(&key, &new_amount)?)
+    block_st_transaction
+        .storage::<CoinBalances>()
+        .insert(&key, &new_amount)
+        .map_err(Into::into)
 }
 
 fn decrease_coin_balance<T>(
@@ -153,22 +158,22 @@ where
 {
     let key = CoinBalancesKey::new(&coin.owner, &coin.asset_id);
     let storage = block_st_transaction.storage::<CoinBalances>();
-    let current_amount = *storage.get(&key)?.unwrap_or_default();
-
-    current_amount
-        .checked_sub(coin.amount as u128)
-        .ok_or_else(|| IndexationError::CoinBalanceWouldUnderflow {
-            owner: coin.owner,
-            asset_id: coin.asset_id,
-            current_amount,
-            requested_deduction: coin.amount as u128,
-        })
-        .and_then(|new_amount| {
-            block_st_transaction
-                .storage::<CoinBalances>()
-                .insert(&key, &new_amount)
-                .map_err(Into::into)
-        })
+    let current_amount = storage.get(&key)?.unwrap_or_default().into_owned();
+
+    let new_amount =
+        current_amount
+            .checked_sub(coin.amount as u128)
+            .ok_or_else(|| IndexationError::CoinBalanceWouldUnderflow {
+                owner: coin.owner,
+                asset_id: coin.asset_id,
+                current_amount,
+                requested_deduction: coin.amount as u128,
+            })?;
+
+    block_st_transaction
+        .storage::<CoinBalances>()
+        .insert(&key, &new_amount)
+        .map_err(Into::into)
 }
 
 pub(crate) fn process_balances_update<T>(

crates/fuel-core/src/graphql_api/storage.rs

@@ -114,9 +114,9 @@ pub enum Column {
     DaCompressionTemporalRegistryScriptCode = 21,
     /// See [`DaCompressionTemporalRegistryPredicateCode`](da_compression::DaCompressionTemporalRegistryPredicateCode)
     DaCompressionTemporalRegistryPredicateCode = 22,
-    /// Coin balances per user and asset.
+    /// Coin balances per account and asset.
     CoinBalances = 23,
-    /// Message balances per user.
+    /// Message balances per account.
     MessageBalances = 24,
 }
 

crates/fuel-core/src/query/balance/asset_query.rs

@@ -175,7 +175,7 @@ impl<'a> AssetsQuery<'a> {
             .try_flatten()
             .filter(|result| {
                 if let Ok(message) = result {
-                    !message.has_retryable_amount()
+                    message.is_non_retryable_message()
                 } else {
                     true
                 }

crates/fuel-core/src/service.rs

@@ -126,7 +126,6 @@ impl FuelService {
         // initialize state
         tracing::info!("Initializing database");
         database.check_version()?;
-        database.initialize()?;
 
         Self::make_database_compatible_with_config(
             &mut database,