Use BlockHeight as a primary key for the FuelsBlock table (#1587)

Closes FuelLabs/fuel-core#1580

CHANGELOG.md

@@ -11,6 +11,7 @@ Description of the upcoming release here.
 ### Changed
 
 - [#1591](https://github.com/FuelLabs/fuel-core/pull/1591): Simplify libp2p dependencies and not depend on all sub modules directly.
+- [#1587](https://github.com/FuelLabs/fuel-core/pull/1587): Use `BlockHeight` as a primary key for the `FuelsBlock` table.
 - [#1585](https://github.com/FuelLabs/fuel-core/pull/1585): Let `NetworkBehaviour` macro generate `FuelBehaviorEvent` in p2p
 - [#1579](https://github.com/FuelLabs/fuel-core/pull/1579): The change extracts the off-chain-related logic from the executor and moves it to the GraphQL off-chain worker. It creates two new concepts - Off-chain and On-chain databases where the GraphQL worker has exclusive ownership of the database and may modify it without intersecting with the On-chain database.
 - [#1577](https://github.com/FuelLabs/fuel-core/pull/1577): Moved insertion of sealed blocks into the `BlockImporter` instead of the executor.

crates/fuel-core/src/database.rs

@@ -13,7 +13,11 @@ use fuel_core_chain_config::{
 };
 use fuel_core_storage::{
     blueprint::Blueprint,
-    codec::Decode,
+    codec::{
+        Decode,
+        Encode,
+        Encoder,
+    },
     iter::IterDirection,
     kv_store::{
         BatchOperations,
@@ -253,18 +257,18 @@ impl BatchOperations for DataSource {
 
 /// Read-only methods.
 impl Database {
-    fn iter_all<M>(
+    pub(crate) fn iter_all<M>(
         &self,
         direction: Option<IterDirection>,
     ) -> impl Iterator<Item = StorageResult<(M::OwnedKey, M::OwnedValue)>> + '_
     where
         M: Mappable + TableWithBlueprint,
         M::Blueprint: Blueprint<M, DataSource>,
     {
-        self.iter_all_filtered::<M, Vec<u8>, Vec<u8>>(None, None, direction)
+        self.iter_all_filtered::<M, [u8; 0]>(None, None, direction)
     }
 
-    fn iter_all_by_prefix<M, P>(
+    pub(crate) fn iter_all_by_prefix<M, P>(
         &self,
         prefix: Option<P>,
     ) -> impl Iterator<Item = StorageResult<(M::OwnedKey, M::OwnedValue)>> + '_
@@ -273,40 +277,44 @@ impl Database {
         M::Blueprint: Blueprint<M, DataSource>,
         P: AsRef<[u8]>,
     {
-        self.iter_all_filtered::<M, P, [u8; 0]>(prefix, None, None)
+        self.iter_all_filtered::<M, P>(prefix, None, None)
     }
 
-    fn iter_all_by_start<M, S>(
+    pub(crate) fn iter_all_by_start<M>(
         &self,
-        start: Option<S>,
+        start: Option<&M::Key>,
         direction: Option<IterDirection>,
     ) -> impl Iterator<Item = StorageResult<(M::OwnedKey, M::OwnedValue)>> + '_
     where
         M: Mappable + TableWithBlueprint,
         M::Blueprint: Blueprint<M, DataSource>,
-        S: AsRef<[u8]>,
     {
-        self.iter_all_filtered::<M, [u8; 0], S>(None, start, direction)
+        self.iter_all_filtered::<M, [u8; 0]>(None, start, direction)
     }
 
-    fn iter_all_filtered<M, P, S>(
+    pub(crate) fn iter_all_filtered<M, P>(
         &self,
         prefix: Option<P>,
-        start: Option<S>,
+        start: Option<&M::Key>,
         direction: Option<IterDirection>,
     ) -> impl Iterator<Item = StorageResult<(M::OwnedKey, M::OwnedValue)>> + '_
     where
         M: Mappable + TableWithBlueprint,
         M::Blueprint: Blueprint<M, DataSource>,
         P: AsRef<[u8]>,
-        S: AsRef<[u8]>,
     {
+        let encoder = start.map(|start| {
+            <M::Blueprint as Blueprint<M, DataSource>>::KeyCodec::encode(start)
+        });
+
+        let start = encoder.as_ref().map(|encoder| encoder.as_bytes());
+
         self.data
             .as_ref()
             .iter_all(
                 M::column(),
                 prefix.as_ref().map(|p| p.as_ref()),
-                start.as_ref().map(|s| s.as_ref()),
+                start.as_ref().map(|cow| cow.as_ref()),
                 direction.unwrap_or_default(),
             )
             .map(|val| {
@@ -379,7 +387,7 @@ impl ChainConfigDb for Database {
     }
 
     fn get_block_height(&self) -> StorageResult<BlockHeight> {
-        Self::latest_height(self)
+        self.latest_height()
     }
 }
 

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

@@ -1,7 +1,6 @@
 use crate::database::{
     Column,
     Database,
-    Error as DatabaseError,
 };
 use fuel_core_storage::{
     blueprint::plain::Plain,
@@ -49,21 +48,21 @@ use std::borrow::{
     Cow,
 };
 
-/// The table of fuel block's secondary key - `BlockHeight`.
-/// It links the `BlockHeight` to corresponding `BlockId`.
+/// The table of fuel block's secondary key - `BlockId`.
+/// It links the `BlockId` to corresponding `BlockHeight`.
 pub struct FuelBlockSecondaryKeyBlockHeights;
 
 impl Mappable for FuelBlockSecondaryKeyBlockHeights {
-    /// Secondary key - `BlockHeight`.
-    type Key = BlockHeight;
-    type OwnedKey = Self::Key;
     /// Primary key - `BlockId`.
-    type Value = BlockId;
+    type Key = BlockId;
+    type OwnedKey = Self::Key;
+    /// Secondary key - `BlockHeight`.
+    type Value = BlockHeight;
     type OwnedValue = Self::Value;
 }
 
 impl TableWithBlueprint for FuelBlockSecondaryKeyBlockHeights {
-    type Blueprint = Plain<Primitive<4>, Raw>;
+    type Blueprint = Plain<Raw, Primitive<4>>;
 
     fn column() -> Column {
         Column::FuelBlockSecondaryKeyBlockHeights
@@ -80,29 +79,36 @@ fuel_core_storage::basic_storage_tests!(
 impl StorageInspect<FuelBlocks> for Database {
     type Error = StorageError;
 
-    fn get(&self, key: &BlockId) -> Result<Option<Cow<CompressedBlock>>, Self::Error> {
+    fn get(
+        &self,
+        key: &<FuelBlocks as Mappable>::Key,
+    ) -> Result<Option<Cow<<FuelBlocks as Mappable>::OwnedValue>>, Self::Error> {
         self.data.storage::<FuelBlocks>().get(key)
     }
 
-    fn contains_key(&self, key: &BlockId) -> Result<bool, Self::Error> {
+    fn contains_key(
+        &self,
+        key: &<FuelBlocks as Mappable>::Key,
+    ) -> Result<bool, Self::Error> {
         self.data.storage::<FuelBlocks>().contains_key(key)
     }
 }
 
 impl StorageMutate<FuelBlocks> for Database {
     fn insert(
         &mut self,
-        key: &BlockId,
-        value: &CompressedBlock,
-    ) -> Result<Option<CompressedBlock>, Self::Error> {
+        key: &<FuelBlocks as Mappable>::Key,
+        value: &<FuelBlocks as Mappable>::Value,
+    ) -> Result<Option<<FuelBlocks as Mappable>::OwnedValue>, Self::Error> {
         let prev = self
             .data
             .storage_as_mut::<FuelBlocks>()
             .insert(key, value)?;
 
         let height = value.header().height();
+        let block_id = value.id();
         self.storage::<FuelBlockSecondaryKeyBlockHeights>()
-            .insert(height, key)?;
+            .insert(&block_id, key)?;
 
         // Get latest metadata entry
         let prev_metadata = self
@@ -116,8 +122,7 @@ impl StorageMutate<FuelBlocks> for Database {
         let mut tree: MerkleTree<FuelBlockMerkleData, _> =
             MerkleTree::load(storage, prev_metadata.version)
                 .map_err(|err| StorageError::Other(anyhow::anyhow!(err)))?;
-        let data = key.as_slice();
-        tree.push(data)?;
+        tree.push(block_id.as_slice())?;
 
         // Generate new metadata for the updated tree
         let version = tree.leaves_count();
@@ -129,15 +134,18 @@ impl StorageMutate<FuelBlocks> for Database {
         Ok(prev)
     }
 
-    fn remove(&mut self, key: &BlockId) -> Result<Option<CompressedBlock>, Self::Error> {
+    fn remove(
+        &mut self,
+        key: &<FuelBlocks as Mappable>::Key,
+    ) -> Result<Option<<FuelBlocks as Mappable>::OwnedValue>, Self::Error> {
         let prev: Option<CompressedBlock> =
             self.data.storage_as_mut::<FuelBlocks>().remove(key)?;
 
         if let Some(block) = &prev {
             let height = block.header().height();
             let _ = self
                 .storage::<FuelBlockSecondaryKeyBlockHeights>()
-                .remove(height);
+                .remove(&block.id());
             // We can't clean up `MerkleTree<FuelBlockMerkleData>`.
             // But if we plan to insert a new block, it will override old values in the
             // `FuelBlockMerkleData` table.
@@ -150,68 +158,56 @@ impl StorageMutate<FuelBlocks> for Database {
 
 impl Database {
     pub fn latest_height(&self) -> StorageResult<BlockHeight> {
-        self.ids_of_latest_block()?
-            .map(|(height, _)| height)
-            .ok_or(not_found!("BlockHeight"))
+        let pair = self
+            .iter_all::<FuelBlocks>(Some(IterDirection::Reverse))
+            .next()
+            .transpose()?;
+
+        let (block_height, _) = pair.ok_or(not_found!("BlockHeight"))?;
+
+        Ok(block_height)
+    }
+
+    pub fn latest_compressed_block(&self) -> StorageResult<Option<CompressedBlock>> {
+        let pair = self
+            .iter_all::<FuelBlocks>(Some(IterDirection::Reverse))
+            .next()
+            .transpose()?;
+
+        Ok(pair.map(|(_, compressed_block)| compressed_block))
     }
 
     /// Get the current block at the head of the chain.
-    pub fn get_current_block(&self) -> StorageResult<Option<Cow<CompressedBlock>>> {
-        let block_ids = self.ids_of_latest_block()?;
-        match block_ids {
-            Some((_, id)) => Ok(StorageAsRef::storage::<FuelBlocks>(self).get(&id)?),
-            None => Ok(None),
-        }
+    pub fn get_current_block(&self) -> StorageResult<Option<CompressedBlock>> {
+        self.latest_compressed_block()
     }
 
     pub fn block_time(&self, height: &BlockHeight) -> StorageResult<Tai64> {
-        let id = self.get_block_id(height)?.unwrap_or_default();
         let block = self
             .storage::<FuelBlocks>()
-            .get(&id)?
+            .get(height)?
             .ok_or(not_found!(FuelBlocks))?;
         Ok(block.header().time().to_owned())
     }
 
     pub fn get_block_id(&self, height: &BlockHeight) -> StorageResult<Option<BlockId>> {
-        self.storage::<FuelBlockSecondaryKeyBlockHeights>()
+        self.storage::<FuelBlocks>()
             .get(height)
-            .map(|v| v.map(|v| v.into_owned()))
+            .map(|v| v.map(|v| v.id()))
     }
 
-    pub fn all_block_ids(
-        &self,
-        start: Option<BlockHeight>,
-        direction: IterDirection,
-    ) -> impl Iterator<Item = StorageResult<(BlockHeight, BlockId)>> + '_ {
-        let start = start.map(|b| b.to_bytes());
-        self.iter_all_by_start::<FuelBlockSecondaryKeyBlockHeights, _>(
-            start,
-            Some(direction),
-        )
-    }
-
-    pub fn ids_of_genesis_block(&self) -> StorageResult<(BlockHeight, BlockId)> {
-        self.iter_all::<FuelBlockSecondaryKeyBlockHeights>(Some(IterDirection::Forward))
-            .next()
-            .ok_or(DatabaseError::ChainUninitialized)?
-    }
-
-    pub fn ids_of_latest_block(&self) -> StorageResult<Option<(BlockHeight, BlockId)>> {
-        let ids = self
-            .iter_all::<FuelBlockSecondaryKeyBlockHeights>(Some(IterDirection::Reverse))
-            .next()
-            .transpose()?;
-
-        Ok(ids)
+    pub fn get_block_height(&self, id: &BlockId) -> StorageResult<Option<BlockHeight>> {
+        self.storage::<FuelBlockSecondaryKeyBlockHeights>()
+            .get(id)
+            .map(|v| v.map(|v| v.into_owned()))
     }
 
     /// Retrieve the full block and all associated transactions
     pub(crate) fn get_full_block(
         &self,
-        block_id: &BlockId,
+        height: &BlockHeight,
     ) -> StorageResult<Option<Block>> {
-        let db_block = self.storage::<FuelBlocks>().get(block_id)?;
+        let db_block = self.storage::<FuelBlocks>().get(height)?;
         if let Some(block) = db_block {
             // fetch all the transactions
             // TODO: optimize with multi-key get
@@ -334,7 +330,7 @@ mod tests {
         for block in &blocks {
             StorageMutate::<FuelBlocks>::insert(
                 &mut database,
-                &block.id(),
+                block.header().height(),
                 &block.compress(&ChainId::default()),
             )
             .unwrap();
@@ -398,7 +394,7 @@ mod tests {
         for block in &blocks {
             StorageMutate::<FuelBlocks>::insert(
                 database,
-                &block.id(),
+                block.header().height(),
                 &block.compress(&ChainId::default()),
             )
             .unwrap();

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

@@ -126,9 +126,9 @@ impl Database {
         start_coin: Option<UtxoId>,
         direction: Option<IterDirection>,
     ) -> impl Iterator<Item = StorageResult<UtxoId>> + '_ {
-        self.iter_all_filtered::<OwnedCoins, _, _>(
-            Some(*owner),
-            start_coin.map(|b| owner_coin_id_key(owner, &b)),
+        let start_coin = start_coin.map(|b| owner_coin_id_key(owner, &b));
+        self.iter_all_filtered::<OwnedCoins, _>(
+            Some(*owner), start_coin.as_ref(),
             direction,
         )
         // Safety: key is always 64 bytes

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

@@ -93,9 +93,11 @@ impl Database {
         start_asset: Option<AssetId>,
         direction: Option<IterDirection>,
     ) -> impl Iterator<Item = StorageResult<(AssetId, Word)>> + '_ {
-        self.iter_all_filtered::<ContractsAssets, _, _>(
+        let start_asset =
+            start_asset.map(|asset| ContractsAssetKey::new(&contract, &asset));
+        self.iter_all_filtered::<ContractsAssets, _>(
             Some(contract),
-            start_asset.map(|asset_id| ContractsAssetKey::new(&contract, &asset_id)),
+            start_asset.as_ref(),
             direction,
         )
         .map(|res| res.map(|(key, balance)| (*key.asset_id(), balance)))

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

@@ -32,10 +32,7 @@ use fuel_core_types::{
         Nonce,
     },
 };
-use std::{
-    borrow::Cow,
-    ops::Deref,
-};
+use std::borrow::Cow;
 
 fuel_core_types::fuel_vm::double_key!(OwnedMessageKey, Address, address, Nonce, nonce);
 
@@ -120,9 +117,11 @@ impl Database {
         start_message_id: Option<Nonce>,
         direction: Option<IterDirection>,
     ) -> impl Iterator<Item = StorageResult<Nonce>> + '_ {
-        self.iter_all_filtered::<OwnedMessageIds, _, _>(
+        let start_message_id =
+            start_message_id.map(|msg_id| OwnedMessageKey::new(owner, &msg_id));
+        self.iter_all_filtered::<OwnedMessageIds, _>(
             Some(*owner),
-            start_message_id.map(|msg_id| OwnedMessageKey::new(owner, &msg_id)),
+            start_message_id.as_ref(),
             direction,
         )
         .map(|res| res.map(|(key, _)| *key.nonce()))
@@ -133,8 +132,7 @@ impl Database {
         start: Option<Nonce>,
         direction: Option<IterDirection>,
     ) -> impl Iterator<Item = StorageResult<Message>> + '_ {
-        let start = start.map(|v| v.deref().to_vec());
-        self.iter_all_by_start::<Messages, _>(start, direction)
+        self.iter_all_by_start::<Messages>(start.as_ref(), direction)
             .map(|res| res.map(|(_, message)| message))
     }