Skip to content

spec: add nomatch notification#535

Merged
chappjc merged 3 commits into
decred:masterfrom
buck54321:spec-unmatch
Jul 20, 2020
Merged

spec: add nomatch notification#535
chappjc merged 3 commits into
decred:masterfrom
buck54321:spec-unmatch

Conversation

@buck54321
Copy link
Copy Markdown
Member

@buck54321 buck54321 commented Jul 7, 2020

If there is no match for an order, the client must currently infer that from the match proof. This PR adds a nomatch notification when the client's order does not match during the match cycle. The notification will be sent for market and limit orders (regardless of time-in-force). This is just one possible solution the issue raised in #525 (review).

@chappjc
Copy link
Copy Markdown
Member

chappjc commented Jul 7, 2020

'nomatch' sent just before 'match' would be simple enough. Although I was hoping we could infer the lack of a match: user knows what preimages they supplied so they can determine what is not included in 'match' and call that unmatched. Thoughts about that to avoid code or spec changes?

Basing it on the preimage submission rules out missed epochs that you'd have to guess about if you just based it on the order submissions.

@buck54321
Copy link
Copy Markdown
Member Author

so they can determine what is not included in match

They won't receive a match request if they don't have any matches. So they are then looking for a lack of a match request, but there is no way to know if the match cycle is actually complete.

Basing it on the preimage submission rules out missed epochs that you'd have to guess about if you just based it on the order submissions.

I could use some clarification on this. Not sure I follow.

@chappjc
Copy link
Copy Markdown
Member

chappjc commented Jul 7, 2020

Good point about no matches.

Regarding preimage submission ruling out missed epochs, I mean that if the server requests a preimage for a given order, you know that order will go through matching in that epoch rather than falling into the next epoch by a hair if you just guessed based on your order submission time.

Comment thread spec/orders.mediawiki Outdated
Comment on lines +818 to +822
It is also possible for an order to go through the matching cycle without
generating a match. This will be common for limit orders, but can also occur for
market orders if there are no booked orders to match with. When the server fails
to find any matches, a <code>nomatch</code> notification will be sent to the
client.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

extra space: an order

Comment thread spec/orders.mediawiki
redemption transaction.

It is also possible for an order to go through the matching cycle without
generating a match. This will be common for limit orders, but can also occur for
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Common for limit orders with time-in-force "immediate" would be clearer I think. Oh, were you thinking nomatch would be sent for limit orders that get booked too? That seems a little unnecessary because the client does not need to take any action for unmatched but booked limit orders.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It actually solves a small problem that I've been cheating around. See (*trackedTrade).processEpoch. For standing limit orders, I'm setting them as booked as soon as I know their epoch has passed, but that isn't necessarily accurate, since if they fill they are never actually booked. Having a nomatch notification allows me to set them as booked confidently in either negotiate or via the upcoming handleNoMatch function.

@chappjc chappjc added the spec label Jul 14, 2020
@dnldd
Copy link
Copy Markdown
Member

dnldd commented Jul 17, 2020

Looks good once the additional space is resolved. Wondering if its possible to reduce the number notifications that'd be generated for long standing market orders with too high/too low of a price. With the current spec a nomatch will go out every match cycle, that could add up for the server.

@buck54321
Copy link
Copy Markdown
Member Author

Wondering if its possible to reduce the number notifications that'd be generated for long standing market orders with too high/too low of a price. With the current spec a nomatch will go out every match cycle, that could add up for the server.

Only orders from the epoch queue will receive these notifications. Booked orders don't get subsequent notifications every match cycle.

I've added a word to specify epoch orders only. Thanks.

Copy link
Copy Markdown
Member

@chappjc chappjc left a comment

Choose a reason for hiding this comment

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

Looks good. Noting that the implementation is completed in #525, which is ready to merge as well.

@chappjc chappjc merged commit 4c2ab9b into decred:master Jul 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants