From 58080cc8cfe50cd4e971398419aab12fa5e73465 Mon Sep 17 00:00:00 2001 From: Guilherme Lawless Date: Fri, 14 Feb 2020 17:44:58 +0000 Subject: [PATCH] Incremental options for ws confirmation subscription (#2566) Possibility to add or remove accounts in an existing subscription. This is useful for external wallets that can't use the all_local_accounts flag. --- nano/core_test/websocket.cpp | 68 ++++++++++++++++++++++++++++++++ nano/node/websocket.cpp | 76 +++++++++++++++++++++++++++++++++--- nano/node/websocket.hpp | 27 ++++++++++++- 3 files changed, 164 insertions(+), 7 deletions(-) diff --git a/nano/core_test/websocket.cpp b/nano/core_test/websocket.cpp index 66c242eeb5..c2cb4069c2 100644 --- a/nano/core_test/websocket.cpp +++ b/nano/core_test/websocket.cpp @@ -371,6 +371,74 @@ TEST (websocket, confirmation_options) } } +// Tests updating options of block confirmations +TEST (websocket, confirmation_options_update) +{ + nano::system system; + nano::node_config config (nano::get_available_port (), system.logging); + config.websocket_config.enabled = true; + config.websocket_config.port = nano::get_available_port (); + auto node1 (system.add_node (config)); + + std::atomic added{ false }; + std::atomic deleted{ false }; + auto task = ([&added, &deleted, config, &node1]() { + fake_websocket_client client (config.websocket_config.port); + // Subscribe initially with empty options, everything will be filtered + client.send_message (R"json({"action": "subscribe", "topic": "confirmation", "ack": "true", "options": {}})json"); + client.await_ack (); + EXPECT_EQ (1, node1->websocket_server->subscriber_count (nano::websocket::topic::confirmation)); + // Now update filter with an account and wait for a response + std::string add_message = boost::str (boost::format (R"json({"action": "update", "topic": "confirmation", "ack": "true", "options": {"accounts_add": ["%1%"]}})json") % nano::test_genesis_key.pub.to_account ()); + client.send_message (add_message); + client.await_ack (); + EXPECT_EQ (1, node1->websocket_server->subscriber_count (nano::websocket::topic::confirmation)); + added = true; + EXPECT_TRUE (client.get_response ()); + // Update the filter again, removing the account + std::string delete_message = boost::str (boost::format (R"json({"action": "update", "topic": "confirmation", "ack": "true", "options": {"accounts_del": ["%1%"]}})json") % nano::test_genesis_key.pub.to_account ()); + client.send_message (delete_message); + client.await_ack (); + EXPECT_EQ (1, node1->websocket_server->subscriber_count (nano::websocket::topic::confirmation)); + deleted = true; + EXPECT_FALSE (client.get_response (1s)); + }); + auto future = std::async (std::launch::async, task); + + // Wait for update acknowledgement + system.deadline_set (5s); + while (!added) + { + ASSERT_NO_ERROR (system.poll ()); + } + + // Confirm a block + system.wallet (0)->insert_adhoc (nano::test_genesis_key.prv); + nano::genesis genesis; + nano::keypair key; + auto previous (node1->latest (nano::test_genesis_key.pub)); + auto send (std::make_shared (nano::test_genesis_key.pub, previous, nano::test_genesis_key.pub, nano::genesis_amount - nano::Gxrb_ratio, key.pub, nano::test_genesis_key.prv, nano::test_genesis_key.pub, *system.work.generate (previous))); + node1->process_active (send); + + // Wait for delete acknowledgement + system.deadline_set (5s); + while (!deleted) + { + ASSERT_NO_ERROR (system.poll ()); + } + + // Confirm another block + previous = send->hash (); + auto send2 (std::make_shared (nano::test_genesis_key.pub, previous, nano::test_genesis_key.pub, nano::genesis_amount - 2 * nano::Gxrb_ratio, key.pub, nano::test_genesis_key.prv, nano::test_genesis_key.pub, *system.work.generate (previous))); + node1->process_active (send2); + + system.deadline_set (5s); + while (future.wait_for (0s) != std::future_status::ready) + { + ASSERT_NO_ERROR (system.poll ()); + } +} + // Subscribes to votes, sends a block and awaits websocket notification of a vote arrival TEST (websocket, vote) { diff --git a/nano/node/websocket.cpp b/nano/node/websocket.cpp index 863ba28243..f4febe491a 100644 --- a/nano/node/websocket.cpp +++ b/nano/node/websocket.cpp @@ -18,7 +18,8 @@ wallets (wallets_a) } nano::websocket::confirmation_options::confirmation_options (boost::property_tree::ptree const & options_a, nano::wallets & wallets_a, nano::logger_mt & logger_a) : -wallets (wallets_a) +wallets (wallets_a), +logger (logger_a) { // Non-account filtering options include_block = options_a.get ("include_block", true); @@ -83,11 +84,7 @@ wallets (wallets_a) logger_a.always_log ("Websocket: Filtering option \"accounts\" requires that \"include_block\" is set to true to be effective"); } } - // Warn the user if the options resulted in an empty filter - if (has_account_filtering_options && !all_local_accounts && accounts.empty ()) - { - logger_a.always_log ("Websocket: provided options resulted in an empty block confirmation filter"); - } + check_filter_empty (); } bool nano::websocket::confirmation_options::should_filter (nano::websocket::message const & message_a) const @@ -136,6 +133,60 @@ bool nano::websocket::confirmation_options::should_filter (nano::websocket::mess return should_filter_conf_type_l || should_filter_account; } +bool nano::websocket::confirmation_options::update (boost::property_tree::ptree const & options_a) +{ + auto update_accounts = [this](boost::property_tree::ptree const & accounts_text_a, bool insert_a) { + this->has_account_filtering_options = true; + for (auto const & account_l : accounts_text_a) + { + nano::account result_l (0); + if (!result_l.decode_account (account_l.second.data ())) + { + // Re-encode to keep old prefix support + auto encoded_l (result_l.to_account ()); + if (insert_a) + { + this->accounts.insert (encoded_l); + } + else + { + this->accounts.erase (encoded_l); + } + } + else if (this->logger.is_initialized ()) + { + this->logger->always_log ("Websocket: invalid account provided for filtering blocks: ", account_l.second.data ()); + } + } + }; + + // Adding accounts as filter exceptions + auto accounts_add_l (options_a.get_child_optional ("accounts_add")); + if (accounts_add_l) + { + update_accounts (*accounts_add_l, true); + } + + // Removing accounts as filter exceptions + auto accounts_del_l (options_a.get_child_optional ("accounts_del")); + if (accounts_del_l) + { + update_accounts (*accounts_del_l, false); + } + + check_filter_empty (); + return false; +} + +void nano::websocket::confirmation_options::check_filter_empty () const +{ + // Warn the user if the options resulted in an empty filter + if (logger.is_initialized () && has_account_filtering_options && !all_local_accounts && accounts.empty ()) + { + logger->always_log ("Websocket: provided options resulted in an empty block confirmation filter"); + } +} + nano::websocket::vote_options::vote_options (boost::property_tree::ptree const & options_a, nano::logger_mt & logger_a) { include_replays = options_a.get ("include_replays", false); @@ -428,6 +479,19 @@ void nano::websocket::session::handle_message (boost::property_tree::ptree const } action_succeeded = true; } + else if (action == "update") + { + nano::lock_guard lk (subscriptions_mutex); + auto existing (subscriptions.find (topic_l)); + if (existing != subscriptions.end ()) + { + auto options_text_l (message_a.get_child_optional ("options")); + if (options_text_l.is_initialized () && !existing->second->update (*options_text_l)) + { + action_succeeded = true; + } + } + } else if (action == "unsubscribe" && topic_l != nano::websocket::topic::invalid) { nano::lock_guard lk (subscriptions_mutex); diff --git a/nano/node/websocket.hpp b/nano/node/websocket.hpp index 890f6cb9cb..4838c2522a 100644 --- a/nano/node/websocket.hpp +++ b/nano/node/websocket.hpp @@ -37,6 +37,7 @@ namespace websocket { class listener; class confirmation_options; + class session; /** Supported topics */ enum class topic @@ -102,6 +103,9 @@ namespace websocket class options { public: + virtual ~options () = default; + + protected: /** * Checks if a message should be filtered for default options (no options given). * @param message_a the message to be checked @@ -111,7 +115,16 @@ namespace websocket { return false; } - virtual ~options () = default; + /** + * Update options, if available for a given topic + * @return false on success + */ + virtual bool update (boost::property_tree::ptree const & options_a) + { + return true; + } + + friend class session; }; /** @@ -137,6 +150,15 @@ namespace websocket */ bool should_filter (message const & message_a) const override; + /** + * Update some existing options + * Filtering options: + * - "accounts_add" (array of std::strings) - additional accounts for which blocks should not be filtered + * - "accounts_del" (array of std::strings) - accounts for which blocks should be filtered + * @return false + */ + bool update (boost::property_tree::ptree const & options_a) override; + /** Returns whether or not block contents should be included */ bool get_include_block () const { @@ -156,7 +178,10 @@ namespace websocket static constexpr const uint8_t type_all = type_all_active | type_inactive; private: + void check_filter_empty () const; + nano::wallets & wallets; + boost::optional logger; bool include_election_info{ false }; bool include_block{ true }; bool has_account_filtering_options{ false };