diff --git a/nano/core_test/confirmation_height.cpp b/nano/core_test/confirmation_height.cpp index 7d0f21865c..16df1c21a0 100644 --- a/nano/core_test/confirmation_height.cpp +++ b/nano/core_test/confirmation_height.cpp @@ -730,6 +730,46 @@ TEST (confirmation_height, conflict_rollback_cemented) test_mode (nano::confirmation_height_mode::unbounded); } +TEST (confirmation_heightDeathTest, rollback_added_block) +{ + // For ASSERT_DEATH_IF_SUPPORTED + testing::FLAGS_gtest_death_test_style = "threadsafe"; + + // valgrind can be noisy with death tests + if (!nano::running_within_valgrind ()) + { + nano::logger_mt logger; + auto path (nano::unique_path ()); + nano::mdb_store store (logger, path); + ASSERT_TRUE (!store.init_error ()); + nano::genesis genesis; + nano::stat stats; + nano::ledger ledger (store, stats); + nano::write_database_queue write_database_queue; + nano::work_pool pool (std::numeric_limits::max ()); + nano::keypair key1; + auto send = std::make_shared (genesis.hash (), key1.pub, nano::genesis_amount - nano::Gxrb_ratio, nano::test_genesis_key.prv, nano::test_genesis_key.pub, *pool.generate (genesis.hash ())); + { + auto transaction (store.tx_begin_write ()); + store.initialize (transaction, genesis, ledger.cache); + } + + auto block_hash_being_processed (send->hash ()); + uint64_t batch_write_size = 2048; + std::atomic stopped{ false }; + nano::confirmation_height_unbounded unbounded_processor ( + ledger, write_database_queue, 10ms, logger, stopped, block_hash_being_processed, batch_write_size, [](auto const &) {}, [](auto const &) {}, []() { return 0; }); + + // Processing a block which doesn't exist should bail + ASSERT_DEATH_IF_SUPPORTED (unbounded_processor.process (), ""); + + nano::confirmation_height_bounded bounded_processor ( + ledger, write_database_queue, 10ms, logger, stopped, block_hash_being_processed, batch_write_size, [](auto const &) {}, [](auto const &) {}, []() { return 0; }); + // Processing a block which doesn't exist should bail + ASSERT_DEATH_IF_SUPPORTED (bounded_processor.process (), ""); + } +} + TEST (confirmation_height, observers) { auto test_mode = [](nano::confirmation_height_mode mode_a) { @@ -766,78 +806,143 @@ TEST (confirmation_height, observers) } // This tests when a read has been done, but the block no longer exists by the time a write is done -TEST (confirmation_height, modified_chain) +TEST (confirmation_heightDeathTest, modified_chain) { - auto test_mode = [](nano::confirmation_height_mode mode_a) { - nano::system system; - nano::node_flags node_flags; - node_flags.confirmation_height_processor_mode = mode_a; - nano::node_config node_config (nano::get_available_port (), system.logging); - node_config.frontiers_confirmation = nano::frontiers_confirmation_mode::disabled; - auto node = system.add_node (node_config, node_flags); - - system.wallet (0)->insert_adhoc (nano::test_genesis_key.prv); - nano::block_hash latest (node->latest (nano::test_genesis_key.pub)); - + // For ASSERT_DEATH_IF_SUPPORTED + testing::FLAGS_gtest_death_test_style = "threadsafe"; + + // valgrind can be noisy with death tests + if (!nano::running_within_valgrind ()) + { + nano::logger_mt logger; + auto path (nano::unique_path ()); + nano::mdb_store store (logger, path); + ASSERT_TRUE (!store.init_error ()); + nano::genesis genesis; + nano::stat stats; + nano::ledger ledger (store, stats); + nano::write_database_queue write_database_queue; + boost::latch initialized_latch{ 0 }; + nano::work_pool pool (std::numeric_limits::max ()); nano::keypair key1; - auto & store = node->store; - auto send = std::make_shared (latest, key1.pub, nano::genesis_amount - nano::Gxrb_ratio, nano::test_genesis_key.prv, nano::test_genesis_key.pub, *system.work.generate (latest)); - + auto send = std::make_shared (nano::genesis_hash, key1.pub, nano::genesis_amount - nano::Gxrb_ratio, nano::test_genesis_key.prv, nano::test_genesis_key.pub, *pool.generate (nano::genesis_hash)); { - auto transaction = node->store.tx_begin_write (); - ASSERT_EQ (nano::process_result::progress, node->ledger.process (transaction, *send).code); + auto transaction (store.tx_begin_write ()); + store.initialize (transaction, genesis, ledger.cache); + ASSERT_EQ (nano::process_result::progress, ledger.process (transaction, *send).code); } - node->confirmation_height_processor.add (send->hash ()); + auto block_hash_being_processed (send->hash ()); + uint64_t batch_write_size = 2048; + std::atomic stopped{ false }; + nano::confirmation_height_bounded bounded_processor ( + ledger, write_database_queue, 10ms, logger, stopped, block_hash_being_processed, batch_write_size, [](auto const &) {}, [](auto const &) {}, []() { return 0; }); + { - // The write guard prevents the confirmation height processor doing any writes - system.deadline_set (10s); - auto write_guard = node->write_database_queue.wait (nano::writer::testing); - while (!node->write_database_queue.contains (nano::writer::confirmation_height)) - { - ASSERT_NO_ERROR (system.poll ()); - } + // This reads the blocks in the account, but prevents any writes from occuring yet + auto scoped_write_guard = write_database_queue.wait (nano::writer::testing); + bounded_processor.process (); + } - store.block_del (store.tx_begin_write (), send->hash (), send->type ()); + // Rollback the block and now try to write, the block no longer exists so should bail + ledger.rollback (store.tx_begin_write (), send->hash ()); + { + auto scoped_write_guard = write_database_queue.wait (nano::writer::confirmation_height); + ASSERT_DEATH_IF_SUPPORTED (bounded_processor.cement_blocks (scoped_write_guard), ""); } - system.deadline_set (10s); - while (node->write_database_queue.contains (nano::writer::confirmation_height)) +#ifndef NDEBUG + // Reset conditions and test with the unbounded processor if in debug mode. + // Release config does not check that a chain has been modified prior to setting the confirmation height (as an optimization) + ASSERT_EQ (nano::process_result::progress, ledger.process (store.tx_begin_write (), *send).code); + store.confirmation_height_put (store.tx_begin_write (), nano::genesis_account, { 1, nano::genesis_hash }); + + nano::confirmation_height_unbounded unbounded_processor ( + ledger, write_database_queue, 10ms, logger, stopped, block_hash_being_processed, batch_write_size, [](auto const &) {}, [](auto const &) {}, []() { return 0; }); + { - ASSERT_NO_ERROR (system.poll ()); + // This reads the blocks in the account, but prevents any writes from occuring yet + auto scoped_write_guard = write_database_queue.wait (nano::writer::testing); + unbounded_processor.process (); } - auto check_for_modified_chains = true; - if (mode_a == nano::confirmation_height_mode::unbounded) + // Rollback the block and now try to write, the block no longer exists so should bail + + ledger.rollback (store.tx_begin_write (), send->hash ()); { -#ifdef NDEBUG - // Unbounded processor in release config does not check that a chain has been modified prior to setting the confirmation height (as an optimization) - check_for_modified_chains = false; + auto scoped_write_guard = write_database_queue.wait (nano::writer::confirmation_height); + ASSERT_DEATH_IF_SUPPORTED (unbounded_processor.cement_blocks (scoped_write_guard), ""); + } #endif + } +} + +// This tests when a read has been done, but the account no longer exists by the time a write is done +TEST (confirmation_heightDeathTest, modified_chain_account_removed) +{ + // For ASSERT_DEATH_IF_SUPPORTED + testing::FLAGS_gtest_death_test_style = "threadsafe"; + + // valgrind can be noisy with death tests + if (!nano::running_within_valgrind ()) + { + nano::logger_mt logger; + auto path (nano::unique_path ()); + nano::mdb_store store (logger, path); + ASSERT_TRUE (!store.init_error ()); + nano::genesis genesis; + nano::stat stats; + nano::ledger ledger (store, stats); + nano::write_database_queue write_database_queue; + boost::latch initialized_latch{ 0 }; + nano::work_pool pool (std::numeric_limits::max ()); + nano::keypair key1; + auto send = std::make_shared (nano::genesis_hash, key1.pub, nano::genesis_amount - nano::Gxrb_ratio, nano::test_genesis_key.prv, nano::test_genesis_key.pub, *pool.generate (nano::genesis_hash)); + auto open = std::make_shared (key1.pub, 0, 0, nano::Gxrb_ratio, send->hash (), key1.prv, key1.pub, *pool.generate (key1.pub)); + { + auto transaction (store.tx_begin_write ()); + store.initialize (transaction, genesis, ledger.cache); + ASSERT_EQ (nano::process_result::progress, ledger.process (transaction, *send).code); + ASSERT_EQ (nano::process_result::progress, ledger.process (transaction, *open).code); } - nano::confirmation_height_info confirmation_height_info; - ASSERT_FALSE (node->store.confirmation_height_get (node->store.tx_begin_read (), nano::test_genesis_key.pub, confirmation_height_info)); - if (check_for_modified_chains) + auto block_hash_being_processed (open->hash ()); + uint64_t batch_write_size = 2048; + std::atomic stopped{ false }; + nano::confirmation_height_unbounded unbounded_processor ( + ledger, write_database_queue, 10ms, logger, stopped, block_hash_being_processed, batch_write_size, [](auto const &) {}, [](auto const &) {}, []() { return 0; }); + { - ASSERT_EQ (1, confirmation_height_info.height); - ASSERT_EQ (nano::genesis_hash, confirmation_height_info.frontier); - ASSERT_EQ (1, node->stats.count (nano::stat::type::confirmation_height, nano::stat::detail::invalid_block, nano::stat::dir::in)); - ASSERT_EQ (1, node->ledger.cache.cemented_count); + // This reads the blocks in the account, but prevents any writes from occuring yet + auto scoped_write_guard = write_database_queue.wait (nano::writer::testing); + unbounded_processor.process (); } - else + + // Rollback the block and now try to write, the send should be cemented but the account which the open block belongs no longer exists so should bail + ledger.rollback (store.tx_begin_write (), open->hash ()); { - // A non-existent block is cemented, expected given these conditions but is of course incorrect. - ASSERT_EQ (2, confirmation_height_info.height); - ASSERT_EQ (send->hash (), confirmation_height_info.frontier); - ASSERT_EQ (2, node->ledger.cache.cemented_count); + auto scoped_write_guard = write_database_queue.wait (nano::writer::confirmation_height); + ASSERT_DEATH_IF_SUPPORTED (unbounded_processor.cement_blocks (scoped_write_guard), ""); } - ASSERT_EQ (0, node->active.election_winner_details_size ()); - }; + // Reset conditions and test with the bounded processor + ASSERT_EQ (nano::process_result::progress, ledger.process (store.tx_begin_write (), *open).code); + store.confirmation_height_put (store.tx_begin_write (), nano::genesis_account, { 1, nano::genesis_hash }); - test_mode (nano::confirmation_height_mode::bounded); - test_mode (nano::confirmation_height_mode::unbounded); + nano::confirmation_height_bounded bounded_processor ( + ledger, write_database_queue, 10ms, logger, stopped, block_hash_being_processed, batch_write_size, [](auto const &) {}, [](auto const &) {}, []() { return 0; }); + + { + // This reads the blocks in the account, but prevents any writes from occuring yet + auto scoped_write_guard = write_database_queue.wait (nano::writer::testing); + bounded_processor.process (); + } + + // Rollback the block and now try to write, the send should be cemented but the account which the open block belongs no longer exists so should bail + ledger.rollback (store.tx_begin_write (), open->hash ()); + auto scoped_write_guard = write_database_queue.wait (nano::writer::confirmation_height); + ASSERT_DEATH_IF_SUPPORTED (bounded_processor.cement_blocks (scoped_write_guard), ""); + } } namespace nano @@ -1503,4 +1608,4 @@ TEST (confirmation_height, election_winner_details_clearing) test_mode (nano::confirmation_height_mode::bounded); test_mode (nano::confirmation_height_mode::unbounded); } -} +} \ No newline at end of file diff --git a/nano/core_test/difficulty.cpp b/nano/core_test/difficulty.cpp index e27bc0c9b5..1a987ec37e 100644 --- a/nano/core_test/difficulty.cpp +++ b/nano/core_test/difficulty.cpp @@ -23,7 +23,7 @@ TEST (system, work_generate_limited) } } -TEST (difficulty, multipliers) +TEST (difficultyDeathTest, multipliers) { // For ASSERT_DEATH_IF_SUPPORTED testing::FLAGS_gtest_death_test_style = "threadsafe"; @@ -64,20 +64,19 @@ TEST (difficulty, multipliers) ASSERT_EQ (difficulty, nano::difficulty::from_multiplier (expected_multiplier, base)); } - { + // The death checks don't fail on a release config, so guard against them #ifndef NDEBUG - // Causes valgrind to be noisy - if (!nano::running_within_valgrind ()) - { - uint64_t base = 0xffffffc000000000; - uint64_t difficulty_nil = 0; - double multiplier_nil = 0.; - - ASSERT_DEATH_IF_SUPPORTED (nano::difficulty::to_multiplier (difficulty_nil, base), ""); - ASSERT_DEATH_IF_SUPPORTED (nano::difficulty::from_multiplier (multiplier_nil, base), ""); - } -#endif + // Causes valgrind to be noisy + if (!nano::running_within_valgrind ()) + { + uint64_t base = 0xffffffc000000000; + uint64_t difficulty_nil = 0; + double multiplier_nil = 0.; + + ASSERT_DEATH_IF_SUPPORTED (nano::difficulty::to_multiplier (difficulty_nil, base), ""); + ASSERT_DEATH_IF_SUPPORTED (nano::difficulty::from_multiplier (multiplier_nil, base), ""); } +#endif } TEST (difficulty, overflow) diff --git a/nano/lib/stats.cpp b/nano/lib/stats.cpp index 323dd7cdbf..a473f3438d 100644 --- a/nano/lib/stats.cpp +++ b/nano/lib/stats.cpp @@ -691,9 +691,6 @@ std::string nano::stat::detail_to_string (uint32_t key) case nano::stat::detail::outdated_version: res = "outdated_version"; break; - case nano::stat::detail::invalid_block: - res = "invalid_block"; - break; case nano::stat::detail::blocks_confirmed: res = "blocks_confirmed"; break; diff --git a/nano/lib/stats.hpp b/nano/lib/stats.hpp index 6254ac323c..6643b61ea3 100644 --- a/nano/lib/stats.hpp +++ b/nano/lib/stats.hpp @@ -313,7 +313,6 @@ class stat final blocks_confirmed, blocks_confirmed_unbounded, blocks_confirmed_bounded, - invalid_block, // [request] aggregator aggregator_accepted, diff --git a/nano/node/confirmation_height_bounded.cpp b/nano/node/confirmation_height_bounded.cpp index 29dda34656..5ccba172f2 100644 --- a/nano/node/confirmation_height_bounded.cpp +++ b/nano/node/confirmation_height_bounded.cpp @@ -70,7 +70,13 @@ void nano::confirmation_height_bounded::process () auto top_level_hash = current; auto block = ledger.store.block_get (transaction, current); - debug_assert (block != nullptr); + if (!block) + { + auto error_str = (boost::format ("Ledger mismatch trying to set confirmation height for block %1% (bounded processor)") % current.to_string ()).str (); + logger.always_log (error_str); + std::cerr << error_str << std::endl; + } + release_assert (block); nano::account account (block->account ()); if (account.is_zero ()) { @@ -169,23 +175,16 @@ void nano::confirmation_height_bounded::process () if ((max_batch_write_size_reached || should_output || force_write) && !pending_writes.empty ()) { - bool error = false; // If nothing is currently using the database write lock then write the cemented pending blocks otherwise continue iterating if (write_database_queue.process (nano::writer::confirmation_height)) { auto scoped_write_guard = write_database_queue.pop (); - error = cement_blocks (scoped_write_guard); + cement_blocks (scoped_write_guard); } else if (force_write) { auto scoped_write_guard = write_database_queue.wait (nano::writer::confirmation_height); - error = cement_blocks (scoped_write_guard); - } - // Don't set any more cemented blocks from the original hash if an inconsistency is found - if (error) - { - checkpoints.clear (); - break; + cement_blocks (scoped_write_guard); } } } @@ -334,7 +333,7 @@ void nano::confirmation_height_bounded::prepare_iterated_blocks_for_cementing (p } } -bool nano::confirmation_height_bounded::cement_blocks (nano::write_guard & scoped_write_guard_a) +void nano::confirmation_height_bounded::cement_blocks (nano::write_guard & scoped_write_guard_a) { // Will contain all blocks that have been cemented (bounded by batch_write_size) // and will get run through the cemented observer callback @@ -344,13 +343,14 @@ bool nano::confirmation_height_bounded::cement_blocks (nano::write_guard & scope auto const amount_to_change = batch_write_size / 10; // 10% auto const minimum_batch_write_size = 16384u; nano::timer<> cemented_batch_timer; + auto error = false; { // This only writes to the confirmation_height table and is the only place to do so in a single process auto transaction (ledger.store.tx_begin_write ({}, { nano::tables::confirmation_height })); cemented_batch_timer.start (); // Cement all pending entries, each entry is specific to an account and contains the least amount // of blocks to retain consistent cementing across all account chains to genesis. - while (!pending_writes.empty ()) + while (!error && !pending_writes.empty ()) { const auto & pending = pending_writes.front (); const auto & account = pending.account; @@ -371,10 +371,16 @@ bool nano::confirmation_height_bounded::cement_blocks (nano::write_guard & scope }; nano::confirmation_height_info confirmation_height_info; - release_assert (!ledger.store.confirmation_height_get (transaction, pending.account, confirmation_height_info)); + error = ledger.store.confirmation_height_get (transaction, pending.account, confirmation_height_info); + if (error) + { + auto error_str = (boost::format ("Failed to read confirmation height for account %1% (bounded processor)") % pending.account.to_account ()).str (); + logger.always_log (error_str); + std::cerr << error_str << std::endl; + } // Some blocks need to be cemented at least - if (pending.top_height > confirmation_height_info.height) + if (!error && pending.top_height > confirmation_height_info.height) { // The highest hash which will be cemented nano::block_hash new_cemented_frontier; @@ -397,21 +403,21 @@ bool nano::confirmation_height_bounded::cement_blocks (nano::write_guard & scope } auto total_blocks_cemented = 0; - auto num_blocks_iterated = 0; - auto block = ledger.store.block_get (transaction, new_cemented_frontier); // Cementing starts from the bottom of the chain and works upwards. This is because chains can have effectively // an infinite number of send/change blocks in a row. We don't want to hold the write transaction open for too long. - for (; num_blocks_confirmed - num_blocks_iterated != 0; ++num_blocks_iterated) + for (auto num_blocks_iterated = 0; num_blocks_confirmed - num_blocks_iterated != 0; ++num_blocks_iterated) { if (!block) { - logger.always_log ("Failed to write confirmation height for: ", new_cemented_frontier.to_string ()); - ledger.stats.inc (nano::stat::type::confirmation_height, nano::stat::detail::invalid_block); - pending_writes.clear (); - pending_writes_size = 0; - return true; + auto error_str = (boost::format ("Failed to write confirmation height for block %1% (bounded processor)") % new_cemented_frontier.to_string ()).str (); + logger.always_log (error_str); + std::cerr << error_str << std::endl; + // Undo any blocks about to be cemented from this account for this pending write. + cemented_blocks.erase (cemented_blocks.end () - num_blocks_iterated, cemented_blocks.end ()); + error = true; + break; } auto last_iteration = (num_blocks_confirmed - num_blocks_iterated) == 1; @@ -470,6 +476,12 @@ bool nano::confirmation_height_bounded::cement_blocks (nano::write_guard & scope } } + if (error) + { + // There was an error writing a block, do not process any more + break; + } + auto num_blocks_cemented = num_blocks_confirmed - total_blocks_cemented; if (num_blocks_cemented > 0) { @@ -499,9 +511,12 @@ bool nano::confirmation_height_bounded::cement_blocks (nano::write_guard & scope scoped_write_guard_a.release (); notify_observers_callback (cemented_blocks); } + release_assert (!error); // Tests should check this already at the end, but not all blocks may have elections (e.g from manual calls to confirmation_height_processor::add), this should catch any inconsistencies on live/beta though if (!network_params.network.is_test_network ()) { + // Bail if there was an error. This indicates that there was a fatal issue with the ledger + // (the blocks probably got rolled back when they shouldn't have). auto blocks_confirmed_stats = ledger.stats.count (nano::stat::type::confirmation_height, nano::stat::detail::blocks_confirmed); auto observer_stats = ledger.stats.count (nano::stat::type::confirmation_observer, nano::stat::detail::all, nano::stat::dir::out); debug_assert (blocks_confirmed_stats == observer_stats); @@ -516,7 +531,6 @@ bool nano::confirmation_height_bounded::cement_blocks (nano::write_guard & scope debug_assert (pending_writes.empty ()); debug_assert (pending_writes_size == 0); - return false; } bool nano::confirmation_height_bounded::pending_empty () const diff --git a/nano/node/confirmation_height_bounded.hpp b/nano/node/confirmation_height_bounded.hpp index 9694f71493..812f991dec 100644 --- a/nano/node/confirmation_height_bounded.hpp +++ b/nano/node/confirmation_height_bounded.hpp @@ -21,7 +21,7 @@ class confirmation_height_bounded final bool pending_empty () const; void reset (); void process (); - bool cement_blocks (nano::write_guard & scoped_write_guard_a); + void cement_blocks (nano::write_guard & scoped_write_guard_a); private: class top_and_next_hash final diff --git a/nano/node/confirmation_height_unbounded.cpp b/nano/node/confirmation_height_unbounded.cpp index ed2d890d1d..95f94678f4 100644 --- a/nano/node/confirmation_height_unbounded.cpp +++ b/nano/node/confirmation_height_unbounded.cpp @@ -54,6 +54,14 @@ void nano::confirmation_height_unbounded::process () } auto block (get_block_and_sideband (current, read_transaction)); + if (!block) + { + auto error_str = (boost::format ("Ledger mismatch trying to set confirmation height for block %1% (unbounded processor)") % current.to_string ()).str (); + logger.always_log (error_str); + std::cerr << error_str << std::endl; + } + release_assert (block); + nano::account account (block->account ()); if (account.is_zero ()) { @@ -145,23 +153,16 @@ void nano::confirmation_height_unbounded::process () if ((max_write_size_reached || should_output || force_write) && !pending_writes.empty ()) { - bool error = false; if (write_database_queue.process (nano::writer::confirmation_height)) { auto scoped_write_guard = write_database_queue.pop (); - error = cement_blocks (scoped_write_guard); + cement_blocks (scoped_write_guard); } else if (force_write) { // Unbounded processor has grown too large, force a write auto scoped_write_guard = write_database_queue.wait (nano::writer::confirmation_height); - error = cement_blocks (scoped_write_guard); - } - - // Don't set any more cemented blocks from the original hash if an inconsistency is found - if (error) - { - break; + cement_blocks (scoped_write_guard); } } @@ -331,10 +332,11 @@ void nano::confirmation_height_unbounded::prepare_iterated_blocks_for_cementing /* * Returns true if there was an error in finding one of the blocks to write a confirmation height for, false otherwise */ -bool nano::confirmation_height_unbounded::cement_blocks (nano::write_guard & scoped_write_guard_a) +void nano::confirmation_height_unbounded::cement_blocks (nano::write_guard & scoped_write_guard_a) { nano::timer cemented_batch_timer; std::vector> cemented_blocks; + auto error = false; { auto transaction (ledger.store.tx_begin_write ({}, { nano::tables::confirmation_height })); cemented_batch_timer.start (); @@ -342,25 +344,29 @@ bool nano::confirmation_height_unbounded::cement_blocks (nano::write_guard & sco { auto & pending = pending_writes.front (); nano::confirmation_height_info confirmation_height_info; - auto error = ledger.store.confirmation_height_get (transaction, pending.account, confirmation_height_info); - release_assert (!error); + error = ledger.store.confirmation_height_get (transaction, pending.account, confirmation_height_info); + if (error) + { + auto error_str = (boost::format ("Failed to read confirmation height for account %1% when writing block %2% (unbounded processor)") % pending.account.to_account () % pending.hash.to_string ()).str (); + logger.always_log (error_str); + std::cerr << error_str << std::endl; + } auto confirmation_height = confirmation_height_info.height; - if (pending.height > confirmation_height) + if (!error && pending.height > confirmation_height) { #ifndef NDEBUG // Do more thorough checking in Debug mode, indicates programming error. auto block = ledger.store.block_get (transaction, pending.hash); - static nano::network_constants network_constants; - debug_assert (network_constants.is_test_network () || block != nullptr); - debug_assert (network_constants.is_test_network () || block->sideband ().height == pending.height); + debug_assert (network_params.network.is_test_network () || block != nullptr); + debug_assert (network_params.network.is_test_network () || block->sideband ().height == pending.height); if (!block) { - logger.always_log ("Failed to write confirmation height for: ", pending.hash.to_string ()); - ledger.stats.inc (nano::stat::type::confirmation_height, nano::stat::detail::invalid_block); - pending_writes.clear (); - pending_writes_size = 0; - return true; + auto error_str = (boost::format ("Failed to write confirmation height for block %1% (unbounded processor)") % pending.hash.to_string ()).str (); + logger.always_log (error_str); + std::cerr << error_str << std::endl; + error = true; + break; } #endif ledger.stats.add (nano::stat::type::confirmation_height, nano::stat::detail::blocks_confirmed, nano::stat::dir::in, pending.height - confirmation_height); @@ -391,6 +397,7 @@ bool nano::confirmation_height_unbounded::cement_blocks (nano::write_guard & sco scoped_write_guard_a.release (); notify_observers_callback (cemented_blocks); + release_assert (!error); // Tests should check this already at the end, but not all blocks may have elections (e.g from manual calls to confirmation_height_processor::add), this should catch any inconsistencies on live/beta though if (!network_params.network.is_test_network ()) @@ -400,7 +407,7 @@ bool nano::confirmation_height_unbounded::cement_blocks (nano::write_guard & sco debug_assert (blocks_confirmed_stats == observer_stats); } debug_assert (pending_writes.empty ()); - return false; + debug_assert (pending_writes_size == 0); } std::shared_ptr nano::confirmation_height_unbounded::get_block_and_sideband (nano::block_hash const & hash_a, nano::transaction const & transaction_a) diff --git a/nano/node/confirmation_height_unbounded.hpp b/nano/node/confirmation_height_unbounded.hpp index 5cdb1b1dd7..14954bb8ca 100644 --- a/nano/node/confirmation_height_unbounded.hpp +++ b/nano/node/confirmation_height_unbounded.hpp @@ -22,7 +22,7 @@ class confirmation_height_unbounded final bool pending_empty () const; void reset (); void process (); - bool cement_blocks (nano::write_guard &); + void cement_blocks (nano::write_guard &); private: class confirmed_iterated_pair diff --git a/nano/secure/blockstore_partial.hpp b/nano/secure/blockstore_partial.hpp index 31d7bfaa03..4cdfe508e7 100644 --- a/nano/secure/blockstore_partial.hpp +++ b/nano/secure/blockstore_partial.hpp @@ -45,6 +45,7 @@ class block_store_partial : public block_store nano::uint128_t block_balance (nano::transaction const & transaction_a, nano::block_hash const & hash_a) override { auto block (block_get (transaction_a, hash_a)); + release_assert (block); nano::uint128_t result (block_balance_calculated (block)); return result; }