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

Add usdx incentives calculation test #765

Merged
merged 4 commits into from
Jan 18, 2021

Conversation

rhuairahrighairidh
Copy link
Member

@rhuairahrighairidh rhuairahrighairidh commented Jan 14, 2021

This PR adds an incentive test to compares cdp rewards calculated using previous style (updated every block) to the current style (using rewardFactors).

The difference is noticable if the interest rate on the cdp is high enough. The test uses 500% APR, and 10 super long blocks (in total covering 1/5th of a year to get to large accumulated fees).

The second commit adds the expected calculation fix and the numbers get much closer, but not exact. I expect it's due to rounding somewhere but haven't tracked it down.

Optional TODOs

  • why did the exising tests still pass when the calculation was changed?
  • use Suite in the new test to keep style the same

@rhuairahrighairidh rhuairahrighairidh marked this pull request as draft January 14, 2021 15:41
@rhuairahrighairidh rhuairahrighairidh marked this pull request as ready for review January 14, 2021 16:26
@rhuairahrighairidh rhuairahrighairidh added the R4R When a PR is ready for review label Jan 15, 2021
Copy link
Member

@karzak karzak left a comment

Choose a reason for hiding this comment

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

LGTM

x/incentive/keeper/rewards_test.go Outdated Show resolved Hide resolved
@karzak karzak merged commit 067ecde into kd-usdx-incentive-accumulators Jan 18, 2021
@karzak karzak deleted the ro-usdx-incentive-maths-test branch January 18, 2021 16:18
karzak added a commit that referenced this pull request Jan 18, 2021
* feat: split liquidations between external keepers and automated begin blocker

* wip: refactor usdx minting incentives to use accumulators/hooks

* wip: refactor usdx minting claim object

* feat: use accumulators/hooks for usdx minting rewards

* fix: get tests passing

* fix: don't create claim objects unless that cdp type is eligable for rewards

* add begin blocker

* update client

* cleanup comments/tests

* update querier

* address review comments

* fix: check for division by zero

* address review comments

* run hook before interest is synced

* Remove savings rate (#764)

* remove savings rate

* remove savings rate from debt param

* update migrations

* address review comments

* Add usdx incentives calculation test (#765)

* add usdx incentive calculation test

* update reward calculation

* add allowable error to test criteria

* Update x/incentive/keeper/rewards_test.go

Co-authored-by: Kevin Davis <karzak@users.noreply.github.com>

* fix: remove old fields from test genesis state

Co-authored-by: Ruaridh <rhuairahrighairidh@users.noreply.github.com>
karzak added a commit that referenced this pull request Jan 18, 2021
* Add 'InterestFactor' to CDP type (#734)

* update cdp type to include interest factor

* fix build

* Add cdp accumulator methods (#735)

* remame fees to interest

* add accumulate interest method

* add basic test

* add note

* address review comments

* update tests

* Add sync cdp interest method (#737)

* remame fees to interest

* add accumulate interest method

* add basic test

* add note

* address review comments

* update tests

* remove old fee functions

* add method to synchronize cdp interest

* add multi-cdp tests

* add test with many blocks

* add test for interest getter

* address review comments

* calculate time difference then convert to seconds

* fix: update collateral index when syncing interest

* fix: differentiate between case when apy is zero and all fees are being rounded to zero

* fix: round time difference properly

* update cdp genesis state and migrations (#738)

* remame fees to interest

* add accumulate interest method

* add basic test

* add note

* address review comments

* update tests

* remove old fee functions

* add method to synchronize cdp interest

* add multi-cdp tests

* add test with many blocks

* add test for interest getter

* update cdp genesis state and migrations

* address review comments

* calculate time difference then convert to seconds

* fix: update collateral index when syncing interest

* fix: differentiate between case when apy is zero and all fees are being rounded to zero

* fix: simplify add/remove/update collateral index

* update genesis state to include total principal amounts

* update migration

* Delete kava-4-cdp-state-block-500000.json

* Add cdp liquidations by external keeper (#750)

* feat: split liquidations between external keepers and automated begin blocker

* address review comments

* USDX incentive accumulators (#752)

* feat: split liquidations between external keepers and automated begin blocker

* wip: refactor usdx minting incentives to use accumulators/hooks

* wip: refactor usdx minting claim object

* feat: use accumulators/hooks for usdx minting rewards

* fix: get tests passing

* fix: don't create claim objects unless that cdp type is eligable for rewards

* add begin blocker

* update client

* cleanup comments/tests

* update querier

* address review comments

* fix: check for division by zero

* address review comments

* run hook before interest is synced

* Remove savings rate (#764)

* remove savings rate

* remove savings rate from debt param

* update migrations

* address review comments

* Add usdx incentives calculation test (#765)

* add usdx incentive calculation test

* update reward calculation

* add allowable error to test criteria

* Update x/incentive/keeper/rewards_test.go

Co-authored-by: Kevin Davis <karzak@users.noreply.github.com>

* fix: remove old fields from test genesis state

Co-authored-by: Ruaridh <rhuairahrighairidh@users.noreply.github.com>

Co-authored-by: Ruaridh <rhuairahrighairidh@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
R4R When a PR is ready for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants