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

Feature/cross side order fees #1906

Merged

Conversation

davidvuong
Copy link
Member

This PR resolves an issue in reference to how order fees are calculated. Currently, a trader is charged a makerFee if the size of their order reduces the market skew and a takerFee if the order increases the skew. However, in certain situations, an order can decrease and increase the skew. In these scenarios, the proportion of size that decreases the skew should be charged a makerFee and the remaining, a takerFee.

For example,

price = 100
makerFee = 0.001
takerFee = 0.003

# Increasing the skew
skew = 10
size = 5
skewResult = 15
fee = 5 * price * takerFee

# Decreasing the skew
skew = 10
size = -3
skewResult = 7
fee = 3 * price * makerFee

# Increasing and decreasing the skew
skew = 10
size = -15
skewResult = -5
fee = 10 * price * makerFee + 5 * price * takerFee

Note: This PR should not be merged until both #1893 and #1899 are merged.

Please see comments in code and tests for more detail.

@davidvuong davidvuong self-assigned this Sep 29, 2022
@codecov
Copy link

codecov bot commented Sep 29, 2022

Codecov Report

Merging #1906 (4e8d16d) into feature/time-based-next-price (2772c88) will increase coverage by 0.08%.
The diff coverage is 100.00%.

❗ Current head 4e8d16d differs from pull request most recent head 8fd5433. Consider uploading reports for the commit 8fd5433 to get more accurate results

@@                        Coverage Diff                        @@
##           feature/time-based-next-price    #1906      +/-   ##
=================================================================
+ Coverage                          95.02%   95.10%   +0.08%     
=================================================================
  Files                                116      117       +1     
  Lines                               2853     2900      +47     
  Branches                             852      868      +16     
=================================================================
+ Hits                                2711     2758      +47     
  Misses                               142      142              
Impacted Files Coverage Δ
contracts/PerpsV2Market.sol 100.00% <ø> (ø)
contracts/PerpsV2MarketProxyable.sol 100.00% <ø> (ø)
contracts/PerpsV2MarketState.sol 75.00% <ø> (ø)
contracts/MixinPerpsV2MarketSettings.sol 100.00% <100.00%> (ø)
contracts/PerpsV2ExchangeRate.sol 100.00% <100.00%> (ø)
contracts/PerpsV2MarketBase.sol 100.00% <100.00%> (ø)
contracts/PerpsV2MarketDelayedOrders.sol 97.36% <100.00%> (+1.71%) ⬆️
contracts/PerpsV2MarketSettings.sol 94.73% <100.00%> (+1.11%) ⬆️
contracts/PerpsV2MarketViews.sol 100.00% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Contributor

@leomassazza leomassazza left a comment

Choose a reason for hiding this comment

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

LGTM, Just added a suggestion to rename a variable (though the name gets too long)

@davidvuong
Copy link
Member Author

This PR depends on time-based-next-price, which depends on futuresV2. See #1899 for updates.

@davidvuong davidvuong marked this pull request as ready for review October 19, 2022 00:59
leomassazza and others added 3 commits November 4, 2022 10:34
* use helper to reduce contracts size

* add Pyth interfaces

* add new params and interfaces

* make test pass again

* add Pyth Orders functionality

* bootstrap pyth test

* add mocked pyth

* more tests

* complete tests for exchange rate

* add two missing test to exchange rate

* fix some tests

* fix tests

* fix missing settings

* use different price onchain than offchain

* remove commented content

* unify delayed orders

* remove redundant code (reduce size)

* Improve offchain timing control

* Reduce submit order params

* remove minFeedAge and latestPublishTime

* prevent cancelations before time (offchain)

* remove unnecessary pyth references in names
* First pass at SIP-279 new funding calc

No tests, no documentation, just Solidity changes (all entirely
untested)... but it compiles!

* Explicit types so runtime functions can be detected

* Emit new FundingRateRecomputed event after every recomputeFunding

* Extend emitFundingRecomputed to include fundingRate

(rather than adding a new event)

* Be more explicit when comparing events and resolve tests

