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

market: verify unspent status of unfilled orders #732

Merged
merged 3 commits into from
Oct 16, 2020

Conversation

chappjc
Copy link
Member

@chappjc chappjc commented Oct 8, 2020

Addressing #615

When a user submits a new order, all of that user's unfilled book orders have their funding coins checked. If any of them are spent, those orders are unbooked. The newly submitted order is processed normally after that. The idea behind checking on order placement is to prevent a user from moving the same bunch of coins around into new utxos to fund a large number of bogus (unfunded) orders. Although a user could submit multiple orders with valid funding coins, then proceed to spend those coins, this user could affect similar disruption by simple not executing swaps on match, so continually checking funding coins for online users is not particularly useful. PR #725 handles users going offline.

e.g.

[TRC] MKT: Checking 2 funding coins for order 8926f56b...
[WRN] MKT: Coin ad94f5c84...:0 not unspent for unfilled order 8926f56b.... Revoking the order.
[TRC] AUTH: User 6423e2d92710dba3e490986b02c2e02cf3be847f05e22ee9850e718be412158c cancellation rate is now 100.00% (1 cancels : 0 successes). Violation = false
[INF] MKT: Unbooked unfunded order 8926f56b... for user 6423e2d92710dba3e490986b02c2e02cf3be847f05e22ee9850e718be412158c

Notes:

  • revoke_order is implemented in auth,book,market,ws: Unbook MIA users orders after a timeout #725, thus any unbooked orders are only removed from the book, but the owner does not revoke the order until they reconnect or dexc restarts. This will change automatically with the updates to (*Market).Unbook in auth,book,market,ws: Unbook MIA users orders after a timeout #725
  • This requires spending txns to be mined, not just in mempool, before the check detects the funding coins as spent. This is the same with the check in NewMarket
  • The check uses gettxout (for btc) on each coin ID, but a better check could involve pulling the blocks since the previous check and checking the block contents against a list of coins of interest. DCR could use cfilters. More complex, asset specific.
  • Perhaps the best approach would be to register coin IDs with the backends so the backends watch and notify, but that's even more complex.

@chappjc

This comment has been minimized.

@chappjc chappjc marked this pull request as ready for review October 12, 2020 20:04
@chappjc
Copy link
Member Author

chappjc commented Oct 12, 2020

Revamped with funding check in order router on order placement. Targeted to the owning user and funding coin asset.

We may decide to implement mempool checks too. For now, the unspent checks in the dcr and btc backends use gettxout, which will return the utxo unless a spending tx is mined.

server/market/orderrouter.go Outdated Show resolved Hide resolved
server/market/market.go Outdated Show resolved Hide resolved
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.

Works perfectly. We do need a notification on the client side, but it's a separate issue.

@chappjc chappjc merged commit a9a8439 into decred:master Oct 16, 2020
@chappjc chappjc deleted the unfunded2 branch October 22, 2020 20:24
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.

2 participants