use getblock verbose=3 instead of two getblock calls

If available, use `getblock verbose=3`. If not available, use the old way,
which requires making two `getblock` RPCs. This is only a performance
improvement.

Author: Larry Ruane

common/common.go

@@ -10,7 +10,7 @@ import (
 	"errors"
 	"fmt"
 	"strconv"
-	"strings"
+	"sync"
 	"time"
 
 	"github.com/sirupsen/logrus"
@@ -154,6 +154,7 @@ type (
 	ZcashRpcReplyGetblock1 struct {
 		Hash  string
 		Tx    []string
+		Hex   string
 		Trees struct {
 			Sapling struct {
 				Size uint32
@@ -295,6 +296,12 @@ func GetLightdInfo() (*walletrpc.LightdInfo, error) {
 	}, nil
 }
 
+// For performance, cache whether getblock verbose level 3 is available
+// (we may be using an older zcashd that doesn't support it).
+var triedGetblock3 bool
+var haveGetblock3 bool
+var getblock3mutex sync.Mutex
+
 func getBlockFromRPC(height int) (*walletrpc.CompactBlock, error) {
 	// `block.ParseFromSlice` correctly parses blocks containing v5
 	// transactions, but incorrectly computes the IDs of the v5 transactions.
@@ -311,21 +318,56 @@ func getBlockFromRPC(height int) (*walletrpc.CompactBlock, error) {
 	}
 	params := make([]json.RawMessage, 2)
 	params[0] = heightJSON
-	// Fetch the block using the verbose option ("1") because it provides
-	// both the list of txids, which we're not yet able to compute for
-	// Orchard (V5) transactions, and the block hash (block ID), which
-	// we need to fetch the raw data format of the same block. Don't fetch
+
+	// We're not (yet) able to compute v5 transactions IDs (from the raw
+	// serialized transaction), so we get this from zcashd 'getblock'.
+	//
+	// First try verbose=3, which returns a JSON object with exactly
+	// what's needed: the block raw hex and the list of txids.
+	//
+	// If unsuccessful (zcashd isn't upgraded),
+	// fetch the block using the verbose option ("1") because it provides
+	// both the list of txids and the block hash (block ID), which we then
+	// use to fetch the raw data format of the same block. Don't fetch
 	// by height in case a reorg occurs between the two getblock calls;
 	// using block hash ensures that we're fetching the same block.
-	params[1] = json.RawMessage("1")
-	result, rpcErr := RawRequest("getblock", params)
-	if rpcErr != nil {
-		// Check to see if we are requesting a height the zcashd doesn't have yet
-		if (strings.Split(rpcErr.Error(), ":"))[0] == "-8" {
-			return nil, nil
+	var result json.RawMessage
+	var rpcErr error
+	getblock3mutex.Lock()
+	tried := triedGetblock3
+	have := haveGetblock3
+	getblock3mutex.Unlock()
+	if !tried || have {
+		params[1] = json.RawMessage("3")
+		result, rpcErr = RawRequest("getblock", params)
+		if rpcErr != nil {
+			if rpcErr.Error() == "-8: Block height out of range" {
+				return nil, nil
+			}
+			// Check for an unexpected error
+			if rpcErr.Error() != "-8: Verbosity must be in range from 0 to 2" {
+				return nil, fmt.Errorf("error requesting getblock 3: %w", rpcErr)
+			}
+			// zcashd must not be upgraded to support verbose=3
+		}
+		// zcashd supports verbose=3 if we got a result
+		getblock3mutex.Lock()
+		triedGetblock3 = true
+		haveGetblock3 = (rpcErr == nil)
+		getblock3mutex.Unlock()
+	}
+	if len(result) == 0 {
+		params[1] = json.RawMessage("1")
+		result, rpcErr = RawRequest("getblock", params)
+		if rpcErr != nil {
+			// Check to see if we are requesting a height zcashd doesn't have yet
+			if rpcErr.Error() == "-8: Block height out of range" {
+				return nil, nil
+			}
+			return nil, fmt.Errorf("error requesting verbose block: %w", rpcErr)
 		}
-		return nil, fmt.Errorf("error requesting verbose block: %w", rpcErr)
 	}
+	// This type works for both the verbose=1 or 3 reply.
 	var block1 ZcashRpcReplyGetblock1
 	err = json.Unmarshal(result, &block1)
 	if err != nil {
@@ -335,26 +377,26 @@ func getBlockFromRPC(height int) (*walletrpc.CompactBlock, error) {
 	if err != nil {
 		Log.Fatal("getBlockFromRPC bad block hash", block1.Hash)
 	}
-	params[0] = blockHash
-	params[1] = json.RawMessage("0") // non-verbose (raw hex)
-	result, rpcErr = RawRequest("getblock", params)
+	if block1.Hex == "" {
+		// the getblock verbose=3 attempt must have failed, get the raw hex now
+		params[0] = blockHash
+		params[1] = json.RawMessage("0") // non-verbose (raw hex, non-JSON)
+		result, rpcErr = RawRequest("getblock", params)
 
-	// For some reason, the error responses are not JSON
-	if rpcErr != nil {
-		return nil, fmt.Errorf("error requesting block: %w", rpcErr)
-	}
+		// For some reason, the error responses are not JSON
+		if rpcErr != nil {
+			return nil, fmt.Errorf("error requesting block: %w", rpcErr)
+		}
 
-	var blockDataHex string
-	err = json.Unmarshal(result, &blockDataHex)
-	if err != nil {
-		return nil, fmt.Errorf("error reading JSON response: %w", err)
+		err = json.Unmarshal(result, &block1.Hex)
+		if err != nil {
+			return nil, fmt.Errorf("error reading JSON response: %w", err)
+		}
 	}
-
-	blockData, err := hex.DecodeString(blockDataHex)
+	blockData, err := hex.DecodeString(block1.Hex)
 	if err != nil {
-		return nil, fmt.Errorf("error decoding getblock output: %w", err)
+		return nil, fmt.Errorf("error decoding block hex: %w", err)
 	}
-
 	block := parser.NewBlock()
 	rest, err := block.ParseFromSlice(blockData)
 	if err != nil {

frontend/frontend_test.go

@@ -360,11 +360,14 @@ func TestGetTaddressTxidsNilArgs(t *testing.T) {
 	}
 }
 
+var lmrt *testing.T
+
 func getblockStub(method string, params []json.RawMessage) (json.RawMessage, error) {
 	if method != "getblock" {
 		testT.Fatal("unexpected method:", method)
 	}
 	step++
+	testT.Log("getblockStub new step: ", step)
 	var arg string
 	err := json.Unmarshal(params[0], &arg)
 	if err != nil {
@@ -381,7 +384,7 @@ func getblockStub(method string, params []json.RawMessage) (json.RawMessage, err
 		return []byte("{\"Tx\": [\"00\"], \"Hash\": \"0000380640\"}"), nil
 	case 2:
 		if arg != "0000380640" {
-			testT.Fatal("unexpected getblock height", arg)
+			testT.Fatal("unexpected getblock hash", arg)
 		}
 		return blocks[0], nil
 	case 3:
@@ -395,15 +398,18 @@ func TestGetBlock(t *testing.T) {
 	testT = t
 	common.RawRequest = getblockStub
 	lwd, _ := testsetup()
+	testT.Log("starting TestGetBlock")
 
 	_, err := lwd.GetBlock(context.Background(), &walletrpc.BlockID{})
 	if err == nil {
 		t.Fatal("GetBlock should have failed")
 	}
+	testT.Log("about to TestGetBlock")
 	_, err = lwd.GetBlock(context.Background(), &walletrpc.BlockID{Height: 0})
 	if err == nil {
 		t.Fatal("GetBlock should have failed")
 	}
+	testT.Log("about to TestGetBlock")
 	_, err = lwd.GetBlock(context.Background(), &walletrpc.BlockID{Hash: []byte{0}})
 	if err == nil {
 		t.Fatal("GetBlock should have failed")
@@ -413,6 +419,7 @@ func TestGetBlock(t *testing.T) {
 	}
 
 	// getblockStub() case 1: return error
+	testT.Log("about to TestGetBlock")
 	block, err := lwd.GetBlock(context.Background(), &walletrpc.BlockID{Height: 380640})
 	if err != nil {
 		t.Fatal("GetBlock failed:", err)
@@ -421,13 +428,15 @@ func TestGetBlock(t *testing.T) {
 		t.Fatal("GetBlock returned unexpected block:", err)
 	}
 	// getblockStub() case 2: return error
+	testT.Log("about to TestGetBlock")
 	block, err = lwd.GetBlock(context.Background(), &walletrpc.BlockID{Height: 380640})
 	if err == nil {
 		t.Fatal("GetBlock should have failed")
 	}
 	if block != nil {
 		t.Fatal("GetBlock returned unexpected non-nil block")
 	}
+	testT.Log("end TestGetBlock")
 	step = 0
 }