diff --git a/nano/core_test/confirmation_height.cpp b/nano/core_test/confirmation_height.cpp index 41fb1aff73..1bae89b90d 100644 --- a/nano/core_test/confirmation_height.cpp +++ b/nano/core_test/confirmation_height.cpp @@ -640,66 +640,98 @@ TEST (confirmation_height, all_block_types) test_mode (nano::confirmation_height_mode::unbounded); } -/* Bulk of the this test was taken from the node.fork_flip test */ -// Test disabled because it's failing intermittently. -// PR in which it got disabled: https://github.com/nanocurrency/nano-node/pull/3629 -// Issue for investigating it: https://github.com/nanocurrency/nano-node/issues/3633 -TEST (confirmation_height, DISABLED_conflict_rollback_cemented) +// this test cements a block on one node and another block on another node +// it therefore tests that once a block is confirmed it cannot be rolled back +// and if both nodes have different branches of the fork cemented then it is a permanent fork +TEST (confirmation_height, conflict_rollback_cemented) { + // functor to perform the conflict_rollback_cemented test using a certain mode auto test_mode = [] (nano::confirmation_height_mode mode_a) { - boost::iostreams::stream_buffer sb; - sb.open (nano::stringstream_mt_sink{}); - nano::boost_log_cerr_redirect redirect_cerr (&sb); - nano::system system; - nano::node_flags node_flags; + nano::state_block_builder builder{}; + auto const genesis_hash = nano::dev::genesis->hash (); + + nano::system system{}; + nano::node_flags node_flags{}; node_flags.confirmation_height_processor_mode = mode_a; + + // create node 1 and account key1 (no voting key yet) auto node1 = system.add_node (node_flags); - auto node2 = system.add_node (node_flags); - ASSERT_EQ (1, node1->network.size ()); - nano::keypair key1; - auto send1 (std::make_shared (nano::dev::genesis->hash (), key1.pub, nano::dev::constants.genesis_amount - 100, nano::dev::genesis_key.prv, nano::dev::genesis_key.pub, *system.work.generate (nano::dev::genesis->hash ()))); - nano::publish publish1{ nano::dev::network_params.network, send1 }; - nano::keypair key2; - auto send2 (std::make_shared (nano::dev::genesis->hash (), key2.pub, nano::dev::constants.genesis_amount - 100, nano::dev::genesis_key.prv, nano::dev::genesis_key.pub, *system.work.generate (nano::dev::genesis->hash ()))); - nano::publish publish2{ nano::dev::network_params.network, send2 }; - auto channel1 (node1->network.udp_channels.create (node1->network.endpoint ())); - node1->network.inbound (publish1, channel1); - node1->block_processor.flush (); - node1->scheduler.flush (); - auto channel2 (node2->network.udp_channels.create (node1->network.endpoint ())); - node2->network.inbound (publish2, channel2); - node2->block_processor.flush (); - node2->scheduler.flush (); - ASSERT_EQ (1, node1->active.size ()); - ASSERT_EQ (1, node2->active.size ()); - system.wallet (0)->insert_adhoc (nano::dev::genesis_key.prv); - node1->network.inbound (publish2, channel1); - node1->block_processor.flush (); - node2->network.inbound (publish1, channel2); - node2->block_processor.flush (); - auto election (node2->active.election (nano::qualified_root (nano::dev::genesis->hash (), nano::dev::genesis->hash ()))); - ASSERT_NE (nullptr, election); - ASSERT_EQ (1, election->votes ().size ()); - // Force blocks to be cemented on both nodes - { - auto transaction (node1->store.tx_begin_write ()); - ASSERT_TRUE (node1->store.block.exists (transaction, publish1.block->hash ())); - node1->store.confirmation_height.put (transaction, nano::dev::genesis->account (), nano::confirmation_height_info{ 2, send2->hash () }); - } - { - auto transaction (node2->store.tx_begin_write ()); - ASSERT_TRUE (node2->store.block.exists (transaction, publish2.block->hash ())); - node2->store.confirmation_height.put (transaction, nano::dev::genesis->account (), nano::confirmation_height_info{ 2, send2->hash () }); - } + nano::keypair key1{}; + + // create one side of a forked transaction on node1 + auto send1 = builder.make_block () + .previous (genesis_hash) + .account (nano::dev::genesis_key.pub) + .representative (nano::dev::genesis_key.pub) + .link (key1.pub) + .balance (nano::dev::constants.genesis_amount - 100) + .sign (nano::dev::genesis_key.prv, nano::dev::genesis_key.pub) + .work (*system.work.generate (genesis_hash)) + .build_shared (); + node1->process_active (send1); + ASSERT_TIMELY (5s, node1->active.election (send1->qualified_root ()) != nullptr); - auto rollback_log_entry = boost::str (boost::format ("Failed to roll back %1%") % send2->hash ().to_string ()); - ASSERT_TIMELY (20s, sb.component ()->str ().find (rollback_log_entry) != std::string::npos); - auto winner (*election->tally ().begin ()); - ASSERT_EQ (*publish1.block, *winner.second); - ASSERT_EQ (nano::dev::constants.genesis_amount - 100, winner.first); - ASSERT_TRUE (node1->ledger.block_or_pruned_exists (publish1.block->hash ())); - ASSERT_TRUE (node2->ledger.block_or_pruned_exists (publish2.block->hash ())); - ASSERT_FALSE (node2->ledger.block_or_pruned_exists (publish1.block->hash ())); + // create the other side of the fork on node2 + nano::keypair key2; + auto send2 = builder.make_block () + .previous (genesis_hash) + .account (nano::dev::genesis_key.pub) + .representative (nano::dev::genesis_key.pub) + .link (key2.pub) + .balance (nano::dev::constants.genesis_amount - 100) + .sign (nano::dev::genesis_key.prv, nano::dev::genesis_key.pub) + .work (*system.work.generate (genesis_hash)) + .build_shared (); + + // create node2, with send2 pre-initialised in the ledger so that block send1 cannot possibly get in the ledger first + system.initialization_blocks.push_back (send2); + auto node2 = system.add_node (node_flags); + system.initialization_blocks.clear (); + auto wallet1 = system.wallet (0); + node2->process_active (send2); + ASSERT_TIMELY (5s, node2->active.election (send2->qualified_root ()) != nullptr); + + // force confirm send2 on node2 + ASSERT_TIMELY (5s, node2->ledger.store.block.get (node2->ledger.store.tx_begin_read (), send2->hash ())); + node2->process_confirmed (nano::election_status{ send2 }); + ASSERT_TIMELY (5s, node2->block_confirmed (send2->hash ())); + + // make node1 a voting node (it has all the voting weight) + // from now on, node1 can vote for send1 at any time + wallet1->insert_adhoc (nano::dev::genesis_key.prv); + + // we expect node1 to vote for one side of the fork only, whichever side + std::shared_ptr election_send1_node1{}; + ASSERT_EQ (send1->qualified_root (), send2->qualified_root ()); + ASSERT_TIMELY (5s, (election_send1_node1 = node1->active.election (send1->qualified_root ())) != nullptr); + ASSERT_TIMELY (5s, 2 == election_send1_node1->votes ().size ()); + + // check that the send1 on node1 won the election and got confirmed + // this happens because send1 is seen first by node1, and therefore it already winning and it cannot replaced by send2 + ASSERT_TIMELY (5s, election_send1_node1->confirmed ()); + auto const winner = election_send1_node1->winner (); + ASSERT_NE (nullptr, winner); + ASSERT_EQ (*winner, *send1); + + // node2 already has send2 forced confirmed whilst node1 should have confirmed send1 and therefore we have a cemented fork on node2 + // and node2 should print an error message on the log that it cannot rollback send2 because it is already cemented + ASSERT_TIMELY (5s, 1 == node2->stats.count (nano::stat::type::ledger, nano::stat::detail::rollback_failed)); + + // get the tally for election the election on node1 + // we expect the winner to be send1 and we expect send1 to have "genesis balance" vote weight + auto const tally = election_send1_node1->tally (); + ASSERT_FALSE (tally.empty ()); + auto const & [amount, winner_alias] = *tally.begin (); + ASSERT_EQ (*winner_alias, *send1); + ASSERT_EQ (amount, nano::dev::constants.genesis_amount - 100); + + // we expect send1 to exist on node1, is that because send2 is rolled back? + ASSERT_TRUE (node1->ledger.block_or_pruned_exists (send1->hash ())); + ASSERT_FALSE (node1->ledger.block_or_pruned_exists (send2->hash ())); + + // we expect only send2 to be existing on node2 + ASSERT_TRUE (node2->ledger.block_or_pruned_exists (send2->hash ())); + ASSERT_FALSE (node2->ledger.block_or_pruned_exists (send1->hash ())); }; test_mode (nano::confirmation_height_mode::bounded); diff --git a/nano/lib/stats.cpp b/nano/lib/stats.cpp index 4c04cad5b5..db30ffbd92 100644 --- a/nano/lib/stats.cpp +++ b/nano/lib/stats.cpp @@ -612,6 +612,9 @@ std::string nano::stat::detail_to_string (stat::detail detail) case nano::stat::detail::gap_source: res = "gap_source"; break; + case nano::stat::detail::rollback_failed: + res = "rollback_failed"; + break; case nano::stat::detail::frontier_confirmation_failed: res = "frontier_confirmation_failed"; break; diff --git a/nano/lib/stats.hpp b/nano/lib/stats.hpp index ca04d8c9c9..a8b0b1bd7e 100644 --- a/nano/lib/stats.hpp +++ b/nano/lib/stats.hpp @@ -272,6 +272,7 @@ class stat final old, gap_previous, gap_source, + rollback_failed, // message specific keepalive, diff --git a/nano/node/blockprocessor.cpp b/nano/node/blockprocessor.cpp index daf69083d5..6e7272a6cf 100644 --- a/nano/node/blockprocessor.cpp +++ b/nano/node/blockprocessor.cpp @@ -280,6 +280,7 @@ void nano::block_processor::process_batch (nano::unique_lock & lock std::vector> rollback_list; if (node.ledger.rollback (transaction, successor->hash (), rollback_list)) { + node.stats.inc (nano::stat::type::ledger, nano::stat::detail::rollback_failed); node.logger.always_log (nano::severity_level::error, boost::str (boost::format ("Failed to roll back %1% because it or a successor was confirmed") % successor->hash ().to_string ())); } else if (node.config.logging.ledger_rollback_logging ()) diff --git a/nano/secure/ledger.hpp b/nano/secure/ledger.hpp index e994908522..43d13fbc19 100644 --- a/nano/secure/ledger.hpp +++ b/nano/secure/ledger.hpp @@ -12,6 +12,7 @@ class store; class stat; class write_transaction; +// map of vote weight per block, ordered greater first using tally_t = std::map, std::greater>; class uncemented_info diff --git a/nano/test_common/system.hpp b/nano/test_common/system.hpp index 8db2b92dee..78cf36413b 100644 --- a/nano/test_common/system.hpp +++ b/nano/test_common/system.hpp @@ -55,8 +55,6 @@ class system final std::chrono::time_point> deadline{ std::chrono::steady_clock::time_point::max () }; double deadline_scaling_factor{ 1.0 }; unsigned node_sequence{ 0 }; - -private: std::vector> initialization_blocks; }; std::unique_ptr upgrade_epoch (nano::work_pool &, nano::ledger &, nano::epoch);