-
Notifications
You must be signed in to change notification settings - Fork 92
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
multi: add market data API endpoints #796
Conversation
If there's connectivity trouble with the server or the server restarts, and all clients have to reconnect, each client will hit /config on reconnect. We'll have to ensure a global rate limit doesn't dos ourselves with that endpoint. I've been very nervous about the implications of this addition, but I'd feel a lot better if there was a way to switch off the data api with an admin command and a conf setting. Does that seem like it can be done easily? In a way that permits all websocket connectivity to function as before... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Srry no substantial comments. Code looks great to me. Am able to hit all the endpoints with curl, no problem.
server/comms/middleware.go
Outdated
// fulfilled. | ||
func (s *Server) limitRate(next http.Handler) http.Handler { | ||
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | ||
code, err := s.meterIP(r.RemoteAddr) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When I had played with per IP limits in chappjc@9607307#diff-e7126c606b49bf8ca255171ae2f485e8d92f442af17e953700f49bcea7215507R27, I realized that dealing with IPv6 was a bit trickier because it's really the prefix that you need to consider since the device address is trivially adjusted by the client. https://www.ibm.com/support/knowledgecenter/en/STCMML8/com.ibm.storage.ts3500.doc/opg_3584_IPv4_IPv6_prefix_subnet_mask.
The code I landed on to take an IP address string like RemoteAddr and give a key that masks out the the non-prefix bits of IPv6:
type IP [net.IPv6len]byte
func parseIP(addr string) IP {
ip := net.ParseIP(addr)
if ip.To4() == nil {
ip = ip.Mask(net.CIDRMask(64, 128))
}
var ipKey IP
copy(ipKey[:], ip)
return ipKey
}
This is also convenient for an efficient map key.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm still thinking about how the apidata
package integrates with Market and BookRouter, but here are some minor comments.
dfcc4b0
to
28b74fb
Compare
|
||
// ReportEpoch should be called by every Market after every match cycle to | ||
// report their epoch stats. | ||
func (s *DataAPI) ReportEpoch(base, quote uint32, epochIdx uint64, stats *matcher.MatchCycleStats) (*msgjson.Spot, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've added the Spot
return value here because I'd like clients to be able to subscribe to a feed of Spots
. This would allow us to display little green arrows and percent changes next to the market names in the UI.
I'll follow up.
bb51b3f
to
e8f7986
Compare
@@ -459,7 +455,7 @@ func (c *WSLink) Off() bool { | |||
} | |||
|
|||
// IP is the peer address passed to the constructor. | |||
func (c *WSLink) IP() string { | |||
func (c *WSLink) IP() dex.IPKey { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With this and the ip
field not being a string, we're killing the ability to have a legible log. I don't think we want to obscure things for the operator like this.
We could jump through hoops with a Stringer for dex.IPKey, but the IP parsing in net.(IP).String
is way more than we want for frequent stringing.
Exposes a rate-limited HTTP API. Data endpoints are accessible via HTTP or WebSockets. The result for the WebSocket request is identical to the REST API response. Three type of data are available. /spots is the spot price and booked volume of all markets. /candles is candlestick data, available in bin sizes of 24h, 1h, and 15m. An example URL is /candles/dcr/btc/15m. /orderbook is already a WebSocket route, but is now also accessible by HTTP. An example URL is /orderbook/dcr/btc /config is another WebSocket route that is also now available over HTTP too. The data API implements a data cache, but does not cache pre-encoded responses for /candles or /orderbook (yet).
Exposes a rate-limited HTTP API. Data endpoints are accessible via HTTP or WebSockets. The result for the WebSocket request is identical to the REST API response.
/spots is the spot price and booked volume of all markets.
/candles is candlestick data, available in bin sizes of 24h, 1h, 15m, and per-epoch sticks. An example URL is /candles/dcr/btc/15m.
/orderbook is already a WebSocket route, but is now also accessible by HTTP. An example URL is /orderbook/dcr/btc
/config is another WebSocket route that is also now available over HTTP too.
The data API implements a data cache, but does not cache pre-encoded responses for /candles or /orderbook (yet).
server/db
Market history is stored as a row per epoch. I've created a new table for this, though we could also combine with the existing epoch table that holds match proof info if preferred.
I've made no attempt to integrate pre-existing data. Data will be collected from the time of upgrade only. The upgrade is silent, since we don't have upgrade infrastructure in place yet for the server.
server/comms
The config route is now registered as an http handler, but also called from the websocket message handler.
HTTP requests are rate-limited using
"golang.org/x/time/rate"
. There is both a global rate limiter and a per-IP rate limiter. The rate limits apply to un-authenticated routes only. The /config request is has an exception as a "critical' route.Try these with the simnet harness running...
wget -qO- --no-check-certificate 'https://127.0.0.1:17232/api/candles/dcr/btc/5m'
wget -qO- --no-check-certificate 'https://127.0.0.1:17232/api/spots'
wget -qO- --no-check-certificate 'https://127.0.0.1:17232/api/config'
wget -qO- --no-check-certificate 'https://127.0.0.1:17232/api/orderbook/dcr/btc'