diff --git a/nano/core_test/request_aggregator.cpp b/nano/core_test/request_aggregator.cpp index 0e28586de8..55f04af189 100644 --- a/nano/core_test/request_aggregator.cpp +++ b/nano/core_test/request_aggregator.cpp @@ -1,4 +1,5 @@ #include +#include #include #include #include @@ -9,12 +10,17 @@ #include #include #include +#include #include #include #include #include +#include +#include +#include + using namespace std::chrono_literals; TEST (request_aggregator, one) @@ -450,3 +456,194 @@ TEST (request_aggregator, cannot_vote) ASSERT_EQ (0, node.stats.count (nano::stat::type::requests, nano::stat::detail::requests_unknown)); ASSERT_TIMELY (3s, 1 <= node.stats.count (nano::stat::type::message, nano::stat::detail::confirm_ack, nano::stat::dir::out)); } + +namespace +{ +std::future observe_confirm_ack (std::shared_ptr const & channel) +{ + std::promise promise; + auto future = promise.get_future (); + + channel->observers.clear (); + channel->observers.add (nano::wrap_move_only ([&, promise = std::move (promise)] (nano::message const & message, nano::transport::traffic_type const & type) mutable { + struct visitor : public nano::message_visitor + { + std::optional result; + + void confirm_ack (nano::confirm_ack const & msg) override + { + result = msg; + } + }; + + visitor visitor{}; + message.visit (visitor); + if (visitor.result) + { + promise.set_value (visitor.result.value ()); + } + })); + + return future; +} +} + +/* + * Request for a forked open block should return vote for the correct fork alternative + */ +TEST (request_aggregator, forked_open) +{ + nano::test::system system; + auto & node = *system.add_node (); + + // Voting needs a rep key set up on the node + system.wallet (0)->insert_adhoc (nano::dev::genesis_key.prv); + + // Setup two forks of the open block + nano::keypair key; + nano::block_builder builder; + auto send0 = builder.send () + .previous (nano::dev::genesis->hash ()) + .destination (key.pub) + .balance (nano::dev::constants.genesis_amount - 500) + .sign (nano::dev::genesis_key.prv, nano::dev::genesis_key.pub) + .work (*system.work.generate (nano::dev::genesis->hash ())) + .build (); + auto open0 = builder.open () + .source (send0->hash ()) + .representative (1) + .account (key.pub) + .sign (key.prv, key.pub) + .work (*system.work.generate (key.pub)) + .build (); + auto open1 = builder.open () + .source (send0->hash ()) + .representative (2) + .account (key.pub) + .sign (key.prv, key.pub) + .work (*system.work.generate (key.pub)) + .build (); + + nano::test::process (node, { send0, open0 }); + nano::test::confirm (node, { open0 }); + + auto channel = nano::test::test_channel (node); + auto future = observe_confirm_ack (channel); + + // Request vote for the wrong fork + std::vector> request{ { open1->hash (), open1->root () } }; + ASSERT_TRUE (node.aggregator.request (request, channel)); + + ASSERT_EQ (future.wait_for (5s), std::future_status::ready); + + auto ack = future.get (); + ASSERT_EQ (ack.vote->hashes.size (), 1); + ASSERT_EQ (ack.vote->hashes[0], open0->hash ()); // Vote for the correct fork alternative + ASSERT_EQ (ack.vote->account, nano::dev::genesis_key.pub); +} + +/* + * Request for a conflicting epoch block should return vote for the correct alternative + */ +TEST (request_aggregator, epoch_conflict) +{ + nano::test::system system; + auto & node = *system.add_node (); + + // Voting needs a rep key set up on the node + system.wallet (0)->insert_adhoc (nano::dev::genesis_key.prv); + + // Setup the initial chain and the conflicting blocks + nano::keypair key; + nano::keypair epoch_signer (nano::dev::genesis_key); + nano::state_block_builder builder; + + // Create initial chain: send -> open -> change + auto send = builder.make_block () + .account (nano::dev::genesis_key.pub) + .previous (nano::dev::genesis->hash ()) + .representative (nano::dev::genesis_key.pub) + .balance (nano::dev::constants.genesis_amount - 1) + .link (key.pub) + .sign (nano::dev::genesis_key.prv, nano::dev::genesis_key.pub) + .work (*system.work.generate (nano::dev::genesis->hash ())) + .build (); + + auto open = builder.make_block () + .account (key.pub) + .previous (0) + .representative (key.pub) + .balance (1) + .link (send->hash ()) + .sign (key.prv, key.pub) + .work (*system.work.generate (key.pub)) + .build (); + + // Change block root is the open block hash, qualified root: {open, open} + auto change = builder.make_block () + .account (key.pub) + .previous (open->hash ()) + .representative (key.pub) + .balance (1) + .link (0) + .sign (key.prv, key.pub) + .work (*system.work.generate (open->hash ())) + .build (); + + // Pending entry is needed first to process the epoch open block + auto pending = builder.make_block () + .account (nano::dev::genesis_key.pub) + .previous (send->hash ()) + .representative (nano::dev::genesis_key.pub) + .balance (nano::dev::constants.genesis_amount - 2) + .link (change->root ().as_account ()) + .sign (nano::dev::genesis_key.prv, nano::dev::genesis_key.pub) + .work (*system.work.generate (send->hash ())) + .build (); + + // Create conflicting epoch block with the same root as the change block, qualified root: {open, 0} + // This block is intentionally not processed immediately so the node doesn't know about it + auto epoch_open = builder.make_block () + .account (change->root ().as_account ()) + .previous (0) + .representative (0) + .balance (0) + .link (node.ledger.epoch_link (nano::epoch::epoch_1)) + .sign (epoch_signer.prv, epoch_signer.pub) + .work (*system.work.generate (open->hash ())) + .build (); + + // Process and confirm the initial chain with the change block + nano::test::process (node, { send, open, change }); + nano::test::confirm (node, { change }); + ASSERT_TIMELY (5s, node.block_confirmed (change->hash ())); + + auto channel = nano::test::test_channel (node); + + // Request vote for the conflicting epoch block + std::vector> request{ { epoch_open->hash (), epoch_open->root () } }; + auto future1 = observe_confirm_ack (channel); + ASSERT_TRUE (node.aggregator.request (request, channel)); + + ASSERT_EQ (future1.wait_for (5s), std::future_status::ready); + + auto ack1 = future1.get (); + ASSERT_EQ (ack1.vote->hashes.size (), 1); + ASSERT_EQ (ack1.vote->hashes[0], change->hash ()); // Vote for the correct alternative (change block) + ASSERT_EQ (ack1.vote->account, nano::dev::genesis_key.pub); + + // Process the conflicting epoch block + nano::test::process (node, { pending, epoch_open }); + nano::test::confirm (node, { pending, epoch_open }); + + // Request vote for the conflicting epoch block again + auto future2 = observe_confirm_ack (channel); + ASSERT_TRUE (node.aggregator.request (request, channel)); + + ASSERT_EQ (future2.wait_for (5s), std::future_status::ready); + + auto ack2 = future2.get (); + ASSERT_EQ (ack2.vote->hashes.size (), 1); + ASSERT_EQ (ack2.vote->hashes[0], epoch_open->hash ()); // Vote for the epoch block + ASSERT_EQ (ack2.vote->account, nano::dev::genesis_key.pub); +} \ No newline at end of file diff --git a/nano/secure/vote.hpp b/nano/secure/vote.hpp index c4e24f149d..7da9b32780 100644 --- a/nano/secure/vote.hpp +++ b/nano/secure/vote.hpp @@ -26,7 +26,7 @@ class vote final * @returns true if there was an error */ bool deserialize (nano::stream &); - static std::size_t size (uint8_t count); + static std::size_t size (uint8_t count); // TODO: This name is confusing, vote size is number of hashes present, not the message size nano::block_hash hash () const; nano::block_hash full_hash () const;