lightwalletd icon indicating copy to clipboard operation
lightwalletd copied to clipboard

Zcash rpc error codes are not being correctly mapped to GRPC responses.

Open nuttycom opened this issue 1 year ago • 1 comments

What is the bug? lightwalletd appears to be directly propagating RPC errors from zcashd and zebra, instead of reinterpreting them as the correct GRPC error types.

Here is an example of a grpcurl session:

~/work/lightwalletd on ᚠfix/get_transaction_notfound [$] via 🐹 v1.22.2 on ☁️   [email protected](us-east1)
✗ grpcurl -d @ na-lax.zcashd.zec.rocks:443 cash.z.wallet.sdk.rpc.CompactTxStreamer/GetTransaction <<EOM
{"hash": "OVmYYbd17odbutVyi9YaHZqAbyBzixVG6q/wTdwgEFM="}
EOM
ERROR:
  Code: Unknown
  Message: -5: No such mempool or blockchain transaction. Use gettransaction for wallet transactions.

~/work/lightwalletd on ᚠfix/get_transaction_notfound [$] via 🐹 v1.22.2 on ☁️   [email protected](us-east1)
✗ grpcurl -d @ zec.rocks:443 cash.z.wallet.sdk.rpc.CompactTxStreamer/GetTransaction <<EOM
{"hash": "OVmYYbd17odbutVyi9YaHZqAbyBzixVG6q/wTdwgEFM="}
EOM
ERROR:
  Code: Unknown
  Message: 0: Transaction not found

In both cases, the error being returned should actually be code 5: NotFound, as described in https://github.com/grpc/grpc/blob/master/doc/statuscodes.md#status-codes-and-their-use-in-grpc

In general, the error codes from zcashd rpc methods should be interrogated and mapped to the corresponding GRPC error codes when possible; 2: Unknown should be used only as a last resort.

nuttycom avatar Aug 20 '24 19:08 nuttycom

This is a very good suggestion, I started working on this last week, getting close to having a PR (which I'll link to here) ready for review.

LarryRuane avatar Sep 03 '24 20:09 LarryRuane