From 0fbf23541f28b70c823d045c0542dbb17709c7a3 Mon Sep 17 00:00:00 2001 From: Simon Esposito Date: Thu, 4 Feb 2021 12:52:50 +0000 Subject: [PATCH] Improve error on invalid wallet update (#546) Return WalletNegativeError when updating a wallet with an amount higher than the available funds. Resolves #468 Do not log such an error when it occurs. Resolves #540 --- go.mod | 2 +- go.sum | 6 ++---- server/core_wallet.go | 21 +++++++++---------- .../nakama-common/runtime/runtime.go | 12 +++++++++++ vendor/modules.txt | 2 +- 5 files changed, 26 insertions(+), 17 deletions(-) diff --git a/go.mod b/go.mod index 7b7c04985..38c50b805 100644 --- a/go.mod +++ b/go.mod @@ -25,7 +25,7 @@ require ( github.com/gorilla/mux v1.7.4 github.com/gorilla/websocket v1.4.2 github.com/grpc-ecosystem/grpc-gateway/v2 v2.0.1 - github.com/heroiclabs/nakama-common v1.11.1-0.20210203184030-64cd8d070d43 + github.com/heroiclabs/nakama-common v1.11.1-0.20210204123435-b06844f83f0c github.com/jackc/fake v0.0.0-20150926172116-812a484cc733 // indirect github.com/jackc/pgx v3.5.0+incompatible github.com/jmhodges/levigo v1.0.0 // indirect diff --git a/go.sum b/go.sum index dbd7e1344..689cdbe9d 100644 --- a/go.sum +++ b/go.sum @@ -198,10 +198,8 @@ github.com/grpc-ecosystem/grpc-gateway/v2 v2.0.1/go.mod h1:oVMjMN64nzEcepv1kdZKg github.com/hashicorp/golang-lru v0.5.0/go.mod h1:/m3WP610KZHVQ1SGc6re/UDhFvYD7pJ4Ao+sR/qLZy8= github.com/hashicorp/golang-lru v0.5.1/go.mod h1:/m3WP610KZHVQ1SGc6re/UDhFvYD7pJ4Ao+sR/qLZy8= github.com/hashicorp/hcl v1.0.0/go.mod h1:E5yfLk+7swimpb2L/Alb/PJmXilQ/rhwaUYs4T20WEQ= -github.com/heroiclabs/nakama-common v1.11.0 h1:IbQvD736ppD0UiYAoFwWwMefQmVaaLp5SU763j4y6vA= -github.com/heroiclabs/nakama-common v1.11.0/go.mod h1:li7bMQwOYA0NjT3DM4NKQBNruULPa2hrqdiSaaTwui4= -github.com/heroiclabs/nakama-common v1.11.1-0.20210203184030-64cd8d070d43 h1:+q51X8av/JPue1dqq++eXarA/Wco6q2jvyAZdPvzWC8= -github.com/heroiclabs/nakama-common v1.11.1-0.20210203184030-64cd8d070d43/go.mod h1:li7bMQwOYA0NjT3DM4NKQBNruULPa2hrqdiSaaTwui4= +github.com/heroiclabs/nakama-common v1.11.1-0.20210204123435-b06844f83f0c h1:h3vOxFXzI59n2zN9O2I6y8hxFs3D5c2bVTcy0AL+fZE= +github.com/heroiclabs/nakama-common v1.11.1-0.20210204123435-b06844f83f0c/go.mod h1:li7bMQwOYA0NjT3DM4NKQBNruULPa2hrqdiSaaTwui4= github.com/hpcloud/tail v1.0.0 h1:nfCOvKYfkgYP8hkirhJocXT2+zOD8yUNjXaWfTlyFKI= github.com/hpcloud/tail v1.0.0/go.mod h1:ab1qPbhIpdTxEkNHXyeSf5vhxWSCs/tWer42PpOxQnU= github.com/ianlancetaylor/demangle v0.0.0-20181102032728-5e5cf60278f6/go.mod h1:aSSvb/t6k1mPoxDqO4vJh6VOCGPwU4O0C2/Eqndh1Sc= diff --git a/server/core_wallet.go b/server/core_wallet.go index f25be8c33..bf27180b9 100644 --- a/server/core_wallet.go +++ b/server/core_wallet.go @@ -104,7 +104,9 @@ func UpdateWallets(ctx context.Context, logger *zap.Logger, db *sql.DB, updates } return nil }); err != nil { - logger.Error("Error updating wallets.", zap.Error(err)) + if _, ok := err.(*runtime.WalletNegativeError); !ok { + logger.Error("Error updating wallets.", zap.Error(err)) + } // Ensure there are no partially updated wallets returned as results, they would not be reflected in database anyway. for _, result := range results { result.Updated = nil @@ -171,7 +173,6 @@ func updateWallets(ctx context.Context, logger *zap.Logger, tx *sql.Tx, updates } // Go through the changesets and attempt to calculate the new state for each wallet. - var changesetErr error for _, update := range updates { userID := update.UserID.String() walletMap, ok := wallets[userID] @@ -191,15 +192,16 @@ func updateWallets(ctx context.Context, logger *zap.Logger, tx *sql.Tx, updates // Existing value may be 0 or missing. newValue := walletMap[k] + v if newValue < 0 { - // Programmer error, no need to log. - changesetErr = fmt.Errorf("wallet update rejected negative value at path '%v'", k) - continue + // Insufficient funds + return nil, &runtime.WalletNegativeError{ + UserID: userID, + Path: k, + Current: walletMap[k], + Amount: v, + } } walletMap[k] = newValue } - if changesetErr != nil { - continue - } result.Updated = walletMap results = append(results, result) @@ -224,9 +226,6 @@ func updateWallets(ctx context.Context, logger *zap.Logger, tx *sql.Tx, updates statements = append(statements, fmt.Sprintf("($%v::UUID, $%v, $%v, $%v)", strconv.Itoa(len(params)-3), strconv.Itoa(len(params)-2), strconv.Itoa(len(params)-1), strconv.Itoa(len(params)))) } } - if changesetErr != nil { - return nil, changesetErr - } if len(updatedWallets) > 0 { // Ensure updates are done in natural order of user ID. diff --git a/vendor/github.com/heroiclabs/nakama-common/runtime/runtime.go b/vendor/github.com/heroiclabs/nakama-common/runtime/runtime.go index 8a5596c77..1ebae8244 100644 --- a/vendor/github.com/heroiclabs/nakama-common/runtime/runtime.go +++ b/vendor/github.com/heroiclabs/nakama-common/runtime/runtime.go @@ -88,6 +88,7 @@ package runtime import ( "context" "database/sql" + "fmt" "os" "github.com/heroiclabs/nakama-common/api" @@ -773,6 +774,17 @@ type WalletUpdateResult struct { Previous map[string]int64 } +type WalletNegativeError struct { + UserID string + Path string + Current int64 + Amount int64 +} + +func (e *WalletNegativeError) Error() string { + return fmt.Sprintf("wallet update rejected negative value at path '%v'", e.Path) +} + type WalletLedgerItem interface { GetID() string GetUserID() string diff --git a/vendor/modules.txt b/vendor/modules.txt index 910df2974..e2f43e3a7 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -135,7 +135,7 @@ github.com/grpc-ecosystem/grpc-gateway/v2/protoc-gen-openapiv2/internal/genopena github.com/grpc-ecosystem/grpc-gateway/v2/protoc-gen-openapiv2/options github.com/grpc-ecosystem/grpc-gateway/v2/runtime github.com/grpc-ecosystem/grpc-gateway/v2/utilities -# github.com/heroiclabs/nakama-common v1.11.1-0.20210203184030-64cd8d070d43 +# github.com/heroiclabs/nakama-common v1.11.1-0.20210204123435-b06844f83f0c github.com/heroiclabs/nakama-common/api github.com/heroiclabs/nakama-common/rtapi github.com/heroiclabs/nakama-common/runtime -- GitLab