ALM internal audit fixes #122
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* Emit struct elements separately for greater clarity * Add corresponding test
0xSachinK
force-pushed
the
sachin/alm-internal-audit-fixes
branch
from
75332d4 to
d54d860
Compare
* Add header for external functions * Add _addCollateralAssets and _addBorrowAssets internal functions * Note: New internal functions are a straight copy of the previous external function implementations. Specific changes to _addCollateralAssets and _addBorrowAssets would be done later.
* Improve javadocs * Revert if borrow balance is zero * Add corresponding test
* Made the function external * Removed call to _updateUnderlyingToReserveTokens() from the associated internal function * Added note to javadoc
* Made the function external * Removed call to _updateUnderlyingToReserveTokens() from the associated internal function * Added note to javadoc * Removed tests associated with confirming underlyingToReserveTokens mappings update
…nd removeBorrowAssets() functions
* Fix javadocs * Add check to validate passed address is valid SetToken * Add corresponding tests
* Fix javadocs * Move emitting events to internal function * Add check for valid aave reserve * Rename event AaveReserveUpdated to more suitable ReserveTokensUpddated
* Pass setTotalSupply as an argument
* Put underlyingToReserveTokens[actionInfo.collateralAsset].aToken into local variable to avoid fetching from storage twice
* Update preciseDiv to preciseDivCeil
* Fix javadocs: Add impossible case
Improve revert message
* Check latest reserve token addresses are saved in underlyingToReserveTokens mapping before adding a new collateral/borrow asset * Add tests for introduced changes
* Add check to revert if mapping already exists. * Rename to _addUnderlyingToReserveTokensMapping * Add tests for changes
* Since we fetch and store lendingPool proxy address in the constructor which would never change. * Aave would only update the lendingPool implementation. * Remove corresponding tests
* Easier to review the diff.
bweick
reviewed
Aug 10, 2021
bweick
approved these changes
Aug 11, 2021
richardliang
pushed a commit
that referenced
this pull request
Aug 25, 2021
* Fix: CollateralAssetsUpdated and BorrowAssetsUpdated events * Fix: AavReserveUpdated event * Emit struct elements separately for greater clarity * Add corresponding test * Add test for LendingPoolUpdated event * Fix: Line 214 & 182 comments * Fix: Initialize function * Add header for external functions * Add _addCollateralAssets and _addBorrowAssets internal functions * Note: New internal functions are a straight copy of the previous external function implementations. Specific changes to _addCollateralAssets and _addBorrowAssets would be done later. * Fix: lever javadocs * Improve javadocs for lever() and delever() * Fix: deleverToZeroBorrowBalance() function * Improve javadocs * Revert if borrow balance is zero * Add corresponding test * Fix: Improve sync() javadocs * Fix: sync(), store borrowAssets[i] in local variable * Fix: addCollateralAssets() function * Made the function external * Removed call to _updateUnderlyingToReserveTokens() from the associated internal function * Added note to javadoc * Fix: addBorrowAssets() function * Made the function external * Removed call to _updateUnderlyingToReserveTokens() from the associated internal function * Added note to javadoc * Removed tests associated with confirming underlyingToReserveTokens mappings update * Add Note in caps about adding only required collateral assets * Fix: componentIssueHook(), use preciseMul * Remove call to sync() from removeModule(), removeCollateralAssets() and removeBorrowAssets() functions * Fix: updateAllowedSetToken function() * Fix javadocs * Add check to validate passed address is valid SetToken * Add corresponding tests * Fix: updateUnderlyingToReserveTokensMapping * Fix javadocs * Move emitting events to internal function * Add check for valid aave reserve * Rename event AaveReserveUpdated to more suitable ReserveTokensUpddated * Add test for invalid reserve in updateUnderlyingToReserveTokensMapping * Fix: Add reference to AaveV2 library in internal function javadocs * Fix: _createAndValidateActionInfoNotional() * Pass setTotalSupply as an argument * Fix: _updateLeverPositions() * Put underlyingToReserveTokens[actionInfo.collateralAsset].aToken into local variable to avoid fetching from storage twice * Fix: _getBorrowPosition() math * Update preciseDiv to preciseDivCeil * Fix: _updateUseReserveAsCollateral() * Fix javadocs: Add impossible case * Fix: _validateCommon() Improve revert message * Fix: _validateNewCollateralAssets() and _validatNewBorrowAsset() * Check latest reserve token addresses are saved in underlyingToReserveTokens mapping before adding a new collateral/borrow asset * Add tests for introduced changes * Refactor updateUnderlyingToReserveTokensMapping * Add check to revert if mapping already exists. * Rename to _addUnderlyingToReserveTokensMapping * Add tests for changes * Remove updateLendingPool function and corresponding event * Since we fetch and store lendingPool proxy address in the constructor which would never change. * Aave would only update the lendingPool implementation. * Remove corresponding tests * Reorder functions to match CLM * Easier to review the diff. * Add suggested changes * Rename parameters to reflect position units
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Internal audit doc: ALM internal audit
Note: Each commit contains a single fix. Fixes are in the same order as mentioned in the doc. Makes it easier to review.