Skip to content

Commit

Permalink
test(x/gamm): remove liquidity events (#2186)
Browse files Browse the repository at this point in the history
* test(x/gamm): remove liquidity events

* remove capacity pre-allocation

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

Co-authored-by: Adam Tucker <[email protected]>

* Update x/gamm/keeper/msg_server_test.go

Co-authored-by: Adam Tucker <[email protected]>

* Update x/gamm/keeper/pool_service_test.go

Co-authored-by: Adam Tucker <[email protected]>

* shareIn

* update event comments

* changelog

* Update CHANGELOG.md

Co-authored-by: Aleksandr Bezobchuk <[email protected]>

* add docs

Co-authored-by: Adam Tucker <[email protected]>
Co-authored-by: Aleksandr Bezobchuk <[email protected]>
(cherry picked from commit 40f19a3)

# Conflicts:
#	CHANGELOG.md
#	x/gamm/keeper/pool_service_test.go
#	x/superfluid/keeper/unpool_test.go
  • Loading branch information
p0mvn authored and mergify[bot] committed Aug 3, 2022
1 parent 681ffae commit f74bc21
Show file tree
Hide file tree
Showing 9 changed files with 597 additions and 84 deletions.
12 changes: 12 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,18 @@ This release contains minor CLI bug fixes.
* [1759](https://github.com/osmosis-labs/osmosis/pull/1759) Fix pagination filter in incentives query.
* [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`
<<<<<<< HEAD
=======
* [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

### SDK Upgrades
* [#2245](https://github.com/osmosis-labs/osmosis/pull/2244) Upgrade SDK with the following major changes:
* Minimum deposit on proposer at submission time: https://github.com/osmosis-labs/cosmos-sdk/pull/296
>>>>>>> 40f19a38 (test(x/gamm): remove liquidity events (#2186))
## [v9.0.0 - Nitrogen](https://github.com/osmosis-labs/osmosis/releases/tag/v9.0.0)

Expand Down
17 changes: 17 additions & 0 deletions app/apptesting/events.go
Original file line number Diff line number Diff line change
@@ -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))
}
54 changes: 54 additions & 0 deletions x/gamm/keeper/internal/events/emit_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
}
}
4 changes: 0 additions & 4 deletions x/gamm/keeper/msg_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
114 changes: 89 additions & 25 deletions x/gamm/keeper/msg_server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import (

const (
doesNotExistDenom = "nodenom"
// Max positive int64.
int64Max = int64(^uint64(0) >> 1)
)

var _ = suite.TestingSuite(nil)
Expand Down Expand Up @@ -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{
Expand All @@ -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{
Expand Down Expand Up @@ -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)
})
}
}
Expand All @@ -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
)

Expand Down Expand Up @@ -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{
Expand All @@ -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{
Expand Down Expand Up @@ -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)
})
}
}
Expand All @@ -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
)

Expand All @@ -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,
Expand All @@ -254,7 +254,6 @@ func (suite *KeeperTestSuite) TestJoinPool_Events() {
suite.Setup()
ctx := suite.Ctx

suite.PrepareBalancerPool()
suite.PrepareBalancerPool()

msgServer := keeper.NewMsgServerImpl(suite.App.GAMMKeeper)
Expand All @@ -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.
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))
}
Loading

0 comments on commit f74bc21

Please sign in to comment.