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
Protocol fee adjustment in driver #2213
Protocol fee adjustment in driver #2213
Changes from 29 commits
bc6df24
36443b9
5acaaf3
ffe84f9
950e747
6db55cd
1de0456
9d5abd6
06da3ef
1bff251
04c1c5a
42b442d
bfd38a1
3176004
121cb95
a59be01
60d200a
15bccce
41d8b2f
0b135aa
490f0b8
b9df28f
7b4939d
490ecb4
654181b
e2bc18c
3767853
0cb7c19
32c100e
fe86304
c10e82c
010cfc4
42cd81d
3fa36b9
0f5d374
59d8dae
e7cd0e3
7fccdcd
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.
If this is used for the surplus based fee we need to reference the quoted price somehow.
Otherwise we'll only compare against the signed limit price which already has the slippage baked in. This is not what we want, right?
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 current implementation only works for out of market limit orders (where the quote is worse than the limit price). It's true that the naming is a bit misleading and the real fee policy we are implementing here is is
SurplusFee
notPriceImprovement
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 see the
PriceImprovement
as a general term - improvement over SOME price, whether it's a limit price or some other artificial price (artificial becausedriver
works with limit orders technically and doesn't care whether it has slippage baked in and in which amount).If it were called
MarketPriceImprovement
then it would be misleading IMO. Having a general term is better since we will implement the improvement over quotes 99.99% so renaming it would lead to renaming the autopilot/driver interface which is not too problematic right now, but it could be in the future.OTOH, I might be biased since I spent a lot of time thinking in those terms, so if you both prefer to have
SurplusFee
I will update it.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've captured the potential renaming #2266
I believe this is ok since chances are higher that we will implement this so it's easier to close the issue than to revert the changes.