Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

client/dcr: Remove GetRawTransaction. #3012

Merged
merged 1 commit into from
Oct 30, 2024
Merged

client/dcr: Remove GetRawTransaction. #3012

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
98 changes: 43 additions & 55 deletions client/asset/dcr/dcr.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3539,15 +3539,11 @@ func (dcr *ExchangeWallet) lookupTxOutput(ctx context.Context, txHash *chainhash
if err != nil {
return nil, 0, 0, err // asset.CoinNotFoundError if not found
}
msgTx, err := msgTxFromHex(tx.Hex)
if err != nil {
return nil, 0, 0, fmt.Errorf("invalid hex for tx %s: %v", txHash, err)
}
if int(vout) >= len(msgTx.TxOut) {
if int(vout) >= len(tx.MsgTx.TxOut) {
return nil, 0, 0, fmt.Errorf("tx %s has no output at %d", txHash, vout)
}

txOut = msgTx.TxOut[vout]
txOut = tx.MsgTx.TxOut[vout]
confs = uint32(tx.Confirmations)

// We have the requested output. Check if it is spent.
Expand Down Expand Up @@ -3716,15 +3712,11 @@ func (dcr *ExchangeWallet) queueFindRedemptionRequest(ctx context.Context, contr
if err != nil {
return nil, nil, err
}
msgTx, err := msgTxFromHex(tx.Hex)
if err != nil {
return nil, nil, fmt.Errorf("invalid contract tx hex %s: %w", tx.Hex, err)
}
if int(vout) > len(msgTx.TxOut)-1 {
if int(vout) > len(tx.MsgTx.TxOut)-1 {
return nil, nil, fmt.Errorf("vout index %d out of range for transaction %s", vout, txHash)
}
contractScript := msgTx.TxOut[vout].PkScript
contractScriptVer := msgTx.TxOut[vout].Version
contractScript := tx.MsgTx.TxOut[vout].PkScript
contractScriptVer := tx.MsgTx.TxOut[vout].Version
if !stdscript.IsScriptHashScript(contractScriptVer, contractScript) {
return nil, nil, fmt.Errorf("coin %s not a valid contract", contractOutpoint.String())
}
Expand Down Expand Up @@ -3787,12 +3779,12 @@ func (dcr *ExchangeWallet) findRedemptionsInMempool(contractOutpoints []outPoint
}

for _, txHash := range mempoolTxs {
tx, err := dcr.wallet.GetRawTransaction(dcr.ctx, txHash)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't getrawtransaction work for mempool transaction even if txindex is not enabled?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, for mempool transactions.

Should we leave it in with a note that it can only be used for mempool?

My reasoning is that if GetTransaction is good enough for rpc clients with an spv node, then it's good enough for full nodes.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to note for testing: the dcr simnet test harness node alpha has --txindex; node beta does not have a tx index.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

beta wallet is also an --spv wallet so I dont think we have a simnet wallet connecting to beta node

tx, err := dcr.wallet.GetTransaction(dcr.ctx, txHash)
if err != nil {
logAbandon(fmt.Sprintf("getrawtransaction error for tx hash %v: %v", txHash, err))
return
}
found, canceled := dcr.findRedemptionsInTx("mempool", tx, contractOutpoints)
found, canceled := dcr.findRedemptionsInTx("mempool", tx.MsgTx, contractOutpoints)
totalFound += found
totalCanceled += canceled
if totalFound+totalCanceled == contractsCount {
Expand Down Expand Up @@ -4588,11 +4580,7 @@ func (dcr *ExchangeWallet) FindBond(ctx context.Context, coinID []byte, searchUn
// to this wallet, it should be found here.
tx, err := dcr.wallet.GetTransaction(ctx, txHash)
if err == nil {
msgTx, err := msgTxFromHex(tx.Hex)
if err != nil {
return nil, fmt.Errorf("invalid hex for tx %s: %v", txHash, err)
}
return decodeV0BondTx(msgTx)
return decodeV0BondTx(tx.MsgTx)
}
if !errors.Is(err, asset.CoinNotFoundError) {
dcr.log.Warnf("Unexpected error looking up bond output %v:%d", txHash, vout)
Expand Down Expand Up @@ -5937,52 +5925,52 @@ func isMixTx(tx *wire.MsgTx) (isMix bool, mixDenom int64) {

// idUnknownTx identifies the type and details of a transaction either made
// or received by the wallet.
func (dcr *ExchangeWallet) idUnknownTx(ctx context.Context, tx *ListTransactionsResult) (*asset.WalletTransaction, error) {
txHash, err := chainhash.NewHashFromStr(tx.TxID)
func (dcr *ExchangeWallet) idUnknownTx(ctx context.Context, ltxr *ListTransactionsResult) (*asset.WalletTransaction, error) {
txHash, err := chainhash.NewHashFromStr(ltxr.TxID)
if err != nil {
return nil, fmt.Errorf("error decoding tx hash %s: %v", tx.TxID, err)
return nil, fmt.Errorf("error decoding tx hash %s: %v", ltxr.TxID, err)
}
msgTx, err := dcr.wallet.GetRawTransaction(ctx, txHash)
tx, err := dcr.wallet.GetTransaction(ctx, txHash)
if err != nil {
return nil, fmt.Errorf("GetRawTransaction error: %v", err)
return nil, fmt.Errorf("GetTransaction error: %v", err)
}

var totalIn uint64
for _, txIn := range msgTx.TxIn {
for _, txIn := range tx.MsgTx.TxIn {
if txIn.ValueIn > 0 {
totalIn += uint64(txIn.ValueIn)
}
}

var totalOut uint64
for _, txOut := range msgTx.TxOut {
for _, txOut := range tx.MsgTx.TxOut {
totalOut += uint64(txOut.Value)
}

fee := rpcTxFee(tx)
fee := rpcTxFee(ltxr)
if fee == 0 && totalIn > totalOut {
fee = totalIn - totalOut
}

switch *tx.TxType {
switch *ltxr.TxType {
case walletjson.LTTTVote:
return &asset.WalletTransaction{
Type: asset.TicketVote,
ID: tx.TxID,
ID: ltxr.TxID,
Amount: totalOut,
Fees: fee,
}, nil
case walletjson.LTTTRevocation:
return &asset.WalletTransaction{
Type: asset.TicketRevocation,
ID: tx.TxID,
ID: ltxr.TxID,
Amount: totalOut,
Fees: fee,
}, nil
case walletjson.LTTTTicket:
return &asset.WalletTransaction{
Type: asset.TicketPurchase,
ID: tx.TxID,
ID: ltxr.TxID,
Amount: totalOut,
Fees: fee,
}, nil
Expand All @@ -6003,11 +5991,11 @@ func (dcr *ExchangeWallet) idUnknownTx(ctx context.Context, tx *ListTransactions
BondID: pkHash[:],
}
}
if isBond, bondInfo := txIsBond(msgTx); isBond {
if isBond, bondInfo := txIsBond(tx.MsgTx); isBond {
return &asset.WalletTransaction{
Type: asset.CreateBond,
ID: tx.TxID,
Amount: uint64(msgTx.TxOut[0].Value),
ID: ltxr.TxID,
Amount: uint64(tx.MsgTx.TxOut[0].Value),
Fees: fee,
BondInfo: bondInfo,
}, nil
Expand All @@ -6023,10 +6011,10 @@ func (dcr *ExchangeWallet) idUnknownTx(ctx context.Context, tx *ListTransactions
}
return
}
if v := txPaysToScriptHash(msgTx); v > 0 {
if v := txPaysToScriptHash(tx.MsgTx); v > 0 {
return &asset.WalletTransaction{
Type: asset.SwapOrSend,
ID: tx.TxID,
ID: ltxr.TxID,
Amount: v,
Fees: fee,
}, nil
Expand Down Expand Up @@ -6059,21 +6047,21 @@ func (dcr *ExchangeWallet) idUnknownTx(ctx context.Context, tx *ListTransactions
redeemsSwap := func(msgTx *wire.MsgTx) bool {
return containsContractAtPushIndex(msgTx, 4, contractIsSwap)
}
if redeemsSwap(msgTx) {
if redeemsSwap(tx.MsgTx) {
return &asset.WalletTransaction{
Type: asset.Redeem,
ID: tx.TxID,
ID: ltxr.TxID,
Amount: totalOut + fee,
Fees: fee,
}, nil
}
refundsSwap := func(msgTx *wire.MsgTx) bool {
return containsContractAtPushIndex(msgTx, 3, contractIsSwap)
}
if refundsSwap(msgTx) {
if refundsSwap(tx.MsgTx) {
return &asset.WalletTransaction{
Type: asset.Refund,
ID: tx.TxID,
ID: ltxr.TxID,
Amount: totalOut + fee,
Fees: fee,
}, nil
Expand All @@ -6097,10 +6085,10 @@ func (dcr *ExchangeWallet) idUnknownTx(ctx context.Context, tx *ListTransactions
}
return containsContractAtPushIndex(msgTx, 2, isBond), bondInfo
}
if isBondRedemption, bondInfo := redeemsBond(msgTx); isBondRedemption {
if isBondRedemption, bondInfo := redeemsBond(tx.MsgTx); isBondRedemption {
return &asset.WalletTransaction{
Type: asset.RedeemBond,
ID: tx.TxID,
ID: ltxr.TxID,
Amount: totalOut,
Fees: fee,
BondInfo: bondInfo,
Expand Down Expand Up @@ -6130,17 +6118,17 @@ func (dcr *ExchangeWallet) idUnknownTx(ctx context.Context, tx *ListTransactions
return true
}

if tx.Send && allOutputsPayUs(msgTx) && len(msgTx.TxIn) == 1 {
if ltxr.Send && allOutputsPayUs(tx.MsgTx) && len(tx.MsgTx.TxIn) == 1 {
return &asset.WalletTransaction{
Type: asset.Split,
ID: tx.TxID,
ID: ltxr.TxID,
Fees: fee,
}, nil
}

if isMix, mixDenom := isMixTx(msgTx); isMix {
if isMix, mixDenom := isMixTx(tx.MsgTx); isMix {
var mixedAmount uint64
for _, txOut := range msgTx.TxOut {
for _, txOut := range tx.MsgTx.TxOut {
if txOut.Value == mixDenom {
_, addrs := stdscript.ExtractAddrs(scriptVersion, txOut.PkScript, dcr.chainParams)
if err != nil {
Expand All @@ -6166,7 +6154,7 @@ func (dcr *ExchangeWallet) idUnknownTx(ctx context.Context, tx *ListTransactions

return &asset.WalletTransaction{
Type: asset.Mix,
ID: tx.TxID,
ID: ltxr.TxID,
Amount: mixedAmount,
Fees: fee,
}, nil
Expand Down Expand Up @@ -6224,30 +6212,30 @@ func (dcr *ExchangeWallet) idUnknownTx(ctx context.Context, tx *ListTransactions
return
}

in, out := txOutDirection(msgTx)
in, out := txOutDirection(tx.MsgTx)

if tx.Send {
if ltxr.Send {
txType := asset.Send
amt := out
if allOutputsPayUs(msgTx) {
if allOutputsPayUs(tx.MsgTx) {
txType = asset.SelfSend
amt = in
}
return &asset.WalletTransaction{
Type: txType,
ID: tx.TxID,
ID: ltxr.TxID,
Amount: amt,
Fees: fee,
Recipient: getRecipient(msgTx, false),
Recipient: getRecipient(tx.MsgTx, false),
}, nil
}

return &asset.WalletTransaction{
Type: asset.Receive,
ID: tx.TxID,
ID: ltxr.TxID,
Amount: in,
Fees: fee,
Recipient: getRecipient(msgTx, true),
Recipient: getRecipient(tx.MsgTx, true),
}, nil
}

Expand Down
33 changes: 17 additions & 16 deletions client/asset/dcr/dcr_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -450,17 +450,6 @@ func (c *tRPCClient) GetRawMempool(_ context.Context, txType chainjson.GetRawMem
return txHashes, nil
}

func (c *tRPCClient) GetRawTransaction(_ context.Context, txHash *chainhash.Hash) (*dcrutil.Tx, error) {
if c.rawTxErr != nil {
return nil, c.rawTxErr
}
tx, found := c.blockchain.rawTxs[*txHash]
if !found {
return nil, dcrjson.NewRPCError(dcrjson.ErrRPCNoTxInfo, "no test raw tx "+txHash.String())
}
return dcrutil.NewTx(tx.tx), nil
}

func (c *tRPCClient) GetBalanceMinConf(_ context.Context, account string, minConfirms int) (*walletjson.GetBalanceResult, error) {
return c.balanceResult, c.balanceErr
}
Expand Down Expand Up @@ -492,6 +481,18 @@ func (c *tRPCClient) GetTransaction(_ context.Context, txHash *chainhash.Hash) (
if c.walletTxFn != nil {
return c.walletTxFn()
}
c.blockchain.mtx.RLock()
defer c.blockchain.mtx.RUnlock()
if rawTx, has := c.blockchain.rawTxs[*txHash]; has {
b, err := rawTx.tx.Bytes()
if err != nil {
return nil, err
}
walletTx := &walletjson.GetTransactionResult{
Hex: hex.EncodeToString(b),
}
return walletTx, nil
}
return nil, dcrjson.NewRPCError(dcrjson.ErrRPCNoTxInfo, "no test transaction")
}

Expand Down Expand Up @@ -3200,10 +3201,6 @@ func TestFindRedemption(t *testing.T) {
Hex: txHex,
}

node.walletTxFn = func() (*walletjson.GetTransactionResult, error) {
return walletTx, nil
}

// Add an intermediate block for good measure.
node.blockchain.addRawTx(contractHeight+1, dummyTx())

Expand All @@ -3221,6 +3218,10 @@ func TestFindRedemption(t *testing.T) {
t.Fatalf("wrong secret. expected %x, got %x", secret, checkSecret)
}

node.walletTxFn = func() (*walletjson.GetTransactionResult, error) {
return walletTx, nil
}

// Move the redemption to a new block and check if wallet.FindRedemption finds it.
_, redeemBlock := node.blockchain.addRawTx(contractHeight+2, makeRawTx(inputs, []dex.Bytes{otherScript}))
_, checkSecret, err = wallet.FindRedemption(tCtx, coinID, nil)
Expand Down Expand Up @@ -4654,7 +4655,7 @@ func TestFindBond(t *testing.T) {
}, {
name: "bad msgtx",
coinID: bond.CoinID,
txRes: txFn(nil, bond.SignedTx[1:]),
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have not idea why I had to change this, maybe I should.

txRes: txFn(nil, bond.SignedTx[100:]),
wantErr: true,
}, {
name: "get best block error",
Expand Down
33 changes: 5 additions & 28 deletions client/asset/dcr/rpcwallet.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,6 @@ type rpcClient interface {
GetBlockHeaderVerbose(ctx context.Context, blockHash *chainhash.Hash) (*chainjson.GetBlockHeaderVerboseResult, error)
GetBlockHeader(ctx context.Context, blockHash *chainhash.Hash) (*wire.BlockHeader, error)
GetRawMempool(ctx context.Context, txType chainjson.GetRawMempoolTxTypeCmd) ([]*chainhash.Hash, error)
GetRawTransaction(ctx context.Context, txHash *chainhash.Hash) (*dcrutil.Tx, error)
LockUnspent(ctx context.Context, unlock bool, ops []*wire.OutPoint) error
GetRawChangeAddress(ctx context.Context, account string, net stdaddr.AddressParams) (stdaddr.Address, error)
GetNewAddressGapPolicy(ctx context.Context, account string, gap dcrwallet.GapPolicy) (stdaddr.Address, error)
Expand Down Expand Up @@ -755,40 +754,18 @@ func (w *rpcWallet) GetTransaction(ctx context.Context, txHash *chainhash.Hash)
}
return nil, fmt.Errorf("error finding transaction %s in wallet: %w", txHash, translateRPCCancelErr(err))
}
msgTx, err := msgTxFromHex(tx.Hex)
if err != nil {
return nil, fmt.Errorf("invalid tx hex %s: %w", tx.Hex, err)
}
return &WalletTransaction{
Confirmations: tx.Confirmations,
BlockHash: tx.BlockHash,
Details: tx.Details,
Hex: tx.Hex,
MsgTx: msgTx,
}, nil
}

// GetRawTransaction returns details of the tx with the provided hash. Returns
// asset.CoinNotFoundError if the tx is not found.
// Part of the Wallet interface.
func (w *rpcWallet) GetRawTransaction(ctx context.Context, txHash *chainhash.Hash) (*wire.MsgTx, error) {
if w.spvMode {
gtr, err := w.rpcClient.GetTransaction(ctx, txHash)
if err != nil {
return nil, err
}

txB, err := hex.DecodeString(gtr.Hex)
if err != nil {
return nil, err
}

return msgTxFromBytes(txB)
}

utilTx, err := w.rpcClient.GetRawTransaction(ctx, txHash)
if err != nil {
return nil, err
}

return utilTx.MsgTx(), nil
}

func (w *rpcWallet) ListSinceBlock(ctx context.Context, start int32) ([]ListTransactionsResult, error) {
hash, err := w.GetBlockHash(ctx, int64(start))
if err != nil {
Expand Down
Loading
Loading