From 0207ed206455e20fc51353c17bb6f5d4c0f2bad1 Mon Sep 17 00:00:00 2001 From: Alex Yocom-Piatt Date: Fri, 7 May 2021 11:59:49 -0500 Subject: [PATCH 1/5] various feepayment fixes --- internal/vsp/feepayment.go | 31 +++++++++++++++++++++---------- wallet/wallet.go | 3 +++ 2 files changed, 24 insertions(+), 10 deletions(-) diff --git a/internal/vsp/feepayment.go b/internal/vsp/feepayment.go index 85e3e1412..52e856575 100644 --- a/internal/vsp/feepayment.go +++ b/internal/vsp/feepayment.go @@ -189,6 +189,7 @@ func (c *Client) feePayment(ticketHash *chainhash.Hash, policy Policy) (fp *feeP if fp == nil { return } + var schedule bool c.mu.Lock() fp2 := c.jobs[*ticketHash] if fp2 != nil { @@ -196,8 +197,12 @@ func (c *Client) feePayment(ticketHash *chainhash.Hash, policy Policy) (fp *feeP fp = fp2 } else { c.jobs[*ticketHash] = fp + schedule = true } c.mu.Unlock() + if schedule { + fp.schedule("reconcile payment", fp.reconcilePayment) + } }() ctx := context.Background() @@ -250,9 +255,9 @@ func (c *Client) feePayment(ticketHash *chainhash.Hash, policy Policy) (fp *feeP return nil } + fp.state = unprocessed // XXX fee created, but perhaps not submitted with vsp. feeHash, err := w.VSPFeeHashForTicket(ctx, ticketHash) if err != nil { - fp.state = unprocessed // caller must schedule next method, as paying the fee may // require using provided transaction inputs. return fp @@ -271,9 +276,7 @@ func (c *Client) feePayment(ticketHash *chainhash.Hash, policy Policy) (fp *feeP } fp.feeTx = fee - fp.fee = -1 // XXX fee amount (not needed anymore?) - fp.state = unprocessed // XXX fee created, but perhaps not submitted with vsp. - fp.schedule("reconcile payment", fp.reconcilePayment) + fp.fee = -1 // XXX fee amount (not needed anymore?) return fp } @@ -427,8 +430,6 @@ func (fp *feePayment) receiveFeeAddress() error { fp.feeAddr = feeAddr fp.mu.Unlock() - _ = fp.makeFeeTx(nil) - fp.schedule("reconcile payment", fp.reconcilePayment) return nil } @@ -471,8 +472,6 @@ func (fp *feePayment) makeFeeTx(tx *wire.MsgTx) error { // XXX fp.fee == -1? if fee == 0 { - // XXX locking - // this schedules paying the fee err := fp.receiveFeeAddress() if err != nil { return err @@ -560,12 +559,17 @@ func (fp *feePayment) makeFeeTx(tx *wire.MsgTx) error { // sign sigErrs, err := w.SignTransaction(ctx, tx, txscript.SigHashAll, nil, nil, nil) - if err != nil { + if err != nil || len(sigErrs) > 0 { log.Errorf("failed to sign transaction: %v", err) + sigErrStr := "" for _, sigErr := range sigErrs { log.Errorf("\t%v", sigErr) + sigErrStr = fmt.Sprintf("\t%v", sigErr) + " " } - return err + if err != nil { + return err + } + return fmt.Errorf(sigErrStr) } err = w.SetPublished(ctx, &feeHash, false) @@ -731,6 +735,13 @@ func (fp *feePayment) reconcilePayment() error { if feeTx == nil || len(feeTx.TxOut) == 0 { err := fp.makeFeeTx(nil) if err != nil { + var apiErr *BadRequestError + if errors.As(err, &apiErr) { + if apiErr.Code == codeTicketCannotVote { + fp.remove("ticket cannot vote, needs to be revoked") + return nil + } + } fp.schedule("reconcile payment", fp.reconcilePayment) return err } diff --git a/wallet/wallet.go b/wallet/wallet.go index e5db49b97..68b89a2ef 100644 --- a/wallet/wallet.go +++ b/wallet/wallet.go @@ -5481,6 +5481,9 @@ func (w *Wallet) VSPFeeHashForTicket(ctx context.Context, ticketHash *chainhash. feeHash = data.FeeHash return nil }) + if err == nil && feeHash == (chainhash.Hash{}) { + err = errors.E(errors.NotExist) + } return feeHash, err } From f5a4fbd46e4dbd17e9b1f750e9bb388f48e39a0d Mon Sep 17 00:00:00 2001 From: Alex Yocom-Piatt Date: Fri, 7 May 2021 12:39:52 -0500 Subject: [PATCH 2/5] add revoke attempt --- internal/vsp/feepayment.go | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/internal/vsp/feepayment.go b/internal/vsp/feepayment.go index 52e856575..381261bc1 100644 --- a/internal/vsp/feepayment.go +++ b/internal/vsp/feepayment.go @@ -12,6 +12,7 @@ import ( "decred.org/dcrwallet/v2/errors" "decred.org/dcrwallet/v2/internal/uniformprng" + "decred.org/dcrwallet/v2/rpc/client/dcrd" "decred.org/dcrwallet/v2/wallet" "decred.org/dcrwallet/v2/wallet/txrules" "decred.org/dcrwallet/v2/wallet/txsizes" @@ -738,11 +739,26 @@ func (fp *feePayment) reconcilePayment() error { var apiErr *BadRequestError if errors.As(err, &apiErr) { if apiErr.Code == codeTicketCannotVote { - fp.remove("ticket cannot vote, needs to be revoked") + fp.remove("ticket cannot vote") + // Attempt to Revoke Tickets, we're not returning any errors here + // and just logging. + n, err := w.NetworkBackend() + if err != nil { + log.Errorf("unable to get network backend for revoking tickets %v", err) + } else { + if rpc, ok := n.(*dcrd.RPC); ok { + err := w.RevokeTickets(ctx, rpc) + if err != nil { + log.Errorf("cannot revoke vsp tickets %v", err) + } + } else { + log.Errorf("cannot revoke ticket since SPV mode") + } + } return nil } } - fp.schedule("reconcile payment", fp.reconcilePayment) + return err } } From 390e97674144a697102c88540a11477e0374ce93 Mon Sep 17 00:00:00 2001 From: Alex Yocom-Piatt Date: Fri, 7 May 2021 14:02:14 -0500 Subject: [PATCH 3/5] add expired fee tx handling --- internal/vsp/feepayment.go | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/internal/vsp/feepayment.go b/internal/vsp/feepayment.go index 381261bc1..69c94ff09 100644 --- a/internal/vsp/feepayment.go +++ b/internal/vsp/feepayment.go @@ -892,6 +892,20 @@ func (fp *feePayment) submitPayment() (err error) { err = fp.client.post(ctx, "/api/v3/payfee", fp.commitmentAddr, &payfeeResponse, json.RawMessage(requestBody)) if err != nil { + var apiErr *BadRequestError + if errors.As(err, &apiErr) { + if apiErr.Code == codeFeeExpired { + // Fee has been expired, so abandon current feetx, set fp.feeTx + // to nil and retry submit payment to make a new fee tx. + feeHash := feeTx.TxHash() + err := w.AbandonTransaction(ctx, &feeHash) + if err != nil { + log.Errorf("error abandoning expired fee tx %v", err) + } + fp.feeTx = nil + fp.submitPayment() + } + } return fmt.Errorf("payfee: %w", err) } From 6bf230e4f217c10c3c5ccafdf45c956006f4c7d3 Mon Sep 17 00:00:00 2001 From: Alex Yocom-Piatt Date: Mon, 10 May 2021 09:24:20 -0500 Subject: [PATCH 4/5] Review nits for @jrick --- internal/vsp/feepayment.go | 50 ++++++++++++++++---------------------- 1 file changed, 21 insertions(+), 29 deletions(-) diff --git a/internal/vsp/feepayment.go b/internal/vsp/feepayment.go index 69c94ff09..7c4278779 100644 --- a/internal/vsp/feepayment.go +++ b/internal/vsp/feepayment.go @@ -737,28 +737,22 @@ func (fp *feePayment) reconcilePayment() error { err := fp.makeFeeTx(nil) if err != nil { var apiErr *BadRequestError - if errors.As(err, &apiErr) { - if apiErr.Code == codeTicketCannotVote { - fp.remove("ticket cannot vote") - // Attempt to Revoke Tickets, we're not returning any errors here - // and just logging. - n, err := w.NetworkBackend() - if err != nil { - log.Errorf("unable to get network backend for revoking tickets %v", err) - } else { - if rpc, ok := n.(*dcrd.RPC); ok { - err := w.RevokeTickets(ctx, rpc) - if err != nil { - log.Errorf("cannot revoke vsp tickets %v", err) - } - } else { - log.Errorf("cannot revoke ticket since SPV mode") + if errors.As(err, &apiErr) && apiErr.Code == codeTicketCannotVote { + fp.remove("ticket cannot vote") + // Attempt to Revoke Tickets, we're not returning any errors here + // and just logging. + n, err := w.NetworkBackend() + if err != nil { + log.Errorf("unable to get network backend for revoking tickets %v", err) + } else { + if rpc, ok := n.(*dcrd.RPC); ok { + err := w.RevokeTickets(ctx, rpc) + if err != nil { + log.Errorf("cannot revoke vsp tickets %v", err) } } - return nil } } - return err } } @@ -893,18 +887,16 @@ func (fp *feePayment) submitPayment() (err error) { &payfeeResponse, json.RawMessage(requestBody)) if err != nil { var apiErr *BadRequestError - if errors.As(err, &apiErr) { - if apiErr.Code == codeFeeExpired { - // Fee has been expired, so abandon current feetx, set fp.feeTx - // to nil and retry submit payment to make a new fee tx. - feeHash := feeTx.TxHash() - err := w.AbandonTransaction(ctx, &feeHash) - if err != nil { - log.Errorf("error abandoning expired fee tx %v", err) - } - fp.feeTx = nil - fp.submitPayment() + if errors.As(err, &apiErr) && apiErr.Code == codeFeeExpired { + // Fee has been expired, so abandon current feetx, set fp.feeTx + // to nil and retry submit payment to make a new fee tx. + feeHash := feeTx.TxHash() + err := w.AbandonTransaction(ctx, &feeHash) + if err != nil { + log.Errorf("error abandoning expired fee tx %v", err) } + fp.feeTx = nil + fp.submitPayment() } return fmt.Errorf("payfee: %w", err) } From 2d42f9cc9bedd3a02fca75409fd3b5a55c616d93 Mon Sep 17 00:00:00 2001 From: Alex Yocom-Piatt Date: Mon, 10 May 2021 10:50:08 -0500 Subject: [PATCH 5/5] Remove submitPayment call, allow scheduler to happen --- internal/vsp/feepayment.go | 1 - 1 file changed, 1 deletion(-) diff --git a/internal/vsp/feepayment.go b/internal/vsp/feepayment.go index 7c4278779..6624cd258 100644 --- a/internal/vsp/feepayment.go +++ b/internal/vsp/feepayment.go @@ -896,7 +896,6 @@ func (fp *feePayment) submitPayment() (err error) { log.Errorf("error abandoning expired fee tx %v", err) } fp.feeTx = nil - fp.submitPayment() } return fmt.Errorf("payfee: %w", err) }