osmosis icon indicating copy to clipboard operation
osmosis copied to clipboard

x/gamm: test that swap and LP events are emitted

Open p0mvn opened this issue 3 years ago • 3 comments

Background

Currently, our tests are missing assertions about events. Moreover, it seems that some of our events are missing based on #1939

Therefore, this issue is twofold:

  1. Investigate if / which events are missing and restore them
  2. Add assertions in tests that events are being emitted in the correct places

Acceptance Criteria

  • all events are in the right places
    • [x] swap events:
    • [x] pool joined
    • [x] pool exited
    • [ ] pool created
    • [ ] msg events
  • consider emitting all events from the message server
  • assertions in unit tests are added
  • all unit tests continue to pass

p0mvn avatar Jul 01 '22 18:07 p0mvn

I have some progress started here: https://github.com/osmosis-labs/osmosis/tree/roman/gamm-events

p0mvn avatar Jul 01 '22 18:07 p0mvn

@mattverse following up on your comment in one of the gamm event PRs. After doing some investigation, I think we should not move the logic for emitting swap and liquidity events to the message server.

The reason is that some gamm keeper methods might be called from other modules. For example, superfluid: https://github.com/osmosis-labs/osmosis/blob/4970f165f439ad43c8a188e056b4ad69f54aebb4/x/superfluid/keeper/unpool.go#L60

If we were emitting events in the message server, the remove liquidity event would not be emitted. Therefore, I think it makes sense to keep the locations as is and thoroughly test.

p0mvn avatar Jul 21 '22 21:07 p0mvn

good point! Agreed 😎

mattverse avatar Jul 22 '22 06:07 mattverse

Pretty sure this got completed!

ValarDragon avatar Jan 30 '23 13:01 ValarDragon