Refactor: remove sdk.Result from market keeper

x/buyback/abci.go

@@ -2,6 +2,7 @@ package buyback
 
 import (
 	"fmt"
+
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	"github.com/e-money/em-ledger/x/buyback/internal/types"
 	markettypes "github.com/e-money/em-ledger/x/market/types"
@@ -51,13 +52,12 @@ func BeginBlocker(ctx sdk.Context, k Keeper, bk types.BankKeeper) {
 			continue
 		}
 
-		result, err := k.SendOrderToMarket(ctx, order)
-		if err != nil {
+		if err := k.SendOrderToMarket(ctx, order); err != nil {
 			ctx.Logger().Error("Error sending buyback order to market", "err", err)
 			continue
 		}
 
-		for _, ev := range result.Events {
+		for _, ev := range ctx.EventManager().Events() {
 			ctx.EventManager().EmitEvent(sdk.Event(ev))
 		}
 	}

x/buyback/abci_test.go

@@ -2,6 +2,10 @@ package buyback
 
 import (
 	"fmt"
+	"strings"
+	"testing"
+	"time"
+
 	"github.com/cosmos/cosmos-sdk/codec"
 	codectypes "github.com/cosmos/cosmos-sdk/codec/types"
 	simappparams "github.com/cosmos/cosmos-sdk/simapp/params"
@@ -15,9 +19,6 @@ import (
 	paramskeeper "github.com/cosmos/cosmos-sdk/x/params/keeper"
 	embank "github.com/e-money/em-ledger/hooks/bank"
 	tmproto "github.com/tendermint/tendermint/proto/tendermint/types"
-	"strings"
-	"testing"
-	"time"
 
 	"github.com/stretchr/testify/require"
 
@@ -141,7 +142,7 @@ func TestBuyback3(t *testing.T) {
 	// Generate some prices for the pesos <-> ungm instrument
 	acc2 := createAccount(ctx, accountKeeper, bankKeeper, randomAddress(), "10000ungm")
 
-	_, err := market.NewOrderSingle(ctx, order(acc2, "1ungm", "4000000pesos"))
+	err := market.NewOrderSingle(ctx, order(acc2, "1ungm", "4000000pesos"))
 	require.NoError(t, err)
 
 	// Attempt to create a position using the meager pesos balance of the module

x/buyback/internal/keeper/expected_keepers.go

@@ -7,10 +7,10 @@ import (
 
 type (
 	MarketKeeper interface {
-		NewOrderSingle(ctx sdk.Context, order market.Order) (*sdk.Result, error)
+		NewOrderSingle(ctx sdk.Context, order market.Order) error
 		GetOrdersByOwner(ctx sdk.Context, owner sdk.AccAddress) []*market.Order
 		GetBestPrice(ctx sdk.Context, src, dst string) *sdk.Dec
-		CancelOrder(ctx sdk.Context, owner sdk.AccAddress, clientOrderId string) (*sdk.Result, error)
+		CancelOrder(ctx sdk.Context, owner sdk.AccAddress, clientOrderId string) error
 	}
 
 	AccountKeeper interface {

x/buyback/internal/keeper/keeper.go

@@ -1,12 +1,13 @@
 package keeper
 
 import (
+	"time"
+
 	"github.com/cosmos/cosmos-sdk/codec"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	"github.com/e-money/em-ledger/x/buyback/internal/types"
 	market "github.com/e-money/em-ledger/x/market/types"
 	ptypes "github.com/gogo/protobuf/types"
-	"time"
 )
 
 type Keeper struct {
@@ -39,17 +40,16 @@ func (k Keeper) CancelCurrentModuleOrders(ctx sdk.Context) {
 	orders := k.marketKeeper.GetOrdersByOwner(ctx, buybackAccount)
 
 	for _, order := range orders {
-		result, err := k.marketKeeper.CancelOrder(ctx, buybackAccount, order.ClientOrderID)
-		if err != nil {
+		if err := k.marketKeeper.CancelOrder(ctx, buybackAccount, order.ClientOrderID); err != nil {
 			panic(err)
 		}
-		for _, ev := range result.Events {
+		for _, ev := range ctx.EventManager().Events() {
 			ctx.EventManager().EmitEvent(sdk.Event(ev))
 		}
 	}
 }
 
-func (k Keeper) SendOrderToMarket(ctx sdk.Context, order market.Order) (*sdk.Result, error) {
+func (k Keeper) SendOrderToMarket(ctx sdk.Context, order market.Order) error {
 	return k.marketKeeper.NewOrderSingle(ctx, order)
 }
 

x/market/keeper/grpc_query_test.go

@@ -1,14 +1,15 @@
 package keeper
 
 import (
+	"testing"
+	"time"
+
 	"github.com/cosmos/cosmos-sdk/baseapp"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"
 	"github.com/e-money/em-ledger/x/market/types"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
-	"testing"
-	"time"
 )
 
 func TestQueryByAccount(t *testing.T) {
@@ -31,7 +32,7 @@ func TestQueryByAccount(t *testing.T) {
 	k.setOrder(ctx, &o)
 
 	expectedPlusOne := o
-	expectedPlusOne.Created = expectedPlusOne.Created.Add(1*time.Second)
+	expectedPlusOne.Created = expectedPlusOne.Created.Add(1 * time.Second)
 
 	specs := map[string]struct {
 		req      *types.QueryByAccountRequest
@@ -45,8 +46,8 @@ func TestQueryByAccount(t *testing.T) {
 			expState: []*types.Order{&o},
 		},
 		"created plus a sec": {
-			req:      &types.QueryByAccountRequest{Address: myAddress.String()},
-			expState: []*types.Order{&expectedPlusOne},
+			req:            &types.QueryByAccountRequest{Address: myAddress.String()},
+			expState:       []*types.Order{&expectedPlusOne},
 			createdPlusOne: true,
 		},
 		"empty address": {
@@ -73,7 +74,7 @@ func TestQueryByAccount(t *testing.T) {
 			if spec.createdPlusOne {
 				assert.NotEqual(t, spec.expState, gotRsp.Orders)
 				// set equal
-				gotRsp.Orders[0].Created = gotRsp.Orders[0].Created.Add(1*time.Second)
+				gotRsp.Orders[0].Created = gotRsp.Orders[0].Created.Add(1 * time.Second)
 			}
 
 			assert.Equal(t, spec.expState, gotRsp.Orders)
@@ -129,13 +130,13 @@ func TestInstrument(t *testing.T) {
 	acc := createAccount(ctx, ak, bk, randomAddress(), "1000usd")
 
 	o := order(ctx.BlockTime(), acc, "100usd", "100chf")
-	_, err := k.NewOrderSingle(ctx, o)
+	err := k.NewOrderSingle(ctx, o)
 	require.NoError(t, err)
 
 	oPlusOne := order(
 		ctx.BlockTime().Add(time.Second), acc, "100usd", "100gbp",
 	)
-	_, err = k.NewOrderSingle(ctx, oPlusOne)
+	err = k.NewOrderSingle(ctx, oPlusOne)
 	require.NoError(t, err)
 
 	specs := map[string]struct {

x/market/keeper/keeper.go

@@ -139,7 +139,7 @@ func (k *Keeper) GetSrcFromSlippage(
 	return slippageSource, nil
 }
 
-func (k *Keeper) NewOrderSingle(ctx sdk.Context, aggressiveOrder types.Order) (*sdk.Result, error) {
+func (k *Keeper) NewOrderSingle(ctx sdk.Context, aggressiveOrder types.Order) error {
 	// save caller's event manager
 	retEvManager := ctx.EventManager()
 
@@ -162,22 +162,25 @@ func (k *Keeper) NewOrderSingle(ctx sdk.Context, aggressiveOrder types.Order) (*
 	}()
 
 	if err := aggressiveOrder.IsValid(); err != nil {
-		return nil, err
+		return err
 	}
 
 	if aggressiveOrder.IsFilled() {
-		return nil, sdkerrors.Wrapf(types.ErrInvalidPrice, "Order price is invalid: %s -> %s", aggressiveOrder.Source, aggressiveOrder.Destination)
+		return sdkerrors.Wrapf(
+			types.ErrInvalidPrice, "Order price is invalid: %s -> %s",
+			aggressiveOrder.Source, aggressiveOrder.Destination,
+		)
 	}
 
 	owner, err := sdk.AccAddressFromBech32(aggressiveOrder.Owner)
 	if err != nil {
-		return nil, sdkerrors.Wrap(sdkerrors.ErrInvalidAddress, "owner")
+		return sdkerrors.Wrap(sdkerrors.ErrInvalidAddress, "owner")
 	}
 	spendableCoins := k.bk.SpendableCoins(ctx, owner)
 
 	// Verify account balance
 	if _, anyNegative := spendableCoins.SafeSub(sdk.NewCoins(aggressiveOrder.Source)); anyNegative {
-		return nil, sdkerrors.Wrapf(
+		return sdkerrors.Wrapf(
 			types.ErrAccountBalanceInsufficient,
 			"Account %v has insufficient balance to execute trade: %v < %v",
 			owner,
@@ -193,17 +196,23 @@ func (k *Keeper) NewOrderSingle(ctx sdk.Context, aggressiveOrder types.Order) (*
 	totalSourceDemand = totalSourceDemand.Add(aggressiveOrder.Source)
 	if _, anyNegative := spendableCoins.SafeSub(sdk.NewCoins(totalSourceDemand)); anyNegative {
 		// TODO Improve message
-		return nil, sdkerrors.Wrapf(types.ErrAccountBalanceInsufficientForInstrument, "")
+		return sdkerrors.Wrapf(
+			types.ErrAccountBalanceInsufficientForInstrument, "",
+		)
 	}
 
 	// Verify uniqueness of client order id among active orders
 	if containsClientId(accountOrders, aggressiveOrder.ClientOrderID) {
-		return nil, sdkerrors.Wrap(types.ErrNonUniqueClientOrderId, aggressiveOrder.ClientOrderID)
+		return sdkerrors.Wrap(
+			types.ErrNonUniqueClientOrderId, aggressiveOrder.ClientOrderID,
+		)
 	}
 
 	// Verify that the destination asset actually exists on chain before creating an instrument
 	if !k.assetExists(ctx, aggressiveOrder.Destination) {
-		return nil, sdkerrors.Wrap(types.ErrUnknownAsset, aggressiveOrder.Destination.Denom)
+		return sdkerrors.Wrap(
+			types.ErrUnknownAsset, aggressiveOrder.Destination.Denom,
+		)
 	}
 	k.registerMarketData(ctx, aggressiveOrder.Source.Denom, aggressiveOrder.Destination.Denom)
 	k.registerMarketData(ctx, aggressiveOrder.Destination.Denom, aggressiveOrder.Source.Denom)
@@ -349,7 +358,7 @@ func (k *Keeper) NewOrderSingle(ctx sdk.Context, aggressiveOrder types.Order) (*
 		}
 	}
 
-	return &sdk.Result{}, nil
+	return nil
 }
 
 // Check whether an asset even exists on the chain at the moment.
@@ -358,36 +367,38 @@ func (k Keeper) assetExists(ctx sdk.Context, asset sdk.Coin) bool {
 	return total.AmountOf(asset.Denom).GT(sdk.ZeroInt())
 }
 
-func (k *Keeper) CancelReplaceLimitOrder(ctx sdk.Context, newOrder types.Order, origClientOrderId string) (*sdk.Result, error) {
+func (k *Keeper) CancelReplaceLimitOrder(ctx sdk.Context, newOrder types.Order, origClientOrderId string) error {
 	// Use a fixed gas amount
 	ctx.GasMeter().ConsumeGas(gasPriceCancelReplaceOrder, "CancelReplaceOrder")
 	ctx = ctx.WithGasMeter(sdk.NewInfiniteGasMeter())
 
 	origOrder := k.GetOrderByOwnerAndClientOrderId(ctx, newOrder.Owner, origClientOrderId)
 
 	if origOrder == nil {
-		return nil, sdkerrors.Wrap(types.ErrClientOrderIdNotFound, origClientOrderId)
+		return sdkerrors.Wrap(types.ErrClientOrderIdNotFound, origClientOrderId)
 	}
 
 	// Verify that instrument is the same.
 	if origOrder.Source.Denom != newOrder.Source.Denom || origOrder.Destination.Denom != newOrder.Destination.Denom {
-		return nil, sdkerrors.Wrap(
+		return sdkerrors.Wrap(
 			types.ErrOrderInstrumentChanged, fmt.Sprintf(
-				"source %s != %s Or dest %s != %s", origOrder.Source, newOrder.Source,
+				"source %s != %s Or dest %s != %s", origOrder.Source,
+				newOrder.Source,
 				origOrder.Destination.Denom, newOrder.Destination.Denom,
 			),
 		)
 	}
 
 	if origOrder.ClientOrderID == newOrder.ClientOrderID {
-		return nil, sdkerrors.Wrap(
-			types.ErrInvalidClientOrderId, fmt.Sprintf("ClientOrderId is already in use"),
+		return sdkerrors.Wrap(
+			types.ErrInvalidClientOrderId,
+			fmt.Sprintf("ClientOrderId is already in use"),
 		)
 	}
 
 	// Has the previous order already achieved the goal on the source side?
 	if origOrder.SourceFilled.GTE(newOrder.Source.Amount) {
-		return nil, sdkerrors.Wrap(types.ErrNoSourceRemaining, "")
+		return sdkerrors.Wrap(types.ErrNoSourceRemaining, "")
 	}
 
 	k.deleteOrder(ctx, origOrder)
@@ -422,7 +433,9 @@ func (k *Keeper) GetOrderByOwnerAndClientOrderId(ctx sdk.Context, owner, clientO
 	return o
 }
 
-func (k *Keeper) CancelOrder(ctx sdk.Context, owner sdk.AccAddress, clientOrderId string) (*sdk.Result, error) {
+func (k *Keeper) CancelOrder(
+	ctx sdk.Context, owner sdk.AccAddress, clientOrderId string,
+) error {
 	// Use a fixed gas amount
 	ctx.GasMeter().ConsumeGas(gasPriceCancelOrder, "CancelOrder")
 	ctx = ctx.WithGasMeter(sdk.NewInfiniteGasMeter())
@@ -431,13 +444,13 @@ func (k *Keeper) CancelOrder(ctx sdk.Context, owner sdk.AccAddress, clientOrderI
 	order := k.GetOrderByOwnerAndClientOrderId(ctx, owner.String(), clientOrderId)
 
 	if order == nil {
-		return nil, sdkerrors.Wrap(types.ErrClientOrderIdNotFound, clientOrderId)
+		return sdkerrors.Wrap(types.ErrClientOrderIdNotFound, clientOrderId)
 	}
 
 	types.EmitExpireEvent(ctx, *order)
 	k.deleteOrder(ctx, order)
 
-	return &sdk.Result{}, nil
+	return nil
 }
 
 // Update any orders that can no longer be filled with the account's balance.

x/market/keeper/keeper_fuzzing_test.go

@@ -7,14 +7,15 @@ package keeper
 import (
 	crand "crypto/rand"
 	"fmt"
-	authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
 	"math"
 	"math/big"
 	"math/rand"
 	"runtime/debug"
 	"testing"
 	"time"
 
+	authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
+
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	"github.com/e-money/em-ledger/x/market/types"
 	"github.com/stretchr/testify/require"
@@ -93,7 +94,7 @@ func TestFuzzing1(t *testing.T) {
 	})
 
 	for _, order := range allOrders {
-		_, err := k.NewOrderSingle(ctx, order)
+		err := k.NewOrderSingle(ctx, order)
 		if order.IsFilled() {
 			fmt.Println("Order is filled on creation. Ignoring.", order)
 			continue