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

@@ -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<bool> added{ false };
+	std::atomic<bool> 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::state_block> (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::state_block> (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)
 {

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<bool> ("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
+	check_filter_empty ();
-	if (has_account_filtering_options && !all_local_accounts && accounts.empty ())
-	{
-		logger_a.always_log ("Websocket: provided options resulted in an empty block confirmation filter");
-	}
 }
 
 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<bool> ("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<std::mutex> 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<std::mutex> lk (subscriptions_mutex);

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<nano::logger_mt &> logger;
 		bool include_election_info{ false };
 		bool include_block{ true };
 		bool has_account_filtering_options{ false };