diff --git a/nano/core_test/node.cpp b/nano/core_test/node.cpp index f0e7bad8..22331010 100644 --- a/nano/core_test/node.cpp +++ b/nano/core_test/node.cpp @@ -2985,38 +2985,66 @@ TEST (node, fork_invalid_block_signature) ASSERT_EQ (node1.block (send2->hash ())->block_signature (), send2->block_signature ()); } -TEST (node, fork_invalid_block_signature_vote_by_hash) +TEST (node, fork_election_invalid_block_signature) { nano::system system (1); auto & node1 (*system.nodes[0]); - nano::keypair key2; nano::genesis genesis; - auto send1 (std::make_shared (genesis.hash (), key2.pub, std::numeric_limits::max () - node1.config.receive_minimum.number (), nano::test_genesis_key.prv, nano::test_genesis_key.pub, *system.work.generate (genesis.hash ()))); - auto send2 (std::make_shared (genesis.hash (), key2.pub, std::numeric_limits::max () - node1.config.receive_minimum.number () * 2, nano::test_genesis_key.prv, nano::test_genesis_key.pub, *system.work.generate (genesis.hash ()))); - auto send2_corrupt (std::make_shared (*send2)); - send2_corrupt->signature = nano::signature (123); - node1.process_active (send1); + nano::block_builder builder; + std::shared_ptr send1 = builder.state () + .account (nano::test_genesis_key.pub) + .previous (genesis.hash ()) + .representative (nano::test_genesis_key.pub) + .balance (nano::genesis_amount - nano::Gxrb_ratio) + .link (nano::test_genesis_key.pub) + .work (*system.work.generate (genesis.hash ())) + .sign (nano::test_genesis_key.prv, nano::test_genesis_key.pub) + .build (); + std::shared_ptr send2 = builder.state () + .account (nano::test_genesis_key.pub) + .previous (genesis.hash ()) + .representative (nano::test_genesis_key.pub) + .balance (nano::genesis_amount - 2 * nano::Gxrb_ratio) + .link (nano::test_genesis_key.pub) + .work (*system.work.generate (genesis.hash ())) + .sign (nano::test_genesis_key.prv, nano::test_genesis_key.pub) + .build (); + std::shared_ptr send3 = builder.state () + .account (nano::test_genesis_key.pub) + .previous (genesis.hash ()) + .representative (nano::test_genesis_key.pub) + .balance (nano::genesis_amount - 2 * nano::Gxrb_ratio) + .link (nano::test_genesis_key.pub) + .work (*system.work.generate (genesis.hash ())) + .sign (nano::test_genesis_key.prv, 0) // Invalid signature + .build (); + auto channel1 (node1.network.udp_channels.create (node1.network.endpoint ())); + node1.network.process_message (nano::publish (send1), channel1); system.deadline_set (5s); - while (!node1.block (send1->hash ())) + std::shared_ptr election; + while (election == nullptr) { ASSERT_NO_ERROR (system.poll ()); + nano::lock_guard lock (node1.active.mutex); + auto existing = node1.active.blocks.find (send1->hash ()); + if (existing != node1.active.blocks.end ()) + { + election = existing->second; + } } - node1.active.publish (send2_corrupt); - ASSERT_NO_ERROR (system.poll ()); - node1.active.publish (send2); - std::vector vote_blocks; - vote_blocks.push_back (send2->hash ()); - auto vote (std::make_shared (nano::test_genesis_key.pub, nano::test_genesis_key.prv, 0, vote_blocks)); - node1.vote_processor.vote_blocking (vote, std::make_shared (node1.network.udp_channels, node1.network.endpoint (), node1.network_params.protocol.protocol_version)); - while (node1.block (send1->hash ())) + nano::unique_lock lock (node1.active.mutex); + ASSERT_EQ (1, election->blocks.size ()); + lock.unlock (); + node1.network.process_message (nano::publish (send3), channel1); + node1.network.process_message (nano::publish (send2), channel1); + lock.lock (); + while (election->blocks.size () == 1) { + lock.unlock (); ASSERT_NO_ERROR (system.poll ()); + lock.lock (); } - while (!node1.block (send2->hash ())) - { - ASSERT_NO_ERROR (system.poll ()); - } - ASSERT_EQ (node1.block (send2->hash ())->block_signature (), send2->block_signature ()); + ASSERT_EQ (election->blocks[send2->hash ()]->block_signature (), send2->block_signature ()); } TEST (node, block_processor_signatures) diff --git a/nano/node/election.cpp b/nano/node/election.cpp index 4a36c4e8..028b579e 100644 --- a/nano/node/election.cpp +++ b/nano/node/election.cpp @@ -211,21 +211,16 @@ bool nano::election::publish (std::shared_ptr block_a) } if (!result) { - auto transaction (node.store.tx_begin_read ()); - result = node.validate_block_by_previous (transaction, block_a); - if (!result) + if (blocks.find (block_a->hash ()) == blocks.end ()) { - if (blocks.find (block_a->hash ()) == blocks.end ()) - { - blocks.emplace (block_a->hash (), block_a); - insert_inactive_votes_cache (block_a->hash ()); - confirm_if_quorum (); - node.network.flood_block (block_a, false); - } - else - { - result = true; - } + blocks.emplace (std::make_pair (block_a->hash (), block_a)); + insert_inactive_votes_cache (block_a->hash ()); + confirm_if_quorum (); + node.network.flood_block (block_a, false); + } + else + { + result = true; } } return result; diff --git a/nano/node/network.cpp b/nano/node/network.cpp index 4b82b31b..149fa847 100644 --- a/nano/node/network.cpp +++ b/nano/node/network.cpp @@ -423,7 +423,6 @@ public: { node.stats.inc (nano::stat::type::drop, nano::stat::detail::publish, nano::stat::dir::in); } - node.active.publish (message_a.block); } void confirm_req (nano::confirm_req const & message_a) override { @@ -482,7 +481,6 @@ public: { node.stats.inc (nano::stat::type::drop, nano::stat::detail::confirm_ack, nano::stat::dir::in); } - node.active.publish (block); } } node.vote_processor.vote (message_a.vote, channel); diff --git a/nano/node/node.cpp b/nano/node/node.cpp index 9df58722..9f8c798d 100644 --- a/nano/node/node.cpp +++ b/nano/node/node.cpp @@ -541,6 +541,7 @@ void nano::node::process_fork (nano::transaction const & transaction_a, std::sha auto root (block_a->root ()); if (!store.block_exists (transaction_a, block_a->type (), block_a->hash ()) && store.root_exists (transaction_a, block_a->root ())) { + active.publish (block_a); std::shared_ptr ledger_block (ledger.forked_block (transaction_a, *block_a)); if (ledger_block && !block_confirmed_or_being_confirmed (transaction_a, ledger_block->hash ())) { @@ -1294,55 +1295,6 @@ std::shared_ptr nano::node::shared () return shared_from_this (); } -bool nano::node::validate_block_by_previous (nano::transaction const & transaction, std::shared_ptr block_a) -{ - bool result (false); - nano::root account; - if (!block_a->previous ().is_zero ()) - { - if (store.block_exists (transaction, block_a->previous ())) - { - account = ledger.account (transaction, block_a->previous ()); - } - else - { - result = true; - } - } - else - { - account = block_a->root (); - } - if (!result && block_a->type () == nano::block_type::state) - { - std::shared_ptr block_l (std::static_pointer_cast (block_a)); - nano::amount prev_balance (0); - if (!block_l->hashables.previous.is_zero ()) - { - if (store.block_exists (transaction, block_l->hashables.previous)) - { - prev_balance = ledger.balance (transaction, block_l->hashables.previous); - } - else - { - result = true; - } - } - if (!result) - { - if (block_l->hashables.balance == prev_balance && ledger.is_epoch_link (block_l->hashables.link)) - { - account = ledger.epoch_signer (block_l->link ()); - } - } - } - if (!result && (account.is_zero () || nano::validate_message (account, block_a->hash (), block_a->block_signature ()))) - { - result = true; - } - return result; -} - int nano::node::store_version () { auto transaction (store.tx_begin_read ()); diff --git a/nano/node/node.hpp b/nano/node/node.hpp index 6cf68be7..41e674f4 100644 --- a/nano/node/node.hpp +++ b/nano/node/node.hpp @@ -139,7 +139,6 @@ public: void block_confirm (std::shared_ptr); bool block_confirmed_or_being_confirmed (nano::transaction const &, nano::block_hash const &); void process_fork (nano::transaction const &, std::shared_ptr); - bool validate_block_by_previous (nano::transaction const &, std::shared_ptr); void do_rpc_callback (boost::asio::ip::tcp::resolver::iterator i_a, std::string const &, uint16_t, std::shared_ptr, std::shared_ptr, std::shared_ptr); nano::uint128_t delta () const; void ongoing_online_weight_calculation ();