Create a scan_block function to use across scanning tasks

[mpguerra]

Motivation

Encapsulate any logic related to using the zcash_client_backend::scan_block function, like converting the Zebra block format to CompactBlock, recording results, or tracking progress, that will be needed in both scanning tasks.

Possible Design

fn scan_one_block(
    block: Block,
    sapling_commitment_tree_size_after_block: u32,
    viewing_keys: Vec<SaplingScanningKey>,
) -> Result<Vec<WalletTx<()>> {
    if sapling_commitment_tree_size_after_block == 0 {
        // Nothing to scan here
        return Ok(());
    }

    let chain_metadata = ChainMetadata {
        sapling_commitment_tree_size: sapling_commitment_tree_size_after_block,

        // TODO: Update this when scan_block supports orchard
        orchard_commitment_tree_size: 0,
    };

    // TODO: Update this when we support multiple accounts
    let vks: Vec<_> = viewing_keys.iter().map(|| (0.into(), key)).collect();
    let scanned_block = scan_block(
        // TODO: Update this when we support testnet
        &zcash_primitives::consensus::MainNetwork,
        block_to_compact(block.into(), chain_metadata),
        &vks[..],
        // ignore whether notes are change from a viewer's own spends
        &[],
        None,
    )
    .map_err(|_| eyre!("scan block failed"))?;

    // Replace the nullifiers with () if needed.
    Ok(scanned_block.transactions())
}

Optional / Out-of-scope

This issue and the epic it is a part of (the zebra-scan MVP) should exclude:

• Scanning Orchard transactions

The scope of this issue only includes scanning Sapling transactions.

Other out of scope things:

• Account IDs
• channels
• Splitting results by key

Complex task handling pseudocode

fn viewing_keys(
    viewers: &HashMap<Sender<transaction::Hash>, Vec<(SaplingIvk, (Height, Height))>>,
) -> Vec<(AccountId, SaplingIvk)> {
    viewers
        .iter()
        .flat_map(|(_, queries)| queries)
        .enumerate()
        .map(|(idx, (key, _query))| ((idx as u32).into(), key.clone()))
        .collect()
}


async fn spawn_scan_blocks<BlockIterator: Iterator<Item = (Block, u32)> + Send + 'static>(
    block_iter: BlockIterator,
    viewers: HashMap<Sender<transaction::Hash>, Vec<(SaplingIvk, (Height, Height))>>,
) -> Result<()> {
    let (rsp_tx, rsp_rx) = tokio::sync::oneshot::channel();

    rayon::spawn_fifo(move || {
        let result = block_iter
            .into_iter()
            .par_bridge()
            .try_for_each(|(block, tree_sizes)| scan_one_block(block, tree_sizes, &viewers));

        let _ = rsp_tx.send(result);
    });

    rsp_rx.await?
}

async fn spawn_scan_block(
    block: Block,
    tree_sizes: u32,
    viewers: HashMap<Sender<transaction::Hash>, Vec<(SaplingIvk, (Height, Height))>>,
) -> Result<()> {
    let (rsp_tx, rsp_rx) = tokio::sync::oneshot::channel();

    rayon::spawn_fifo(move || {
        let _ = rsp_tx.send(scan_one_block(block, tree_sizes, &viewers));
    });

    rsp_rx.await?
}

[teor2345]

Is this about transactions in the mempool, or transactions in blocks?

[mpguerra]

Is this about transactions in the mempool, or transactions in blocks?

This is for the MVP so I think we should start with transactions in finalised blocks

[teor2345]

Is this about transactions in the mempool, or transactions in blocks?

This is for the MVP so I think we should start with transactions in finalised blocks

That seems like ticket #7953. (It was already in the tracking issue so I pressed the ticket button for it.)

Since we're just focusing on blocks in the MVP, and we current have a block-based API in the scanner tests, saying that we're scanning transactions could be confusing. Would this ticket be better named "Create a function to scan a single block"?

[mpguerra]

Is this about transactions in the mempool, or transactions in blocks?

This is for the MVP so I think we should start with transactions in finalised blocks

That seems like ticket #7953. (It was already in the tracking issue so I pressed the ticket button for it.)

This is supposed to be the equivalent to the checklist item in #7728 which says: " Implement Orchard transaction scanning"

So, do we want to close this one as a duplicate of #7953 and rename #7953 to "Scan previously verified blocks for Sapling transactions"?

Or, do we want to split "creating the function to scan a single block" (this issue) from using this function to "scan all previously verified blocks" (issue #7953)?

Since we're just focusing on blocks in the MVP, and we current have a block-based API in the scanner tests, saying that we're scanning transactions could be confusing. Would this ticket be better named "Create a function to scan a single block"?

I just want to make it clear we're only working with Sapling viewing keys for the MVP.

[mpguerra]

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

[arya2]

This is supposed to be the equivalent to the checklist item in #7728 which says: " Implement Orchard transaction scanning"

We want to use the scan_block function that's already implemented in zcash_client_backend to find Sapling transactions, but create our own scan_blocks function to encapsulate any related logic like converting the Zebra block format to CompactBlock, recording results, tracking progress, etc. that will be needed in both scanning tasks.

So, do we want to close this one as a duplicate of #7953 and rename #7953 to "Scan previously verified blocks for Sapling transactions"?

Or, do we want to split "creating the function to scan a single block" (this issue) from using this function to "scan all previously verified blocks" (issue #7953)?

I think #7953 is blocked on this, but can be split into two PRs:

• a PR to read viewing keys and previously verified blocks
• a PR that passes those to this function and closes Scan verified blocks from sapling activation to the tip #7953 (or could be done as part of the PR for this issue)

I just want to make it clear we're only working with Sapling viewing keys for the MVP.

👍 - Orchard should definitely be out-of-scope for this issue.

[teor2345]

I simplified the pseudocode to make account IDs, splitting results by key, and nullifiers out of scope. They don't have tickets or checklist items yet.

I also removed any mention of channels, because they aren't part of this ticket.