Skip to content

Commit

Permalink
remove Details field from GetTransactionResult
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@chappjc
chappjc committed May 6, 2022
1 parent 0bd21bd commit 2e857bd
Show file tree
Hide file tree
Showing 4 changed files with 98 additions and 141 deletions.
80 changes: 54 additions & 26 deletions client/asset/btc/btc.go
View file
Expand Up @@ -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 {
Expand All @@ -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)
}

Expand Down
27 changes: 5 additions & 22 deletions client/asset/btc/btc_test.go
View file
Expand Up @@ -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()
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand Down
77 changes: 19 additions & 58 deletions client/asset/btc/spv.go
View file
Expand Up @@ -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
Expand Down
55 changes: 20 additions & 35 deletions client/asset/btc/wallettypes.go
View file
Expand Up @@ -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.
Expand Down

0 comments on commit 2e857bd

Please sign in to comment.