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

Fix fork publish inactive #4091

Merged
merged 4 commits into from
Feb 8, 2023
Merged

Fix fork publish inactive #4091

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
960ac72
Tidy up test case
dsiganos Jan 31, 2023
d0a29ed
Ensure the election is started before the forked block is processed
dsiganos Jan 31, 2023
41f9706
Re-enable test case
dsiganos Jan 31, 2023
a187ca8
Add comment with concern
dsiganos Feb 8, 2023
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
29 changes: 21 additions & 8 deletions nano/core_test/node.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -617,36 +617,49 @@ TEST (node, fork_publish)
ASSERT_TRUE (node0.expired ());
}

// Test disabled because it's failing intermittently.
// PR in which it got disabled: https://github.com/nanocurrency/nano-node/pull/3611
// Issue for investigating it: https://github.com/nanocurrency/nano-node/issues/3614
TEST (node, DISABLED_fork_publish_inactive)
// In test case there used to be a race condition, it was worked around in:.
// https://github.com/nanocurrency/nano-node/pull/4091
// The election and the processing of block send2 happen in parallel.
// Usually the election happens first and the send2 block is added to the election.
// However, if the send2 block is processed before the election is started then
// there is a race somewhere and the election might not notice the send2 block.
// The test case can be made to pass by ensuring the election is started before the send2 is processed.
// However, is this a problem with the test case or this is a problem with the node handling of forks?
TEST (node, fork_publish_inactive)
{
nano::test::system system (1);
auto & node = *system.nodes[0];
nano::keypair key1;
nano::keypair key2;

nano::send_block_builder builder;

auto send1 = builder.make_block ()
.previous (nano::dev::genesis->hash ())
.destination (key1.pub)
.balance (nano::dev::constants.genesis_amount - 100)
.sign (nano::dev::genesis_key.prv, nano::dev::genesis_key.pub)
.work (*system.work.generate (nano::dev::genesis->hash ()))
.build_shared ();

auto send2 = builder.make_block ()
.previous (nano::dev::genesis->hash ())
.destination (key2.pub)
.balance (nano::dev::constants.genesis_amount - 100)
.sign (nano::dev::genesis_key.prv, nano::dev::genesis_key.pub)
.work (send1->block_work ())
.build_shared ();
auto & node = *system.nodes[0];

node.process_active (send1);
ASSERT_TIMELY (3s, nullptr != node.block (send1->hash ()));
ASSERT_TIMELY (5s, node.block (send1->hash ()));

std::shared_ptr<nano::election> election;
ASSERT_TIMELY (5s, election = node.active.election (send1->qualified_root ()));

ASSERT_EQ (nano::process_result::fork, node.process_local (send2).code);
auto election = node.active.election (send1->qualified_root ());
ASSERT_NE (election, nullptr);

auto blocks = election->blocks ();
ASSERT_TIMELY_EQ (5s, blocks.size (), 2);
ASSERT_NE (blocks.end (), blocks.find (send1->hash ()));
ASSERT_NE (blocks.end (), blocks.find (send2->hash ()));
ASSERT_EQ (election->winner ()->hash (), send1->hash ());
Expand Down