Improve handling of fee status.

- Fee tx status is now tracked using a dedicated field, with values none/received/broadcast/confirmed/error.
- Fee tx hex and hash are now both set in /payfee. The absense of txhash is no longer used to determine if a fee tx has been broadcast or not.
- setvotechoices can no longer be called before a fee is received.
- Remove `binding:required` from response types. It has no effect on responses, it is only needed on request types which are validated by gin.

background/background.go

@@ -50,6 +50,8 @@ func (n *NotificationHandler) Notify(method string, params json.RawMessage) erro
 	return nil
 }
 
+// blockConnected is called once when vspd starts up, and once each time a
+// blockconnected notification is received from dcrd.
 func blockConnected() {
 
 	dcrdClient, err := dcrdRPC.Client(ctx, netParams)
@@ -92,30 +94,19 @@ func blockConnected() {
 	}
 
 	for _, ticket := range pending {
-		feeTxHash, err := dcrdClient.SendRawTransaction(ticket.FeeTxHex)
+		err = dcrdClient.SendRawTransaction(ticket.FeeTxHex)
 		if err != nil {
 			log.Errorf("SendRawTransaction error: %v", err)
-
+			ticket.FeeTxStatus = database.FeeError
-			// Unset fee related fields and update the database.
+		} else {
-			ticket.FeeTxHex = ""
+			log.Debugf("Fee tx broadcast for ticket: ticketHash=%s, feeHash=%s", ticket.Hash, ticket.FeeTxHash)
-			ticket.VotingWIF = ""
+			ticket.FeeTxStatus = database.FeeBroadcast
-			ticket.VoteChoices = make(map[string]string)
+		}
 
 		err = db.UpdateTicket(ticket)
 		if err != nil {
 			log.Errorf("UpdateTicket error: %v", err)
 		}
-			log.Infof("Removed fee transaction for ticket %v", ticket.Hash)
-			continue
-		}
-
-		ticket.FeeTxHash = feeTxHash
-		err = db.UpdateTicket(ticket)
-		if err != nil {
-			log.Errorf("UpdateTicket error: %v", err)
-			continue
-		}
-		log.Debugf("Fee tx broadcast for ticket: ticketHash=%s, feeHash=%s", ticket.Hash, feeTxHash)
 	}
 
 	// Step 3/3: Add tickets with confirmed fees to voting wallets.
