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.

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

vspclient: More fixes for vsp fee confirmation tracking #2048

Merged
merged 5 commits into from
May 10, 2021
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
55 changes: 44 additions & 11 deletions internal/vsp/feepayment.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -189,15 +190,20 @@ 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 {
fp.stop()
fp = fp2
} else {
c.jobs[*ticketHash] = fp
schedule = true
}
c.mu.Unlock()
if schedule {
fp.schedule("reconcile payment", fp.reconcilePayment)
}
}()

ctx := context.Background()
Expand Down Expand Up @@ -250,9 +256,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
Expand All @@ -271,9 +277,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
}
Expand Down Expand Up @@ -427,8 +431,6 @@ func (fp *feePayment) receiveFeeAddress() error {
fp.feeAddr = feeAddr
fp.mu.Unlock()

_ = fp.makeFeeTx(nil)
fp.schedule("reconcile payment", fp.reconcilePayment)
return nil
}

Expand Down Expand Up @@ -471,8 +473,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
Expand Down Expand Up @@ -560,12 +560,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)
Expand Down Expand Up @@ -731,7 +736,23 @@ func (fp *feePayment) reconcilePayment() error {
if feeTx == nil || len(feeTx.TxOut) == 0 {
err := fp.makeFeeTx(nil)
if err != nil {
fp.schedule("reconcile payment", fp.reconcilePayment)
var apiErr *BadRequestError
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 err
}
}
Expand Down Expand Up @@ -865,6 +886,18 @@ 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) && 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()
Copy link
Member

Choose a reason for hiding this comment

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

I don't like calling submitPayment from itself, since this perhaps has the possibility to recurse endlessly (and slowly because of the api call) until it eventually stack overflows. Can we just return an error and let the caller deal with it?

Copy link
Member

Choose a reason for hiding this comment

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

clearing the fee tx is still correct though, but we should just let the caller deal with recreating and resubmitting it.

}
return fmt.Errorf("payfee: %w", err)
}

Expand Down
3 changes: 3 additions & 0 deletions wallet/wallet.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down