diff --git a/nano/core_test/block_store.cpp b/nano/core_test/block_store.cpp index 1b085c394e..191b7575fc 100644 --- a/nano/core_test/block_store.cpp +++ b/nano/core_test/block_store.cpp @@ -1630,11 +1630,8 @@ TEST (block_store, final_vote) ASSERT_EQ (store->final_vote.count (transaction), 0); store->final_vote.put (transaction, qualified_root, nano::block_hash (2)); ASSERT_EQ (store->final_vote.count (transaction), 1); - // Clearing with incorrect root shouldn't remove - store->final_vote.clear (transaction, qualified_root.previous ()); - ASSERT_EQ (store->final_vote.count (transaction), 1); // Clearing with correct root should remove - store->final_vote.clear (transaction, qualified_root.root ()); + store->final_vote.del (transaction, qualified_root); ASSERT_EQ (store->final_vote.count (transaction), 0); } } diff --git a/nano/core_test/ledger.cpp b/nano/core_test/ledger.cpp index 0cb99f9a76..18b4e44c5e 100644 --- a/nano/core_test/ledger.cpp +++ b/nano/core_test/ledger.cpp @@ -5482,8 +5482,8 @@ TEST (ledger, migrate_lmdb_to_rocksdb) ASSERT_FALSE (rocksdb_store.confirmation_height.get (rocksdb_transaction, nano::dev::genesis_key.pub, confirmation_height_info)); ASSERT_EQ (confirmation_height_info.height, 2); ASSERT_EQ (confirmation_height_info.frontier, send->hash ()); - ASSERT_EQ (rocksdb_store.final_vote.get (rocksdb_transaction, nano::root (send->previous ())).size (), 1); - ASSERT_EQ (rocksdb_store.final_vote.get (rocksdb_transaction, nano::root (send->previous ()))[0], nano::block_hash (2)); + ASSERT_TRUE (rocksdb_store.final_vote.get (rocksdb_transaction, send->qualified_root ()).has_value ()); + ASSERT_EQ (rocksdb_store.final_vote.get (rocksdb_transaction, send->qualified_root ()).value (), nano::block_hash (2)); // Retry migration while rocksdb folder is still present auto error_on_retry = ledger.migrate_lmdb_to_rocksdb (path); diff --git a/nano/core_test/node.cpp b/nano/core_test/node.cpp index d441b626f4..7a50183cad 100644 --- a/nano/core_test/node.cpp +++ b/nano/core_test/node.cpp @@ -2157,6 +2157,13 @@ TEST (node, epoch_conflict_confirm) nano::keypair key; nano::keypair epoch_signer (nano::dev::genesis_key); nano::state_block_builder builder; + + // Node 1 is the voting node + // Send sends to an account we control: send -> open -> change + // Send2 sends to an account with public key of the open block + // Epoch open qualified root: (open, 0) on account with the same public key as the hash of the open block + // Epoch open and change have the same root! + auto send = builder.make_block () .account (nano::dev::genesis_key.pub) .previous (nano::dev::genesis->hash ()) @@ -2203,34 +2210,30 @@ TEST (node, epoch_conflict_confirm) .work (*system.work.generate (open->hash ())) .build (); - // Process initial blocks on node1 - ASSERT_TRUE (nano::test::process (node1, { send, send2, open })); - - // Confirm open block in node1 to allow generating votes - nano::test::confirm (node1.ledger, open); - - // Process initial blocks on node0 - ASSERT_TRUE (nano::test::process (node0, { send, send2, open })); + // Process initial blocks + ASSERT_TRUE (nano::test::process (node0, nano::test::clone ({ send, send2, open }))); + ASSERT_TRUE (nano::test::process (node1, nano::test::clone ({ send, send2, open }))); - // Process conflicting blocks on node 0 as blocks coming from live network - ASSERT_TRUE (nano::test::process_live (node0, { change, epoch_open })); + // Process conflicting blocks on nodes as blocks coming from live network + ASSERT_TRUE (nano::test::process_live (node0, nano::test::clone ({ change, epoch_open }))); + ASSERT_TRUE (nano::test::process_live (node1, nano::test::clone ({ change, epoch_open }))); // Ensure blocks were propagated to both nodes ASSERT_TIMELY (5s, nano::test::exists (node0, { change, epoch_open })); ASSERT_TIMELY (5s, nano::test::exists (node1, { change, epoch_open })); // Confirm initial blocks in node1 to allow generating votes later - ASSERT_TRUE (nano::test::start_elections (system, node1, { change, epoch_open, send2 }, true)); + nano::test::confirm (node1, { change, epoch_open, send2 }); ASSERT_TIMELY (5s, nano::test::confirmed (node1, { change, epoch_open, send2 })); - // Start elections for node0 for conflicting change and epoch_open blocks (those two blocks have the same root) + // Start elections on node0 for conflicting change and epoch_open blocks (these two blocks have the same root) ASSERT_TRUE (nano::test::activate (node0, { change, epoch_open })); ASSERT_TIMELY (5s, nano::test::active (node0, { change, epoch_open })); - // Make node1 a representative + // Make node1 a representative so it can vote for both blocks system.wallet (1)->insert_adhoc (nano::dev::genesis_key.prv); - // Ensure the elections for conflicting blocks have completed + // Ensure the elections for conflicting blocks have started ASSERT_TIMELY (5s, nano::test::active (node0, { change, epoch_open })); // Ensure both conflicting blocks were successfully processed and confirmed diff --git a/nano/lib/blocks.cpp b/nano/lib/blocks.cpp index d61233e4a1..d7d3cb8663 100644 --- a/nano/lib/blocks.cpp +++ b/nano/lib/blocks.cpp @@ -210,7 +210,7 @@ nano::block_hash nano::block::full_hash () const nano::block_sideband const & nano::block::sideband () const { - debug_assert (sideband_m.is_initialized ()); + release_assert (sideband_m.is_initialized ()); return *sideband_m; } @@ -569,6 +569,11 @@ nano::send_block::send_block (bool & error_a, boost::property_tree::ptree const } } +std::shared_ptr nano::send_block::clone () const +{ + return std::make_shared (*this); +} + bool nano::send_block::operator== (nano::block const & other_a) const { return blocks_equal (*this, other_a); @@ -758,6 +763,11 @@ nano::open_block::open_block (bool & error_a, boost::property_tree::ptree const } } +std::shared_ptr nano::open_block::clone () const +{ + return std::make_shared (*this); +} + void nano::open_block::generate_hash (blake2b_state & hash_a) const { hashables.hash (hash_a); @@ -1029,6 +1039,11 @@ nano::change_block::change_block (bool & error_a, boost::property_tree::ptree co } } +std::shared_ptr nano::change_block::clone () const +{ + return std::make_shared (*this); +} + void nano::change_block::generate_hash (blake2b_state & hash_a) const { hashables.hash (hash_a); @@ -1326,6 +1341,11 @@ nano::state_block::state_block (bool & error_a, boost::property_tree::ptree cons } } +std::shared_ptr nano::state_block::clone () const +{ + return std::make_shared (*this); +} + void nano::state_block::generate_hash (blake2b_state & hash_a) const { nano::uint256_union preamble (static_cast (nano::block_type::state)); @@ -1792,6 +1812,11 @@ nano::receive_block::receive_block (bool & error_a, boost::property_tree::ptree } } +std::shared_ptr nano::receive_block::clone () const +{ + return std::make_shared (*this); +} + void nano::receive_block::generate_hash (blake2b_state & hash_a) const { hashables.hash (hash_a); diff --git a/nano/lib/blocks.hpp b/nano/lib/blocks.hpp index 14524e556d..5bdb0a4af0 100644 --- a/nano/lib/blocks.hpp +++ b/nano/lib/blocks.hpp @@ -23,6 +23,7 @@ namespace nano class block { public: + virtual ~block () = default; // Return a digest of the hashables in this block. nano::block_hash const & hash () const; // Return a digest of hashables and non-hashables in this block. @@ -46,10 +47,10 @@ class block virtual nano::block_type type () const = 0; virtual nano::signature const & block_signature () const = 0; virtual void signature_set (nano::signature const &) = 0; - virtual ~block () = default; virtual bool valid_predecessor (nano::block const &) const = 0; static size_t size (nano::block_type); virtual nano::work_version work_version () const; + virtual std::shared_ptr clone () const = 0; // If there are any changes to the hashables, call this to update the cached hash void refresh (); bool is_send () const noexcept; @@ -138,6 +139,7 @@ class send_block : public nano::block bool operator== (nano::block const &) const override; bool operator== (nano::send_block const &) const; bool valid_predecessor (nano::block const &) const override; + std::shared_ptr clone () const override; send_hashables hashables; nano::signature signature; uint64_t work; @@ -192,6 +194,7 @@ class receive_block : public nano::block bool operator== (nano::block const &) const override; bool operator== (nano::receive_block const &) const; bool valid_predecessor (nano::block const &) const override; + std::shared_ptr clone () const override; receive_hashables hashables; nano::signature signature; uint64_t work; @@ -247,6 +250,7 @@ class open_block : public nano::block bool operator== (nano::block const &) const override; bool operator== (nano::open_block const &) const; bool valid_predecessor (nano::block const &) const override; + std::shared_ptr clone () const override; nano::open_hashables hashables; nano::signature signature; uint64_t work; @@ -302,6 +306,7 @@ class change_block : public nano::block bool operator== (nano::block const &) const override; bool operator== (nano::change_block const &) const; bool valid_predecessor (nano::block const &) const override; + std::shared_ptr clone () const override; nano::change_hashables hashables; nano::signature signature; uint64_t work; @@ -368,6 +373,7 @@ class state_block : public nano::block bool operator== (nano::block const &) const override; bool operator== (nano::state_block const &) const; bool valid_predecessor (nano::block const &) const override; + std::shared_ptr clone () const override; nano::state_hashables hashables; nano::signature signature; uint64_t work; diff --git a/nano/node/cli.cpp b/nano/node/cli.cpp index 6128a1c3e5..e0622e940a 100644 --- a/nano/node/cli.cpp +++ b/nano/node/cli.cpp @@ -646,10 +646,10 @@ std::error_code nano::handle_node_options (boost::program_options::variables_map { auto root_str = root_it->second.as (); auto transaction (node.node->store.tx_begin_write ()); - nano::root root; + nano::qualified_root root; if (!root.decode_hex (root_str)) { - node.node->store.final_vote.clear (transaction, root); + node.node->store.final_vote.del (transaction, root); std::cout << "Successfully cleared final votes" << std::endl; } else diff --git a/nano/node/request_aggregator.cpp b/nano/node/request_aggregator.cpp index 75faab069d..3311978d22 100644 --- a/nano/node/request_aggregator.cpp +++ b/nano/node/request_aggregator.cpp @@ -12,6 +12,7 @@ #include #include #include +#include #include nano::request_aggregator::request_aggregator (request_aggregator_config const & config_a, nano::node & node_a, nano::stats & stats_a, nano::vote_generator & generator_a, nano::vote_generator & final_generator_a, nano::local_vote_history & history_a, nano::ledger & ledger_a, nano::wallets & wallets_a, nano::vote_router & vote_router_a) : @@ -208,68 +209,45 @@ void nano::request_aggregator::erase_duplicates (std::vector const & channel_a) const -> aggregate_result { std::vector> to_generate; std::vector> to_generate_final; for (auto const & [hash, root] : requests_a) { - bool generate_final_vote (false); - std::shared_ptr block; + // Ledger by hash + std::shared_ptr block = ledger.any.block_get (transaction, hash); - // 2. Final votes - auto final_vote_hashes (ledger.store.final_vote.get (transaction, root)); - if (!final_vote_hashes.empty ()) + // Ledger by root + if (!block && !root.is_zero ()) { - generate_final_vote = true; - block = ledger.any.block_get (transaction, final_vote_hashes[0]); - // Allow same root vote - if (block != nullptr && final_vote_hashes.size () > 1) + // Search for block root + if (auto successor = ledger.any.block_successor (transaction, root.as_block_hash ())) { - // WTF? This shouldn't be done like this - to_generate_final.push_back (block); - block = ledger.any.block_get (transaction, final_vote_hashes[1]); - debug_assert (final_vote_hashes.size () == 2); + block = ledger.any.block_get (transaction, successor.value ()); + release_assert (block); } } - // 4. Ledger by hash - if (block == nullptr) - { - block = ledger.any.block_get (transaction, hash); - // Confirmation status. Generate final votes for confirmed - if (block != nullptr) + auto should_generate_final_vote = [&] (auto const & block) { + release_assert (block); + + // Check if final vote is set for this block + if (auto final_hash = ledger.store.final_vote.get (transaction, block->qualified_root ())) { - nano::confirmation_height_info confirmation_height_info; - ledger.store.confirmation_height.get (transaction, block->account (), confirmation_height_info); - generate_final_vote = (confirmation_height_info.height >= block->sideband ().height); + return final_hash == block->hash (); } - } - - // 5. Ledger by root - if (block == nullptr && !root.is_zero ()) - { - // Search for block root - auto successor = ledger.any.block_successor (transaction, root.as_block_hash ()); - if (successor) + // If the final vote is not set, generate vote if the block is confirmed + else { - auto successor_block = ledger.any.block_get (transaction, successor.value ()); - release_assert (successor_block != nullptr); - block = std::move (successor_block); - - // Confirmation status. Generate final votes for confirmed successor - if (block != nullptr) - { - nano::confirmation_height_info confirmation_height_info; - ledger.store.confirmation_height.get (transaction, block->account (), confirmation_height_info); - generate_final_vote = (confirmation_height_info.height >= block->sideband ().height); - } + return ledger.confirmed.block_exists (transaction, block->hash ()); } - } + }; if (block) { - if (generate_final_vote) + if (should_generate_final_vote (block)) { to_generate_final.push_back (block); stats.inc (nano::stat::type::requests, nano::stat::detail::requests_final); diff --git a/nano/store/final_vote.hpp b/nano/store/final_vote.hpp index f6f96f9796..8e692d4045 100644 --- a/nano/store/final_vote.hpp +++ b/nano/store/final_vote.hpp @@ -22,10 +22,9 @@ class final_vote public: virtual bool put (store::write_transaction const & transaction_a, nano::qualified_root const & root_a, nano::block_hash const & hash_a) = 0; - virtual std::vector get (store::transaction const & transaction_a, nano::root const & root_a) = 0; - virtual void del (store::write_transaction const & transaction_a, nano::root const & root_a) = 0; + virtual std::optional get (store::transaction const & transaction_a, nano::qualified_root const & qualified_root_a) = 0; + virtual void del (store::write_transaction const & transaction_a, nano::qualified_root const & root_a) = 0; virtual size_t count (store::transaction const & transaction_a) const = 0; - virtual void clear (store::write_transaction const &, nano::root const &) = 0; virtual void clear (store::write_transaction const &) = 0; virtual iterator begin (store::transaction const & transaction_a, nano::qualified_root const & root_a) const = 0; virtual iterator begin (store::transaction const & transaction_a) const = 0; diff --git a/nano/store/lmdb/final_vote.cpp b/nano/store/lmdb/final_vote.cpp index c7fdf7acad..547dd600b6 100644 --- a/nano/store/lmdb/final_vote.cpp +++ b/nano/store/lmdb/final_vote.cpp @@ -23,30 +23,22 @@ bool nano::store::lmdb::final_vote::put (store::write_transaction const & transa return result; } -std::vector nano::store::lmdb::final_vote::get (store::transaction const & transaction, nano::root const & root_a) +std::optional nano::store::lmdb::final_vote::get (store::transaction const & transaction, nano::qualified_root const & qualified_root_a) { - std::vector result; - nano::qualified_root key_start{ root_a.raw, 0 }; - for (auto i = begin (transaction, key_start), n = end (transaction); i != n && nano::qualified_root{ i->first }.root () == root_a; ++i) + nano::store::lmdb::db_val result; + auto status = store.get (transaction, tables::final_votes, qualified_root_a, result); + std::optional final_vote_hash; + if (store.success (status)) { - result.push_back (i->second); + final_vote_hash = static_cast (result); } - return result; + return final_vote_hash; } -void nano::store::lmdb::final_vote::del (store::write_transaction const & transaction, nano::root const & root) +void nano::store::lmdb::final_vote::del (store::write_transaction const & transaction, nano::qualified_root const & root) { - std::vector final_vote_qualified_roots; - for (auto i = begin (transaction, nano::qualified_root{ root.raw, 0 }), n = end (transaction); i != n && nano::qualified_root{ i->first }.root () == root; ++i) - { - final_vote_qualified_roots.push_back (i->first); - } - - for (auto & final_vote_qualified_root : final_vote_qualified_roots) - { - auto status = store.del (transaction, tables::final_votes, final_vote_qualified_root); - store.release_assert_success (status); - } + auto status = store.del (transaction, tables::final_votes, root); + store.release_assert_success (status); } size_t nano::store::lmdb::final_vote::count (store::transaction const & transaction_a) const @@ -54,11 +46,6 @@ size_t nano::store::lmdb::final_vote::count (store::transaction const & transact return store.count (transaction_a, tables::final_votes); } -void nano::store::lmdb::final_vote::clear (store::write_transaction const & transaction_a, nano::root const & root_a) -{ - del (transaction_a, root_a); -} - void nano::store::lmdb::final_vote::clear (store::write_transaction const & transaction_a) { store.drop (transaction_a, nano::tables::final_votes); diff --git a/nano/store/lmdb/final_vote.hpp b/nano/store/lmdb/final_vote.hpp index c905edc9dd..193cb6e717 100644 --- a/nano/store/lmdb/final_vote.hpp +++ b/nano/store/lmdb/final_vote.hpp @@ -18,10 +18,9 @@ class final_vote : public nano::store::final_vote public: explicit final_vote (nano::store::lmdb::component & store); bool put (store::write_transaction const & transaction_a, nano::qualified_root const & root_a, nano::block_hash const & hash_a) override; - std::vector get (store::transaction const & transaction_a, nano::root const & root_a) override; - void del (store::write_transaction const & transaction_a, nano::root const & root_a) override; + std::optional get (store::transaction const & transaction_a, nano::qualified_root const & qualified_root_a) override; + void del (store::write_transaction const & transaction_a, nano::qualified_root const & root_a) override; size_t count (store::transaction const & transaction_a) const override; - void clear (store::write_transaction const & transaction_a, nano::root const & root_a) override; void clear (store::write_transaction const & transaction_a) override; iterator begin (store::transaction const & transaction_a, nano::qualified_root const & root_a) const override; iterator begin (store::transaction const & transaction_a) const override; diff --git a/nano/store/rocksdb/final_vote.cpp b/nano/store/rocksdb/final_vote.cpp index 52afdfe34e..0875c11b7b 100644 --- a/nano/store/rocksdb/final_vote.cpp +++ b/nano/store/rocksdb/final_vote.cpp @@ -24,30 +24,22 @@ bool nano::store::rocksdb::final_vote::put (store::write_transaction const & tra return result; } -std::vector nano::store::rocksdb::final_vote::get (store::transaction const & transaction, nano::root const & root_a) +std::optional nano::store::rocksdb::final_vote::get (store::transaction const & transaction, nano::qualified_root const & qualified_root_a) { - std::vector result; - nano::qualified_root key_start{ root_a.raw, 0 }; - for (auto i = begin (transaction, key_start), n = end (transaction); i != n && nano::qualified_root{ i->first }.root () == root_a; ++i) + nano::store::rocksdb::db_val result; + auto status = store.get (transaction, tables::final_votes, qualified_root_a, result); + std::optional final_vote_hash; + if (store.success (status)) { - result.push_back (i->second); + final_vote_hash = static_cast (result); } - return result; + return final_vote_hash; } -void nano::store::rocksdb::final_vote::del (store::write_transaction const & transaction, nano::root const & root) +void nano::store::rocksdb::final_vote::del (store::write_transaction const & transaction, nano::qualified_root const & root) { - std::vector final_vote_qualified_roots; - for (auto i = begin (transaction, nano::qualified_root{ root.raw, 0 }), n = end (transaction); i != n && nano::qualified_root{ i->first }.root () == root; ++i) - { - final_vote_qualified_roots.push_back (i->first); - } - - for (auto & final_vote_qualified_root : final_vote_qualified_roots) - { - auto status = store.del (transaction, tables::final_votes, final_vote_qualified_root); - store.release_assert_success (status); - } + auto status = store.del (transaction, tables::final_votes, root); + store.release_assert_success (status); } size_t nano::store::rocksdb::final_vote::count (store::transaction const & transaction_a) const @@ -55,11 +47,6 @@ size_t nano::store::rocksdb::final_vote::count (store::transaction const & trans return store.count (transaction_a, tables::final_votes); } -void nano::store::rocksdb::final_vote::clear (store::write_transaction const & transaction_a, nano::root const & root_a) -{ - del (transaction_a, root_a); -} - void nano::store::rocksdb::final_vote::clear (store::write_transaction const & transaction_a) { store.drop (transaction_a, nano::tables::final_votes); diff --git a/nano/store/rocksdb/final_vote.hpp b/nano/store/rocksdb/final_vote.hpp index 67a1e0d52b..04b470fede 100644 --- a/nano/store/rocksdb/final_vote.hpp +++ b/nano/store/rocksdb/final_vote.hpp @@ -16,10 +16,9 @@ class final_vote : public nano::store::final_vote public: explicit final_vote (nano::store::rocksdb::component & store); bool put (store::write_transaction const & transaction_a, nano::qualified_root const & root_a, nano::block_hash const & hash_a) override; - std::vector get (store::transaction const & transaction_a, nano::root const & root_a) override; - void del (store::write_transaction const & transaction_a, nano::root const & root_a) override; + std::optional get (store::transaction const & transaction_a, nano::qualified_root const & qualified_root_a) override; + void del (store::write_transaction const & transaction_a, nano::qualified_root const & root_a) override; size_t count (store::transaction const & transaction_a) const override; - void clear (store::write_transaction const & transaction_a, nano::root const & root_a) override; void clear (store::write_transaction const & transaction_a) override; iterator begin (store::transaction const & transaction_a, nano::qualified_root const & root_a) const override; iterator begin (store::transaction const & transaction_a) const override; diff --git a/nano/test_common/testutil.cpp b/nano/test_common/testutil.cpp index 8218fd22c4..75a72baae3 100644 --- a/nano/test_common/testutil.cpp +++ b/nano/test_common/testutil.cpp @@ -124,6 +124,11 @@ bool nano::test::exists (nano::node & node, std::vector> const blocks) +{ + confirm (node.ledger, blocks); +} + void nano::test::confirm (nano::ledger & ledger, std::vector> const blocks) { for (auto const block : blocks) @@ -237,6 +242,13 @@ std::vector nano::test::blocks_to_hashes (std::vector> nano::test::clone (std::vector> blocks) +{ + std::vector> clones; + std::transform (blocks.begin (), blocks.end (), std::back_inserter (clones), [] (auto & block) { return block->clone (); }); + return clones; +} + std::shared_ptr nano::test::fake_channel (nano::node & node, nano::account node_id) { auto channel = std::make_shared (node); diff --git a/nano/test_common/testutil.hpp b/nano/test_common/testutil.hpp index 017d616550..a30925131c 100644 --- a/nano/test_common/testutil.hpp +++ b/nano/test_common/testutil.hpp @@ -329,6 +329,7 @@ namespace test void confirm (nano::ledger & ledger, std::vector> const blocks); void confirm (nano::ledger & ledger, std::shared_ptr const block); void confirm (nano::ledger & ledger, nano::block_hash const & hash); + void confirm (nano::node & node, std::vector> const blocks); /* * Convenience function to check whether *all* of the hashes exists in node ledger or in the pruned table. * @return true if all blocks are fully processed and inserted in the ledger, false otherwise @@ -389,6 +390,10 @@ namespace test * Converts list of blocks to list of hashes */ std::vector blocks_to_hashes (std::vector> blocks); + /* + * Clones list of blocks + */ + std::vector> clone (std::vector> blocks); /* * Creates a new fake channel associated with `node` */