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 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
Implement BSIP 74: Margin Call Fee Ratio #2130
Implement BSIP 74: Margin Call Fee Ratio #2130
Changes from 31 commits
e9e0e7e
00b2ed1
8d5aad6
fee9fe6
ca6f361
fa7d4d5
a43e18a
2b120c9
4fd251c
35d797a
3ea265a
4684e51
1aee59c
750c720
9ea7962
84c3e04
75374de
a394deb
d11ecbf
fad10a6
0fae92f
3531df9
96c2d98
1424257
476e3fd
efff548
d062cd2
eb9a898
4c0ac06
f3f363c
7df91b4
5c2210b
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
The function name is misleading due to the
ratio-GRAPHENE_COLLATERAL_RATIO_DENOM
calculation in the implementation. By the wayratio-GRAPHENE_COLLATERAL_RATIO_DENOM
may underflow.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.
It's better to keep the comment.
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.
The margin call fee should be stored in the
fee
field of thefill_order_operation
which contains the call order, but not the limit order.Market fees is unrelated to the margin call fee. The limit order should pay market fee regardless.
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 didn't fully understand what's your definition of "a margin call" nor the logic around this parameter. Looks like it is always equal to
!is_maker
.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 think it's better to move this function from
database
class toprice_feed
class.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 thought about that, but all other HARDFORK code (which is the basis for this function) has been kept away from the protocol code. Should I break that rule?
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.
True, protocol code doesn't include chain/hardfork.hpp. We usually check the time in chain code and convert the result to another meaningful parameter to pass in.
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.
It's inefficient to adjust the param on every order. Since it's only related to the price feed, it's best to save an adjusted value whenever the price feed changes or the param itself changes.
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.
This is incorrect.
All the specs and additional comments say
fee = debt / feed_price * fee_rate
, but notcollateral * fee_rate
, becausedebt / feed_price != collateral
due toMSSR
.