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

Use by_collateral when globally settling #1672

Merged
merged 5 commits into from
Sep 14, 2019

Conversation

abitmore
Copy link
Member

We'd like to remove by_price index at some time in the future to
improve performance, thus we should avoid using the index after the
hardfork.
#1669

We'd like to remove `by_price` index at some time in the future to
improve performance, thus we should avoid using the index after the
hardfork.
#1669
The distinction before/after hardfork #1270 (Call price inconsistent when MCR changed) now looks clearer.
#1669
@ryanRfox
Copy link
Contributor

Please consider/discuss risks associated with deferring this PR to 4.0.0 Protocol Upgrade Release.

@pmconrad
Copy link
Contributor

This is in preparation for a future optimization. Not critical nor high-priority.

@abitmore
Copy link
Member Author

It would be a bit annoying if not included in this release. E.G. need to define another HF date and etc.

The "future optimization" can only take effect after the hard fork time. Postponing it means to postpone the optimization, although not critical.

pmconrad
pmconrad previously approved these changes Mar 26, 2019
@pmconrad
Copy link
Contributor

We shouldn't put new things into upcoming hardfork. We'd have to either postpone the hardfork date or bypass testnet, neither of which I think is justified for this change.

pmconrad
pmconrad previously approved these changes Apr 3, 2019
@abitmore abitmore dismissed pmconrad’s stale review April 3, 2019 10:26

Since it's moved to next consensus release, we need to use another HARDFORK_CORE_TIME macro.

@abitmore
Copy link
Member Author

@pmconrad please review again. Thanks.

Copy link
Contributor

@pmconrad pmconrad left a comment

Choose a reason for hiding this comment

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

Thanks!

@abitmore abitmore merged commit c2cafd1 into hardfork Sep 14, 2019
@abitmore abitmore changed the title Use by_collateral when globally settling after hf Use by_collateral when globally settling Sep 14, 2019
@abitmore abitmore deleted the pr-1669-global-settle-index branch September 14, 2019 20:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants