Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(zebra-scan): Create a scanner results channel #7906

Closed
4 tasks
oxarbitrage opened this issue Nov 3, 2023 · 7 comments
Closed
4 tasks

feat(zebra-scan): Create a scanner results channel #7906

oxarbitrage opened this issue Nov 3, 2023 · 7 comments
Labels
A-blockchain-scanner Area: Blockchain scanner of shielded transactions C-feature Category: New features S-needs-triage Status: A bug report needs triage

Comments

@oxarbitrage
Copy link
Contributor

oxarbitrage commented Nov 3, 2023

This task is about creating a results channel within the zebra-scan crate. The results channel is designed to transmit related transactions for all stored keys over the channel.

To accomplish this task, we need to:

  • Define the structure of the channel, specifying that the data in the channel should consist of triples in the form of (AccountId [linked to key], block hash/height, Vec<WalletTx>).
  • Create the channel when the module is initialized using zebra-scan::new() or similar.

If we don't want to drop any results, we should use a mpsc channel for results.

Out of Scope

If we use a tokio, futures, or std channel, we don't need to do these tests, because those channels are already well-tested:

  • Implement the results channel functionality to ensure that related transactions are properly transmitted.
  • Create tests to verify the correct functioning of this channel.

The output of the scanner task should send data to this channel but this will be done in another ticket.

In a later ticket we can send the results directly to storage, and read results out of storage.

Account filtering - out of scope

Use a separate API to filter results per account.

Have a separate channel and results client for each account:

  • Store an mpsc channel for every client (HashMap<Client, Vec<(SaplingIvk, RangeBounds<Height>)>>)
  • Send transactions that are trial decrypted by a client's viewing keys directly to that client
@oxarbitrage oxarbitrage changed the title feat(zebra-scan): Create a broadcast channel. feat(zebra-scan): Create a broadcast channel Nov 3, 2023
@oxarbitrage oxarbitrage added the A-blockchain-scanner Area: Blockchain scanner of shielded transactions label Nov 3, 2023
@mpguerra mpguerra added S-needs-triage Status: A bug report needs triage P-Medium ⚡ C-feature Category: New features labels Nov 6, 2023
@teor2345
Copy link
Collaborator

teor2345 commented Nov 7, 2023

@arya2 @oxarbitrage I think there's a design decision we need to make here:

Is it ok for all broadcast channel receivers to receive all the results of the scanner task?
Or do we need to split results by the account or viewing key inside the task?

If we send all results to all receivers, we can filter or split them later if we need to. I'd like to do that outside the scanner task to reduce the complexity of the task, and the assumptions we're making about the rest of the design. (If we assume too much here, we'll have to rewrite the code later if the design changes.)

For example, the broadcast channel receiver could store results in the scanner storage. Then the result reading API could check the account ID in the channel or storage before returning results for that ID.

@teor2345 teor2345 changed the title feat(zebra-scan): Create a broadcast channel feat(zebra-scan): Create a scanner results channel Nov 7, 2023
@teor2345
Copy link
Collaborator

teor2345 commented Nov 7, 2023

I changed the title of the ticket because a broadcast channel is a specific type of tokio channel, but we haven't decided to use that type of channel yet.

@teor2345
Copy link
Collaborator

teor2345 commented Nov 8, 2023

  • If we don't want to drop any results, use a mpsc channel instead, send the results directly to storage, and read results out of storage

It looks like we'll have a scanner task for new incoming blocks, and multiple tasks for cached state blocks (each new key needs to re-scan the whole cached state). So a mpsc channel to storage seems like a good design here.

#7908 (comment)

@arya2
Copy link
Contributor

arya2 commented Nov 8, 2023

Is it ok for all broadcast channel receivers to receive all the results of the scanner task?
Or do we need to split results by the account or viewing key inside the task?

I feel it would be simpler to split the results by account or client.

read results out of storage

We could also send unordered relevant transactions and let clients make a new request with a set of queries that fill the gaps when Zebra restarts.

@mpguerra
Copy link
Contributor

Hey team! Please add your planning poker estimate with Zenhub @arya2 @oxarbitrage @teor2345 @upbqdn

@teor2345
Copy link
Collaborator

Is it ok for all broadcast channel receivers to receive all the results of the scanner task?
Or do we need to split results by the account or viewing key inside the task?

I feel it would be simpler to split the results by account or client.

read results out of storage

We could also send unordered relevant transactions and let clients make a new request with a set of queries that fill the gaps when Zebra restarts.

I think it will be simpler to read the results out of storage, and leave the filtering by account or key to the caller of the storage reads.

@teor2345
Copy link
Collaborator

teor2345 commented Dec 4, 2023

This ticket might not be needed, writing results directly to the database seems to work just fine for now.

@teor2345 teor2345 closed this as not planned Won't fix, can't repro, duplicate, stale Dec 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-blockchain-scanner Area: Blockchain scanner of shielded transactions C-feature Category: New features S-needs-triage Status: A bug report needs triage
Projects
Archived in project
Development

No branches or pull requests

4 participants