return correct gRPC error codes

zcash · Dec 1, 2024 · fc4548b · fc4548b
1 parent 1e63bee
commit fc4548b
Show file tree

Hide file tree

Showing 8 changed files with 483 additions and 356 deletions.
diff --git a/cmd/root.go b/cmd/root.go
@@ -212,6 +212,11 @@ func startServer(opts *common.Options) error {
 				"error": err,
 			}).Fatal("getting initial information from zebrad or zcashd")
 		}
+		// Zebrad always supports lightwalletd (no special configuration needed);
+		// zcashd needs the lightwalletd=1 configuration option, so check that here.
+		if getLightdInfo.LightwalletdDisabled && !strings.Contains(getLightdInfo.ZcashdSubversion, "Zebra") {
+			common.Log.Fatal("zcashd is not configured as a lightwalletd server")
+		}
 		common.Log.Info("Got sapling height ", getLightdInfo.SaplingActivationHeight,
 			" block height ", getLightdInfo.BlockHeight,
 			" chain ", getLightdInfo.ChainName,

diff --git a/common/common.go b/common/common.go
@@ -16,6 +16,8 @@ import (
 	"github.com/sirupsen/logrus"
 	"github.com/zcash/lightwalletd/parser"
 	"github.com/zcash/lightwalletd/walletrpc"
+	"google.golang.org/grpc/codes"
+	"google.golang.org/grpc/status"
 )
 
 // 'make build' will overwrite this string with the output of git-describe (tag)
@@ -93,9 +95,13 @@ type (
 	}
 
 	// zcashd rpc "getinfo"
+	// If we're connected to an old version of zcashd that doesn't return
+	// LightwalletdDisabled, it will be false (the default bool value),
+	// and that will let us start up.
 	ZcashdRpcReplyGetinfo struct {
-		Build      string
-		Subversion string
+		Build                string
+		Subversion           string
+		LightwalletdDisabled bool
 	}
 
 	// zcashd rpc "getaddresstxids"
@@ -294,6 +300,7 @@ func GetLightdInfo() (*walletrpc.LightdInfo, error) {
 		EstimatedHeight:         uint64(getblockchaininfoReply.EstimatedHeight),
 		ZcashdBuild:             getinfoReply.Build,
 		ZcashdSubversion:        getinfoReply.Subversion,
+		LightwalletdDisabled:    getinfoReply.LightwalletdDisabled,
 	}, nil
 }
 
