Add transparent data to the CompactBlock format.

[nuttycom]

This also modifies the BlockRange type to allow callers to specify a set of pool types for which data should be returned. In the case that no pool types are specified, a server should return compact blocks containing only data relevant to the shielded (Sapling and Orchard) pools.

[nuttycom]

This replaces the .proto changes previously suggested in zcash/librustzcash#1781

[pacu]

Thanks for doing this!

[pacu]

The way I see it this proto file looks good to me.

I assume that once merged the different repos will be updating their copies

Example for Zaino

git subtree add --prefix=zaino-proto/lightwallet-protocol --squash git@github.com:zcash/lightwallet-protocol.git main

[LarryRuane]

tACK bf5975a (tested with a lightwalletd branch with the corresponding changes but which doesn't have a PR yet), although I don't see this change yet.

Yeah, POOL_TYPE_INVALID seems good.

Also, I think the three commits could be squashed into one.

[LarryRuane]

I just thought of another question for both of you ... I'm implementing the GetBlockRange PoolType filtering. Suppose that after filtering, the block contains no transactions. Should we still return the block (in effect, just its header)?

Currently, we do return these "empty" blocks, so I guess for compatibility, we should continue doing that. Alternatively, we could define that if no PoolType list is given (which comes to lightwalletd as a zero-length list, i.e., existing clients), we return all blocks in the range, even if they have zero transactions (current behavior). But if the caller specifies one or more PoolType values, then we could suppress the empty blocks.

[nuttycom]

I just thought of another question for both of you ... I'm implementing the GetBlockRange PoolType filtering. Suppose that after filtering, the block contains no transactions. Should we still return the block (in effect, just its header)?

Currently, we do return these "empty" blocks, so I guess for compatibility, we should continue doing that. Alternatively, we could define that if no PoolType list is given (which comes to lightwalletd as a zero-length list, i.e., existing clients), we return all blocks in the range, even if they have zero transactions (current behavior). But if the caller specifies one or more PoolType values, then we could suppress the empty blocks.

We should continue to return the empty blocks; particular with out-of-order scanning, we need to verify that a block did not contain any outputs for our wallet, and also gaps in block order would break our ability to check the hash chaining between blocks.

[LarryRuane]

Got it, thanks.

We should continue to return the empty blocks; particular with out-of-order scanning, we need to verify that a block did not contain any outputs for our wallet ...

Not that it matters, I'm just curious, what is out-of-order scanning? (I do understand the need to check block chaining.)

re-ACK f9b777c (although I'd be fine with squashing the commits, that's personal preference)

[LarryRuane]

ACK f9b777c

[LarryRuane]

@pacu if you're comfortable, please review zcash/lightwalletd#534; I can't seem to add you as a reviewer on that PR.

[nuttycom]

force-pushed to address comments from code review and rebase atop #4

[pacu]

LGTM modulo two documentation issues.

[nuttycom]

force-pushed t address comments from review.

[LarryRuane]

tACK a95359d
Is this ready to merge? I think this PR may need to be rebased onto #4 because of the CHANGELOG changes?

The merge-base changed after approval.

[nuttycom]

tACK a95359d Is this ready to merge? I think this PR may need to be rebased onto #4 because of the CHANGELOG changes?

It's already based on #4

[str4d]

ACK a95359d

The merge-base changed after approval.

[str4d]

ACK a95359d

The merge-base changed after approval.