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

feature: match zcashd RPC error codes #8782

Open
1 task done
conradoplg opened this issue Aug 20, 2024 · 2 comments
Open
1 task done

feature: match zcashd RPC error codes #8782

conradoplg opened this issue Aug 20, 2024 · 2 comments
Labels
C-enhancement Category: This is an improvement S-needs-triage Status: A bug report needs triage

Comments

@conradoplg
Copy link
Collaborator

conradoplg commented Aug 20, 2024

Motivation

We currently return the error code 0 for most RPC errors.

We might want to return the same error codes that zcashd does. (However, we might want to investigate if people rely on them at all before spending time working on this)

Here are some specific instances which we might want to do right away to support specific use cases:

Specifications

The return codes are not documented. There is a list of them in bitcoin source; we would need to identify which error is returned when in the zcashd source (or do a best-effort of trying to assign the errors that make more sense in each case)

Complex Code or Requirements

No response

Testing

We might want to extend zcash-rpc-diff to test this

Related Work

No response

@conradoplg conradoplg added C-enhancement Category: This is an improvement S-needs-triage Status: A bug report needs triage labels Aug 20, 2024
@github-project-automation github-project-automation bot moved this to New in Zebra Aug 20, 2024
@arya2 arya2 changed the title feature: support RPC error codes feature: match zcashd RPC error codes Aug 20, 2024
@arya2
Copy link
Contributor

arya2 commented Aug 20, 2024

However, we might want to investigate if people rely on them at all before spending time working on this

This should be a quick change that may not be worth investigating too deeply.

We might want to extend zcash-rpc-diff to test this

zcash-rpc-diff should support this already, we'll just need to run it with parameters or in circumstances where we expect the errors.

@oxarbitrage
Copy link
Contributor

For a bit of more context here, @nuttycom tried to run some tests for the lightwalletd GetTransaction grpc method using zcashd and then zebrad as the backend.

~/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

https://discordapp.com/channels/809218587167293450/809251029673312267/1275488658421448736

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Category: This is an improvement S-needs-triage Status: A bug report needs triage
Projects
Status: New
Development

No branches or pull requests

4 participants