Skip to content

Commit

Permalink
Remove support for confirm req with block payload
Browse files Browse the repository at this point in the history
  • Loading branch information
@pwojcikdev
pwojcikdev committed Nov 9, 2023
1 parent 39814e3 commit 0e72333
Show file tree
Hide file tree
Showing 7 changed files with 34 additions and 139 deletions.
31 changes: 0 additions & 31 deletions nano/core_test/message.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -153,35 +153,6 @@ TEST (message, confirm_ack_hash_serialization)
ASSERT_EQ (hashes, con2.vote->hashes);
// Check overflow with max hashes
ASSERT_EQ (header.count_get (), hashes.size ());
ASSERT_EQ (header.block_type (), nano::block_type::not_a_block);
}

TEST (message, confirm_req_serialization)
{
nano::keypair key1;
nano::keypair key2;
nano::block_builder builder;
auto block = builder
.send ()
.previous (0)
.destination (key2.pub)
.balance (200)
.sign (nano::keypair ().prv, 2)
.work (3)
.build_shared ();
nano::confirm_req req{ nano::dev::network_params.network, block };
std::vector<uint8_t> bytes;
{
nano::vectorstream stream (bytes);
req.serialize (stream);
}
auto error (false);
nano::bufferstream stream2 (bytes.data (), bytes.size ());
nano::message_header header (error, stream2);
nano::confirm_req req2 (error, stream2, header);
ASSERT_FALSE (error);
ASSERT_EQ (req, req2);
ASSERT_EQ (*req.block, *req2.block);
}

TEST (message, confirm_req_hash_serialization)
Expand Down Expand Up @@ -210,7 +181,6 @@ TEST (message, confirm_req_hash_serialization)
ASSERT_FALSE (error);
ASSERT_EQ (req, req2);
ASSERT_EQ (req.roots_hashes, req2.roots_hashes);
ASSERT_EQ (header.block_type (), nano::block_type::not_a_block);
ASSERT_EQ (header.count_get (), req.roots_hashes.size ());
}

Expand Down Expand Up @@ -264,7 +234,6 @@ TEST (message, confirm_req_hash_batch_serialization)
ASSERT_EQ (req.roots_hashes, req2.roots_hashes);
ASSERT_EQ (req.roots_hashes, roots_hashes);
ASSERT_EQ (req2.roots_hashes, roots_hashes);
ASSERT_EQ (header.block_type (), nano::block_type::not_a_block);
ASSERT_EQ (header.count_get (), req.roots_hashes.size ());
}

Expand Down
17 changes: 0 additions & 17 deletions nano/core_test/message_deserializer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,23 +74,6 @@ TEST (message_deserializer, exact_confirm_ack)
message_deserializer_success_checker<decltype (message)> (message);
}

TEST (message_deserializer, exact_confirm_req)
{
nano::test::system system{ 1 };
nano::block_builder builder;
auto block = builder
.send ()
.previous (1)
.destination (1)
.balance (2)
.sign (nano::keypair ().prv, 4)
.work (*system.work.generate (nano::root (1)))
.build_shared ();
nano::confirm_req message{ nano::dev::network_params.network, block };

message_deserializer_success_checker<decltype (message)> (message);
}

