From 63e15c21ae2b0098db266ef23c6f58f1e40676df Mon Sep 17 00:00:00 2001 From: clemahieu Date: Thu, 15 Mar 2018 14:16:16 -0500 Subject: [PATCH] Fix vote republish rules and duplicate sequence number detection --- rai/blockstore.cpp | 16 ----- rai/blockstore.hpp | 1 - rai/core_test/block_store.cpp | 23 ------- rai/core_test/ledger.cpp | 111 +++++++++++++++++++++++++++++++ rai/node/node.cpp | 122 ++++++++++++++++++++++++---------- rai/node/node.hpp | 10 +-- rai/slow_test/node.cpp | 3 +- 7 files changed, 205 insertions(+), 81 deletions(-) diff --git a/rai/blockstore.cpp b/rai/blockstore.cpp index c83ebee6..d06f9ea2 100644 --- a/rai/blockstore.cpp +++ b/rai/blockstore.cpp @@ -1262,22 +1262,6 @@ std::shared_ptr rai::block_store::vote_max (MDB_txn * transaction_a, return result; } -rai::vote_result rai::block_store::vote_validate (MDB_txn * transaction_a, std::shared_ptr vote_a) -{ - rai::vote_result result ({ rai::vote_code::invalid, 0 }); - // Reject unsigned votes - if (!rai::validate_message (vote_a->account, vote_a->hash (), vote_a->signature)) - { - result.code = rai::vote_code::replay; - result.vote = vote_max (transaction_a, vote_a); // Make sure this sequence number is > any we've seen from this account before - if (result.vote == vote_a) - { - result.code = rai::vote_code::vote; - } - } - return result; -} - rai::store_iterator rai::block_store::latest_begin (MDB_txn * transaction_a, rai::account const & account_a) { rai::store_iterator result (transaction_a, accounts, rai::mdb_val (account_a)); diff --git a/rai/blockstore.hpp b/rai/blockstore.hpp index 77ecce89..a93015ff 100644 --- a/rai/blockstore.hpp +++ b/rai/blockstore.hpp @@ -119,7 +119,6 @@ public: bool checksum_get (MDB_txn *, uint64_t, uint8_t, rai::checksum &); void checksum_del (MDB_txn *, uint64_t, uint8_t); - rai::vote_result vote_validate (MDB_txn *, std::shared_ptr); // Return latest vote for an account from store std::shared_ptr vote_get (MDB_txn *, rai::account const &); // Populate vote with the next sequence number diff --git a/rai/core_test/block_store.cpp b/rai/core_test/block_store.cpp index d46cb6e3..b75e4ba0 100644 --- a/rai/core_test/block_store.cpp +++ b/rai/core_test/block_store.cpp @@ -755,29 +755,6 @@ TEST (block_store, block_random) ASSERT_EQ (*block, *genesis.open); } -TEST (vote, validate) -{ - bool init (false); - rai::block_store store (init, rai::unique_path ()); - ASSERT_TRUE (!init); - rai::keypair key1; - auto send1 (std::make_shared (0, key1.pub, 0, rai::test_genesis_key.prv, rai::test_genesis_key.pub, 0)); - auto vote1 (std::make_shared (key1.pub, key1.prv, 2, send1)); - rai::transaction transaction (store.environment, nullptr, true); - auto vote_result1 (store.vote_validate (transaction, vote1)); - ASSERT_EQ (rai::vote_code::vote, vote_result1.code); - ASSERT_EQ (*vote1, *vote_result1.vote); - vote1->signature.bytes[8] ^= 1; - auto vote_result2 (store.vote_validate (transaction, vote1)); - ASSERT_EQ (rai::vote_code::invalid, vote_result2.code); - // If the signature is invalid, we don't need to take the overhead of checking the current sequence value - ASSERT_EQ (nullptr, vote_result2.vote); - auto vote2 (std::make_shared (key1.pub, key1.prv, 1, send1)); - auto vote_result3 (store.vote_validate (transaction, vote2)); - ASSERT_EQ (rai::vote_code::replay, vote_result3.code); - ASSERT_EQ (*vote1, *vote_result3.vote); -} - TEST (block_store, upgrade_v5_v6) { auto path (rai::unique_path ()); diff --git a/rai/core_test/ledger.cpp b/rai/core_test/ledger.cpp index 364f17c2..6d9f5229 100644 --- a/rai/core_test/ledger.cpp +++ b/rai/core_test/ledger.cpp @@ -731,6 +731,32 @@ TEST (ledegr, double_receive) ASSERT_EQ (rai::process_result::unreceivable, ledger.process (transaction, receive1).code); } +TEST (votes, check_signature) +{ + rai::system system (24000, 1); + auto & node1 (*system.nodes[0]); + rai::genesis genesis; + rai::keypair key1; + auto send1 (std::make_shared (genesis.hash (), key1.pub, rai::genesis_amount - 100, rai::test_genesis_key.prv, rai::test_genesis_key.pub, 0)); + { + rai::transaction transaction (node1.store.environment, nullptr, true); + ASSERT_EQ (rai::process_result::progress, node1.ledger.process (transaction, *send1).code); + } + auto node_l (system.nodes[0]); + { + rai::transaction transaction (node1.store.environment, nullptr, true); + node1.active.start (transaction, send1); + } + auto votes1 (node1.active.roots.find (send1->root ())->election); + 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); + 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); +} + TEST (votes, add_one) { rai::system system (24000, 1); @@ -819,7 +845,12 @@ TEST (votes, add_existing) rai::keypair key2; auto send2 (std::make_shared (genesis.hash (), key2.pub, 0, rai::test_genesis_key.prv, rai::test_genesis_key.pub, 0)); auto vote2 (std::make_shared (rai::test_genesis_key.pub, rai::test_genesis_key.prv, 2, send2)); + // Pretend we've waited the timeout + votes1->last_votes[rai::test_genesis_key.pub].first = std::chrono::steady_clock::now () - std::chrono::seconds (20); votes1->vote (vote2); + // Also resend the old vote, and see if we respect the sequence number + votes1->last_votes[rai::test_genesis_key.pub].first = std::chrono::steady_clock::now () - std::chrono::seconds (20); + votes1->vote (vote1); ASSERT_EQ (2, votes1->votes.rep_votes.size ()); ASSERT_NE (votes1->votes.rep_votes.end (), votes1->votes.rep_votes.find (rai::test_genesis_key.pub)); ASSERT_EQ (*send2, *votes1->votes.rep_votes[rai::test_genesis_key.pub]); @@ -851,6 +882,86 @@ TEST (votes, add_old) rai::keypair key2; auto send2 (std::make_shared (genesis.hash (), key2.pub, 0, rai::test_genesis_key.prv, rai::test_genesis_key.pub, 0)); auto vote2 (std::make_shared (rai::test_genesis_key.pub, rai::test_genesis_key.prv, 1, send2)); + votes1->last_votes[rai::test_genesis_key.pub].first = std::chrono::steady_clock::now () - std::chrono::seconds (20); + node1.vote_processor.vote (vote2, rai::endpoint ()); + ASSERT_EQ (2, votes1->votes.rep_votes.size ()); + ASSERT_NE (votes1->votes.rep_votes.end (), votes1->votes.rep_votes.find (rai::test_genesis_key.pub)); + ASSERT_EQ (*send1, *votes1->votes.rep_votes[rai::test_genesis_key.pub]); + rai::transaction transaction (system.nodes[0]->store.environment, nullptr, false); + auto winner (node1.ledger.winner (transaction, votes1->votes)); + ASSERT_EQ (*send1, *winner.second); +} + +// Lower sequence numbers are accepted for different accounts +TEST (votes, add_old_different_account) +{ + rai::system system (24000, 1); + auto & node1 (*system.nodes[0]); + rai::genesis genesis; + rai::keypair key1; + auto send1 (std::make_shared (genesis.hash (), key1.pub, 0, rai::test_genesis_key.prv, rai::test_genesis_key.pub, 0)); + auto send2 (std::make_shared (send1->hash (), key1.pub, 0, rai::test_genesis_key.prv, rai::test_genesis_key.pub, 0)); + { + rai::transaction transaction (node1.store.environment, nullptr, true); + ASSERT_EQ (rai::process_result::progress, node1.ledger.process (transaction, *send1).code); + ASSERT_EQ (rai::process_result::progress, node1.ledger.process (transaction, *send2).code); + } + auto node_l (system.nodes[0]); + { + rai::transaction transaction (node1.store.environment, nullptr, true); + node1.active.start (transaction, send1); + node1.active.start (transaction, send2); + } + auto votes1 (node1.active.roots.find (send1->root ())->election); + auto votes2 (node1.active.roots.find (send2->root ())->election); + ASSERT_EQ (1, votes1->votes.rep_votes.size ()); + 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 (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 (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)); + ASSERT_NE (votes2->votes.rep_votes.end (), votes2->votes.rep_votes.find (rai::test_genesis_key.pub)); + ASSERT_EQ (*send1, *votes1->votes.rep_votes[rai::test_genesis_key.pub]); + ASSERT_EQ (*send2, *votes2->votes.rep_votes[rai::test_genesis_key.pub]); + rai::transaction transaction (system.nodes[0]->store.environment, nullptr, false); + auto winner1 (node1.ledger.winner (transaction, votes1->votes)); + ASSERT_EQ (*send1, *winner1.second); + auto winner2 (node1.ledger.winner (transaction, votes2->votes)); + ASSERT_EQ (*send2, *winner2.second); +} + +// The voting cooldown is respected +TEST (votes, add_cooldown) +{ + rai::system system (24000, 1); + auto & node1 (*system.nodes[0]); + rai::genesis genesis; + rai::keypair key1; + auto send1 (std::make_shared (genesis.hash (), key1.pub, 0, rai::test_genesis_key.prv, rai::test_genesis_key.pub, 0)); + { + rai::transaction transaction (node1.store.environment, nullptr, true); + ASSERT_EQ (rai::process_result::progress, node1.ledger.process (transaction, *send1).code); + } + auto node_l (system.nodes[0]); + { + rai::transaction transaction (node1.store.environment, nullptr, true); + node1.active.start (transaction, send1); + } + auto votes1 (node1.active.roots.find (send1->root ())->election); + auto vote1 (std::make_shared (rai::test_genesis_key.pub, rai::test_genesis_key.prv, 1, send1)); + node1.vote_processor.vote (vote1, rai::endpoint ()); + rai::keypair key2; + auto send2 (std::make_shared (genesis.hash (), key2.pub, 0, rai::test_genesis_key.prv, rai::test_genesis_key.pub, 0)); + auto vote2 (std::make_shared (rai::test_genesis_key.pub, rai::test_genesis_key.prv, 2, send2)); node1.vote_processor.vote (vote2, rai::endpoint ()); ASSERT_EQ (2, votes1->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 fac6addb..b64901e6 100644 --- a/rai/node/node.cpp +++ b/rai/node/node.cpp @@ -222,26 +222,23 @@ void rai::network::republish_block (MDB_txn * transaction, std::shared_ptr Y to prevent creating a lot of small-weight accounts to send out votes -void rai::network::republish_vote (std::chrono::steady_clock::time_point const & last_vote, std::shared_ptr vote_a) +// 2) The rep has a weight > Y to prevent creating a lot of small-weight accounts to send out votes +// 3) Only if a vote for this block from this representative hasn't been received in the previous X second. +// This prevents rapid publishing of votes with increasing sequence numbers. +// +// These rules are implemented by the caller, not this function. +void rai::network::republish_vote (std::shared_ptr vote_a) { - if (last_vote < std::chrono::steady_clock::now () - std::chrono::seconds (1)) + rai::confirm_ack confirm (vote_a); + std::shared_ptr> bytes (new std::vector); { - if (node.weight (vote_a->account) > rai::Mxrb_ratio * 256) - { - rai::confirm_ack confirm (vote_a); - std::shared_ptr> bytes (new std::vector); - { - rai::vectorstream stream (*bytes); - confirm.serialize (stream); - } - auto list (node.peers.list_sqrt ()); - for (auto j (list.begin ()), m (list.end ()); j != m; ++j) - { - node.network.confirm_send (confirm, bytes, *j); - } - } + rai::vectorstream stream (*bytes); + confirm.serialize (stream); + } + auto list (node.peers.list_sqrt ()); + for (auto j (list.begin ()), m (list.end ()); j != m; ++j) + { + node.network.confirm_send (confirm, bytes, *j); } } @@ -373,11 +370,10 @@ public: auto vote (node.vote_processor.vote (message_a.vote, sender)); if (vote.code == rai::vote_code::replay) { - assert (vote.vote->sequence > message_a.vote->sequence); // 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) + if (vote.vote->sequence > message_a.vote->sequence + 10000) { rai::confirm_ack confirm (vote.vote); std::shared_ptr> bytes (new std::vector); @@ -1084,10 +1080,23 @@ node (node_a) rai::vote_result rai::vote_processor::vote (std::shared_ptr vote_a, rai::endpoint endpoint_a) { - rai::vote_result result; + rai::vote_result result = { rai::vote_code::invalid, vote_a }; + if (!rai::validate_message (vote_a->account, vote_a->hash (), vote_a->signature)) { - rai::transaction transaction (node.store.environment, nullptr, false); - result = node.store.vote_validate (transaction, vote_a); + result.code = rai::vote_code::replay; + std::shared_ptr newest_vote; + { + rai::transaction transaction (node.store.environment, nullptr, false); + newest_vote = node.store.vote_max (transaction, vote_a); + } + if (!node.active.vote (vote_a)) + { + result.code = rai::vote_code::vote; + } + else + { + result.vote = newest_vote; + } } if (node.config.logging.vote_logging ()) { @@ -1557,9 +1566,6 @@ block_processor_thread ([this]() { this->block_processor.process_blocks (); }) this->network.send_keepalive (endpoint_a); rep_query (*this, endpoint_a); }); - observers.vote.add ([this](std::shared_ptr vote_a, rai::endpoint const &) { - active.vote (vote_a); - }); observers.vote.add ([this](std::shared_ptr vote_a, rai::endpoint const &) { this->gap_cache.vote (vote_a); }); @@ -2791,7 +2797,6 @@ rai::election::election (MDB_txn * transaction_a, rai::node & node_a, std::share confirmation_action (confirmation_action_a), votes (block_a), node (node_a), -last_vote (std::chrono::steady_clock::now ()), last_winner (block_a) { assert (node_a.store.block_exists (transaction_a, block_a->hash ())); @@ -2893,14 +2898,59 @@ void rai::election::confirm_cutoff (MDB_txn * transaction_a) confirm_once (transaction_a); } -void rai::election::vote (std::shared_ptr vote_a) +bool rai::election::vote (std::shared_ptr vote_a) { - node.network.republish_vote (last_vote, vote_a); - last_vote = std::chrono::steady_clock::now (); + assert (!rai::validate_message (vote_a->account, vote_a->hash (), vote_a->signature)); + // see republish_vote documentation for an explanation of these rules rai::transaction transaction (node.store.environment, nullptr, true); - assert (node.store.vote_validate (transaction, vote_a).code != rai::vote_code::invalid); - votes.vote (vote_a); - confirm_if_quorum (transaction); + auto replay (false); + auto supply (node.ledger.supply (transaction)); + auto weight (node.ledger.weight (transaction, vote_a->account)); + if (rai::rai_network == rai::rai_networks::rai_test_network || weight > supply / 1000) // 0.1% or above + { + unsigned int cooldown; + if (weight < supply / 100) // 0.1% to 1% + { + cooldown = 15; + } + else if (weight < supply / 20) // 1% to 5% + { + cooldown = 5; + } + else // 5% or above + { + cooldown = 1; + } + auto should_process (false); + auto last_vote_it (last_votes.find (vote_a->account)); + if (last_vote_it == last_votes.end ()) + { + should_process = true; + } + else + { + auto last_vote (last_vote_it->second); + if (vote_a->sequence > last_vote.second) + { + if (last_vote.first <= std::chrono::steady_clock::now () - std::chrono::seconds (cooldown)) + { + should_process = true; + } + } + else + { + replay = true; + } + } + if (should_process) + { + last_votes.insert (std::make_pair (vote_a->account, std::make_pair (std::chrono::steady_clock::now (), vote_a->sequence))); + node.network.republish_vote (vote_a); + votes.vote (vote_a); + confirm_if_quorum (transaction); + } + } + return replay; } void rai::active_transactions::announce_votes () @@ -2979,7 +3029,7 @@ bool rai::active_transactions::start (MDB_txn * transaction_a, std::shared_ptr vote_a) +bool rai::active_transactions::vote (std::shared_ptr vote_a) { std::shared_ptr election; { @@ -2991,10 +3041,12 @@ void rai::active_transactions::vote (std::shared_ptr vote_a) election = existing->election; } } + auto result (false); if (election) { - election->vote (vote_a); + result = election->vote (vote_a); } + return result; } bool rai::active_transactions::active (rai::block const & block_a) diff --git a/rai/node/node.hpp b/rai/node/node.hpp index ab603b6d..7774be06 100644 --- a/rai/node/node.hpp +++ b/rai/node/node.hpp @@ -43,7 +43,7 @@ class election : public std::enable_shared_from_this public: election (MDB_txn *, rai::node &, std::shared_ptr, std::function, bool)> const &); - void vote (std::shared_ptr); + bool vote (std::shared_ptr); // Check if we have vote quorum bool have_quorum (MDB_txn *); // Tell the network our view of the winner @@ -58,7 +58,7 @@ public: rai::uint128_t minimum_threshold (MDB_txn *, rai::ledger &); rai::votes votes; rai::node & node; - std::chrono::steady_clock::time_point last_vote; + std::unordered_map> last_votes; std::shared_ptr last_winner; std::atomic_flag confirmed; }; @@ -79,7 +79,9 @@ public: // Start an election for a block // Call action with confirmed block, may be different than what we started with bool start (MDB_txn *, std::shared_ptr, std::function, bool)> const & = [](std::shared_ptr, bool) {}); - void vote (std::shared_ptr); + // If this returns true, the vote is a replay + // If this returns false, the vote may or may not be a replay + bool vote (std::shared_ptr); // Is the root of this block in the roots container bool active (rai::block const &); void announce_votes (); @@ -304,7 +306,7 @@ public: void receive_action (boost::system::error_code const &, size_t); void rpc_action (boost::system::error_code const &, size_t); void rebroadcast_reps (std::shared_ptr); - void republish_vote (std::chrono::steady_clock::time_point const &, std::shared_ptr); + void republish_vote (std::shared_ptr); void republish_block (MDB_txn *, std::shared_ptr); void republish (rai::block_hash const &, std::shared_ptr>, rai::endpoint); void publish_broadcast (std::vector &, std::unique_ptr); diff --git a/rai/slow_test/node.cpp b/rai/slow_test/node.cpp index 8b441405..0a8a771e 100644 --- a/rai/slow_test/node.cpp +++ b/rai/slow_test/node.cpp @@ -424,8 +424,7 @@ TEST (store, vote_load) auto block (std::make_shared (0, 0, 0, rai::test_genesis_key.prv, rai::test_genesis_key.pub, 0)); for (auto i (0); i < 1000000; ++i) { - rai::transaction transaction (node.store.environment, nullptr, true); auto vote (std::make_shared (rai::test_genesis_key.pub, rai::test_genesis_key.prv, i, block)); - node.store.vote_validate (transaction, vote); + node.vote_processor.vote (vote, system.nodes[0]->network.endpoint ()); } }