From 6597d789b77d930588ece023a98486198a3af737 Mon Sep 17 00:00:00 2001 From: Josh Rickmar Date: Mon, 2 Jun 2014 11:49:59 -0500 Subject: [PATCH] Avoid slice out-of-bounds indexing panic. The gettransaction handler was attempting to lookup the "sent-to" address of an outgoing transaction from the transaction store (as a wallet credit). This is the incorrect address when sending to an address controlled by another wallet, and panics when there are no credits (for example, sending to another wallet without any change address). Instead, use the first non-change output address is used as the address of the "send" result. This fixes the panic reported when debugging issue #91. While here, fix the category strings used for wallet credits to support immature and generate (the categories for coinbase outputs). --- rpcserver.go | 44 +++++++++++++++++++------------------- txstore/json.go | 56 ++++++++++++++++++++++++++++++++++++------------- 2 files changed, 64 insertions(+), 36 deletions(-) diff --git a/rpcserver.go b/rpcserver.go index bfac69d..16a8d4b 100644 --- a/rpcserver.go +++ b/rpcserver.go @@ -950,6 +950,8 @@ func GetReceivedByAccount(icmd btcjson.Cmd) (interface{}, *btcjson.Error) { return amt, nil } +// GetTransaction handles a gettransaction request by returning details about +// a single transaction saved by wallet. func GetTransaction(icmd btcjson.Cmd) (interface{}, *btcjson.Error) { // Type assert icmd to access parameters. cmd, ok := icmd.(*btcjson.GetTransactionCmd) @@ -957,19 +959,28 @@ func GetTransaction(icmd btcjson.Cmd) (interface{}, *btcjson.Error) { return nil, &btcjson.ErrInternal } - txsha, err := btcwire.NewShaHashFromStr(cmd.Txid) + txSha, err := btcwire.NewShaHashFromStr(cmd.Txid) if err != nil { return nil, &btcjson.ErrDecodeHexString } - accumulatedTxen := AcctMgr.GetTransaction(txsha) + accumulatedTxen := AcctMgr.GetTransaction(txSha) if len(accumulatedTxen) == 0 { return nil, &btcjson.ErrNoTxInfo } + bs, err := GetCurBlock() + if err != nil { + return nil, &btcjson.Error{ + Code: btcjson.ErrWallet.Code, + Message: err.Error(), + } + } + received := btcutil.Amount(0) var debitTx *txstore.TxRecord var debitAccount string + var targetAddr string ret := btcjson.GetTransactionResult{ Details: []btcjson.GetTransactionDetailsResult{}, @@ -991,16 +1002,16 @@ func GetTransaction(icmd btcjson.Cmd) (interface{}, *btcjson.Error) { _, addrs, _, _ := cred.Addresses(activeNet.Params) if len(addrs) == 1 { addr = addrs[0].EncodeAddress() + // The first non-change output address is considered the + // target for sent transactions. + if targetAddr == "" { + targetAddr = addr + } } details = append(details, btcjson.GetTransactionDetailsResult{ - Account: e.Account, - // TODO(oga) We don't mine for now so there - // won't be any special coinbase types. If the - // tx is a coinbase then we should handle it - // specially with the category depending on - // whether it is an orphan or in the blockchain. - Category: "receive", + Account: e.Account, + Category: cred.Category(bs.Height).String(), Amount: cred.Amount().ToUnit(btcutil.AmountBTC), Address: addr, }) @@ -1020,17 +1031,12 @@ func GetTransaction(icmd btcjson.Cmd) (interface{}, *btcjson.Error) { totalAmount -= debits.InputAmount() info := btcjson.GetTransactionDetailsResult{ Account: debitAccount, + Address: targetAddr, Category: "send", // negative since it is a send Amount: (-debits.OutputAmount(true)).ToUnit(btcutil.AmountBTC), Fee: debits.Fee().ToUnit(btcutil.AmountBTC), } - // Errors don't matter here, as we only consider the - // case where len(addrs) == 1. - _, addrs, _, _ := debitTx.Credits()[0].Addresses(activeNet.Params) - if len(addrs) == 1 { - info.Address = addrs[0].EncodeAddress() - } ret.Fee += info.Fee // Add sent information to front. ret.Details = append(ret.Details, info) @@ -1069,13 +1075,7 @@ func GetTransaction(icmd btcjson.Cmd) (interface{}, *btcjson.Error) { Message: err.Error(), } } - bs, err := GetCurBlock() - if err != nil { - return nil, &btcjson.Error{ - Code: btcjson.ErrWallet.Code, - Message: err.Error(), - } - } + ret.BlockIndex = int64(first.Tx.Tx().Index()) ret.BlockHash = txBlock.Hash.String() ret.BlockTime = txBlock.Time.Unix() diff --git a/txstore/json.go b/txstore/json.go index 666b8ac..517ff22 100644 --- a/txstore/json.go +++ b/txstore/json.go @@ -24,7 +24,7 @@ import ( "github.com/conformal/btcutil" ) -// ToJSON returns a slice of btcjson listtransaction result types for all credits +// ToJSON returns a slice of btcjson listtransactions result types for all credits // and debits of this transaction. func (t *TxRecord) ToJSON(account string, chainHeight int32, net *btcnet.Params) ([]btcjson.ListTransactionsResult, error) { @@ -90,6 +90,46 @@ func (d *Debits) ToJSON(account string, chainHeight int32, return reply, nil } +// CreditCategory describes the type of wallet transaction output. The category +// of "sent transactions" (debits) is always "send", and is not expressed by +// this type. +type CreditCategory int + +// These constants define the possible credit categories. +const ( + CreditReceive CreditCategory = iota + CreditGenerate + CreditImmature +) + +// Category returns the category of the credit. The passed block chain height is +// used to distinguish immature from mature coinbase outputs. +func (c *Credit) Category(chainHeight int32) CreditCategory { + if c.IsCoinbase() { + if c.Confirmed(btcchain.CoinbaseMaturity, chainHeight) { + return CreditGenerate + } + return CreditImmature + } + return CreditReceive +} + +// String returns the category as a string. This string may be used as the +// JSON string for categories as part of listtransactions and gettransaction +// RPC responses. +func (c CreditCategory) String() string { + switch c { + case CreditReceive: + return "receive" + case CreditGenerate: + return "generate" + case CreditImmature: + return "immature" + default: + return "unknown" + } +} + // ToJSON returns a slice of objects that may be marshaled as a JSON array // of JSON objects for a listtransactions RPC reply. func (c *Credit) ToJSON(account string, chainHeight int32, @@ -104,21 +144,9 @@ func (c *Credit) ToJSON(account string, chainHeight int32, address = addrs[0].EncodeAddress() } - var category string - switch { - case c.IsCoinbase(): - if c.Confirmed(btcchain.CoinbaseMaturity, chainHeight) { - category = "generate" - } else { - category = "immature" - } - default: - category = "receive" - } - result := btcjson.ListTransactionsResult{ Account: account, - Category: category, + Category: c.Category(chainHeight).String(), Address: address, Amount: btcutil.Amount(txout.Value).ToUnit(btcutil.AmountBTC), TxID: c.Tx().Sha().String(),