TEST (message_deserializer, exact_confirm_req_hash)
{
nano::test::system system{ 1 };
Expand Down
8 changes: 4 additions & 4 deletions nano/core_test/node.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2277,8 +2277,8 @@ TEST (node, local_votes_cache)
election->force_confirm ();
ASSERT_TIMELY (3s, node.ledger.cache.cemented_count == 3);
system.wallet (0)->insert_adhoc (nano::dev::genesis_key.prv);
nano::confirm_req message1{ nano::dev::network_params.network, send1 };
nano::confirm_req message2{ nano::dev::network_params.network, send2 };
nano::confirm_req message1{ nano::dev::network_params.network, send1->hash (), send1->root () };
nano::confirm_req message2{ nano::dev::network_params.network, send2->hash (), send2->root () };
auto channel = std::make_shared<nano::transport::fake::channel> (node);
node.network.inbound (message1, channel);
ASSERT_TIMELY (3s, node.stats.count (nano::stat::type::requests, nano::stat::detail::requests_generated_votes) == 1);
Expand All @@ -2300,7 +2300,7 @@ TEST (node, local_votes_cache)
auto transaction (node.store.tx_begin_write ());
ASSERT_EQ (nano::process_result::progress, node.ledger.process (transaction, *send3).code);
}
nano::confirm_req message3{ nano::dev::network_params.network, send3 };
nano::confirm_req message3{ nano::dev::network_params.network, send3->hash (), send3->root () };
for (auto i (0); i < 100; ++i)
{
node.network.inbound (message3, channel);
Expand Down Expand Up @@ -2400,7 +2400,7 @@ TEST (node, local_votes_cache_generate_new_vote)
system.wallet (0)->insert_adhoc (nano::dev::genesis_key.prv);

// Send a confirm req for genesis block to node
nano::confirm_req message1{ nano::dev::network_params.network, nano::dev::genesis };
nano::confirm_req message1{ nano::dev::network_params.network, nano::dev::genesis->hash (), nano::dev::genesis->root () };
auto channel = std::make_shared<nano::transport::fake::channel> (node);
node.network.inbound (message1, channel);

Expand Down
96 changes: 28 additions & 68 deletions nano/node/messages.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -183,11 +183,15 @@ std::string nano::message_header::to_string () const

nano::block_type nano::message_header::block_type () const
{
debug_assert (type == nano::message_type::publish);

return static_cast<nano::block_type> (((extensions & block_type_mask) >> 8).to_ullong ());
}

void nano::message_header::block_type_set (nano::block_type type_a)
{
debug_assert (type == nano::message_type::publish);

extensions &= ~block_type_mask;
extensions |= std::bitset<16> (static_cast<unsigned long long> (type_a) << 8);
}
Expand Down Expand Up @@ -560,22 +564,13 @@ nano::confirm_req::confirm_req (bool & error_a, nano::stream & stream_a, nano::m
}
}

nano::confirm_req::confirm_req (nano::network_constants const & constants, std::shared_ptr<nano::block> const & block_a) :
message (constants, nano::message_type::confirm_req),
block (block_a)
{
header.block_type_set (block->type ());
}

nano::confirm_req::confirm_req (nano::network_constants const & constants, std::vector<std::pair<nano::block_hash, nano::root>> const & roots_hashes_a) :
message (constants, nano::message_type::confirm_req),
roots_hashes (roots_hashes_a)
{
debug_assert (!roots_hashes.empty ());
debug_assert (roots_hashes.size () <= nano::vote::max_hashes);

// not_a_block (1) block type for hashes + roots request
header.block_type_set (nano::block_type::not_a_block);
header.count_set (static_cast<uint8_t> (roots_hashes.size ()));
}

Expand All @@ -591,52 +586,39 @@ void nano::confirm_req::visit (nano::message_visitor & visitor_a) const

void nano::confirm_req::serialize (nano::stream & stream_a) const
{
debug_assert (!roots_hashes.empty ());

header.serialize (stream_a);
if (header.block_type () == nano::block_type::not_a_block)
{
debug_assert (!roots_hashes.empty ());
// Write hashes & roots
for (auto & root_hash : roots_hashes)
{
write (stream_a, root_hash.first);
write (stream_a, root_hash.second);
}
}
else

// Write hashes & roots
for (auto & root_hash : roots_hashes)
{
debug_assert (block != nullptr);
block->serialize (stream_a);
nano::write (stream_a, root_hash.first);
nano::write (stream_a, root_hash.second);
}
}

bool nano::confirm_req::deserialize (nano::stream & stream_a, nano::block_uniquer * uniquer_a)
{
bool result (false);
debug_assert (header.type == nano::message_type::confirm_req);

bool result = false;
try
{
if (header.block_type () == nano::block_type::not_a_block)
uint8_t const count = header.count_get ();
for (auto i (0); i != count && !result; ++i)
{
uint8_t count (header.count_get ());
for (auto i (0); i != count && !result; ++i)
nano::block_hash block_hash (0);
nano::block_hash root (0);
nano::read (stream_a, block_hash);
nano::read (stream_a, root);
if (!block_hash.is_zero () || !root.is_zero ())
{
nano::block_hash block_hash (0);
nano::block_hash root (0);
read (stream_a, block_hash);
read (stream_a, root);
if (!block_hash.is_zero () || !root.is_zero ())
{
roots_hashes.emplace_back (block_hash, root);
}
roots_hashes.emplace_back (block_hash, root);
}

result = roots_hashes.empty () || (roots_hashes.size () != count);
}
else
{
block = nano::deserialize_block (stream_a, header.block_type (), uniquer_a);
result = block == nullptr;
}

result = roots_hashes.empty () || (roots_hashes.size () != count);
}
catch (std::runtime_error const &)
{
Expand All @@ -649,11 +631,7 @@ bool nano::confirm_req::deserialize (nano::stream & stream_a, nano::block_unique
bool nano::confirm_req::operator== (nano::confirm_req const & other_a) const
{
bool equal (false);
if (block != nullptr && other_a.block != nullptr)
{
equal = *block == *other_a.block;
}
else if (!roots_hashes.empty () && !other_a.roots_hashes.empty ())
if (!roots_hashes.empty () && !other_a.roots_hashes.empty ())
{
equal = roots_hashes == other_a.roots_hashes;
}
Expand All @@ -675,33 +653,17 @@ std::string nano::confirm_req::roots_string () const

std::size_t nano::confirm_req::size (const nano::message_header & header)
{
auto const type = header.block_type ();
if (type != nano::block_type::invalid && type != nano::block_type::not_a_block)
{
return nano::block::size (type);
}
else if (type == nano::block_type::not_a_block)
{
auto const count = header.count_get ();
return count * (sizeof (hash_root_pair));
}
return 0;
auto const count = header.count_get ();
return count * (sizeof (hash_root_pair));
}

std::string nano::confirm_req::to_string () const
{
std::string s = header.to_string ();

if (header.block_type () == nano::block_type::not_a_block)
{
for (auto && roots_hash : roots_hashes)
{
s += "\n" + roots_hash.first.to_string () + ":" + roots_hash.second.to_string ();
}
}
else
for (auto && roots_hash : roots_hashes)
{
s += "\n" + block->to_json ();
s += "\n" + roots_hash.first.to_string () + ":" + roots_hash.second.to_string ();
}

return s;
Expand All @@ -725,13 +687,11 @@ nano::confirm_ack::confirm_ack (nano::network_constants const & constants, std::
message (constants, nano::message_type::confirm_ack),
vote (vote_a)
{
header.block_type_set (nano::block_type::not_a_block);
header.count_set (static_cast<uint8_t> (vote_a->hashes.size ()));
}

void nano::confirm_ack::serialize (nano::stream & stream_a) const
{
debug_assert (header.block_type () == nano::block_type::not_a_block || header.block_type () == nano::block_type::send || header.block_type () == nano::block_type::receive || header.block_type () == nano::block_type::open || header.block_type () == nano::block_type::change || header.block_type () == nano::block_type::state);
header.serialize (stream_a);
vote->serialize (stream_a);
}
Expand Down
2 changes: 0 additions & 2 deletions nano/node/messages.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,6 @@ class confirm_req final : public message
{
public:
confirm_req (bool & error, nano::stream &, nano::message_header const &, nano::block_uniquer * = nullptr);
confirm_req (nano::network_constants const & constants, std::shared_ptr<nano::block> const &);
confirm_req (nano::network_constants const & constants, std::vector<std::pair<nano::block_hash, nano::root>> const &);
confirm_req (nano::network_constants const & constants, nano::block_hash const &, nano::root const &);

Expand All @@ -182,7 +181,6 @@ class confirm_req final : public message
public: // Payload
using hash_root_pair = std::pair<nano::block_hash, nano::root>;

std::shared_ptr<nano::block> block;
std::vector<hash_root_pair> roots_hashes;
};

Expand Down
10 changes: 1 addition & 9 deletions nano/node/network.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -417,20 +417,12 @@ class network_message_visitor : public nano::message_visitor
{
node.logger.try_log (boost::str (boost::format ("Confirm_req message from %1% for hashes:roots %2%") % channel->to_string () % message_a.roots_string ()));
}
else
{
node.logger.try_log (boost::str (boost::format ("Confirm_req message from %1% for %2%") % channel->to_string () % message_a.block->hash ().to_string ()));
}
}

// Don't load nodes with disabled voting
if (node.config.enable_voting && node.wallets.reps ().voting > 0)
{
if (message_a.block != nullptr)
{
node.aggregator.add (channel, { { message_a.block->hash (), message_a.block->root () } });
}
else if (!message_a.roots_hashes.empty ())
if (!message_a.roots_hashes.empty ())
{
node.aggregator.add (channel, message_a.roots_hashes);
}
Expand Down
9 changes: 1 addition & 8 deletions nano/node/transport/message_deserializer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -236,14 +236,7 @@ std::unique_ptr<nano::confirm_req> nano::transport::message_deserializer::deseri
auto incoming = std::make_unique<nano::confirm_req> (error, stream, header, &block_uniquer_m);
if (!error && nano::at_end (stream))
{
if (incoming->block == nullptr || !network_constants_m.work.validate_entry (*incoming->block))
{
return incoming;
}
else
{
status = parse_status::insufficient_work;
}
return incoming;
}
else
{
Expand Down

0 comments on commit 0e72333

Please sign in to comment.