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

Remove withdrawals from the matching algorithm for increased efficiency and security #398

Open
elliottdehn opened this issue Jul 31, 2023 · 4 comments
Labels
disaster Implementing this would cause more issues

Comments

@elliottdehn
Copy link
Contributor

Right now, when an order is matched, we move the funds owed to the maker into their market account. This means the market is susceptible to DOS attack by making many small limit orders to gum up execution. This is dealt with using a minimum size configuration for the market. This however results in a fragmentation of markets, where we would prefer to have just one market for every pair.

I propose that we simply remove the part where funds are moved from the matching algorithm, instead in favor of using a withdraw function called on the maker side once their orders are filled.

This would allow us to perform matching without having to address every order one-by-one, meaning we could perform matching with O(log(N)) writes instead of O(M*log(N) + M) writes where M is the match size and N is the order book size. This is more efficient, and allows us to get rid of the minimum size parameter that exists on every market due to its increased security.

@elliottdehn elliottdehn added the enhancement New feature or request label Jul 31, 2023
@elliottdehn
Copy link
Contributor Author

For clarity, this would be a step on the path to removing all market parameters except base type and quote type. It's required if we ever intend to do that.

@elliottdehn
Copy link
Contributor Author

Addendum: doing this will also allow us to eliminate the need to have at least one dedicated account per pair because it will no longer be necessary to gain the associated optimization

@bogberry
Copy link
Contributor

bogberry commented Aug 7, 2023

A fill must necessarily be a write operation, as it is essential to update the orderbook state with those fills to make sure that any subsequent fill is performed on the orderbook as it is after that order was made.

This would allow us to perform matching without having to address every order one-by-one, meaning we could perform matching with O(log(N)) writes instead of O(M*log(N) + M) writes where M is the match size and N is the order book size.

No. I’m not sure you understand how an orderbook works.

This is more efficient, and allows us to get rid of the minimum size parameter that exists on every market due to its increased security.

Eliminating minimum size would still allow someone to spam the book with a ton of extremely small orders, and it’s irrelevant how many fills each order constitutes - even if each order only results in one fill, it's problematic.

I don’t understand why you think this is more secure either.

@qdrs
Copy link
Contributor

qdrs commented Aug 8, 2023

This however results in a fragmentation of markets, where we would prefer to have just one market for every pair.

@elliottdehn can you elaborate on this? I think you don't fundamentally understand the difference between an orderbook vs a market...

@qdrs qdrs added disaster Implementing this would cause more issues and removed enhancement New feature or request labels Aug 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
disaster Implementing this would cause more issues
Projects
None yet
Development

No branches or pull requests

3 participants