Cleaning up some TODOs (#50)

database/database_test.go

@@ -24,7 +24,8 @@ func exampleTicket() Ticket {
 		VoteChoices:         map[string]string{"AgendaID": "Choice"},
 		VotingKey:           "VotingKey",
 		VSPFee:              0.1,
-		Expiration:          4,
+		FeeExpiration:       4,
+		FeeTxHash:           "",
 	}
 }
 
@@ -109,7 +110,8 @@ func testGetTicketByHash(t *testing.T) {
 		!reflect.DeepEqual(retrieved.VoteChoices, ticket.VoteChoices) ||
 		retrieved.VotingKey != ticket.VotingKey ||
 		retrieved.VSPFee != ticket.VSPFee ||
-		retrieved.Expiration != ticket.Expiration {
+		retrieved.FeeTxHash != ticket.FeeTxHash ||
+		retrieved.FeeExpiration != ticket.FeeExpiration {
 		t.Fatal("retrieved ticket value didnt match expected")
 	}
 
@@ -131,8 +133,9 @@ func testSetTicketVotingKey(t *testing.T) {
 	// Update values.
 	newVotingKey := ticket.VotingKey + "2"
 	newVoteChoices := ticket.VoteChoices
+	feeTxHash := ticket.FeeTxHash + "3"
 	newVoteChoices["AgendaID"] = "Different choice"
-	err = db.SetTicketVotingKey(ticket.Hash, newVotingKey, newVoteChoices)
+	err = db.SetTicketVotingKey(ticket.Hash, newVotingKey, newVoteChoices, feeTxHash)
 	if err != nil {
 		t.Fatalf("error updating votingkey and votechoices: %v", err)
 	}
@@ -145,6 +148,7 @@ func testSetTicketVotingKey(t *testing.T) {
 
 	// Check ticket fields match expected.
 	if !reflect.DeepEqual(newVoteChoices, retrieved.VoteChoices) ||
+		feeTxHash != retrieved.FeeTxHash ||
 		newVotingKey != retrieved.VotingKey {
 		t.Fatal("retrieved ticket value didnt match expected")
 	}
@@ -159,7 +163,7 @@ func testUpdateExpireAndFee(t *testing.T) {
 	}
 
 	// Update ticket with new values.
-	newExpiry := ticket.Expiration + 1
+	newExpiry := ticket.FeeExpiration + 1
 	newFee := ticket.VSPFee + 1
 	err = db.UpdateExpireAndFee(ticket.Hash, newExpiry, newFee)
 	if err != nil {
@@ -173,7 +177,7 @@ func testUpdateExpireAndFee(t *testing.T) {
 	}
 
 	// Check ticket fields match expected.
-	if retrieved.VSPFee != newFee || retrieved.Expiration != newExpiry {
+	if retrieved.VSPFee != newFee || retrieved.FeeExpiration != newExpiry {
 		t.Fatal("retrieved ticket value didnt match expected")
 	}
 }

database/ticket.go

@@ -4,6 +4,7 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
+	"time"
 
 	bolt "go.etcd.io/bbolt"
 )
@@ -18,7 +19,13 @@ type Ticket struct {
 	VoteChoices         map[string]string `json:"votechoices"`
 	VotingKey           string            `json:"votingkey"`
 	VSPFee              float64           `json:"vspfee"`
-	Expiration          int64             `json:"expiration"`
+	FeeExpiration       int64             `json:"feeexpiration"`
+	FeeTxHash           string            `json:"feetxhash"`
+}
+
+func (t *Ticket) FeeExpired() bool {
+	now := time.Now()
+	return now.After(time.Unix(t.FeeExpiration, 0))
 }
 
 var (
@@ -34,6 +41,8 @@ func (vdb *VspDatabase) InsertTicket(ticket Ticket) error {
 			return fmt.Errorf("ticket already exists with hash %s", ticket.Hash)
 		}
 
+		// TODO: Error if a ticket already exists with the same fee address.
+
 		ticketBytes, err := json.Marshal(ticket)
 		if err != nil {
 			return err
@@ -43,7 +52,7 @@ func (vdb *VspDatabase) InsertTicket(ticket Ticket) error {
 	})
 }
 
-func (vdb *VspDatabase) SetTicketVotingKey(ticketHash, votingKey string, voteChoices map[string]string) error {
+func (vdb *VspDatabase) SetTicketVotingKey(ticketHash, votingKey string, voteChoices map[string]string, feeTxHash string) error {
 	return vdb.db.Update(func(tx *bolt.Tx) error {
 		ticketBkt := tx.Bucket(vspBktK).Bucket(ticketBktK)
 
@@ -62,6 +71,8 @@ func (vdb *VspDatabase) SetTicketVotingKey(ticketHash, votingKey string, voteCho
 
 		ticket.VotingKey = votingKey
 		ticket.VoteChoices = voteChoices
+		ticket.FeeTxHash = feeTxHash
+
 		ticketBytes, err = json.Marshal(ticket)
 		if err != nil {
 			return fmt.Errorf("could not marshal ticket: %v", err)
@@ -133,7 +144,7 @@ func (vdb *VspDatabase) UpdateExpireAndFee(ticketHash string, expiration int64,
 		if err != nil {
 			return fmt.Errorf("could not unmarshal ticket: %v", err)
 		}
-		ticket.Expiration = expiration
+		ticket.FeeExpiration = expiration
 		ticket.VSPFee = vspFee
 
 		ticketBytes, err = json.Marshal(ticket)

main.go

@@ -15,7 +15,6 @@ import (
 
 const (
 	feeAccountName              = "fees"
-	// TODO: Make expiration configurable?
 	defaultFeeAddressExpiration = 24 * time.Hour
 )
 

webapi/getfeeaddress.go

@@ -54,9 +54,9 @@ func feeAddress(c *gin.Context) {
 		// Ticket already exists
 		if signature == ticket.CommitmentSignature {
 			now := time.Now()
-			expire := ticket.Expiration
+			expire := ticket.FeeExpiration
 			VSPFee := ticket.VSPFee
-			if now.After(time.Unix(ticket.Expiration, 0)) {
+			if ticket.FeeExpired() {
 				expire = now.Add(cfg.FeeAddressExpiration).Unix()
 				VSPFee = cfg.VSPFee
 
@@ -183,7 +183,7 @@ func feeAddress(c *gin.Context) {
 		SDiff:               blockHeader.SBits,
 		BlockHeight:         int64(blockHeader.Height),
 		VSPFee:              cfg.VSPFee,
-		Expiration:          expire,
+		FeeExpiration:       expire,
 		// VotingKey and VoteChoices: set during payfee
 	}
 

webapi/payfee.go

@@ -24,8 +24,21 @@ func payFee(c *gin.Context) {
 		return
 	}
 
-	// TODO: Respond early if the fee tx has already been broadcast for this
+	ticket, err := db.GetTicketByHash(payFeeRequest.TicketHash)
-	// ticket. Maybe indicate status - mempool/awaiting confs/confirmed.
+	if err != nil {
+		log.Warnf("Invalid ticket from %s", c.ClientIP())
+		sendErrorResponse("invalid ticket", http.StatusBadRequest, c)
+		return
+	}
+
+	// Fee transaction has already been broadcast for this ticket.
+	if ticket.FeeTxHash != "" {
+		sendJSONResponse(payFeeResponse{
+			Timestamp: time.Now().Unix(),
+			TxHash:    ticket.FeeTxHash,
+			Request:   payFeeRequest,
+		}, c)
+	}
 
 	// Validate VotingKey.
 	votingKey := payFeeRequest.VotingKey
@@ -70,12 +83,9 @@ func payFee(c *gin.Context) {
 		return
 	}
 
-	// TODO: DB - check expiration given during fee address request
+	if ticket.FeeExpired() {
-
+		log.Warnf("Expired payfee request from %s", c.ClientIP())
-	ticket, err := db.GetTicketByHash(payFeeRequest.TicketHash)
+		sendErrorResponse("fee has expired", http.StatusBadRequest, c)
-	if err != nil {
-		log.Warnf("Invalid ticket from %s", c.ClientIP())
-		sendErrorResponse("invalid ticket", http.StatusBadRequest, c)
 		return
 	}
 
@@ -152,20 +162,20 @@ findAddress:
 	// pays sufficient fees to the expected address.
 	// Proceed to update the database and broadcast the transaction.
 
-	err = db.SetTicketVotingKey(ticket.Hash, votingWIF.String(), voteChoices)
+	feeTxHash, err := fWalletClient.SendRawTransaction(hex.EncodeToString(feeTxBuf.Bytes()))
-	if err != nil {
-		log.Errorf("SetTicketVotingKey failed: %v", err)
-		sendErrorResponse("database error", http.StatusInternalServerError, c)
-		return
-	}
-
-	sendTxHash, err := fWalletClient.SendRawTransaction(hex.EncodeToString(feeTxBuf.Bytes()))
 	if err != nil {
 		log.Errorf("SendRawTransaction failed: %v", err)
 		sendErrorResponse("dcrwallet RPC error", http.StatusInternalServerError, c)
 		return
 	}
 
+	err = db.SetTicketVotingKey(ticket.Hash, votingWIF.String(), voteChoices, feeTxHash)
+	if err != nil {
+		log.Errorf("SetTicketVotingKey failed: %v", err)
+		sendErrorResponse("database error", http.StatusInternalServerError, c)
+		return
+	}
+
 	// TODO: Should return a response here. We don't want to add the ticket to
 	// the voting wallets until the fee tx has been confirmed.
 
@@ -216,7 +226,7 @@ findAddress:
 
 	sendJSONResponse(payFeeResponse{
 		Timestamp: time.Now().Unix(),
-		TxHash:    sendTxHash,
+		TxHash:    feeTxHash,
 		Request:   payFeeRequest,
 	}, c)
 }

webapi/setvotechoices.go

@@ -77,6 +77,15 @@ func setVoteChoices(c *gin.Context) {
 		return
 	}
 
+	// Update VoteChoices in the database before updating the wallets. DB is
+	// source of truth and is less likely to error.
+	err = db.UpdateVoteChoices(txHash.String(), voteChoices)
+	if err != nil {
+		log.Errorf("UpdateVoteChoices error: %v", err)
+		sendErrorResponse("database error", http.StatusInternalServerError, c)
+		return
+	}
+
 	// Update vote choices on voting wallets.
 	for agenda, choice := range voteChoices {
 		err = vWalletClient.SetVoteChoice(agenda, choice, ticket.Hash)
@@ -87,15 +96,6 @@ func setVoteChoices(c *gin.Context) {
 		}
 	}
 
-	// TODO: Update database before updating wallets. DB is source of truth and
-	// is less likely to error.
-	err = db.UpdateVoteChoices(txHash.String(), voteChoices)
-	if err != nil {
-		log.Errorf("UpdateVoteChoices error: %v", err)
-		sendErrorResponse("database error", http.StatusInternalServerError, c)
-		return
-	}
-
 	// TODO: DB - error if given timestamp is older than any previous requests
 
 	// TODO: DB - store setvotechoices receipt in log