test(x/gamm): remove liquidity events (backport #2186)

CHANGELOG.md

@@ -40,6 +40,11 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## Unreleased
+
+#### Bug Fixes
+* [2291](https://github.com/osmosis-labs/osmosis/pull/2291) Remove liquidity event that was emitted twice per message.
+
 ## v11
 
 #### Improvements

app/apptesting/events.go

@@ -0,0 +1,17 @@
+package apptesting
+
+import sdk "github.com/cosmos/cosmos-sdk/types"
+
+// AssertEventEmitted asserts that ctx's event manager has emitted the given number of events
+// of the given type.
+func (s *KeeperTestHelper) AssertEventEmitted(ctx sdk.Context, eventTypeExpected string, numEventsExpected int) {
+	allEvents := ctx.EventManager().Events()
+	// filter out other events
+	actualEvents := make([]sdk.Event, 0)
+	for _, event := range allEvents {
+		if event.Type == eventTypeExpected {
+			actualEvents = append(actualEvents, event)
+		}
+	}
+	s.Equal(numEventsExpected, len(actualEvents))
+}

x/gamm/keeper/internal/events/emit_test.go

@@ -140,3 +140,57 @@ func (suite *GammEventsTestSuite) TestEmitAddLiquidityEvent() {
 		})
 	}
 }
+
+func (suite *GammEventsTestSuite) TestEmitRemoveLiquidityEvent() {
+	testcases := map[string]struct {
+		ctx             sdk.Context
+		testAccountAddr sdk.AccAddress
+		poolId          uint64
+		tokensOut       sdk.Coins
+	}{
+		"basic valid": {
+			ctx:             suite.CreateTestContext(),
+			testAccountAddr: sdk.AccAddress([]byte(addressString)),
+			poolId:          1,
+			tokensOut:       sdk.NewCoins(sdk.NewCoin(testDenomA, sdk.NewInt(1234))),
+		},
+		"context with no event manager": {
+			ctx: sdk.Context{},
+		},
+		"valid with multiple tokens out": {
+			ctx:             suite.CreateTestContext(),
+			testAccountAddr: sdk.AccAddress([]byte(addressString)),
+			poolId:          200,
+			tokensOut:       sdk.NewCoins(sdk.NewCoin(testDenomA, sdk.NewInt(12)), sdk.NewCoin(testDenomB, sdk.NewInt(99))),
+		},
+	}
+
+	for name, tc := range testcases {
+		suite.Run(name, func() {
+			expectedEvents := sdk.Events{
+				sdk.NewEvent(
+					types.TypeEvtPoolExited,
+					sdk.NewAttribute(sdk.AttributeKeyModule, types.AttributeValueCategory),
+					sdk.NewAttribute(sdk.AttributeKeySender, tc.testAccountAddr.String()),
+					sdk.NewAttribute(types.AttributeKeyPoolId, strconv.FormatUint(tc.poolId, 10)),
+					sdk.NewAttribute(types.AttributeKeyTokensOut, tc.tokensOut.String()),
+				),
+			}
+
+			hasNoEventManager := tc.ctx.EventManager() == nil
+
+			// System under test.
+			events.EmitRemoveLiquidityEvent(tc.ctx, tc.testAccountAddr, tc.poolId, tc.tokensOut)
+
+			// Assertions
+			if hasNoEventManager {
+				// If there is no event manager on context, this is a no-op.
+				return
+			}
+
+			eventManager := tc.ctx.EventManager()
+			actualEvents := eventManager.Events()
+			suite.Equal(expectedEvents, actualEvents)
+		})
+	}
+}

x/gamm/keeper/msg_server.go

