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

server: add order router #65

Merged
merged 3 commits into from
Oct 30, 2019
Merged

server: add order router #65

merged 3 commits into from
Oct 30, 2019

Conversation

buck54321
Copy link
Member

The order router routes market, limit, and cancel orders to waiting markets, performing validation and type translation along the way.

To accommodate the needs of the order router, two new methods were added to common types from the asset package. asset.UTXO got a value getter, and asset.DEXAsset got a method to validate addresses.

Resolves #52

The order router routes market, limit, and cancel orders to waiting
markets, performing validation and type translation along the way.

To accommodate the needs of the order router, two new methods were
added to common types from the asset package. asset.UTXO got a
value getter, and asset.DEXAsset got a method to validate addresses.
Copy link
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.

Good stuff. Minor issues and questions only.

server/comms/msgjson/types.go Outdated Show resolved Hide resolved
server/comms/msgjson/types.go Outdated Show resolved Hide resolved
server/market/orderrouter.go Show resolved Hide resolved
server/market/orderrouter.go Show resolved Hide resolved
server/market/orderrouter.go Outdated Show resolved Hide resolved
server/market/orderrouter.go Outdated Show resolved Hide resolved
server/market/orderrouter.go Show resolved Hide resolved
Comment on lines +498 to +503
// Check that the outpoint isn't locked.
locked := tunnel.OutpointLocked(txid, utxo.Vout)
if locked {
return errSet(msgjson.FundingError,
fmt.Sprintf("utxo %s:%d is locked", utxo.TxID.String(), utxo.Vout))
}
Copy link
Member

Choose a reason for hiding this comment

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

OutpointLocked could be made to return more details. What would be helpful here? Locked by another order X?

Copy link
Member Author

Choose a reason for hiding this comment

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

Could be a funding UTXO in an epoch order, and order book order, or any order with an active match. This check could also be handled by storage, since it's all tracked there too.

server/market/orderrouter.go Show resolved Hide resolved
server/market/orderrouter.go Outdated Show resolved Hide resolved
Comment on lines +232 to +233
tunnel.SubmitOrderAsync(oRecord)
r.respondOrder(oRecord)
Copy link
Member Author

Choose a reason for hiding this comment

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

I've separated the signing/response from the epoch submission, but if we move this respondOrder method to Market and do it in the main loop, that will solve the epoch synchronization issue.

Copy link
Member

Choose a reason for hiding this comment

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

Presently, Market has access to the auth manager via the swapper, so it can implement respondOrder. Let's try that.

Copy link
Member Author

Choose a reason for hiding this comment

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

If Market is dealing with authenticated messages, it might as well have it's own copy of AuthManager.

type MarketTunnel interface {
// SubmitOrderAsync submits the order to the market for insertion into the
// epoch queue.
SubmitOrderAsync(*orderRecord)
Copy link
Member Author

Choose a reason for hiding this comment

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

See comments below about respondOrder, but using this new orderRecord wrapper in anticipation of moving server time stamping to the Market mainloop.

}
respMsg, err := msgjson.NewResponse(oRecord.msgID, res, nil)
if err != nil {
log.Errorf("failed to create msgjson.Message for 'cancel' response: %v", err)
Copy link
Member Author

Choose a reason for hiding this comment

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

'cancel' -> order

@chappjc
Copy link
Member

chappjc commented Oct 30, 2019

I'm comfortable merging this when you're ready, @buck54321. We're going to need to iterate on a bunch of things, but having it in will help move things only in my work.

@chappjc chappjc merged commit 364b8a0 into decred:master Oct 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

server: Order Router
2 participants