[ECO-247] Add event emission unit tests, with uncovered cancel reasons

[alnoki]

Background

#321 and several associated follow up PRs added comprehensive event emissions for market events like PlaceLimitOrderEvent. At the time, however, event emissions could not verified using the Move unit testing framework, and several minor issues were uncovered through discussions/manual testing:

• Remove cancel event for limit order when all size fills, enable posts below min size #347
• [ECO-256] Correct equality comparator for internal cancel event logic #360

Aptos Labs recently enabled unit testing of events in Move via aptos-labs/aptos-core#9181, and as such this PR updates the Move unit testing suite to test event emissions.

Methodology

Before this PR, Move code was already tested to 100% coverage, and as such this PR simply adds event assert statements to the existing coverage suite where applicable. Notably, this includes all non-expected-failure tests listed under the doc comments for:

• market::match()
• market::place_limit_order()
• market::place_market_order(),
• market::swap_between_coinstores()
• market::swap_coins()
• market::swap_generic()

Auxiliary event scenarios are also validated for manual order size changes and manual order cancellations, as well as registry events.

Gaps addressed

The two issues listed under background are now tested for and would have been quickly caught if event testing had been available at the time of submission of #321. Two additional edge cases were discovered during testing for this PR, both of which are cancel reasons that only apply to swaps. All applicable cancel reasons for all scenarios (limit order, market order, swap, manual cancel, eviction, self match maker and taker cancel) are now tested. The cancel reason logic and associated documentation is included in this PR per:

• CANCEL_REASON_TOO_SMALL_TO_FILL_LOT: dd968e9
• CANCEL_REASON_VIOLATED_LIMIT_PRICE: a894fdb

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
econia	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Aug 1, 2023 0:57am

[elliottdehn]

See note about adding a helper to unify the assertions being made at each location, perhaps have it accept a structure that makes it very clear what's being asserted. I'd call it assert_events or something. We want to know for a fact that all events that were emitted have been asserted, and all events that weren't emitted were also asserted--and the way to do that would be a unifying helper that asserts all of the event streams.

[elliottdehn]

I would also advise you to write out above the assertion what's being expected, i.e:

- Expect Cancel event emitted to maker resulting from X.
- Expect no other events emitted to taker or maker resulting from Y, Z.

It's hard to review this in GitHub because you have to click several times to see what the test name is in order to figure out what SHOULD be asserted and then check that this is correct. Just a note though. I'll have to pull it down to review it anyway.

[alnoki]

I would also advise you to write out above the assertion what's being expected, i.e:

- Expect Cancel event emitted to maker resulting from X.
- Expect no other events emitted to taker or maker resulting from Y, Z.

It's hard to review this in GitHub because you have to click several times to see what the test name is in order to figure out what SHOULD be asserted and then check that this is correct. Just a note though. I'll have to pull it down to review it anyway.

This should be much easier to review in VS Code using the pull requests extension https://marketplace.visualstudio.com/items?itemName=GitHub.vscode-pull-request-github

That way you can open the same editor side by side and review the function badge name

[elliottdehn]

I feel comfortable approving this after reviewing most (75%) of the changes closely because for something to be wrong in the specific case of an assert statement two things would have to be true:

• You're asserting something as true that shouldn't be.
• The code is doing the wrong thing and satisfying the assertion.

I wasn't able to find any missing cases or incorrect assertions.