Refactor market keeper against master #150
Conversation
* Add upgrade handler for lilmermaid-16 * Externalize inflation types, keeper packages * Group & sort imports Co-authored-by: Martin Dyring-Andersen <[email protected]> Co-authored-by: Mario Karagiorgas <[email protected]>
| } | ||
| } | ||
|
|
||
| Expect(foundAccept).To(BeTrue(), "did not find the accept event") |
There was a problem hiding this comment.
These checks appear to never be hit due to the return statements above.
There was a problem hiding this comment.
there's an extra boolean condition && ...
The flow falls to the expect statements when it did not find the fill when it should be and affirm the accept was found and the opposite case accept was there but the fill...
x/market/keeper/keeper.go
Outdated
| @@ -150,6 +152,8 @@ func (k *Keeper) NewOrderSingle(ctx sdk.Context, aggressiveOrder types.Order) (* | |||
| ctx, commitTrade := ctx.CacheContext() | |||
|
|
|||
| defer func() { | |||
| retEvManager.EmitEvents(ctx.EventManager().Events()) | |||
There was a problem hiding this comment.
Should this happen after the check to see whether the order should be killed?
There was a problem hiding this comment.
There's the expire event for kill orders and IMHO the event manager should be operable for all code return paths for negative events if needed.
There was a problem hiding this comment.
If it always needs to happen, it can be placed outside the deferred function.
There was a problem hiding this comment.
outside the defer would miss events when returning on errors
There was a problem hiding this comment.
The function is actually divided into two: Precondition checks, which may return errors, and "order accepted" which runs the rest of the function. After order accepted, there is no return statements until the end of the function.
There was a problem hiding this comment.
Moved it before return.
|
@Mergifyio update |
|
Command
|
* Rely on ctx's event manager * 136 inflation (#141) * Add upgrade handler for lilmermaid-16 * Externalize inflation types, keeper packages * Group & sort imports Co-authored-by: Martin Dyring-Andersen <[email protected]> Co-authored-by: Mario Karagiorgas <[email protected]> * Refactor: remove result from keeper methods * Add description to the order errors * Parse tx events and add market tx events checks * Update AddMarketOrder to return events * Add event check * Add event validation for all market orders * Simplify event validations * Change error wording * Move event manager before return, PR feedback Co-authored-by: Henrik Aasted Sørensen <[email protected]> Co-authored-by: Martin Dyring-Andersen <[email protected]> (cherry picked from commit feaf93a)
* Rely on ctx's event manager * 136 inflation (#141) * Add upgrade handler for lilmermaid-16 * Externalize inflation types, keeper packages * Group & sort imports Co-authored-by: Martin Dyring-Andersen <[email protected]> Co-authored-by: Mario Karagiorgas <[email protected]> * Refactor: remove result from keeper methods * Add description to the order errors * Parse tx events and add market tx events checks * Update AddMarketOrder to return events * Add event check * Add event validation for all market orders * Simplify event validations * Change error wording * Move event manager before return, PR feedback Co-authored-by: Henrik Aasted Sørensen <[email protected]> Co-authored-by: Martin Dyring-Andersen <[email protected]> (cherry picked from commit feaf93a) Co-authored-by: Mario Karagiorgas <[email protected]>
add-market,cancelreplaceorder type bdd tests.