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

refactor: eliminate recordedDebtSum (G) from ActivePool #150

Merged
merged 4 commits into from
May 6, 2024

Conversation

danielattilasimon
Copy link
Collaborator

From the perspective of the core system, it was a write-only variable.

It would have been used in:

  • the emergency minimum rate, which won't be implemented,
  • approximation of the the size-weighted interest rate, which can be done alternatively (albeit less accurately) using S & D.

Also, change some function names in ActivePool to improve readability. The function mintAggInterest() has 2 versions:

  1. mintAggInterestNoTroveChange(): mints aggregate interest,
  2. mintAggInterest(): mints aggregate interest and accounts for Trove changes (in debt or interest rate).

Rather than saying what a function doesn't do, it's more helpful to say what it does, so:

  1. mintAggInterestNoTroveChange() -> mintAggInterest()
  2. mintAggInteres() -> mintAggInterestAndAccountForTroveChange()

From the perspective of the core system, it was a write-only variable.

It would have been used in:
- the emergency minimum rate, which won't be implemented,
- approximation of the the size-weighted interest rate, which can be
  done alternatively (albeit less accurately) using S & D.
Copy link
Collaborator

@RickGriff RickGriff left a comment

Choose a reason for hiding this comment

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

Looks great - nice simplification!

@@ -1116,7 +1116,7 @@ contract("BorrowerOperations", async (accounts) => {
assert.isTrue(aliceDebtBefore.gt(toBN(0)));

// check before
const activePool_Bold_Before = await activePool.getRecordedDebtSum();
const activePool_Bold_Before = await activePool.getTotalActiveDebt();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Right, we can swap getRecordedDebtSum out for getTotalActiveDebt in these HH tests presumably because no interest accrues between "before" and "after".

@RickGriff
Copy link
Collaborator

Rather than saying what a function doesn't do, it's more helpful to say what it does, so:

mintAggInterestNoTroveChange() -> mintAggInterest()
mintAggInteres() -> mintAggInterestAndAccountForTroveChange()

Good call!

@danielattilasimon danielattilasimon merged commit c8c55da into main May 6, 2024
5 checks passed
@danielattilasimon danielattilasimon deleted the activepool-small-cleanup branch May 6, 2024 08:10
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.

2 participants