GetTaddressTxids is misnamed

[defuse]

The name GetTaddressTxids suggests that the RPC returns a list of txids, but it actually returns the raw transaction data for all of the transactions.

[LarryRuane]

It doesn't seem possible to rename this method in a backward-compatible way. I found this article on the subject:

The following items are protocol and binary breaking changes:

• Renaming a package, service or method

GetTAddressTxids is a method introduced in #149.

We could add a new method with a better name (I'd favor GetTAddressTransactions) that does exactly the same thing, and then remove the old name when we're sure that all gRPC clients have changed to using the new name, but it's hard to know when we've reached that point. So I'd say it probably isn't worth making any changes.

What do you think?

We should at least document that the method is misnamed, I just create #427 to do that.

[LarryRuane]

Closing for now, let me know if you'd like this addressed.

[str4d]

I've been bitten by this again, because the documentation change was not propagated throughout the rest of the ecosystem and there are now multiple client codebases with generated-from-protobuf APIs that do not declare this issue.

[LarryRuane]

Hi @str4d, sorry you ran into this problem again. What do you think is the best solution? I'm not sure what to do. I could just go ahead and rename the method, and when wallet devs are testing (even if they don't read the release notes), they'll discover that the name has changed and make the necessary changes. Or I could add a new method (with the correct name) with exactly the same thing behavior, deprecating the old method for some unspecified amount of time.

[daira]

It has to be done via a new method and deprecation of the old method. I know it's a pain, but the earlier it is done, the less time the pain will last. (I was not asked in February 2023; I would have said the same thing then.)

[wolftivy]

In trying to actually use this method, whichever way its called, one runs into the problem of not having the txids (this only returns the raw tx data and the height). In Zcash it seems computing the txid is extremely involved; the reference implementation is several hundred lines over multiple files. Therefore using this method from other languages (eg golang) is severely cramped. We actually do need to get a list of txids sometimes, but the lightwalletd service definition currently contains no way to get them. Under the hood, the client seems to call through to zcashd methods (deprecated?) to get a list of txids and then get each transaction by txid. How hard would it be to add the txid field, which we already have, to this method's return?