From 7107df2b129241da2389331faa64eb1f51b4df78 Mon Sep 17 00:00:00 2001 From: SW van Heerden Date: Tue, 27 Feb 2024 18:43:38 +0200 Subject: [PATCH 1/5] update logs --- base_layer/core/src/chain_storage/blockchain_database.rs | 9 ++++++++- base_layer/core/src/transactions/aggregated_body.rs | 2 +- .../aggregate_body/aggregate_body_internal_validator.rs | 2 +- 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/base_layer/core/src/chain_storage/blockchain_database.rs b/base_layer/core/src/chain_storage/blockchain_database.rs index bea2c9d16a..85a4e5c474 100644 --- a/base_layer/core/src/chain_storage/blockchain_database.rs +++ b/base_layer/core/src/chain_storage/blockchain_database.rs @@ -1334,7 +1334,14 @@ pub fn calculate_mmr_roots( if !output.is_burned() { let smt_key = NodeKey::try_from(output.commitment.as_bytes())?; let smt_node = ValueHash::try_from(output.smt_hash(header.height).as_slice())?; - output_smt.insert(smt_key, smt_node)?; + if let Err(e) = output_smt.insert(smt_key, smt_node) { + error!( + target: LOG_TARGET, + "Output commitment({}) already in SMT", + output.commitment.to_hex(), + ); + return Err(e.into()); + } } } diff --git a/base_layer/core/src/transactions/aggregated_body.rs b/base_layer/core/src/transactions/aggregated_body.rs index 69b7a75887..8a6631dc7e 100644 --- a/base_layer/core/src/transactions/aggregated_body.rs +++ b/base_layer/core/src/transactions/aggregated_body.rs @@ -408,7 +408,7 @@ impl From for AggregateBody { impl Display for AggregateBody { fn fmt(&self, fmt: &mut Formatter<'_>) -> Result<(), Error> { if !self.is_sorted() { - writeln!(fmt, "WARNING: Block body is not sorted.")?; + writeln!(fmt, "WARNING: Body is not sorted.")?; } writeln!(fmt, "--- Transaction Kernels ---")?; for (i, kernel) in self.kernels.iter().enumerate() { diff --git a/base_layer/core/src/validation/aggregate_body/aggregate_body_internal_validator.rs b/base_layer/core/src/validation/aggregate_body/aggregate_body_internal_validator.rs index 0edff76f0e..f2a2b4d929 100644 --- a/base_layer/core/src/validation/aggregate_body/aggregate_body_internal_validator.rs +++ b/base_layer/core/src/validation/aggregate_body/aggregate_body_internal_validator.rs @@ -311,7 +311,7 @@ fn check_weight( if block_weight <= max_weight { trace!( target: LOG_TARGET, - "SV - Block contents for block #{} : {}; weight {}.", + "Aggregated body at height #{} : {}; weight {} is valid.", height, body.to_counts_string(), block_weight, From f2b35f022048aec3310482bbff51204d6cf0b35a Mon Sep 17 00:00:00 2001 From: SW van Heerden Date: Tue, 27 Feb 2024 19:12:21 +0200 Subject: [PATCH 2/5] more changes --- .../sync/horizon_state_sync/synchronizer.rs | 20 +++++++++++++--- .../src/chain_storage/blockchain_database.rs | 2 -- .../core/src/chain_storage/lmdb_db/lmdb_db.rs | 23 ++++++++++++++++--- 3 files changed, 37 insertions(+), 8 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 302ca53a4b..458622800c 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 @@ -32,7 +32,7 @@ use log::*; use tari_common_types::types::{Commitment, FixedHash, RangeProofService}; use tari_comms::{connectivity::ConnectivityRequester, peer_manager::NodeId, protocol::rpc::RpcClient, PeerConnection}; use tari_crypto::commitment::HomomorphicCommitment; -use tari_mmr::sparse_merkle_tree::{NodeKey, ValueHash}; +use tari_mmr::sparse_merkle_tree::{DeleteResult, NodeKey, ValueHash}; use tari_utilities::{hex::Hex, ByteArray}; use tokio::task; @@ -663,7 +663,14 @@ impl<'a, B: BlockchainBackend + 'static> HorizonStateSynchronization<'a, B> { batch_verify_range_proofs(&self.prover, &[&output])?; let smt_key = NodeKey::try_from(output.commitment.as_bytes())?; let smt_node = ValueHash::try_from(output.smt_hash(current_header.height).as_slice())?; - output_smt.insert(smt_key, smt_node)?; + if let Err(e) = output_smt.insert(smt_key, smt_node) { + error!( + target: LOG_TARGET, + "Output commitment({}) already in SMT", + output.commitment.to_hex(), + ); + return Err(e.into()); + } txn.insert_output_via_horizon_sync( output, current_header.hash(), @@ -691,7 +698,14 @@ impl<'a, B: BlockchainBackend + 'static> HorizonStateSynchronization<'a, B> { stxo_counter, ); let smt_key = NodeKey::try_from(commitment_bytes.as_slice())?; - output_smt.delete(&smt_key)?; + match output_smt.delete(&smt_key)? { + DeleteResult::Deleted(_value_hash) => {}, + DeleteResult::KeyNotFound => { + return Err(HorizonSyncError::ChainStorageError( + ChainStorageError::UnspendableInput, + )) + }, + }; // This will only be committed once the SMT has been verified due to rewind difficulties if // we need to abort the sync inputs_to_delete.push((output_hash, commitment)); diff --git a/base_layer/core/src/chain_storage/blockchain_database.rs b/base_layer/core/src/chain_storage/blockchain_database.rs index 85a4e5c474..f79b278b15 100644 --- a/base_layer/core/src/chain_storage/blockchain_database.rs +++ b/base_layer/core/src/chain_storage/blockchain_database.rs @@ -1348,8 +1348,6 @@ pub fn calculate_mmr_roots( for input in body.inputs().iter() { input_mmr.push(input.canonical_hash().to_vec())?; - // Search the DB for the output leaf index so that it can be marked as spent/deleted. - // If the output hash is not found, check the current output_mmr. This allows zero-conf transactions let smt_key = NodeKey::try_from(input.commitment()?.as_bytes())?; match output_smt.delete(&smt_key)? { DeleteResult::Deleted(_value_hash) => {}, 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 2b4faffe2a..928b628dea 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 @@ -941,7 +941,10 @@ impl LMDBDatabase { continue; } let smt_key = NodeKey::try_from(utxo.output.commitment.as_bytes())?; - output_smt.delete(&smt_key)?; + match output_smt.delete(&smt_key)? { + DeleteResult::Deleted(_value_hash) => {}, + DeleteResult::KeyNotFound => return Err(ChainStorageError::UnspendableInput), + }; lmdb_delete( txn, &self.utxo_commitment_index, @@ -993,7 +996,14 @@ impl LMDBDatabase { ); let smt_key = NodeKey::try_from(input.commitment()?.as_bytes())?; let smt_node = ValueHash::try_from(input.smt_hash(row.spent_height).as_slice())?; - output_smt.insert(smt_key, smt_node)?; + if let Err(e) = output_smt.insert(smt_key, smt_node) { + error!( + target: LOG_TARGET, + "Output commitment({}) already in SMT", + input.commitment()?.to_hex(), + ); + return Err(e.into()); + } trace!(target: LOG_TARGET, "Input moved to UTXO set: {}", input); lmdb_insert( @@ -1210,7 +1220,14 @@ impl LMDBDatabase { if !output.is_burned() { let smt_key = NodeKey::try_from(output.commitment.as_bytes())?; let smt_node = ValueHash::try_from(output.smt_hash(header.height).as_slice())?; - output_smt.insert(smt_key, smt_node)?; + if let Err(e) = output_smt.insert(smt_key, smt_node) { + error!( + target: LOG_TARGET, + "Output commitment({}) already in SMT", + output.commitment.to_hex(), + ); + return Err(e.into()); + } } let output_hash = output.hash(); From a1209e060ee51bd268aa4bbba80ace9f6ffd808b Mon Sep 17 00:00:00 2001 From: SW van Heerden Date: Wed, 28 Feb 2024 08:09:50 +0200 Subject: [PATCH 3/5] hard stop checking mmr on rewind --- .../core/src/chain_storage/lmdb_db/lmdb_db.rs | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) 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 928b628dea..c7d4161d92 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 @@ -888,6 +888,7 @@ impl LMDBDatabase { .fetch_height_from_hash(write_txn, block_hash) .or_not_found("Block", "hash", hash_hex)?; let next_height = height.saturating_add(1); + let prev_height = height.saturating_sub(1); if self.fetch_block_accumulated_data(write_txn, next_height)?.is_some() { return Err(ChainStorageError::InvalidOperation(format!( "Attempted to delete block at height {} while next block still exists", @@ -904,6 +905,21 @@ impl LMDBDatabase { let mut smt = self.fetch_tip_smt()?; self.delete_block_inputs_outputs(write_txn, block_hash.as_slice(), &mut smt)?; + + let new_tip_header = self.fetch_chain_header_by_height(prev_height)?; + let root = FixedHash::try_from(smt.hash().as_slice())?; + if root != new_tip_header.header().output_mr { + error!( + target: LOG_TARGET, + "Deleting block, new smt root(#{}) did not match expected (#{}) smt root", + root.to_hex(), + new_tip_header.header().output_mr.to_hex(), + ); + return Err(ChainStorageError::InvalidOperation( + "Deleting block, new smt root did not match expected smt root".to_string(), + )); + } + self.insert_tip_smt(write_txn, &smt)?; self.delete_block_kernels(write_txn, block_hash.as_slice())?; From 585374db618411cb7f4986f5228a68018607bdca Mon Sep 17 00:00:00 2001 From: SW van Heerden Date: Thu, 29 Feb 2024 11:28:41 +0200 Subject: [PATCH 4/5] add smt check to unit test --- .../sync/horizon_state_sync/synchronizer.rs | 7 +- .../src/chain_storage/blockchain_database.rs | 117 +++++++++++++++--- .../core/src/chain_storage/lmdb_db/lmdb_db.rs | 18 ++- .../core/src/test_helpers/blockchain.rs | 86 +++++++++---- base_layer/mmr/src/sparse_merkle_tree/node.rs | 12 +- base_layer/mmr/src/sparse_merkle_tree/tree.rs | 2 +- 6 files changed, 187 insertions(+), 55 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 458622800c..5ff534d3ca 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 @@ -701,9 +701,14 @@ impl<'a, B: BlockchainBackend + 'static> HorizonStateSynchronization<'a, B> { match output_smt.delete(&smt_key)? { DeleteResult::Deleted(_value_hash) => {}, DeleteResult::KeyNotFound => { + error!( + target: LOG_TARGET, + "Could not find input({}) in SMT", + commitment.to_hex(), + ); return Err(HorizonSyncError::ChainStorageError( ChainStorageError::UnspendableInput, - )) + )); }, }; // This will only be committed once the SMT has been verified due to rewind difficulties if diff --git a/base_layer/core/src/chain_storage/blockchain_database.rs b/base_layer/core/src/chain_storage/blockchain_database.rs index f79b278b15..a1a5413341 100644 --- a/base_layer/core/src/chain_storage/blockchain_database.rs +++ b/base_layer/core/src/chain_storage/blockchain_database.rs @@ -1351,7 +1351,14 @@ pub fn calculate_mmr_roots( let smt_key = NodeKey::try_from(input.commitment()?.as_bytes())?; match output_smt.delete(&smt_key)? { DeleteResult::Deleted(_value_hash) => {}, - DeleteResult::KeyNotFound => return Err(ChainStorageError::UnspendableInput), + DeleteResult::KeyNotFound => { + error!( + target: LOG_TARGET, + "Could not find input({}) in SMT", + input.commitment()?.to_hex(), + ); + return Err(ChainStorageError::UnspendableInput); + }, }; } @@ -2515,6 +2522,8 @@ mod test { create_new_blockchain, create_orphan_chain, create_test_blockchain_db, + rewind_smt, + update_block_and_smt, TempDatabase, }, BlockSpecs, @@ -2565,8 +2574,14 @@ mod test { .try_into_chain_block() .map(Arc::new) .unwrap(); - let (_, chain) = - create_orphan_chain(&db, &[("A->GB", 1, 120), ("B->A", 1, 120), ("C->B", 1, 120)], genesis).await; + let mut smt = db.fetch_tip_smt().unwrap(); + let (_, chain) = create_orphan_chain( + &db, + &[("A->GB", 1, 120), ("B->A", 1, 120), ("C->B", 1, 120)], + genesis, + &mut smt, + ) + .await; let access = db.db_read_access().unwrap(); let orphan_chain = get_orphan_link_main_chain(&*access, chain.get("C").unwrap().hash()).unwrap(); assert_eq!(orphan_chain[2].hash(), chain.get("C").unwrap().hash()); @@ -2587,6 +2602,11 @@ mod test { ]) .await; // Create reorg chain + let mut smt = db.fetch_tip_smt().unwrap(); + let d_block = mainchain.get("D").unwrap().clone(); + rewind_smt(d_block, &mut smt); + let c_block = mainchain.get("C").unwrap().clone(); + rewind_smt(c_block, &mut smt); let fork_root = mainchain.get("B").unwrap().clone(); let (_, reorg_chain) = create_orphan_chain( &db, @@ -2597,6 +2617,7 @@ mod test { ("F2->E2", 1, 120), ], fork_root, + &mut smt, ) .await; let access = db.db_read_access().unwrap(); @@ -2631,7 +2652,8 @@ mod test { .try_into_chain_block() .map(Arc::new) .unwrap(); - let (_, chain) = create_chained_blocks(&[("A->GB", 1u64, 120u64)], genesis_block).await; + let mut smt = db.fetch_tip_smt().unwrap(); + let (_, chain) = create_chained_blocks(&[("A->GB", 1u64, 120u64)], genesis_block, &mut smt).await; let block = chain.get("A").unwrap().clone(); let mut access = db.db_write_access().unwrap(); insert_orphan_and_find_new_tips(&mut *access, block.to_arc_block(), &validator, &db.consensus_manager) @@ -2648,8 +2670,13 @@ mod test { let (_, main_chain) = create_main_chain(&db, &[("A->GB", 1, 120), ("B->A", 1, 120)]).await; let block_b = main_chain.get("B").unwrap().clone(); - let (_, orphan_chain) = - create_chained_blocks(&[("C2->GB", 1, 120), ("D2->C2", 1, 120), ("E2->D2", 1, 120)], block_b).await; + let mut smt = db.fetch_tip_smt().unwrap(); + let (_, orphan_chain) = create_chained_blocks( + &[("C2->GB", 1, 120), ("D2->C2", 1, 120), ("E2->D2", 1, 120)], + block_b, + &mut smt, + ) + .await; let mut access = db.db_write_access().unwrap(); let block_d2 = orphan_chain.get("D2").unwrap().clone(); @@ -2671,7 +2698,8 @@ mod test { let (_, main_chain) = create_main_chain(&db, &[("A->GB", 1, 120)]).await; let fork_root = main_chain.get("A").unwrap().clone(); - let (_, orphan_chain) = create_chained_blocks(&[("B2->GB", 1, 120)], fork_root).await; + let mut smt = db.fetch_tip_smt().unwrap(); + let (_, orphan_chain) = create_chained_blocks(&[("B2->GB", 1, 120)], fork_root, &mut smt).await; let mut access = db.db_write_access().unwrap(); let block = orphan_chain.get("B2").unwrap().clone(); @@ -2694,6 +2722,7 @@ mod test { #[tokio::test] async fn it_correctly_detects_strongest_orphan_tips() { let db = create_new_blockchain(); + let mut gen_smt = db.fetch_tip_smt().unwrap(); let validator = MockValidator::new(true); let (_, main_chain) = create_main_chain(&db, &[ ("A->GB", 1, 120), @@ -2708,19 +2737,35 @@ mod test { // Fork 1 (with 3 blocks) let fork_root_1 = main_chain.get("A").unwrap().clone(); + let mut smt = db.fetch_tip_smt().unwrap(); + let g_block = main_chain.get("G").unwrap().clone(); + rewind_smt(g_block, &mut smt); + let f_block = main_chain.get("F").unwrap().clone(); + rewind_smt(f_block, &mut smt); + let e_block = main_chain.get("E").unwrap().clone(); + rewind_smt(e_block, &mut smt); + let d_block = main_chain.get("D").unwrap().clone(); + rewind_smt(d_block, &mut smt); + let c_block = main_chain.get("C").unwrap().clone(); + rewind_smt(c_block, &mut smt); + let mut c_smt = smt.clone(); + let b_block = main_chain.get("B").unwrap().clone(); + rewind_smt(b_block, &mut smt); + let (_, orphan_chain_1) = create_chained_blocks( &[("B2->GB", 1, 120), ("C2->B2", 1, 120), ("D2->C2", 1, 120)], fork_root_1, + &mut smt, ) .await; // Fork 2 (with 1 block) let fork_root_2 = main_chain.get("GB").unwrap().clone(); - let (_, orphan_chain_2) = create_chained_blocks(&[("B3->GB", 1, 120)], fork_root_2).await; + let (_, orphan_chain_2) = create_chained_blocks(&[("B3->GB", 1, 120)], fork_root_2, &mut gen_smt).await; // Fork 3 (with 1 block) let fork_root_3 = main_chain.get("B").unwrap().clone(); - let (_, orphan_chain_3) = create_chained_blocks(&[("B4->GB", 1, 120)], fork_root_3).await; + let (_, orphan_chain_3) = create_chained_blocks(&[("B4->GB", 1, 120)], fork_root_3, &mut c_smt).await; // Add blocks to db let mut access = db.db_write_access().unwrap(); @@ -2785,25 +2830,35 @@ mod test { } mod handle_possible_reorg { - use super::*; + use crate::test_helpers::blockchain::update_block_and_smt; #[ignore] #[tokio::test] async fn it_links_many_orphan_branches_to_main_chain() { let test = TestHarness::setup(); - + let mut smt = test.db.fetch_tip_smt().unwrap(); let (_, main_chain) = create_main_chain(&test.db, block_specs!(["1a->GB"], ["2a->1a"], ["3a->2a"], ["4a->3a"])).await; let genesis = main_chain.get("GB").unwrap().clone(); let fork_root = main_chain.get("1a").unwrap().clone(); + let mut a1_block = fork_root.block().clone(); + update_block_and_smt(&mut a1_block, &mut smt); let (_, orphan_chain_b) = create_chained_blocks( block_specs!(["2b->GB"], ["3b->2b"], ["4b->3b"], ["5b->4b"], ["6b->5b"]), fork_root, + &mut smt, ) .await; + let b6_block = main_chain.get("6b").unwrap().clone(); + rewind_smt(b6_block, &mut smt); + let b5_block = main_chain.get("5b").unwrap().clone(); + rewind_smt(b5_block, &mut smt); + let b4_block = main_chain.get("4b").unwrap().clone(); + rewind_smt(b4_block, &mut smt); + // Add orphans out of height order for name in ["5b", "3b", "4b", "6b"] { let block = orphan_chain_b.get(name).unwrap(); @@ -2813,9 +2868,15 @@ mod test { // Add chain c orphans branching from chain b let fork_root = orphan_chain_b.get("3b").unwrap().clone(); - let (_, orphan_chain_c) = - create_chained_blocks(block_specs!(["4c->GB"], ["5c->4c"], ["6c->5c"], ["7c->6c"]), fork_root).await; + let (_, orphan_chain_c) = create_chained_blocks( + block_specs!(["4c->GB"], ["5c->4c"], ["6c->5c"], ["7c->6c"]), + fork_root, + &mut smt, + ) + .await; + let c7_block = main_chain.get("7c").unwrap().clone(); + rewind_smt(c7_block, &mut smt); for name in ["7c", "5c", "6c", "4c"] { let block = orphan_chain_c.get(name).unwrap(); let result = test.handle_possible_reorg(block.to_arc_block()).unwrap(); @@ -2826,6 +2887,7 @@ mod test { let (_, orphan_chain_d) = create_chained_blocks( block_specs!(["7d->GB", difficulty: Difficulty::from_u64(10).unwrap()]), fork_root, + &mut smt, ) .await; @@ -2880,7 +2942,7 @@ mod test { let test = TestHarness::setup(); // This test assumes a MTC of 11 assert_eq!(test.consensus.consensus_constants(0).median_timestamp_count(), 11); - + let mut smt = test.db.fetch_tip_smt().unwrap(); let (_, main_chain) = create_main_chain( &test.db, block_specs!( @@ -2901,8 +2963,9 @@ mod test { ) .await; let genesis = main_chain.get("GB").unwrap().clone(); - let fork_root = main_chain.get("1a").unwrap().clone(); + let mut a1_block = fork_root.block().clone(); + update_block_and_smt(&mut a1_block, &mut smt); let (_, orphan_chain_b) = create_chained_blocks( block_specs!( ["2b->GB"], @@ -2918,6 +2981,7 @@ mod test { ["12b->11b", difficulty: Difficulty::from_u64(5).unwrap()] ), fork_root, + &mut smt, ) .await; @@ -2970,13 +3034,17 @@ mod test { #[tokio::test] async fn it_errors_if_reorging_to_an_invalid_height() { let test = TestHarness::setup(); + let mut smt = test.db.fetch_tip_smt().unwrap(); let (_, main_chain) = create_main_chain(&test.db, block_specs!(["1a->GB"], ["2a->1a"], ["3a->2a"], ["4a->3a"])).await; let fork_root = main_chain.get("1a").unwrap().clone(); + let mut a1_block = fork_root.block().clone(); + update_block_and_smt(&mut a1_block, &mut smt); let (_, orphan_chain_b) = create_chained_blocks( block_specs!(["2b->GB", height: 10, difficulty: Difficulty::from_u64(10).unwrap()]), fork_root, + &mut smt, ) .await; @@ -2988,6 +3056,7 @@ mod test { #[tokio::test] async fn it_allows_orphan_blocks_with_any_height() { let test = TestHarness::setup(); + let mut smt = test.db.fetch_tip_smt().unwrap(); let (_, main_chain) = create_main_chain( &test.db, block_specs!(["1a->GB", difficulty: Difficulty::from_u64(2).unwrap()]), @@ -2996,7 +3065,7 @@ mod test { let fork_root = main_chain.get("GB").unwrap().clone(); let (_, orphan_chain_b) = - create_orphan_chain(&test.db, block_specs!(["1b->GB", height: 10]), fork_root).await; + create_orphan_chain(&test.db, block_specs!(["1b->GB", height: 10]), fork_root, &mut smt).await; let block = orphan_chain_b.get("1b").unwrap().clone(); test.handle_possible_reorg(block.to_arc_block()) @@ -3105,6 +3174,7 @@ mod test { #[tokio::test] async fn test_handle_possible_reorg_case6_orphan_chain_link() { let db = create_new_blockchain(); + let mut smt = db.fetch_tip_smt().unwrap(); let (_, mainchain) = create_main_chain(&db, &[ ("A->GB", 1, 120), ("B->A", 1, 120), @@ -3116,10 +3186,15 @@ mod test { let mock_validator = MockValidator::new(true); let chain_strength_comparer = strongest_chain().by_sha3x_difficulty().build(); + let mut a_block = mainchain.get("A").unwrap().block().clone(); let fork_block = mainchain.get("B").unwrap().clone(); + let mut b_block = fork_block.block().clone(); + update_block_and_smt(&mut a_block, &mut smt); + update_block_and_smt(&mut b_block, &mut smt); let (_, reorg_chain) = create_chained_blocks( &[("C2->GB", 1, 120), ("D2->C2", 1, 120), ("E2->D2", 1, 120)], fork_block, + &mut smt, ) .await; @@ -3195,9 +3270,12 @@ mod test { let mock_validator = MockValidator::new(true); let chain_strength_comparer = strongest_chain().by_sha3x_difficulty().build(); - + let mut smt = db.fetch_tip_smt().unwrap(); + let d_block = mainchain.get("D").unwrap().clone(); + rewind_smt(d_block, &mut smt); let fork_block = mainchain.get("C").unwrap().clone(); - let (_, reorg_chain) = create_chained_blocks(&[("D2->GB", 1, 120), ("E2->D2", 2, 120)], fork_block).await; + let (_, reorg_chain) = + create_chained_blocks(&[("D2->GB", 1, 120), ("E2->D2", 2, 120)], fork_block, &mut smt).await; // Add true orphans let mut access = db.db_write_access().unwrap(); @@ -3478,7 +3556,8 @@ mod test { .try_into_chain_block() .map(Arc::new) .unwrap(); - let (block_names, chain) = create_chained_blocks(blocks, genesis_block).await; + let mut smt = test.db.fetch_tip_smt().unwrap(); + let (block_names, chain) = create_chained_blocks(blocks, genesis_block, &mut smt).await; let mut results = vec![]; for name in block_names { 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 c7d4161d92..2d468d4d93 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 @@ -959,7 +959,14 @@ impl LMDBDatabase { let smt_key = NodeKey::try_from(utxo.output.commitment.as_bytes())?; match output_smt.delete(&smt_key)? { DeleteResult::Deleted(_value_hash) => {}, - DeleteResult::KeyNotFound => return Err(ChainStorageError::UnspendableInput), + DeleteResult::KeyNotFound => { + error!( + target: LOG_TARGET, + "Could not find input({}) in SMT", + utxo.output.commitment.to_hex(), + ); + return Err(ChainStorageError::UnspendableInput); + }, }; lmdb_delete( txn, @@ -1279,7 +1286,14 @@ impl LMDBDatabase { let smt_key = NodeKey::try_from(input_with_output_data.commitment()?.as_bytes())?; match output_smt.delete(&smt_key)? { DeleteResult::Deleted(_value_hash) => {}, - DeleteResult::KeyNotFound => return Err(ChainStorageError::UnspendableInput), + DeleteResult::KeyNotFound => { + error!( + target: LOG_TARGET, + "Could not find input({}) in SMT", + input_with_output_data.commitment()?.to_hex(), + ); + return Err(ChainStorageError::UnspendableInput); + }, }; let features = input_with_output_data.features()?; diff --git a/base_layer/core/src/test_helpers/blockchain.rs b/base_layer/core/src/test_helpers/blockchain.rs index 87338619ea..47a7d67d1c 100644 --- a/base_layer/core/src/test_helpers/blockchain.rs +++ b/base_layer/core/src/test_helpers/blockchain.rs @@ -19,9 +19,9 @@ // SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, // WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE // USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. - use std::{ collections::HashMap, + convert::TryFrom, fs, ops::Deref, path::{Path, PathBuf}, @@ -32,10 +32,12 @@ use tari_common::configuration::Network; use tari_common_types::{ chain_metadata::ChainMetadata, tari_address::TariAddress, - types::{Commitment, HashOutput, PublicKey, Signature}, + types::{Commitment, FixedHash, HashOutput, PublicKey, Signature}, }; +use tari_mmr::sparse_merkle_tree::{DeleteResult, NodeKey, ValueHash}; use tari_storage::lmdb_store::LMDBConfig; use tari_test_utils::paths::create_temporary_data_path; +use tari_utilities::ByteArray; use super::{create_block, mine_to_difficulty}; use crate::{ @@ -425,6 +427,7 @@ impl BlockchainBackend for TempDatabase { pub async fn create_chained_blocks>( blocks: T, genesis_block: Arc, + output_smt: &mut OutputSmt, ) -> (Vec, HashMap>) { let mut block_hashes = HashMap::new(); block_hashes.insert("GB".to_string(), genesis_block); @@ -439,7 +442,7 @@ pub async fn create_chained_blocks>( .unwrap_or_else(|| panic!("Could not find block {}", block_spec.parent)); let name = block_spec.name; let difficulty = block_spec.difficulty; - let (block, _) = create_block( + let (mut block, _) = create_block( &rules, prev_block.block(), block_spec, @@ -449,6 +452,7 @@ pub async fn create_chained_blocks>( None, ) .await; + update_block_and_smt(&mut block, output_smt); let block = mine_block(block, prev_block.accumulated_data(), difficulty); block_names.push(name.to_string()); block_hashes.insert(name.to_string(), block); @@ -479,7 +483,8 @@ pub async fn create_main_chain>( .try_into_chain_block() .map(Arc::new) .unwrap(); - let (names, chain) = create_chained_blocks(blocks, genesis_block).await; + let mut smt = db.fetch_tip_smt().unwrap(); + let (names, chain) = create_chained_blocks(blocks, genesis_block, &mut smt).await; names.iter().for_each(|name| { let block = chain.get(name).unwrap(); db.add_block(block.to_arc_block()).unwrap(); @@ -488,12 +493,28 @@ pub async fn create_main_chain>( (names, chain) } +pub fn rewind_smt(block: Arc, smt: &mut OutputSmt) { + for input in block.block().body.inputs() { + let smt_key = NodeKey::try_from(input.commitment().unwrap().as_bytes()).unwrap(); + let smt_node = ValueHash::try_from(input.smt_hash(block.header().height).as_slice()).unwrap(); + smt.insert(smt_key, smt_node).unwrap(); + } + for output in block.block().body.outputs() { + let smt_key = NodeKey::try_from(output.commitment.as_bytes()).unwrap(); + match smt.delete(&smt_key).unwrap() { + DeleteResult::Deleted(_value_hash) => {}, + DeleteResult::KeyNotFound => panic!("key should be found"), + }; + } +} + pub async fn create_orphan_chain>( db: &BlockchainDatabase, blocks: T, root_block: Arc, + smt: &mut OutputSmt, ) -> (Vec, HashMap>) { - let (names, chain) = create_chained_blocks(blocks, root_block).await; + let (names, chain) = create_chained_blocks(blocks, root_block, smt).await; let mut txn = DbTransaction::new(); for name in &names { let block = chain.get(name).unwrap().clone(); @@ -504,9 +525,24 @@ pub async fn create_orphan_chain>( (names, chain) } +pub fn update_block_and_smt(block: &mut Block, smt: &mut OutputSmt) { + for output in block.body.outputs() { + let smt_key = NodeKey::try_from(output.commitment.as_bytes()).unwrap(); + let smt_node = ValueHash::try_from(output.smt_hash(block.header.height).as_slice()).unwrap(); + // suppress this error as some unit tests rely on this not being completely correct. + let _result = smt.insert(smt_key, smt_node); + } + for input in block.body.inputs() { + let smt_key = NodeKey::try_from(input.commitment().unwrap().as_bytes()).unwrap(); + smt.delete(&smt_key).unwrap(); + } + let root = FixedHash::try_from(smt.hash().as_slice()).unwrap(); + block.header.output_mr = root; +} + pub struct TestBlockchain { db: BlockchainDatabase, - chain: Vec<(&'static str, Arc)>, + chain: Vec<(&'static str, Arc, OutputSmt)>, rules: ConsensusManager, pub km: MemoryDbKeyManager, script_key_id: TariKeyId, @@ -533,8 +569,9 @@ impl TestBlockchain { wallet_payment_address, range_proof_type: RangeProofType::BulletProofPlus, }; + let smt = blockchain.db.fetch_tip_smt().unwrap(); - blockchain.chain.push(("GB", genesis)); + blockchain.chain.push(("GB", genesis, smt)); blockchain } @@ -611,25 +648,29 @@ impl TestBlockchain { block: Arc, ) -> Result { let result = self.db.add_block(block.to_arc_block())?; - self.chain.push((name, block)); + let smt = self.db.fetch_tip_smt().unwrap(); + self.chain.push((name, block, smt)); Ok(result) } - pub fn get_block_by_name(&self, name: &'static str) -> Option> { - self.chain.iter().find(|(n, _)| *n == name).map(|(_, ch)| ch.clone()) + pub fn get_block_and_smt_by_name(&self, name: &'static str) -> Option<(Arc, OutputSmt)> { + self.chain + .iter() + .find(|(n, _, _)| *n == name) + .map(|(_, ch, smt)| (ch.clone(), smt.clone())) } - pub fn get_tip_block(&self) -> (&'static str, Arc) { + pub fn get_tip_block(&self) -> (&'static str, Arc, OutputSmt) { self.chain.last().cloned().unwrap() } pub async fn create_chained_block(&self, block_spec: BlockSpec) -> (Arc, WalletOutput) { - let parent = self - .get_block_by_name(block_spec.parent) + let (parent, mut parent_smt) = self + .get_block_and_smt_by_name(block_spec.parent) .ok_or_else(|| format!("Parent block not found with name '{}'", block_spec.parent)) .unwrap(); let difficulty = block_spec.difficulty; - let (block, coinbase) = create_block( + let (mut block, coinbase) = create_block( &self.rules, parent.block(), block_spec, @@ -639,13 +680,14 @@ impl TestBlockchain { Some(self.range_proof_type), ) .await; + update_block_and_smt(&mut block, &mut parent_smt); let block = mine_block(block, parent.accumulated_data(), difficulty); (block, coinbase) } pub async fn create_unmined_block(&self, block_spec: BlockSpec) -> (Block, WalletOutput) { - let parent = self - .get_block_by_name(block_spec.parent) + let (parent, mut parent_smt) = self + .get_block_and_smt_by_name(block_spec.parent) .ok_or_else(|| format!("Parent block not found with name '{}'", block_spec.parent)) .unwrap(); let (mut block, outputs) = create_block( @@ -658,17 +700,19 @@ impl TestBlockchain { Some(self.range_proof_type), ) .await; + update_block_and_smt(&mut block, &mut parent_smt); block.body.sort(); (block, outputs) } - pub fn mine_block(&self, parent_name: &'static str, block: Block, difficulty: Difficulty) -> Arc { - let parent = self.get_block_by_name(parent_name).unwrap(); + pub fn mine_block(&self, parent_name: &'static str, mut block: Block, difficulty: Difficulty) -> Arc { + let (parent, mut parent_smt) = self.get_block_and_smt_by_name(parent_name).unwrap(); + update_block_and_smt(&mut block, &mut parent_smt); mine_block(block, parent.accumulated_data(), difficulty) } pub async fn create_next_tip(&self, spec: BlockSpec) -> (Arc, WalletOutput) { - let (name, _) = self.get_tip_block(); + let (name, _, _) = self.get_tip_block(); self.create_chained_block(spec.with_parent_block(name)).await } @@ -676,7 +720,7 @@ impl TestBlockchain { &mut self, spec: BlockSpec, ) -> Result<(Arc, WalletOutput), ChainStorageError> { - let (tip, _) = self.get_tip_block(); + let (tip, _, _) = self.get_tip_block(); self.append(spec.with_parent_block(tip)).await } @@ -688,6 +732,6 @@ impl TestBlockchain { } pub fn get_genesis_block(&self) -> Arc { - self.chain.first().map(|(_, block)| block).unwrap().clone() + self.chain.first().map(|(_, block, _)| block).unwrap().clone() } } diff --git a/base_layer/mmr/src/sparse_merkle_tree/node.rs b/base_layer/mmr/src/sparse_merkle_tree/node.rs index 1a7467b669..7c9249af8e 100644 --- a/base_layer/mmr/src/sparse_merkle_tree/node.rs +++ b/base_layer/mmr/src/sparse_merkle_tree/node.rs @@ -177,7 +177,7 @@ impl<'a> ExactSizeIterator for PathIterator<'a> { } } -#[derive(Debug, Serialize, Deserialize)] +#[derive(Debug, Serialize, Deserialize, Clone)] #[serde(bound(deserialize = "H:"))] #[serde(bound(serialize = "H:"))] pub enum Node { @@ -186,16 +186,6 @@ pub enum Node { Branch(BranchNode), } -impl Clone for Node { - fn clone(&self) -> Self { - match self { - Empty(n) => Empty(n.clone()), - Leaf(n) => Leaf(n.clone()), - Branch(_) => panic!("Branch nodes cannot be cloned"), - } - } -} - impl Node { /// A non-mutable version of [`Node::hash`], which you can use if you _absolutely know_ that the hash is correct. /// This would be the case for Empty or Leaf nodes, but you should never call this if the node might be a branch diff --git a/base_layer/mmr/src/sparse_merkle_tree/tree.rs b/base_layer/mmr/src/sparse_merkle_tree/tree.rs index 922b4396b4..73366350f3 100644 --- a/base_layer/mmr/src/sparse_merkle_tree/tree.rs +++ b/base_layer/mmr/src/sparse_merkle_tree/tree.rs @@ -31,7 +31,7 @@ pub enum DeleteResult { KeyNotFound, } -#[derive(Debug, Serialize, Deserialize)] +#[derive(Debug, Serialize, Deserialize, Clone)] #[serde(bound(deserialize = "H:"))] #[serde(bound(serialize = "H:"))] pub struct SparseMerkleTree { From eb88735d72eb190f4da97bb64bd46dacb652648b Mon Sep 17 00:00:00 2001 From: SW van Heerden Date: Thu, 29 Feb 2024 15:02:41 +0200 Subject: [PATCH 5/5] fix bug in rewind --- base_layer/core/src/chain_storage/lmdb_db/lmdb_db.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 2d468d4d93..a4d9806b01 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 @@ -1018,7 +1018,7 @@ impl LMDBDatabase { utxo_mined_info.output.minimum_value_promise, ); let smt_key = NodeKey::try_from(input.commitment()?.as_bytes())?; - let smt_node = ValueHash::try_from(input.smt_hash(row.spent_height).as_slice())?; + let smt_node = ValueHash::try_from(input.smt_hash(utxo_mined_info.mined_height).as_slice())?; if let Err(e) = output_smt.insert(smt_key, smt_node) { error!( target: LOG_TARGET,