multi: connect order book #278
Conversation
| numBuys = 0 | ||
| numSells = 0 | ||
| feedPeriod = 5000 * time.Millisecond | ||
| register := true |
There was a problem hiding this comment.
Just a reminder to go test -tags live -timeout 60m and connect to localhost:54321 for the test server. I've added numBuys, numSells, and feedPeriod to control the initial state of the orderbook, and register to control the initial user registration state on the test server so you don't have to go through all of the forms. Try the live server with feedPeriod = 250 * time.Millisecond too.
There was a problem hiding this comment.
Been doing some simnet testing and it's all working well with the depth chart, notifications, and trades, smooth as butter. I have noticed a couple UI quirks that don't necessarily pertain to the work in this PR though.
When you have the page opened at a given width and you narrow the browser window, the components don't resize as one would expect. You actually have to F5 to make it adjust. e.g. before:
narrowed window:
Note that this shows more than just the visible portion, with the visible portion indicated by the header/menu bar's width. This was a FF screenshot of the full page vs. visible.
F5:
still cramped because I made the windows really narrow, but the elements take reasonable size and position.
I haven't done live_test yet, or finished code review but I'm going to be done reviewing pretty soon.
|
Another note for future work. We'll need the ability to clear out inactive orders from Your Orders, including 100% filled and canceled orders:
|
|
Some other testing observations (webserver/site rebuilt): When cancelling I see two errors, but the cancel succeeds: BTW, these two errors could use some context to say where they are coming from. The client/UI also gets into a state if I try to cancel more than one order without a F5 between: The "rpc error: 19: target order not known" and "Trade password error: encryption key deserialization error: incorrect password" happen together it seems. The pass is correct, tried repeatedly. |
|
Your Orders table suggestions (unrelated to PR):
|
app: Completes the `MarketsPage` js class, and hooks up the /markets page. Better handling of zoom in the depth chart. Epoch orders are also added to depth, but I plan to follow up with a customization to differentiate the display of order book and epoch depth. core: Subscribe to the order book when the user navigates to the /market page. Unsubscribe from the order 1 minute after last tab navigates away from the market's page. Send notifications for book, unbook and epoch orders. Send a notification when a new epoch is known to have started, which could be triggered by either a order book notification or a match_proof notification. client/orders: Modifies client/orders to be more permissive of sequencing issues, which might be temporary, and also to allow orders from multiple epochs to be in the queue at the same time. The latter is necessary because order book notifications might come before the match_proof notification for the prior epoch.
|
This one should be good now. I threw in a bug fix that I found during final testing though, so check that out. I'll open some issues for the other stuff. |
app
Completes the
MarketsPagejs class, and hooks up the /markets page. Better handling of zoom in the depth chart. Epoch orders are also added to depth, but I plan to follow up with a customization todifferentiate the display of order book and epoch depth.
core
Subscribe to the order book when the user navigates to the /market page. Unsubscribe from the order 1 minute after last tab navigates away from the market's page. Send notifications for book, unbook and epoch orders. Send a notification when a new epoch is known to have started, which could be triggered by either a order book notification or a match_proof notification.
client/orders
Modifies client/orders to be more permissive of sequencing issues, which might be temporary, and also to allow orders from multiple epochs to be in the queue at the same time. The latter is necessary because order book notifications might come before the match_proof notification for the prior epoch.