Skip to content

Conversation

0xSachinK
Copy link
Contributor

@0xSachinK 0xSachinK commented Jul 15, 2021

Aave V2 Library contract:

  • Hosts a collection of invoking functions that enable SetToken modules to invoke SetToken interactions with AaveV2
  • Has getter functions that generate calldata to interact with AaveV2
    • Can be used by off-chain workers/managers to generate the required calldata

Aave V2 Fixture:

  • Creates the entire AaveV2 environment locally and initializes it with a known state to run our tests
  • Has helper functions to perform common tasks such as creating & activating a reserve on AaveV2
  • Uses

    • Write tests for AaveV2 Library contract
    • Write tests for Aave Leverage Module contract
    • Write tests for any contracts we write in the future which needs an AaveV2 mock environment

Implementation

  • Write Aave V2 Fixture
  • Write tests for Aave V2 Fixture
  • Write Aave V2 Library contract
  • Write tests for Aave V2 Library contract
    • Write Aave V2 Mock contract
    • Use fixture and mock contract to write tests for Aave V2 Library contract

Further Reading

STIP: Aave Leverage Module

Note: @ncitron needs to port over the existing Aave V2 governance-related functionality from the Aave fixture to the new Aave V2 fixture.

Copy link
Contributor

@ncitron ncitron left a comment

Choose a reason for hiding this comment

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

Looks pretty solid to me. Made some usability comments in the fixture. Also make sure to add parameter lists and return values to your javadoc comments

Copy link
Contributor

@richardliang richardliang left a comment

Choose a reason for hiding this comment

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

Overall pretty close. Assuming we're changing all the public get calldata functions to internal functions to save gas as we discussed in DMs

@0xSachinK
Copy link
Contributor Author

Assuming we're changing all the public get calldata functions to internal functions to save gas as we discussed in DMs

Calldata generation method Gas Used
External invoke function generating calldata by itself 23563
External invoke function calling internal getter function to generate calldata 23583
External invoke function calling public getter function to generate calldata 23627

All cost approximately the same amount of gas. Hence, we can stick to the current version, as it is comparatively cheaper to deploy than the others.

@0xSachinK 0xSachinK merged commit 266de14 into master Jul 25, 2021
richardliang pushed a commit that referenced this pull request Aug 25, 2021
* Move Aave V1 ABIs to external/abi/aave/v1 subfolder

* Move Aave V1 contracts to external/contracts/aave/v1 subfolder

* Remove unnecessary diff

* Add Aave V2 ABIs and contracts

* Add interfaces to interfaces/external/aave-v2

* Add AaveV2 library contract

* Add AaveV2 Mock contract

* Add deployment utils for AaveV2 Library and mock contract

* Add AaveV2 fixture

* Add AaveV2 fixture tests

* Fix bug in AaveV2Mock and library contracts

* Add deployWethReserve and deployDaiReserve as separate functions in fixture

* Add tests for Aave V2 Library contract

* * Add javadocs & other suggested changes to Library and Mock contract

* Add suggested changes to fixture

* Initialize weth and dai reserves in initialize() function
* Improve comments

* Return values from library

* Add detailed javadocs to library contract

* Fix tests: Remove unnecessary checks

* Untabify javadocs

* Remove extra line
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.

4 participants