From 6912d627edf0202988dcb9243ab49376d80b6754 Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Thu, 30 Sep 2021 12:02:16 +0000 Subject: [PATCH] Refactor market keeper against master (#150) (#161) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Rely on ctx's event manager * 136 inflation (#141) * Add upgrade handler for lilmermaid-16 * Externalize inflation types, keeper packages * Group & sort imports Co-authored-by: Martin Dyring-Andersen Co-authored-by: Mario Karagiorgas * Refactor: remove result from keeper methods * Add description to the order errors * Parse tx events and add market tx events checks * Update AddMarketOrder to return events * Add event check * Add event validation for all market orders * Simplify event validations * Change error wording * Move event manager before return, PR feedback Co-authored-by: Henrik Aasted Sørensen Co-authored-by: Martin Dyring-Andersen (cherry picked from commit feaf93a7377ca517c9fc6a09ae9fb2f0775adce0) Co-authored-by: Mario Karagiorgas --- market_test.go | 76 ++++- networktest/emcli.go | 114 +++++++- x/buyback/abci.go | 8 +- x/buyback/abci_test.go | 9 +- x/buyback/internal/keeper/expected_keepers.go | 4 +- x/buyback/internal/keeper/keeper.go | 8 +- x/market/keeper/grpc_query_test.go | 17 +- x/market/keeper/keeper.go | 63 +++-- x/market/keeper/keeper_fuzzing_test.go | 5 +- x/market/keeper/keeper_test.go | 266 +++++++++--------- x/market/keeper/msg_server.go | 24 +- x/market/keeper/msg_server_test.go | 110 ++++---- x/market/keeper/querier_test.go | 31 +- x/market/types/errors.go | 25 +- 14 files changed, 472 insertions(+), 288 deletions(-) diff --git a/market_test.go b/market_test.go index 8cf1f859..c283636c 100644 --- a/market_test.go +++ b/market_test.go @@ -8,6 +8,11 @@ package emoney_test import ( "fmt" + "io/ioutil" + "os" + "strings" + "time" + sdk "github.com/cosmos/cosmos-sdk/types" banktypes "github.com/cosmos/cosmos-sdk/x/bank/types" emoney "github.com/e-money/em-ledger" @@ -18,10 +23,6 @@ import ( tmrand "github.com/tendermint/tendermint/libs/rand" "github.com/tidwall/gjson" "github.com/tidwall/sjson" - "io/ioutil" - "os" - "strings" - "time" ) var _ = Describe("Market", func() { @@ -75,6 +76,39 @@ var _ = Describe("Market", func() { Expect(ir.Get("orders").Array()).To(HaveLen(10)) }) + It("Check accompanying events for all types of market orders", func() { + output, events, success, err := emcli.MarketAddLimitOrderRetEvents(acc1, "120eeur", "90echf", "acc1ev1") + Expect(err).ToNot(HaveOccurred(), "Error output: %v", output) + Expect(success).To(BeTrue()) + checkTxEvents(events, false) + + // A complement order from another account to be filled and check events + output, events, success, err = emcli.MarketAddLimitOrderRetEvents(acc2, "90echf", "120eeur", "acc2ev2") + Expect(err).ToNot(HaveOccurred(), "Error output: %v", output) + Expect(success).To(BeTrue()) + checkTxEvents(events, true) + + // Place an order that won't be filled + firstOrderID := "acc1ev3" + output, events, success, err = emcli.MarketAddLimitOrderRetEvents(acc1, "100eeur", "100echf", firstOrderID) + Expect(err).ToNot(HaveOccurred(), "Error output: %v", output) + Expect(success).To(BeTrue()) + checkTxEvents(events, false) + + // Replace order with another pessimal + replacingOrderID := "acc1ev4" + output, events, success, err = emcli.MarketCancelReplaceOrder(acc1, firstOrderID, "200eeur", "200echf", replacingOrderID) + Expect(err).ToNot(HaveOccurred(), "Error output: %v", output) + Expect(success).To(BeTrue()) + checkTxEvents(events, false) + + // Cancel last order + _, success, err = emcli.MarketCancelOrder(acc1, replacingOrderID) + Expect(err).ToNot(HaveOccurred()) + Expect(success).To(BeTrue()) + checkTxEvents(events, false) + }) + It("Crashing validator can catch up", func() { var ( err error @@ -223,3 +257,37 @@ var _ = Describe("Market", func() { }) }) }) + +// checkTxEvents looks for the `accept` and optionally the `fill` event attributes +func checkTxEvents(events sdk.Events, searchFill bool) { + const ( + fillEventAttrValue = "fill" + acceptEventAttrValue = "accept" + ) + + Expect(len(events) >= 2).To(BeTrue()) + var foundAccept, foundFill bool + for _, event := range events { + if event.Type == "market" { + for _, evAttr := range event.Attributes { + if string(evAttr.Key) == "action" { + if string(evAttr.Value) == acceptEventAttrValue && !searchFill { + return + } + foundAccept = true + } + if searchFill { + if string(evAttr.Value) == fillEventAttrValue && foundAccept { + return + } + foundFill = true + } + } + } + } + + Expect(foundAccept).To(BeTrue(), "did not find the accept event") + if searchFill { + Expect(foundFill).To(BeTrue(), "did not find the fill event") + } +} diff --git a/networktest/emcli.go b/networktest/emcli.go index 0e3d7898..b3ef0377 100644 --- a/networktest/emcli.go +++ b/networktest/emcli.go @@ -14,6 +14,8 @@ import ( "strconv" "strings" + "github.com/tendermint/tendermint/abci/types" + sdk "github.com/cosmos/cosmos-sdk/types" "github.com/tidwall/gjson" ) @@ -316,20 +318,39 @@ func (cli Emcli) LiquidityProviderBurn(key Key, amount string) (string, bool, er } func (cli Emcli) MarketAddLimitOrder(key Key, source, destination, cid string, moreflags ...string) (string, bool, error) { + tx, _, success, err := cli.MarketAddLimitOrderRetEvents(key, source, destination, cid, moreflags...) + + return tx, success, err +} + +func (cli Emcli) MarketAddLimitOrderRetEvents(key Key, source, destination, cid string, moreflags ...string) (string, sdk.Events, bool, error) { args := cli.addTransactionFlags("tx", "market", "add-limit", source, destination, cid, "--from", key.name) args = append(args, moreflags...) - return execCmdWithInput(args, KeyPwd) + return execCmdWithInputRetEvents(args, KeyPwd) } -func (cli Emcli) MarketAddMarketOrder(key Key, sourceDenom, destination, cid string, slippage sdk.Dec, moreflags ...string) (string, bool, error) { +func (cli Emcli) MarketAddMarketOrder(key Key, sourceDenom, destination, cid string, slippage sdk.Dec, moreflags ...string) (string, sdk.Events, bool, error) { args := cli.addTransactionFlags("tx", "market", "add-market", sourceDenom, destination, slippage.String(), cid, "--from", key.name) args = append(args, moreflags...) - return execCmdWithInput(args, KeyPwd) + + return execCmdWithInputRetEvents(args, KeyPwd) +} + +func (cli Emcli) MarketCancelReplaceOrder(key Key, prevCid, sourceDenom, destination, newCid string) (string, sdk.Events, bool, error) { + args := cli.addTransactionFlags("tx", "market", "cancelreplace", prevCid, sourceDenom, destination, newCid, "--from", key.name) + + return execCmdWithInputRetEvents(args, KeyPwd) } func (cli Emcli) MarketCancelOrder(key Key, cid string) (string, bool, error) { + tx, _, success, err := cli.MarketCancelOrderRetEvents(key, cid) + return tx, success, err +} + +func (cli Emcli) MarketCancelOrderRetEvents(key Key, cid string) (string, sdk.Events, bool, error) { args := cli.addTransactionFlags("tx", "market", "cancel", cid, "--from", key.name) - return execCmdWithInput(args, KeyPwd) + + return execCmdWithInputRetEvents(args, KeyPwd) } func (cli Emcli) UnjailValidator(key string) (string, bool, error) { @@ -386,6 +407,68 @@ func extractTxHash(bz []byte) (txhash string, success bool, err error) { return txhashjson.Str, true, nil } +func extractTxWithEvents(bz []byte) (txhash string, evList sdk.Events, success bool, err error) { + if !gjson.ValidBytes(bz) { + return "", evList, false, fmt.Errorf("extractTxWithEvents received input that was not valid JSON:\n%v", string(bz)) + } + + json := gjson.ParseBytes(bz) + + txhashjson := json.Get("txhash") + logs := json.Get("logs") + code := json.Get("code") + + // todo (reviewer) : emd command returns `exit 0` although the TX has failed with `signature verification failed` + // any non zero `code` in response json is a failure code + if !txhashjson.Exists() || !logs.Exists() || code.Int() != 0 { + return "", evList, false, fmt.Errorf("tx appears to have failed %v", string(bz)) + } + + if strings.Contains(logs.Raw, "failed") { + return "", evList, false, fmt.Errorf("tx failed: %s", logs.Raw) + } + + evList = getTxEvents(logs) + + return txhashjson.Str, evList, true, nil +} + +func getTxEvents(logs gjson.Result) (evList sdk.Events) { + logs.ForEach( + func(_, value gjson.Result) bool { + events := value.Get("events") + events.ForEach( + func(key, value gjson.Result) bool { + ev := sdk.Event{ + Type: value.Get("type").Str, + Attributes: []types.EventAttribute{}, + } + + evAttrs := value.Get("attributes") + evAttrs.ForEach( + func(_, value gjson.Result) bool { + k := value.Get("key").Str + v := value.Get("value").Str + ev.Attributes = append( + ev.Attributes, types.EventAttribute{ + Key: []byte(k), + Value: []byte(v), + }, + ) + + return true + }, + ) + evList = append(evList, ev) + + return true + }) + return true + }) + + return evList +} + func execCmdCollectOutput(arguments []string, input string, checkTxRes bool) (string, error) { cmd := exec.Command(EMCLI, arguments...) @@ -448,6 +531,29 @@ func execCmdWithInput(arguments []string, input string) (string, bool, error) { return extractTxHash(bz) } +func execCmdWithInputRetEvents(arguments []string, input string) (string, sdk.Events, bool, error) { + cmd := exec.Command(EMCLI, arguments...) + + stdin, err := cmd.StdinPipe() + if err != nil { + return "", sdk.Events{}, false, err + } + + _, err = io.WriteString(stdin, input+"\n") + if err != nil { + return "", sdk.Events{}, false, err + } + + // fmt.Println(" *** Running command: ", EMCLI, strings.Join(arguments, " ")) + bz, err := cmd.CombinedOutput() + // fmt.Println(" *** CombinedOutput", string(bz)) + if err != nil { + return "", sdk.Events{}, false, err + } + + return extractTxWithEvents(bz) +} + func execCmdAndCollectResponse(arguments []string) ([]byte, error) { // fmt.Println(" *** Running command: ", EMCLI, strings.Join(arguments, " ")) bz, err := exec.Command(EMCLI, arguments...).CombinedOutput() diff --git a/x/buyback/abci.go b/x/buyback/abci.go index ea615488..7b6a12ec 100644 --- a/x/buyback/abci.go +++ b/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,15 +52,10 @@ 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 { - ctx.EventManager().EmitEvent(sdk.Event(ev)) - } } err := k.BurnStakingToken(ctx) diff --git a/x/buyback/abci_test.go b/x/buyback/abci_test.go index 78ad55c3..e7b88dcc 100644 --- a/x/buyback/abci_test.go +++ b/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 diff --git a/x/buyback/internal/keeper/expected_keepers.go b/x/buyback/internal/keeper/expected_keepers.go index fed08c67..fa4655b5 100644 --- a/x/buyback/internal/keeper/expected_keepers.go +++ b/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 { diff --git a/x/buyback/internal/keeper/keeper.go b/x/buyback/internal/keeper/keeper.go index 20775bcb..89c3e830 100644 --- a/x/buyback/internal/keeper/keeper.go +++ b/x/buyback/internal/keeper/keeper.go @@ -41,8 +41,7 @@ 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 { ctx.Logger().Error( fmt.Sprintf( "The buyback module could not create market order %s, error:%v", @@ -52,13 +51,10 @@ func (k Keeper) CancelCurrentModuleOrders(ctx sdk.Context) { return } - for _, ev := range result.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) } diff --git a/x/market/keeper/grpc_query_test.go b/x/market/keeper/grpc_query_test.go index 7789aa6e..e59614e9 100644 --- a/x/market/keeper/grpc_query_test.go +++ b/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 { diff --git a/x/market/keeper/keeper.go b/x/market/keeper/keeper.go index 7bdf7e1c..06da80b7 100644 --- a/x/market/keeper/keeper.go +++ b/x/market/keeper/keeper.go @@ -6,16 +6,15 @@ package keeper import ( "fmt" - banktypes "github.com/cosmos/cosmos-sdk/x/bank/types" "math" "sync" "time" - sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" - "github.com/e-money/em-ledger/x/market/types" - "github.com/cosmos/cosmos-sdk/codec" sdk "github.com/cosmos/cosmos-sdk/types" + sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" + banktypes "github.com/cosmos/cosmos-sdk/x/bank/types" + "github.com/e-money/em-ledger/x/market/types" ) const ( @@ -140,7 +139,10 @@ 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() + // Use a fixed gas amount ctx.GasMeter().ConsumeGas(gasPriceNewOrder, "NewOrderSingle") ctx = ctx.WithGasMeter(sdk.NewInfiniteGasMeter()) @@ -158,22 +160,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, @@ -189,17 +194,17 @@ 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) @@ -345,7 +350,9 @@ func (k *Keeper) NewOrderSingle(ctx sdk.Context, aggressiveOrder types.Order) (* } } - return &sdk.Result{Events: ctx.EventManager().ABCIEvents()}, nil + retEvManager.EmitEvents(ctx.EventManager().Events()) + + return nil } // Check whether an asset even exists on the chain at the moment. @@ -354,7 +361,7 @@ 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()) @@ -362,28 +369,30 @@ func (k *Keeper) CancelReplaceLimitOrder(ctx sdk.Context, newOrder types.Order, 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) @@ -396,13 +405,7 @@ func (k *Keeper) CancelReplaceLimitOrder(ctx sdk.Context, newOrder types.Order, newOrder.TimeInForce = origOrder.TimeInForce - resAdd, err := k.NewOrderSingle(ctx, newOrder) - if err != nil { - return nil, err - } - - evts := append(ctx.EventManager().ABCIEvents(), resAdd.Events...) - return &sdk.Result{Events: evts}, nil + return k.NewOrderSingle(ctx, newOrder) } func (k *Keeper) GetOrderByOwnerAndClientOrderId(ctx sdk.Context, owner, clientOrderId string) *types.Order { @@ -424,7 +427,7 @@ 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()) @@ -433,13 +436,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. diff --git a/x/market/keeper/keeper_fuzzing_test.go b/x/market/keeper/keeper_fuzzing_test.go index 3b832940..6986d13d 100644 --- a/x/market/keeper/keeper_fuzzing_test.go +++ b/x/market/keeper/keeper_fuzzing_test.go @@ -7,7 +7,6 @@ package keeper import ( crand "crypto/rand" "fmt" - authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" "math" "math/big" "math/rand" @@ -15,6 +14,8 @@ import ( "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 diff --git a/x/market/keeper/keeper_test.go b/x/market/keeper/keeper_test.go index 2c870c1e..1b575c7d 100644 --- a/x/market/keeper/keeper_test.go +++ b/x/market/keeper/keeper_test.go @@ -7,6 +7,11 @@ package keeper import ( "encoding/json" "fmt" + "math" + "strings" + "testing" + "time" + "github.com/cosmos/cosmos-sdk/client" clienttx "github.com/cosmos/cosmos-sdk/client/tx" "github.com/cosmos/cosmos-sdk/codec" @@ -36,10 +41,6 @@ import ( tmrand "github.com/tendermint/tendermint/libs/rand" tmproto "github.com/tendermint/tendermint/proto/tendermint/types" dbm "github.com/tendermint/tm-db" - "math" - "strings" - "testing" - "time" ) func init() { @@ -63,7 +64,7 @@ func TestBasicTrade(t *testing.T) { gasmeter := sdk.NewGasMeter(math.MaxUint64) src1, dst1 := "eur", "usd" order1 := order(ctx.BlockTime(), acc1, "100"+src1, "120"+dst1) - _, err := k.NewOrderSingle(ctx.WithGasMeter(gasmeter), order1) + err := k.NewOrderSingle(ctx.WithGasMeter(gasmeter), order1) require.NoError(t, err) require.Equal(t, gasPriceNewOrder, gasmeter.GasConsumed()) require.Equal(t, ctx.BlockTime(), order1.Created) @@ -78,7 +79,7 @@ func TestBasicTrade(t *testing.T) { gasmeter = sdk.NewGasMeter(math.MaxUint64) src2, dst2 := dst1, src1 order2 := order(ctx.BlockTime(), acc2, "60"+src2, "50"+dst2) - _, err = k.NewOrderSingle(ctx.WithGasMeter(gasmeter), order2) + err = k.NewOrderSingle(ctx.WithGasMeter(gasmeter), order2) require.NoError(t, err) // Ensure that the trade has been correctly registered in market data. @@ -130,12 +131,12 @@ func TestBasicTrade2(t *testing.T) { totalSupply := snapshotAccounts(ctx, bk) order1 := order(ctx.BlockTime(), acc1, "888eur", "1121usd") - _, err := k.NewOrderSingle(ctx, order1) + err := k.NewOrderSingle(ctx, order1) require.NoError(t, err) order2 := order(ctx.BlockTime(), acc2, "1120usd", "890eur") - res, err := k.NewOrderSingle(ctx, order2) - require.True(t, err == nil, res.Log) + err = k.NewOrderSingle(ctx, order2) + require.NoError(t, err) require.True(t, totalSupply.Sub(snapshotAccounts(ctx, bk)).IsZero()) } @@ -150,16 +151,16 @@ func TestBasicTrade3(t *testing.T) { totalSupply := snapshotAccounts(ctx, bk) order1 := order(ctx.BlockTime(), acc2, "888850eur", "22807162chf") - _, err := k.NewOrderSingle(ctx, order1) + err := k.NewOrderSingle(ctx, order1) require.NoError(t, err) order2 := order(ctx.BlockTime(), acc3, "12chf", "4usd") - _, err = k.NewOrderSingle(ctx, order2) + err = k.NewOrderSingle(ctx, order2) require.NoError(t, err) order3 := order(ctx.BlockTime(), acc1, "227156usd", "24971eur") - _, err = k.NewOrderSingle(ctx, order3) + err = k.NewOrderSingle(ctx, order3) require.NoError(t, err) require.True(t, totalSupply.Sub(snapshotAccounts(ctx, bk)).IsZero()) @@ -176,24 +177,28 @@ func TestMarketOrderSlippage1(t *testing.T) { // Establish market price by executing a 1:1 trade o = order(ctx.BlockTime(), acc2, "1eur", "1gbp") - _, err = k.NewOrderSingle(ctx, o) + err = k.NewOrderSingle(ctx, o) require.NoError(t, err) + require.True(t, findEventAttr(ctx, "accept")) o = order(ctx.BlockTime(), acc1, "1gbp", "1eur") - _, err = k.NewOrderSingle(ctx, o) + err = k.NewOrderSingle(ctx, o) require.NoError(t, err) + require.True(t, findEventAttr(ctx, "accept")) + require.True(t, findEventAttr(ctx, "fill")) + require.True(t, findEventAttr(ctx, "expire")) // Sell eur at various prices o = order(ctx.BlockTime(), acc2, "50eur", "50gbp") - _, err = k.NewOrderSingle(ctx, o) + err = k.NewOrderSingle(ctx, o) require.NoError(t, err) o = order(ctx.BlockTime(), acc2, "50eur", "75gbp") - _, err = k.NewOrderSingle(ctx, o) + err = k.NewOrderSingle(ctx, o) require.NoError(t, err) o = order(ctx.BlockTime(), acc2, "50eur", "100gbp") - _, err = k.NewOrderSingle(ctx, o) + err = k.NewOrderSingle(ctx, o) require.NoError(t, err) // Make a market order that allows slippage @@ -206,7 +211,7 @@ func TestMarketOrderSlippage1(t *testing.T) { ) require.NoError(t, err) limitOrder := order(ctx.BlockTime(), acc1, slippageSource.String(), dest.String()) - _, err = k.NewOrderSingle(ctx, limitOrder) + err = k.NewOrderSingle(ctx, limitOrder) require.NoError(t, err) // Check that the balance matches the first two orders being executed while the third did not fall within the slippage @@ -223,7 +228,7 @@ func TestMarketOrderSlippage1(t *testing.T) { ) require.NoError(t, err) limitOrder = order(ctx.BlockTime(), acc1, slippageSource.String(), dest.String()) - _, err = k.NewOrderSingle(ctx, limitOrder) + err = k.NewOrderSingle(ctx, limitOrder) require.True(t, types.ErrAccountBalanceInsufficient.Is(err)) } @@ -238,11 +243,11 @@ func TestCancelReplaceMarketOrderZeroSlippage(t *testing.T) { // Establish market price by executing a 1:1 trade o = order(ctx.BlockTime(), acc2, "1eur", "1gbp") - _, err = k.NewOrderSingle(ctx, o) + err = k.NewOrderSingle(ctx, o) require.NoError(t, err) o = order(ctx.BlockTime(), acc1, "1gbp", "1eur") - _, err = k.NewOrderSingle(ctx, o) + err = k.NewOrderSingle(ctx, o) require.NoError(t, err) // Make a market order that allows slippage @@ -254,7 +259,7 @@ func TestCancelReplaceMarketOrderZeroSlippage(t *testing.T) { ) require.NoError(t, err) limitOrder := order(ctx.BlockTime(), acc1, slippageSource.String(), dest.String()) - _, err = k.NewOrderSingle(ctx, limitOrder) + err = k.NewOrderSingle(ctx, limitOrder) require.NoError(t, err) clientID := limitOrder.ClientOrderID @@ -280,13 +285,13 @@ func TestCancelReplaceMarketOrderZeroSlippage(t *testing.T) { require.NoError(t, err) // The order id has been re-used from the previous order, which causes an error - _, err = k.CancelReplaceLimitOrder(ctx, order, clientID) + err = k.CancelReplaceLimitOrder(ctx, order, clientID) require.True(t, types.ErrInvalidClientOrderId.Is(err), "Unexpected error \"%v\"", err) newClientID := cid() order.ClientOrderID = newClientID - _, err = k.CancelReplaceLimitOrder(ctx, order, clientID) + err = k.CancelReplaceLimitOrder(ctx, order, clientID) require.NoError(t, err) expOrder := &types.Order{ @@ -330,11 +335,11 @@ func TestCancelReplaceMarketOrder100Slippage(t *testing.T) { // Establish market price by executing a 2:1 trade o = order(ctx.BlockTime(), acc2, "20eur", "10gbp") - _, err = k.NewOrderSingle(ctx, o) + err = k.NewOrderSingle(ctx, o) require.NoError(t, err) o = order(ctx.BlockTime(), acc1, "10gbp", "20eur") - _, err = k.NewOrderSingle(ctx, o) + err = k.NewOrderSingle(ctx, o) require.NoError(t, err) acc1b := bk.GetAllBalances(ctx, acc1.GetAddress()) require.Equal(t, coins("20eur,90gbp").String(), acc1b.String()) @@ -348,7 +353,7 @@ func TestCancelReplaceMarketOrder100Slippage(t *testing.T) { ) require.NoError(t, err) limitOrder := order(ctx.BlockTime(), acc1, slippageSource.String(), dest.String()) - _, err = k.NewOrderSingle(ctx, limitOrder) + err = k.NewOrderSingle(ctx, limitOrder) require.NoError(t, err) clientID := limitOrder.ClientOrderID @@ -394,7 +399,7 @@ func TestCancelReplaceMarketOrder100Slippage(t *testing.T) { ) require.NoError(t, err) - _, err = k.CancelReplaceLimitOrder(ctx, newOrder, clientID) + err = k.CancelReplaceLimitOrder(ctx, newOrder, clientID) require.NoError(t, err) expOrder := &types.Order{ ID: 3, @@ -465,16 +470,16 @@ func TestGetSrcFromSlippage(t *testing.T) { // Establish market price by executing a 1:1 trade o = order(ctx.BlockTime(), acc2, "1eur", "1gbp") - _, err = k.NewOrderSingle(ctx, o) + err = k.NewOrderSingle(ctx, o) require.NoError(t, err) o = order(ctx.BlockTime(), acc1, "1gbp", "1eur") - _, err = k.NewOrderSingle(ctx, o) + err = k.NewOrderSingle(ctx, o) require.NoError(t, err) // Add liquidity o = order(ctx.BlockTime(), acc2, "100eur", "100gbp") - _, err = k.NewOrderSingle(ctx, o) + err = k.NewOrderSingle(ctx, o) require.NoError(t, err) srcDenom = "gbp" @@ -529,25 +534,22 @@ func TestFillOrKillMarketOrder1(t *testing.T) { // Establish market price by executing a 1:1 trade o = order(ctx.BlockTime(), acc2, "1eur", "1gbp") - _, err = k.NewOrderSingle(ctx, o) + err = k.NewOrderSingle(ctx, o) require.NoError(t, err) o = order(ctx.BlockTime(), acc1, "1gbp", "1eur") - _, err = k.NewOrderSingle(ctx, o) + err = k.NewOrderSingle(ctx, o) require.NoError(t, err) // Create a market for eur o = order(ctx.BlockTime(), acc2, "100eur", "100gbp") - res, err := k.NewOrderSingle(ctx, o) + err = k.NewOrderSingle(ctx, o) require.NoError(t, err) - require.Equal( - t, "accept", - string(res.Events[0].Attributes[0].GetValue()), - ) + require.True(t, findEventAttr(ctx, "accept")) require.Equal( t, ctx.BlockTime().Format(time.RFC3339), - string(res.Events[0].Attributes[len(res.Events[0].Attributes)-1].GetValue()), + string(ctx.EventManager().Events()[0].Attributes[len(ctx.EventManager().Events()[0].Attributes)-1].GetValue()), ) // Create a fill or kill order that cannot be satisfied by the current market @@ -559,12 +561,10 @@ func TestFillOrKillMarketOrder1(t *testing.T) { require.NoError(t, err) limitOrder := order(ctx.BlockTime(), acc1, slippageSource.String(), dest.String()) limitOrder.TimeInForce = types.TimeInForce_FillOrKill - result, err := k.NewOrderSingle(ctx, limitOrder) + ctx = ctx.WithEventManager(sdk.NewEventManager()) + err = k.NewOrderSingle(ctx, limitOrder) require.NoError(t, err) - require.Len(t, result.Events, 1) - require.Equal(t, types.EventTypeMarket, result.Events[0].Type) - require.Equal(t, "action", string(result.Events[0].Attributes[0].GetKey())) - require.Equal(t, "expire", string(result.Events[0].Attributes[0].GetValue())) + require.True(t, findEventAttr(ctx, "expire")) // Last order must fail completely due to not being fillable acc1Bal := bk.GetAllBalances(ctx, acc1.GetAddress()) @@ -582,12 +582,12 @@ func TestFillOrKillLimitOrder1(t *testing.T) { // Create a tiny market for eur o := order(ctx.BlockTime(), acc2, "100eur", "100gbp") - _, err := k.NewOrderSingle(ctx, o) + err := k.NewOrderSingle(ctx, o) require.NoError(t, err) order2 := order(ctx.BlockTime(), acc1, "200gbp", "200eur") order2.TimeInForce = types.TimeInForce_FillOrKill - _, err = k.NewOrderSingle(ctx, order2) + err = k.NewOrderSingle(ctx, order2) require.NoError(t, err) // Order must fail completely due to not being fillable @@ -614,13 +614,13 @@ func TestImmediateOrCancel(t *testing.T) { var err error o = order(ctx.BlockTime(), acc2, "1eur", "1gbp") - _, err = k.NewOrderSingle(ctx, o) + err = k.NewOrderSingle(ctx, o) require.NoError(t, err) o = order(ctx.BlockTime(), acc1, "2gbp", "2eur") o.TimeInForce = types.TimeInForce_ImmediateOrCancel cid := o.ClientOrderID - _, err = k.NewOrderSingle(ctx, o) + err = k.NewOrderSingle(ctx, o) require.NoError(t, err) require.Equal(t, o.Created, ctx.BlockTime()) @@ -655,19 +655,19 @@ func TestMultipleOrders(t *testing.T) { totalSupply := snapshotAccounts(ctx, bk) // Add two orders that draw on the same balance. - _, err := k.NewOrderSingle(ctx, order(ctx.BlockTime(), acc1, "10000eur", "11000usd")) + err := k.NewOrderSingle(ctx, order(ctx.BlockTime(), acc1, "10000eur", "11000usd")) require.NoError(t, err) - _, err = k.NewOrderSingle(ctx, order(ctx.BlockTime(), acc1, "10000eur", "1400chf")) + err = k.NewOrderSingle(ctx, order(ctx.BlockTime(), acc1, "10000eur", "1400chf")) require.NoError(t, err) // require.Len(t, k.instruments, 2) - res, err := k.NewOrderSingle(ctx, order(ctx.BlockTime(), acc2, "7400usd", "5000eur")) - require.True(t, err == nil, res.Log) + err = k.NewOrderSingle(ctx, order(ctx.BlockTime(), acc2, "7400usd", "5000eur")) + require.NoError(t, err) - res, err = k.NewOrderSingle(ctx, order(ctx.BlockTime(), acc3, "2200chf", "5000eur")) - require.True(t, err == nil, res.Log) + err = k.NewOrderSingle(ctx, order(ctx.BlockTime(), acc3, "2200chf", "5000eur")) + require.NoError(t, err) // All acc1's EUR are sold by now. No orders should be on books orders := k.GetOrdersByOwner(ctx, acc1.GetAddress()) @@ -683,7 +683,7 @@ func TestCancelZeroRemainingOrders(t *testing.T) { ctx, k, ak, bk := createTestComponents(t) acc := createAccount(ctx, ak, bk, randomAddress(), "10000eur") - _, err := k.NewOrderSingle(ctx, order(ctx.BlockTime(), acc, "10000eur", "11000usd")) + err := k.NewOrderSingle(ctx, order(ctx.BlockTime(), acc, "10000eur", "11000usd")) require.NoError(t, err) err = bk.SendCoins(ctx, acc.GetAddress(), sdk.AccAddress([]byte("void")), coins("10000eur")) @@ -703,7 +703,7 @@ func TestInsufficientBalance1(t *testing.T) { totalSupply := snapshotAccounts(ctx, bk) o := order(ctx.BlockTime(), acc1, "300eur", "360usd") - _, err := k.NewOrderSingle(ctx, o) + err := k.NewOrderSingle(ctx, o) require.NoError(t, err) // Modify account balance to be below order source @@ -711,7 +711,7 @@ func TestInsufficientBalance1(t *testing.T) { require.NoError(t, err) o = order(ctx.BlockTime(), acc2, "360usd", "300eur") - _, err = k.NewOrderSingle(ctx, o) + err = k.NewOrderSingle(ctx, o) require.NoError(t, err) bal1 := bk.GetAllBalances(ctx, acc1.GetAddress()) @@ -731,11 +731,11 @@ func Test2(t *testing.T) { totalSupply := snapshotAccounts(ctx, bk) o := order(ctx.BlockTime(), acc1, "100eur", "120usd") - _, err := k.NewOrderSingle(ctx, o) + err := k.NewOrderSingle(ctx, o) require.NoError(t, err) o = order(ctx.BlockTime(), acc2, "121usd", "100eur") - _, err = k.NewOrderSingle(ctx, o) + err = k.NewOrderSingle(ctx, o) require.NoError(t, err) // require.Empty(t, k.instruments) @@ -753,17 +753,17 @@ func TestAllInstruments(t *testing.T) { acc3 := createAccount(ctx, ak, bk, randomAddress(), "2200chf") // Add two orders that draw on the same balance. - _, err := k.NewOrderSingle(ctx, order(ctx.BlockTime(), acc1, "10000eur", "11000usd")) + err := k.NewOrderSingle(ctx, order(ctx.BlockTime(), acc1, "10000eur", "11000usd")) require.NoError(t, err) - _, err = k.NewOrderSingle(ctx, order(ctx.BlockTime(), acc1, "10000eur", "1400chf")) + err = k.NewOrderSingle(ctx, order(ctx.BlockTime(), acc1, "10000eur", "1400chf")) require.NoError(t, err) - res, err := k.NewOrderSingle(ctx, order(ctx.BlockTime(), acc2, "7400usd", "5000eur")) - require.True(t, err == nil, res.Log) + err = k.NewOrderSingle(ctx, order(ctx.BlockTime(), acc2, "7400usd", "5000eur")) + require.NoError(t, err) - res, err = k.NewOrderSingle(ctx, order(ctx.BlockTime(), acc3, "2200chf", "5000eur")) - require.True(t, err == nil, res.Log) + err = k.NewOrderSingle(ctx, order(ctx.BlockTime(), acc3, "2200chf", "5000eur")) + require.NoError(t, err) // All acc1's EUR are sold by now. No orders should be on books orders := k.GetOrdersByOwner(ctx, acc1.GetAddress()) @@ -832,11 +832,11 @@ func TestDeleteOrder(t *testing.T) { cid := cid() order1, _ := types.NewOrder(ctx.BlockTime(), types.TimeInForce_GoodTillCancel, coin("100eur"), coin("120usd"), acc1.GetAddress(), cid) - _, err := k.NewOrderSingle(ctx, order1) + err := k.NewOrderSingle(ctx, order1) require.NoError(t, err) order2, _ := types.NewOrder(ctx.BlockTime(), types.TimeInForce_GoodTillCancel, coin("100eur"), coin("77chf"), acc1.GetAddress(), cid) - _, err = k.NewOrderSingle(ctx, order2) + err = k.NewOrderSingle(ctx, order2) require.Error(t, err) // Verify that client order ids cannot be duplicated. // require.Len(t, k.instruments, 1) // Ensure that the eur->chf pair was not added. @@ -854,14 +854,14 @@ func TestGetOrdersByOwnerAndCancel(t *testing.T) { for i := 0; i < 5; i++ { order, _ := types.NewOrder(ctx.BlockTime(), types.TimeInForce_GoodTillCancel, coin("5eur"), coin("12usd"), acc1.GetAddress(), cid()) - _, err := k.NewOrderSingle(ctx, order) + err := k.NewOrderSingle(ctx, order) require.NoError(t, err) } for i := 0; i < 5; i++ { order, _ := types.NewOrder(ctx.BlockTime(), types.TimeInForce_GoodTillCancel, coin("7usd"), coin("3chf"), acc2.GetAddress(), cid()) - res, err := k.NewOrderSingle(ctx, order) - require.True(t, err == nil, res.Log) + err := k.NewOrderSingle(ctx, order) + require.NoError(t, err) } allOrders1 := k.GetOrdersByOwner(ctx, acc1.GetAddress()) @@ -869,8 +869,8 @@ func TestGetOrdersByOwnerAndCancel(t *testing.T) { { order, _ := types.NewOrder(ctx.BlockTime(), types.TimeInForce_GoodTillCancel, coin("12usd"), coin("5eur"), acc2.GetAddress(), cid()) - res, err := k.NewOrderSingle(ctx, order) - require.True(t, err == nil, res.Log) + err := k.NewOrderSingle(ctx, order) + require.NoError(t, err) } allOrders2 := k.GetOrdersByOwner(ctx, acc1.GetAddress()) @@ -878,10 +878,10 @@ func TestGetOrdersByOwnerAndCancel(t *testing.T) { cid := allOrders2[2].ClientOrderID gasMeter := sdk.NewGasMeter(math.MaxUint64) - _, err := k.CancelOrder(ctx.WithGasMeter(gasMeter), acc1.GetAddress(), cid) + err := k.CancelOrder(ctx.WithGasMeter(gasMeter), acc1.GetAddress(), cid) require.NoError(t, err) - _, err = k.CancelOrder(ctx.WithGasMeter(gasMeter), acc1.GetAddress(), cid) + err = k.CancelOrder(ctx.WithGasMeter(gasMeter), acc1.GetAddress(), cid) require.Error(t, err) require.Equal(t, 2*gasPriceCancelOrder, gasMeter.GasConsumed()) @@ -889,12 +889,7 @@ func TestGetOrdersByOwnerAndCancel(t *testing.T) { allOrders3 := k.GetOrdersByOwner(ctx, acc1.GetAddress()) require.Len(t, allOrders3, 3) - found := false - for _, e := range ctx.EventManager().ABCIEvents() { - found = found || (e.Type == types.EventTypeMarket && string(e.Attributes[0].GetValue()) == "expire") - } - - require.True(t, found) + require.True(t, findEventAttr(ctx, "expire")) } func TestCancelOrders1(t *testing.T) { @@ -902,7 +897,7 @@ func TestCancelOrders1(t *testing.T) { ctx, k, ak, bk := createTestComponents(t) acc := createAccount(ctx, ak, bk, randomAddress(), "100eur") - _, err := k.CancelOrder(ctx, acc.GetAddress(), "abcde") + err := k.CancelOrder(ctx, acc.GetAddress(), "abcde") require.Error(t, err) } @@ -915,14 +910,14 @@ func TestKeeperCancelReplaceLimitOrder(t *testing.T) { order1cid := cid() order1, _ := types.NewOrder(ctx.BlockTime(), types.TimeInForce_GoodTillCancel, coin("500eur"), coin("1200usd"), acc1.GetAddress(), order1cid) - _, err := k.NewOrderSingle(ctx, order1) + err := k.NewOrderSingle(ctx, order1) require.NoError(t, err) gasMeter := sdk.NewGasMeter(math.MaxUint64) order2cid := cid() order2, _ := types.NewOrder(ctx.BlockTime(), types.TimeInForce_GoodTillCancel, coin("5000eur"), coin("17000usd"), acc1.GetAddress(), order2cid) - res, err := k.CancelReplaceLimitOrder(ctx.WithGasMeter(gasMeter), order2, order1cid) - require.True(t, err == nil, res.Log) + err = k.CancelReplaceLimitOrder(ctx.WithGasMeter(gasMeter), order2, order1cid) + require.NoError(t, err) require.Equal(t, gasPriceCancelReplaceOrder, gasMeter.GasConsumed()) { @@ -936,17 +931,17 @@ func TestKeeperCancelReplaceLimitOrder(t *testing.T) { order3, _ := types.NewOrder(ctx.BlockTime(), types.TimeInForce_GoodTillCancel, coin("500chf"), coin("1700usd"), acc1.GetAddress(), cid()) // Wrong client order id for previous order submitted. - _, err = k.CancelReplaceLimitOrder(ctx, order3, order1cid) + err = k.CancelReplaceLimitOrder(ctx, order3, order1cid) require.True(t, types.ErrClientOrderIdNotFound.Is(err)) // Changing instrument of order gasMeter = sdk.NewGasMeter(math.MaxUint64) - _, err = k.CancelReplaceLimitOrder(ctx.WithGasMeter(gasMeter), order3, order2cid) + err = k.CancelReplaceLimitOrder(ctx.WithGasMeter(gasMeter), order3, order2cid) require.True(t, types.ErrOrderInstrumentChanged.Is(err)) require.Equal(t, gasPriceCancelReplaceOrder, gasMeter.GasConsumed()) o := order(ctx.BlockTime(), acc2, "2600usd", "300eur") - _, err = k.NewOrderSingle(ctx, o) + err = k.NewOrderSingle(ctx, o) require.NoError(t, err) bal1 := bk.GetAllBalances(ctx, acc1.GetAddress()) @@ -965,8 +960,8 @@ func TestKeeperCancelReplaceLimitOrder(t *testing.T) { // CancelReplace and verify that previously filled amount is subtracted from the resulting order order4cid := cid() order4, _ := types.NewOrder(ctx.BlockTime(), types.TimeInForce_GoodTillCancel, coin("10000eur"), coin("35050usd"), acc1.GetAddress(), order4cid) - res, err = k.CancelReplaceLimitOrder(ctx, order4, order2cid) - require.True(t, err == nil, res.Log) + err = k.CancelReplaceLimitOrder(ctx, order4, order2cid) + require.NoError(t, err) { orders := k.GetOrdersByOwner(ctx, acc1.GetAddress()) @@ -979,11 +974,11 @@ func TestKeeperCancelReplaceLimitOrder(t *testing.T) { // CancelReplace with an order that asks for a larger source than the replaced order has remaining order5 := order(ctx.BlockTime(), acc2, "42000usd", "8000eur") - k.NewOrderSingle(ctx, order5) - require.True(t, err == nil, res.Log) + err = k.NewOrderSingle(ctx, order5) + require.NoError(t, err) order6 := order(ctx.BlockTime(), acc1, "8000eur", "30000usd") - _, err = k.CancelReplaceLimitOrder(ctx, order6, order4cid) + err = k.CancelReplaceLimitOrder(ctx, order6, order4cid) require.True(t, types.ErrNoSourceRemaining.Is(err)) require.True(t, totalSupply.Sub(snapshotAccounts(ctx, bk)).IsZero()) @@ -998,14 +993,14 @@ func TestKeeperCancelReplaceMarketOrder(t *testing.T) { order1cid := cid() order1, _ := types.NewOrder(ctx.BlockTime(), types.TimeInForce_GoodTillCancel, coin("500eur"), coin("1200usd"), acc1.GetAddress(), order1cid) - _, err := k.NewOrderSingle(ctx, order1) + err := k.NewOrderSingle(ctx, order1) require.NoError(t, err) gasMeter := sdk.NewGasMeter(math.MaxUint64) order2cid := cid() order2, _ := types.NewOrder(ctx.BlockTime(), types.TimeInForce_GoodTillCancel, coin("5000eur"), coin("17000usd"), acc1.GetAddress(), order2cid) - res, err := k.CancelReplaceLimitOrder(ctx.WithGasMeter(gasMeter), order2, order1cid) - require.True(t, err == nil, res.Log) + err = k.CancelReplaceLimitOrder(ctx.WithGasMeter(gasMeter), order2, order1cid) + require.NoError(t, err) require.Equal(t, gasPriceCancelReplaceOrder, gasMeter.GasConsumed()) { @@ -1019,17 +1014,17 @@ func TestKeeperCancelReplaceMarketOrder(t *testing.T) { order3, _ := types.NewOrder(ctx.BlockTime(), types.TimeInForce_GoodTillCancel, coin("500chf"), coin("1700usd"), acc1.GetAddress(), cid()) // Wrong client order id for previous order submitted. - _, err = k.CancelReplaceLimitOrder(ctx, order3, order1cid) + err = k.CancelReplaceLimitOrder(ctx, order3, order1cid) require.True(t, types.ErrClientOrderIdNotFound.Is(err)) // Changing instrument of order gasMeter = sdk.NewGasMeter(math.MaxUint64) - _, err = k.CancelReplaceLimitOrder(ctx.WithGasMeter(gasMeter), order3, order2cid) + err = k.CancelReplaceLimitOrder(ctx.WithGasMeter(gasMeter), order3, order2cid) require.True(t, types.ErrOrderInstrumentChanged.Is(err)) require.Equal(t, gasPriceCancelReplaceOrder, gasMeter.GasConsumed()) o := order(ctx.BlockTime(), acc2, "2600usd", "300eur") - _, err = k.NewOrderSingle(ctx, o) + err = k.NewOrderSingle(ctx, o) require.NoError(t, err) bal1 := bk.GetAllBalances(ctx, acc1.GetAddress()) @@ -1048,8 +1043,8 @@ func TestKeeperCancelReplaceMarketOrder(t *testing.T) { // CancelReplace and verify that previously filled amount is subtracted from the resulting order order4cid := cid() order4, _ := types.NewOrder(ctx.BlockTime(), types.TimeInForce_GoodTillCancel, coin("10000eur"), coin("35050usd"), acc1.GetAddress(), order4cid) - res, err = k.CancelReplaceLimitOrder(ctx, order4, order2cid) - require.True(t, err == nil, res.Log) + err = k.CancelReplaceLimitOrder(ctx, order4, order2cid) + require.NoError(t, err) { orders := k.GetOrdersByOwner(ctx, acc1.GetAddress()) @@ -1062,11 +1057,11 @@ func TestKeeperCancelReplaceMarketOrder(t *testing.T) { // CancelReplace with an order that asks for a larger source than the replaced order has remaining order5 := order(ctx.BlockTime(), acc2, "42000usd", "8000eur") - k.NewOrderSingle(ctx, order5) - require.True(t, err == nil, res.Log) + err = k.NewOrderSingle(ctx, order5) + require.NoError(t, err) order6 := order(ctx.BlockTime(), acc1, "8000eur", "30000usd") - _, err = k.CancelReplaceLimitOrder(ctx, order6, order4cid) + err = k.CancelReplaceLimitOrder(ctx, order6, order4cid) require.True(t, types.ErrNoSourceRemaining.Is(err)) require.True(t, totalSupply.Sub(snapshotAccounts(ctx, bk)).IsZero()) @@ -1078,14 +1073,14 @@ func TestOrdersChangeWithAccountBalance(t *testing.T) { acc2 := createAccount(ctx, ak, bk, randomAddress(), "11000chf,100000eur") order, _ := types.NewOrder(ctx.BlockTime(), types.TimeInForce_GoodTillCancel, coin("10000eur"), coin("1000usd"), acc.GetAddress(), cid()) - _, err := k.NewOrderSingle(ctx, order) + err := k.NewOrderSingle(ctx, order) require.NoError(t, err) { // Partially fill the order above acc2 := createAccount(ctx, ak, bk, randomAddress(), "900000usd") order2, _ := types.NewOrder(ctx.BlockTime(), types.TimeInForce_GoodTillCancel, coin("400usd"), coin("4000eur"), acc2.GetAddress(), cid()) - _, err = k.NewOrderSingle(ctx, order2) + err = k.NewOrderSingle(ctx, order2) require.NoError(t, err) } @@ -1128,7 +1123,7 @@ func TestUnknownAsset(t *testing.T) { // Make an order with a destination that is not known by the supply module o := order(ctx.BlockTime(), acc1, "1000eur", "1200nok") - _, err := k1.NewOrderSingle(ctx.WithGasMeter(gasMeter), o) + err := k1.NewOrderSingle(ctx.WithGasMeter(gasMeter), o) require.True(t, types.ErrUnknownAsset.Is(err)) require.Equal(t, gasPriceNewOrder, gasMeter.GasConsumed()) } @@ -1141,7 +1136,7 @@ func TestVestingAccount(t *testing.T) { vestingAcc := vestingtypes.NewDelayedVestingAccount(account.(*authtypes.BaseAccount), amount, math.MaxInt64) ak.SetAccount(ctx, vestingAcc) - _, err := keeper.NewOrderSingle(ctx, order(ctx.BlockTime(), vestingAcc, "5000eur", "4700chf")) + err := keeper.NewOrderSingle(ctx, order(ctx.BlockTime(), vestingAcc, "5000eur", "4700chf")) require.True(t, types.ErrAccountBalanceInsufficient.Is(err)) } @@ -1161,7 +1156,7 @@ func TestInvalidInstrument(t *testing.T) { TimeInForce: types.TimeInForce_GoodTillCancel, } - _, err := k.NewOrderSingle(ctx, o) + err := k.NewOrderSingle(ctx, o) require.True(t, types.ErrInvalidInstrument.Is(err)) } @@ -1174,20 +1169,20 @@ func TestSyntheticInstruments1(t *testing.T) { totalSupply := snapshotAccounts(ctx, bk) o := order(ctx.BlockTime(), acc1, "1000eur", "1114usd") - _, err := k.NewOrderSingle(ctx, o) + err := k.NewOrderSingle(ctx, o) require.NoError(t, err) o = order(ctx.BlockTime(), acc1, "500eur", "542chf") - _, err = k.NewOrderSingle(ctx, o) + err = k.NewOrderSingle(ctx, o) require.NoError(t, err) o = order(ctx.BlockTime(), acc3, "1000chf", "1028usd") - _, err = k.NewOrderSingle(ctx, o) + err = k.NewOrderSingle(ctx, o) require.NoError(t, err) gasMeter := sdk.NewGasMeter(math.MaxUint64) o = order(ctx.BlockTime(), acc2, "5000usd", "4485eur") - _, err = k.NewOrderSingle(ctx.WithGasMeter(gasMeter), o) + err = k.NewOrderSingle(ctx.WithGasMeter(gasMeter), o) require.NoError(t, err) require.Equal(t, gasMeter.GasConsumed(), gasPriceNewOrder) // Matches several orders, but should pay only the fixed fee @@ -1210,9 +1205,9 @@ func TestNonMatchingOrders(t *testing.T) { acc1 := createAccount(ctx, ak, bk, randomAddress(), "100000usd") acc2 := createAccount(ctx, ak, bk, randomAddress(), "240000eur") - _, err := k.NewOrderSingle(ctx, order(ctx.BlockTime(), acc1, "20000usd", "20000eur")) + err := k.NewOrderSingle(ctx, order(ctx.BlockTime(), acc1, "20000usd", "20000eur")) require.NoError(t, err) - _, err = k.NewOrderSingle(ctx, order(ctx.BlockTime(), acc2, "20000eur", "50000usd")) + err = k.NewOrderSingle(ctx, order(ctx.BlockTime(), acc2, "20000eur", "50000usd")) require.NoError(t, err) acc1Orders := k.GetOrdersByOwner(ctx, acc1.GetAddress()) @@ -1251,14 +1246,14 @@ func TestSyntheticInstruments2(t *testing.T) { } for _, o := range passiveOrders { - res, err := k.NewOrderSingle(ctx, o) - require.NoError(t, err, res.Log) + err := k.NewOrderSingle(ctx, o) + require.NoError(t, err) } gasMeter := sdk.NewGasMeter(math.MaxUint64) monsterOrder := order(ctx.BlockTime(), acc3, "3700000eur", "4000000usd") - res, err := k.NewOrderSingle(ctx.WithGasMeter(gasMeter), monsterOrder) - require.NoError(t, err, res.Log) + err := k.NewOrderSingle(ctx.WithGasMeter(gasMeter), monsterOrder) + require.NoError(t, err) require.Equal(t, gasPriceNewOrder, gasMeter.GasConsumed()) // require.Len(t, k.instruments, 0) @@ -1281,11 +1276,11 @@ func TestDestinationCapacity(t *testing.T) { order1.SourceFilled = sdk.NewInt(618000000) order1.DestinationFilled = sdk.NewInt(645161290) - _, err := k.NewOrderSingle(ctx, order1) + err := k.NewOrderSingle(ctx, order1) require.NoError(t, err) order2 := order(ctx.BlockTime(), acc2, "471096868463eur", "500182000000usd") - _, err = k.NewOrderSingle(ctx, order2) + err = k.NewOrderSingle(ctx, order2) require.NoError(t, err) } @@ -1303,15 +1298,15 @@ func TestDestinationCapacity2(t *testing.T) { order1.SourceFilled = sdk.NewInt(618000000) order1.DestinationFilled = sdk.NewInt(645161290) - _, err := k.NewOrderSingle(ctx, order1) + err := k.NewOrderSingle(ctx, order1) require.NoError(t, err) order2 := order(ctx.BlockTime(), acc3, "130000000000chf", "800000000usd") - _, err = k.NewOrderSingle(ctx, order2) + err = k.NewOrderSingle(ctx, order2) require.NoError(t, err) aggressiveOrder := order(ctx.BlockTime(), acc2, "471096868463eur", "120000000000chf") - _, err = k.NewOrderSingle(ctx, aggressiveOrder) + err = k.NewOrderSingle(ctx, aggressiveOrder) require.NoError(t, err) } @@ -1321,17 +1316,17 @@ func TestPreventPhantomLiquidity(t *testing.T) { acc1 := createAccount(ctx, ak, bk, randomAddress(), "10000eur") order1 := order(ctx.BlockTime(), acc1, "8000eur", "9000usd") - _, err := k.NewOrderSingle(ctx, order1) + err := k.NewOrderSingle(ctx, order1) require.NoError(t, err) // Cannot sell more than the balance in the same instrument order2 := order(ctx.BlockTime(), acc1, "8000eur", "9000usd") - _, err = k.NewOrderSingle(ctx, order2) + err = k.NewOrderSingle(ctx, order2) require.Error(t, err) // Can sell the balance in another instrument order3 := order(ctx.BlockTime(), acc1, "8000eur", "6000chf") - _, err = k.NewOrderSingle(ctx, order3) + err = k.NewOrderSingle(ctx, order3) require.NoError(t, err) } @@ -1360,7 +1355,7 @@ func TestListInstruments(t *testing.T) { ) order := order(ctx.BlockTime(), acc, s, d) - _, err := k.NewOrderSingle(ctx.WithGasMeter(gasmeter), order) + err := k.NewOrderSingle(ctx.WithGasMeter(gasmeter), order) require.NoError(t, err) } } @@ -1570,3 +1565,20 @@ func snapshotAccounts(ctx sdk.Context, bk bankkeeper.ViewKeeper) (totalBalance s func randomAddress() sdk.AccAddress { return tmrand.Bytes(sdk.AddrLen) } + +// findEventAttr find attr i.e. accept, fill, expire within an event attribute +// collection +func findEventAttr(ctx sdk.Context, eventAttr string) bool { + for _, event := range ctx.EventManager().ABCIEvents() { + if event.Type == "market" { + for _, evAttr := range event.Attributes { + if string(evAttr.Key) == "action" { + if string(evAttr.Value) == eventAttr { + return true + } + } + } + } + } + return false +} diff --git a/x/market/keeper/msg_server.go b/x/market/keeper/msg_server.go index e812dc6e..3b01b320 100644 --- a/x/market/keeper/msg_server.go +++ b/x/market/keeper/msg_server.go @@ -2,6 +2,7 @@ package keeper import ( "context" + sdk "github.com/cosmos/cosmos-sdk/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" "github.com/e-money/em-ledger/x/market/types" @@ -10,9 +11,9 @@ import ( var _ types.MsgServer = msgServer{} type marketKeeper interface { - NewOrderSingle(ctx sdk.Context, aggressiveOrder types.Order) (*sdk.Result, error) - CancelOrder(ctx sdk.Context, owner sdk.AccAddress, clientOrderId string) (*sdk.Result, error) - CancelReplaceLimitOrder(ctx sdk.Context, newOrder types.Order, origClientOrderId string) (*sdk.Result, error) + NewOrderSingle(ctx sdk.Context, aggressiveOrder types.Order) error + CancelOrder(ctx sdk.Context, owner sdk.AccAddress, clientOrderId string) error + CancelReplaceLimitOrder(ctx sdk.Context, newOrder types.Order, origClientOrderId string) error GetSrcFromSlippage(ctx sdk.Context, srcDenom string, dst sdk.Coin, maxSlippage sdk.Dec) (sdk.Coin, error) } type msgServer struct { @@ -35,14 +36,11 @@ func (m msgServer) AddLimitOrder(c context.Context, msg *types.MsgAddLimitOrder) return nil, err } - result, err := m.k.NewOrderSingle(ctx, order) + err = m.k.NewOrderSingle(ctx, order) if err != nil { return nil, err } - for _, e := range result.Events { - ctx.EventManager().EmitEvent(sdk.Event(e)) - } return &types.MsgAddLimitOrderResponse{}, nil } @@ -76,13 +74,11 @@ func (m msgServer) CancelOrder(c context.Context, msg *types.MsgCancelOrder) (*t return nil, sdkerrors.Wrap(sdkerrors.ErrInvalidAddress, "owner") } - result, err := m.k.CancelOrder(ctx, owner, msg.ClientOrderId) + err = m.k.CancelOrder(ctx, owner, msg.ClientOrderId) if err != nil { return nil, err } - for _, e := range result.Events { - ctx.EventManager().EmitEvent(sdk.Event(e)) - } + return &types.MsgCancelOrderResponse{}, nil } @@ -97,13 +93,11 @@ func (m msgServer) CancelReplaceLimitOrder(c context.Context, msg *types.MsgCanc return nil, err } - result, err := m.k.CancelReplaceLimitOrder(ctx, order, msg.OrigClientOrderId) + err = m.k.CancelReplaceLimitOrder(ctx, order, msg.OrigClientOrderId) if err != nil { return nil, err } - for _, e := range result.Events { - ctx.EventManager().EmitEvent(sdk.Event(e)) - } + return &types.MsgCancelReplaceLimitOrderResponse{}, nil } diff --git a/x/market/keeper/msg_server_test.go b/x/market/keeper/msg_server_test.go index 8e73c11b..913235b0 100644 --- a/x/market/keeper/msg_server_test.go +++ b/x/market/keeper/msg_server_test.go @@ -3,6 +3,8 @@ package keeper import ( "context" "errors" + "testing" + sdk "github.com/cosmos/cosmos-sdk/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" "github.com/e-money/em-ledger/x/market/types" @@ -10,7 +12,6 @@ import ( "github.com/stretchr/testify/require" abcitypes "github.com/tendermint/tendermint/abci/types" "github.com/tendermint/tendermint/libs/rand" - "testing" ) func TestAddLimitOrder(t *testing.T) { @@ -24,7 +25,7 @@ func TestAddLimitOrder(t *testing.T) { specs := map[string]struct { req *types.MsgAddLimitOrder - mockFn func(ctx sdk.Context, aggressiveOrder types.Order) (*sdk.Result, error) + mockFn func(ctx sdk.Context, aggressiveOrder types.Order) error expErr bool expEvents sdk.Events expOrder types.Order @@ -37,14 +38,15 @@ func TestAddLimitOrder(t *testing.T) { Source: sdk.Coin{Denom: "eeur", Amount: sdk.OneInt()}, Destination: sdk.Coin{Denom: "alx", Amount: sdk.OneInt()}, }, - mockFn: func(ctx sdk.Context, aggressiveOrder types.Order) (*sdk.Result, error) { + mockFn: func(ctx sdk.Context, aggressiveOrder types.Order) error { gotOrder = aggressiveOrder - return &sdk.Result{ - Events: []abcitypes.Event{{ + ctx.EventManager().EmitEvents([]sdk.Event{ + { Type: "testing", Attributes: []abcitypes.EventAttribute{{Key: []byte("foo"), Value: []byte("bar")}}, - }}, - }, nil + }, + }) + return nil }, expEvents: sdk.Events{{ Type: "testing", @@ -88,8 +90,8 @@ func TestAddLimitOrder(t *testing.T) { Source: sdk.Coin{Denom: "eeur", Amount: sdk.OneInt()}, Destination: sdk.Coin{Denom: "alx", Amount: sdk.OneInt()}, }, - mockFn: func(ctx sdk.Context, aggressiveOrder types.Order) (*sdk.Result, error) { - return nil, errors.New("testing") + mockFn: func(ctx sdk.Context, aggressiveOrder types.Order) error { + return errors.New("testing") }, expErr: true, }, @@ -113,11 +115,11 @@ func TestAddLimitOrder(t *testing.T) { func TestAddMarketOrder(t *testing.T) { var ( - ownerAddr = randomAccAddress() - gotSrc sdk.Coin - gotDst sdk.Coin - gotMaxSlippage sdk.Dec - gotOrder types.Order + ownerAddr = randomAccAddress() + gotSrc sdk.Coin + gotDst sdk.Coin + gotMaxSlippage sdk.Dec + gotOrder types.Order ) keeper := marketKeeperMock{} @@ -125,7 +127,7 @@ func TestAddMarketOrder(t *testing.T) { specs := map[string]struct { req *types.MsgAddMarketOrder - mockAddLimitOrderFn func(ctx sdk.Context, aggressiveOrder types.Order) (*sdk.Result, error) + mockAddLimitOrderFn func(ctx sdk.Context, aggressiveOrder types.Order) error mockGetSrcFromSlippageFn func(ctx sdk.Context, srcDenom string, dst sdk.Coin, maxSlippage sdk.Dec) (sdk.Coin, error) expErr bool expSrc sdk.Coin @@ -146,14 +148,15 @@ func TestAddMarketOrder(t *testing.T) { gotDst, gotMaxSlippage = dst, maxSlippage return gotSrc, nil }, - mockAddLimitOrderFn: func(ctx sdk.Context, aggressiveOrder types.Order) (*sdk.Result, error) { + mockAddLimitOrderFn: func(ctx sdk.Context, aggressiveOrder types.Order) error { gotOrder = aggressiveOrder - return &sdk.Result{ - Events: []abcitypes.Event{{ + ctx.EventManager().EmitEvents([]sdk.Event{ + { Type: "testing", Attributes: []abcitypes.EventAttribute{{Key: []byte("foo"), Value: []byte("bar")}}, - }}, - }, nil + }, + }) + return nil }, expEvents: sdk.Events{{ Type: "testing", @@ -228,8 +231,8 @@ func TestAddMarketOrder(t *testing.T) { gotSrc = sdk.NewCoin(srcDenom, sdk.OneInt()) return gotSrc, nil }, - mockAddLimitOrderFn: func(ctx sdk.Context, aggressiveOrder types.Order) (*sdk.Result, error) { - return nil, errors.New("testing") + mockAddLimitOrderFn: func(ctx sdk.Context, aggressiveOrder types.Order) error { + return errors.New("testing") }, expErr: true, }, @@ -266,7 +269,7 @@ func TestCancelOrder(t *testing.T) { specs := map[string]struct { req *types.MsgCancelOrder - mockFn func(ctx sdk.Context, owner sdk.AccAddress, clientOrderId string) (*sdk.Result, error) + mockFn func(ctx sdk.Context, owner sdk.AccAddress, clientOrderId string) error expErr bool expEvents sdk.Events }{ @@ -275,14 +278,15 @@ func TestCancelOrder(t *testing.T) { Owner: ownerAddr.String(), ClientOrderId: "myClientIOrderID", }, - mockFn: func(ctx sdk.Context, owner sdk.AccAddress, clientOrderId string) (*sdk.Result, error) { + mockFn: func(ctx sdk.Context, owner sdk.AccAddress, clientOrderId string) error { gotOwner, gotClientOrderId = owner, clientOrderId - return &sdk.Result{ - Events: []abcitypes.Event{{ + ctx.EventManager().EmitEvents([]sdk.Event{ + { Type: "testing", Attributes: []abcitypes.EventAttribute{{Key: []byte("foo"), Value: []byte("bar")}}, - }}, - }, nil + }, + }) + return nil }, expEvents: sdk.Events{{ Type: "testing", @@ -307,8 +311,8 @@ func TestCancelOrder(t *testing.T) { Owner: ownerAddr.String(), ClientOrderId: "myClientIOrderID", }, - mockFn: func(ctx sdk.Context, owner sdk.AccAddress, clientOrderId string) (*sdk.Result, error) { - return nil, errors.New("testing") + mockFn: func(ctx sdk.Context, owner sdk.AccAddress, clientOrderId string) error { + return errors.New("testing") }, expErr: true, }, @@ -342,7 +346,7 @@ func TestCancelReplaceLimitOrder(t *testing.T) { specs := map[string]struct { req *types.MsgCancelReplaceLimitOrder - mockFn func(ctx sdk.Context, newOrder types.Order, origClientOrderId string) (*sdk.Result, error) + mockFn func(ctx sdk.Context, newOrder types.Order, origClientOrderId string) error expErr bool expEvents sdk.Events expOrder types.Order @@ -356,14 +360,15 @@ func TestCancelReplaceLimitOrder(t *testing.T) { Source: sdk.Coin{Denom: "eeur", Amount: sdk.OneInt()}, Destination: sdk.Coin{Denom: "alx", Amount: sdk.OneInt()}, }, - mockFn: func(ctx sdk.Context, newOrder types.Order, origClientOrderId string) (*sdk.Result, error) { + mockFn: func(ctx sdk.Context, newOrder types.Order, origClientOrderId string) error { gotOrder, gotOrigClientOrderId = newOrder, origClientOrderId - return &sdk.Result{ - Events: []abcitypes.Event{{ + ctx.EventManager().EmitEvents([]sdk.Event{ + { Type: "testing", Attributes: []abcitypes.EventAttribute{{Key: []byte("foo"), Value: []byte("bar")}}, - }}, - }, nil + }, + }) + return nil }, expEvents: sdk.Events{{ Type: "testing", @@ -418,8 +423,8 @@ func TestCancelReplaceLimitOrder(t *testing.T) { Source: sdk.Coin{Denom: "eeur", Amount: sdk.OneInt()}, Destination: sdk.Coin{Denom: "alx", Amount: sdk.OneInt()}, }, - mockFn: func(ctx sdk.Context, newOrder types.Order, origClientOrderId string) (*sdk.Result, error) { - return nil, errors.New("testing") + mockFn: func(ctx sdk.Context, newOrder types.Order, origClientOrderId string) error { + return errors.New("testing") }, expErr: true, }, @@ -456,7 +461,7 @@ func TestCancelReplaceMarketOrder(t *testing.T) { specs := map[string]struct { req *types.MsgCancelReplaceMarketOrder mockGetSrcFromSlippageFn func(ctx sdk.Context, srcDenom string, dst sdk.Coin, maxSlippage sdk.Dec) (sdk.Coin, error) - mockCancelReplaceLimitOrderFn func(ctx sdk.Context, newOrder types.Order, origClientOrderId string) (*sdk.Result, error) + mockCancelReplaceLimitOrderFn func(ctx sdk.Context, newOrder types.Order, origClientOrderId string) error expErr bool expEvents sdk.Events expSrc sdk.Coin @@ -476,14 +481,15 @@ func TestCancelReplaceMarketOrder(t *testing.T) { gotSrc = sdk.NewCoin(srcDenom, sdk.OneInt()) return gotSrc, nil }, - mockCancelReplaceLimitOrderFn: func(ctx sdk.Context, newOrder types.Order, origClientOrderId string) (*sdk.Result, error) { + mockCancelReplaceLimitOrderFn: func(ctx sdk.Context, newOrder types.Order, origClientOrderId string) error { gotOrder, gotOrigClientOrderId = newOrder, origClientOrderId - return &sdk.Result{ - Events: []abcitypes.Event{{ + ctx.EventManager().EmitEvents([]sdk.Event{ + { Type: "testing", Attributes: []abcitypes.EventAttribute{{Key: []byte("foo"), Value: []byte("bar")}}, - }}, - }, nil + }, + }) + return nil }, expEvents: sdk.Events{{ Type: "testing", @@ -594,35 +600,35 @@ func TestCancelReplaceMarketOrder(t *testing.T) { } type marketKeeperMock struct { - NewMarketOrderWithSlippageFn func(ctx sdk.Context, srcDenom string, dst sdk.Coin, maxSlippage sdk.Dec, owner sdk.AccAddress, timeInForce types.TimeInForce, clientOrderId string) (*sdk.Result, error) - NewOrderSingleFn func(ctx sdk.Context, aggressiveOrder types.Order) (*sdk.Result, error) - CancelOrderFn func(ctx sdk.Context, owner sdk.AccAddress, clientOrderId string) (*sdk.Result, error) - CancelReplaceLimitOrderFn func(ctx sdk.Context, newOrder types.Order, origClientOrderId string) (*sdk.Result, error) + NewMarketOrderWithSlippageFn func(ctx sdk.Context, srcDenom string, dst sdk.Coin, maxSlippage sdk.Dec, owner sdk.AccAddress, timeInForce types.TimeInForce, clientOrderId string) error + NewOrderSingleFn func(ctx sdk.Context, aggressiveOrder types.Order) error + CancelOrderFn func(ctx sdk.Context, owner sdk.AccAddress, clientOrderId string) error + CancelReplaceLimitOrderFn func(ctx sdk.Context, newOrder types.Order, origClientOrderId string) error GetSrcFromSlippageFn func(ctx sdk.Context, srcDenom string, dst sdk.Coin, maxSlippage sdk.Dec) (sdk.Coin, error) } -func (m marketKeeperMock) NewMarketOrderWithSlippage(ctx sdk.Context, srcDenom string, dst sdk.Coin, maxSlippage sdk.Dec, owner sdk.AccAddress, timeInForce types.TimeInForce, clientOrderId string) (*sdk.Result, error) { +func (m marketKeeperMock) NewMarketOrderWithSlippage(ctx sdk.Context, srcDenom string, dst sdk.Coin, maxSlippage sdk.Dec, owner sdk.AccAddress, timeInForce types.TimeInForce, clientOrderId string) error { if m.NewMarketOrderWithSlippageFn == nil { panic("not expected to be called") } return m.NewMarketOrderWithSlippageFn(ctx, srcDenom, dst, maxSlippage, owner, timeInForce, clientOrderId) } -func (m marketKeeperMock) NewOrderSingle(ctx sdk.Context, aggressiveOrder types.Order) (*sdk.Result, error) { +func (m marketKeeperMock) NewOrderSingle(ctx sdk.Context, aggressiveOrder types.Order) error { if m.NewOrderSingleFn == nil { panic("not expected to be called") } return m.NewOrderSingleFn(ctx, aggressiveOrder) } -func (m marketKeeperMock) CancelOrder(ctx sdk.Context, owner sdk.AccAddress, clientOrderId string) (*sdk.Result, error) { +func (m marketKeeperMock) CancelOrder(ctx sdk.Context, owner sdk.AccAddress, clientOrderId string) error { if m.CancelOrderFn == nil { panic("not expected to be called") } return m.CancelOrderFn(ctx, owner, clientOrderId) } -func (m marketKeeperMock) CancelReplaceLimitOrder(ctx sdk.Context, newOrder types.Order, origClientOrderId string) (*sdk.Result, error) { +func (m marketKeeperMock) CancelReplaceLimitOrder(ctx sdk.Context, newOrder types.Order, origClientOrderId string) error { if m.CancelReplaceLimitOrderFn == nil { panic("not expected to be called") } diff --git a/x/market/keeper/querier_test.go b/x/market/keeper/querier_test.go index b68316ce..2e74b9a3 100644 --- a/x/market/keeper/querier_test.go +++ b/x/market/keeper/querier_test.go @@ -5,11 +5,12 @@ package keeper import ( - "github.com/e-money/em-ledger/x/market/types" "strings" "testing" "time" + "github.com/e-money/em-ledger/x/market/types" + json2 "encoding/json" "github.com/stretchr/testify/require" @@ -25,23 +26,23 @@ func TestQryGetAllInstrumentsWithNonZeroBestPrices(t *testing.T) { // generate passive order o := order(ctx.BlockTime(), acc1, "100eur", "120usd") - _, err := k.NewOrderSingle(ctx, o) + err := k.NewOrderSingle(ctx, o) require.NoError(t, err) // generate passive order o = order(ctx.BlockTime(), acc1, "72eur", "1213jpy") - _, err = k.NewOrderSingle(ctx, o) + err = k.NewOrderSingle(ctx, o) require.NoError(t, err) // generate passive order of half balances o = order(ctx.BlockTime(), acc1, "72chf", "312usd") - _, err = k.NewOrderSingle(ctx, o) + err = k.NewOrderSingle(ctx, o) require.NoError(t, err) // Execute an order // fulfilled o = order(ctx.BlockTime(), acc2, "156usd", "36chf") - _, err = k.NewOrderSingle(ctx, o) + err = k.NewOrderSingle(ctx, o) require.NoError(t, err) { @@ -81,17 +82,17 @@ func TestQryGetAllInstrumentsWithNilBestPrices(t *testing.T) { acc3 := createAccount(ctx, ak, bk, randomAddress(), "2200chf") // generate passive order - _, err := k.NewOrderSingle(ctx, order(ctx.BlockTime(), acc1, "10000eur", "11000usd")) + err := k.NewOrderSingle(ctx, order(ctx.BlockTime(), acc1, "10000eur", "11000usd")) require.NoError(t, err) - _, err = k.NewOrderSingle(ctx, order(ctx.BlockTime(), acc1, "10000eur", "1400chf")) + err = k.NewOrderSingle(ctx, order(ctx.BlockTime(), acc1, "10000eur", "1400chf")) require.NoError(t, err) - res, err := k.NewOrderSingle(ctx, order(ctx.BlockTime(), acc2, "7400usd", "5000eur")) - require.True(t, err == nil, res.Log) + err = k.NewOrderSingle(ctx, order(ctx.BlockTime(), acc2, "7400usd", "5000eur")) + require.NoError(t, err) - res, err = k.NewOrderSingle(ctx, order(ctx.BlockTime(), acc3, "2200chf", "5000eur")) - require.True(t, err == nil, res.Log) + err = k.NewOrderSingle(ctx, order(ctx.BlockTime(), acc3, "2200chf", "5000eur")) + require.NoError(t, err) // All acc1's EUR are sold by now. No orders should be on books orders := k.GetOrdersByOwner(ctx, acc1.GetAddress()) @@ -136,20 +137,20 @@ func TestQuerier1(t *testing.T) { acc2 := createAccount(ctx, ak, bk, randomAddress(), "1000usd") o := order(ctx.BlockTime(), acc1, "100eur", "120usd") - _, err := k.NewOrderSingle(ctx, o) + err := k.NewOrderSingle(ctx, o) require.NoError(t, err) o = order(ctx.BlockTime(), acc1, "72eur", "1213jpy") - _, err = k.NewOrderSingle(ctx, o) + err = k.NewOrderSingle(ctx, o) require.NoError(t, err) o = order(ctx.BlockTime(), acc1, "72chf", "312usd") - _, err = k.NewOrderSingle(ctx, o) + err = k.NewOrderSingle(ctx, o) require.NoError(t, err) // Execute an order o = order(ctx.BlockTime(), acc2, "156usd", "36chf") - _, err = k.NewOrderSingle(ctx, o) + err = k.NewOrderSingle(ctx, o) require.NoError(t, err) { diff --git a/x/market/types/errors.go b/x/market/types/errors.go index bb21b0a1..30e77a7e 100644 --- a/x/market/types/errors.go +++ b/x/market/types/errors.go @@ -10,17 +10,16 @@ import ( var ( ErrAccountBalanceInsufficient = sdkerrors.Register(ModuleName, 1, "insufficient account balance") - ErrAccountBalanceInsufficientForInstrument = sdkerrors.Register(ModuleName, 2, "") - ErrNonUniqueClientOrderId = sdkerrors.Register(ModuleName, 3, "") - ErrClientOrderIdNotFound = sdkerrors.Register(ModuleName, 4, "") - ErrOrderInstrumentChanged = sdkerrors.Register(ModuleName, 5, "") - ErrInvalidClientOrderId = sdkerrors.Register(ModuleName, 6, "") - ErrInvalidInstrument = sdkerrors.Register(ModuleName, 7, "") - ErrInvalidPrice = sdkerrors.Register(ModuleName, 8, "") - ErrNoSourceRemaining = sdkerrors.Register(ModuleName, 9, "") - ErrUnknownAsset = sdkerrors.Register(ModuleName, 10, "") - ErrUnknownOrderType = sdkerrors.Register(ModuleName, 11, "Unknown order type") - ErrUnknownTimeInForce = sdkerrors.Register(ModuleName, 12, "") - ErrNoMarketDataAvailable = sdkerrors.Register(ModuleName, 13, "No market data available for instrument") - ErrInvalidSlippage = sdkerrors.Register(ModuleName, 14, "Invalid slippage") + ErrAccountBalanceInsufficientForInstrument = sdkerrors.Register(ModuleName, 2, "an order exists with the same quantity of source, destination instruments") + ErrNonUniqueClientOrderId = sdkerrors.Register(ModuleName, 3, "the client order id is duplicate and has been used before") + ErrClientOrderIdNotFound = sdkerrors.Register(ModuleName, 4, "the client order cannot be found") + ErrOrderInstrumentChanged = sdkerrors.Register(ModuleName, 5, "cannot change the instrument from the original order") + ErrInvalidClientOrderId = sdkerrors.Register(ModuleName, 6, "the order id length is greater than 32") + ErrInvalidInstrument = sdkerrors.Register(ModuleName, 7, "source and destination instruments are the same") + ErrInvalidPrice = sdkerrors.Register(ModuleName, 8, "insufficient source instrument quantity to pay for 1 unit of destination instrument") + ErrNoSourceRemaining = sdkerrors.Register(ModuleName, 9, "the original order has spent the entire source instrument quantity") + ErrUnknownAsset = sdkerrors.Register(ModuleName, 10, "unknown destination instrument denomination") + ErrUnknownTimeInForce = sdkerrors.Register(ModuleName, 12, "unknown time in force value. Valid values are TimeInForce_GoodTillCancel, TimeInForce_FillOrKill, TimeInForce_ImmediateOrCancel") + ErrNoMarketDataAvailable = sdkerrors.Register(ModuleName, 13, "no market data available for instrument") + ErrInvalidSlippage = sdkerrors.Register(ModuleName, 14, "invalid slippage") )