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

Use replace_order_id instead of rm/create new order #23

Closed
kilrau opened this issue Jun 18, 2020 · 2 comments · Fixed by #75
Closed

Use replace_order_id instead of rm/create new order #23

kilrau opened this issue Jun 18, 2020 · 2 comments · Fixed by #75
Labels
enhancement New feature or request P1 top priority

Comments

@kilrau
Copy link
Contributor

kilrau commented Jun 18, 2020

Currently arby removes all orders and issues new ones, which causes two packets on the p2p layer and all receiving nodes first to remove and then to replace an order. We consider this becoming an issue once there are a 100s/1000s of orders being shared per second:

18/06/2020 16:13:22.551 [P2P] verbose: received order invalidation from 02819c4892b0730e787d63cc9b184179c807eb5ef6f3c41000923cd83395e6c873 (DinosaurCancel): {"id":"a0a895a0-b17e-11ea-8159-4f081f239e40","pairId":"ETH/BTC","quantity":9908711525}
18/06/2020 16:13:22.551 [ORDERBOOK] debug: order removed: a0a895a0-b17e-11ea-8159-4f081f239e40
18/06/2020 16:13:22.552 [P2P] verbose: received order invalidation from 02819c4892b0730e787d63cc9b184179c807eb5ef6f3c41000923cd83395e6c873 (DinosaurCancel): {"id":"a0a82070-b17e-11ea-8159-4f081f239e40","pairId":"ETH/BTC","quantity":1000000000}
18/06/2020 16:13:22.552 [ORDERBOOK] debug: order removed: a0a82070-b17e-11ea-8159-4f081f239e40
18/06/2020 16:13:22.557 [P2P] verbose: received order from 02819c4892b0730e787d63cc9b184179c807eb5ef6f3c41000923cd83395e6c873 (DinosaurCancel): {"id":"a177a570-b17e-11ea-8159-4f081f239e40","pairId":"ETH/BTC","price":0.02562144,"quantity":1000000000,"isBuy":false}
18/06/2020 16:13:22.558 [ORDERBOOK] debug: order added: {"id":"a177a570-b17e-11ea-8159-4f081f239e40","pairId":"ETH/BTC","price":0.02562144,"quantity":1000000000,"isBuy":false,"peerPubKey":"02819c4892b0730e787d63cc9b184179c807eb5ef6f3c41000923cd83395e6c873","createdAt":1592496802557,"initialQuantity":1000000000}
18/06/2020 16:13:22.558 [P2P] verbose: received order from 02819c4892b0730e787d63cc9b184179c807eb5ef6f3c41000923cd83395e6c873 (DinosaurCancel): {"id":"a17841b0-b17e-11ea-8159-4f081f239e40","pairId":"ETH/BTC","price":0.02365056,"quantity":9907504912,"isBuy":true}

Instead arby should track order id's of it's issued orders and replace them with replace_order_id.

@kilrau kilrau added enhancement New feature or request P3 low priority labels Jun 18, 2020
@kilrau kilrau assigned ghost Jun 18, 2020
@hatmer
Copy link

hatmer commented Jun 19, 2020

opendexnetwork/opendex.network#22 has a decent solution for this issue. Here is the proposed order format in protocol buffer format:

string signature;
string pubkey;
uint32 timestamp_secs;
string request_asset_id;
uint64 request_amount;
string offer_asset_id;
uint64 offer_amount;
optional string replace_order_id;

But changing to an entirely new order format might be a major change and so perhaps this is the intermediary step

@kilrau
Copy link
Contributor Author

kilrau commented Jul 27, 2020

Bumping prio here since today I noticed arby replacing orders every 100ms or so due to fast price movements on binance, resulted frequently in the following:

simnet > sell 1 eth/btc mkt
matched 1 ETH @ 0.02883907 with peer DoveSlim order 9e894580-d032-11ea-94f8-19f64578a848, attempting swap...
failed to swap 1 ETH due to OrderNotFound with peer order 9e894580-d032-11ea-94f8-19f64578a848, continuing with matching routine...
matched 1 ETH @ 0.02884004 with peer DoveSlim order 9ef6d462-d032-11ea-94f8-19f64578a848, attempting swap...
failed to swap 1 ETH due to OrderNotFound with peer order 9ef6d462-d032-11ea-94f8-19f64578a848, continuing with matching routine...
matched 1 ETH @ 0.02885168 with peer DoveSlim order 9f39d0d0-d032-11ea-94f8-19f64578a848, attempting swap...
successfully swapped 1 ETH with peer order 9f39d0d0-d032-11ea-94f8-19f64578a848

Because orders disappear for some ms when being cancelled and re-issued by arby.

@kilrau kilrau added P2 mid priority and removed P3 low priority labels Jul 27, 2020
@kilrau kilrau added user request Requested by end-user and removed user request Requested by end-user labels Jul 29, 2020
@ghost ghost added P1 top priority and removed P2 mid priority labels Aug 5, 2020
@ghost ghost mentioned this issue Aug 13, 2020
@ghost ghost closed this as completed in #75 Aug 15, 2020
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request P1 top priority
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants