Skip to content

web: setEpoch should keep orders with no epoch property#571

Merged
chappjc merged 1 commit into
decred:masterfrom
chappjc:web-epoch-filter-undefined
Jul 28, 2020
Merged

web: setEpoch should keep orders with no epoch property#571
chappjc merged 1 commit into
decred:masterfrom
chappjc:web-epoch-filter-undefined

Conversation

@chappjc
Copy link
Copy Markdown
Member

@chappjc chappjc commented Jul 26, 2020

Orders loaded on startup do not have an epoch property set. This changes
OrderBook's setEpoch method to keep orders where the epoch property is
undefined in addition to 0 or the new epochIndex.

Resolves #568 and probably #553 too

Orders loaded on startup do not have an epoch property set. This changes
OrderBook's setEpoch method to keep orders where the epoch property is
undefined in addition to 0 or the new epochIndex.
Copy link
Copy Markdown
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.

I should have noticed back here. Really, I'd like to just make the field a uint64 again. It's not clear to me anymore why it needed to be a pointer. @JoeGruffins?

Copy link
Copy Markdown
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.

Even if we go back to uint64, the omitempty on the field means epoch will be undefined, so this solution is still required.

@chappjc chappjc merged commit 803edff into decred:master Jul 28, 2020
@chappjc chappjc deleted the web-epoch-filter-undefined branch July 28, 2020 14:51
@chappjc
Copy link
Copy Markdown
Member Author

chappjc commented Jul 28, 2020

I should have noticed back here. Really, I'd like to just make the field a uint64 again. It's not clear to me anymore why it needed to be a pointer. @JoeGruffins?

I think the idea was to allow "epoch": 0 in the marshalled json by having a pointer, and to allow the field to be omitted when retrieving the order book and the epoch field is irrelevant wire baggage.

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.

depth chart zooms way in when epoch note arrives

2 participants