Refactor market keeper against master (#150)

* 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 <[email protected]>
Co-authored-by: Mario Karagiorgas <[email protected]>

* 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 <[email protected]>
Co-authored-by: Martin Dyring-Andersen <[email protected]>

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")
+	}
+}

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()

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)

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

@@ -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)
 }