Blockchain: add alt-block handling

helper/Cargo.toml

@@ -9,9 +9,8 @@ repository = "https://github.com/Cuprate/cuprate/tree/main/consensus"
 
 
 [features]
-# TODO: I don't think this is a good idea
-# All features on by default.
-default   = ["std", "atomic", "asynch", "cast", "fs", "num", "map", "time", "thread", "constants", "tx-utils"]
+# All features off by default.
+default   = []
 std       = []
 atomic    = ["dep:crossbeam"]
 asynch    = ["dep:futures", "dep:rayon"]
@@ -22,7 +21,7 @@ num       = []
 map       = ["cast", "dep:monero-serai"]
 time      = ["dep:chrono", "std"]
 thread    = ["std", "dep:target_os_lib"]
-tx-utils  = ["dep:monero-serai"]
+tx        = ["dep:monero-serai"]
 
 [dependencies]
 crossbeam    = { workspace = true, optional = true }
@@ -41,7 +40,8 @@ target_os_lib = { package = "windows", version = ">=0.51", features = ["Win32_Sy
 target_os_lib = { package = "libc", version = "0.2.151", optional = true }
 
 [dev-dependencies]
-tokio = { workspace = true, features = ["full"] }
+tokio            = { workspace = true, features = ["full"] }
+curve25519-dalek = { workspace = true }
 
 [lints]
 workspace = true

helper/src/lib.rs

@@ -31,8 +31,8 @@ pub mod thread;
 #[cfg(feature = "time")]
 pub mod time;
 
-#[cfg(feature = "tx-utils")]
-pub mod tx_utils;
+#[cfg(feature = "tx")]
+pub mod tx;
 //---------------------------------------------------------------------------------------------------- Private Usage
 
 //----------------------------------------------------------------------------------------------------

helper/src/tx_utils.rs → helper/src/tx.rs

@@ -32,3 +32,39 @@ pub fn tx_fee(tx: &Transaction) -> u64 {
 
     fee
 }
+
+#[cfg(test)]
+mod test {
+    use curve25519_dalek::{edwards::CompressedEdwardsY, EdwardsPoint};
+    use monero_serai::transaction::{NotPruned, Output, Timelock, TransactionPrefix};
+
+    use super::*;
+
+    #[test]
+    #[should_panic(expected = "called `Option::unwrap()` on a `None` value")]
+    fn tx_fee_panic() {
+        let input = Input::ToKey {
+            amount: Some(u64::MAX),
+            key_offsets: vec![],
+            key_image: EdwardsPoint::default(),
+        };
+
+        let output = Output {
+            amount: Some(u64::MAX),
+            key: CompressedEdwardsY::default(),
+            view_tag: None,
+        };
+
+        let tx = Transaction::<NotPruned>::V1 {
+            prefix: TransactionPrefix {
+                additional_timelock: Timelock::None,
+                inputs: vec![input; 2],
+                outputs: vec![output],
+                extra: vec![],
+            },
+            signatures: vec![],
+        };
+
+        tx_fee(&tx);
+    }
+}

storage/blockchain/src/ops/alt_block/block.rs

@@ -15,6 +15,26 @@ use crate::{
     types::{AltBlockHeight, BlockHash, BlockHeight, CompactAltBlockInfo},
 };
 
+/// Flush all alt-block data from all the alt-block tables.
+///
+/// This function completely empties the alt block tables.
+pub fn flush_alt_blocks<'a, E: cuprate_database::EnvInner<'a>>(
+    env_inner: &E,
+    tx_rw: &mut E::Rw<'_>,
+) -> Result<(), RuntimeError> {
+    use crate::tables::{
+        AltBlockBlobs, AltBlockHeights, AltBlocksInfo, AltChainInfos, AltTransactionBlobs,
+        AltTransactionInfos,
+    };
+
+    env_inner.clear_db::<AltChainInfos>(tx_rw)?;
+    env_inner.clear_db::<AltBlockHeights>(tx_rw)?;
+    env_inner.clear_db::<AltBlocksInfo>(tx_rw)?;
+    env_inner.clear_db::<AltBlockBlobs>(tx_rw)?;
+    env_inner.clear_db::<AltTransactionBlobs>(tx_rw)?;
+    env_inner.clear_db::<AltTransactionInfos>(tx_rw)
+}
+
 /// Add a [`AltBlockInformation`] to the database.
 ///
 /// This extracts all the data from the input block and
@@ -211,7 +231,7 @@ mod tests {
         types::AltBlockHeight,
     };
 
-    #[allow(clippy::range_plus_one)]
+    #[expect(clippy::range_plus_one)]
     #[test]
     fn all_alt_blocks() {
         let (env, _tmp) = tmp_concrete_env();

storage/blockchain/src/ops/alt_block/chain.rs

@@ -1,4 +1,4 @@
-use std::cmp::max;
+use std::cmp::{max, min};
 
 use cuprate_database::{DatabaseRo, DatabaseRw, RuntimeError};
 use cuprate_types::{Chain, ChainId};
@@ -22,10 +22,26 @@ pub fn update_alt_chain_info(
     prev_hash: &BlockHash,
     tables: &mut impl TablesMut,
 ) -> Result<(), RuntimeError> {
+    let parent_chain = match tables.alt_block_heights().get(prev_hash) {
+        Ok(alt_parent_height) => Chain::Alt(alt_parent_height.chain_id.into()),
+        Err(RuntimeError::KeyNotFound) => Chain::Main,
+        Err(e) => return Err(e),
+    };
+
     // try update the info if one exists for this chain.
     let update = tables
         .alt_chain_infos_mut()
         .update(&alt_block_height.chain_id, |mut info| {
+            if info.chain_height < alt_block_height.height + 1 {
+                // If the chain height is increasing we only need to update the chain height.
+                info.chain_height = alt_block_height.height + 1;
+            } else {
+                // If the chain height is not increasing we are popping blocks and need to update the
+                // split point.
+                info.common_ancestor_height = alt_block_height.height.checked_sub(1).unwrap();
+                info.parent_chain = parent_chain.into();
+            }
+
             info.chain_height = alt_block_height.height + 1;
             Some(info)
         });
@@ -38,12 +54,6 @@ pub fn update_alt_chain_info(
 
     // If one doesn't already exist add it.
 
-    let parent_chain = match tables.alt_block_heights().get(prev_hash) {
-        Ok(alt_parent_height) => Chain::Alt(alt_parent_height.chain_id.into()),
-        Err(RuntimeError::KeyNotFound) => Chain::Main,
-        Err(e) => return Err(e),
-    };
-
     tables.alt_chain_infos_mut().put(
         &alt_block_height.chain_id,
         &AltChainInfo {
@@ -73,8 +83,12 @@ pub fn get_alt_chain_history_ranges(
         let chain_info = alt_chain_infos.get(&current_chain_id)?;
 
         let start_height = max(range.start, chain_info.common_ancestor_height + 1);
+        let end_height = min(i, chain_info.chain_height);
 
-        ranges.push((Chain::Alt(current_chain_id.into()), start_height..i));
+        ranges.push((
+            Chain::Alt(current_chain_id.into()),
+            start_height..end_height,
+        ));
         i = chain_info.common_ancestor_height + 1;
 
         match chain_info.parent_chain.into() {
@@ -83,7 +97,17 @@ pub fn get_alt_chain_history_ranges(
                 break;
             }
             Chain::Alt(alt_chain_id) => {
-                current_chain_id = alt_chain_id.into();
+                let alt_chain_id = alt_chain_id.into();
+
+                // This shouldn't be possible to hit, however in a test with custom (invalid) block data
+                // this caused an infinite loop.
+                if alt_chain_id == current_chain_id {
+                    return Err(RuntimeError::Io(std::io::Error::other(
+                        "Loop detected in ChainIDs, invalid alt chain.",
+                    )));
+                }
+
+                current_chain_id = alt_chain_id;
                 continue;
             }
         }

storage/blockchain/src/ops/alt_block/mod.rs

@@ -40,36 +40,19 @@
 //! to get all the alt-blocks in a given [`ChainID`](cuprate_types::ChainId).
 //!
 //! Although this should be kept in mind as a possibility, because Cuprate's block downloader will only track a single chain it is
-//! unlikely that we will be tracking [`ChainID`](cuprate_types::ChainId) that don't immediately connect to the main-chain.
+//! unlikely that we will be tracking [`ChainID`](cuprate_types::ChainId)s that don't immediately connect to the main-chain.
 //!
-//! ## Why not use block's previous field?
+//! ## Why not use the block's `previous` field?
 //!
 //! Although that would be easier, it makes getting a range of block extremely slow, as we have to build the weight cache to verify
-//! blocks, roughly 100,000 block headers needed, this cost was seen as too high.
-pub use block::*;
-pub use chain::*;
-pub use tx::*;
-
+//! blocks, roughly 100,000 block headers needed, this cost is too high.
 mod block;
 mod chain;
 mod tx;
 
-/// Flush all alt-block data from all the alt-block tables.
-///
-/// This function completely empties the alt block tables.
-pub fn flush_alt_blocks<'a, E: cuprate_database::EnvInner<'a>>(
-    env_inner: &E,
-    tx_rw: &mut E::Rw<'_>,
-) -> Result<(), cuprate_database::RuntimeError> {
-    use crate::tables::{
-        AltBlockBlobs, AltBlockHeights, AltBlocksInfo, AltChainInfos, AltTransactionBlobs,
-        AltTransactionInfos,
-    };
-
-    env_inner.clear_db::<AltChainInfos>(tx_rw)?;
-    env_inner.clear_db::<AltBlockHeights>(tx_rw)?;
-    env_inner.clear_db::<AltBlocksInfo>(tx_rw)?;
-    env_inner.clear_db::<AltBlockBlobs>(tx_rw)?;
-    env_inner.clear_db::<AltTransactionBlobs>(tx_rw)?;
-    env_inner.clear_db::<AltTransactionInfos>(tx_rw)
-}
+pub use block::{
+    add_alt_block, flush_alt_blocks, get_alt_block, get_alt_block_extended_header_from_height,
+    get_alt_block_hash,
+};
+pub use chain::{get_alt_chain_history_ranges, update_alt_chain_info};
+pub use tx::{add_alt_transaction_blob, get_alt_transaction};

storage/blockchain/src/ops/alt_block/tx.rs

@@ -10,8 +10,8 @@ use crate::{
     types::{AltTransactionInfo, TxHash},
 };
 
-/// Adds a [`VerifiedTransactionInformation`] form an alt-block to the DB, if
-/// that transaction is not already in the DB.
+/// Adds a [`VerifiedTransactionInformation`] from an alt-block
+/// if it is not already in the DB.
 ///
 /// If the transaction is in the main-chain this function will still fill in the
 /// [`AltTransactionInfos`](crate::tables::AltTransactionInfos) table, as that
@@ -40,7 +40,9 @@ pub fn add_alt_transaction_blob(
 
     tables
         .alt_transaction_blobs_mut()
-        .put(&tx.tx_hash, StorableVec::wrap_ref(&tx.tx_blob))
+        .put(&tx.tx_hash, StorableVec::wrap_ref(&tx.tx_blob))?;
+
+    Ok(())
 }
 
 /// Retrieve a [`VerifiedTransactionInformation`] from the database.

storage/blockchain/src/ops/block.rs

@@ -9,7 +9,7 @@ use cuprate_database::{
 };
 use cuprate_helper::{
     map::{combine_low_high_bits_to_u128, split_u128_into_low_high_bits},
-    tx_utils::tx_fee,
+    tx::tx_fee,
 };
 use cuprate_types::{
     AltBlockInformation, ChainId, ExtendedBlockHeader, HardFork, VerifiedBlockInformation,
@@ -157,32 +157,34 @@ pub fn pop_block(
     let block = Block::read(&mut block_blob.as_slice())?;
 
     //------------------------------------------------------ Transaction / Outputs / Key Images
-    let mut txs = Vec::with_capacity(block.transactions.len());
-
     remove_tx(&block.miner_transaction.hash(), tables)?;
-    for tx_hash in &block.transactions {
-        let (_, tx) = remove_tx(tx_hash, tables)?;
 
-        if move_to_alt_chain.is_some() {
-            txs.push(VerifiedTransactionInformation {
-                tx_weight: tx.weight(),
-                tx_blob: tx.serialize(),
-                tx_hash: tx.hash(),
-                fee: tx_fee(&tx),
-                tx,
-            });
-        }
-    }
+    let remove_tx_iter = block.transactions.iter().map(|tx_hash| {
+        let (_, tx) = remove_tx(tx_hash, tables)?;
+        Ok::<_, RuntimeError>(tx)
+    });
 
     if let Some(chain_id) = move_to_alt_chain {
+        let txs = remove_tx_iter
+            .map(|result| {
+                let tx = result?;
+                Ok(VerifiedTransactionInformation {
+                    tx_weight: tx.weight(),
+                    tx_blob: tx.serialize(),
+                    tx_hash: tx.hash(),
+                    fee: tx_fee(&tx),
+                    tx,
+                })
+            })
+            .collect::<Result<Vec<VerifiedTransactionInformation>, RuntimeError>>()?;
+
         alt_block::add_alt_block(
             &AltBlockInformation {
                 block: block.clone(),
                 block_blob,
                 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.
+                // We know the PoW is valid for this block so just set it so it will always verify as valid.
                 pow_hash: [0; 32],
                 height: block_height,
                 weight: block_info.weight,
@@ -195,6 +197,10 @@ pub fn pop_block(
             },
             tables,
         )?;
+    } else {
+        for result in remove_tx_iter {
+            drop(result?);
+        }
     }
 
     Ok((block_height, block_info.block_hash, block))

storage/blockchain/src/ops/macros.rs

@@ -32,10 +32,11 @@ When calling this function, ensure that either:
 }
 pub(super) use doc_add_block_inner_invariant;
 
-// This is pretty much the same as [`doc_add_block_inner_invariant`], it's not worth the effort to reduce
-// the duplication.
 /// Generate `# Invariant` documentation for internal alt block `fn`'s
 /// that should be called directly with caution.
+///
+/// This is pretty much the same as [`doc_add_block_inner_invariant`],
+/// it's not worth the effort to reduce the duplication.
 macro_rules! doc_add_alt_block_inner_invariant {
     () => {
             r#"# ⚠️ Invariant ⚠️

storage/blockchain/src/service/read.rs

@@ -230,12 +230,14 @@ fn find_block(env: &ConcreteEnv, block_hash: BlockHash) -> ResponseResult {
 
     let table_alt_block_heights = env_inner.open_db_ro::<AltBlockHeights>(&tx_ro)?;
 
-    let height = table_alt_block_heights.get(&block_hash)?;
-
-    Ok(BlockchainResponse::FindBlock(Some((
-        Chain::Alt(height.chain_id.into()),
-        height.height,
-    ))))
+    match table_alt_block_heights.get(&block_hash) {
+        Ok(height) => Ok(BlockchainResponse::FindBlock(Some((
+            Chain::Alt(height.chain_id.into()),
+            height.height,
+        )))),
+        Err(RuntimeError::KeyNotFound) => Ok(BlockchainResponse::FindBlock(None)),
+        Err(e) => Err(e),
+    }
 }
 
 /// [`BlockchainReadRequest::FilterUnknownHashes`].
@@ -300,7 +302,7 @@ fn block_extended_header_in_range(
             ranges
                 .par_iter()
                 .rev()
-                .map(|(chain, range)| {
+                .flat_map(|(chain, range)| {
                     range.clone().into_par_iter().map(|height| {
                         let tx_ro = tx_ro.get_or_try(|| env_inner.tx_ro())?;
                         let tables = get_tables!(env_inner, tx_ro, tables)?.as_ref();
@@ -317,7 +319,6 @@ fn block_extended_header_in_range(
                         }
                     })
                 })
-                .flatten()
                 .collect::<Result<Vec<_>, _>>()?
         }
     };