review changes

binaries/cuprated/src/p2p/request_handler.rs

@@ -38,7 +38,7 @@ use cuprate_p2p_core::{
 use cuprate_txpool::service::TxpoolReadHandle;
 use cuprate_types::{
     blockchain::{BlockchainReadRequest, BlockchainResponse},
-    BlockCompleteEntry, MissingTxsInBlock, TransactionBlobs,
+    BlockCompleteEntry, TransactionBlobs, TxsInBlock,
 };
 use cuprate_wire::protocol::{
     ChainRequest, ChainResponse, FluffyMissingTransactionsRequest, GetObjectsRequest,
@@ -56,9 +56,7 @@ use crate::{
 #[derive(Clone)]
 pub struct P2pProtocolRequestHandlerMaker {
     pub blockchain_read_handle: BlockchainReadHandle,
-
     pub blockchain_context_service: BlockChainContextService,
-
     pub txpool_read_handle: TxpoolReadHandle,
 
     /// The [`IncomingTxHandler`], wrapped in an [`Option`] as there is a cyclic reference between [`P2pProtocolRequestHandlerMaker`]
@@ -115,13 +113,9 @@ where
 #[derive(Clone)]
 pub struct P2pProtocolRequestHandler<N: NetZoneAddress> {
     peer_information: PeerInformation<N>,
-
     blockchain_read_handle: BlockchainReadHandle,
-
     blockchain_context_service: BlockChainContextService,
-
     txpool_read_handle: TxpoolReadHandle,
-
     incoming_tx_handler: IncomingTxHandler,
 }
 
@@ -196,7 +190,7 @@ async fn get_objects(
         .call(BlockchainReadRequest::BlockCompleteEntries(block_hashes))
         .await?
     else {
-        panic!("blockchain returned wrong response!");
+        unreachable!();
     };
 
     Ok(ProtocolResponse::GetObjects(GetObjectsResponse {
@@ -233,12 +227,12 @@ async fn get_chain(
         .call(BlockchainReadRequest::NextChainEntry(block_hashes, 10_000))
         .await?
     else {
-        panic!("blockchain returned wrong response!");
+        unreachable!();
     };
 
-    if start_height == 0 {
+    let Some(start_height) = start_height else {
         anyhow::bail!("The peers chain has a different genesis block than ours.");
-    }
+    };
 
     let (cumulative_difficulty_low64, cumulative_difficulty_top64) =
         split_u128_into_low_high_bits(cumulative_difficulty);
@@ -271,19 +265,19 @@ async fn fluffy_missing_txs(
     // deallocate the backing `Bytes`.
     drop(request);
 
-    let BlockchainResponse::MissingTxsInBlock(res) = blockchain_read_handle
+    let BlockchainResponse::TxsInBlock(res) = blockchain_read_handle
         .ready()
         .await?
-        .call(BlockchainReadRequest::MissingTxsInBlock {
+        .call(BlockchainReadRequest::TxsInBlock {
             block_hash,
             tx_indexes,
         })
         .await?
     else {
-        panic!("blockchain returned wrong response!");
+        unreachable!();
     };
 
-    let Some(MissingTxsInBlock { block, txs }) = res else {
+    let Some(TxsInBlock { block, txs }) = res else {
         anyhow::bail!("The peer requested txs out of range.");
     };
 
@@ -412,11 +406,7 @@ where
     };
 
     // Drop all the data except the stuff we still need.
-    let NewTransactions {
+    let NewTransactions { txs, .. } = request;
-        txs,
-        dandelionpp_fluff: _,
-        padding: _,
-    } = request;
 
     let res = incoming_tx_handler
         .ready()

storage/blockchain/Cargo.toml

@@ -34,7 +34,7 @@ serde            = { workspace = true, optional = true }
 tower        = { workspace = true }
 thread_local = { workspace = true }
 rayon        = { workspace = true }
-bytes = "1.7.2"
+bytes        = { workspace = true }
 
 [dev-dependencies]
 cuprate-constants  = { workspace = true }

storage/blockchain/src/ops/block.rs

@@ -226,6 +226,7 @@ pub fn pop_block(
 
     Ok((block_height, block_info.block_hash, block))
 }
+
 //---------------------------------------------------------------------------------------------------- `get_block_blob_with_tx_indexes`
 /// Retrieve a block's raw bytes, the index of the miner transaction and the number of non miner-txs in the block.
 ///
@@ -234,11 +235,8 @@ pub fn get_block_blob_with_tx_indexes(
     block_height: &BlockHeight,
     tables: &impl Tables,
 ) -> Result<(Vec<u8>, u64, usize), RuntimeError> {
-    use monero_serai::io::write_varint;
+    let miner_tx_idx = tables.block_infos().get(block_height)?.mining_tx_index;
 
-    let block_info = tables.block_infos().get(block_height)?;
-
-    let miner_tx_idx = block_info.mining_tx_index;
     let block_txs = tables.block_txs_hashes().get(block_height)?.0;
     let numb_txs = block_txs.len();
 
@@ -250,10 +248,10 @@ pub fn get_block_blob_with_tx_indexes(
     block.append(&mut miner_tx_blob);
 
     // Add the blocks tx hashes.
-    write_varint(&block_txs.len(), &mut block)
+    monero_serai::io::write_varint(&block_txs.len(), &mut block)
         .expect("The number of txs per block will not exceed u64::MAX");
 
-    let block_txs_bytes = bytemuck::cast_slice(&block_txs);
+    let block_txs_bytes = bytemuck::must_cast_slice(&block_txs);
     block.extend_from_slice(block_txs_bytes);
 
     Ok((block, miner_tx_idx, numb_txs))
@@ -275,7 +273,7 @@ pub fn get_block_complete_entry(
 
     let tx_blobs = tables
         .tx_blobs_iter()
-        .get_range(first_tx_idx..=usize_to_u64(numb_non_miner_txs))?
+        .get_range(first_tx_idx..(usize_to_u64(numb_non_miner_txs) + first_tx_idx))?
         .map(|tx_blob| Ok(Bytes::from(tx_blob?.0)))
         .collect::<Result<_, RuntimeError>>()?;
 

storage/blockchain/src/ops/blockchain.rs

@@ -98,17 +98,16 @@ pub fn find_split_point(
     let mut err = None;
 
     // Do a binary search to find the first unknown/known block in the batch.
-    let idx =
+    let idx = block_ids.partition_point(|block_id| {
-        block_ids.partition_point(
+        match block_exists(block_id, table_block_heights) {
-            |block_id| match block_exists(block_id, table_block_heights) {
+            Ok(exists) => exists == chronological_order,
-                Ok(exists) => exists & chronological_order,
+            Err(e) => {
-                Err(e) => {
+                err.get_or_insert(e);
-                    err.get_or_insert(e);
+                // if this happens the search is scrapped, just return `false` back.
-                    // if this happens the search is scrapped, just return `false` back.
+                false
-                    false
+            }
-                }
+        }
-            },
+    });
-        );
 
     if let Some(e) = err {
         return Err(e);

storage/blockchain/src/service/read.rs

@@ -27,7 +27,7 @@ use cuprate_database_service::{init_thread_pool, DatabaseReadService, ReaderThre
 use cuprate_helper::map::combine_low_high_bits_to_u128;
 use cuprate_types::{
     blockchain::{BlockchainReadRequest, BlockchainResponse},
-    Chain, ChainId, ExtendedBlockHeader, MissingTxsInBlock, OutputHistogramInput, OutputOnChain,
+    Chain, ChainId, ExtendedBlockHeader, OutputHistogramInput, OutputOnChain, TxsInBlock,
 };
 
 use crate::{
@@ -118,10 +118,10 @@ fn map_request(
         R::CompactChainHistory => compact_chain_history(env),
         R::NextChainEntry(block_hashes, amount) => next_chain_entry(env, &block_hashes, amount),
         R::FindFirstUnknown(block_ids) => find_first_unknown(env, &block_ids),
-        R::MissingTxsInBlock {
+        R::TxsInBlock {
             block_hash,
             tx_indexes,
-        } => missing_txs_in_block(env, block_hash, tx_indexes),
+        } => txs_in_block(env, block_hash, tx_indexes),
         R::AltBlocksInChain(chain_id) => alt_blocks_in_chain(env, chain_id),
         R::Block { height } => block(env, height),
         R::BlockByHash(hash) => block_by_hash(env, hash),
@@ -598,7 +598,7 @@ fn next_chain_entry(
     // This will happen if we have a different genesis block.
     if idx == block_ids.len() {
         return Ok(BlockchainResponse::NextChainEntry {
-            start_height: 0,
+            start_height: None,
             chain_height: 0,
             block_ids: vec![],
             block_weights: vec![],
@@ -632,7 +632,7 @@ fn next_chain_entry(
     };
 
     Ok(BlockchainResponse::NextChainEntry {
-        start_height: first_known_height,
+        start_height: Some(first_known_height),
         chain_height,
         block_ids,
         block_weights,
@@ -669,12 +669,8 @@ fn find_first_unknown(env: &ConcreteEnv, block_ids: &[BlockHash]) -> ResponseRes
     })
 }
 
-/// [`BlockchainReadRequest::MissingTxsInBlock`]
+/// [`BlockchainReadRequest::TxsInBlock`]
-fn missing_txs_in_block(
+fn txs_in_block(env: &ConcreteEnv, block_hash: [u8; 32], missing_txs: Vec<u64>) -> ResponseResult {
-    env: &ConcreteEnv,
-    block_hash: [u8; 32],
-    missing_txs: Vec<u64>,
-) -> ResponseResult {
     // Single-threaded, no `ThreadLocal` required.
     let env_inner = env.env_inner();
     let tx_ro = env_inner.tx_ro()?;
@@ -686,7 +682,7 @@ fn missing_txs_in_block(
     let first_tx_index = miner_tx_index + 1;
 
     if numb_txs < missing_txs.len() {
-        return Ok(BlockchainResponse::MissingTxsInBlock(None));
+        return Ok(BlockchainResponse::TxsInBlock(None));
     }
 
     let txs = missing_txs
@@ -694,9 +690,10 @@ fn missing_txs_in_block(
         .map(|index_offset| Ok(tables.tx_blobs().get(&(first_tx_index + index_offset))?.0))
         .collect::<Result<_, RuntimeError>>()?;
 
-    Ok(BlockchainResponse::MissingTxsInBlock(Some(
+    Ok(BlockchainResponse::TxsInBlock(Some(TxsInBlock {
-        MissingTxsInBlock { block, txs },
+        block,
-    )))
+        txs,
+    })))
 }
 
 /// [`BlockchainReadRequest::AltBlocksInChain`]

types/src/blockchain.rs

@@ -11,9 +11,7 @@ use std::{
 use monero_serai::block::Block;
 
 use crate::{
-    types::{
+    types::{Chain, ExtendedBlockHeader, OutputOnChain, TxsInBlock, VerifiedBlockInformation},
-        Chain, ExtendedBlockHeader, MissingTxsInBlock, OutputOnChain, VerifiedBlockInformation,
-    },
     AltBlockInformation, BlockCompleteEntry, ChainId, ChainInfo, CoinbaseTxSum,
     OutputHistogramEntry, OutputHistogramInput,
 };
@@ -122,7 +120,7 @@ pub enum BlockchainReadRequest {
     FindFirstUnknown(Vec<[u8; 32]>),
 
     /// A request for transactions from a specific block.
-    MissingTxsInBlock {
+    TxsInBlock {
         /// The block to get transactions from.
         block_hash: [u8; 32],
         /// The indexes of the transactions from the block.
@@ -287,10 +285,10 @@ pub enum BlockchainResponse {
 
     /// Response to [`BlockchainReadRequest::NextChainEntry`].
     ///
-    /// If all blocks were unknown `start_height` will be `0`, the other fields will be meaningless.
+    /// If all blocks were unknown `start_height` will be [`None`], the other fields will be meaningless.
     NextChainEntry {
-        /// The start height of this entry, `0` if we could not find the split point.
+        /// The start height of this entry, [`None`] if we could not find the split point.
-        start_height: usize,
+        start_height: Option<usize>,
         /// The current chain height.
         chain_height: usize,
         /// The next block hashes in the entry.
@@ -310,10 +308,10 @@ pub enum BlockchainResponse {
     /// This will be [`None`] if all blocks were known.
     FindFirstUnknown(Option<(usize, usize)>),
 
-    /// The response for [`BlockchainReadRequest::MissingTxsInBlock`].
+    /// The response for [`BlockchainReadRequest::TxsInBlock`].
     ///
     /// Will return [`None`] if the request contained an index out of range.
-    MissingTxsInBlock(Option<MissingTxsInBlock>),
+    TxsInBlock(Option<TxsInBlock>),
 
     /// The response for [`BlockchainReadRequest::AltBlocksInChain`].
     ///

types/src/lib.rs

@@ -26,7 +26,7 @@ pub use transaction_verification_data::{
 pub use types::{
     AddAuxPow, AltBlockInformation, AuxPow, Chain, ChainId, ChainInfo, CoinbaseTxSum,
     ExtendedBlockHeader, FeeEstimate, HardForkInfo, MinerData, MinerDataTxBacklogEntry,
-    MissingTxsInBlock, OutputHistogramEntry, OutputHistogramInput, OutputOnChain,
+    OutputHistogramEntry, OutputHistogramInput, OutputOnChain, TxsInBlock,
     VerifiedBlockInformation, VerifiedTransactionInformation,
 };
 

types/src/types.rs

@@ -261,7 +261,7 @@ pub struct AddAuxPow {
 
 /// The inner response for a request for missing txs.
 #[derive(Clone, Debug, PartialEq, Eq)]
-pub struct MissingTxsInBlock {
+pub struct TxsInBlock {
     pub block: Vec<u8>,
     pub txs: Vec<Vec<u8>>,
 }