Support empty block range end in getaddresstxids calls.

[nuttycom]

Please ensure this checklist is followed for any pull requests for this repo. This checklist must be checked by both the PR creator and by anyone who reviews the PR.

• Relevant documentation for this PR has to be completed before the PR can be merged
• A test plan for the PR must be documented in the PR notes and included in the test plan for the next regular release

As a note, all CI tests need to be passing and all appropriate code reviews need to be done before this PR can be merged

[LarryRuane]

Testing, found a problem because the zcashd getaddresstxids has a quirky behavior: if you specify a start height but not an end height, it returns the entire range (block 1 (or zero?) to the tip), that is, it ignores start. We can work around it in lightwalletd (if no end is given, query zcashd for the tip height, and use that), but it would be better to fix it in zcashd. I'd be glad to do that (I'm almost certainly the one who introduced the bug in the first place). Maybe we should do both for some time to work around zcashd upgrade delays.

Here an example that shows the problem. This mainnet taddr is a few years old and hasn't had any recent activity. The first three give the expected result.

$ src/zcash-cli getaddresstxids '{"addresses": ["t1aY1Rc7rm5TRbXJzCxgtBhDeyN7q1bZWVm"]}' | jq '.|length'
37
$ src/zcash-cli getaddresstxids '{"addresses": ["t1aY1Rc7rm5TRbXJzCxgtBhDeyN7q1bZWVm"], "start":1}' | jq '.|length'
37
$ src/zcash-cli getaddresstxids '{"addresses": ["t1aY1Rc7rm5TRbXJzCxgtBhDeyN7q1bZWVm"],"start":2618005,"end":2618006}'| jq '.|length'
0
$ ### this will give an unexpected result, zero txids should be returned, not 37
$ src/zcash-cli getaddresstxids '{"addresses": ["t1aY1Rc7rm5TRbXJzCxgtBhDeyN7q1bZWVm"],"start":2618005}'| jq '.|length'
37
$

[nuttycom]

Okay, yep, that looks like a zcashd bug. I can see how it's coming from the very weird implementation of https://github.com/zcash/zcash/blob/master/src/rpc/misc.cpp#L826-L852

[LarryRuane]

that looks like a zcashd bug

I can fix that tonight, have a PR ready by tomorrow morning, if that's okay

[nuttycom]

Let me consult with @daira and/or @str4d on what we need to do here.

[nuttycom]

Okay, yeah, if you think you introduced this bug then fixing it would be helpful! :)

[LarryRuane]

if you think you introduced this bug

Yes, I do remember saying at a meeting, "Bug, meet everyone; everyone, this is bug."

[LarryRuane]

ACK aa60184

$ grpcurl -plaintext -d '{"address":"t1aY1Rc7rm5TRbXJzCxgtBhDeyN7q1bZWVm","range":{"start":{"height":997000},"end":{"height":997311}}}' localhost:9067 cash.z.wallet.sdk.rpc.CompactTxStreamer/GetTaddressTxids | jq '.height'
"997044"
"997074"
"997310"
$ grpcurl -plaintext -d '{"address":"t1aY1Rc7rm5TRbXJzCxgtBhDeyN7q1bZWVm","range":{"start":{"height":1086930}}}' localhost:9067 cash.z.wallet.sdk.rpc.CompactTxStreamer/GetTaddressTxids | jq '.height'
"1086930"
"1108150"
"1108168"
$ grpcurl -plaintext -d '{"address":"t1aY1Rc7rm5TRbXJzCxgtBhDeyN7q1bZWVm","range":{"start":{"height":1108149},"end":{"height":99999999}}}' localhost:9067 cash.z.wallet.sdk.rpc.CompactTxStreamer/GetTaddressTxids | jq '.height'
"1108150"
"1108168"
$ ###### this will search the full chain (just count up the results this time)
$ grpcurl -plaintext -d '{"address":"t1aY1Rc7rm5TRbXJzCxgtBhDeyN7q1bZWVm","range":{"start":{"height":0}}}' localhost:9067 cash.z.wallet.sdk.rpc.CompactTxStreamer/GetTaddressTxids | jq '.height' | wc -l
37
$ 

[LarryRuane]

@nuttycom - I just discovered that zebrad's version of the getaddresstxids RPC doesn't support zero for either the start or end height params. (On zcashd, this RPC allows either to be zero (or unspecified, which is equivalent to zero), with the expected semantics for end=0, the height of the tip of the best chain.)

As is, won't lightwalletd not work with zebrad for this reason? If so, this is only a problem for end, because if the gRPC start argument is zero, lightwalletd can simply call getaddresstxids with a value of 1. As for end, I see three alternatives:

• create an issue on zebrad requesting this change (so that end=0 means the tip height)
• implement a PR on zebrad ourselves to do this
• modify lightwalletd so that it explicitly specifies the correct end value (it knows the height of the current tip)

Of these, I think the third (modify lightwalletd) is probably the most direct and least disruptive.

But I want to make sure from you, Kris, that this is a real problem if people try to use zebrad as their lightwalletd backend.