From df4e7d784e007ac418d865f56fdd1eb278ae7f7c Mon Sep 17 00:00:00 2001 From: Vadym Struts Date: Fri, 27 Jan 2023 18:38:11 +0100 Subject: [PATCH 1/2] Implemented filtering options for eth_subscribe with logs #27842 Signed-off-by: Vadym Struts --- .../ethereum_provider_impl_unittest.cc | 73 +++++++++++++++++++ .../browser/asset_discovery_manager.cc | 10 ++- .../brave_wallet/browser/eth_logs_tracker.cc | 26 +++++-- .../brave_wallet/browser/eth_logs_tracker.h | 13 +++- .../browser/ethereum_provider_impl.cc | 65 ++++++++++++++--- .../browser/ethereum_provider_impl.h | 7 +- .../brave_wallet/browser/json_rpc_service.cc | 51 +++++++++++-- .../brave_wallet/browser/json_rpc_service.h | 5 +- .../browser/json_rpc_service_unittest.cc | 8 +- .../brave_wallet/common/eth_request_helper.cc | 15 ++-- .../brave_wallet/common/eth_request_helper.h | 2 +- 11 files changed, 231 insertions(+), 44 deletions(-) diff --git a/browser/brave_wallet/ethereum_provider_impl_unittest.cc b/browser/brave_wallet/ethereum_provider_impl_unittest.cc index 0e3bc07be4f5..760dff048a4c 100644 --- a/browser/brave_wallet/ethereum_provider_impl_unittest.cc +++ b/browser/brave_wallet/ethereum_provider_impl_unittest.cc @@ -125,6 +125,15 @@ std::vector DecodeHexHash(const std::string& hash_hex) { return hash; } +absl::optional ToValue(const network::ResourceRequest& request) { + base::StringPiece request_string(request.request_body->elements() + ->at(0) + .As() + .AsStringPiece()); + return base::JSONReader::Read(request_string, + base::JSONParserOptions::JSON_PARSE_RFC); +} + } // namespace class TestEventsListener : public brave_wallet::mojom::EventsListener { @@ -2171,6 +2180,70 @@ TEST_F(EthereumProviderImplUnitTest, EthSubscribeLogs) { EXPECT_FALSE(provider_->eth_logs_tracker_.IsRunning()); } +TEST_F(EthereumProviderImplUnitTest, EthSubscribeLogsFiltered) { + CreateWallet(); + url_loader_factory_.SetInterceptor( + base::BindLambdaForTesting([&](const network::ResourceRequest& request) { + url_loader_factory_.ClearResponses(); + + std::string header_value; + EXPECT_TRUE(request.headers.GetHeader("X-Eth-Method", &header_value)); + + if (header_value == "eth_getLogs") { + const absl::optional req_body_payload = + base::JSONReader::Read( + R"({"id":1,"jsonrpc":"2.0","method":"eth_getLogs","params": +[{"address":["0x1111"],"fromBlock":"0x2211","toBlock":"0xab65", +"topics":["0x2edc","0xb832","0x8dc8"]}]})", + base::JSON_PARSE_CHROMIUM_EXTENSIONS | + base::JSONParserOptions::JSON_PARSE_RFC); + + const auto payload = ToValue(request); + EXPECT_EQ(*payload, req_body_payload.value()); + } + url_loader_factory_.AddResponse( + request.url.spec(), + R"({"id":1,"jsonrpc":"2.0","result":[{"address":"0x91", + "blockHash":"0xe8","blockNumber":"0x10","data":"0x0067", + "logIndex":"0x0","removed":false, + "topics":["0x4b","0x06e","0x085"], + "transactionHash":"0x22f7","transactionIndex":"0x0"}]})"); + })); + + // Logs subscription with parameters + std::string request_payload_json = + R"({"id":1,"jsonrpc:": "2.0","method":"eth_subscribe", + "params": ["logs", {"address": "0x1111", "fromBlock": "0x2211", + "toBlock": "0xab65", "topics": ["0x2edc", "0xb832", "0x8dc8"]}]})"; + absl::optional request_payload = base::JSONReader::Read( + request_payload_json, base::JSON_PARSE_CHROMIUM_EXTENSIONS | + base::JSONParserOptions::JSON_PARSE_RFC); + std::string error_message; + auto response = CommonRequestOrSendAsync(request_payload.value()); + EXPECT_EQ(response.first, false); + EXPECT_TRUE(response.second.is_string()); + std::string subscription = *response.second.GetIfString(); + browser_task_environment_.FastForwardBy( + base::Seconds(kLogTrackerDefaultTimeInSeconds)); + EXPECT_TRUE(observer_->MessageEventFired()); + base::Value rv = observer_->GetLastMessage(); + ASSERT_TRUE(rv.is_dict()); + + std::string* address = rv.GetDict().FindString("address"); + EXPECT_EQ(*address, "0x91"); + + // The first unsubscribe should not stop the block tracker + request_payload_json = base::StringPrintf(R"({"id":1,"jsonrpc:": "2.0", + "method":"eth_unsubscribe", + "params": ["%s"]})", + subscription.c_str()); + request_payload = base::JSONReader::Read( + request_payload_json, base::JSON_PARSE_CHROMIUM_EXTENSIONS | + base::JSONParserOptions::JSON_PARSE_RFC); + response = CommonRequestOrSendAsync(request_payload.value()); + EXPECT_FALSE(provider_->eth_logs_tracker_.IsRunning()); +} + TEST_F(EthereumProviderImplUnitTest, Web3ClientVersion) { std::string expected_version = base::StringPrintf( "BraveWallet/v%s", version_info::GetBraveChromiumVersionNumber().c_str()); diff --git a/components/brave_wallet/browser/asset_discovery_manager.cc b/components/brave_wallet/browser/asset_discovery_manager.cc index 067b4080839c..641947f64ced 100644 --- a/components/brave_wallet/browser/asset_discovery_manager.cc +++ b/components/brave_wallet/browser/asset_discovery_manager.cc @@ -300,9 +300,13 @@ void AssetDiscoveryManager::OnGetEthTokenRegistry( weak_ptr_factory_.GetWeakPtr(), base::OwnedRef(std::move(tokens_to_search)), triggered_by_accounts_added, chain_id); - json_rpc_service_->EthGetLogs(chain_id, from_block, to_block, - std::move(contract_addresses_to_search), - std::move(topics), std::move(callback)); + base::Value::Dict filtering; + filtering.Set("fromBlock", from_block); + filtering.Set("toBlock", to_block); + filtering.Set("address", std::move(contract_addresses_to_search)); + filtering.Set("topics", std::move(topics)); + json_rpc_service_->EthGetLogs(chain_id, base::Value(std::move(filtering)), + std::move(callback)); } void AssetDiscoveryManager::OnGetTokenTransferLogs( diff --git a/components/brave_wallet/browser/eth_logs_tracker.cc b/components/brave_wallet/browser/eth_logs_tracker.cc index 8eca83c5154b..a718ef744c05 100644 --- a/components/brave_wallet/browser/eth_logs_tracker.cc +++ b/components/brave_wallet/browser/eth_logs_tracker.cc @@ -6,7 +6,7 @@ #include "brave/components/brave_wallet/browser/eth_logs_tracker.h" #include -#include +#include namespace brave_wallet { @@ -31,6 +31,16 @@ bool EthLogsTracker::IsRunning() const { return timer_.IsRunning(); } +void EthLogsTracker::AddSubscriber(const std::string& subscription_id, + const base::Value& params) { + eth_logs_subscription_info_.insert( + std::pair(subscription_id, params.Clone())); +} + +void EthLogsTracker::RemoveSubscriber(const std::string& subscription_id) { + eth_logs_subscription_info_.erase(subscription_id); +} + void EthLogsTracker::AddObserver(EthLogsTracker::Observer* observer) { observers_.AddObserver(observer); } @@ -42,18 +52,22 @@ void EthLogsTracker::RemoveObserver(EthLogsTracker::Observer* observer) { void EthLogsTracker::GetLogs() { const auto chain_id = json_rpc_service_->GetChainId(mojom::CoinType::ETH); - json_rpc_service_->EthGetLogs( - chain_id, {}, {}, {}, {}, - base::BindOnce(&EthLogsTracker::OnGetLogs, weak_factory_.GetWeakPtr())); + for (auto const& esi : std::as_const(eth_logs_subscription_info_)) { + json_rpc_service_->EthGetLogs( + chain_id, esi.second, + base::BindOnce(&EthLogsTracker::OnGetLogs, weak_factory_.GetWeakPtr(), + esi.first)); + } } -void EthLogsTracker::OnGetLogs([[maybe_unused]] const std::vector& logs, +void EthLogsTracker::OnGetLogs(const std::string& subscription, + [[maybe_unused]] const std::vector& logs, base::Value rawlogs, mojom::ProviderError error, const std::string& error_message) { if (error == mojom::ProviderError::kSuccess && rawlogs.is_dict()) { for (auto& observer : observers_) - observer.OnLogsReceived(rawlogs.Clone()); + observer.OnLogsReceived(subscription, rawlogs.Clone()); } else { LOG(ERROR) << "OnGetLogs failed"; } diff --git a/components/brave_wallet/browser/eth_logs_tracker.h b/components/brave_wallet/browser/eth_logs_tracker.h index 02111b497713..bfbc0667cfd5 100644 --- a/components/brave_wallet/browser/eth_logs_tracker.h +++ b/components/brave_wallet/browser/eth_logs_tracker.h @@ -6,6 +6,7 @@ #ifndef BRAVE_COMPONENTS_BRAVE_WALLET_BROWSER_ETH_LOGS_TRACKER_H_ #define BRAVE_COMPONENTS_BRAVE_WALLET_BROWSER_ETH_LOGS_TRACKER_H_ +#include #include #include @@ -34,7 +35,8 @@ class EthLogsTracker { class Observer : public base::CheckedObserver { public: - virtual void OnLogsReceived(base::Value rawlogs) = 0; + virtual void OnLogsReceived(const std::string& subscription, + base::Value rawlogs) = 0; }; // If timer is already running, it will be replaced with new interval @@ -42,12 +44,17 @@ class EthLogsTracker { void Stop(); bool IsRunning() const; + void AddSubscriber(const std::string& subscription_id, + const base::Value& params); + void RemoveSubscriber(const std::string& subscription_id); + void AddObserver(Observer* observer); void RemoveObserver(Observer* observer); private: void GetLogs(); - void OnGetLogs(const std::vector& logs, + void OnGetLogs(const std::string& subscription, + const std::vector& logs, base::Value rawlogs, mojom::ProviderError error, const std::string& error_message); @@ -55,6 +62,8 @@ class EthLogsTracker { base::RepeatingTimer timer_; raw_ptr json_rpc_service_ = nullptr; + std::map eth_logs_subscription_info_; + base::ObserverList observers_; base::WeakPtrFactory weak_factory_{this}; diff --git a/components/brave_wallet/browser/ethereum_provider_impl.cc b/components/brave_wallet/browser/ethereum_provider_impl.cc index a3449a980969..6f387aeedd03 100644 --- a/components/brave_wallet/browser/ethereum_provider_impl.cc +++ b/components/brave_wallet/browser/ethereum_provider_impl.cc @@ -578,7 +578,7 @@ void EthereumProviderImpl::RecoverAddress(const std::string& message, false); } -void EthereumProviderImpl::EthSubscribe(const std::string& event_type, +void EthereumProviderImpl::EthSubscribe(const base::Value& params, RequestCallback callback, base::Value id) { const auto generateHexBytes = [](std::vector& subscriptions) { @@ -589,7 +589,24 @@ void EthereumProviderImpl::EthSubscribe(const std::string& event_type, return std::tuple{subscriptions.size() == 1, hex_bytes}; }; - if (event_type == kEthSubscribeNewHeads) { + const auto is_event_type = [&](const std::string& eq_val) { + if (!params.is_list()) { + return false; + } + + const auto& list = params.GetList(); + auto it = std::find_if(list.begin(), list.end(), [&](const auto& item) { + return item.is_string() && eq_val == item.GetString(); + }); + + if (it != list.end()) { + return true; + } + + return false; + }; + + if (is_event_type(kEthSubscribeNewHeads)) { const auto gen_res = generateHexBytes(eth_subscriptions_); if (std::get<0>(gen_res)) { eth_block_tracker_.Start( @@ -597,11 +614,36 @@ void EthereumProviderImpl::EthSubscribe(const std::string& event_type, } std::move(callback).Run(std::move(id), base::Value(std::get<1>(gen_res)), false, "", false); - } else if (event_type == kEthSubscribeLogs) { + } else if (is_event_type(kEthSubscribeLogs)) { const auto gen_res = generateHexBytes(eth_log_subscriptions_); + + const auto get_filter_options = [&]() { + if (!params.is_list()) { + return base::Value(); + } + + const auto& list = params.GetList(); + auto it = std::find_if(list.begin(), list.end(), [](const auto& item) { + return item.is_dict() && (nullptr != item.GetDict().Find("address") || + nullptr != item.GetDict().Find("topics") || + nullptr != item.GetDict().Find("fromBlock") || + nullptr != item.GetDict().Find("toBlock") || + nullptr != item.GetDict().Find("blockHash")); + }); + if (it == list.end()) { + return base::Value(); + } + + return base::Value(it->Clone()); + }; + if (std::get<0>(gen_res)) { eth_logs_tracker_.Start(base::Seconds(kLogTrackerDefaultTimeInSeconds)); } + + eth_logs_tracker_.AddSubscriber(std::get<1>(gen_res), + std::move(get_filter_options())); + std::move(callback).Run(std::move(id), base::Value(std::get<1>(gen_res)), false, "", false); } else { @@ -641,9 +683,10 @@ bool EthereumProviderImpl::UnsubscribeBlockObserver( bool EthereumProviderImpl::UnsubscribeLogObserver( const std::string& subscription_id) { if (base::Erase(eth_log_subscriptions_, subscription_id)) { - if (eth_log_subscriptions_.empty()) + eth_logs_tracker_.RemoveSubscriber(subscription_id); + if (eth_log_subscriptions_.empty()) { eth_logs_tracker_.Stop(); - + } return true; } return false; @@ -1236,13 +1279,13 @@ void EthereumProviderImpl::CommonRequestOrSendAsync(base::ValueView input_value, } else if (method == kWeb3ClientVersion) { Web3ClientVersion(std::move(callback), std::move(id)); } else if (method == kEthSubscribe) { - std::string event_type; - if (!ParseEthSubscribeParams(normalized_json_request, &event_type)) { + base::Value params; + if (!ParseEthSubscribeParams(normalized_json_request, ¶ms)) { SendErrorOnRequest(error, error_message, std::move(callback), std::move(id)); return; } - EthSubscribe(event_type, std::move(callback), std::move(id)); + EthSubscribe(std::move(params), std::move(callback), std::move(id)); } else if (method == kEthUnsubscribe) { std::string subscription_id; if (!ParseEthUnsubscribeParams(normalized_json_request, &subscription_id)) { @@ -1705,7 +1748,8 @@ void EthereumProviderImpl::OnGetBlockByNumber( void EthereumProviderImpl::OnNewBlock(uint256_t block_num) {} -void EthereumProviderImpl::OnLogsReceived(base::Value rawlogs) { +void EthereumProviderImpl::OnLogsReceived(const std::string& subscription, + base::Value rawlogs) { if (!rawlogs.is_dict() || !events_listener_.is_bound()) { return; } @@ -1718,8 +1762,7 @@ void EthereumProviderImpl::OnLogsReceived(base::Value rawlogs) { } for (auto& results_item : *results) { - for (const auto& subscription_id : eth_log_subscriptions_) - events_listener_->MessageEvent(subscription_id, results_item.Clone()); + events_listener_->MessageEvent(subscription, results_item.Clone()); } } diff --git a/components/brave_wallet/browser/ethereum_provider_impl.h b/components/brave_wallet/browser/ethereum_provider_impl.h index 50e3614d8786..dd5712df6ad3 100644 --- a/components/brave_wallet/browser/ethereum_provider_impl.h +++ b/components/brave_wallet/browser/ethereum_provider_impl.h @@ -88,7 +88,7 @@ class EthereumProviderImpl final RequestCallback callback, base::Value id); - void EthSubscribe(const std::string& event_type, + void EthSubscribe(const base::Value& params, RequestCallback callback, base::Value id); void EthUnsubscribe(const std::string& subscription_id, @@ -173,6 +173,8 @@ class EthereumProviderImpl final RequestEthereumPermissionsWithAccounts); FRIEND_TEST_ALL_PREFIXES(EthereumProviderImplUnitTest, EthSubscribe); FRIEND_TEST_ALL_PREFIXES(EthereumProviderImplUnitTest, EthSubscribeLogs); + FRIEND_TEST_ALL_PREFIXES(EthereumProviderImplUnitTest, + EthSubscribeLogsFiltered); friend class EthereumProviderImplUnitTest; // mojom::BraveWalletProvider: @@ -378,7 +380,8 @@ class EthereumProviderImpl final bool UnsubscribeBlockObserver(const std::string& subscription_id); // EthLogsTracker::Observer: - void OnLogsReceived(base::Value rawlogs) override; + void OnLogsReceived(const std::string& subscription, + base::Value rawlogs) override; bool UnsubscribeLogObserver(const std::string& subscription_id); raw_ptr host_content_settings_map_ = nullptr; diff --git a/components/brave_wallet/browser/json_rpc_service.cc b/components/brave_wallet/browser/json_rpc_service.cc index cb99db088931..ea8cd226a740 100644 --- a/components/brave_wallet/browser/json_rpc_service.cc +++ b/components/brave_wallet/browser/json_rpc_service.cc @@ -2168,10 +2168,7 @@ void JsonRpcService::GetERC1155TokenBalance( } void JsonRpcService::EthGetLogs(const std::string& chain_id, - const std::string& from_block, - const std::string& to_block, - base::Value::List contract_addresses, - base::Value::List topics, + const base::Value& params, EthGetLogsCallback callback) { auto network_url = GetNetworkURL(prefs_, chain_id, mojom::CoinType::ETH); if (!network_url.is_valid()) { @@ -2181,12 +2178,56 @@ void JsonRpcService::EthGetLogs(const std::string& chain_id, return; } + const auto load_params = [&](std::string& from_block, std::string& to_block, + std::string& block_hash, + base::Value::List* address, + base::Value::List* topics) { + auto* filtering_opts = params.GetIfDict(); + if (filtering_opts == nullptr) { + return; + } + + const base::Value* from_block_val = filtering_opts->Find("fromBlock"); + if (from_block_val != nullptr && from_block_val->is_string()) { + from_block = from_block_val->GetString(); + } + + const base::Value* to_block_val = filtering_opts->Find("toBlock"); + if (to_block_val != nullptr && to_block_val->is_string()) { + to_block = *(to_block_val->GetIfString()); + } + + const base::Value* block_hash_val = filtering_opts->Find("blockHash"); + if (block_hash_val != nullptr && block_hash_val->is_string()) { + block_hash = *(block_hash_val->GetIfString()); + } + + const auto* contract_addresses_str = filtering_opts->FindString("address"); + if (contract_addresses_str != nullptr) { + address->Append(*contract_addresses_str); + } + + const base::Value::List* contract_addresses = + filtering_opts->FindList("address"); + if (contract_addresses != nullptr) { + *address = contract_addresses->Clone(); + } + + const base::Value::List* topics_list = filtering_opts->FindList("topics"); + if (topics_list != nullptr) { + *topics = topics_list->Clone(); + } + }; + + std::string from_block, to_block, block_hash; + base::Value::List contract_addresses, topics; + load_params(from_block, to_block, block_hash, &contract_addresses, &topics); auto internal_callback = base::BindOnce(&JsonRpcService::OnEthGetLogs, weak_ptr_factory_.GetWeakPtr(), std::move(callback)); RequestInternal( eth::eth_getLogs(from_block, to_block, std::move(contract_addresses), - std::move(topics), ""), + std::move(topics), block_hash), true, network_url, std::move(internal_callback)); } diff --git a/components/brave_wallet/browser/json_rpc_service.h b/components/brave_wallet/browser/json_rpc_service.h index 95a33bb3589b..6959d7aa79e9 100644 --- a/components/brave_wallet/browser/json_rpc_service.h +++ b/components/brave_wallet/browser/json_rpc_service.h @@ -330,10 +330,7 @@ class JsonRpcService : public KeyedService, public mojom::JsonRpcService { GetEthTokenUriCallback callback); void EthGetLogs(const std::string& chain_id, - const std::string& from_block, - const std::string& to_block, - base::Value::List addresses, - base::Value::List topics, + const base::Value& params, EthGetLogsCallback callback); void OnEthGetLogs(EthGetLogsCallback callback, diff --git a/components/brave_wallet/browser/json_rpc_service_unittest.cc b/components/brave_wallet/browser/json_rpc_service_unittest.cc index 037a05b07b6c..12662bb6a163 100644 --- a/components/brave_wallet/browser/json_rpc_service_unittest.cc +++ b/components/brave_wallet/browser/json_rpc_service_unittest.cc @@ -1160,9 +1160,13 @@ class JsonRpcServiceUnitTest : public testing::Test { mojom::ProviderError expected_error, const std::string& expected_error_message) { base::RunLoop run_loop; + base::Value::Dict params; + params.Set("fromBlock", from_block); + params.Set("toBlock", to_block); + params.Set("address", std::move(contract_addresses)); + params.Set("topics", std::move(topics)); json_rpc_service_->EthGetLogs( - chain_id, from_block, to_block, std::move(contract_addresses), - std::move(topics), + chain_id, base::Value(std::move(params)), base::BindLambdaForTesting( [&](const std::vector& logs, base::Value rawlogs, mojom::ProviderError error, const std::string& error_message) { diff --git a/components/brave_wallet/common/eth_request_helper.cc b/components/brave_wallet/common/eth_request_helper.cc index 4619bc9fffb8..83f3b8a6e0d7 100644 --- a/components/brave_wallet/common/eth_request_helper.cc +++ b/components/brave_wallet/common/eth_request_helper.cc @@ -677,19 +677,18 @@ bool ParseEthSendRawTransactionParams(const std::string& json, return true; } -bool ParseEthSubscribeParams(const std::string& json, std::string* event_type) { - if (!event_type) +bool ParseEthSubscribeParams(const std::string& json, base::Value* params) { + if (params == nullptr) { return false; + } - auto list = GetParamsList(json); - if (!list || list->size() != 1) + auto params_value = GetParamsList(json); + if (!params_value) { return false; + } - const std::string* event_type_str = (*list)[0].GetIfString(); - if (!event_type_str) - return false; + *params = base::Value(std::move(*params_value)); - *event_type = *event_type_str; return true; } diff --git a/components/brave_wallet/common/eth_request_helper.h b/components/brave_wallet/common/eth_request_helper.h index 76519990f21d..371d89c27da3 100644 --- a/components/brave_wallet/common/eth_request_helper.h +++ b/components/brave_wallet/common/eth_request_helper.h @@ -74,7 +74,7 @@ bool ParseRequestPermissionsParams( bool ParseEthSendRawTransactionParams(const std::string& json, std::string* signed_transaction); -bool ParseEthSubscribeParams(const std::string& json, std::string* event_type); +bool ParseEthSubscribeParams(const std::string& json, base::Value* params); bool ParseEthUnsubscribeParams(const std::string& json, std::string* subscription_id); From 2e131d062045cf3b3589793155b3753e27df6043 Mon Sep 17 00:00:00 2001 From: Vadym Struts Date: Thu, 2 Feb 2023 17:11:47 +0100 Subject: [PATCH 2/2] Fixed after code review #27842 Signed-off-by: Vadym Struts --- .../ethereum_provider_impl_unittest.cc | 6 +- .../browser/asset_discovery_manager.cc | 2 +- .../brave_wallet/browser/eth_logs_tracker.cc | 8 +-- .../brave_wallet/browser/eth_logs_tracker.h | 4 +- .../brave_wallet/browser/eth_requests.cc | 21 +------ .../brave_wallet/browser/eth_requests.h | 6 +- .../browser/eth_requests_unittest.cc | 13 +++- .../browser/ethereum_provider_impl.cc | 61 +++++-------------- .../browser/ethereum_provider_impl.h | 3 +- .../brave_wallet/browser/json_rpc_service.cc | 52 +--------------- .../brave_wallet/browser/json_rpc_service.h | 2 +- .../browser/json_rpc_service_unittest.cc | 2 +- .../brave_wallet/common/eth_request_helper.cc | 24 ++++++-- .../brave_wallet/common/eth_request_helper.h | 4 +- 14 files changed, 65 insertions(+), 143 deletions(-) diff --git a/browser/brave_wallet/ethereum_provider_impl_unittest.cc b/browser/brave_wallet/ethereum_provider_impl_unittest.cc index 760dff048a4c..becdcebc6101 100644 --- a/browser/brave_wallet/ethereum_provider_impl_unittest.cc +++ b/browser/brave_wallet/ethereum_provider_impl_unittest.cc @@ -2193,7 +2193,7 @@ TEST_F(EthereumProviderImplUnitTest, EthSubscribeLogsFiltered) { const absl::optional req_body_payload = base::JSONReader::Read( R"({"id":1,"jsonrpc":"2.0","method":"eth_getLogs","params": -[{"address":["0x1111"],"fromBlock":"0x2211","toBlock":"0xab65", +[{"address":["0x1111", "0x1112"],"fromBlock":"0x2211","toBlock":"0xab65", "topics":["0x2edc","0xb832","0x8dc8"]}]})", base::JSON_PARSE_CHROMIUM_EXTENSIONS | base::JSONParserOptions::JSON_PARSE_RFC); @@ -2213,8 +2213,8 @@ TEST_F(EthereumProviderImplUnitTest, EthSubscribeLogsFiltered) { // Logs subscription with parameters std::string request_payload_json = R"({"id":1,"jsonrpc:": "2.0","method":"eth_subscribe", - "params": ["logs", {"address": "0x1111", "fromBlock": "0x2211", - "toBlock": "0xab65", "topics": ["0x2edc", "0xb832", "0x8dc8"]}]})"; + "params": ["logs", {"address": ["0x1111", "0x1112"], "fromBlock": "0x2211", + "toBlock": "0xab65", "topics": ["0x2edc", "0xb832", "0x8dc8"]}]})"; absl::optional request_payload = base::JSONReader::Read( request_payload_json, base::JSON_PARSE_CHROMIUM_EXTENSIONS | base::JSONParserOptions::JSON_PARSE_RFC); diff --git a/components/brave_wallet/browser/asset_discovery_manager.cc b/components/brave_wallet/browser/asset_discovery_manager.cc index 641947f64ced..ce1f19ee4b80 100644 --- a/components/brave_wallet/browser/asset_discovery_manager.cc +++ b/components/brave_wallet/browser/asset_discovery_manager.cc @@ -305,7 +305,7 @@ void AssetDiscoveryManager::OnGetEthTokenRegistry( filtering.Set("toBlock", to_block); filtering.Set("address", std::move(contract_addresses_to_search)); filtering.Set("topics", std::move(topics)); - json_rpc_service_->EthGetLogs(chain_id, base::Value(std::move(filtering)), + json_rpc_service_->EthGetLogs(chain_id, std::move(filtering), std::move(callback)); } diff --git a/components/brave_wallet/browser/eth_logs_tracker.cc b/components/brave_wallet/browser/eth_logs_tracker.cc index a718ef744c05..ef7eea287468 100644 --- a/components/brave_wallet/browser/eth_logs_tracker.cc +++ b/components/brave_wallet/browser/eth_logs_tracker.cc @@ -32,9 +32,9 @@ bool EthLogsTracker::IsRunning() const { } void EthLogsTracker::AddSubscriber(const std::string& subscription_id, - const base::Value& params) { - eth_logs_subscription_info_.insert( - std::pair(subscription_id, params.Clone())); + base::Value::Dict filter) { + eth_logs_subscription_info_.insert(std::pair( + subscription_id, std::move(filter))); } void EthLogsTracker::RemoveSubscriber(const std::string& subscription_id) { @@ -54,7 +54,7 @@ void EthLogsTracker::GetLogs() { for (auto const& esi : std::as_const(eth_logs_subscription_info_)) { json_rpc_service_->EthGetLogs( - chain_id, esi.second, + chain_id, esi.second.Clone(), base::BindOnce(&EthLogsTracker::OnGetLogs, weak_factory_.GetWeakPtr(), esi.first)); } diff --git a/components/brave_wallet/browser/eth_logs_tracker.h b/components/brave_wallet/browser/eth_logs_tracker.h index bfbc0667cfd5..d39a81c1848f 100644 --- a/components/brave_wallet/browser/eth_logs_tracker.h +++ b/components/brave_wallet/browser/eth_logs_tracker.h @@ -45,7 +45,7 @@ class EthLogsTracker { bool IsRunning() const; void AddSubscriber(const std::string& subscription_id, - const base::Value& params); + base::Value::Dict filter); void RemoveSubscriber(const std::string& subscription_id); void AddObserver(Observer* observer); @@ -62,7 +62,7 @@ class EthLogsTracker { base::RepeatingTimer timer_; raw_ptr json_rpc_service_ = nullptr; - std::map eth_logs_subscription_info_; + std::map eth_logs_subscription_info_; base::ObserverList observers_; diff --git a/components/brave_wallet/browser/eth_requests.cc b/components/brave_wallet/browser/eth_requests.cc index 108863b9c5d4..184ef2feb26f 100644 --- a/components/brave_wallet/browser/eth_requests.cc +++ b/components/brave_wallet/browser/eth_requests.cc @@ -336,27 +336,8 @@ std::string eth_getFilterLogs(const std::string& filter_id) { return GetJsonRpcString("eth_getFilterLogs", filter_id); } -std::string eth_getLogs(const std::string& from_block_quantity_tag, - const std::string& to_block_quantity_tag, - base::Value::List addresses, - base::Value::List topics, - const std::string& block_hash) { +std::string eth_getLogs(base::Value::Dict filter_options) { base::Value::List params; - base::Value::Dict filter_options; - // The `address` filter option accepts either a single address, or a list of - // addresses (See spec: - // https://ethereum.org/en/developers/docs/apis/json-rpc/#eth_getlogs). At - // time of writing Infura's documentation suggests they only support a single - // address, however we have verified they also support a list. - if (!addresses.empty()) { - filter_options.Set("address", std::move(addresses)); - } - AddKeyIfNotEmpty(&filter_options, "fromBlock", from_block_quantity_tag); - AddKeyIfNotEmpty(&filter_options, "toBlock", to_block_quantity_tag); - if (!topics.empty()) { - filter_options.Set("topics", std::move(topics)); - } - AddKeyIfNotEmpty(&filter_options, "blockhash", block_hash); params.Append(std::move(filter_options)); base::Value::Dict dictionary = GetJsonRpcDictionary("eth_getLogs", std::move(params)); diff --git a/components/brave_wallet/browser/eth_requests.h b/components/brave_wallet/browser/eth_requests.h index 4501c3c384c7..399b939d4391 100644 --- a/components/brave_wallet/browser/eth_requests.h +++ b/components/brave_wallet/browser/eth_requests.h @@ -190,11 +190,7 @@ std::string eth_getFilterChanges(const std::string& filter_id); // Returns an array of all logs matching filter with given id. std::string eth_getFilterLogs(const std::string& filter_id); // Returns an array of all logs matching a given filter object. -std::string eth_getLogs(const std::string& from_block_quantity_tag, - const std::string& to_block_quantity_tag, - base::Value::List addresses, - base::Value::List topics, - const std::string& block_hash); +std::string eth_getLogs(base::Value::Dict filter_options); // Returns the hash of the current block, the seedHash, and the boundary // condition to be met (“target”). std::string eth_getWork(); diff --git a/components/brave_wallet/browser/eth_requests_unittest.cc b/components/brave_wallet/browser/eth_requests_unittest.cc index e1c80b223f9e..1fc718f26a26 100644 --- a/components/brave_wallet/browser/eth_requests_unittest.cc +++ b/components/brave_wallet/browser/eth_requests_unittest.cc @@ -337,10 +337,17 @@ TEST(EthRequestUnitTest, eth_getLogs) { base::Value::List addresses; addresses.Append(base::Value("0x8888f1f195afa192cfee860698584c030f4c9db1")); + + base::Value::Dict filtering; + filtering.Set("fromBlock", "0x1"); + filtering.Set("toBlock", "0x2"); + filtering.Set("address", std::move(addresses)); + filtering.Set("topics", std::move(topics)); + filtering.Set( + "blockhash", + "0xb903239f8543d04b5dc1ba6579132b143087c68db1b2168786408fcbce568238"); ASSERT_EQ( - eth_getLogs( - "0x1", "0x2", std::move(addresses), std::move(topics), - "0xb903239f8543d04b5dc1ba6579132b143087c68db1b2168786408fcbce568238"), + eth_getLogs(std::move(filtering)), R"({"id":1,"jsonrpc":"2.0","method":"eth_getLogs","params":[{"address":["0x8888f1f195afa192cfee860698584c030f4c9db1"],"blockhash":"0xb903239f8543d04b5dc1ba6579132b143087c68db1b2168786408fcbce568238","fromBlock":"0x1","toBlock":"0x2","topics":["0x000000000000000000000000a94f5374fce5edbc8e2a8697c15331677e6ebf0b",["0x000000000000000000000000a94f5374fce5edbc8e2a8697c15331677e6ebf0b","0x0000000000000000000000000aff3454fce5edbc8cca8697c15331677e6ebccc"]]}]})"); // NOLINT } diff --git a/components/brave_wallet/browser/ethereum_provider_impl.cc b/components/brave_wallet/browser/ethereum_provider_impl.cc index 6f387aeedd03..5a950f5db932 100644 --- a/components/brave_wallet/browser/ethereum_provider_impl.cc +++ b/components/brave_wallet/browser/ethereum_provider_impl.cc @@ -578,9 +578,11 @@ void EthereumProviderImpl::RecoverAddress(const std::string& message, false); } -void EthereumProviderImpl::EthSubscribe(const base::Value& params, - RequestCallback callback, - base::Value id) { +void EthereumProviderImpl::EthSubscribe( + const std::string& event_type, + absl::optional filter, + RequestCallback callback, + base::Value id) { const auto generateHexBytes = [](std::vector& subscriptions) { std::vector bytes(16); crypto::RandBytes(&bytes.front(), bytes.size()); @@ -589,24 +591,7 @@ void EthereumProviderImpl::EthSubscribe(const base::Value& params, return std::tuple{subscriptions.size() == 1, hex_bytes}; }; - const auto is_event_type = [&](const std::string& eq_val) { - if (!params.is_list()) { - return false; - } - - const auto& list = params.GetList(); - auto it = std::find_if(list.begin(), list.end(), [&](const auto& item) { - return item.is_string() && eq_val == item.GetString(); - }); - - if (it != list.end()) { - return true; - } - - return false; - }; - - if (is_event_type(kEthSubscribeNewHeads)) { + if (event_type == kEthSubscribeNewHeads) { const auto gen_res = generateHexBytes(eth_subscriptions_); if (std::get<0>(gen_res)) { eth_block_tracker_.Start( @@ -614,35 +599,14 @@ void EthereumProviderImpl::EthSubscribe(const base::Value& params, } std::move(callback).Run(std::move(id), base::Value(std::get<1>(gen_res)), false, "", false); - } else if (is_event_type(kEthSubscribeLogs)) { + } else if (event_type == kEthSubscribeLogs && filter) { const auto gen_res = generateHexBytes(eth_log_subscriptions_); - const auto get_filter_options = [&]() { - if (!params.is_list()) { - return base::Value(); - } - - const auto& list = params.GetList(); - auto it = std::find_if(list.begin(), list.end(), [](const auto& item) { - return item.is_dict() && (nullptr != item.GetDict().Find("address") || - nullptr != item.GetDict().Find("topics") || - nullptr != item.GetDict().Find("fromBlock") || - nullptr != item.GetDict().Find("toBlock") || - nullptr != item.GetDict().Find("blockHash")); - }); - if (it == list.end()) { - return base::Value(); - } - - return base::Value(it->Clone()); - }; - if (std::get<0>(gen_res)) { eth_logs_tracker_.Start(base::Seconds(kLogTrackerDefaultTimeInSeconds)); } - eth_logs_tracker_.AddSubscriber(std::get<1>(gen_res), - std::move(get_filter_options())); + eth_logs_tracker_.AddSubscriber(std::get<1>(gen_res), std::move(*filter)); std::move(callback).Run(std::move(id), base::Value(std::get<1>(gen_res)), false, "", false); @@ -1279,13 +1243,16 @@ void EthereumProviderImpl::CommonRequestOrSendAsync(base::ValueView input_value, } else if (method == kWeb3ClientVersion) { Web3ClientVersion(std::move(callback), std::move(id)); } else if (method == kEthSubscribe) { - base::Value params; - if (!ParseEthSubscribeParams(normalized_json_request, ¶ms)) { + std::string event_type; + base::Value::Dict filter; + if (!ParseEthSubscribeParams(normalized_json_request, &event_type, + &filter)) { SendErrorOnRequest(error, error_message, std::move(callback), std::move(id)); return; } - EthSubscribe(std::move(params), std::move(callback), std::move(id)); + EthSubscribe(event_type, std::move(filter), std::move(callback), + std::move(id)); } else if (method == kEthUnsubscribe) { std::string subscription_id; if (!ParseEthUnsubscribeParams(normalized_json_request, &subscription_id)) { diff --git a/components/brave_wallet/browser/ethereum_provider_impl.h b/components/brave_wallet/browser/ethereum_provider_impl.h index dd5712df6ad3..59d4fcfc7125 100644 --- a/components/brave_wallet/browser/ethereum_provider_impl.h +++ b/components/brave_wallet/browser/ethereum_provider_impl.h @@ -88,7 +88,8 @@ class EthereumProviderImpl final RequestCallback callback, base::Value id); - void EthSubscribe(const base::Value& params, + void EthSubscribe(const std::string& event_type, + absl::optional filter, RequestCallback callback, base::Value id); void EthUnsubscribe(const std::string& subscription_id, diff --git a/components/brave_wallet/browser/json_rpc_service.cc b/components/brave_wallet/browser/json_rpc_service.cc index ea8cd226a740..122de90681f9 100644 --- a/components/brave_wallet/browser/json_rpc_service.cc +++ b/components/brave_wallet/browser/json_rpc_service.cc @@ -2168,7 +2168,7 @@ void JsonRpcService::GetERC1155TokenBalance( } void JsonRpcService::EthGetLogs(const std::string& chain_id, - const base::Value& params, + base::Value::Dict filter_options, EthGetLogsCallback callback) { auto network_url = GetNetworkURL(prefs_, chain_id, mojom::CoinType::ETH); if (!network_url.is_valid()) { @@ -2178,57 +2178,11 @@ void JsonRpcService::EthGetLogs(const std::string& chain_id, return; } - const auto load_params = [&](std::string& from_block, std::string& to_block, - std::string& block_hash, - base::Value::List* address, - base::Value::List* topics) { - auto* filtering_opts = params.GetIfDict(); - if (filtering_opts == nullptr) { - return; - } - - const base::Value* from_block_val = filtering_opts->Find("fromBlock"); - if (from_block_val != nullptr && from_block_val->is_string()) { - from_block = from_block_val->GetString(); - } - - const base::Value* to_block_val = filtering_opts->Find("toBlock"); - if (to_block_val != nullptr && to_block_val->is_string()) { - to_block = *(to_block_val->GetIfString()); - } - - const base::Value* block_hash_val = filtering_opts->Find("blockHash"); - if (block_hash_val != nullptr && block_hash_val->is_string()) { - block_hash = *(block_hash_val->GetIfString()); - } - - const auto* contract_addresses_str = filtering_opts->FindString("address"); - if (contract_addresses_str != nullptr) { - address->Append(*contract_addresses_str); - } - - const base::Value::List* contract_addresses = - filtering_opts->FindList("address"); - if (contract_addresses != nullptr) { - *address = contract_addresses->Clone(); - } - - const base::Value::List* topics_list = filtering_opts->FindList("topics"); - if (topics_list != nullptr) { - *topics = topics_list->Clone(); - } - }; - - std::string from_block, to_block, block_hash; - base::Value::List contract_addresses, topics; - load_params(from_block, to_block, block_hash, &contract_addresses, &topics); auto internal_callback = base::BindOnce(&JsonRpcService::OnEthGetLogs, weak_ptr_factory_.GetWeakPtr(), std::move(callback)); - RequestInternal( - eth::eth_getLogs(from_block, to_block, std::move(contract_addresses), - std::move(topics), block_hash), - true, network_url, std::move(internal_callback)); + RequestInternal(eth::eth_getLogs(std::move(filter_options)), true, + network_url, std::move(internal_callback)); } void JsonRpcService::OnEthGetLogs(EthGetLogsCallback callback, diff --git a/components/brave_wallet/browser/json_rpc_service.h b/components/brave_wallet/browser/json_rpc_service.h index 6959d7aa79e9..b09ad3391b31 100644 --- a/components/brave_wallet/browser/json_rpc_service.h +++ b/components/brave_wallet/browser/json_rpc_service.h @@ -330,7 +330,7 @@ class JsonRpcService : public KeyedService, public mojom::JsonRpcService { GetEthTokenUriCallback callback); void EthGetLogs(const std::string& chain_id, - const base::Value& params, + base::Value::Dict filter_options, EthGetLogsCallback callback); void OnEthGetLogs(EthGetLogsCallback callback, diff --git a/components/brave_wallet/browser/json_rpc_service_unittest.cc b/components/brave_wallet/browser/json_rpc_service_unittest.cc index 12662bb6a163..f3b3c5c4b234 100644 --- a/components/brave_wallet/browser/json_rpc_service_unittest.cc +++ b/components/brave_wallet/browser/json_rpc_service_unittest.cc @@ -1166,7 +1166,7 @@ class JsonRpcServiceUnitTest : public testing::Test { params.Set("address", std::move(contract_addresses)); params.Set("topics", std::move(topics)); json_rpc_service_->EthGetLogs( - chain_id, base::Value(std::move(params)), + chain_id, std::move(params), base::BindLambdaForTesting( [&](const std::vector& logs, base::Value rawlogs, mojom::ProviderError error, const std::string& error_message) { diff --git a/components/brave_wallet/common/eth_request_helper.cc b/components/brave_wallet/common/eth_request_helper.cc index 83f3b8a6e0d7..4b049daeec20 100644 --- a/components/brave_wallet/common/eth_request_helper.cc +++ b/components/brave_wallet/common/eth_request_helper.cc @@ -677,17 +677,31 @@ bool ParseEthSendRawTransactionParams(const std::string& json, return true; } -bool ParseEthSubscribeParams(const std::string& json, base::Value* params) { - if (params == nullptr) { +bool ParseEthSubscribeParams(const std::string& json, + std::string* event_type, + base::Value::Dict* filter) { + if (filter == nullptr) { return false; } - auto params_value = GetParamsList(json); - if (!params_value) { + auto list = GetParamsList(json); + if (!list || list->empty() || list->size() > 2) { return false; } - *params = base::Value(std::move(*params_value)); + if (!(*list)[0].is_string()) { + return false; + } + + *event_type = (*list)[0].GetString(); + + if (list->size() == 2) { + if (!(*list)[1].is_dict()) { + return false; + } + + *filter = std::move((*list)[1].GetDict()); + } return true; } diff --git a/components/brave_wallet/common/eth_request_helper.h b/components/brave_wallet/common/eth_request_helper.h index 371d89c27da3..8425a2124afa 100644 --- a/components/brave_wallet/common/eth_request_helper.h +++ b/components/brave_wallet/common/eth_request_helper.h @@ -74,7 +74,9 @@ bool ParseRequestPermissionsParams( bool ParseEthSendRawTransactionParams(const std::string& json, std::string* signed_transaction); -bool ParseEthSubscribeParams(const std::string& json, base::Value* params); +bool ParseEthSubscribeParams(const std::string& json, + std::string* event_type, + base::Value::Dict* filter); bool ParseEthUnsubscribeParams(const std::string& json, std::string* subscription_id);