Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: rewind bug causing SMT to be broken #6172

Merged
merged 5 commits into from
Mar 1, 2024
Merged

fix: rewind bug causing SMT to be broken #6172

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
7107df2
update logs
SWvheerden Feb 27, 2024
f2b35f0
more changes
SWvheerden Feb 27, 2024
a1209e0
hard stop checking mmr on rewind
SWvheerden Feb 28, 2024
585374d
add smt check to unit test
SWvheerden Feb 29, 2024
eb88735
fix bug in rewind
SWvheerden Feb 29, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 22 additions & 3 deletions base_layer/core/src/base_node/sync/horizon_state_sync/synchronizer.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
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 => {
SWvheerden marked this conversation as resolved.
Show resolved Hide resolved
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
View file
Open in desktop
Original file line number Diff line number Diff line change
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
Loading
Loading