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

Fix for #315 (v2 Polygon) #316

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Fix for #315 (v2 Polygon) #316

wants to merge 6 commits into from

Conversation

eboadom
Copy link
Collaborator

@eboadom eboadom commented Oct 13, 2022

Changes

  • On updateState() of ReserveLogic, add a condition to not do any update if the timestamp has not changed since the last stored on the reserve's data. This is not related to the 100% RF problem, but it simplifies reasoning about this part of the core code and saves important gas on actions happening in the same block.
  • On _updateIndexes of ReserveLogic, split the 2 update conditions of supply/variable borrow indexes, this way covering the case of 100% RF, on which borrow variable index should be updated (if there is borrow variable debt). The change tries to keep the diff to the minimum
  • This PR affects only the version of v2 used on v2 Avalanche and v2 Polygon, the so-called "light deployment"

Tests

- Changed condition for when indexes should be updated on _updateIndexes().
- Added condition to not do any update on updateState() if time has not passed since the previous update.
- Moved update location of reserve.lastUpdateTimestamp.
@eboadom eboadom changed the title WIP Fix for https://github.com/aave/protocol-v2/issues/315 WIP Fix for #315 Oct 13, 2022
@eboadom eboadom changed the title WIP Fix for #315 WIP Fix for #315 (v2 Polygon/Avalanche) Oct 17, 2022
@eboadom
Copy link
Collaborator Author

eboadom commented Oct 17, 2022

Gas report diff

As expected, there is a slight increase in gas on the most common actions, mainly because of the added condition to skip updates whenever the timestamp didn't change.
At the same time, there are savings on flashLoan(), as the tests involve multiple actions, and there is where savings are noticeable.

Before fix

pre-100-rf-fix-gas-master

After fix
100-rf-fix-gas

@eboadom eboadom changed the title WIP Fix for #315 (v2 Polygon/Avalanche) Fix for #315 (v2 Polygon/Avalanche) Oct 17, 2022
@eboadom eboadom changed the title Fix for #315 (v2 Polygon/Avalanche) Fix for #315 (v2 Polygon) Oct 18, 2022
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.

1 participant