Skip to content

Commit

Permalink
fix: rewind bug causing SMT to be broken (#6172)
Browse files Browse the repository at this point in the history
Description
---
Fixes rewind bug in the SMT
Makes SMT instructions more strict. 
Updates logs to give a clearer indication as to what happens
  • Loading branch information
SWvheerden committed Mar 1, 2024
1 parent abd64d8 commit 4cb61a3
Show file tree
Hide file tree
Showing 8 changed files with 249 additions and 65 deletions.
Expand Up @@ -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;

Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -691,7 +698,19 @@ 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 => {
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
// we need to abort the sync
inputs_to_delete.push((output_hash, commitment));
Expand Down
128 changes: 106 additions & 22 deletions base_layer/core/src/chain_storage/blockchain_database.rs
Expand Up @@ -1334,19 +1334,31 @@ pub fn calculate_mmr_roots<T: BlockchainBackend>(
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());
}
}
}

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) => {},
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);
},
};
}

Expand Down Expand Up @@ -2510,6 +2522,8 @@ mod test {
create_new_blockchain,
create_orphan_chain,
create_test_blockchain_db,
rewind_smt,
update_block_and_smt,
TempDatabase,
},
BlockSpecs,
Expand Down Expand Up @@ -2560,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());
Expand All @@ -2582,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,
Expand All @@ -2592,6 +2617,7 @@ mod test {
("F2->E2", 1, 120),
],
fork_root,
&mut smt,
)
.await;
let access = db.db_read_access().unwrap();
Expand Down Expand Up @@ -2626,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)
Expand All @@ -2643,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();
Expand All @@ -2666,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();
Expand All @@ -2689,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),
Expand All @@ -2703,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();
Expand Down Expand Up @@ -2780,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();
Expand All @@ -2808,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();
Expand All @@ -2821,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;

Expand Down Expand Up @@ -2875,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!(
Expand All @@ -2896,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"],
Expand All @@ -2913,6 +2981,7 @@ mod test {
["12b->11b", difficulty: Difficulty::from_u64(5).unwrap()]
),
fork_root,
&mut smt,
)
.await;

Expand Down Expand Up @@ -2965,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;

Expand All @@ -2983,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()]),
Expand All @@ -2991,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())
Expand Down Expand Up @@ -3100,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),
Expand All @@ -3111,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;

Expand Down Expand Up @@ -3190,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();
Expand Down Expand Up @@ -3473,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 {
Expand Down

0 comments on commit 4cb61a3

Please sign in to comment.