Resolving some TODOs (#71)

* Dont allow duplicate fee addresses

* Load webapi keypair in webapi package.

* Split database test file

* Add extra DB testing.

database/addressindex_test.go

@@ -0,0 +1,33 @@
+package database
+
+import (
+	"testing"
+)
+
+func testAddressIndex(t *testing.T) {
+
+	// Getting index before it has been set should return 0.
+	idx, err := db.GetLastAddressIndex()
+	if err != nil {
+		t.Fatalf("error getting address index: %v", err)
+	}
+	if idx != 0 {
+		t.Fatalf("retrieved addr index value didnt match expected")
+	}
+
+	// Update address index.
+	idx = uint32(99)
+	err = db.SetLastAddressIndex(idx)
+	if err != nil {
+		t.Fatalf("error setting address index: %v", err)
+	}
+
+	// Check for updated value.
+	retrievedIdx, err := db.GetLastAddressIndex()
+	if err != nil {
+		t.Fatalf("error getting address index: %v", err)
+	}
+	if idx != retrievedIdx {
+		t.Fatalf("retrieved addr index value didnt match expected")
+	}
+}

database/database_test.go

@@ -3,7 +3,6 @@ package database
 import (
 	"context"
 	"os"
-	"reflect"
 	"sync"
 	"testing"
 )
@@ -13,23 +12,6 @@ var (
 	db     *VspDatabase
 )
 
-func exampleTicket() Ticket {
-	return Ticket{
-		Hash:              "Hash",
-		CommitmentAddress: "Address",
-		FeeAddressIndex:   12345,
-		FeeAddress:        "FeeAddress",
-		FeeAmount:         0.1,
-		FeeExpiration:     4,
-		Confirmed:         false,
-		VoteChoices:       map[string]string{"AgendaID": "Choice"},
-		VotingWIF:         "VotingKey",
-		FeeTxHex:          "FeeTransction",
-		FeeTxHash:         "",
-		FeeConfirmed:      true,
-	}
-}
-
 // TestDatabase runs all database tests.
 func TestDatabase(t *testing.T) {
 	// Ensure we are starting with a clean environment.
@@ -37,8 +19,11 @@ func TestDatabase(t *testing.T) {
 
 	// All sub-tests to run.
 	tests := map[string]func(*testing.T){
-		"testInsertNewTicket": testInsertNewTicket,
-		"testGetTicketByHash": testGetTicketByHash,
+		"testInsertNewTicket":  testInsertNewTicket,
+		"testGetTicketByHash":  testGetTicketByHash,
+		"testUpdateTicket":     testUpdateTicket,
+		"testTicketFeeExpired": testTicketFeeExpired,
+		"testAddressIndex":     testAddressIndex,
 	}
 
 	for testName, test := range tests {
@@ -62,72 +47,5 @@ func TestDatabase(t *testing.T) {
 	}
 }
 
-func testInsertNewTicket(t *testing.T) {
-	// Insert a ticket into the database.
-	ticket := exampleTicket()
-	err := db.InsertNewTicket(ticket)
-	if err != nil {
-		t.Fatalf("error storing ticket in database: %v", err)
-	}
-
-	// Inserting a ticket with the same hash should fail.
-	err = db.InsertNewTicket(ticket)
-	if err == nil {
-		t.Fatal("expected an error inserting ticket with duplicate hash")
-	}
-
-	// Inserting a ticket with empty hash should fail.
-	ticket.Hash = ""
-	err = db.InsertNewTicket(ticket)
-	if err == nil {
-		t.Fatal("expected an error inserting ticket with no hash")
-	}
-}
-
-func testGetTicketByHash(t *testing.T) {
-	ticket := exampleTicket()
-	// Insert a ticket into the database.
-	err := db.InsertNewTicket(ticket)
-	if err != nil {
-		t.Fatalf("error storing ticket in database: %v", err)
-	}
-
-	// Retrieve ticket from database.
-	retrieved, found, err := db.GetTicketByHash(ticket.Hash)
-	if err != nil {
-		t.Fatalf("error retrieving ticket by ticket hash: %v", err)
-	}
-	if !found {
-		t.Fatal("expected found==true")
-	}
-
-	// Check ticket fields match expected.
-	if retrieved.Hash != ticket.Hash ||
-		retrieved.CommitmentAddress != ticket.CommitmentAddress ||
-		retrieved.FeeAddressIndex != ticket.FeeAddressIndex ||
-		retrieved.FeeAddress != ticket.FeeAddress ||
-		retrieved.FeeAmount != ticket.FeeAmount ||
-		retrieved.FeeExpiration != ticket.FeeExpiration ||
-		retrieved.Confirmed != ticket.Confirmed ||
-		!reflect.DeepEqual(retrieved.VoteChoices, ticket.VoteChoices) ||
-		retrieved.VotingWIF != ticket.VotingWIF ||
-		retrieved.FeeTxHex != ticket.FeeTxHex ||
-		retrieved.FeeTxHash != ticket.FeeTxHash ||
-		retrieved.FeeConfirmed != ticket.FeeConfirmed {
-		t.Fatal("retrieved ticket value didnt match expected")
-	}
-
-	// Check found==false when requesting a non-existent ticket.
-	_, found, err = db.GetTicketByHash("Not a real ticket hash")
-	if err != nil {
-		t.Fatalf("error retrieving ticket by ticket hash: %v", err)
-	}
-	if found {
-		t.Fatal("expected found==false")
-	}
-}
-
-// TODO: Add tests for UpdateTicket, CountTickets, GetUnconfirmedTickets,
-// GetPendingFees, GetUnconfirmedFees.
-
-// TODO: Add tests for ticket.FeeExpired.
+// TODO: Add tests for CountTickets, GetUnconfirmedTickets, GetPendingFees,
+// GetUnconfirmedFees.

database/ticket.go

@@ -9,7 +9,6 @@ import (
 	bolt "go.etcd.io/bbolt"
 )
 
-// TODO: Properly document ticket lifecycle.
 // TODO: Shorten json keys, they are stored in the db and duplicated many times.
 
 type Ticket struct {
@@ -56,7 +55,27 @@ func (vdb *VspDatabase) InsertNewTicket(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.
+		// Error if a ticket already exists with the same fee address.
+		err := ticketBkt.ForEach(func(k, v []byte) error {
+			var t Ticket
+			err := json.Unmarshal(v, &t)
+			if err != nil {
+				return fmt.Errorf("could not unmarshal ticket: %v", err)
+			}
+
+			if t.FeeAddress == ticket.FeeAddress {
+				return fmt.Errorf("ticket with fee address %s already exists", t.FeeAddress)
+			}
+
+			if t.FeeAddressIndex == ticket.FeeAddressIndex {
+				return fmt.Errorf("ticket with fee address index %d already exists", t.FeeAddressIndex)
+			}
+
+			return nil
+		})
+		if err != nil {
+			return err
+		}
 
 		ticketBytes, err := json.Marshal(ticket)
 		if err != nil {
@@ -77,8 +96,6 @@ func (vdb *VspDatabase) UpdateTicket(ticket Ticket) error {
 			return fmt.Errorf("ticket does not exist 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 fmt.Errorf("could not marshal ticket: %v", err)

database/ticket_test.go

@@ -0,0 +1,155 @@
+package database
+
+import (
+	"reflect"
+	"testing"
+	"time"
+)
+
+func exampleTicket() Ticket {
+	return Ticket{
+		Hash:              "Hash",
+		CommitmentAddress: "Address",
+		FeeAddressIndex:   12345,
+		FeeAddress:        "FeeAddress",
+		FeeAmount:         0.1,
+		FeeExpiration:     4,
+		Confirmed:         false,
+		VoteChoices:       map[string]string{"AgendaID": "Choice"},
+		VotingWIF:         "VotingKey",
+		FeeTxHex:          "FeeTransction",
+		FeeTxHash:         "",
+		FeeConfirmed:      true,
+	}
+}
+
+func testInsertNewTicket(t *testing.T) {
+	// Insert a ticket into the database.
+	ticket := exampleTicket()
+	err := db.InsertNewTicket(ticket)
+	if err != nil {
+		t.Fatalf("error storing ticket in database: %v", err)
+	}
+
+	// Inserting a ticket with different fee address but same hash should fail.
+	ticket2 := exampleTicket()
+	ticket2.FeeAddress = ticket.FeeAddress + "2"
+	err = db.InsertNewTicket(ticket)
+	if err == nil {
+		t.Fatal("expected an error inserting ticket with duplicate hash")
+	}
+
+	// Inserting a ticket with different hash but same fee address should fail.
+	ticket3 := exampleTicket()
+	ticket3.FeeAddress = ticket.Hash + "2"
+	err = db.InsertNewTicket(ticket)
+	if err == nil {
+		t.Fatal("expected an error inserting ticket with duplicate fee addr")
+	}
+
+	// Inserting a ticket with empty hash should fail.
+	ticket.Hash = ""
+	err = db.InsertNewTicket(ticket)
+	if err == nil {
+		t.Fatal("expected an error inserting ticket with no hash")
+	}
+}
+
+func testGetTicketByHash(t *testing.T) {
+	ticket := exampleTicket()
+	// Insert a ticket into the database.
+	err := db.InsertNewTicket(ticket)
+	if err != nil {
+		t.Fatalf("error storing ticket in database: %v", err)
+	}
+
+	// Retrieve ticket from database.
+	retrieved, found, err := db.GetTicketByHash(ticket.Hash)
+	if err != nil {
+		t.Fatalf("error retrieving ticket by ticket hash: %v", err)
+	}
+	if !found {
+		t.Fatal("expected found==true")
+	}
+
+	// Check ticket fields match expected.
+	if retrieved.Hash != ticket.Hash ||
+		retrieved.CommitmentAddress != ticket.CommitmentAddress ||
+		retrieved.FeeAddressIndex != ticket.FeeAddressIndex ||
+		retrieved.FeeAddress != ticket.FeeAddress ||
+		retrieved.FeeAmount != ticket.FeeAmount ||
+		retrieved.FeeExpiration != ticket.FeeExpiration ||
+		retrieved.Confirmed != ticket.Confirmed ||
+		!reflect.DeepEqual(retrieved.VoteChoices, ticket.VoteChoices) ||
+		retrieved.VotingWIF != ticket.VotingWIF ||
+		retrieved.FeeTxHex != ticket.FeeTxHex ||
+		retrieved.FeeTxHash != ticket.FeeTxHash ||
+		retrieved.FeeConfirmed != ticket.FeeConfirmed {
+		t.Fatal("retrieved ticket value didnt match expected")
+	}
+
+	// Check found==false when requesting a non-existent ticket.
+	_, found, err = db.GetTicketByHash("Not a real ticket hash")
+	if err != nil {
+		t.Fatalf("error retrieving ticket by ticket hash: %v", err)
+	}
+	if found {
+		t.Fatal("expected found==false")
+	}
+}
+
+func testUpdateTicket(t *testing.T) {
+	ticket := exampleTicket()
+	// Insert a ticket into the database.
+	err := db.InsertNewTicket(ticket)
+	if err != nil {
+		t.Fatalf("error storing ticket in database: %v", err)
+	}
+
+	// Update ticket with new values
+	ticket.FeeAmount = ticket.FeeAmount + 1
+	ticket.FeeExpiration = ticket.FeeExpiration + 1
+	err = db.UpdateTicket(ticket)
+	if err != nil {
+		t.Fatalf("error updating ticket: %v", err)
+	}
+
+	// Retrieve ticket from database.
+	retrieved, found, err := db.GetTicketByHash(ticket.Hash)
+	if err != nil {
+		t.Fatalf("error retrieving ticket by ticket hash: %v", err)
+	}
+	if !found {
+		t.Fatal("expected found==true")
+	}
+
+	if ticket.FeeAmount != retrieved.FeeAmount ||
+		ticket.FeeExpiration != retrieved.FeeExpiration {
+		t.Fatal("retrieved ticket value didnt match expected")
+	}
+
+	// Updating a non-existent ticket should fail.
+	ticket.Hash = "doesnt exist"
+	err = db.UpdateTicket(ticket)
+	if err == nil {
+		t.Fatal("expected an error updating a ticket with non-existent hash")
+	}
+}
+
+func testTicketFeeExpired(t *testing.T) {
+	ticket := exampleTicket()
+
+	now := time.Now()
+	hourBefore := now.Add(-time.Hour).Unix()
+	hourAfter := now.Add(time.Hour).Unix()
+
+	ticket.FeeExpiration = hourAfter
+	if ticket.FeeExpired() {
+		t.Fatal("expected ticket not to be expired")
+	}
+
+	ticket.FeeExpiration = hourBefore
+	if !ticket.FeeExpired() {
+		t.Fatal("expected ticket to be expired")
+	}
+}

main.go

@@ -111,19 +111,8 @@ func run(ctx context.Context) error {
 	// dcrd if the connection drops.
 	background.Start(notifHandler, dcrdWithNotifHandler)
 
-	// TODO: This can move into webapi.Start()
-	signKey, pubKey, err := db.KeyPair()
-	if err != nil {
-		log.Errorf("Failed to get keypair: %v", err)
-		requestShutdown()
-		shutdownWg.Wait()
-		return err
-	}
-
 	// Create and start webapi server.
 	apiCfg := webapi.Config{
-		SignKey:              signKey,
-		PubKey:               pubKey,
 		VSPFee:               cfg.VSPFee,
 		NetParams:            cfg.netParams.Params,
 		FeeAddressExpiration: defaultFeeAddressExpiration,

webapi/status.go

@@ -10,7 +10,7 @@ import (
 func pubKey(c *gin.Context) {
 	sendJSONResponse(pubKeyResponse{
 		Timestamp: time.Now().Unix(),
-		PubKey:    cfg.PubKey,
+		PubKey:    signPubKey,
 	}, c)
 }
 

webapi/webapi.go

@@ -19,8 +19,6 @@ import (
 )
 
 type Config struct {
-	SignKey              ed25519.PrivateKey
-	PubKey               ed25519.PublicKey
 	VSPFee               float64
 	NetParams            *chaincfg.Params
 	FeeAccountName       string
@@ -42,12 +40,21 @@ var db *database.VspDatabase
 var dcrdConnect rpc.Connect
 var walletConnect rpc.Connect
 var addrGen *addressGenerator
+var signPrivKey ed25519.PrivateKey
+var signPubKey ed25519.PublicKey
 
 func Start(ctx context.Context, requestShutdownChan chan struct{}, shutdownWg *sync.WaitGroup,
 	listen string, vdb *database.VspDatabase, dConnect rpc.Connect, wConnect rpc.Connect, debugMode bool, feeXPub string, config Config) error {
 
-	// Populate template data before starting webserver.
 	var err error
+
+	// Get keys for signing API responses from the database.
+	signPrivKey, signPubKey, err = vdb.KeyPair()
+	if err != nil {
+		return fmt.Errorf("Failed to get keypair: %v", err)
+	}
+
+	// Populate template data before starting webserver.
 	homepageData, err = updateHomepageData(vdb, config)
 	if err != nil {
 		return fmt.Errorf("could not initialize homepage data: %v", err)
@@ -215,7 +222,7 @@ func sendJSONResponse(resp interface{}, c *gin.Context) {
 		return
 	}
 
-	sig := ed25519.Sign(cfg.SignKey, dec)
+	sig := ed25519.Sign(signPrivKey, dec)
 	c.Writer.Header().Set("VSP-Server-Signature", hex.EncodeToString(sig))
 
 	c.AbortWithStatusJSON(http.StatusOK, resp)