Implemented filtering options for eth_subscribe with logs #27842 #16826

vadimstruts · 2023-01-24T15:40:21Z

PR represents the implementation of advanced functionality for log subscription. It provides filtering options for retrieving logs

For example, the next JS code illustrates new approach:

window.ethereum.on('message', (x, y, z) => console.log('message event: ', x, y, z))
window.ethereum.send({ id:1, method: 'eth_subscribe', params:['logs', {"address":  ["0xdac17f958d2ee523a2206206994597c13d831ec7", "0x51fe8e8fa3f9f10288fa8c2aff1400f887d21c42"]}]}, console.log)

As you can see, only the logs for the next two addresses will be retrieved each time:

"0xdac17f958d2ee523a2206206994597c13d831ec7", 
"0x51fe8e8fa3f9f10288fa8c2aff1400f887d21c42"

Full list of supported filtering options described here: https://www.quicknode.com/docs/ethereum/eth_getLogs

Resolves brave/brave-browser/issues/27842

Submitter Checklist:

I confirm that no security/privacy review is needed, or that I have requested one
There is a ticket for my issue
Used Github auto-closing keywords in the PR description above
Wrote a good PR/commit description
Squashed any review feedback or "fixup" commits before merge, so that history is a record of what happened in the repo, not your PR
Added appropriate labels (QA/Yes or QA/No; release-notes/include or release-notes/exclude; OS/...) to the associated issue
Checked the PR locally:
- npm run test -- brave_browser_tests, npm run test -- brave_unit_tests wiki
- npm run lint, npm run presubmit wiki, npm run gn_check, npm run tslint
Ran git rebase master (if needed)

Reviewer Checklist:

A security review is not needed, or a link to one is included in the PR description
New files have MPL-2.0 license header
Adequate test coverage exists to prevent regressions
Major classes, functions and non-trivial code blocks are well-commented
Changes in component dependencies are properly reflected in gn
Code follows the style guide
Test plan is specified in PR before merging

After-merge Checklist:

Test Plan:

Subscribe for logs with additional filtering options

window.ethereum.on('message', (x, y, z) => console.log('message event: ', x, y, z))
window.ethereum.send({ id:1, method: 'eth_subscribe', params:['logs', {"address":  ["0xdac17f958d2ee523a2206206994597c13d831ec7", "0x51fe8e8fa3f9f10288fa8c2aff1400f887d21c42"]}]}, console.log)

Response:

{
    "id": 1,
    "jsonrpc": "2.0",
    "result": "0xac2fa41a9975eeac8a2ef8f8ae1c97e8"
}

Receiving logs only for specified address (or another filtering options): 0xdac17f958d2ee523a2206206994597c13d831ec7
Unsubscribe with using subscription id:

window.ethereum.on('message', (x, y, z) => console.log('message event: ', x, y, z))
window.ethereum.send({ id:5, method: 'eth_unsubscribe', params:['0xac2fa41a9975eeac8a2ef8f8ae1c97e8']}, console.log)

Logging is stopped

supermassive · 2023-01-26T02:00:07Z

browser/brave_wallet/ethereum_provider_impl_unittest.cc

+          auto payload = ToValue(request);
+          EXPECT_TRUE(payload && payload->is_dict());
+
+          auto* method = payload->GetDict().FindString("method");
+          EXPECT_TRUE(method);
+          EXPECT_EQ(*method, "eth_getLogs");
+
+          auto* params = payload->GetDict().FindList("params");
+          EXPECT_TRUE(params);
+          EXPECT_TRUE(params->front().is_dict());
+
+          auto* address = params->front().GetDict().FindList("address");
+          EXPECT_TRUE(address);
+          auto addr = std::find(address->begin(), address->end(), "0x9125");
+          EXPECT_TRUE(addr != address->end());
+
+          auto* fromBlock = params->front().GetDict().FindString("fromBlock");
+          EXPECT_TRUE(fromBlock);
+          EXPECT_EQ(*fromBlock, "0x2211");
+
+          auto* toBlock = params->front().GetDict().FindString("toBlock");
+          EXPECT_TRUE(toBlock);
+          EXPECT_EQ(*toBlock, "0xab65");
+
+          auto* topics = params->front().GetDict().FindList("topics");
+          EXPECT_TRUE(topics);
+          auto topic1 = std::find(topics->begin(), topics->end(), "0x2edc");
+          EXPECT_TRUE(topic1 != topics->end());
+          auto topic2 = std::find(topics->begin(), topics->end(), "0xb832");
+          EXPECT_TRUE(topic2 != topics->end());
+          auto topic3 = std::find(topics->begin(), topics->end(), "0x8dc8");
+          EXPECT_TRUE(topic3 != topics->end());


Worth just parsing hardcoded json string to base::Value to compare with payload

supermassive · 2023-01-26T04:42:52Z

components/brave_wallet/common/eth_request_helper.cc

+    for (const auto& topics_list_item : (*vvalue_list)) {
+      if (const auto* topics_list_item_str = topics_list_item.GetIfString()) {
+        arr_out->emplace_back(*topics_list_item_str);
+      }
+    }


I think topics filter for get_ethLogs and eth_newFilter has same format
https://ethereum.org/ru/developers/docs/apis/json-rpc/#eth_newfilter
which means [A, [B, C]] and [A, B, C] are different filters, but this parser flattens such array. Nulls are also supposed to be supported.

Well it seems to me logging params should be transparent to browser, i.e. we should get filter params as base::Value from renderer and pass it to rpc endpoint as is.
So interface should look like this

