From b5f3f6348df4f6f4a2690a8f8806c571557fe642 Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Tue, 13 Aug 2024 11:51:14 -0600 Subject: [PATCH] Uniformly return height 0 for mempool `RawTransaction` results. --- common/common.go | 37 +++++++++++++++++++++ common/common_test.go | 71 ++++++++++++++++++++++++++++++++++++---- common/mempool.go | 25 +++++--------- frontend/service.go | 51 ++++------------------------- frontend/service_test.go | 68 -------------------------------------- 5 files changed, 116 insertions(+), 136 deletions(-) delete mode 100644 frontend/service_test.go diff --git a/common/common.go b/common/common.go index 3c8a5416..82b0155a 100644 --- a/common/common.go +++ b/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)) } diff --git a/common/common_test.go b/common/common_test.go index 9fc19f32..ee67a981 100644 --- a/common/common_test.go +++ b/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) + } +} diff --git a/common/mempool.go b/common/mempool.go index 241e0520..e641b8c6 100644 --- a/common/mempool.go +++ b/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 } diff --git a/frontend/service.go b/frontend/service.go index 8e20bf01..991b1d1e 100644 --- a/frontend/service.go +++ b/frontend/service.go @@ -307,43 +307,6 @@ 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) { @@ -351,21 +314,19 @@ func (s *lwdStreamer) GetTransaction(ctx context.Context, txf *walletrpc.TxFilte 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 { diff --git a/frontend/service_test.go b/frontend/service_test.go deleted file mode 100644 index 552b8acb..00000000 --- a/frontend/service_test.go +++ /dev/null @@ -1,68 +0,0 @@ -package frontend - -import ( - "encoding/json" - "testing" - "math" - - "github.com/zcash/lightwalletd/common" -) - -func TestZcashdRpcReplyUnmarshalling(t *testing.T) { - var txinfo0 common.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 common.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 common.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) - } -} -