Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
nuttycom opened this issue Aug 20, 2024 · 1 comment · May be fixed by zcash/zcash#6972 or #503
Open

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

nuttycom opened this issue Aug 20, 2024 · 1 comment · May be fixed by zcash/zcash#6972 or #503
Assignees
Labels
bug Something isn't working

Comments

@nuttycom
Copy link
Contributor

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.

@LarryRuane
Copy link
Collaborator

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
2 participants