nanocurrency / nano-node

Nano is digital currency. Its ticker is: XNO and its currency symbol is: Ӿ
https://nano.org
BSD 3-Clause "New" or "Revised" License
3.49k stars 787 forks source link

Unit test conflicts.add_two failed in CI #3535

Closed thsfs closed 3 years ago

thsfs commented 3 years ago

[ RUN ] conflicts.add_two /workspace/nano/core_test/conflicts.cpp:68: Failure Expected equality of these values: 2 node1.active.size () Which is: 1 [ FAILED ] conflicts.add_two (226 ms)

https://github.com/nanocurrency/nano-node/runs/4038550570?check_suite_focus=true

To reproduce this, let the CPU load to be high and run ./core_test with the options: ./core_test --gtest_filter=conflicts.add_two --gtest_repeat=100 --gtest_throw_on_failure

thsfs commented 3 years ago

Disabled by PR: https://github.com/nanocurrency/nano-node/pull/3536

dsiganos commented 3 years ago

Some notes from a meeting analysing this unit test.

ASSUMPTION: We think that send1 is removed from AEC very quickly Adding a sleep for 5 seconds, just before the last assert, reliably makes it fail

SOLUTIONS: 1) Create 2 accounts and do 2 simultaneous sends that will remain in the AEC for at least 5 minutes Steps: a) send 100 nano to account a from genesis (use force confirm) b) receive the 100 nano on account a (use force confirm) c) send 1 nano to account b from genesis (this will not reach quorum) d) send 2 nano to account b from account a (this will not reach quorum) 2) Stop elections being removed from the AEC (Active Elections Container)

dsiganos commented 3 years ago

Annotations explaining the current code:

TEST (conflicts, DISABLED_add_two)
{
    nano::system system (1);

    // node1 does not have a private key and therefore it cannot vote
    auto & node1 (*system.nodes[0]);

    // send all of genesis balance to account a
    nano::keypair account_a;
    auto send1 (std::make_shared<nano::send_block> (nano::dev::genesis->hash (), account_a.pub, 0,
                nano::dev::genesis_key.prv, nano::dev::genesis_key.pub, 0));
    node1.work_generate_blocking (*send1);
    ASSERT_EQ (nano::process_result::progress, node1.process (*send1).code);

    // start an election for send1, the election will not reach quorum because node1
    // has not been given the genesis private key
    node1.block_confirm (send1);

    // force-confirm send1 bypassing the voting stage
    node1.active.election (send1->qualified_root ())->force_confirm ();

    // send 0 raw from genesis to account b, note that sending 0 amount is possible
    // with legacy send block but it is not possible with state blocks
    nano::keypair account_b;
    auto send2 (std::make_shared<nano::send_block> (send1->hash (), account_b.pub, 0,
                nano::dev::genesis_key.prv, nano::dev::genesis_key.pub, 0));
    node1.work_generate_blocking (*send2);
    ASSERT_EQ (nano::process_result::progress, node1.process (*send2).code);

    // activate election of send2 block
    node1.scheduler.activate (nano::dev::genesis_key.pub, node1.store.tx_begin_read ());
    node1.scheduler.flush ();

    // we expect 2 active entries in the elections container, this asserts fails however, because the election
    // for send1 can be deleted at any time since it is marked as confirmed
    // the election for send2 will stick around for a while (5 min) because it will not reach quorum
    ASSERT_EQ (2, node1.active.size ());
}
thsfs commented 3 years ago

@theohax , can we remove the old conflicts.add_two test, since you wrote a new one for it?

zhyatt commented 3 years ago

@thsfs Can we close this issue as resolved now?

thsfs commented 3 years ago

@thsfs Can we close this issue as resolved now?

Yes, we can.