Uniformly return height 0 for mempool RawTransaction results.

common/common.go

@@ -522,6 +522,43 @@ func GetBlockRange(cache *BlockCache, blockOut chan<- *walletrpc.CompactBlock, e
 	errOut <- nil
 }
 
+// ParseRawTransaction converts between the JSON result of a `zcashd`
+// `getrawtransaction` call and the `RawTransaction` protobuf type.
+//
+// Due to an error in the original protobuf definition, it is necessary to
+// reinterpret the result of the `getrawtransaction` RPC call. Zcashd will
+// return the int64 value `-1` for the height of transactions that appear in
+// the block index, but which are not mined in the main chain. `service.proto`
+// defines the height field of `RawTransaction` to be a `uint64`, and as such
+// we must map the response from the zcashd RPC API to be representable within
+// this space. Additionally, the `height` field will be absent for transactions
+// in the mempool, resulting in the default value of `0` being set. Therefore,
+// 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
+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
+		}
+
+		return &walletrpc.RawTransaction{
+			Data:   txBytes,
+			Height: uint64(txinfo.Height),
+		}, nil
+}
+
 func displayHash(hash []byte) string {
 	return hex.EncodeToString(parser.Reverse(hash))
 }

common/common_test.go

@@ -9,6 +9,7 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
+	"math"
 	"os"
 	"strings"
 	"testing"
@@ -604,7 +605,7 @@ func mempoolStub(method string, params []json.RawMessage) (json.RawMessage, erro
 		if txid != "mempooltxid-1" {
 			testT.Fatal("unexpected txid")
 		}
-		r, _ := json.Marshal("aabb")
+		r, _ := json.Marshal(map[string]string{"hex":"aabb"})
 		return r, nil
 	case 5:
 		// Simulate that still no new block has arrived ...
@@ -636,7 +637,7 @@ func mempoolStub(method string, params []json.RawMessage) (json.RawMessage, erro
 		if txid != "mempooltxid-2" {
 			testT.Fatal("unexpected txid")
 		}
-		r, _ := json.Marshal("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
@@ -668,7 +669,7 @@ func TestMempoolStream(t *testing.T) {
 		return nil
 	})
 	if err != nil {
-		t.Fatal("GetMempool failed")
+		t.Errorf("GetMempool failed: %v", err)
 	}
 
 	// This should return two transactions.
@@ -677,7 +678,7 @@ func TestMempoolStream(t *testing.T) {
 		return nil
 	})
 	if err != nil {
-		t.Fatal("GetMempool failed")
+		t.Errorf("GetMempool failed: %v", err)
 	}
 	if len(replies) != 2 {
 		t.Fatal("unexpected number of tx")
@@ -687,13 +688,13 @@ func TestMempoolStream(t *testing.T) {
 	if !bytes.Equal([]byte(replies[0].GetData()), []byte{0xaa, 0xbb}) {
 		t.Fatal("unexpected tx contents")
 	}
-	if replies[0].GetHeight() != 200 {
+	if replies[0].GetHeight() != 0 {
 		t.Fatal("unexpected tx height")
 	}
 	if !bytes.Equal([]byte(replies[1].GetData()), []byte{0xcc, 0xdd}) {
 		t.Fatal("unexpected tx contents")
 	}
-	if replies[1].GetHeight() != 200 {
+	if replies[1].GetHeight() != 0 {
 		t.Fatal("unexpected tx height")
 	}
 
@@ -710,3 +711,61 @@ func TestMempoolStream(t *testing.T) {
 	sleepCount = 0
 	sleepDuration = 0
 }
+
+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 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)
+		}
+}
+
+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)
+		}
+
+		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)
+		}
+}

common/mempool.go

