From 093e03f9191ccfdc2ee1ffba8b7baf06ff84ec84 Mon Sep 17 00:00:00 2001 From: jholdstock Date: Mon, 28 Mar 2022 09:35:15 +0100 Subject: [PATCH 1/5] Minor cleanups for new helpers. - Ensure errors are properly wrapped with `%w` - Error strings should not starts with caps. - Add missing params to "Bad signature" error log --- webapi/helpers.go | 20 +++++++------------- webapi/middleware.go | 15 ++++++++++----- 2 files changed, 17 insertions(+), 18 deletions(-) diff --git a/webapi/helpers.go b/webapi/helpers.go index ec82c02e..cc5c4afc 100644 --- a/webapi/helpers.go +++ b/webapi/helpers.go @@ -115,7 +115,7 @@ func validateSignature(hash, commitmentAddress, signature, message string, // first check if we have an alternate sign address for this ticket. altSigData, err := db.AltSignAddrData(hash) if err != nil { - return fmt.Errorf("db.AltSignAddrData failed: %v", err) + return fmt.Errorf("db.AltSignAddrData failed: %w", err) } // If we have no alternate sign address, or if validating with the @@ -162,7 +162,7 @@ func validateTicketHash(hash string) error { } _, err := chainhash.NewHashFromStr(hash) if err != nil { - return fmt.Errorf("invalid hash: %v", err) + return fmt.Errorf("invalid hash: %w", err) } @@ -172,31 +172,25 @@ func validateTicketHash(hash string) error { // getCommitmentAddress gets the commitment address of the provided ticket hash // from the chain. func getCommitmentAddress(hash string, dcrdClient *rpc.DcrdRPC, params *chaincfg.Params) (string, error) { - var commitmentAddress string resp, err := dcrdClient.GetRawTransaction(hash) if err != nil { - return commitmentAddress, fmt.Errorf("dcrd.GetRawTransaction for ticket failed: %v", err) - + return "", fmt.Errorf("dcrd.GetRawTransaction for ticket failed: %w", err) } msgTx, err := decodeTransaction(resp.Hex) if err != nil { - return commitmentAddress, fmt.Errorf("Failed to decode ticket hex: %v", err) - + return "", fmt.Errorf("failed to decode ticket hex: %w", err) } err = isValidTicket(msgTx) if err != nil { - return commitmentAddress, fmt.Errorf("Invalid ticket: %w", errInvalidTicket) - + return "", fmt.Errorf("invalid ticket: %w", errInvalidTicket) } addr, err := stake.AddrFromSStxPkScrCommitment(msgTx.TxOut[1].PkScript, params) if err != nil { - return commitmentAddress, fmt.Errorf("AddrFromSStxPkScrCommitment error: %v", err) - + return "", fmt.Errorf("AddrFromSStxPkScrCommitment error: %w", err) } - commitmentAddress = addr.String() - return commitmentAddress, nil + return addr.String(), nil } diff --git a/webapi/middleware.go b/webapi/middleware.go index 3f6e5f9b..6d0b45b7 100644 --- a/webapi/middleware.go +++ b/webapi/middleware.go @@ -140,7 +140,8 @@ func (s *Server) broadcastTicket(c *gin.Context) { // Ensure the provided ticket hex is a valid ticket. msgTx, err := decodeTransaction(request.TicketHex) if err != nil { - log.Errorf("%s: Failed to decode ticket hex (ticketHash=%s): %v", funcName, request.TicketHash, err) + log.Errorf("%s: Failed to decode ticket hex (ticketHash=%s): %v", + funcName, request.TicketHash, err) s.sendErrorWithMsg("cannot decode ticket hex", errBadRequest, c) return } @@ -297,7 +298,6 @@ func (s *Server) vspAuth(c *gin.Context) { // If the ticket was found in the database, we already know its // commitment address. Otherwise we need to get it from the chain. - var commitmentAddress string dcrdClient := c.MustGet(dcrdKey).(*rpc.DcrdRPC) dcrdErr := c.MustGet(dcrdErrorKey) if dcrdErr != nil { @@ -306,18 +306,22 @@ func (s *Server) vspAuth(c *gin.Context) { return } + var commitmentAddress string if ticketFound { commitmentAddress = ticket.CommitmentAddress } else { commitmentAddress, err = getCommitmentAddress(hash, dcrdClient, s.cfg.NetParams) if err != nil { + log.Errorf("%s: Failed to get commitment address (clientIP=%s, ticketHash=%s): %v", + funcName, c.ClientIP(), hash, err) + var apiErr *apiError if errors.Is(err, apiErr) { s.sendError(errInvalidTicket, c) } else { s.sendError(errInternalError, c) } - log.Errorf("%s: (clientIP: %s, ticketHash: %s): %v", funcName, c.ClientIP(), hash, err) + return } } @@ -325,7 +329,7 @@ func (s *Server) vspAuth(c *gin.Context) { // Ensure a signature is provided. signature := c.GetHeader("VSP-Client-Signature") if signature == "" { - log.Warnf("%s: Bad request (clientIP=%s): %v", funcName, c.ClientIP(), err) + log.Warnf("%s: No VSP-Client-Signature header (clientIP=%s)", funcName, c.ClientIP()) s.sendErrorWithMsg("no VSP-Client-Signature header", errBadRequest, c) return } @@ -333,7 +337,8 @@ func (s *Server) vspAuth(c *gin.Context) { // Validate request signature to ensure ticket ownership. err = validateSignature(hash, commitmentAddress, signature, string(reqBytes), s.db, s.cfg.NetParams) if err != nil { - log.Errorf("%s: Bad signature (clientIP=%s, ticketHash=%s): %v", funcName, err) + log.Errorf("%s: Couldn't validate signature (clientIP=%s, ticketHash=%s): %v", + funcName, c.ClientIP(), hash, err) s.sendError(errBadSignature, c) return } From 9e8044d4f9a1a3e1f9000d0de06beb904d4507bf Mon Sep 17 00:00:00 2001 From: jholdstock Date: Mon, 28 Mar 2022 13:43:11 +0100 Subject: [PATCH 2/5] Move database encoding helpers. --- database/database.go | 33 --------------------------------- database/helpers.go | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 33 deletions(-) diff --git a/database/database.go b/database/database.go index e9b88819..e56cbf0d 100644 --- a/database/database.go +++ b/database/database.go @@ -8,7 +8,6 @@ import ( "context" "crypto/ed25519" "crypto/rand" - "encoding/binary" "fmt" "io" "net/http" @@ -89,38 +88,6 @@ func writeHotBackupFile(db *bolt.DB) error { return err } -func int64ToBytes(i int64) []byte { - bytes := make([]byte, 8) - binary.LittleEndian.PutUint64(bytes, uint64(i)) - return bytes -} - -func bytesToInt64(bytes []byte) int64 { - return int64(binary.LittleEndian.Uint64(bytes)) -} - -func uint32ToBytes(i uint32) []byte { - bytes := make([]byte, 4) - binary.LittleEndian.PutUint32(bytes, i) - return bytes -} - -func bytesToUint32(bytes []byte) uint32 { - return binary.LittleEndian.Uint32(bytes) -} - -func bytesToBool(bytes []byte) bool { - return bytes[0] == 1 -} - -func boolToBytes(b bool) []byte { - if b { - return []byte{1} - } - - return []byte{0} -} - // CreateNew intializes a new bbolt database with all of the necessary vspd // buckets, and inserts: // - the provided extended pubkey (to be used for deriving fee addresses). diff --git a/database/helpers.go b/database/helpers.go index 099d46e7..a000757a 100644 --- a/database/helpers.go +++ b/database/helpers.go @@ -5,6 +5,7 @@ package database import ( + "encoding/binary" "encoding/json" ) @@ -26,3 +27,35 @@ func bytesToStringMap(bytes []byte) (map[string]string, error) { return stringMap, nil } + +func int64ToBytes(i int64) []byte { + bytes := make([]byte, 8) + binary.LittleEndian.PutUint64(bytes, uint64(i)) + return bytes +} + +func bytesToInt64(bytes []byte) int64 { + return int64(binary.LittleEndian.Uint64(bytes)) +} + +func uint32ToBytes(i uint32) []byte { + bytes := make([]byte, 4) + binary.LittleEndian.PutUint32(bytes, i) + return bytes +} + +func bytesToUint32(bytes []byte) uint32 { + return binary.LittleEndian.Uint32(bytes) +} + +func bytesToBool(bytes []byte) bool { + return bytes[0] == 1 +} + +func boolToBytes(b bool) []byte { + if b { + return []byte{1} + } + + return []byte{0} +} From 2a45363dd792f5b10b5eb2ac46f7970b90edd6ba Mon Sep 17 00:00:00 2001 From: jholdstock Date: Mon, 28 Mar 2022 13:44:49 +0100 Subject: [PATCH 3/5] Critical log startup and shutdown. These messages should always be logged, even if log level is set to WARN or ERROR. --- vspd.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/vspd.go b/vspd.go index f9922e15..2ce786db 100644 --- a/vspd.go +++ b/vspd.go @@ -52,7 +52,7 @@ func run(ctx context.Context) error { } // Show version at startup. - log.Infof("Version %s (Go version %s %s/%s)", version.String(), runtime.Version(), + log.Criticalf("Version %s (Go version %s %s/%s)", version.String(), runtime.Version(), runtime.GOOS, runtime.GOARCH) if cfg.VspClosed { @@ -62,7 +62,7 @@ func run(ctx context.Context) error { // WaitGroup for services to signal when they have shutdown cleanly. var shutdownWg sync.WaitGroup - defer log.Info("Shutdown complete") + defer log.Criticalf("Shutdown complete") // Open database. db, err := database.Open(ctx, &shutdownWg, cfg.dbPath, cfg.BackupInterval, maxVoteChangeRecords) From 47d69a3510f59f017c9a91d10d075576b315a32b Mon Sep 17 00:00:00 2001 From: jholdstock Date: Mon, 28 Mar 2022 14:35:15 +0100 Subject: [PATCH 4/5] Use time.After instead of tickers. --- background/background.go | 4 +--- database/database.go | 6 ++---- webapi/webapi.go | 6 ++---- 3 files changed, 5 insertions(+), 11 deletions(-) diff --git a/background/background.go b/background/background.go index f4531438..92886f0e 100644 --- a/background/background.go +++ b/background/background.go @@ -379,14 +379,12 @@ func Start(shutdownCtx context.Context, wg *sync.WaitGroup, vdb *database.VspDat // Run voting wallet consistency check periodically. wg.Add(1) go func() { - ticker := time.NewTicker(consistencyInterval) consistencyLoop: for { select { case <-shutdownCtx.Done(): - ticker.Stop() break consistencyLoop - case <-ticker.C: + case <-time.After(consistencyInterval): checkWalletConsistency() } } diff --git a/database/database.go b/database/database.go index e56cbf0d..f319e2bd 100644 --- a/database/database.go +++ b/database/database.go @@ -202,19 +202,17 @@ func Open(ctx context.Context, shutdownWg *sync.WaitGroup, dbFile string, backup return nil, fmt.Errorf("upgrade failed: %w", err) } - // Start a ticker to update the backup file at the specified interval. + // Periodically update the database backup file. shutdownWg.Add(1) go func() { - ticker := time.NewTicker(backupInterval) for { select { - case <-ticker.C: + case <-time.After(backupInterval): err := writeHotBackupFile(db) if err != nil { log.Errorf("Failed to write database backup: %v", err) } case <-ctx.Done(): - ticker.Stop() shutdownWg.Done() return } diff --git a/webapi/webapi.go b/webapi/webapi.go index 7a352959..e5e29ec3 100644 --- a/webapi/webapi.go +++ b/webapi/webapi.go @@ -159,7 +159,7 @@ func Start(ctx context.Context, requestShutdown func(), shutdownWg *sync.WaitGro } }() - // Use a ticker to update cached VSP stats. + // Periodically update cached VSP stats. var refresh time.Duration if s.cfg.Debug { refresh = 1 * time.Second @@ -168,14 +168,12 @@ func Start(ctx context.Context, requestShutdown func(), shutdownWg *sync.WaitGro } shutdownWg.Add(1) go func() { - ticker := time.NewTicker(refresh) for { select { case <-ctx.Done(): - ticker.Stop() shutdownWg.Done() return - case <-ticker.C: + case <-time.After(refresh): err := updateCache(ctx, vdb, dcrd, config.NetParams, wallets) if err != nil { log.Errorf("Failed to update cached VSP stats: %v", err) From e63feed16889908749febd3b4636f3d94b2722df Mon Sep 17 00:00:00 2001 From: jholdstock Date: Mon, 28 Mar 2022 15:19:43 +0100 Subject: [PATCH 5/5] Tidy hard-coded strings. --- config.go | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/config.go b/config.go index 155a10e9..14eb30bc 100644 --- a/config.go +++ b/config.go @@ -18,20 +18,25 @@ import ( "decred.org/dcrwallet/v2/wallet/txrules" "github.com/decred/dcrd/dcrutil/v4" "github.com/decred/dcrd/hdkeychain/v3" + "github.com/decred/slog" "github.com/decred/vspd/database" "github.com/decred/vspd/version" flags "github.com/jessevdk/go-flags" ) +const appName = "vspd" + var ( defaultListen = ":8800" - defaultLogLevel = "debug" + defaultLogLevel = slog.LevelDebug.String() defaultMaxLogSize = int64(10) defaultLogsToKeep = 20 defaultVSPFee = 3.0 defaultNetwork = "testnet" - defaultHomeDir = dcrutil.AppDataDir("vspd", false) - defaultConfigFilename = "vspd.conf" + defaultHomeDir = dcrutil.AppDataDir(appName, false) + defaultConfigFilename = fmt.Sprintf("%s.conf", appName) + defaultLogFilename = fmt.Sprintf("%s.log", appName) + defaultDBFilename = fmt.Sprintf("%s.db", appName) defaultConfigFile = filepath.Join(defaultHomeDir, defaultConfigFilename) defaultDcrdHost = "127.0.0.1" defaultWalletHost = "127.0.0.1" @@ -403,11 +408,11 @@ func loadConfig() (*config, error) { // Initialize loggers and log rotation. logDir := filepath.Join(cfg.HomeDir, "logs", cfg.netParams.Name) - initLogRotator(filepath.Join(logDir, "vspd.log"), cfg.MaxLogSize, cfg.LogsToKeep) + initLogRotator(filepath.Join(logDir, defaultLogFilename), cfg.MaxLogSize, cfg.LogsToKeep) setLogLevels(cfg.LogLevel) // Set the database path - cfg.dbPath = filepath.Join(dataDir, "vspd.db") + cfg.dbPath = filepath.Join(dataDir, defaultDBFilename) // If xpub has been provided, create a new database and exit. if cfg.FeeXPub != "" {