@@ -153,7 +144,7 @@ func blockConnected() {
 		// If fee is confirmed, update the database and add ticket to voting
 		// wallets.
 		if feeTx.Confirmations >= requiredConfs {
-			ticket.FeeConfirmed = true
+			ticket.FeeTxStatus = database.FeeConfirmed
 			err = db.UpdateTicket(ticket)
 			if err != nil {
 				log.Errorf("UpdateTicket error: %v", err)

database/ticket.go

@@ -9,6 +9,22 @@ import (
 	bolt "go.etcd.io/bbolt"
 )
 
+// FeeStatus represents the current state of a ticket fee payment.
+type FeeStatus string
+
+const (
+	// No fee transaction has been received yet.
+	NoFee FeeStatus = "none"
+	// Fee transaction has been received but not broadcast.
+	FeeReceieved FeeStatus = "received"
+	// Fee transaction has been broadcast but not confirmed.
+	FeeBroadcast FeeStatus = "broadcast"
+	// Fee transaction has been broadcast and confirmed.
+	FeeConfirmed FeeStatus = "confirmed"
+	// Fee transaction could not be broadcast due to an error.
+	FeeError FeeStatus = "error"
+)
+
 // Ticket is serialized to json and stored in bbolt db. The json keys are
 // deliberately kept short because they are duplicated many times in the db.
 type Ticket struct {
@@ -22,18 +38,19 @@ type Ticket struct {
 	// Confirmed will be set when the ticket has 6+ confirmations.
 	Confirmed bool `json:"conf"`
 
-	// VoteChoices and VotingWIF are set in /payfee.
+	// VotingWIF is set in /payfee.
-	VoteChoices map[string]string `json:"vchces"`
 	VotingWIF string `json:"vwif"`
 
-	// FeeTxHex will be set when the fee tx has been received from the user.
+	// VoteChoices is initially set in /payfee, but can be updated in
-	FeeTxHex string `json:"fhex"`
+	// /setvotechoices.
+	VoteChoices map[string]string `json:"vchces"`
 
-	// FeeTxHash will be set when the fee tx has been broadcast.
+	// FeeTxHex and FeeTxHash will be set when the fee tx has been received.
+	FeeTxHex  string `json:"fhex"`
 	FeeTxHash string `json:"fhsh"`
 
-	// FeeConfirmed will be set when the fee tx has 6+ confirmations.
+	// FeeTxStatus indicates the current state of the fee transaction.
-	FeeConfirmed bool `json:"fconf"`
+	FeeTxStatus FeeStatus `json:"fsts"`
 }
 
 func (t *Ticket) FeeExpired() bool {
@@ -142,7 +159,7 @@ func (vdb *VspDatabase) CountTickets() (int, int, error) {
 				return fmt.Errorf("could not unmarshal ticket: %v", err)
 			}
 
-			if ticket.FeeConfirmed {
+			if ticket.FeeTxStatus == FeeConfirmed {
 				feePaid++
 			}
 
@@ -188,11 +205,10 @@ func (vdb *VspDatabase) GetPendingFees() ([]Ticket, error) {
 				return fmt.Errorf("could not unmarshal ticket: %v", err)
 			}
 
-			// Add ticket if it is confirmed, and we have a fee tx, and the tx
+			// Add ticket if it is confirmed, and we have a fee tx which is not
-			// is not broadcast yet.
+			// yet broadcast.
 			if ticket.Confirmed &&
-				ticket.FeeTxHex != "" &&
+				ticket.FeeTxStatus == FeeReceieved {
-				ticket.FeeTxHash == "" {
 				tickets = append(tickets, ticket)
 			}
 
@@ -216,8 +232,7 @@ func (vdb *VspDatabase) GetUnconfirmedFees() ([]Ticket, error) {
 			}
 
 			// Add ticket if fee tx is broadcast but not confirmed yet.
-			if ticket.FeeTxHash != "" &&
+			if ticket.FeeTxStatus == FeeBroadcast {
-				!ticket.FeeConfirmed {
 				tickets = append(tickets, ticket)
 			}
 

database/ticket_test.go

@@ -19,7 +19,7 @@ func exampleTicket() Ticket {
 		VotingWIF:         "VotingKey",
 		FeeTxHex:          "FeeTransction",
 		FeeTxHash:         "",
-		FeeConfirmed:      true,
+		FeeTxStatus:       FeeBroadcast,
 	}
 }
 
@@ -84,7 +84,7 @@ func testGetTicketByHash(t *testing.T) {
 		retrieved.VotingWIF != ticket.VotingWIF ||
 		retrieved.FeeTxHex != ticket.FeeTxHex ||
 		retrieved.FeeTxHash != ticket.FeeTxHash ||
-		retrieved.FeeConfirmed != ticket.FeeConfirmed {
+		retrieved.FeeTxStatus != ticket.FeeTxStatus {
 		t.Fatal("retrieved ticket value didnt match expected")
 	}
 

docs/api.md

@@ -28,8 +28,8 @@
 Clients should retrieve the VSP's public key so they can check the signature on
 future API responses. A VSP should never change their public key, so it can be
 requested once and cached indefinitely. `vspclosed` indicates that the VSP is
-not currently accepting new tickets. Calling `/feeaddress` when a VSP is closed
+not currently accepting new tickets. Calling `/api/feeaddress` or `/api/payfee`
-will result in an error.
+when a VSP is closed will result in an error.
 
 - `GET /api/vspinfo`
 
@@ -61,7 +61,7 @@ DCR. Returns an error if the specified ticket is not currently immature or live.
 This call will return an error if a fee transaction has already been provided
 for the specified ticket.
 
-- `POST /feeaddress`
+- `POST /api/feeaddress`
 
     Request:
 
@@ -90,7 +90,7 @@ for the specified ticket.
 Provide the voting key for the ticket, voting preference, and a signed
 transaction which pays the fee to the specified address. If the fee has expired,
 this call will return an error and the client will need to request a new fee by
-calling `/feeaddress` again. Returns an error if the specified ticket is not
+calling `/api/feeaddress` again. Returns an error if the specified ticket is not
 currently immature or live.
 
 The VSP will not broadcast the fee transaction until the ticket purchase has 6
@@ -103,7 +103,7 @@ has 6 confirmations.
 This call will return an error if a fee transaction has already been provided
 for the specified ticket.
 
-- `POST /payfee`
+- `POST /api/payfee`
 
     Request:
 
@@ -129,20 +129,22 @@ for the specified ticket.
 ### Ticket Status
 
 Clients can check the status of a ticket at any time after calling
-`/feeaddress`. The lifecycle of the ticket is represented with a set of boolean
+`/api/feeaddress`.
-fields:
 
-- `ticketconfirmed` is true when the ticket transaction has 6 confirmations.
+- `ticketconfirmed` is true when the ticket purchase has 6 confirmations.
-- `feetxreceived` is true when the VSP has received a valid fee transaction.
+- `feetxstatus` can have the following values:
-   If the broadcast of the fee transaction fails, this will be reset to false.
+  - `none` - No fee transaction has been received yet.
-- `feetxbroadcast` is true when the VSP has broadcast the fee transaction.
+  - `received` - Fee transaction has been received but not broadcast.
-- `feeconfirmed` is true when the fee transaction has 6 confirmations.
+  - `broadcast` - Fee transaction has been broadcast but not confirmed.
+  - `confirmed` - Fee transaction has been broadcast and confirmed.
+  - `error` - Fee transaction could not be broadcast due to an error (eg. output
+    in the tx was double spent).
 
-The VSP will only add tickets to the voting wallets when all four of these
+If `feetxstatus` is `error`, the client needs to provide a new fee transaction
-conditions are met. `feetxhash` will only be populated if `feetxbroadcast` is
+using `/api/payfee`. The VSP will only add a ticket to the voting wallets once
-true.
+its `feetxstatus` is `confirmed`.
 
-- `GET /ticketstatus`
+- `GET /api/ticketstatus`
 
     Request:
 
@@ -159,9 +161,7 @@ true.
     {
       "timestamp":1590509066,
       "ticketconfirmed":true,
-      "feetxreceived":true,
+      "feetxstatus":"broadcast",
-      "feetxbroadcast":true,
-      "feeconfirmed":false,
       "feetxhash": "e1c02b04b5bbdae66cf8e3c88366c4918d458a2d27a26144df37f54a2bc956ac",
       "votechoices":{"headercommitments":"no"},
       "request": {"<Copy of request body>"}
@@ -171,9 +171,9 @@ true.
 ### Update vote choices
 
 Clients can update the voting preferences of their ticket at any time after
-after calling `/payfee`.
+after calling `/api/payfee`.
 
-- `POST /setvotechoices`
+- `POST /api/setvotechoices`
 
     Request:
 

rpc/client.go

@@ -56,13 +56,13 @@ func setup(ctx context.Context, shutdownWg *sync.WaitGroup, user, pass, addr str
 		if c != nil {
 			select {
 			case <-c.Done():
-				log.Debugf("RPC already closed (%s)", addr)
+				log.Tracef("RPC already closed (%s)", addr)
 
 			default:
 				if err := c.Close(); err != nil {
 					log.Errorf("Failed to close RPC (%s): %v", addr, err)
 				} else {
-					log.Debugf("RPC closed (%s)", addr)
+					log.Tracef("RPC closed (%s)", addr)
 				}
 			}
 		}

rpc/dcrd.go

@@ -96,31 +96,19 @@ func (c *DcrdRPC) GetRawTransaction(txHash string) (*dcrdtypes.TxRawResult, erro
 	return &resp, nil
 }
 
-func (c *DcrdRPC) SendRawTransaction(txHex string) (string, error) {
+func (c *DcrdRPC) SendRawTransaction(txHex string) error {
 	allowHighFees := false
-	var txHash string
+	err := c.Call(c.ctx, "sendrawtransaction", nil, txHex, allowHighFees)
-	err := c.Call(c.ctx, "sendrawtransaction", &txHash, txHex, allowHighFees)
 	if err != nil {
-		// It's not a problem if the transaction has already been broadcast,
+
-		// just need to calculate and return its hash.
+		// It's not a problem if the transaction has already been broadcast.
-		if !strings.Contains(err.Error(), "transaction already exists") {
+		if strings.Contains(err.Error(), "transaction already exists") {
-			return "", err
+			return nil
 		}
 
-		msgHex, err := hex.DecodeString(txHex)
+		return err
-		if err != nil {
-			return "", fmt.Errorf("DecodeString error: %v", err)
-
 	}
-		msgTx := wire.NewMsgTx()
+	return nil
-		if err = msgTx.FromBytes(msgHex); err != nil {
-			return "", fmt.Errorf("FromBytes error: %v", err)
-
-		}
-
-		txHash = msgTx.TxHash().String()
-	}
-	return txHash, nil
 }
 
 func (c *DcrdRPC) GetTicketCommitmentAddress(ticketHash string, netParams *chaincfg.Params) (string, error) {

webapi/errors.go

@@ -17,6 +17,7 @@ const (
 	errInvalidVoteChoices
 	errBadSignature
 	errInvalidPrivKey
+	errFeeNotReceived
 )
 
 // httpStatus maps application error codes to HTTP status codes.
@@ -46,6 +47,8 @@ func (e apiError) httpStatus() int {
 		return http.StatusBadRequest
 	case errInvalidPrivKey:
 		return http.StatusBadRequest
+	case errFeeNotReceived:
+		return http.StatusBadRequest
 	default:
 		return http.StatusInternalServerError
 	}
@@ -78,6 +81,8 @@ func (e apiError) defaultMessage() string {
 		return "bad request signature"
 	case errInvalidPrivKey:
 		return "invalid private key"
+	case errFeeNotReceived:
+		return "no fee tx received for this ticket"
 	default:
 		return "unknown error"
 	}

webapi/getfeeaddress.go

@@ -83,7 +83,9 @@ func feeAddress(c *gin.Context) {
 	ticketHash := feeAddressRequest.TicketHash
 
 	// Respond early if we already have the fee tx for this ticket.
-	if ticket.FeeTxHex != "" {
+	if ticket.FeeTxStatus == database.FeeReceieved ||
+		ticket.FeeTxStatus == database.FeeBroadcast ||
+		ticket.FeeTxStatus == database.FeeConfirmed {
 		log.Warnf("Fee tx already received from %s: ticketHash=%s", c.ClientIP(), ticket.Hash)
 		sendError(errFeeAlreadyReceived, c)
 		return
@@ -173,7 +175,7 @@ func feeAddress(c *gin.Context) {
 		Confirmed:         confirmed,
 		FeeAmount:         int64(fee),
 		FeeExpiration:     expire,
-		// VotingKey and VoteChoices: set during payfee
+		FeeTxStatus:       database.NoFee,
 	}
 
 	err = db.InsertNewTicket(dbTicket)

webapi/payfee.go

@@ -42,7 +42,9 @@ func payFee(c *gin.Context) {
 	}
 
 	// Respond early if we already have the fee tx for this ticket.
-	if ticket.FeeTxHex != "" {
+	if ticket.FeeTxStatus == database.FeeReceieved ||
+		ticket.FeeTxStatus == database.FeeBroadcast ||
+		ticket.FeeTxStatus == database.FeeConfirmed {
 		log.Warnf("Fee tx already received from %s: ticketHash=%s", c.ClientIP(), ticket.Hash)
 		sendError(errFeeAlreadyReceived, c)
 		return
@@ -199,7 +201,9 @@ findAddress:
 
 	ticket.VotingWIF = votingWIF.String()
 	ticket.FeeTxHex = payFeeRequest.FeeTx
+	ticket.FeeTxHash = feeTx.TxHash().String()
 	ticket.VoteChoices = voteChoices
+	ticket.FeeTxStatus = database.FeeReceieved
 
 	err = db.UpdateTicket(ticket)
 	if err != nil {
@@ -212,34 +216,31 @@ findAddress:
 		"ticketHash=%s", minFee, feePaid, ticket.Hash)
 
 	if ticket.Confirmed {
-		feeTxHash, err := dcrdClient.SendRawTransaction(payFeeRequest.FeeTx)
+		err = dcrdClient.SendRawTransaction(payFeeRequest.FeeTx)
 		if err != nil {
 			log.Errorf("SendRawTransaction failed: %v", err)
 
-			// Unset fee related fields and update the database.
+			ticket.FeeTxStatus = database.FeeError
-			ticket.FeeTxHex = ""
-			ticket.VotingWIF = ""
-			ticket.VoteChoices = make(map[string]string)
 
 			err = db.UpdateTicket(ticket)
 			if err != nil {
 				log.Errorf("UpdateTicket error: %v", err)
 			}
-			log.Infof("Removed fee transaction for ticket %v", ticket.Hash)
 
 			sendErrorWithMsg("could not broadcast fee transaction", errInvalidFeeTx, c)
 			return
 		}
-		ticket.FeeTxHash = feeTxHash
+
+		ticket.FeeTxStatus = database.FeeBroadcast
 
 		err = db.UpdateTicket(ticket)
 		if err != nil {
-			log.Errorf("InsertTicket failed: %v", err)
+			log.Errorf("UpdateTicket failed: %v", err)
 			sendError(errInternalError, c)
 			return
 		}
 
-		log.Debugf("Fee tx broadcast for ticket: ticketHash=%s, feeHash=%s", ticket.Hash, feeTxHash)
+		log.Debugf("Fee tx broadcast for ticket: ticketHash=%s, feeHash=%s", ticket.Hash, ticket.FeeTxHash)
 	}
 
 	sendJSONResponse(payFeeResponse{

webapi/setvotechoices.go

@@ -24,7 +24,11 @@ func setVoteChoices(c *gin.Context) {
 		return
 	}
 
-	// TODO: Return an error if we dont have a FeeTx for this ticket yet.
+	if ticket.FeeTxStatus == database.NoFee {
+		log.Warnf("Setvotechoices without fee tx from %s", c.ClientIP())
+		sendError(errFeeNotReceived, c)
+		return
+	}
 
 	var setVoteChoicesRequest SetVoteChoicesRequest
 	if err := binding.JSON.BindBody(rawRequest, &setVoteChoicesRequest); err != nil {
@@ -53,7 +57,7 @@ func setVoteChoices(c *gin.Context) {
 
 	// Update vote choices on voting wallets. Tickets are only added to voting
 	// wallets if their fee is confirmed.
-	if ticket.FeeConfirmed {
+	if ticket.FeeTxStatus == database.FeeConfirmed {
 		for agenda, choice := range voteChoices {
 			for _, walletClient := range walletClients {
 				err = walletClient.SetVoteChoice(agenda, choice, ticket.Hash)

webapi/templates/admin.html

@@ -17,7 +17,7 @@
         <td>VotingWIF</td>
         <td>FeeTxHex</td>
         <td>FeeTxHash</td>
-        <td>FeeConfirmed</td>
+        <td>FeeTxStatus</td>
     </tr>
     {{ range .Tickets }}
     <tr>
@@ -25,14 +25,14 @@
         <td>{{ printf "%.10s" .CommitmentAddress }}...</td>
         <td>{{ printf "%d" .FeeAddressIndex }}</td>
         <td>{{ printf "%.10s" .FeeAddress }}...</td>
-        <td>{{ printf "%f" .FeeAmount }}</td>
+        <td>{{ printf "%d" .FeeAmount }}</td>
         <td>{{ printf "%d" .FeeExpiration }}</td>
         <td>{{ printf "%t" .Confirmed }}</td>
         <td>{{ printf "%.10s" .VoteChoices }}...</td>
         <td>{{ printf "%.10s" .VotingWIF }}...</td>
         <td>{{ printf "%.10s" .FeeTxHex }}...</td>
         <td>{{ printf "%.10s" .FeeTxHash }}...</td>
-        <td>{{ printf "%t" .FeeConfirmed }}</td>
+        <td>{{ printf "%s" .FeeTxStatus }}</td>
     </tr>
     {{ end }}
 </table>

webapi/ticketstatus.go

@@ -33,9 +33,7 @@ func ticketStatus(c *gin.Context) {
 		Timestamp:       time.Now().Unix(),
 		Request:         ticketStatusRequest,
 		TicketConfirmed: ticket.Confirmed,
-		FeeTxReceived:   ticket.FeeTxHex != "",
+		FeeTxStatus:     string(ticket.FeeTxStatus),
-		FeeTxBroadcast:  ticket.FeeTxHash != "",
-		FeeConfirmed:    ticket.FeeConfirmed,
 		FeeTxHash:       ticket.FeeTxHash,
 		VoteChoices:     ticket.VoteChoices,
 	}, c)

webapi/types.go

@@ -1,11 +1,11 @@
 package webapi
 
 type vspInfoResponse struct {
-	Timestamp     int64   `json:"timestamp" binding:"required"`
+	Timestamp     int64   `json:"timestamp"`
-	PubKey        []byte  `json:"pubkey" binding:"required"`
+	PubKey        []byte  `json:"pubkey"`
-	FeePercentage float64 `json:"feepercentage" binding:"required"`
+	FeePercentage float64 `json:"feepercentage"`
-	VspClosed     bool    `json:"vspclosed" binding:"required"`
+	VspClosed     bool    `json:"vspclosed"`
-	Network       string  `json:"network" binding:"required"`
+	Network       string  `json:"network"`
 }
 
 type FeeAddressRequest struct {
@@ -14,11 +14,11 @@ type FeeAddressRequest struct {
 }
 
 type feeAddressResponse struct {
-	Timestamp  int64             `json:"timestamp" binding:"required"`
+	Timestamp  int64             `json:"timestamp"`
-	FeeAddress string            `json:"feeaddress" binding:"required"`
+	FeeAddress string            `json:"feeaddress"`
-	FeeAmount  int64             `json:"feeamount" binding:"required"`
+	FeeAmount  int64             `json:"feeamount"`
-	Expiration int64             `json:"expiration" binding:"required"`
+	Expiration int64             `json:"expiration"`
-	Request    FeeAddressRequest `json:"request" binding:"required"`
+	Request    FeeAddressRequest `json:"request"`
 }
 
 type PayFeeRequest struct {
@@ -30,8 +30,8 @@ type PayFeeRequest struct {
 }
 
 type payFeeResponse struct {
-	Timestamp int64         `json:"timestamp" binding:"required"`
+	Timestamp int64         `json:"timestamp"`
-	Request   PayFeeRequest `json:"request" binding:"required"`
+	Request   PayFeeRequest `json:"request"`
 }
 
 type SetVoteChoicesRequest struct {
@@ -41,8 +41,8 @@ type SetVoteChoicesRequest struct {
 }
 
 type setVoteChoicesResponse struct {
-	Timestamp int64                 `json:"timestamp" binding:"required"`
+	Timestamp int64                 `json:"timestamp"`
-	Request   SetVoteChoicesRequest `json:"request" binding:"required"`
+	Request   SetVoteChoicesRequest `json:"request"`
 }
 
 type TicketStatusRequest struct {
@@ -51,12 +51,10 @@ type TicketStatusRequest struct {
 }
 
 type ticketStatusResponse struct {
-	Timestamp       int64               `json:"timestamp" binding:"required"`
+	Timestamp       int64               `json:"timestamp"`
-	TicketConfirmed bool                `json:"ticketconfirmed" binding:"required"`
+	TicketConfirmed bool                `json:"ticketconfirmed"`
-	FeeTxReceived   bool                `json:"feetxreceived" binding:"required"`
+	FeeTxStatus     string              `json:"feetxstatus"`
-	FeeTxBroadcast  bool                `json:"feetxbroadcast" binding:"required"`
+	FeeTxHash       string              `json:"feetxhash"`
-	FeeConfirmed    bool                `json:"feeconfirmed" binding:"required"`
+	VoteChoices     map[string]string   `json:"votechoices"`
-	FeeTxHash       string              `json:"feetxhash" binding:"required"`
+	Request         TicketStatusRequest `json:"request"`
-	VoteChoices     map[string]string   `json:"votechoices" binding:"required"`
-	Request         TicketStatusRequest `json:"request" binding:"required"`
 }