fix(rpc): Refactor getrawtransaction & RPC error handling

[upbqdn]

Motivation

Close #8945.

Specifications & References

• https://zcash.github.io/rpc/getrawtransaction.html
• https://github.com/zcash/zcash/blob/99ad6fdc3a549ab510422820eea5e5ce9f60a5fd/src/rpc/rawtransaction.cpp#L340

Solution

• Fix the bugs described in bug: getrawtransaction output conflicts with zcashd output #8744. Note that zcashd stores transactions in orphaned blocks and responds to queries requesting such transactions with height to -1. Since Zebra doesn't keep orphaned blocks below the rollback limit, it now returns the error "No such mempool or blockchain transaction". If we wanted to match zcashd, we'd need to start storing orphaned blocks.
• Fix the error codes for getrawtransaction and some other RPCs.
• Related cleanups:
  • Refactor the error handling for RPCs.
  • Add references to the original error codes.
  • Simplify prop tests and run them with arbitrary Networks.
  • Don't derive Serialize & Deserialize for txids

Tests

Update and add

• snapshot tests,
• unit tests, and
• prop tests

to check the new error codes.

Follow-up Work

• None

PR Author's Checklist

• The PR name will make sense to users.
• The PR provides a CHANGELOG summary.
• The solution is tested.
• The documentation is up to date.
• The PR has a priority label.

PR Reviewer's Checklist

• The PR Author's checklist is complete.
• The PR resolves the issue.

[arya2]

Looks good to me, it would be nice to apply this suggestion but it could also be removed in the next PR so I don't think it's a blocker.

[arya2]

The lightwalletd tests seem to be required now, they should pass once #9052 has merged. There was a timeout here: https://github.com/ZcashFoundation/zebra/actions/runs/12265821808/job/34222718084?pr=9049#step:12:2016,

but the same tests passed with rust stable so it was likely spurious.

[arya2]

@mergify update

[mergify]

update

✅ Branch has been successfully updated

[conradoplg]

I think there might be a bug, I'm looking into it

[conradoplg]

Yep, e.g. zcash-cli -conf=$PWD/zcash.conf getrawtransaction 851bf6fbf7a976327817c738c489d7fa657752445430922d94c983c0b9ed4609 1 is returning

error code: -1
error message:
Invalid params: invalid type: string "1e4842fd4962d395cd9905b7030fd5cb54c8c30148468c600416e4f14a264d2c", expected an array of length 32.

on main it returns

{
  "hex": "01000000010000000000000000000000000000000000000000000000000000000000000000ffffffff025100ffffffff0250c30000000000002321027a46eb513588b01b37ea24303f4b628afd12cc20df789fede0921e43cad3e875acd43000000000000017a9147d46a730d31f97b1930d3368a967c309bd4d136a8700000000",
  "height": 1,
  "confirmations": 2740688
}

[upbqdn]

Thanks for spotting the bug. I'll switch back to Strings and deserialize them manually instead of using transaction::Hash directly.

[upbqdn]

I think there might be a bug, I'm looking into it

Fixed.