From db8fbcb187c4dc9c318970e4c107f514d3d3fd62 Mon Sep 17 00:00:00 2001 From: gr0vity Date: Tue, 4 Mar 2025 18:22:01 +0100 Subject: [PATCH] Revert "refactor(websocket): simplify confirmation topic logic and remove _a suffix from parameters" This reverts commit 008637c06104dda0d2f19386f82cdf55cddd10a8. --- nano/core_test/websocket.cpp | 31 +++++++++--------- nano/node/websocket.cpp | 61 +++++++++++++++++++++--------------- nano/node/websocket.hpp | 16 +++++----- 3 files changed, 59 insertions(+), 49 deletions(-) diff --git a/nano/core_test/websocket.cpp b/nano/core_test/websocket.cpp index a04d5fc885..415d4147cd 100644 --- a/nano/core_test/websocket.cpp +++ b/nano/core_test/websocket.cpp @@ -1147,7 +1147,6 @@ TEST (websocket, new_unconfirmed_block) ASSERT_EQ ("send", message_contents.get ("subtype")); } - // Test verifying that multiple subscribers with different options receive messages with their correct // individual settings applied (specifically targeting the bug that was fixed) TEST (websocket, confirmation_options_independent) @@ -1166,7 +1165,7 @@ TEST (websocket, confirmation_options_independent) auto send_amount = node1->online_reps.delta () + 1; auto new_balance = prev_balance - send_amount; nano::block_hash previous (node1->latest (nano::dev::genesis_key.pub)); - + auto send = builder .account (nano::dev::genesis_key.pub) .previous (previous) @@ -1176,13 +1175,13 @@ TEST (websocket, confirmation_options_independent) .sign (nano::dev::genesis_key.prv, nano::dev::genesis_key.pub) .work (*system.work.generate (previous)) .build (); - + // Set up two concurrent tasks to subscribe with different options and wait for responses std::atomic client1_done{ false }; std::atomic client2_done{ false }; boost::optional client1_response; boost::optional client2_response; - + // Client 1: Subscribe with include_block = true but no sideband auto client1_task = ([&client1_done, &client1_response, &node1] () { fake_websocket_client client (node1->websocket.server->listening_port ()); @@ -1192,7 +1191,7 @@ TEST (websocket, confirmation_options_independent) client1_response = response; client1_done = true; }); - + // Client 2: Subscribe with include_block = true AND include_sideband_info = true auto client2_task = ([&client2_done, &client2_response, &node1] () { fake_websocket_client client (node1->websocket.server->listening_port ()); @@ -1202,49 +1201,49 @@ TEST (websocket, confirmation_options_independent) client2_response = response; client2_done = true; }); - + // Start both client tasks concurrently auto future1 = std::async (std::launch::async, client1_task); auto future2 = std::async (std::launch::async, client2_task); - + // Wait for both clients to be set up (both awaiting notifications) ASSERT_TIMELY (5s, node1->websocket.server->subscriber_count (nano::websocket::topic::confirmation) == 2); - + // Now process the block to trigger notifications to both clients node1->process_active (send); - + // Wait for both clients to receive their responses ASSERT_TIMELY (5s, client1_done && client2_done); - + // Verify both clients got responses ASSERT_TRUE (client1_response.has_value ()); ASSERT_TRUE (client2_response.has_value ()); - + // Parse and check client1 response (should have block but no sideband) boost::property_tree::ptree event1; std::stringstream stream1; stream1 << client1_response.get (); boost::property_tree::read_json (stream1, event1); ASSERT_EQ (event1.get ("topic"), "confirmation"); - + auto & message1 = event1.get_child ("message"); ASSERT_EQ (1, message1.count ("block")); ASSERT_EQ (0, message1.count ("sideband")); - + // Parse and check client2 response (should have both block AND sideband) boost::property_tree::ptree event2; std::stringstream stream2; stream2 << client2_response.get (); boost::property_tree::read_json (stream2, event2); ASSERT_EQ (event2.get ("topic"), "confirmation"); - + auto & message2 = event2.get_child ("message"); ASSERT_EQ (1, message2.count ("block")); - + // With the old caching code, this would fail because client2 would receive the same // message as client1 (with no sideband info) despite requesting it ASSERT_EQ (1, message2.count ("sideband")); - + // Verify sideband contains expected fields auto & sideband = message2.get_child ("sideband"); ASSERT_EQ (1, sideband.count ("height")); diff --git a/nano/node/websocket.cpp b/nano/node/websocket.cpp index 6f28fa14a0..401dbee512 100644 --- a/nano/node/websocket.cpp +++ b/nano/node/websocket.cpp @@ -659,11 +659,13 @@ void nano::websocket::listener::on_accept (boost::system::error_code ec) } } -void nano::websocket::listener::broadcast_confirmation (std::shared_ptr const & block, nano::account const & account, nano::amount const & amount, std::string const & subtype, nano::election_status const & election_status, std::vector const & election_votes) +void nano::websocket::listener::broadcast_confirmation (std::shared_ptr const & block_a, nano::account const & account_a, nano::amount const & amount_a, std::string const & subtype, nano::election_status const & election_status_a, std::vector const & election_votes_a) { nano::websocket::message_builder builder{ node.ledger }; nano::lock_guard lk (sessions_mutex); + boost::optional msg_with_block; + boost::optional msg_without_block; for (auto & weak_session : sessions) { auto session_ptr (weak_session.lock ()); @@ -678,9 +680,18 @@ void nano::websocket::listener::broadcast_confirmation (std::shared_ptrget_include_block ()); - auto message = builder.block_confirmed (block, account, amount, subtype, election_status, election_votes, *conf_options); - session_ptr->write (message); + if (include_block && !msg_with_block) + { + msg_with_block = builder.block_confirmed (block_a, account_a, amount_a, subtype, include_block, election_status_a, election_votes_a, *conf_options); + } + else if (!include_block && !msg_without_block) + { + msg_without_block = builder.block_confirmed (block_a, account_a, amount_a, subtype, include_block, election_status_a, election_votes_a, *conf_options); + } + + session_ptr->write (include_block ? msg_with_block.get () : msg_without_block.get ()); } } } @@ -740,19 +751,19 @@ nano::websocket::message nano::websocket::message_builder::stopped_election (nan return message_l; } -nano::websocket::message nano::websocket::message_builder::block_confirmed (std::shared_ptr const & block, nano::account const & account, nano::amount const & amount, std::string subtype, nano::election_status const & election_status, std::vector const & election_votes, nano::websocket::confirmation_options const & options) +nano::websocket::message nano::websocket::message_builder::block_confirmed (std::shared_ptr const & block_a, nano::account const & account_a, nano::amount const & amount_a, std::string subtype, bool include_block_a, nano::election_status const & election_status_a, std::vector const & election_votes_a, nano::websocket::confirmation_options const & options_a) { nano::websocket::message message_l (nano::websocket::topic::confirmation); set_common_fields (message_l); // Block confirmation properties boost::property_tree::ptree message_node_l; - message_node_l.add ("account", account.to_account ()); - message_node_l.add ("amount", amount.to_string_dec ()); - message_node_l.add ("hash", block->hash ().to_string ()); + message_node_l.add ("account", account_a.to_account ()); + message_node_l.add ("amount", amount_a.to_string_dec ()); + message_node_l.add ("hash", block_a->hash ().to_string ()); std::string confirmation_type = "unknown"; - switch (election_status.type) + switch (election_status_a.type) { case nano::election_status_type::active_confirmed_quorum: confirmation_type = "active_quorum"; @@ -768,20 +779,20 @@ nano::websocket::message nano::websocket::message_builder::block_confirmed (std: }; message_node_l.add ("confirmation_type", confirmation_type); - if (options.get_include_election_info () || options.get_include_election_info_with_votes ()) + if (options_a.get_include_election_info () || options_a.get_include_election_info_with_votes ()) { boost::property_tree::ptree election_node_l; - election_node_l.add ("duration", election_status.election_duration.count ()); - election_node_l.add ("time", election_status.election_end.count ()); - election_node_l.add ("tally", election_status.tally.to_string_dec ()); - election_node_l.add ("final", election_status.final_tally.to_string_dec ()); - election_node_l.add ("blocks", std::to_string (election_status.block_count)); - election_node_l.add ("voters", std::to_string (election_status.voter_count)); - election_node_l.add ("request_count", std::to_string (election_status.confirmation_request_count)); - if (options.get_include_election_info_with_votes ()) + election_node_l.add ("duration", election_status_a.election_duration.count ()); + election_node_l.add ("time", election_status_a.election_end.count ()); + election_node_l.add ("tally", election_status_a.tally.to_string_dec ()); + election_node_l.add ("final", election_status_a.final_tally.to_string_dec ()); + election_node_l.add ("blocks", std::to_string (election_status_a.block_count)); + election_node_l.add ("voters", std::to_string (election_status_a.voter_count)); + election_node_l.add ("request_count", std::to_string (election_status_a.confirmation_request_count)); + if (options_a.get_include_election_info_with_votes ()) { boost::property_tree::ptree election_votes_l; - for (auto const & vote_l : election_votes) + for (auto const & vote_l : election_votes_a) { boost::property_tree::ptree entry; entry.put ("representative", vote_l.representative.to_account ()); @@ -795,13 +806,13 @@ nano::websocket::message nano::websocket::message_builder::block_confirmed (std: message_node_l.add_child ("election_info", election_node_l); } - if (options.get_include_block ()) + if (include_block_a) { boost::property_tree::ptree block_node_l; - block->serialize_json (block_node_l); - if (options.get_include_linked_account ()) + block_a->serialize_json (block_node_l); + if (options_a.get_include_linked_account ()) { - auto linked_account = ledger.linked_account (ledger.tx_begin_read (), *block); + auto linked_account = ledger.linked_account (ledger.tx_begin_read (), *block_a); if (linked_account.has_value ()) { block_node_l.add ("linked_account", linked_account.value ().to_account ()); @@ -818,11 +829,11 @@ nano::websocket::message nano::websocket::message_builder::block_confirmed (std: message_node_l.add_child ("block", block_node_l); } - if (options.get_include_sideband_info ()) + if (options_a.get_include_sideband_info ()) { boost::property_tree::ptree sideband_node_l; - sideband_node_l.add ("height", std::to_string (block->sideband ().height)); - sideband_node_l.add ("local_timestamp", std::to_string (block->sideband ().timestamp)); + sideband_node_l.add ("height", std::to_string (block_a->sideband ().height)); + sideband_node_l.add ("local_timestamp", std::to_string (block_a->sideband ().timestamp)); message_node_l.add_child ("sideband", sideband_node_l); } diff --git a/nano/node/websocket.hpp b/nano/node/websocket.hpp index 7552360ef2..2c54754042 100644 --- a/nano/node/websocket.hpp +++ b/nano/node/websocket.hpp @@ -91,13 +91,13 @@ namespace websocket public: message_builder (nano::ledger & ledger); - message block_confirmed (std::shared_ptr const & block, nano::account const & account, nano::amount const & amount, std::string subtype, nano::election_status const & election_status, std::vector const & election_votes, nano::websocket::confirmation_options const & options); - message started_election (nano::block_hash const & hash); - message stopped_election (nano::block_hash const & hash); - message vote_received (std::shared_ptr const & vote, nano::vote_code code); - message work_generation (nano::work_version const version, nano::block_hash const & root, uint64_t work, uint64_t difficulty, uint64_t publish_threshold, std::chrono::milliseconds const & duration, std::string const & peer, std::vector const & bad_peers, bool completed = true, bool cancelled = false); - message work_cancelled (nano::work_version const version, nano::block_hash const & root, uint64_t difficulty, uint64_t publish_threshold, std::chrono::milliseconds const & duration, std::vector const & bad_peers); - message work_failed (nano::work_version const version, nano::block_hash const & root, uint64_t difficulty, uint64_t publish_threshold, std::chrono::milliseconds const & duration, std::vector const & bad_peers); + message block_confirmed (std::shared_ptr const & block_a, nano::account const & account_a, nano::amount const & amount_a, std::string subtype, bool include_block, nano::election_status const & election_status_a, std::vector const & election_votes_a, nano::websocket::confirmation_options const & options_a); + message started_election (nano::block_hash const & hash_a); + message stopped_election (nano::block_hash const & hash_a); + message vote_received (std::shared_ptr const & vote_a, nano::vote_code code_a); + message work_generation (nano::work_version const version_a, nano::block_hash const & root_a, uint64_t const work_a, uint64_t const difficulty_a, uint64_t const publish_threshold_a, std::chrono::milliseconds const & duration_a, std::string const & peer_a, std::vector const & bad_peers_a, bool const completed_a = true, bool const cancelled_a = false); + message work_cancelled (nano::work_version const version_a, nano::block_hash const & root_a, uint64_t const difficulty_a, uint64_t const publish_threshold_a, std::chrono::milliseconds const & duration_a, std::vector const & bad_peers_a); + message work_failed (nano::work_version const version_a, nano::block_hash const & root_a, uint64_t const difficulty_a, uint64_t const publish_threshold_a, std::chrono::milliseconds const & duration_a, std::vector const & bad_peers_a); message bootstrap_started (std::string const & id_a, std::string const & mode_a); message bootstrap_exited (std::string const & id_a, std::string const & mode_a, std::chrono::steady_clock::time_point const start_time_a, uint64_t const total_blocks_a); message telemetry_received (nano::telemetry_data const &, nano::endpoint const &); @@ -325,7 +325,7 @@ namespace websocket void stop (); /** Broadcast block confirmation. The content of the message depends on subscription options (such as "include_block") */ - void broadcast_confirmation (std::shared_ptr const & block, nano::account const & account, nano::amount const & amount, std::string const & subtype, nano::election_status const & election_status, std::vector const & election_votes); + void broadcast_confirmation (std::shared_ptr const & block_a, nano::account const & account_a, nano::amount const & amount_a, std::string const & subtype, nano::election_status const & election_status_a, std::vector const & election_votes_a); /** Broadcast \p message to all session subscribing to the message topic. */ void broadcast (nano::websocket::message message_a);