add(scan): Implement scan gRPC method

[arya2]

Motivation

We want to provide a streaming method for registering keys, getting cached scan results, and listening for any new scan results.

Closes #8256.

PR Author Checklist

Check before marking the PR as ready for review:

• Will the PR name make sense to users?
• Does the PR have a priority label?
• Have you added or updated tests?
• Is the documentation up to date?

For significant changes:

• Is there a summary in the CHANGELOG?
• Can these changes be split into multiple PRs?

If a checkbox isn't relevant to the PR, mark it as done.

Solution

• Calls ScanService with RegisterKeys, Results and SubscribeResults requests
• Creates a response channel
• Creates a new ReceiverStream wrapper around the response receiver
• Spawns a tokio task
  • Transposes the Results response and sends them to the response channel
  • Waits for results from the results_receiver returned by the call to SubscribeResults and sends them to the response channel
• Returns the response stream

Review

Anyone can review.

Reviewer Checklist

Check before approving the PR:

• Does the PR scope match the ticket?
• Are there enough tests to make sure it works? Do the tests cover the PR motivation?
• Are all the PR blockers dealt with?
  PR blockers can be dealt with in new tickets or PRs.

And check the PR Author checklist is complete.

[arya2]

Manual test output with cached results:

(There's a duplicate result here, that should be fixed now that the SubscribeResults request comes after the Results request.)

~/github/zebra$ grpcurl -plaintext -import-path ./zebra-grpc/proto -proto zebra-grpc/proto/scanner.proto -d '{"keys": [{ "key": "zxviews1q0duytgcqqqqpqre26wkl45gvwwwd706xw608hucmvfalr759ejwf7qshjf5r9aa7323zulvz6plhttp5mltqcgs9t039cx2d09mgq05ts63n8u35hyv6h9nc9ctqqtue2u7cer2mqegunuulq2luhq3ywjcz35yyljewa4mgkgjzyfwh6fr6jd0dzd44ghk0nxdv2hnv4j5nxfwv24rwdmgllhe0p8568sgqt9ckt02v2kxf5ahtql6s0ltjpkckw8gtymxtxuu9gcr0swvz", "height": 736000 }]}' '127.0.0.1:8231' scanner.Scanner/Scan
{
  "height": 780532,
  "results": {
    "zxviews1q0duytgcqqqqpqre26wkl45gvwwwd706xw608hucmvfalr759ejwf7qshjf5r9aa7323zulvz6plhttp5mltqcgs9t039cx2d09mgq05ts63n8u35hyv6h9nc9ctqqtue2u7cer2mqegunuulq2luhq3ywjcz35yyljewa4mgkgjzyfwh6fr6jd0dzd44ghk0nxdv2hnv4j5nxfwv24rwdmgllhe0p8568sgqt9ckt02v2kxf5ahtql6s0ltjpkckw8gtymxtxuu9gcr0swvz": {
      "txIds": [
        "HFGctyvCDQJPnAlUF4QD0W+sfOjbWUrGDbdiE+bIgms="
      ]
    }
  }
}
{
  "height": 780532,
  "results": {
    "zxviews1q0duytgcqqqqpqre26wkl45gvwwwd706xw608hucmvfalr759ejwf7qshjf5r9aa7323zulvz6plhttp5mltqcgs9t039cx2d09mgq05ts63n8u35hyv6h9nc9ctqqtue2u7cer2mqegunuulq2luhq3ywjcz35yyljewa4mgkgjzyfwh6fr6jd0dzd44ghk0nxdv2hnv4j5nxfwv24rwdmgllhe0p8568sgqt9ckt02v2kxf5ahtql6s0ltjpkckw8gtymxtxuu9gcr0swvz": {
      "txIds": [
        "HFGctyvCDQJPnAlUF4QD0W+sfOjbWUrGDbdiE+bIgms="
      ]
    }
  }
}
{
  "height": 780616,
  "results": {
    "zxviews1q0duytgcqqqqpqre26wkl45gvwwwd706xw608hucmvfalr759ejwf7qshjf5r9aa7323zulvz6plhttp5mltqcgs9t039cx2d09mgq05ts63n8u35hyv6h9nc9ctqqtue2u7cer2mqegunuulq2luhq3ywjcz35yyljewa4mgkgjzyfwh6fr6jd0dzd44ghk0nxdv2hnv4j5nxfwv24rwdmgllhe0p8568sgqt9ckt02v2kxf5ahtql6s0ltjpkckw8gtymxtxuu9gcr0swvz": {
      "txIds": [
        "nvIywWt5HlwTwGcRV3bc+hVEsYMEAa8cKCiGymOVZ54="
      ]
    }
  }
}
..
^C

Output without cached results:

~/github/zebra$ grpcurl -plaintext -import-path ./zebra-grpc/proto -proto zebra-grpc/proto/scanner.proto -d '{"keys": [{ "key": "zxviews1q0duytgcqqqqpqre26wkl45gvwwwd706xw608hucmvfalr759ejwf7qshjf5r9aa7323zulvz6plhttp5mltqcgs9t039cx2d09mgq05ts63n8u35hyv6h9nc9ctqqtue2u7cer2mqegunuulq2luhq3ywjcz35yyljewa4mgkgjzyfwh6fr6jd0dzd44ghk0nxdv2hnv4j5nxfwv24rwdmgllhe0p8568sgqt9ckt02v2kxf5ahtql6s0ltjpkckw8gtymxtxuu9gcr0swvz", "height": 736000 }]}' '127.0.0.1:8231' scanner.Scanner/Scan
{
  "height": 780532,
  "results": {
    "zxviews1q0duytgcqqqqpqre26wkl45gvwwwd706xw608hucmvfalr759ejwf7qshjf5r9aa7323zulvz6plhttp5mltqcgs9t039cx2d09mgq05ts63n8u35hyv6h9nc9ctqqtue2u7cer2mqegunuulq2luhq3ywjcz35yyljewa4mgkgjzyfwh6fr6jd0dzd44ghk0nxdv2hnv4j5nxfwv24rwdmgllhe0p8568sgqt9ckt02v2kxf5ahtql6s0ltjpkckw8gtymxtxuu9gcr0swvz": {
      "txIds": [
        "HFGctyvCDQJPnAlUF4QD0W+sfOjbWUrGDbdiE+bIgms="
      ]
    }
  }
}
{
  "height": 780616,
  "results": {
    "zxviews1q0duytgcqqqqpqre26wkl45gvwwwd706xw608hucmvfalr759ejwf7qshjf5r9aa7323zulvz6plhttp5mltqcgs9t039cx2d09mgq05ts63n8u35hyv6h9nc9ctqqtue2u7cer2mqegunuulq2luhq3ywjcz35yyljewa4mgkgjzyfwh6fr6jd0dzd44ghk0nxdv2hnv4j5nxfwv24rwdmgllhe0p8568sgqt9ckt02v2kxf5ahtql6s0ltjpkckw8gtymxtxuu9gcr0swvz": {
      "txIds": [
        "nvIywWt5HlwTwGcRV3bc+hVEsYMEAa8cKCiGymOVZ54="
      ]
    }
  }
}
{
  "height": 780773,
  "results": {
    "zxviews1q0duytgcqqqqpqre26wkl45gvwwwd706xw608hucmvfalr759ejwf7qshjf5r9aa7323zulvz6plhttp5mltqcgs9t039cx2d09mgq05ts63n8u35hyv6h9nc9ctqqtue2u7cer2mqegunuulq2luhq3ywjcz35yyljewa4mgkgjzyfwh6fr6jd0dzd44ghk0nxdv2hnv4j5nxfwv24rwdmgllhe0p8568sgqt9ckt02v2kxf5ahtql6s0ltjpkckw8gtymxtxuu9gcr0swvz": {
      "txIds": [
        "uTuqs07MXG4huksWBwWB1ccrGzjX4SG+UWZPPPBCVxM="
      ]
    }
  }
}
..

[oxarbitrage]

For the manual test i think it will be nice if we can show that we are listening and that a new transaction gets in.

[oxarbitrage]

I was expecting a method subscribe that will subscribe to an already registered key but i think this method is even better as it kind of do everything.

I am wondering if we should also have a subscribe method that just subscribe to the stream of an already registered key.

Added:
It feels the method covers too much so it kind of leave obsolete other methods that we need to maintain if we keep.

[arya2]

For the manual test i think it will be nice if we can show that we are listening and that a new transaction gets in.

I did, that's the second console output, I deleted the key, added it back, waited a bit, and hit ctrl-C after seeing a few results.

I am wondering if we should also have a subscribe method that just subscribe to the stream of an already registered key.

Do you have a use case or motivation in mind? I'm thinking clients probably want the results whether their key is registered or not, so it feels redundant.

Added: It feels the method covers too much so it kind of leave obsolete other methods that we need to maintain if we keep.

This method is a push interface, the pull interface is useful for clients that are calling the RPC methods from a browser environment, or for usability if some clients find it simpler to use the pull interface.

The RPC methods are only thin wrappers around the service requests, so there shouldn't be much of a maintenance burden to supporting both.

[oxarbitrage]

I did, that's the second console output, I deleted the key, added it back, waited a bit, and hit ctrl-C after seeing a few results.

Ok, thanks for the clarification.

Do you have a use case or motivation in mind? I'm thinking clients probably want the results whether their key is registered or not, so it feels redundant.

• I was expecting that the users will register a key and start scanning with registerkey.
• Then to get all the cached results with getresults.
• Subscribe to future results with subscribe.

This makes all that in one step, which i think it can be fine but i don't remember it was planned this way.

This method is a push interface, the pull interface is useful for clients that are calling the RPC methods from a browser environment, or for usability if some clients find it simpler to use the pull interface.

I don't remember that we planned to have 2 different interfaces (with overlapping functionality).

The RPC methods are only thin wrappers around the service requests, so there shouldn't be much of a maintenance burden to supporting both.

It is more easy to add than to remove. Once you support an api call you make a commit to support it (otherwise applications will break). You can have deprecation policies, etc but in general you need to keep them.

I don't mind adding this call, i just think it do a lot more than needed.

[arya2]

I'll wait for #8277 to merge then rebase and update/add the snapshots.

[oxarbitrage]

Looks great, thank you for the changes. Can you update the manual tests to show the new output format ? The other option is to add the snapshots, whatever is easier for you.

[arya2]

Can you update the manual tests to show the new output format ? The other option is to add the snapshots, whatever is easier for you.

I'll try adding the snapshots, since we need to do that anyways.

[oxarbitrage]

Looks good, thank for adding the snapshots here.