@@ -138,10 +138,6 @@ func (server msgServer) ExitPool(goCtx context.Context, msg *types.MsgExitPool)
 	}
 
 	ctx.EventManager().EmitEvents(sdk.Events{
-		sdk.NewEvent(
-			types.TypeEvtPoolExited,
-			sdk.NewAttribute(types.AttributeKeyPoolId, strconv.FormatUint(msg.PoolId, 10)),
-		),
 		sdk.NewEvent(
 			sdk.EventTypeMessage,
 			sdk.NewAttribute(sdk.AttributeKeyModule, types.AttributeValueCategory),

x/gamm/keeper/msg_server_test.go

@@ -10,6 +10,8 @@ import (
 
 const (
 	doesNotExistDenom = "nodenom"
+	// Max positive int64.
+	int64Max = int64(^uint64(0) >> 1)
 )
 
 var _ = suite.TestingSuite(nil)
@@ -46,7 +48,7 @@ func (suite *KeeperTestSuite) TestSwapExactAmountIn_Events() {
 			tokenIn:               sdk.NewCoin("foo", sdk.NewInt(tokenIn)),
 			tokenOutMinAmount:     sdk.NewInt(tokenInMinAmount),
 			expectedSwapEvents:    1,
-			expectedMessageEvents: 3, // 1 gamm + 2 tendermint.
+			expectedMessageEvents: 3, // 1 gamm + 2 events emitted by other keeper methods.
 		},
 		"two hops": {
 			routes: []types.SwapAmountInRoute{
@@ -62,7 +64,7 @@ func (suite *KeeperTestSuite) TestSwapExactAmountIn_Events() {
 			tokenIn:               sdk.NewCoin("foo", sdk.NewInt(tokenIn)),
 			tokenOutMinAmount:     sdk.NewInt(tokenInMinAmount),
 			expectedSwapEvents:    2,
-			expectedMessageEvents: 5, // 1 gamm + 4 tendermint.
+			expectedMessageEvents: 5, // 1 gamm + 4 events emitted by other keeper methods.
 		},
 		"invalid - two hops, denom does not exist": {
 			routes: []types.SwapAmountInRoute{
@@ -107,8 +109,8 @@ func (suite *KeeperTestSuite) TestSwapExactAmountIn_Events() {
 				suite.NotNil(response)
 			}
 
-			assertEventEmitted(suite, ctx, types.TypeEvtTokenSwapped, tc.expectedSwapEvents)
-			assertEventEmitted(suite, ctx, sdk.EventTypeMessage, tc.expectedMessageEvents)
+			suite.AssertEventEmitted(ctx, types.TypeEvtTokenSwapped, tc.expectedSwapEvents)
+			suite.AssertEventEmitted(ctx, sdk.EventTypeMessage, tc.expectedMessageEvents)
 		})
 	}
 }
@@ -117,8 +119,7 @@ func (suite *KeeperTestSuite) TestSwapExactAmountIn_Events() {
 // when calling SwapExactAmountOut.
 func (suite *KeeperTestSuite) TestSwapExactAmountOut_Events() {
 	const (
-		// Max positive int64.
-		tokenInMaxAmount = int64(^uint64(0) >> 1)
+		tokenInMaxAmount = int64Max
 		tokenOut         = 5
 	)
 
@@ -146,7 +147,7 @@ func (suite *KeeperTestSuite) TestSwapExactAmountOut_Events() {
 			tokenOut:              sdk.NewCoin("foo", sdk.NewInt(tokenOut)),
 			tokenInMaxAmount:      sdk.NewInt(tokenInMaxAmount),
 			expectedSwapEvents:    1,
-			expectedMessageEvents: 3, // 1 gamm + 2 tendermint.
+			expectedMessageEvents: 3, // 1 gamm + 2 events emitted by other keeper methods.
 		},
 		"two hops": {
 			routes: []types.SwapAmountOutRoute{
@@ -162,7 +163,7 @@ func (suite *KeeperTestSuite) TestSwapExactAmountOut_Events() {
 			tokenOut:              sdk.NewCoin("foo", sdk.NewInt(tokenOut)),
 			tokenInMaxAmount:      sdk.NewInt(tokenInMaxAmount),
 			expectedSwapEvents:    2,
-			expectedMessageEvents: 5, // 1 gamm + 4 tendermint.
+			expectedMessageEvents: 5, // 1 gamm + 4 events emitted by other keeper methods.
 		},
 		"invalid - two hops, denom does not exist": {
 			routes: []types.SwapAmountOutRoute{
@@ -207,8 +208,8 @@ func (suite *KeeperTestSuite) TestSwapExactAmountOut_Events() {
 				suite.NotNil(response)
 			}
 
-			assertEventEmitted(suite, ctx, types.TypeEvtTokenSwapped, tc.expectedSwapEvents)
-			assertEventEmitted(suite, ctx, sdk.EventTypeMessage, tc.expectedMessageEvents)
+			suite.AssertEventEmitted(ctx, types.TypeEvtTokenSwapped, tc.expectedSwapEvents)
+			suite.AssertEventEmitted(ctx, sdk.EventTypeMessage, tc.expectedMessageEvents)
 		})
 	}
 }
@@ -217,8 +218,7 @@ func (suite *KeeperTestSuite) TestSwapExactAmountOut_Events() {
 // when calling JoinPool.
 func (suite *KeeperTestSuite) TestJoinPool_Events() {
 	const (
-		// Max positive int64.
-		tokenInMaxAmount = int64(^uint64(0) >> 1)
+		tokenInMaxAmount = int64Max
 		shareOut         = 110
 	)
 
@@ -239,7 +239,7 @@ func (suite *KeeperTestSuite) TestJoinPool_Events() {
 				sdk.NewCoin("baz", sdk.NewInt(tokenInMaxAmount)),
 			),
 			expectedAddLiquidityEvents: 1,
-			expectedMessageEvents:      3, // 1 gamm + 2 tendermint.
+			expectedMessageEvents:      3, // 1 gamm + 2 events emitted by other keeper methods.
 		},
 		"tokenInMaxs do not match all tokens in pool - invalid join": {
 			poolId:         1,
@@ -254,7 +254,6 @@ func (suite *KeeperTestSuite) TestJoinPool_Events() {
 			suite.Setup()
 			ctx := suite.Ctx
 
-			suite.PrepareBalancerPool()
 			suite.PrepareBalancerPool()
 
 			msgServer := keeper.NewMsgServerImpl(suite.App.GAMMKeeper)
@@ -275,20 +274,85 @@ func (suite *KeeperTestSuite) TestJoinPool_Events() {
 				suite.Require().NotNil(response)
 			}
 
-			assertEventEmitted(suite, ctx, types.TypeEvtPoolJoined, tc.expectedAddLiquidityEvents)
-			assertEventEmitted(suite, ctx, sdk.EventTypeMessage, tc.expectedMessageEvents)
+			suite.AssertEventEmitted(ctx, types.TypeEvtPoolJoined, tc.expectedAddLiquidityEvents)
+			suite.AssertEventEmitted(ctx, sdk.EventTypeMessage, tc.expectedMessageEvents)
 		})
 	}
 }
 
-func assertEventEmitted(suite *KeeperTestSuite, ctx sdk.Context, eventTypeExpected string, numEventsExpected int) {
-	allEvents := ctx.EventManager().Events()
-	// filter out other events
-	actualEvents := make([]sdk.Event, 0, 1)
-	for _, event := range allEvents {
-		if event.Type == eventTypeExpected {
-			actualEvents = append(actualEvents, event)
-		}
+// TestExitPool_Events tests that events are correctly emitted
+// when calling ExitPool.
+func (suite *KeeperTestSuite) TestExitPool_Events() {
+	const (
+		tokenOutMinAmount = 1
+		shareIn           = 110
+	)
+
+	testcases := map[string]struct {
+		poolId                        uint64
+		shareInAmount                 sdk.Int
+		tokenOutMins                  sdk.Coins
+		expectError                   bool
+		expectedRemoveLiquidityEvents int
+		expectedMessageEvents         int
+	}{
+		"successful exit": {
+			poolId:                        1,
+			shareInAmount:                 sdk.NewInt(shareIn),
+			tokenOutMins:                  sdk.NewCoins(),
+			expectedRemoveLiquidityEvents: 1,
+			expectedMessageEvents:         3, // 1 gamm + 2 events emitted by other keeper methods.
+		},
+		"invalid tokenOutMins": {
+			poolId:        1,
+			shareInAmount: sdk.NewInt(shareIn),
+			tokenOutMins:  sdk.NewCoins(sdk.NewCoin("foo", sdk.NewInt(tokenOutMinAmount))),
+			expectError:   true,
+		},
+	}
+
+	for name, tc := range testcases {
+		suite.Run(name, func() {
+			suite.Setup()
+			ctx := suite.Ctx
+
+			suite.PrepareBalancerPool()
+			msgServer := keeper.NewMsgServerImpl(suite.App.GAMMKeeper)
+
+			sender := suite.TestAccs[0].String()
+
+			// Pre-join pool to be able to ExitPool.
+			_, err := msgServer.JoinPool(sdk.WrapSDKContext(ctx), &types.MsgJoinPool{
+				Sender:         sender,
+				PoolId:         tc.poolId,
+				ShareOutAmount: sdk.NewInt(shareIn),
+				TokenInMaxs: sdk.NewCoins(
+					sdk.NewCoin("foo", sdk.NewInt(int64Max)),
+					sdk.NewCoin("bar", sdk.NewInt(int64Max)),
+					sdk.NewCoin("baz", sdk.NewInt(int64Max)),
+				),
+			})
+			suite.Require().NoError(err)
+
+			// Reset event counts to 0 by creating a new manager.
+			ctx = ctx.WithEventManager(sdk.NewEventManager())
+			suite.Require().Equal(0, len(ctx.EventManager().Events()))
+
+			// System under test.
+			response, err := msgServer.ExitPool(sdk.WrapSDKContext(ctx), &types.MsgExitPool{
+				Sender:        sender,
+				PoolId:        tc.poolId,
+				ShareInAmount: sdk.NewInt(shareIn),
+				TokenOutMins:  tc.tokenOutMins,
+			})
+
+			if !tc.expectError {
+				suite.Require().NoError(err)
+				suite.Require().NotNil(response)
+			}
+
+			suite.AssertEventEmitted(ctx, types.TypeEvtPoolExited, tc.expectedRemoveLiquidityEvents)
+			suite.AssertEventEmitted(ctx, sdk.EventTypeMessage, tc.expectedMessageEvents)
+		})
 	}
-	suite.Require().Equal(numEventsExpected, len(actualEvents))
 }

x/gamm/keeper/swap_test.go

@@ -99,7 +99,7 @@ func (suite *KeeperTestSuite) TestBalancerPoolSimpleSwapExactAmountIn() {
 				suite.NoError(err, "test: %v", test.name)
 				suite.True(tokenOutAmount.Equal(test.param.expectedTokenOut), "test: %v", test.name)
 
-				assertEventEmitted(suite, ctx, types.TypeEvtTokenSwapped, 1)
+				suite.AssertEventEmitted(ctx, types.TypeEvtTokenSwapped, 1)
 
 				spotPriceAfter, err := keeper.CalculateSpotPrice(ctx, poolId, test.param.tokenIn.Denom, test.param.tokenOutDenom)
 				suite.NoError(err, "test: %v", test.name)
@@ -205,7 +205,7 @@ func (suite *KeeperTestSuite) TestBalancerPoolSimpleSwapExactAmountOut() {
 					"test: %v\n expect_eq actual: %s, expected: %s",
 					test.name, tokenInAmount, test.param.expectedTokenInAmount)
 
-				assertEventEmitted(suite, ctx, types.TypeEvtTokenSwapped, 1)
+				suite.AssertEventEmitted(ctx, types.TypeEvtTokenSwapped, 1)
 
 				spotPriceAfter, err := keeper.CalculateSpotPrice(ctx, poolId, test.param.tokenInDenom, test.param.tokenOut.Denom)
 				suite.NoError(err, "test: %v", test.name)

x/gamm/spec/README.md

@@ -985,6 +985,17 @@ It consists of the following attributes:
 
 TBD
 
+It consists of the following attributes:
+
+* `sdk.AttributeKeyModule` - "module"
+  * The value is the module's name - "gamm".
+* `sdk.AttributeKeySender`
+  * The value is the address of the sender who created the swap message.
+* `types.AttributeKeyPoolId`
+  * The value is the pool id of the pool where swap occurs.
+* `types.AttributeKeyTokensOut`
+  * The value is the string representation of the tokens being swapped out.
+
 ### `types.TypeEvtPoolCreated`
 
 TBD