Figure out what to do in librustzcash about the broken GetTransaction method of lightwalletd

[daira]

Originally posted by @str4d in #1473 (comment):

It turns out that the GetTransaction method added to lightwalletd is horribly broken, because it does not correctly propagate the output of the getrawtransaction JSON-RPC method.

It says here "optionally includes the block height" (which runs into the gRPC problem that this gets aliased with the genesis block height 0 making the transactions look mined), but below says "block height if mined, or -1" (implying to users of the RPC that -1 means "unmined").

Meanwhile, getrawtransaction actually returns:

• A non-negative height for transactions mined in blocks in the current main chain.
• -1 for transactions mined in blocks that are not in the main chain (i.e. conflicted).
• Omitted for unmined transactions in the mempool.

This is further muddied by the fact that the Go parser is parsing the height field as an int, which is machine-dependent (and thus could differ from zcashd), and also I don't know how it handles omitted values (because it isn't to my knowledge a nullable type). So the unmined transactions might be getting their height fields coerced to some default in Go (0?) rather than being set as optional in the returned Protobuf (aka 0) - this needs further investigation.

RawTransaction was then further misused by the addition of the GetMempoolStream RPC. Transactions in the mempool should never be associated with the latest block height; if any height is associated with them, it should be the "mempool height" which is latest_height + 1 (which is the height at which the consensus rules are applied to the mempool transactions). So that RPC is broken as well.

This is going to be Not Fun™️ to fix.

[daira]

@nuttycom:

We have discovered additional underlying issues: ZcashFoundation/zebra#8744

Lightwalletd issues documented here: zcash/lightwalletd#493