Implement ZIP 244 txid logic for v5 transactions
str4d opened this issue · 3 comments
Support for parsing v5 transactions (as specified in ZIP 225) was added in #367. However, the txid computation for v5 transactions (as specified in ZIP 244) was not implemented. This means that at present, lightwalletd
returns incorrect txids for v5 transactions in compact blocks:
Lines 261 to 274 in ab4c0fe
Lines 108 to 127 in ab4c0fe
lightwalletd/parser/transaction.go
Lines 379 to 383 in ab4c0fe
lightwalletd/parser/transaction.go
Lines 346 to 365 in ab4c0fe
#367 had test vectors that include txids; these passed because the test conditional was backwards, and checking that the derived txid didn't match the test vector.
lightwalletd/parser/transaction_test.go
Lines 88 to 90 in ab4c0fe
Reopening because this shouldn't have been closed by #393 (which only partially fixed the problem, the internal code still derives the wrong txid and needs fixing).
@str4d, prompted by your recent comment, I began looking into this and discovered that the Golang version of blake2b
doesn't have the personalization
interface. Investigating this, I found that you had already opened an issue against this 4 years ago: golang/go#32447 (comment)
Since this has still not been implemented, I went ahead and wrote a patch to zcashd
's getblock
RPC to add another verbosity level, 3, that returns exactly the information that lightwalletd
needs. I know you don't like this, but it bothers me greatly that lightwalletd
makes two RPC calls for each block it fetches (one at verbosity=1 to get the txids, and another at verbosity=0 to get the raw block hex). The lightwalletd
sync now takes about 12 hours on my high-end desktop, and reducing the number of RPCs by half is sure to help.
Think of it as a short-term improvement until Golang can implement blake2
properly.
The lightwalletd PR is #449 and the corresponding zcashd PR is zcash/zcash#6747. (These can be merged and deployed in either order.)