Skip to content

[R4R] #499 failed blocking should not be regarded as closed order#500

Merged
cryptom-dev merged 1 commit intodevelopfrom
issue499
Mar 20, 2019
Merged

[R4R] #499 failed blocking should not be regarded as closed order#500
cryptom-dev merged 1 commit intodevelopfrom
issue499

Conversation

@cryptom-dev
Copy link
Contributor

@cryptom-dev cryptom-dev commented Mar 17, 2019

Description

failed blocking should not be regarded as closed order

Rationale

#499

Example

N/A

Changes

  1. don't delete order in dexkeeper.orderInfoForPub for failedblocking
  2. change payload of failedblocking order to failed message (not rely on orderInfoForPub anymore)
  3. cleaner and safer code to maintain closed and open order. The closed concept is consistent with QS
  4. fix new added order status field is not correctly involved in orderbook calculation

Preflight checks

  • build passed (make build)
  • tests passed (make test)
  • integration tests passed (make integration_test)
  • manual transaction test passed (cli invoke)

Already reviewed by

...

Related issues

#499

@cryptom-dev cryptom-dev added the bug Something isn't working label Mar 17, 2019
@cryptom-dev cryptom-dev self-assigned this Mar 17, 2019
@cryptom-dev cryptom-dev force-pushed the issue499 branch 2 times, most recently from a902930 to eb6e630 Compare March 18, 2019 08:23
@cryptom-dev cryptom-dev changed the title [WIP] #499 failed blocking should not be regarded as closed order [R4R] #499 failed blocking should not be regarded as closed order Mar 18, 2019
}

func (msg Order) isChargedExpire() bool {
return msg.CumQty == 0 && (msg.Status == orderPkg.IocNoFill || msg.Status == orderPkg.Expired)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return msg.Status == orderPkg.IocNoFill || (msg.CumQty == 0 && msg.Status == orderPkg.Expired)

Copy link
Contributor Author

@cryptom-dev cryptom-dev Mar 19, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return msg.Status == orderPkg.IocNoFill || (msg.CumQty == 0 && msg.Status == orderPkg.Expired)

We cannot change to that.
Partial filled IOC order would also publish a IocNoFill status to indicates it's closed. And this method is statistic whether this ioc no fill should be charged.

More background: we publish:
for no fill ioc: Ack, IocNoFill
for partial or fully fill ioc: Ack, PartialFill (with a counterpart1)...PartialFill (with counterpart2), IocNoFill

@cryptom-dev cryptom-dev requested a review from yutianwu March 18, 2019 12:21
@cryptom-dev cryptom-dev merged commit e4bc2bf into develop Mar 20, 2019
@cryptom-dev cryptom-dev mentioned this pull request Mar 27, 2019
4 tasks
@unclezoro unclezoro deleted the issue499 branch May 10, 2022 06:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants