diff --git a/base_layer/core/src/base_node/state_machine_service/states/horizon_state_sync/horizon_state_synchronization.rs b/base_layer/core/src/base_node/state_machine_service/states/horizon_state_sync/horizon_state_synchronization.rs index 9c5924a04f..80937f4d9b 100644 --- a/base_layer/core/src/base_node/state_machine_service/states/horizon_state_sync/horizon_state_synchronization.rs +++ b/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!( diff --git a/base_layer/core/src/base_node/sync/rpc/service.rs b/base_layer/core/src/base_node/sync/rpc/service.rs index adedcb23ef..891775ebc2 100644 --- a/base_layer/core/src/base_node/sync/rpc/service.rs +++ b/base_layer/core/src/base_node/sync/rpc/service.rs @@ -435,7 +435,7 @@ impl 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(); diff --git a/base_layer/core/src/base_node/sync/rpc/sync_utxos_task.rs b/base_layer/core/src/base_node/sync/rpc/sync_utxos_task.rs index bf6c81c5d8..db1e46f94b 100644 --- a/base_layer/core/src/base_node/sync/rpc/sync_utxos_task.rs +++ b/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); diff --git a/base_layer/core/src/chain_storage/blockchain_database.rs b/base_layer/core/src/chain_storage/blockchain_database.rs index 29b62b8a96..35884a5db4 100644 --- a/base_layer/core/src/chain_storage/blockchain_database.rs +++ b/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 { let db = self.db_read_access()?; db.fetch_header_containing_kernel_mmr(mmr_position) diff --git a/base_layer/core/src/chain_storage/lmdb_db/lmdb_db.rs b/base_layer/core/src/chain_storage/lmdb_db/lmdb_db.rs index 14229c5cb3..7bbb2f67e9 100644 --- a/base_layer/core/src/chain_storage/lmdb_db/lmdb_db.rs +++ b/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 { 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 { 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)>(&txn, &self.output_mmr_size_index, &mmr_position.to_be_bytes())? diff --git a/base_layer/core/src/chain_storage/tests/blockchain_database.rs b/base_layer/core/src/chain_storage/tests/blockchain_database.rs index d6a8a10818..5bb31b4d19 100644 --- a/base_layer/core/src/chain_storage/tests/blockchain_database.rs +++ b/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 { .. }); }