From 2e857bdac9f61721a05e854b0a12efba924c4133 Mon Sep 17 00:00:00 2001 From: Jonathan Chappelow Date: Thu, 5 May 2022 08:51:33 -0500 Subject: [PATCH] remove Details field from GetTransactionResult This simplifies the GetTransactionResult type in wallettypes.go. Several fields from the bitcoind/btcwallet response that are both unneeded and difficult to satisfy are now omitted. e.g. the native btcwallet and an external Electrum backend would have to jump through hoops to reproduce certain fields that are not strictly necessary. Namely, the Amount, Fee, and Details fields. All of the required data can be obtained by decoding the raw transaction contined in the Hex field. --- client/asset/btc/btc.go | 80 ++++++++++++++++++++++----------- client/asset/btc/btc_test.go | 27 +++-------- client/asset/btc/spv.go | 77 ++++++++----------------------- client/asset/btc/wallettypes.go | 55 +++++++++-------------- 4 files changed, 98 insertions(+), 141 deletions(-) diff --git a/client/asset/btc/btc.go b/client/asset/btc/btc.go index 41f934270b..bdf3dac6ff 100644 --- a/client/asset/btc/btc.go +++ b/client/asset/btc/btc.go @@ -1819,7 +1819,7 @@ func (btc *baseWallet) FundingCoins(ids []dex.Bytes) (asset.Coins, error) { if err != nil { return nil, err } -outer: + for _, rpcOP := range lockedOutpoints { txHash, err := chainhash.NewHashFromStr(rpcOP.TxID) if err != nil { @@ -1830,34 +1830,62 @@ outer: continue } - tx, err := btc.node.getWalletTransaction(txHash) - if err != nil { - return nil, err + var txRaw []byte + if fast, ok := btc.node.(walletTxChecker); ok { + txRaw, _, err = fast.checkWalletTx(txHash.String()) + if err != nil { + btc.log.Warnf("checkWalletTx: %v", err) + } // fallback to getWalletTransaction } - for _, item := range tx.Details { - if item.Vout != rpcOP.Vout { - continue - } - if item.Amount <= 0 { - return nil, fmt.Errorf("unexpected debit at %s:%v", txHash, rpcOP.Vout) - } - - utxo := &utxo{ - txHash: txHash, - vout: rpcOP.Vout, - address: item.Address, - amount: toSatoshi(item.Amount), - } - coin := newOutput(txHash, rpcOP.Vout, toSatoshi(item.Amount)) - coins = append(coins, coin) - btc.fundingCoins[pt] = utxo - delete(notFound, pt) - if len(notFound) == 0 { - return coins, nil + if len(txRaw) == 0 { + tx, err := btc.node.getWalletTransaction(txHash) // maybe getTxOut()? we don't need block info or the full tx + if err != nil { + return nil, err } - - continue outer + txRaw = tx.Hex } + msgTx, err := msgTxFromBytes(txRaw) + if err != nil { + btc.log.Warnf("Invalid transaction %v (%x): %v", txHash, txRaw, err) + continue + } + if rpcOP.Vout >= uint32(len(msgTx.TxOut)) { + btc.log.Warnf("Invalid vout %d for %v", rpcOP.Vout, txHash) + continue + } + txOut := msgTx.TxOut[rpcOP.Vout] + if txOut.Value <= 0 { + btc.log.Warnf("Invalid value %v for %v", txOut.Value, pt) + continue // try the listunspent output + } + _, addrs, _, err := txscript.ExtractPkScriptAddrs(txOut.PkScript, btc.chainParams) + if err != nil { + btc.log.Warnf("Invalid pkScript for %v: %v", pt, err) + continue + } + if len(addrs) != 1 { + btc.log.Warnf("pkScript for %v contains %d addresses instead of one: %v", pt, len(addrs)) + continue + } + addrStr, err := btc.stringAddr(addrs[0], btc.chainParams) + if err != nil { + btc.log.Errorf("Failed to stringify address %v (default encoding): %v", addrs[0], err) + addrStr = addrs[0].String() // may or may not be able to retrieve the private keys by address! + } + utxo := &utxo{ + txHash: txHash, + vout: rpcOP.Vout, + address: addrStr, // for retrieving private key by address string + amount: uint64(txOut.Value), + } + coin := newOutput(txHash, rpcOP.Vout, uint64(txOut.Value)) + coins = append(coins, coin) + btc.fundingCoins[pt] = utxo + delete(notFound, pt) + if len(notFound) == 0 { + return coins, nil + } + return nil, fmt.Errorf("funding coin %s:%v not found", txHash, rpcOP.Vout) } diff --git a/client/asset/btc/btc_test.go b/client/asset/btc/btc_test.go index 268756f552..564e3ac4c8 100644 --- a/client/asset/btc/btc_test.go +++ b/client/asset/btc/btc_test.go @@ -765,13 +765,7 @@ func testAvailableFund(t *testing.T, segwit bool, walletType string) { node.getTransaction = &GetTransactionResult{ BlockHash: blockHash.String(), BlockIndex: blockHeight, - Details: []*WalletTxDetails{ - { - Amount: float64(lockedVal) / 1e8, - Vout: 1, - }, - }, - Hex: txBuf.Bytes(), + Hex: txBuf.Bytes(), } bal, err = wallet.Balance() @@ -1209,13 +1203,9 @@ func testFundingCoins(t *testing.T, segwit bool, walletType string) { {TxID: p2pkhUnspent.TxID, Vout: p2pkhUnspent.Vout}, } node.listUnspent = []*ListUnspentResult{} + txRaw, _ := serializeMsgTx(tx) getTxRes := &GetTransactionResult{ - Details: []*WalletTxDetails{ - { - Vout: p2pkhUnspent.Vout, - Amount: p2pkhUnspent.Amount, - }, - }, + Hex: txRaw, } node.getTransaction = getTxRes @@ -2041,7 +2031,7 @@ func testFindRedemption(t *testing.T, segwit bool, walletType string) { otherTxHash, _ := chainhash.NewHashFromStr(otherTxid) contractVout := uint32(1) - secret, _, pkScript, contract, addr, contractAddr, _ := makeSwapContract(segwit, time.Hour*12) + secret, _, pkScript, contract, addr, _, _ := makeSwapContract(segwit, time.Hour*12) otherScript, _ := txscript.PayToAddrScript(addr) var redemptionWitness, otherWitness [][]byte @@ -2071,14 +2061,7 @@ func testFindRedemption(t *testing.T, segwit bool, walletType string) { getTxRes := &GetTransactionResult{ BlockHash: blockHash.String(), BlockIndex: contractHeight, - Details: []*WalletTxDetails{ - { - Address: contractAddr.String(), - Category: TxCatSend, - Vout: contractVout, - }, - }, - Hex: txHex, + Hex: txHex, } node.getTransaction = getTxRes diff --git a/client/asset/btc/spv.go b/client/asset/btc/spv.go index df25114cae..24058d755f 100644 --- a/client/asset/btc/spv.go +++ b/client/asset/btc/spv.go @@ -1821,70 +1821,31 @@ func (w *spvWallet) getWalletTransaction(txHash *chainhash.Hash) (*GetTransactio ret.Confirmations = uint64(confirms(details.Block.Height, syncBlock.Height)) } - var ( - debitTotal btcutil.Amount - creditTotal btcutil.Amount // Excludes change - fee btcutil.Amount - feeF64 float64 - ) - for _, deb := range details.Debits { - debitTotal += deb.Amount - } - for _, cred := range details.Credits { - if !cred.Change { - creditTotal += cred.Amount - } - } - // Fee can only be determined if every input is a debit. - if len(details.Debits) == len(details.MsgTx.TxIn) { - var outputTotal btcutil.Amount - for _, output := range details.MsgTx.TxOut { - outputTotal += btcutil.Amount(output.Value) - } - fee = debitTotal - outputTotal - feeF64 = fee.ToBTC() - } - - if len(details.Debits) == 0 { - // Credits must be set later, but since we know the full length - // of the details slice, allocate it with the correct cap. - ret.Details = make([]*WalletTxDetails, 0, len(details.Credits)) - } else { - ret.Details = make([]*WalletTxDetails, 1, len(details.Credits)+1) + return ret, nil - ret.Details[0] = &WalletTxDetails{ - Category: "send", - Amount: (-debitTotal).ToBTC(), // negative since it is a send - Fee: feeF64, + /* + var debitTotal, creditTotal btcutil.Amount // credits excludes change + for _, deb := range details.Debits { + debitTotal += deb.Amount } - ret.Fee = feeF64 - } - - credCat := wallet.RecvCategory(details, syncBlock.Height, w.chainParams).String() - for _, cred := range details.Credits { - // Change is ignored. - if cred.Change { - continue + for _, cred := range details.Credits { + if !cred.Change { + creditTotal += cred.Amount + } } - var address string - _, addrs, _, err := txscript.ExtractPkScriptAddrs( - details.MsgTx.TxOut[cred.Index].PkScript, w.chainParams) - if err == nil && len(addrs) == 1 { - addr := addrs[0] - address = addr.EncodeAddress() + // Fee can only be determined if every input is a debit. + if len(details.Debits) == len(details.MsgTx.TxIn) { + var outputTotal btcutil.Amount + for _, output := range details.MsgTx.TxOut { + outputTotal += btcutil.Amount(output.Value) + } + ret.Fee = (debitTotal - outputTotal).ToBTC() } - ret.Details = append(ret.Details, &WalletTxDetails{ - Address: address, - Category: WalletTxCategory(credCat), - Amount: cred.Amount.ToBTC(), - Vout: cred.Index, - }) - } - - ret.Amount = creditTotal.ToBTC() - return ret, nil + ret.Amount = creditTotal.ToBTC() + return ret, nil + */ } // walletExtender gives us access to a handful of fields or methods on diff --git a/client/asset/btc/wallettypes.go b/client/asset/btc/wallettypes.go index 38a74c940a..71a41c1bb0 100644 --- a/client/asset/btc/wallettypes.go +++ b/client/asset/btc/wallettypes.go @@ -55,42 +55,27 @@ type SignTxError struct { Error string `json:"error"` } -// GetTransactionResult models the data from the gettransaction command. +// GetTransactionResult models the required data from the gettransaction +// command. Several fields from the bitcoind/btcwallet response that are both +// unneeded and difficult to satisfy are omitted. e.g. the native btcwallet and +// the external Electrum backends must jump through hoops to reproduce certain +// fields that are not strictly necessary. type GetTransactionResult struct { - Amount float64 `json:"amount"` - Fee float64 `json:"fee"` - Confirmations uint64 `json:"confirmations"` - BlockHash string `json:"blockhash"` - BlockIndex int64 `json:"blockindex"` - BlockTime uint64 `json:"blocktime"` - TxID string `json:"txid"` - Time uint64 `json:"time"` - TimeReceived uint64 `json:"timereceived"` - BipReplaceable string `json:"bip125-replaceable"` - Hex dex.Bytes `json:"hex"` - Details []*WalletTxDetails `json:"details"` -} - -// WalletTxCategory is the tx output category set in WalletTxDetails. -type WalletTxCategory string - -const ( - TxCatSend WalletTxCategory = "send" - TxCatReceive WalletTxCategory = "receive" - TxCatGenerate WalletTxCategory = "generate" - TxCatImmature WalletTxCategory = "immature" - TxCatOrphan WalletTxCategory = "orphan" -) - -// WalletTxDetails models the details data from the gettransaction command. -type WalletTxDetails struct { - Address string `json:"address"` - Category WalletTxCategory `json:"category"` - Amount float64 `json:"amount"` - Label string `json:"label"` - Vout uint32 `json:"vout"` - Fee float64 `json:"fee"` - Abandoned bool `json:"abandoned"` + // The meaning of Amount and Fee for "wallet" transactions is iffy (and they + // are unused), so we are going to ignore them. We have the raw tx (Hex) if + // it is necessary to compute amounts from the inputs and outputs. + // Amount float64 `json:"amount"` + // Fee float64 `json:"fee"` + Confirmations uint64 `json:"confirmations"` + BlockHash string `json:"blockhash"` + BlockIndex int64 `json:"blockindex"` + BlockTime uint64 `json:"blocktime"` + TxID string `json:"txid"` + Time uint64 `json:"time"` + TimeReceived uint64 `json:"timereceived"` + Hex dex.Bytes `json:"hex"` // []byte, although it marshals/unmarshals a hex string + // BipReplaceable string `json:"bip125-replaceable"` // unused + // Details []*WalletTxDetails `json:"details"` // unused, and nearly impossible to satisfy in a generic interface } // RPCOutpoint is used to specify outputs to lock in calls to lockunspent.