void JsonRpcService::EthGetLogs(const std::string& chain_id, const base::Value& params, EthGetLogsCallback callback)

supermassive · 2023-02-02T03:22:16Z

components/brave_wallet/browser/ethereum_provider_impl.cc

-    std::string event_type;
-    if (!ParseEthSubscribeParams(normalized_json_request, &event_type)) {
+    base::Value params;
+    if (!ParseEthSubscribeParams(normalized_json_request, &params)) {


ParseEthSubscribeParams should check if params is a list of one or two elements and return them. The first one should be a string, the second one should be a dict(if exists)

EthSubscribe gets them as args:
EthereumProviderImpl::EthSubscribe(const std::string& event_type, absl::optional<base::Value::Dict> filter,...) and I don't think we need to do any validation of filter's content(for example we still don't do full validation of address per spec address (optional): either a string representing an address or an array of such strings.) - let that be backend responsibility:

} else if (event_type == kEthSubscribeLogs && filter) { const auto gen_res = generateHexBytes(eth_log_subscriptions_); if (std::get<0>(gen_res)) { eth_logs_tracker_.Start(base::Seconds(kLogTrackerDefaultTimeInSeconds)); } 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); } else {

I agree with just letting RPC endpoint to validate/handle the filter keys, in the end it's always the RPC endpoint that decides which keys they would support, so I don't think we need to hardcode this list in our browser which might be outdated or not matching what a specific RPC endpoint really supports.
Just a side note, it seems the RPC endpoint can support eth_subscribe logs without filter based on your test plan in #16633, so in additional to support filter, we probably want to keep what we currently support (without filter) too.

supermassive · 2023-02-02T04:02:08Z

components/brave_wallet/browser/json_rpc_service.cc

+  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));


We don't need to look into params. Just change eth::eth_getLogs to something like:

std::string eth_getLogs(base::Value::Dict filter_options) { base::Value::List params; params.Append(std::move(filter_options)); base::Value::Dict dictionary = GetJsonRpcDictionary("eth_getLogs", std::move(params)); return GetJSON(dictionary); }

If at some point we need a c++ way to create such filter then that should be

base::Value::Dict MakeEthLogFilter(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);

which makes a filter dict not a rpc call structure

Signed-off-by: Vadym Struts <vstruts@brave.com>

supermassive

LGTM

vadimstruts added this to the 1.49.x - Nightly milestone Jan 24, 2023

vadimstruts requested a review from a team as a code owner January 24, 2023 15:40

vadimstruts self-assigned this Jan 24, 2023

vadimstruts linked an issue Jan 24, 2023 that may be closed by this pull request

Support filtering options for eth_subscribe with logs brave/brave-browser#27842

Closed

vadimstruts removed this from the 1.49.x - Nightly milestone Jan 24, 2023

vadimstruts force-pushed the 27842-support-filtering-options-for-eth_subscribe-with-logs branch from 39b369f to 084b948 Compare January 24, 2023 17:57

supermassive self-requested a review January 25, 2023 02:04

supermassive reviewed Jan 26, 2023

View reviewed changes

vadimstruts force-pushed the 27842-support-filtering-options-for-eth_subscribe-with-logs branch 3 times, most recently from 36d6c0c to b3e8417 Compare January 27, 2023 17:38

supermassive reviewed Feb 2, 2023

View reviewed changes

vadimstruts added 2 commits February 2, 2023 17:02

Implemented filtering options for eth_subscribe with logs #27842

df4e7d7

Signed-off-by: Vadym Struts <vstruts@brave.com>

Fixed after code review #27842

2e131d0

Signed-off-by: Vadym Struts <vstruts@brave.com>

vadimstruts force-pushed the 27842-support-filtering-options-for-eth_subscribe-with-logs branch from b3e8417 to 2e131d0 Compare February 2, 2023 16:12

supermassive approved these changes Feb 3, 2023

View reviewed changes

vadimstruts merged commit 68f0a09 into master Feb 3, 2023

vadimstruts deleted the 27842-support-filtering-options-for-eth_subscribe-with-logs branch February 3, 2023 09:23

github-actions bot added this to the 1.50.x - Nightly milestone Feb 3, 2023

srirambv mentioned this pull request Mar 21, 2023

Support filtering options for eth_subscribe with logs brave/brave-browser#27842

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implemented filtering options for eth_subscribe with logs #27842 #16826

Implemented filtering options for eth_subscribe with logs #27842 #16826

vadimstruts commented Jan 24, 2023 •

edited

Loading

supermassive Jan 26, 2023

vadimstruts Jan 27, 2023

supermassive Jan 26, 2023

supermassive Jan 26, 2023

vadimstruts Jan 27, 2023

supermassive Feb 2, 2023

yrliou Feb 2, 2023 •

edited

Loading

vadimstruts Feb 3, 2023

supermassive Feb 2, 2023

vadimstruts Feb 3, 2023

supermassive left a comment

Implemented filtering options for eth_subscribe with logs #27842 #16826

Implemented filtering options for eth_subscribe with logs #27842 #16826

Conversation

vadimstruts commented Jan 24, 2023 • edited Loading

Submitter Checklist:

Reviewer Checklist:

After-merge Checklist:

Test Plan:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yrliou Feb 2, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

supermassive left a comment

Choose a reason for hiding this comment

vadimstruts commented Jan 24, 2023 •

edited

Loading

yrliou Feb 2, 2023 •

edited

Loading