From cdd8a3135765c3b5a87027f9a5e0103e737c709a Mon Sep 17 00:00:00 2001 From: Aaron Feickert <66188213+AaronFeickert@users.noreply.github.com> Date: Thu, 16 Nov 2023 00:13:38 -0600 Subject: [PATCH] feat!: move kernel MMR position to `u64` (#5956) Description --- Switches the handling of kernel MMR positions from `u32` to `u64`. Closes #5924. Motivation and Context --- Internally, MMR positions are handled as either `usize` or `u64`. However, they are eventually parsed and handled as `u32` for storage. While it doesn't address the internal `usize` handling, it switches storage operations to be `u64`. How Has This Been Tested? --- Existing tests pass. What process can a PR reviewer use to test or verify this change? --- Assert that the logic, especially around LMDB operations, is correct. BREAKING CHANGE: Modifies the handling of kernel MMR data in LMDB. --- .../sync/horizon_state_sync/synchronizer.rs | 6 +----- base_layer/core/src/chain_storage/async_db.rs | 2 +- .../core/src/chain_storage/db_transaction.rs | 4 ++-- .../core/src/chain_storage/lmdb_db/lmdb_db.rs | 14 ++++++-------- base_layer/core/src/chain_storage/lmdb_db/mod.rs | 2 +- 5 files changed, 11 insertions(+), 17 deletions(-) diff --git a/base_layer/core/src/base_node/sync/horizon_state_sync/synchronizer.rs b/base_layer/core/src/base_node/sync/horizon_state_sync/synchronizer.rs index 489b5aa23b..3a4a528841 100644 --- a/base_layer/core/src/base_node/sync/horizon_state_sync/synchronizer.rs +++ b/base_layer/core/src/base_node/sync/horizon_state_sync/synchronizer.rs @@ -373,11 +373,7 @@ impl<'a, B: BlockchainBackend + 'static> HorizonStateSynchronization<'a, B> { )); } - let mmr_position_u32 = u32::try_from(mmr_position).map_err(|_| HorizonSyncError::InvalidMmrPosition { - at_height: current_header.height(), - mmr_position, - })?; - txn.insert_kernel_via_horizon_sync(kernel, *current_header.hash(), mmr_position_u32); + txn.insert_kernel_via_horizon_sync(kernel, *current_header.hash(), mmr_position); if mmr_position == current_header.header().kernel_mmr_size - 1 { let num_kernels = kernel_hashes.len(); debug!( diff --git a/base_layer/core/src/chain_storage/async_db.rs b/base_layer/core/src/chain_storage/async_db.rs index b24cd9bc72..9bc66c092c 100644 --- a/base_layer/core/src/chain_storage/async_db.rs +++ b/base_layer/core/src/chain_storage/async_db.rs @@ -330,7 +330,7 @@ impl<'a, B: BlockchainBackend + 'static> AsyncDbTransaction<'a, B> { &mut self, kernel: TransactionKernel, header_hash: HashOutput, - mmr_position: u32, + mmr_position: u64, ) -> &mut Self { self.transaction.insert_kernel(kernel, header_hash, mmr_position); self diff --git a/base_layer/core/src/chain_storage/db_transaction.rs b/base_layer/core/src/chain_storage/db_transaction.rs index 1b05b8e2ce..3e37c36094 100644 --- a/base_layer/core/src/chain_storage/db_transaction.rs +++ b/base_layer/core/src/chain_storage/db_transaction.rs @@ -91,7 +91,7 @@ impl DbTransaction { &mut self, kernel: TransactionKernel, header_hash: HashOutput, - mmr_position: u32, + mmr_position: u64, ) -> &mut Self { self.operations.push(WriteOperation::InsertKernel { header_hash, @@ -279,7 +279,7 @@ pub enum WriteOperation { InsertKernel { header_hash: HashOutput, kernel: Box, - mmr_position: u32, + mmr_position: u64, }, InsertOutput { header_hash: HashOutput, 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 28e561f1bb..c5ed343240 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 @@ -134,8 +134,8 @@ const LMDB_DB_VALIDATOR_NODES_MAPPING: &str = "validator_nodes_mapping"; const LMDB_DB_TEMPLATE_REGISTRATIONS: &str = "template_registrations"; const LMDB_DB_TIP_UTXO_SMT: &str = "tip_utxo_smt"; -/// HeaderHash(32), mmr_pos(4), hash(32) -type KernelKey = CompositeKey<68>; +/// HeaderHash(32), mmr_pos(8), hash(32) +type KernelKey = CompositeKey<72>; /// Height(8), Hash(32) type ValidatorNodeRegistrationKey = CompositeKey<40>; @@ -566,7 +566,7 @@ impl LMDBDatabase { txn: &WriteTransaction<'_>, header_hash: &HashOutput, kernel: &TransactionKernel, - mmr_position: u32, + mmr_position: u64, ) -> Result<(), ChainStorageError> { let hash = kernel.hash(); let key = KernelKey::try_from_parts(&[ @@ -1131,15 +1131,13 @@ impl LMDBDatabase { for kernel in kernels { total_kernel_sum = &total_kernel_sum + &kernel.excess; - let pos = kernel_mmr.push(kernel.hash().to_vec())?; + let pos = + u64::try_from(kernel_mmr.push(kernel.hash().to_vec())?).map_err(|_| ChainStorageError::OutOfRange)?; trace!( target: LOG_TARGET, "Inserting kernel `{}`", kernel.excess_sig.get_signature().to_hex() ); - let pos = u32::try_from(pos).map_err(|_| { - ChainStorageError::InvalidOperation(format!("Kernel MMR node count ({}) is greater than u32::MAX", pos)) - })?; self.insert_kernel(txn, &block_hash, &kernel, pos)?; } let k = MetadataKey::TipSmt; @@ -1815,7 +1813,7 @@ impl BlockchainBackend for LMDBDatabase { key.extend(excess_sig.get_public_nonce().as_bytes()); key.extend(excess_sig.get_signature().as_bytes()); if let Some((header_hash, mmr_position, hash)) = - lmdb_get::<_, (HashOutput, u32, HashOutput)>(&txn, &self.kernel_excess_sig_index, key.as_slice())? + lmdb_get::<_, (HashOutput, u64, HashOutput)>(&txn, &self.kernel_excess_sig_index, key.as_slice())? { let key = KernelKey::try_from_parts(&[ header_hash.as_slice(), diff --git a/base_layer/core/src/chain_storage/lmdb_db/mod.rs b/base_layer/core/src/chain_storage/lmdb_db/mod.rs index c710b402e5..9e5bcb0821 100644 --- a/base_layer/core/src/chain_storage/lmdb_db/mod.rs +++ b/base_layer/core/src/chain_storage/lmdb_db/mod.rs @@ -70,7 +70,7 @@ pub(crate) struct TransactionInputRowData { pub(crate) struct TransactionKernelRowData { pub kernel: TransactionKernel, pub header_hash: HashOutput, - pub mmr_position: u32, + pub mmr_position: u64, pub hash: HashOutput, }