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

integration: order and status checks, no pending server messages #716

Merged
merged 4 commits into from
Oct 5, 2020

Conversation

chappjc
Copy link
Member

@chappjc chappjc commented Oct 2, 2020

This includes:

  • removal of server-side message caching (pending messages that were stored until user reconnects)
  • 'connect' response including user's active orders
  • 'order_status' server route and client support for checking orders as needed
  • 'connect' response including user's active matches
  • 'match_status' server route and client support for checking matches as needed

Matches status is processed before order statuses.

status-integ PRs: #677, #699, #687, #704, #714

Note: this PR will be merged via rebase, not squash so each of the commits are replayed on master.

@chappjc chappjc marked this pull request as ready for review October 2, 2020 19:53
@chappjc
Copy link
Member Author

chappjc commented Oct 2, 2020

This PR simply needs testing, and notes for breaking changes (API, db scheme, etc.). The individual commits have already gone through review.

Copy link
Member

@buck54321 buck54321 left a comment

Choose a reason for hiding this comment

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

Looks and tests well. Other than API used only internally, my quick tally of breaking changes is

  1. IsRevoked field for db.MatchProof is now a method that returns proof.ServerRevoked || proof.SelfRevoked. (actually, this one isn't used outside core probably)
  2. msgjson.OrderStatusResult renamed to OrderStatus and Filled field removed.
  3. msgjson.ConnectResult Matches field renamed to ActiveMatches (json matches -> activematches).

client/core/core.go Outdated Show resolved Hide resolved
buck54321 and others added 4 commits October 5, 2020 13:17
Removes all server message caching, as well as the requirement to
acknowledge match, audit, and redemption requests before progressing
through match negotiation. This is intended to be the base upon which a
match_status / order_status conflict resolution system is built.
* server/{auth,db}: connect response return active orders

(processes matches before order statuses)

* client/{core,db}: compare known order statuses against connect response

- Log error and issue notification for active orders reported by DEX that are
 unknown to the client.
- Update order statuses where server-reported statuses are different from statuses
 recorded by client.
- Set statuses to Executed, Canceled or Revoked for orders considered active by the
 client but not returned by the DEX in the connect response. TODO: implement an
 `order_status` route to accurately determine current statuses for such orders.

* simnet: add trading test for order status reconciliation

Also modify trade simnet test cases to run distinctly, wtih new clients
created for each test case, to prevent spillages across test cases.

* determine correct order statuses using order_status route

* retire stale cancel orders if target orders are still active
1. Match status resolution is run on startup and reconnects for any
   status mismatches observed when comparing the `connect` result with
   our local state.

2. Client will now begin negotiation based on the extra matches in the
   connect response.

3. Differentiate between server-initiated revocations and
   client-initiated revocations. Required a database upgrade.

- Log error and issue notification for active orders reported by DEX
  that are unknown to the client.
- Update order statuses where server-reported statuses are different
  from statuses recorded by client.
- Set statuses to Executed, Canceled or Revoked for orders considered
  active by the client but not returned by the DEX in the connect
  response.
Before returning "only one cancel order can be submitted per order per epoch error"
message, first check if the existing cancel order is stale and delete the cancel order
to allow submission of new cancel order.
@chappjc
Copy link
Member Author

chappjc commented Oct 5, 2020

Thanks. Debugf fix rebased into the match_status resolution commit via fixup.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants