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 10, 2023
1 parent 083f243 commit dc7d2db
Show file tree
Hide file tree
Showing 7 changed files with 45 additions and 150 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 @@ -128,35 +128,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 @@ -185,7 +156,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 @@ -239,7 +209,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
109 changes: 33 additions & 76 deletions nano/node/messages.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,7 @@ std::size_t nano::message_header::payload_length_bytes () const
}
case nano::message_type::confirm_req:
{
return nano::confirm_req::size (block_type (), count_get ());
return nano::confirm_req::size (*this);
}
case nano::message_type::node_id_handshake:
{
Expand Down Expand Up @@ -515,32 +515,22 @@ 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)
{
// not_a_block (1) block type for hashes + roots request
header.block_type_set (nano::block_type::not_a_block);
debug_assert (!roots_hashes.empty ());
debug_assert (roots_hashes.size () < 16);

// Set `not_a_block` (1) block type for hashes + roots request
// This is needed to keep compatibility with previous protocol versions (<= V25.1)
header.block_type_set (nano::block_type::not_a_block);
header.count_set (static_cast<uint8_t> (roots_hashes.size ()));
}

nano::confirm_req::confirm_req (nano::network_constants const & constants, nano::block_hash const & hash_a, nano::root const & root_a) :
message (constants, nano::message_type::confirm_req),
roots_hashes (std::vector<std::pair<nano::block_hash, nano::root>> (1, std::make_pair (hash_a, root_a)))
confirm_req (constants, std::vector<std::pair<nano::block_hash, nano::root>>{ { hash_a, root_a } })
{
debug_assert (!roots_hashes.empty ());
// not_a_block (1) block type for hashes + roots request
header.block_type_set (nano::block_type::not_a_block);
debug_assert (roots_hashes.size () < 16);
header.count_set (static_cast<uint8_t> (roots_hashes.size ()));
}

void nano::confirm_req::visit (nano::message_visitor & visitor_a) const
Expand All @@ -550,52 +540,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 @@ -608,11 +585,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 @@ -632,34 +605,19 @@ std::string nano::confirm_req::roots_string () const
return result;
}

std::size_t nano::confirm_req::size (nano::block_type type_a, std::size_t count)
std::size_t nano::confirm_req::size (nano::message_header const & header)
{
std::size_t result (0);
if (type_a != nano::block_type::invalid && type_a != nano::block_type::not_a_block)
{
result = nano::block::size (type_a);
}
else if (type_a == nano::block_type::not_a_block)
{
result = count * (sizeof (nano::uint256_union) + sizeof (nano::block_hash));
}
return result;
auto const count = header.count_get ();
return count * (sizeof (decltype (roots_hashes)::value_type::first) + sizeof (decltype (roots_hashes)::value_type::second));
}

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)
{
for (auto && roots_hash : roots_hashes)
{
s += "\n" + roots_hash.first.to_string () + ":" + roots_hash.second.to_string ();
}
}
else
{
s += "\n" + block->to_json ();
s += "\n" + roots_hash.first.to_string () + ":" + roots_hash.second.to_string ();
}

return s;
Expand All @@ -683,14 +641,13 @@ 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);
debug_assert (vote_a->hashes.size () < 16);

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
11 changes: 6 additions & 5 deletions nano/node/messages.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -145,19 +145,20 @@ class publish final : public message
class confirm_req final : public message
{
public:
confirm_req (bool &, 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 (bool & error, nano::stream &, nano::message_header const &, nano::block_uniquer * = nullptr);
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 &);
void serialize (nano::stream &) const override;
bool deserialize (nano::stream &, nano::block_uniquer * = nullptr);
void visit (nano::message_visitor &) const override;
bool operator== (nano::confirm_req const &) const;
std::shared_ptr<nano::block> block;
std::vector<std::pair<nano::block_hash, nano::root>> roots_hashes;
std::string roots_string () const;
static std::size_t size (nano::block_type, std::size_t = 0);
std::string to_string () const;

static std::size_t size (nano::message_header const &);

public: // Payload
std::vector<std::pair<nano::block_hash, nano::root>> roots_hashes;
};

class confirm_ack final : public message
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 dc7d2db

Please sign in to comment.