Database: Split BlockBlobs table + Miscellaneous fixes (#290)

* Split `BlockBlobs` database table + misc fixes

- Split the `BlockBlobs` database table into two new tables: `BlockHeaderBlobs` and `BlockTxsHashes`.
- `add_block`, `pop_block` and `get_block_extended_header` have been edited consequently.
- `VerifiedBlockInformation` now have a `mining_tx_index: u64` field.
- Made `cuprate-helper`'s `thread` feature a dependency of the `service` feature
- Edited service test mapping of output. It is now a full iterator.

* fix fmt

* Update storage/blockchain/src/types.rs

Co-authored-by: Boog900 <boog900@tutanota.com>

* Update storage/blockchain/src/ops/block.rs

Co-authored-by: Boog900 <boog900@tutanota.com>

* fix warning

---------

Co-authored-by: Boog900 <boog900@tutanota.com>

storage/blockchain/Cargo.toml

@@ -15,15 +15,12 @@ default     = ["heed", "service"]
 heed        = ["cuprate-database/heed"]
 redb        = ["cuprate-database/redb"]
 redb-memory = ["cuprate-database/redb-memory"]
-service     = ["dep:thread_local", "dep:rayon"]
+service     = ["dep:thread_local", "dep:rayon", "cuprate-helper/thread"]
 
 [dependencies]
-# FIXME:
-# We only need the `thread` feature if `service` is enabled.
-# Figure out how to enable features of an already pulled in dependency conditionally.
 cuprate-database         = { path = "../database" }
 cuprate-database-service = { path = "../service" }
-cuprate-helper           = { path = "../../helper", features = ["fs", "thread", "map"] }
+cuprate-helper           = { path = "../../helper", features = ["fs", "map"] }
 cuprate-types            = { path = "../../types", features = ["blockchain"] }
 cuprate-pruning          = { path = "../../pruning" }
 

storage/blockchain/src/ops/block.rs

@@ -2,7 +2,10 @@
 
 //---------------------------------------------------------------------------------------------------- Import
 use bytemuck::TransparentWrapper;
-use monero_serai::block::{Block, BlockHeader};
+use monero_serai::{
+    block::{Block, BlockHeader},
+    transaction::Transaction,
+};
 
 use cuprate_database::{
     RuntimeError, StorableVec, {DatabaseRo, DatabaseRw},
@@ -76,10 +79,10 @@ pub fn add_block(
 
     //------------------------------------------------------ Transaction / Outputs / Key Images
     // Add the miner transaction first.
-    {
+    let mining_tx_index = {
         let tx = &block.block.miner_transaction;
-        add_tx(tx, &tx.serialize(), &tx.hash(), &chain_height, tables)?;
+        add_tx(tx, &tx.serialize(), &tx.hash(), &chain_height, tables)?
-    }
+    };
 
     for tx in &block.txs {
         add_tx(&tx.tx, &tx.tx_blob, &tx.tx_hash, &chain_height, tables)?;
@@ -111,13 +114,21 @@ pub fn add_block(
             block_hash: block.block_hash,
             weight: block.weight,
             long_term_weight: block.long_term_weight,
+            mining_tx_index,
         },
     )?;
 
-    // Block blobs.
+    // Block header blob.
-    tables
+    tables.block_header_blobs_mut().put(
-        .block_blobs_mut()
+        &block.height,
-        .put(&block.height, StorableVec::wrap_ref(&block.block_blob))?;
+        StorableVec::wrap_ref(&block.block.header.serialize()),
+    )?;
+
+    // Block transaction hashes
+    tables.block_txs_hashes_mut().put(
+        &block.height,
+        StorableVec::wrap_ref(&block.block.transactions),
+    )?;
 
     // Block heights.
     tables
@@ -151,10 +162,18 @@ pub fn pop_block(
     tables.block_heights_mut().delete(&block_info.block_hash)?;
 
     // Block blobs.
-    // We deserialize the block blob into a `Block`, such
+    //
-    // that we can remove the associated transactions later.
+    // We deserialize the block header blob and mining transaction blob
-    let block_blob = tables.block_blobs_mut().take(&block_height)?.0;
+    // to form a `Block`, such that we can remove the associated transactions
-    let block = Block::read(&mut block_blob.as_slice())?;
+    // later.
+    let block_header = tables.block_header_blobs_mut().take(&block_height)?.0;
+    let block_txs_hashes = tables.block_txs_hashes_mut().take(&block_height)?.0;
+    let miner_transaction = tables.tx_blobs().get(&block_info.mining_tx_index)?.0;
+    let block = Block {
+        header: BlockHeader::read(&mut block_header.as_slice())?,
+        miner_transaction: Transaction::read(&mut miner_transaction.as_slice())?,
+        transactions: block_txs_hashes,
+    };
 
     //------------------------------------------------------ Transaction / Outputs / Key Images
     remove_tx(&block.miner_transaction.hash(), tables)?;
@@ -181,7 +200,7 @@ pub fn pop_block(
         alt_block::add_alt_block(
             &AltBlockInformation {
                 block: block.clone(),
-                block_blob,
+                block_blob: block.serialize(),
                 txs,
                 block_hash: block_info.block_hash,
                 // We know the PoW is valid for this block so just set it so it will always verify as valid.
@@ -236,8 +255,8 @@ pub fn get_block_extended_header_from_height(
     tables: &impl Tables,
 ) -> Result<ExtendedBlockHeader, RuntimeError> {
     let block_info = tables.block_infos().get(block_height)?;
-    let block_blob = tables.block_blobs().get(block_height)?.0;
+    let block_header_blob = tables.block_header_blobs().get(block_height)?.0;
-    let block_header = BlockHeader::read(&mut block_blob.as_slice())?;
+    let block_header = BlockHeader::read(&mut block_header_blob.as_slice())?;
 
     let cumulative_difficulty = combine_low_high_bits_to_u128(
         block_info.cumulative_difficulty_low,
@@ -304,7 +323,7 @@ pub fn block_exists(
 
 //---------------------------------------------------------------------------------------------------- Tests
 #[cfg(test)]
-#[expect(clippy::significant_drop_tightening, clippy::too_many_lines)]
+#[expect(clippy::too_many_lines)]
 mod test {
     use pretty_assertions::assert_eq;
 
@@ -370,7 +389,8 @@ mod test {
             // Assert only the proper tables were added to.
             AssertTableLen {
                 block_infos: 3,
-                block_blobs: 3,
+                block_header_blobs: 3,
+                block_txs_hashes: 3,
                 block_heights: 3,
                 key_images: 69,
                 num_outputs: 41,

storage/blockchain/src/ops/blockchain.rs

@@ -138,7 +138,8 @@ mod test {
             // Assert reads are correct.
             AssertTableLen {
                 block_infos: 3,
-                block_blobs: 3,
+                block_header_blobs: 3,
+                block_txs_hashes: 3,
                 block_heights: 3,
                 key_images: 69,
                 num_outputs: 41,

storage/blockchain/src/ops/output.rs

@@ -316,7 +316,8 @@ mod test {
             // Assert proper tables were added to.
             AssertTableLen {
                 block_infos: 0,
-                block_blobs: 0,
+                block_header_blobs: 0,
+                block_txs_hashes: 0,
                 block_heights: 0,
                 key_images: 0,
                 num_outputs: 1,

storage/blockchain/src/ops/tx.rs

@@ -366,7 +366,8 @@ mod test {
             // Assert only the proper tables were added to.
             AssertTableLen {
                 block_infos: 0,
-                block_blobs: 0,
+                block_header_blobs: 0,
+                block_txs_hashes: 0,
                 block_heights: 0,
                 key_images: 4, // added to key images
                 pruned_tx_blobs: 0,

storage/blockchain/src/service/tests.rs

@@ -241,18 +241,20 @@ async fn test_template(
 
     //----------------------------------------------------------------------- Output checks
     // Create the map of amounts and amount indices.
-    //
-    // FIXME: There's definitely a better way to map
-    // `Vec<PreRctOutputId>` -> `HashMap<u64, HashSet<u64>>`
     let (map, output_count) = {
-        let mut ids = tables
+        let mut map = HashMap::<Amount, HashSet<AmountIndex>>::new();
+
+        // Used later to compare the amount of Outputs
+        // returned in the Response is equal to the amount
+        // we asked for.
+        let mut output_count: usize = 0;
+
+        tables
             .outputs_iter()
             .keys()
             .unwrap()
             .map(Result::unwrap)
-            .collect::<Vec<PreRctOutputId>>();
+            .chain(
-
-        ids.extend(
                 tables
                     .rct_outputs_iter()
                     .keys()
@@ -262,21 +264,15 @@ async fn test_template(
                         amount: 0,
                         amount_index,
                     }),
-        );
+            )
-
+            .for_each(|id| {
-        // Used later to compare the amount of Outputs
+                output_count += 1;
-        // returned in the Response is equal to the amount
-        // we asked for.
-        let output_count = ids.len();
-
-        let mut map = HashMap::<Amount, HashSet<AmountIndex>>::new();
-        for id in ids {
                 map.entry(id.amount)
                     .and_modify(|set| {
                         set.insert(id.amount_index);
                     })
                     .or_insert_with(|| HashSet::from([id.amount_index]));
-        }
+            });
 
         (map, output_count)
     };
@@ -347,7 +343,8 @@ async fn v1_tx2() {
         14_535_350_982_449,
         AssertTableLen {
             block_infos: 1,
-            block_blobs: 1,
+            block_header_blobs: 1,
+            block_txs_hashes: 1,
             block_heights: 1,
             key_images: 65,
             num_outputs: 41,
@@ -373,7 +370,8 @@ async fn v9_tx3() {
         3_403_774_022_163,
         AssertTableLen {
             block_infos: 1,
-            block_blobs: 1,
+            block_header_blobs: 1,
+            block_txs_hashes: 1,
             block_heights: 1,
             key_images: 4,
             num_outputs: 0,
@@ -399,7 +397,8 @@ async fn v16_tx0() {
         600_000_000_000,
         AssertTableLen {
             block_infos: 1,
-            block_blobs: 1,
+            block_header_blobs: 1,
+            block_txs_hashes: 1,
             block_heights: 1,
             key_images: 0,
             num_outputs: 0,

storage/blockchain/src/tables.rs

@@ -9,7 +9,7 @@
 //! Table structs are `CamelCase`, and their static string
 //! names used by the actual database backend are `snake_case`.
 //!
-//! For example: [`BlockBlobs`] -> `block_blobs`.
+//! For example: [`BlockHeaderBlobs`] -> `block_header_blobs`.
 //!
 //! # Traits
 //! This module also contains a set of traits for
@@ -18,9 +18,9 @@
 //---------------------------------------------------------------------------------------------------- Import
 use crate::types::{
     AltBlockHeight, AltChainInfo, AltTransactionInfo, Amount, AmountIndex, AmountIndices,
-    BlockBlob, BlockHash, BlockHeight, BlockInfo, CompactAltBlockInfo, KeyImage, Output,
+    BlockBlob, BlockHash, BlockHeaderBlob, BlockHeight, BlockInfo, BlockTxHashes,
-    PreRctOutputId, PrunableBlob, PrunableHash, PrunedBlob, RawChainId, RctOutput, TxBlob, TxHash,
+    CompactAltBlockInfo, KeyImage, Output, PreRctOutputId, PrunableBlob, PrunableHash, PrunedBlob,
-    TxId, UnlockTime,
+    RawChainId, RctOutput, TxBlob, TxHash, TxId, UnlockTime,
 };
 
 //---------------------------------------------------------------------------------------------------- Tables
@@ -30,22 +30,28 @@ use crate::types::{
 // - If adding/changing a table also edit:
 //   - the tests in `src/backend/tests.rs`
 cuprate_database::define_tables! {
-    /// Serialized block blobs (bytes).
+    /// Serialized block header blobs (bytes).
     ///
-    /// Contains the serialized version of all blocks.
+    /// Contains the serialized version of all blocks headers.
-    0 => BlockBlobs,
+    0 => BlockHeaderBlobs,
-    BlockHeight => BlockBlob,
+    BlockHeight => BlockHeaderBlob,
+
+    /// Block transactions hashes
+    ///
+    /// Contains all the transaction hashes of all blocks.
+    1 => BlockTxsHashes,
+    BlockHeight => BlockTxHashes,
 
     /// Block heights.
     ///
     /// Contains the height of all blocks.
-    1 => BlockHeights,
+    2 => BlockHeights,
     BlockHash => BlockHeight,
 
     /// Block information.
     ///
     /// Contains metadata of all blocks.
-    2 => BlockInfos,
+    3 => BlockInfos,
     BlockHeight => BlockInfo,
 
     /// Set of key images.
@@ -54,38 +60,38 @@ cuprate_database::define_tables! {
     ///
     /// This table has `()` as the value type, as in,
     /// it is a set of key images.
-    3 => KeyImages,
+    4 => KeyImages,
     KeyImage => (),
 
     /// Maps an output's amount to the number of outputs with that amount.
     ///
     /// For example, if there are 5 outputs with `amount = 123`
     /// then calling `get(123)` on this table will return 5.
-    4 => NumOutputs,
+    5 => NumOutputs,
     Amount => u64,
 
     /// Pre-RCT output data.
-    5 => Outputs,
+    6 => Outputs,
     PreRctOutputId => Output,
 
     /// Pruned transaction blobs (bytes).
     ///
     /// Contains the pruned portion of serialized transaction data.
-    6 => PrunedTxBlobs,
+    7 => PrunedTxBlobs,
     TxId => PrunedBlob,
 
     /// Prunable transaction blobs (bytes).
     ///
     /// Contains the prunable portion of serialized transaction data.
     // SOMEDAY: impl when `monero-serai` supports pruning
-    7 => PrunableTxBlobs,
+    8 => PrunableTxBlobs,
     TxId => PrunableBlob,
 
     /// Prunable transaction hashes.
     ///
     /// Contains the prunable portion of transaction hashes.
     // SOMEDAY: impl when `monero-serai` supports pruning
-    8 => PrunableHashes,
+    9 => PrunableHashes,
     TxId => PrunableHash,
 
     // SOMEDAY: impl a properties table:
@@ -95,74 +101,74 @@ cuprate_database::define_tables! {
     // StorableString => StorableVec,
 
     /// RCT output data.
-    9 => RctOutputs,
+    10 => RctOutputs,
     AmountIndex => RctOutput,
 
     /// Transaction blobs (bytes).
     ///
     /// Contains the serialized version of all transactions.
     // SOMEDAY: remove when `monero-serai` supports pruning
-    10 => TxBlobs,
+    11 => TxBlobs,
     TxId => TxBlob,
 
     /// Transaction indices.
     ///
     /// Contains the indices all transactions.
-    11 => TxIds,
+    12 => TxIds,
     TxHash => TxId,
 
     /// Transaction heights.
     ///
     /// Contains the block height associated with all transactions.
-    12 => TxHeights,
+    13 => TxHeights,
     TxId => BlockHeight,
 
     /// Transaction outputs.
     ///
     /// Contains the list of `AmountIndex`'s of the
     /// outputs associated with all transactions.
-    13 => TxOutputs,
+    14 => TxOutputs,
     TxId => AmountIndices,
 
     /// Transaction unlock time.
     ///
     /// Contains the unlock time of transactions IF they have one.
     /// Transactions without unlock times will not exist in this table.
-    14 => TxUnlockTime,
+    15 => TxUnlockTime,
     TxId => UnlockTime,
 
     /// Information on alt-chains.
-    15 => AltChainInfos,
+    16 => AltChainInfos,
     RawChainId => AltChainInfo,
 
     /// Alt-block heights.
     ///
     /// Contains the height of all alt-blocks.
-    16 => AltBlockHeights,
+    17 => AltBlockHeights,
     BlockHash => AltBlockHeight,
 
     /// Alt-block information.
     ///
     /// Contains information on all alt-blocks.
-    17 => AltBlocksInfo,
+    18 => AltBlocksInfo,
     AltBlockHeight => CompactAltBlockInfo,
 
     /// Alt-block blobs.
     ///
     /// Contains the raw bytes of all alt-blocks.
-    18 => AltBlockBlobs,
+    19 => AltBlockBlobs,
     AltBlockHeight => BlockBlob,
 
     /// Alt-block transaction blobs.
     ///
     /// Contains the raw bytes of alt transactions, if those transactions are not in the main-chain.
-    19 => AltTransactionBlobs,
+    20 => AltTransactionBlobs,
     TxHash => TxBlob,
 
     /// Alt-block transaction information.
     ///
     /// Contains information on all alt transactions, even if they are in the main-chain.
-    20 => AltTransactionInfos,
+    21 => AltTransactionInfos,
     TxHash => AltTransactionInfo,
 }
 

storage/blockchain/src/tests.rs

@@ -9,7 +9,7 @@ use std::{borrow::Cow, fmt::Debug};
 
 use pretty_assertions::assert_eq;
 
-use cuprate_database::{ConcreteEnv, DatabaseRo, Env, EnvInner};
+use cuprate_database::{DatabaseRo, Env, EnvInner};
 use cuprate_types::{AltBlockInformation, ChainId, VerifiedBlockInformation};
 
 use crate::{
@@ -26,7 +26,8 @@ use crate::{
 #[derive(Copy, Clone, Debug, Default, PartialEq, Eq, PartialOrd, Ord, Hash)]
 pub(crate) struct AssertTableLen {
     pub(crate) block_infos: u64,
-    pub(crate) block_blobs: u64,
+    pub(crate) block_header_blobs: u64,
+    pub(crate) block_txs_hashes: u64,
     pub(crate) block_heights: u64,
     pub(crate) key_images: u64,
     pub(crate) num_outputs: u64,
@@ -46,7 +47,8 @@ impl AssertTableLen {
     pub(crate) fn assert(self, tables: &impl Tables) {
         let other = Self {
             block_infos: tables.block_infos().len().unwrap(),
-            block_blobs: tables.block_blobs().len().unwrap(),
+            block_header_blobs: tables.block_header_blobs().len().unwrap(),
+            block_txs_hashes: tables.block_txs_hashes().len().unwrap(),
             block_heights: tables.block_heights().len().unwrap(),
             key_images: tables.key_images().len().unwrap(),
             num_outputs: tables.num_outputs().len().unwrap(),
@@ -69,8 +71,7 @@ impl AssertTableLen {
 /// Create an `Env` in a temporarily directory.
 /// The directory is automatically removed after the `TempDir` is dropped.
 ///
-/// FIXME: changing this to `-> impl Env` causes lifetime errors...
+pub(crate) fn tmp_concrete_env() -> (impl Env, tempfile::TempDir) {
-pub(crate) fn tmp_concrete_env() -> (ConcreteEnv, tempfile::TempDir) {
     let tempdir = tempfile::tempdir().unwrap();
     let config = ConfigBuilder::new()
         .db_directory(Cow::Owned(tempdir.path().into()))
@@ -82,7 +83,7 @@ pub(crate) fn tmp_concrete_env() -> (ConcreteEnv, tempfile::TempDir) {
 }
 
 /// Assert all the tables in the environment are empty.
-pub(crate) fn assert_all_tables_are_empty(env: &ConcreteEnv) {
+pub(crate) fn assert_all_tables_are_empty(env: &impl Env) {
     let env_inner = env.env_inner();
     let tx_ro = env_inner.tx_ro().unwrap();
     let tables = env_inner.open_tables(&tx_ro).unwrap();

storage/blockchain/src/types.rs

@@ -66,6 +66,12 @@ pub type AmountIndices = StorableVec<AmountIndex>;
 /// A serialized block.
 pub type BlockBlob = StorableVec<u8>;
 
+/// A serialized block header
+pub type BlockHeaderBlob = StorableVec<u8>;
+
+/// A block transaction hashes
+pub type BlockTxHashes = StorableVec<[u8; 32]>;
+
 /// A block's hash.
 pub type BlockHash = [u8; 32];
 
@@ -166,6 +172,7 @@ impl Key for PreRctOutputId {}
 ///     block_hash: [54; 32],
 ///     cumulative_rct_outs: 2389,
 ///     long_term_weight: 2389,
+///     mining_tx_index: 23
 /// };
 /// let b = Storable::as_bytes(&a);
 /// let c: BlockInfo = Storable::from_bytes(b);
@@ -175,7 +182,7 @@ impl Key for PreRctOutputId {}
 /// # Size & Alignment
 /// ```rust
 /// # use cuprate_blockchain::types::*;
-/// assert_eq!(size_of::<BlockInfo>(), 88);
+/// assert_eq!(size_of::<BlockInfo>(), 96);
 /// assert_eq!(align_of::<BlockInfo>(), 8);
 /// ```
 #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
@@ -202,6 +209,8 @@ pub struct BlockInfo {
     ///
     /// See [`long_term_weight`](https://monero-book.cuprate.org/consensus_rules/blocks/weights.html#long-term-block-weight).
     pub long_term_weight: usize,
+    /// [`TxId`] (u64) of the block coinbase transaction.
+    pub mining_tx_index: TxId,
 }
 
 //---------------------------------------------------------------------------------------------------- OutputFlags