From c40178a6c6b98e7a3b9a39d596cbba28d19f8cb5 Mon Sep 17 00:00:00 2001 From: clemahieu Date: Sun, 6 May 2018 10:56:28 +0100 Subject: [PATCH] Moving vote_replay code in to vote_processor as a more appropriate place to hang it. Only returning vote code instead of vote used since we don't need to track the vote used and might not always have access to it. Vote replay code now works appropriately if there's an active election for the block. --- rai/common.hpp | 6 ---- rai/core_test/ledger.cpp | 12 ++++---- rai/node/node.cpp | 63 ++++++++++++++++++---------------------- rai/node/node.hpp | 2 +- 4 files changed, 34 insertions(+), 49 deletions(-) diff --git a/rai/common.hpp b/rai/common.hpp index 7f9a48a8..6b119dde 100644 --- a/rai/common.hpp +++ b/rai/common.hpp @@ -208,12 +208,6 @@ enum class vote_code replay, // Vote does not have the highest sequence number, it's a replay vote // Vote has the highest sequence number }; -class vote_result -{ -public: - rai::vote_code code; - std::shared_ptr vote; -}; enum class process_result { diff --git a/rai/core_test/ledger.cpp b/rai/core_test/ledger.cpp index 432aaca0..62cd8ae7 100644 --- a/rai/core_test/ledger.cpp +++ b/rai/core_test/ledger.cpp @@ -771,10 +771,10 @@ TEST (votes, check_signature) ASSERT_EQ (1, votes1->votes.rep_votes.size ()); auto vote1 (std::make_shared (rai::test_genesis_key.pub, rai::test_genesis_key.prv, 1, send1)); vote1->signature.bytes[0] ^= 1; - ASSERT_EQ (rai::vote_code::invalid, node1.vote_processor.vote (vote1, rai::endpoint ()).code); + ASSERT_EQ (rai::vote_code::invalid, node1.vote_processor.vote (vote1, rai::endpoint ())); vote1->signature.bytes[0] ^= 1; - ASSERT_EQ (rai::vote_code::vote, node1.vote_processor.vote (vote1, rai::endpoint ()).code); - ASSERT_EQ (rai::vote_code::replay, node1.vote_processor.vote (vote1, rai::endpoint ()).code); + ASSERT_EQ (rai::vote_code::vote, node1.vote_processor.vote (vote1, rai::endpoint ())); + ASSERT_EQ (rai::vote_code::replay, node1.vote_processor.vote (vote1, rai::endpoint ())); } TEST (votes, add_one) @@ -926,14 +926,12 @@ TEST (votes, add_old_different_account) ASSERT_EQ (1, votes2->votes.rep_votes.size ()); auto vote1 (std::make_shared (rai::test_genesis_key.pub, rai::test_genesis_key.prv, 2, send1)); auto vote_result1 (node1.vote_processor.vote (vote1, rai::endpoint ())); - ASSERT_EQ (rai::vote_code::vote, vote_result1.code); - ASSERT_EQ (*vote1, *vote_result1.vote); + ASSERT_EQ (rai::vote_code::vote, vote_result1); ASSERT_EQ (2, votes1->votes.rep_votes.size ()); ASSERT_EQ (1, votes2->votes.rep_votes.size ()); auto vote2 (std::make_shared (rai::test_genesis_key.pub, rai::test_genesis_key.prv, 1, send2)); auto vote_result2 (node1.vote_processor.vote (vote2, rai::endpoint ())); - ASSERT_EQ (rai::vote_code::vote, vote_result2.code); - ASSERT_EQ (*vote2, *vote_result2.vote); + ASSERT_EQ (rai::vote_code::vote, vote_result2); ASSERT_EQ (2, votes1->votes.rep_votes.size ()); ASSERT_EQ (2, votes2->votes.rep_votes.size ()); ASSERT_NE (votes1->votes.rep_votes.end (), votes1->votes.rep_votes.find (rai::test_genesis_key.pub)); diff --git a/rai/node/node.cpp b/rai/node/node.cpp index e62dd093..5140a040 100644 --- a/rai/node/node.cpp +++ b/rai/node/node.cpp @@ -367,23 +367,7 @@ public: node.peers.contacted (sender, message_a.version_using); node.peers.insert (sender, message_a.version_using); node.process_active (message_a.vote->block); - auto vote (node.vote_processor.vote (message_a.vote, sender)); - if (vote.code == rai::vote_code::replay) - { - // This tries to assist rep nodes that have lost track of their highest sequence number by replaying our highest known vote back to them - // Only do this if the sequence number is significantly different to account for network reordering - // Amplify attack considerations: We're sending out a confirm_ack in response to a confirm_ack for no net traffic increase - if (vote.vote->sequence > message_a.vote->sequence + 10000) - { - rai::confirm_ack confirm (vote.vote); - std::shared_ptr> bytes (new std::vector); - { - rai::vectorstream stream (*bytes); - confirm.serialize (stream); - } - node.network.confirm_send (confirm, bytes, sender); - } - } + node.vote_processor.vote (message_a.vote, sender); } void bulk_pull (rai::bulk_pull const &) override { @@ -1102,30 +1086,47 @@ node (node_a) { } -rai::vote_result rai::vote_processor::vote (std::shared_ptr vote_a, rai::endpoint endpoint_a) +rai::vote_code rai::vote_processor::vote (std::shared_ptr vote_a, rai::endpoint endpoint_a) { - rai::vote_result result = { rai::vote_code::invalid, vote_a }; + auto result (rai::vote_code::invalid); if (!rai::validate_message (vote_a->account, vote_a->hash (), vote_a->signature)) { - result.code = rai::vote_code::replay; - std::shared_ptr newest_vote; + result = rai::vote_code::replay; + std::shared_ptr max_vote; { rai::transaction transaction (node.store.environment, nullptr, false); - newest_vote = node.store.vote_max (transaction, vote_a); + max_vote = node.store.vote_max (transaction, vote_a); } - if (!node.active.vote (vote_a)) + if (!node.active.vote (vote_a) || max_vote->sequence > vote_a->sequence) { - result.code = rai::vote_code::vote; + result = rai::vote_code::vote; } - else + switch (result) { - result.vote = newest_vote; + case rai::vote_code::vote: + node.observers.vote (vote_a, endpoint_a); + case rai::vote_code::replay: + // This tries to assist rep nodes that have lost track of their highest sequence number by replaying our highest known vote back to them + // Only do this if the sequence number is significantly different to account for network reordering + // Amplify attack considerations: We're sending out a confirm_ack in response to a confirm_ack for no net traffic increase + if (max_vote->sequence > vote_a->sequence + 10000) + { + rai::confirm_ack confirm (max_vote); + std::shared_ptr> bytes (new std::vector); + { + rai::vectorstream stream (*bytes); + confirm.serialize (stream); + } + node.network.confirm_send (confirm, bytes, endpoint_a); + } + case rai::vote_code::invalid: + break; } } if (node.config.logging.vote_logging ()) { char const * status; - switch (result.code) + switch (result) { case rai::vote_code::invalid: status = "Invalid"; @@ -1142,14 +1143,6 @@ rai::vote_result rai::vote_processor::vote (std::shared_ptr vote_a, r } BOOST_LOG (node.log) << boost::str (boost::format ("Vote from: %1% sequence: %2% block: %3% status: %4%") % vote_a->account.to_account () % std::to_string (vote_a->sequence) % vote_a->block->hash ().to_string () % status); } - switch (result.code) - { - case rai::vote_code::vote: - node.observers.vote (vote_a, endpoint_a); - case rai::vote_code::replay: - case rai::vote_code::invalid: - break; - } return result; } diff --git a/rai/node/node.hpp b/rai/node/node.hpp index b2ac58ec..cf35ce32 100644 --- a/rai/node/node.hpp +++ b/rai/node/node.hpp @@ -466,7 +466,7 @@ class vote_processor { public: vote_processor (rai::node &); - rai::vote_result vote (std::shared_ptr, rai::endpoint); + rai::vote_code vote (std::shared_ptr, rai::endpoint); rai::node & node; }; // The network is crawled for representatives by occasionally sending a unicast confirm_req for a specific block and watching to see if it's acknowledged with a vote.