@@ -1,7 +1,6 @@
 package common
 
 import (
-	"encoding/hex"
 	"encoding/json"
 	"sync"
 	"time"
@@ -105,35 +104,27 @@ func refreshMempoolTxns() error {
 			// We've already fetched this transaction
 			continue
 		}
-		g_txidSeen[txid(txidstr)] = struct{}{}
+
 		// We haven't fetched this transaction already.
+		g_txidSeen[txid(txidstr)] = struct{}{}
 		txidJSON, err := json.Marshal(txidstr)
 		if err != nil {
 			return err
 		}
-		// The "0" is because we only need the raw hex, which is returned as
-		// just a hex string, and not even a json string (with quotes).
-		params := []json.RawMessage{txidJSON, json.RawMessage("0")}
+
+		params := []json.RawMessage{txidJSON, json.RawMessage("1")}
 		result, rpcErr := RawRequest("getrawtransaction", params)
 		if rpcErr != nil {
 			// Not an error; mempool transactions can disappear
 			continue
 		}
-		// strip the quotes
-		var txStr string
-		err = json.Unmarshal(result, &txStr)
-		if err != nil {
-			return err
-		}
-		txBytes, err := hex.DecodeString(txStr)
+
+		rawtx, err := ParseRawTransaction(result)
 		if err != nil {
 			return err
 		}
-		newRtx := &walletrpc.RawTransaction{
-			Data:   txBytes,
-			Height: uint64(g_lastBlockChainInfo.Blocks),
-		}
-		g_txList = append(g_txList, newRtx)
+
+		g_txList = append(g_txList, rawtx)
 	}
 	return nil
 }

frontend/service.go

@@ -307,65 +307,26 @@ func (s *lwdStreamer) GetLatestTreeState(ctx context.Context, in *walletrpc.Empt
 	return s.GetTreeState(ctx, &walletrpc.BlockID{Height: uint64(latestHeight)})
 }
 
-// ParseRawTransaction converts between the JSON result of a `zcashd`
-// `getrawtransaction` call and the `RawTransaction` protobuf type.
-//
-// Due to an error in the original protobuf definition, it is necessary to
-// reinterpret the result of the `getrawtransaction` RPC call. Zcashd will
-// return the int64 value `-1` for the height of transactions that appear in
-// the block index, but which are not mined in the main chain. `service.proto`
-// defines the height field of `RawTransaction` to be a `uint64`, and as such
-// we must map the response from the zcashd RPC API to be representable within
-// this space. Additionally, the `height` field will be absent for transactions
-// in the mempool, resulting in the default value of `0` being set. Therefore,
-// 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
-func ParseRawTransaction(message json.RawMessage) (*walletrpc.RawTransaction, error) {
-		// Many other fields are returned, but we need only these two.
-		var txinfo common.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
-}
-
 // GetTransaction returns the raw transaction bytes that are returned
 // by the zcashd 'getrawtransaction' RPC.
 func (s *lwdStreamer) GetTransaction(ctx context.Context, txf *walletrpc.TxFilter) (*walletrpc.RawTransaction, error) {
 	if txf.Hash != nil {
 		if len(txf.Hash) != 32 {
 			return nil, errors.New("transaction ID has invalid length")
 		}
-		leHashStringJSON, err := json.Marshal(hex.EncodeToString(parser.Reverse(txf.Hash)))
+		txidJSON, err := json.Marshal(hex.EncodeToString(parser.Reverse(txf.Hash)))
 		if err != nil {
 			return nil, err
 		}
-		params := []json.RawMessage{
-			leHashStringJSON,
-			json.RawMessage("1"),
-		}
-		result, rpcErr := common.RawRequest("getrawtransaction", params)
 
-		// For some reason, the error responses are not JSON
+		params := []json.RawMessage{txidJSON, json.RawMessage("1")}
+		result, rpcErr := common.RawRequest("getrawtransaction", params)
 		if rpcErr != nil {
+			// For some reason, the error responses are not JSON
 			return nil, rpcErr
 		}
-		return ParseRawTransaction(result)
+
+		return common.ParseRawTransaction(result)
 	}
 
 	if txf.Block != nil && txf.Block.Hash != nil {