All tests except for funding related changes are now fixed.

* 10 empty test cases for new funding to be implemented next

* Resolve native div truncating decimals and use divideDecimal

* Checkpoint to partially add unit tests for funding

Remaining tests are still failing. More resolutions to come in following
commits.

* UnrecordedFunding to be calc using {divide,multiply}Decimal

* Missing closing parenthesis was bothering me too much

* Further changes to resolve tests

- Various skew rates no longer relevant as funding rate flaots and not
  instantaneous
- assert.bnClose as margin off by ~0.000001
- Add back a larger amount of margin to withdraw in transferMargin

* Refactor and add a clear example in docs

* Liquidation price tests now fixed to reflect new funding

* More passing testse with one leading to some questions

* Rename fundingRate to fundingRateLastRecopmuted

This also includes a bunch of debugging code. Please ignore them

* Add currentFundingRateVelocity as a view fn on markets

* Remove FUNDING_RATE and add FUNDING_VELOCITY

This also updates all references, including tests for perpsv2. It also
renames maxFundingRateVelocity to maxFundingVelocity.

* Fix more unit tests related to funding

* Formatting and shuffle units out of nested desc

* Ensure pSkew cannot be outside of -1 and 1

* More tests are passing

Pos/Neg in funding accrual is raising some eyebrows. Need to investigate

* Invert the funding only at the point of tracking unrecordedFunding

* proportional velocity and funding seq unit tests fixed

* All remaining PerpsV2Market unit tests are now passing

* Remove unused ethers import

* Update comments because english is hard :sadpanda:

* Simplify pSkew cap and rm irrelevant unit test

Co-authored-by: Leonardo Massazza <lmassazza@gmail.com>
@leomassazza leomassazza merged commit a603d3a into feature/time-based-next-price Nov 4, 2022
leomassazza added a commit that referenced this pull request Nov 4, 2022
* Minor typo fixes

* Pull nextPriceOrders into MArketState to ensure upgradeability

* Explicitly destructure NextPriceOrder and pass as primitives

* Resolved .nextPrice unit tests after adding upgradeability

* Remove commented out test code

* Add executableAtTime for time based delayed orders

I'm going to also rename this to `deployedOrder` from `nextPriceOrder`
since the order may be executed without the presence of a 'next price'.

* All existing FuturesV2Market now pass

* Renamed NextPriceOrder to DelayedOrder

* Initial set of test amendments for delayed orders

Next will be the final set of tests for the execution of delayed orders
before or after a CL price update.

* Formatting and more mv next-price -> delayed

* Add delayed order time based test

* Rename MIN_NEXT_PRICE_ORDER_DELAY -> MIN_ORDER_DELAY

* Add missing delayedOrder fns to IFuturesV2MarketState post rebase

* Post futuresV2 rebase fixes due to renames

* Rename remaining nextPrice -> delayedOrder for FuturesV2*

With the exception of the nextPriceConfirmationWindow. We may want to
keep this alongisde a new delayedOrderConfirmationWindow to allow
cancellations permissible on past round confirm XOR past time based
confirm window.

* Pull min/maxDelayTimeDelta into MarketSettings

Additionally, further names next-price in comments, changes to avoid
accessing over the 16th slot, resolved errors in futuresV2 branch
(tests), and renamed maxTimeDelta to desiredTimeDelta to be more
canonical.

* Run lint:fix and mv missing maxTimeDelta

* First round of post merge resolutions on DelayedOrders

* Further post-merge resolutions, mainly around renaming next-price

* .delayedOrder tests are passing again

TODO:

1. Additional tests for delay upperbound
2. Changes to cancellations for keepers to cancel order if past delay
3. Pull delayOrderConfirmationWindow into params for SCCP control
4. Further unit tests

* Fix remaining FuturesV2 tests

* Add delayedOrderConfirmWindow (not yet used)

* Cancellations and executions look an updated confirmationWindow

Confirmation window has been updated to also look at the
delayedOrderConfirmWindow SCCP controllable variable and an additional
unit test has been added to account for cancellations.

