Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove vote-by-block support for confirm req message #4341

Merged
merged 2 commits into from
Dec 1, 2023
Merged

Remove vote-by-block support for confirm req message #4341

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
dc7d2db
Remove support for confirm req with block payload
pwojcikdev Nov 9, 2023
82db34c
Remove unnecessary block uniquer argument
pwojcikdev Nov 11, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 0 additions & 31 deletions nano/core_test/message.cpp
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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);
pwojcikdev marked this conversation as resolved.
Show resolved Hide resolved
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
Open in desktop
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
Open in desktop
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
Loading