Skip to content

Commit

Permalink
Force node exit if ledger inconsistency in the conf height processor …
Browse files Browse the repository at this point in the history 
…is found (#2768)

* Improve conf height processor resilience during fork testing

* Add extra checks during write transaction

* release_assert in conf height processor if a ledger inconsistency is found

* Remove unnecessary header file change

* Removed wrong member variable + typo

* Redesign tests to use bounded/unbounded processor directly and death asserts

* Output to console also (Serg review)

* Use convention from other death test (Gui comment)

* Fix death test failures on release builds

* Fix scope in test (Gui comment)
  • Loading branch information
@wezrule
wezrule authored May 18, 2020
1 parent d8f8a19 commit 5a3f0f8
Show file tree
Hide file tree
Showing 9 changed files with 237 additions and 115 deletions.
207 changes: 156 additions & 51 deletions nano/core_test/confirmation_height.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<unsigned>::max ());
nano::keypair key1;
auto send = std::make_shared<nano::send_block> (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<bool> 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) {
Expand Down Expand Up @@ -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<unsigned>::max ());
nano::keypair key1;
auto & store = node->store;
auto send = std::make_shared<nano::send_block> (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::send_block> (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<bool> 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<unsigned>::max ());
nano::keypair key1;
auto send = std::make_shared<nano::send_block> (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<nano::state_block> (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<bool> 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
Expand Down Expand Up @@ -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);
}
}
}
25 changes: 12 additions & 13 deletions nano/core_test/difficulty.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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)
Expand Down
3 changes: 0 additions & 3 deletions nano/lib/stats.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
1 change: 0 additions & 1 deletion nano/lib/stats.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,6 @@ class stat final
blocks_confirmed,
blocks_confirmed_unbounded,
blocks_confirmed_bounded,
invalid_block,

// [request] aggregator
aggregator_accepted,
Expand Down
Loading

0 comments on commit 5a3f0f8

Please sign in to comment.