diff --git a/common/common.go b/common/common.go index 51955e4c..9e9a9ade 100644 --- a/common/common.go +++ b/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 { diff --git a/frontend/frontend_test.go b/frontend/frontend_test.go index 3460c6d1..8c59b852 100644 --- a/frontend/frontend_test.go +++ b/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,6 +428,7 @@ 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") @@ -428,6 +436,7 @@ func TestGetBlock(t *testing.T) { if block != nil { t.Fatal("GetBlock returned unexpected non-nil block") } + testT.Log("end TestGetBlock") step = 0 }