From dfac02093d85fb31fb9a8475b884dd6abca966c7 Mon Sep 17 00:00:00 2001
From: Larry Ruane <larry@electriccoin.co>
Date: Mon, 23 May 2022 16:37:12 -0600
Subject: [PATCH] Fix V5 transaction txid (#392)

This is a shortcut fix. Instead of replicating the zip244 transaction
logic that's implemented in librustzcash:
zcash_primitives/src/transaction/components/orchard.rs

cheat by calling getblock with verbose level 1, which prints the txid
of each transaction. This currently requires calling getblock twice,
once for the raw hex (as we have always done), and again to get the
transaction IDs. An easy improvement would be to add the raw hex to
verbosity 1 result. (Or have a new verbosity that shows both.)
---
 common/common.go           | 30 ++++++++++++++++++++++++++++++
 parser/transaction.go      | 23 ++++++++---------------
 parser/transaction_test.go |  7 +++----
 3 files changed, 41 insertions(+), 19 deletions(-)

diff --git a/common/common.go b/common/common.go
index 6ba25ec9..56ad5f60 100644
--- a/common/common.go
+++ b/common/common.go
@@ -147,6 +147,11 @@ type (
 		Satoshis    uint64
 		Height      int
 	}
+
+	// reply to getblock verbose=1 (json includes txid list)
+	ZcashRpcReplyGetblock1 struct {
+		Tx []string
+	}
 )
 
 // FirstRPC tests that we can successfully reach zcashd through the RPC
@@ -271,6 +276,31 @@ func getBlockFromRPC(height int) (*walletrpc.CompactBlock, error) {
 		return nil, errors.New("received unexpected height block")
 	}
 
+	// `block.ParseFromSlice` correctly parses blocks containing v5 transactions, but
+	// incorrectly computes the IDs of the v5 transactions. We temporarily paper over this
+	// bug by fetching the correct txids via a second getblock RPC call.
+	// https://github.com/zcash/lightwalletd/issues/392
+	{
+		params[1] = json.RawMessage("1") // JSON with list of txids
+		result, rpcErr := RawRequest("getblock", params)
+		if rpcErr != nil {
+			return nil, errors.Wrap(rpcErr, "error requesting verbose block")
+		}
+		var block1 ZcashRpcReplyGetblock1
+		err = json.Unmarshal(result, &block1)
+		if err != nil {
+			return nil, err
+		}
+		for i, t := range block.Transactions() {
+			txid, err := hex.DecodeString(block1.Tx[i])
+			if err != nil {
+				return nil, errors.Wrap(err, "error decoding getblock txid")
+			}
+			// convert from big-endian
+			t.SetTxID(parser.Reverse(txid))
+		}
+	}
+
 	return block.ToCompact(), nil
 }
 
diff --git a/parser/transaction.go b/parser/transaction.go
index e59367ed..5c3fb24b 100644
--- a/parser/transaction.go
+++ b/parser/transaction.go
@@ -6,7 +6,6 @@
 package parser
 
 import (
-	"crypto/sha256"
 	"fmt"
 
 	"github.com/pkg/errors"
@@ -339,29 +338,23 @@ func (p *action) ToCompact() *walletrpc.CompactOrchardAction {
 // Transaction encodes a full (zcashd) transaction.
 type Transaction struct {
 	*rawTransaction
-	rawBytes   []byte
-	cachedTxID []byte // cached for performance
+	rawBytes []byte
+	txID     []byte // from getblock verbose=1
+}
+
+func (tx *Transaction) SetTxID(txid []byte) {
+	tx.txID = txid
 }
 
 // GetDisplayHash returns the transaction hash in big-endian display order.
 func (tx *Transaction) GetDisplayHash() []byte {
-	if tx.cachedTxID != nil {
-		return tx.cachedTxID
-	}
-
-	// SHA256d
-	digest := sha256.Sum256(tx.rawBytes)
-	digest = sha256.Sum256(digest[:])
 	// Convert to big-endian
-	tx.cachedTxID = Reverse(digest[:])
-	return tx.cachedTxID
+	return Reverse(tx.txID[:])
 }
 
 // GetEncodableHash returns the transaction hash in little-endian wire format order.
 func (tx *Transaction) GetEncodableHash() []byte {
-	digest := sha256.Sum256(tx.rawBytes)
-	digest = sha256.Sum256(digest[:])
-	return digest[:]
+	return tx.txID
 }
 
 // Bytes returns a full transaction's raw bytes.
diff --git a/parser/transaction_test.go b/parser/transaction_test.go
index dd9498a2..018cd892 100644
--- a/parser/transaction_test.go
+++ b/parser/transaction_test.go
@@ -4,7 +4,6 @@
 package parser
 
 import (
-	"bytes"
 	"encoding/hex"
 	"encoding/json"
 	"os"
@@ -85,9 +84,9 @@ func TestV5TransactionParser(t *testing.T) {
 		if len(rest) != 0 {
 			t.Fatalf("Test did not consume entire buffer, %d remaining", len(rest))
 		}
-		if bytes.Equal(tx.cachedTxID, []byte(txtestdata.Txid)) {
-			t.Fatal("txid")
-		}
+		// Currently, we can't check the txid because we get that from
+		// zcashd (getblock rpc) rather than computing it ourselves.
+		// https://github.com/zcash/lightwalletd/issues/392
 		if tx.version != uint32(txtestdata.Version) {
 			t.Fatal("version miscompare")
 		}