* Minor Next Price mv to Delayed Orders

* Post crazy rename merge conflict resolution fixes

* Minor doc updates to reflect DelayedOrder mv

* SIP-281 set desiredTimeDelta = min if 0, tests, comments

* Resolve broken tests after intro of Perps delayed order proxy

* Add missing modifier post merge, missing arg, and rm .only

* Additional perpsv2 renames and fix integration tests

* A few additional missed PerpsV2 rename NextPrice renames

* Separate futures and perps config

Also updates config to include 3 additional params

* DelayedOrders use the correct price on execution when target not reached

* currentRoundId should be inclusive, meeting the targetRoundId

* Always use the current price when executing delayedOrders

Additionally, resolve typos identified during review.

* Feature/cross side order fees (#1906)

* _orderFee now looks at taker/maker fees when crossing over skew

* Update orderFee tests to reflect cross side orders maker/taker

* Avoid hardcoding fees in non orderFee specific tests

* Fix minor formatting

* Fix minor lint error

* Feature/SIP-285 Offchain price feed (#1913)

* use helper to reduce contracts size

* add Pyth interfaces

* add new params and interfaces

* make test pass again

* add Pyth Orders functionality

* bootstrap pyth test

* add mocked pyth

* more tests

* complete tests for exchange rate

* add two missing test to exchange rate

* fix some tests

* fix tests

* fix missing settings

* use different price onchain than offchain

* remove commented content

* unify delayed orders

* remove redundant code (reduce size)

* Improve offchain timing control

* Reduce submit order params

* remove minFeedAge and latestPublishTime

* prevent cancelations before time (offchain)

* remove unnecessary pyth references in names

* Feature/sip 279 new funding (#1914)

* First pass at SIP-279 new funding calc

No tests, no documentation, just Solidity changes (all entirely
untested)... but it compiles!

* Explicit types so runtime functions can be detected

* Emit new FundingRateRecomputed event after every recomputeFunding

* Extend emitFundingRecomputed to include fundingRate

(rather than adding a new event)

* Be more explicit when comparing events and resolve tests

All tests except for funding related changes are now fixed.

* 10 empty test cases for new funding to be implemented next

* Resolve native div truncating decimals and use divideDecimal

* Checkpoint to partially add unit tests for funding

Remaining tests are still failing. More resolutions to come in following
commits.

* UnrecordedFunding to be calc using {divide,multiply}Decimal

* Missing closing parenthesis was bothering me too much

* Further changes to resolve tests

- Various skew rates no longer relevant as funding rate flaots and not
  instantaneous
- assert.bnClose as margin off by ~0.000001
- Add back a larger amount of margin to withdraw in transferMargin

* Refactor and add a clear example in docs

* Liquidation price tests now fixed to reflect new funding

* More passing testse with one leading to some questions

* Rename fundingRate to fundingRateLastRecopmuted

This also includes a bunch of debugging code. Please ignore them

* Add currentFundingRateVelocity as a view fn on markets

* Remove FUNDING_RATE and add FUNDING_VELOCITY

This also updates all references, including tests for perpsv2. It also
renames maxFundingRateVelocity to maxFundingVelocity.

* Fix more unit tests related to funding

* Formatting and shuffle units out of nested desc

* Ensure pSkew cannot be outside of -1 and 1

* More tests are passing

Pos/Neg in funding accrual is raising some eyebrows. Need to investigate

* Invert the funding only at the point of tracking unrecordedFunding

* proportional velocity and funding seq unit tests fixed

* All remaining PerpsV2Market unit tests are now passing

* Remove unused ethers import

* Update comments because english is hard :sadpanda:

* Simplify pSkew cap and rm irrelevant unit test

Co-authored-by: Leonardo Massazza <lmassazza@gmail.com>

Co-authored-by: Leonardo Massazza <lmassazza@gmail.com>

Co-authored-by: Leonardo Massazza <lmassazza@gmail.com>
@davidvuong davidvuong deleted the feature/cross-side-order-fees branch November 17, 2022 22:19
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.

None yet

2 participants