@@ -311,15 +318,13 @@ func getBlockFromRPC(height int) (*walletrpc.CompactBlock, error) {
 	if err != nil {
 		Log.Fatal("getBlockFromRPC bad height argument", height, err)
 	}
-	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
 	// 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")
+	params := []json.RawMessage{heightJSON, 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
@@ -331,14 +336,14 @@ func getBlockFromRPC(height int) (*walletrpc.CompactBlock, error) {
 	var block1 ZcashRpcReplyGetblock1
 	err = json.Unmarshal(result, &block1)
 	if err != nil {
-		return nil, err
+		Log.Fatal("getBlockFromRPC: Can't unmarshal block:", err)
 	}
 	blockHash, err := json.Marshal(block1.Hash)
 	if err != nil {
 		Log.Fatal("getBlockFromRPC bad block hash", block1.Hash)
 	}
-	params[0] = blockHash
-	params[1] = json.RawMessage("0") // non-verbose (raw hex)
+	// non-verbose (raw hex) version of block
+	params = []json.RawMessage{blockHash, json.RawMessage("0")}
 	result, rpcErr = RawRequest("getblock", params)
 
 	// For some reason, the error responses are not JSON
@@ -476,6 +481,7 @@ func BlockIngestor(c *BlockCache, rep int) {
 // GetBlock returns the compact block at the requested height, first by querying
 // the cache, then, if not found, will request the block from zcashd. It returns
 // nil if no block exists at this height.
+// This returns gRPC-compatible errors.
 func GetBlock(cache *BlockCache, height int) (*walletrpc.CompactBlock, error) {
 	// First, check the cache to see if we have the block
 	var block *walletrpc.CompactBlock
@@ -489,11 +495,13 @@ func GetBlock(cache *BlockCache, height int) (*walletrpc.CompactBlock, error) {
 	// Not in the cache
 	block, err := getBlockFromRPC(height)
 	if err != nil {
-		return nil, err
+		return nil, status.Errorf(codes.InvalidArgument,
+			"GetBlock: getblock failed, error: %s", err.Error())
 	}
 	if block == nil {
 		// Block height is too large
-		return nil, errors.New("block requested is newer than latest block")
+		return nil, status.Errorf(codes.OutOfRange,
+			"GetBlock: block requested is newer than latest block: %d", height)
 	}
 	return block, nil
 }
@@ -537,27 +545,27 @@ func GetBlockRange(cache *BlockCache, blockOut chan<- *walletrpc.CompactBlock, e
 // the meanings of the `Height` field of the `RawTransaction` type are as
 // follows:
 //
-// * height 0: the transaction is in the mempool
-// * height 0xffffffffffffffff: the transaction has been mined on a fork that
-//   is not currently the main chain
-// * any other height: the transaction has been mined in the main chain at the
-//   given height
+//   - height 0: the transaction is in the mempool
+//   - height 0xffffffffffffffff: the transaction has been mined on a fork that
+//     is not currently the main chain
+//   - any other height: the transaction has been mined in the main chain at the
+//     given height
 func ParseRawTransaction(message json.RawMessage) (*walletrpc.RawTransaction, error) {
-		// Many other fields are returned, but we need only these two.
-		var txinfo ZcashdRpcReplyGetrawtransaction
-		err := json.Unmarshal(message, &txinfo)
-		if err != nil {
-			return nil, err
-		}
-		txBytes, err := hex.DecodeString(txinfo.Hex)
-		if err != nil {
-			return nil, err
-		}
+	// Many other fields are returned, but we need only these two.
+	var txinfo ZcashdRpcReplyGetrawtransaction
+	err := json.Unmarshal(message, &txinfo)
+	if err != nil {
+		return nil, err
+	}
+	txBytes, err := hex.DecodeString(txinfo.Hex)
+	if err != nil {
+		return nil, err
+	}
 
-		return &walletrpc.RawTransaction{
-			Data:   txBytes,
-			Height: uint64(txinfo.Height),
-		}, nil
+	return &walletrpc.RawTransaction{
+		Data:   txBytes,
+		Height: uint64(txinfo.Height),
+	}, nil
 }
 
 func displayHash(hash []byte) string {

diff --git a/common/common_test.go b/common/common_test.go
@@ -439,7 +439,7 @@ func TestGetBlockRange(t *testing.T) {
 	select {
 	case err := <-errChan:
 		// this will also catch context.DeadlineExceeded from the timeout
-		if err.Error() != "block requested is newer than latest block" {
+		if !strings.Contains(err.Error(), "block requested is newer than latest block") {
 			t.Fatal("unexpected error:", err)
 		}
 	case <-blockChan:
@@ -605,7 +605,7 @@ func mempoolStub(method string, params []json.RawMessage) (json.RawMessage, erro
 		if txid != "mempooltxid-1" {
 			testT.Fatal("unexpected txid")
 		}
-		r, _ := json.Marshal(map[string]string{"hex":"aabb"})
+		r, _ := json.Marshal(map[string]string{"hex": "aabb"})
 		return r, nil
 	case 5:
 		// Simulate that still no new block has arrived ...
@@ -637,7 +637,7 @@ func mempoolStub(method string, params []json.RawMessage) (json.RawMessage, erro
 		if txid != "mempooltxid-2" {
 			testT.Fatal("unexpected txid")
 		}
-		r, _ := json.Marshal(map[string]string{"hex":"ccdd"})
+		r, _ := json.Marshal(map[string]string{"hex": "ccdd"})
 		return r, nil
 	case 8:
 		// A new block arrives, this will cause these two tx to be returned
@@ -713,59 +713,59 @@ func TestMempoolStream(t *testing.T) {
 }
 
 func TestZcashdRpcReplyUnmarshalling(t *testing.T) {
-		var txinfo0 ZcashdRpcReplyGetrawtransaction
-		err0 := json.Unmarshal([]byte("{\"hex\": \"deadbeef\", \"height\": 123456}"), &txinfo0)
-		if err0 != nil {
-			t.Fatal("Failed to unmarshal tx with known height.")
-		}
-		if txinfo0.Height != 123456 {
-			t.Errorf("Unmarshalled incorrect height: got: %d, want: 123456.", txinfo0.Height)
-		}
+	var txinfo0 ZcashdRpcReplyGetrawtransaction
+	err0 := json.Unmarshal([]byte("{\"hex\": \"deadbeef\", \"height\": 123456}"), &txinfo0)
+	if err0 != nil {
+		t.Fatal("Failed to unmarshal tx with known height.")
+	}
+	if txinfo0.Height != 123456 {
+		t.Errorf("Unmarshalled incorrect height: got: %d, want: 123456.", txinfo0.Height)
+	}
 
-		var txinfo1 ZcashdRpcReplyGetrawtransaction
-		err1 := json.Unmarshal([]byte("{\"hex\": \"deadbeef\", \"height\": -1}"), &txinfo1)
-		if err1 != nil {
-			t.Fatal("failed to unmarshal tx not in main chain")
-		}
-		if txinfo1.Height != -1 {
-			t.Errorf("Unmarshalled incorrect height: got: %d, want: -1.", txinfo1.Height)
-		}
+	var txinfo1 ZcashdRpcReplyGetrawtransaction
+	err1 := json.Unmarshal([]byte("{\"hex\": \"deadbeef\", \"height\": -1}"), &txinfo1)
+	if err1 != nil {
+		t.Fatal("failed to unmarshal tx not in main chain")
+	}
+	if txinfo1.Height != -1 {
+		t.Errorf("Unmarshalled incorrect height: got: %d, want: -1.", txinfo1.Height)
+	}
 
-		var txinfo2 ZcashdRpcReplyGetrawtransaction
-		err2 := json.Unmarshal([]byte("{\"hex\": \"deadbeef\"}"), &txinfo2)
-		if err2 != nil {
-			t.Fatal("failed to unmarshal reply lacking height data")
-		}
-		if txinfo2.Height != 0 {
-			t.Errorf("Unmarshalled incorrect height: got: %d, want: 0.", txinfo2.Height)
-		}
+	var txinfo2 ZcashdRpcReplyGetrawtransaction
+	err2 := json.Unmarshal([]byte("{\"hex\": \"deadbeef\"}"), &txinfo2)
+	if err2 != nil {
+		t.Fatal("failed to unmarshal reply lacking height data")
+	}
+	if txinfo2.Height != 0 {
+		t.Errorf("Unmarshalled incorrect height: got: %d, want: 0.", txinfo2.Height)
+	}
 }
 
 func TestParseRawTransaction(t *testing.T) {
-		rt0, err0 := ParseRawTransaction([]byte("{\"hex\": \"deadbeef\", \"height\": 123456}"))
-		if err0 != nil {
-			t.Fatal("Failed to parse raw transaction response with known height.")
-		}
-		if rt0.Height != 123456 {
-			t.Errorf("Unmarshalled incorrect height: got: %d, expected: 123456.", rt0.Height)
-		}
+	rt0, err0 := ParseRawTransaction([]byte("{\"hex\": \"deadbeef\", \"height\": 123456}"))
+	if err0 != nil {
+		t.Fatal("Failed to parse raw transaction response with known height.")
+	}
+	if rt0.Height != 123456 {
+		t.Errorf("Unmarshalled incorrect height: got: %d, expected: 123456.", rt0.Height)
+	}
 
-		rt1, err1 := ParseRawTransaction([]byte("{\"hex\": \"deadbeef\", \"height\": -1}"))
-		if err1 != nil {
-			t.Fatal("Failed to parse raw transaction response for a known tx not in the main chain.")
-		}
-		// We expect the int64 value `-1` to have been reinterpreted as a uint64 value in order
-		// to be representable as a uint64 in `RawTransaction`. The conversion from the twos-complement
-		// signed representation should map `-1` to `math.MaxUint64`.
-		if rt1.Height != math.MaxUint64 {
-			t.Errorf("Unmarshalled incorrect height: got: %d, want: 0x%X.", rt1.Height, uint64(math.MaxUint64))
-		}
+	rt1, err1 := ParseRawTransaction([]byte("{\"hex\": \"deadbeef\", \"height\": -1}"))
+	if err1 != nil {
+		t.Fatal("Failed to parse raw transaction response for a known tx not in the main chain.")
+	}
+	// We expect the int64 value `-1` to have been reinterpreted as a uint64 value in order
+	// to be representable as a uint64 in `RawTransaction`. The conversion from the twos-complement
+	// signed representation should map `-1` to `math.MaxUint64`.
+	if rt1.Height != math.MaxUint64 {
+		t.Errorf("Unmarshalled incorrect height: got: %d, want: 0x%X.", rt1.Height, uint64(math.MaxUint64))
+	}
 
-		rt2, err2 := ParseRawTransaction([]byte("{\"hex\": \"deadbeef\"}"))
-		if err2 != nil {
-			t.Fatal("Failed to parse raw transaction response for a tx in the mempool.")
-		}
-		if rt2.Height != 0 {
-			t.Errorf("Unmarshalled incorrect height: got: %d, expected: 0.", rt2.Height)
-		}
+	rt2, err2 := ParseRawTransaction([]byte("{\"hex\": \"deadbeef\"}"))
+	if err2 != nil {
+		t.Fatal("Failed to parse raw transaction response for a tx in the mempool.")
+	}
+	if rt2.Height != 0 {
+		t.Errorf("Unmarshalled incorrect height: got: %d, expected: 0.", rt2.Height)
+	}
 }
diff --git a/docs/rtd/index.html b/docs/rtd/index.html
@@ -1525,6 +1525,13 @@ <h3 id="cash.z.wallet.sdk.rpc.LightdInfo">LightdInfo</h3>
                   <td><p>example: &#34;/MagicBean:4.1.1/&#34; </p></td>
                 </tr>
 
+                <tr>
+                  <td>lightwalletdDisabled</td>
+                  <td><a href="#bool">bool</a></td>
+                  <td></td>
+                  <td><p>whether zcashd is configured as a lwd server </p></td>
+                </tr>
+
             </tbody>
           </table>
 

diff --git a/frontend/frontend_test.go b/frontend/frontend_test.go
@@ -115,7 +115,7 @@ func TestGetTransaction(t *testing.T) {
 	if err == nil {
 		testT.Fatal("GetTransaction unexpectedly succeeded")
 	}
-	if err.Error() != "please call GetTransaction with txid" {
+	if !strings.Contains(err.Error(), "GetTransaction: specify a txid") {
 		testT.Fatal("GetTransaction unexpected error message")
 	}
 	if rawtx != nil {
@@ -127,7 +127,7 @@ func TestGetTransaction(t *testing.T) {
 	if err == nil {
 		testT.Fatal("GetTransaction unexpectedly succeeded")
 	}
-	if err.Error() != "can't GetTransaction with a blockhash+num, please call GetTransaction with txid" {
+	if !strings.Contains(err.Error(), "GetTransaction: specify a txid, not a blockhash+num") {
 		testT.Fatal("GetTransaction unexpected error message")
 	}
 	if rawtx != nil {
@@ -302,7 +302,7 @@ func TestGetTaddressTxids(t *testing.T) {
 		if err == nil {
 			t.Fatal("GetTaddressTxids should have failed on bad address, case", i)
 		}
-		if err.Error() != "invalid address" {
+		if !strings.Contains(err.Error(), "invalid characters") {
 			t.Fatal("GetTaddressTxids incorrect error on bad address, case", i)
 		}
 	}
@@ -408,7 +408,7 @@ func TestGetBlock(t *testing.T) {
 	if err == nil {
 		t.Fatal("GetBlock should have failed")
 	}
-	if err.Error() != "gRPC GetBlock by Hash is not yet implemented" {
+	if !strings.Contains(err.Error(), "GetBlock: Block hash specifier is not yet implemented") {
 		t.Fatal("GetBlock hash unimplemented error message failed")
 	}