app: show more details in user orders table #378
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
chappjc
reviewed
May 19, 2020
client/core/core.go
Outdated
Comment on lines
299
to
334
| // setEpoch sets the epoch. If the passed epoch is greater than the highest | ||
| // previously passed epoch, send an epoch notification to all subscribers. | ||
| func (dc *dexConnection) setEpoch(mktID string, epochIdx uint64) bool { | ||
| dc.epochMtx.Lock() | ||
| defer dc.epochMtx.Unlock() | ||
| if epochIdx > dc.epoch[mktID] { | ||
| dc.epoch[mktID] = epochIdx | ||
| dc.notify(newEpochNotification(dc.acct.url, mktID, epochIdx)) | ||
| var updateCount int | ||
| dc.tradeMtx.Lock() | ||
| for _, trade := range dc.trades { | ||
| if trade.processEpoch(epochIdx) { | ||
| updateCount++ | ||
| } | ||
| } | ||
| dc.tradeMtx.Unlock() | ||
| if updateCount > 0 { | ||
| dc.refreshMarkets() | ||
| return true | ||
| } | ||
| } | ||
| return false | ||
| } | ||
|
|
||
| // marketEpochDuration gets the market's epoch duration. If the market is not | ||
| // known, an error is logged and 0 is returned. | ||
| func (dc *dexConnection) marketEpochDuration(mktID string) uint64 { | ||
| mkt := dc.market(mktID) | ||
| if mkt == nil { | ||
| log.Errorf("marketEpoch called for unknown market %s", mktID) | ||
| return 0 | ||
| } | ||
| return mkt.EpochLen | ||
| } | ||
|
|
||
| // marketEpoch gets the best known epoch for the specified market and time | ||
| // stamp. If the market is not known, 0 is returned. | ||
| func (dc *dexConnection) marketEpoch(mktID string, stamp time.Time) uint64 { | ||
| epochLen := dc.marketEpochDuration(mktID) | ||
| if epochLen == 0 { | ||
| return 0 | ||
| } | ||
| return encode.UnixMilliU(stamp) / epochLen |
chappjc
reviewed
May 19, 2020
client/core/core.go
Outdated
| return mkt.EpochLen | ||
| } | ||
|
|
||
| // marketEpoch gets the best known epoch for the specified market and time |
There was a problem hiding this comment.
"best known epoch" is a bit confusing. Looks like the function returns the epoch index that includes the specified stamp time.
client/core/trade.go
Outdated
| } | ||
|
|
||
| // newTrackedTrade is a constructor for a trackedTrade. | ||
| func newTrackedTrade(dbOrder *db.MetaOrder, preImg order.Preimage, dc *dexConnection, | ||
| func newTrackedTrade(dbOrder *db.MetaOrder, preImg order.Preimage, dc *dexConnection, mkt *Market, |
There was a problem hiding this comment.
Unless you have plans for the *Market argument, can we just pass epochLen uint64?
There was a problem hiding this comment.
Yeah. I was on the fence with that anyway. At one point I was storing the whole *Market.
buck54321
commented
May 20, 2020
client/core/trade.go
Outdated
Comment on lines
914
to
921
| // DRAFT NOTE: As of now, if the client is not subscribed to the order book when | ||
| // the order actually matches, the status might not be set correctly. This is a | ||
| // consequence of having no notifications for orders which either are 1) | ||
| // unmatched (market order against empty book, or immediate limit order with | ||
| // restrictive rate), or 2) are booked without matching. This also creates the | ||
| // odd case that a fully-matched standing limit order may be set to | ||
| // OrderStatusBooked before a subsequent match notification might change it | ||
| // to OrderStatusExecuted. |
There was a problem hiding this comment.
I'm going to remove this draft note. I don't know if this is an issue or not, but I suspect it is. The crux is that the client has no way of knowing for certain that their order was booked if they don't actually receive a match notification (indicating only partial matching), which will often be the case. The client right now is just assuming that their order was booked, because the epoch expired and they haven't heard otherwise.
|
Got a not-a-conflict to resolve. :) |
Shows additional details in the user orders table. New columns include order type, percent settled, and order status. Additionally, the age column is refreshed every second.
chappjc
approved these changes
May 20, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
New columns include order type, percent settled, and order status. Additionally, the age column is refreshed every second. Corrects a number of related deficiencies with epoch handling and status updates.
Resolves #297