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

Reduce unnecessary updates on updateState() and improve caching dynamics #723

Closed
eboadom opened this issue Nov 1, 2022 · 0 comments · Fixed by #725
Closed

Reduce unnecessary updates on updateState() and improve caching dynamics #723

eboadom opened this issue Nov 1, 2022 · 0 comments · Fixed by #725
Assignees

Comments

@eboadom
Copy link
Collaborator

eboadom commented Nov 1, 2022

Currently, the updateState() function on ReserveLogic https://github.com/aave/aave-v3-core/blob/master/contracts/protocol/libraries/logic/ReserveLogic.sol#L97 has 2 responsibilities:

  • Update the reserve indexes (liquidity and/or variable borrow) since the last action on the pool. This currently includes also the update of reserve.lastUpdateTimestamp.
  • Accrue "reserve factor" to the collector, via _accrueToTreasury().

Even if this works properly, there are 2 aspects that could be improved:

  1. On the same block (timestamp) and actions affecting the same reserve, currently there are unnecessary updates of the indexes and lastUpdateTimestamp, writing the same values over and over. This can be improved by checking that the lastUpdateTimestamp is different from block.timestamp, as it should be an invariant of the system that, if time doesn't pass, indexes remain the same in what concerns borrow/deposit dynamics.
    Given that it should not be the responsibility of the _updateIndexes() to also update the lastUpdateTimestamp, better to move it to updateState() too.
  2. _updateIndexes() also includes initial loading of the reserveCache, specifically for reserveCache.nextLiquidityIndex and reserveCache.nextVariableBorrowIndex on https://github.com/aave/aave-v3-core/blob/master/contracts/protocol/libraries/logic/ReserveLogic.sol#L286. Even if this currently works properly, it does because always the calls to cache() strictly precede updateState(), but there is not really any assurance of that. It is probably better to move that cache setup away from _updateIndexes() to cache()
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 a pull request may close this issue.

2 participants