fix(core): fetch_header_containing_*_mmr functions now take a 0-based mmr position

base_layer/core/src/base_node/state_machine_service/states/horizon_state_sync/horizon_state_synchronization.rs

@@ -199,10 +199,7 @@ impl<'a, B: BlockchainBackend + 'static> HorizonStateSynchronization<'a, B> {
             latency.unwrap_or_default().as_millis()
         );
 
-        let mut current_header = self
-            .db()
-            .fetch_header_containing_kernel_mmr(local_num_kernels + 1)
-            .await?;
+        let mut current_header = self.db().fetch_header_containing_kernel_mmr(local_num_kernels).await?;
         let req = SyncKernelsRequest {
             start: local_num_kernels,
             end_header_hash: to_header.hash(),
@@ -362,7 +359,7 @@ impl<'a, B: BlockchainBackend + 'static> HorizonStateSynchronization<'a, B> {
             include_pruned_utxos: true,
         };
 
-        let mut current_header = self.db().fetch_header_containing_utxo_mmr(start + 1).await?;
+        let mut current_header = self.db().fetch_header_containing_utxo_mmr(start).await?;
         let mut output_stream = client.sync_utxos(req).await?;
 
         debug!(

base_layer/core/src/base_node/sync/rpc/service.rs

@@ -435,7 +435,7 @@ impl<B: BlockchainBackend + 'static> BaseNodeSyncService for BaseNodeSyncRpcServ
         let db = self.db();
 
         let start_header = db
-            .fetch_header_containing_kernel_mmr(req.start + 1)
+            .fetch_header_containing_kernel_mmr(req.start)
             .await
             .map_err(RpcStatus::log_internal_error(LOG_TARGET))?
             .into_header();

base_layer/core/src/base_node/sync/rpc/sync_utxos_task.rs

@@ -54,7 +54,7 @@ where B: BlockchainBackend + 'static
     ) -> Result<(), RpcStatus> {
         let start_header = self
             .db
-            .fetch_header_containing_utxo_mmr(request.start + 1)
+            .fetch_header_containing_utxo_mmr(request.start)
             .await
             .map_err(|err| {
                 error!(target: LOG_TARGET, "{}", err);

base_layer/core/src/chain_storage/blockchain_database.rs

@@ -449,11 +449,6 @@ where B: BlockchainBackend
         Ok(chain_header)
     }
 
-    // TODO: this method is actually off by one. It returns the highest header where the kernel_mmr_count is <=
-    // mmr_position, but should probably only be <.
-    // E.g. if mmr_position == 2, it will return a header where kernel_mmr_count == 2, but this is
-    // confusing because mmr_position ==2 actually would be in the next header
-    // Either the caller needs to be updated or this method needs to be renamed
     pub fn fetch_header_containing_kernel_mmr(&self, mmr_position: u64) -> Result<ChainHeader, ChainStorageError> {
         let db = self.db_read_access()?;
         db.fetch_header_containing_kernel_mmr(mmr_position)

base_layer/core/src/chain_storage/lmdb_db/lmdb_db.rs

@@ -1738,6 +1738,9 @@ impl BlockchainBackend for LMDBDatabase {
 
     fn fetch_header_containing_kernel_mmr(&self, mmr_position: u64) -> Result<ChainHeader, ChainStorageError> {
         let txn = self.read_transaction()?;
+        // LMDB returns the height at the position, so we have to offset the position by 1 so that the mmr_position arg
+        // is an index starting from 0
+        let mmr_position = mmr_position + 1;
 
         let height = lmdb_first_after::<_, u64>(&txn, &self.kernel_mmr_size_index, &mmr_position.to_be_bytes())?
             .ok_or_else(|| ChainStorageError::ValueNotFound {
@@ -1773,6 +1776,9 @@ impl BlockchainBackend for LMDBDatabase {
     // TODO: Can be merged with the method above
     fn fetch_header_containing_utxo_mmr(&self, mmr_position: u64) -> Result<ChainHeader, ChainStorageError> {
         let txn = self.read_transaction()?;
+        // LMDB returns the height at the position, so we have to offset the position by 1 so that the mmr_position arg
+        // is an index starting from 0
+        let mmr_position = mmr_position + 1;
 
         let (height, _hash) =
             lmdb_first_after::<_, (u64, Vec<u8>)>(&txn, &self.output_mmr_size_index, &mmr_position.to_be_bytes())?

base_layer/core/src/chain_storage/tests/blockchain_database.rs

@@ -575,7 +575,7 @@ mod fetch_header_containing_utxo_mmr {
             assert_eq!(header.height(), 0);
             mmr_position += 1;
         });
-        let err = db.fetch_header_containing_utxo_mmr(4002).unwrap_err();
+        let err = db.fetch_header_containing_utxo_mmr(mmr_position).unwrap_err();
         matches!(err, ChainStorageError::ValueNotFound { .. });
     }
 
@@ -586,12 +586,16 @@ mod fetch_header_containing_utxo_mmr {
         let _ = add_many_chained_blocks(5, &db);
         let num_genesis_outputs = genesis.block().body.outputs().len() as u64;
 
+        let header = db.fetch_header_containing_utxo_mmr(num_genesis_outputs - 1).unwrap();
+        assert_eq!(header.height(), 0);
+
         for i in 1..=5 {
-            let header = db.fetch_header_containing_utxo_mmr(num_genesis_outputs + i).unwrap();
-            assert_eq!(header.height(), i);
+            let index = num_genesis_outputs + i - 1;
+            let header = db.fetch_header_containing_utxo_mmr(index).unwrap();
+            assert_eq!(header.height(), i, "Incorrect header for MMR index = {}", index);
         }
         let err = db
-            .fetch_header_containing_utxo_mmr(num_genesis_outputs + 5 + 1)
+            .fetch_header_containing_utxo_mmr(num_genesis_outputs + 5)
             .unwrap_err();
         matches!(err, ChainStorageError::ValueNotFound { .. });
     }
@@ -605,13 +609,13 @@ mod fetch_header_containing_kernel_mmr {
         let db = setup();
         let genesis = db.fetch_block(0).unwrap();
         assert_eq!(genesis.block().body.kernels().len(), 2);
-        // let mut mmr_position = 0;
-        // genesis.block().body.kernels().iter().for_each(|_| {
-        //     let header = db.fetch_header_containing_kernel_mmr(mmr_position).unwrap();
-        //     assert_eq!(header.height(), 0);
-        //     mmr_position += 1;
-        // });
-        let err = db.fetch_header_containing_kernel_mmr(3).unwrap_err();
+        let mut mmr_position = 0;
+        genesis.block().body.kernels().iter().for_each(|_| {
+            let header = db.fetch_header_containing_kernel_mmr(mmr_position).unwrap();
+            assert_eq!(header.height(), 0);
+            mmr_position += 1;
+        });
+        let err = db.fetch_header_containing_kernel_mmr(mmr_position).unwrap_err();
         matches!(err, ChainStorageError::ValueNotFound { .. });
     }
 
@@ -627,22 +631,22 @@ mod fetch_header_containing_kernel_mmr {
         db.add_block(block).unwrap();
         let _ = add_many_chained_blocks(3, &db);
 
-        let header = db.fetch_header_containing_kernel_mmr(num_genesis_kernels).unwrap();
+        let header = db.fetch_header_containing_kernel_mmr(num_genesis_kernels - 1).unwrap();
         assert_eq!(header.height(), 0);
-        let header = db.fetch_header_containing_kernel_mmr(num_genesis_kernels + 1).unwrap();
+        let header = db.fetch_header_containing_kernel_mmr(num_genesis_kernels).unwrap();
         assert_eq!(header.height(), 1);
 
-        for i in 2..=3 {
+        for i in 1..=2 {
             let header = db.fetch_header_containing_kernel_mmr(num_genesis_kernels + i).unwrap();
             assert_eq!(header.height(), 2);
         }
-        for i in 4..=6 {
+        for i in 3..=5 {
             let header = db.fetch_header_containing_kernel_mmr(num_genesis_kernels + i).unwrap();
-            assert_eq!(header.height(), i - 1);
+            assert_eq!(header.height(), i);
         }
 
         let err = db
-            .fetch_header_containing_kernel_mmr(num_genesis_kernels + 6 + 1)
+            .fetch_header_containing_kernel_mmr(num_genesis_kernels + 6)
             .unwrap_err();
         matches!(err, ChainStorageError::ValueNotFound { .. });
     }