Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Aug 3, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
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.
_, 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))
}
4 changes: 2 additions & 2 deletions x/gamm/keeper/swap_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
11 changes: 11 additions & 0 deletions x/gamm/spec/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading