From f9f5a41143ac919a07603a377beec5cb571ed58c Mon Sep 17 00:00:00 2001 From: Colin LeMahieu Date: Wed, 3 Apr 2024 11:05:46 +0200 Subject: [PATCH] Fix pruning tests that did not confirm blocks before pruning. Strongly ensure blocks are confirmed while pruning. node::collect_ledger_pruning_targets already ensures blocks are confirmed before pruning however this was not the case in tests. Removed one test that only tested erroneous behavior. --- nano/core_test/bootstrap.cpp | 1 + nano/core_test/ledger.cpp | 55 +++++-------------------------- nano/core_test/ledger_confirm.cpp | 1 + nano/qt_test/qt.cpp | 4 +++ nano/secure/ledger.cpp | 1 + 5 files changed, 15 insertions(+), 47 deletions(-) diff --git a/nano/core_test/bootstrap.cpp b/nano/core_test/bootstrap.cpp index 97105c3316..30b29e48d6 100644 --- a/nano/core_test/bootstrap.cpp +++ b/nano/core_test/bootstrap.cpp @@ -652,6 +652,7 @@ TEST (bootstrap_processor, push_diamond_pruning) { auto transaction (node1->store.tx_begin_write ()); + node1->ledger.confirm (transaction, open->hash ()); ASSERT_EQ (1, node1->ledger.pruning_action (transaction, send1->hash (), 2)); ASSERT_EQ (1, node1->ledger.pruning_action (transaction, open->hash (), 1)); ASSERT_TRUE (node1->ledger.block_exists (transaction, nano::dev::genesis->hash ())); diff --git a/nano/core_test/ledger.cpp b/nano/core_test/ledger.cpp index aa9b6e54d0..35d1f419d5 100644 --- a/nano/core_test/ledger.cpp +++ b/nano/core_test/ledger.cpp @@ -4900,6 +4900,7 @@ TEST (ledger, pruning_action) ASSERT_EQ (nano::block_status::progress, ledger.process (transaction, send2)); ASSERT_TRUE (store->block.exists (transaction, send2->hash ())); // Pruning action + ledger.confirm (transaction, send1->hash ()); ASSERT_EQ (1, ledger.pruning_action (transaction, send1->hash (), 1)); ASSERT_EQ (0, ledger.pruning_action (transaction, nano::dev::genesis->hash (), 1)); ASSERT_TRUE (ledger.pending_info (transaction, nano::pending_key{ nano::dev::genesis_key.pub, send1->hash () })); @@ -4935,6 +4936,7 @@ TEST (ledger, pruning_action) ASSERT_FALSE (receive1_stored->sideband ().details.is_epoch); // Middle block pruning ASSERT_TRUE (store->block.exists (transaction, send2->hash ())); + ledger.confirm (transaction, send2->hash ()); ASSERT_EQ (1, ledger.pruning_action (transaction, send2->hash (), 1)); ASSERT_TRUE (store->pruned.exists (transaction, send2->hash ())); ASSERT_FALSE (store->block.exists (transaction, send2->hash ())); @@ -4987,6 +4989,7 @@ TEST (ledger, pruning_large_chain) } ASSERT_EQ (0, store->pruned.count (transaction)); ASSERT_EQ (send_receive_pairs * 2 + 1, store->block.count (transaction)); + ledger.confirm (transaction, last_hash); // Pruning action ASSERT_EQ (send_receive_pairs * 2, ledger.pruning_action (transaction, last_hash, 5)); ASSERT_TRUE (store->pruned.exists (transaction, last_hash)); @@ -5045,6 +5048,7 @@ TEST (ledger, pruning_source_rollback) .build (); ASSERT_EQ (nano::block_status::progress, ledger.process (transaction, send2)); ASSERT_TRUE (store->block.exists (transaction, send2->hash ())); + ledger.confirm (transaction, send1->hash ()); // Pruning action ASSERT_EQ (2, ledger.pruning_action (transaction, send1->hash (), 1)); ASSERT_FALSE (store->block.exists (transaction, send1->hash ())); @@ -5131,6 +5135,7 @@ TEST (ledger, pruning_source_rollback_legacy) ASSERT_EQ (nano::block_status::progress, ledger.process (transaction, send3)); ASSERT_TRUE (store->block.exists (transaction, send3->hash ())); ASSERT_TRUE (ledger.pending_info (transaction, nano::pending_key{ nano::dev::genesis_key.pub, send3->hash () })); + ledger.confirm (transaction, send2->hash ()); // Pruning action ASSERT_EQ (2, ledger.pruning_action (transaction, send2->hash (), 1)); ASSERT_FALSE (store->block.exists (transaction, send2->hash ())); @@ -5199,53 +5204,6 @@ TEST (ledger, pruning_source_rollback_legacy) ASSERT_EQ (6, ledger.block_count ()); } -TEST (ledger, pruning_process_error) -{ - nano::logger logger; - auto store = nano::make_store (logger, nano::unique_path (), nano::dev::constants); - ASSERT_TRUE (!store->init_error ()); - nano::stats stats; - nano::ledger ledger (*store, stats, nano::dev::constants); - ledger.pruning = true; - auto transaction (store->tx_begin_write ()); - store->initialize (transaction, ledger.cache, ledger.constants); - nano::work_pool pool{ nano::dev::network_params.network, std::numeric_limits::max () }; - nano::block_builder builder; - auto send1 = builder - .state () - .account (nano::dev::genesis_key.pub) - .previous (nano::dev::genesis->hash ()) - .representative (nano::dev::genesis_key.pub) - .balance (nano::dev::constants.genesis_amount - nano::Gxrb_ratio) - .link (nano::dev::genesis_key.pub) - .sign (nano::dev::genesis_key.prv, nano::dev::genesis_key.pub) - .work (*pool.generate (nano::dev::genesis->hash ())) - .build (); - ASSERT_EQ (nano::block_status::progress, ledger.process (transaction, send1)); - ASSERT_EQ (0, ledger.pruned_count ()); - ASSERT_EQ (2, ledger.block_count ()); - // Pruning action for latest block (not valid action) - ASSERT_EQ (1, ledger.pruning_action (transaction, send1->hash (), 1)); - ASSERT_FALSE (store->block.exists (transaction, send1->hash ())); - ASSERT_TRUE (store->pruned.exists (transaction, send1->hash ())); - // Attempt to process pruned block again - ASSERT_EQ (nano::block_status::old, ledger.process (transaction, send1)); - // Attept to process new block after pruned - auto send2 = builder - .state () - .account (nano::dev::genesis_key.pub) - .previous (send1->hash ()) - .representative (nano::dev::genesis_key.pub) - .balance (nano::dev::constants.genesis_amount - nano::Gxrb_ratio * 2) - .link (nano::dev::genesis_key.pub) - .sign (nano::dev::genesis_key.prv, nano::dev::genesis_key.pub) - .work (*pool.generate (send1->hash ())) - .build (); - ASSERT_EQ (nano::block_status::gap_previous, ledger.process (transaction, send2)); - ASSERT_EQ (1, ledger.pruned_count ()); - ASSERT_EQ (2, ledger.block_count ()); -} - TEST (ledger, pruning_legacy_blocks) { nano::logger logger; @@ -5312,6 +5270,7 @@ TEST (ledger, pruning_legacy_blocks) .work (*pool.generate (open1->hash ())) .build (); ASSERT_EQ (nano::block_status::progress, ledger.process (transaction, send3)); + ledger.confirm (transaction, open1->hash ()); // Pruning action ASSERT_EQ (3, ledger.pruning_action (transaction, change1->hash (), 2)); ASSERT_EQ (1, ledger.pruning_action (transaction, open1->hash (), 1)); @@ -5368,6 +5327,7 @@ TEST (ledger, pruning_safe_functions) .build (); ASSERT_EQ (nano::block_status::progress, ledger.process (transaction, send2)); ASSERT_TRUE (store->block.exists (transaction, send2->hash ())); + ledger.confirm (transaction, send1->hash ()); // Pruning action ASSERT_EQ (1, ledger.pruning_action (transaction, send1->hash (), 1)); ASSERT_FALSE (store->block.exists (transaction, send1->hash ())); @@ -5419,6 +5379,7 @@ TEST (ledger, hash_root_random) .build (); ASSERT_EQ (nano::block_status::progress, ledger.process (transaction, send2)); ASSERT_TRUE (store->block.exists (transaction, send2->hash ())); + ledger.confirm (transaction, send1->hash ()); // Pruning action ASSERT_EQ (1, ledger.pruning_action (transaction, send1->hash (), 1)); ASSERT_FALSE (store->block.exists (transaction, send1->hash ())); diff --git a/nano/core_test/ledger_confirm.cpp b/nano/core_test/ledger_confirm.cpp index ee3a4c1e86..dc6e55c37a 100644 --- a/nano/core_test/ledger_confirm.cpp +++ b/nano/core_test/ledger_confirm.cpp @@ -845,6 +845,7 @@ TEST (ledger_confirm, pruned_source) ASSERT_EQ (nano::block_status::progress, ledger.process (transaction, send2)); ASSERT_EQ (nano::block_status::progress, ledger.process (transaction, send3)); ASSERT_EQ (nano::block_status::progress, ledger.process (transaction, open2)); + ledger.confirm (transaction, send2->hash ()); ASSERT_EQ (2, ledger.pruning_action (transaction, send2->hash (), 2)); ASSERT_FALSE (ledger.block_exists (transaction, send2->hash ())); ASSERT_FALSE (ledger.block_confirmed (transaction, open2->hash ())); diff --git a/nano/qt_test/qt.cpp b/nano/qt_test/qt.cpp index d6758ff883..16f7306cbb 100644 --- a/nano/qt_test/qt.cpp +++ b/nano/qt_test/qt.cpp @@ -581,6 +581,7 @@ TEST (history, pruned_source) ASSERT_EQ (nano::block_status::progress, ledger.process (transaction, receive)); auto open = std::make_shared (send2->hash (), key.pub, key.pub, key.prv, key.pub, *system.work.generate (key.pub)); ASSERT_EQ (nano::block_status::progress, ledger.process (transaction, open)); + ledger.confirm (transaction, send1->hash ()); ASSERT_EQ (1, ledger.pruning_action (transaction, send1->hash (), 2)); next_pruning = send2->hash (); } @@ -593,6 +594,7 @@ TEST (history, pruned_source) // Additional legacy test { auto transaction (store->tx_begin_write ()); + ledger.confirm (transaction, next_pruning); ASSERT_EQ (1, ledger.pruning_action (transaction, next_pruning, 2)); } history1.refresh (); @@ -608,7 +610,9 @@ TEST (history, pruned_source) auto latest_key (ledger.latest (transaction, key.pub)); auto receive = std::make_shared (key.pub, latest_key, key.pub, 200, send->hash (), key.prv, key.pub, *system.work.generate (latest_key)); ASSERT_EQ (nano::block_status::progress, ledger.process (transaction, receive)); + ledger.confirm (transaction, latest); ASSERT_EQ (1, ledger.pruning_action (transaction, latest, 2)); + ledger.confirm (transaction, latest_key); ASSERT_EQ (1, ledger.pruning_action (transaction, latest_key, 2)); } history1.refresh (); diff --git a/nano/secure/ledger.cpp b/nano/secure/ledger.cpp index c6cb0f4ad9..ab13ca171c 100644 --- a/nano/secure/ledger.cpp +++ b/nano/secure/ledger.cpp @@ -1346,6 +1346,7 @@ uint64_t nano::ledger::pruning_action (store::write_transaction & transaction_a, auto block_l = block (transaction_a, hash); if (block_l != nullptr) { + release_assert (block_confirmed (transaction_a, hash)); store.block.del (transaction_a, hash); store.pruned.put (transaction_a, hash); hash = block_l->previous ();