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

* test(x/gamm): remove liquidity events * remove capacity pre-allocation * Update x/gamm/keeper/internal/events/emit_test.go Co-authored-by: Adam Tucker <adam@osmosis.team> * Update x/gamm/keeper/msg_server_test.go Co-authored-by: Adam Tucker <adam@osmosis.team> * Update x/gamm/keeper/pool_service_test.go Co-authored-by: Adam Tucker <adam@osmosis.team> * shareIn * update event comments * changelog * Update CHANGELOG.md Co-authored-by: Aleksandr Bezobchuk <alexanderbez@users.noreply.github.com> * add docs Co-authored-by: Adam Tucker <adam@osmosis.team> Co-authored-by: Aleksandr Bezobchuk <alexanderbez@users.noreply.github.com>
osmosis-labs · Aug 3, 2022 · 40f19a3 · 40f19a3
1 parent 4d50629
commit 40f19a3
Show file tree

Hide file tree

Showing 9 changed files with 333 additions and 156 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -86,6 +86,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 * [1698](https://github.com/osmosis-labs/osmosis/pull/1698) Register wasm snapshotter extension.
 * [1931](https://github.com/osmosis-labs/osmosis/pull/1931) Add explicit check for input denoms to `CalcJoinPoolShares`
 * [2011](https://github.com/osmosis-labs/osmosis/pull/2011) Fix bug in TokenFactory initGenesis, relating to denom creation fee param.
+* [2186](https://github.com/osmosis-labs/osmosis/pull/2186) Remove liquidity event that was emitted twice per message.
 
 ### Improvements
 * [#2214](https://github.com/osmosis-labs/osmosis/pull/2214) Speedup epoch distribution, superfluid component

diff --git a/app/apptesting/events.go b/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))
+}
diff --git a/x/gamm/README.md b/x/gamm/README.md
@@ -583,6 +583,17 @@ It consists of the following attributes:
 This event is emitted after `ExitPool` completes exiting
 the requested pool successfully.
 
+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`
 
 This event is emitted after `CreatePool` completes creating

diff --git a/x/gamm/keeper/internal/events/emit_test.go b/x/gamm/keeper/internal/events/emit_test.go
@@ -139,3 +139,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)
+		})
+	}
+}
diff --git a/x/gamm/keeper/msg_server.go b/x/gamm/keeper/msg_server.go
@@ -145,10 +145,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),

diff --git a/x/gamm/keeper/msg_server_test.go b/x/gamm/keeper/msg_server_test.go
@@ -12,6 +12,8 @@ import (
 
 const (
 	doesNotExistDenom = "nodenom"
+	// Max positive int64.
+	int64Max = int64(^uint64(0) >> 1)
 )
 
 func TestMsgServerTestSuite(t *testing.T) {
@@ -50,7 +52,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{
@@ -66,7 +68,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{
@@ -111,8 +113,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)
 		})
 	}
 }
@@ -121,8 +123,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
 	)
 
@@ -150,7 +151,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{
@@ -166,7 +167,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{
@@ -211,8 +212,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)
 		})
 	}
 }
@@ -221,8 +222,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
 	)
 
@@ -243,7 +243,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,
@@ -258,7 +258,6 @@ func (suite *KeeperTestSuite) TestJoinPool_Events() {
 			suite.Setup()
 			ctx := suite.Ctx
 
-			suite.PrepareBalancerPool()
 			suite.PrepareBalancerPool()
 
 			msgServer := keeper.NewMsgServerImpl(suite.App.GAMMKeeper)
@@ -279,20 +278,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.
+			joinPoolResponse, 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: joinPoolResponse.ShareOutAmount,
+				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))
 }