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.

Sign up for GitHub

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

Jump to bottom

Integration of Aave to support loans with NFTs as collateral #156

Merged
merged 63 commits into from
Oct 14, 2021
Merged

Integration of Aave to support loans with NFTs as collateral #156

merged 63 commits into from
Oct 14, 2021

Conversation

aaitor
Copy link
Member

@aaitor aaitor commented Aug 31, 2021 •
edited

Description

Implementation of the Aave integration:

  • Extracting NFTLock interface as preparation for the new ERC-721 NFT lock condition
  • Implementation of NFT Lock condition for ERC-721 based NFTs
  • bumping to v1.2.0

Pending tasks

  • Have the tests in green again
  • In the e2e flows verify that previous conditions are fulfilled before execute
  • Add ACL to the Vault
  • Explore flexible AAve loans (Nice to Have)

Is this PR related with an open issue?

Related to Issue #

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • Follows the code style of this project.
  • Tests Cover Changes
  • Documentation

@aaitor aaitor added the wip label Sep 1, 2021
aaitor and others added 21 commits September 1, 2021 18:20
@aaitor
Merge branch 'master' into feature/aave_integration
feae551
@aaitor
Merge branch 'master' into feature/aave_integration
accb2ec
@aaitor
Adding e2e testing
94653d8
@aaitor
Merge branch 'feature/aave_integration' of github.com:nevermined-io/c…
c8bbfd6 
…ontracts into feature/aave_integration
@aaitor
Merge branch 'master' into feature/aave_integration
b50db41
@aaitor
e2e test (nft transfer back pending)
b059b97
@aaitor
tabs
39058f3
@aaitor
tests
6ed885e
@aaitor
Merge branch 'master' into feature/aave_integration
e531096
@aaitor
WIP: more changes and tests
f71a529
@aaitor
Merge branch 'master' into feature/aave_integration
ad2689f
@aaitor
linting
9e7332e
@aaitor
Merge branch 'feature/aave_integration' of github.com:nevermined-io/c…
ddad091 
…ontracts into feature/aave_integration
@ialberquilla
repayment condition
642d5f5
@ialberquilla
fee managment
a8dd1f3
@ialberquilla
lint
e071caf
@ialberquilla
Merge pull request #164 from nevermined-io/feature/repay_loan_aave
766a7b2 
repayment condition
@ialberquilla
vault docs
d554015
@ialberquilla
withdraw condition
994c43c
@ialberquilla
lint
0aa3328
@ialberquilla
docs
91d9f8a
@aaitor aaitor marked this pull request as ready for review October 7, 2021 09:30
@aaitor aaitor requested review from a team as code owners October 7, 2021 09:30
@aaitor aaitor requested a review from mrsmkl October 8, 2021 10:06
mrsmkl
mrsmkl reviewed Oct 12, 2021
View reviewed changes
contracts/conditions/NFTs/DistributeNFTCollateralCondition.sol Show resolved Hide resolved
contracts/conditions/NFTs/DistributeNFTCollateralCondition.sol Outdated Show resolved Hide resolved
contracts/conditions/NFTs/DistributeNFTCollateralCondition.sol Outdated Show resolved Hide resolved
contracts/conditions/defi/aave/AaveCreditVault.sol Show resolved Hide resolved
contracts/conditions/defi/aave/AaveCreditVault.sol Show resolved Hide resolved
contracts/conditions/defi/aave/AaveBorrowCondition.sol Show resolved Hide resolved
contracts/conditions/NFTs/TransferNFT721Condition.sol
require(
_nftAmount == 0 || (_nftAmount == 1 && token.ownerOf(uint256(_did)) == msg.sender),
_nftAmount == 0 ||
(_nftAmount == 1 && (nftOwner == msg.sender || nftOwner == _lockConditionAddress)),
Copy link
Contributor

Choose a reason for hiding this comment

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

If lockConditionAddress has an NFT and this contract is able to transfer them, then somebody could create another condition, and use it to transfer the NFT away.

Copy link
Member Author

Choose a reason for hiding this comment

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

This will be resolved as an independent issue

contracts/conditions/defi/aave/AaveCollateralDepositCondition.sol Outdated Show resolved Hide resolved
contracts/conditions/defi/aave/AaveCollateralWithdrawCondition.sol Show resolved Hide resolved
contracts/conditions/defi/aave/AaveBorrowCondition.sol Show resolved Hide resolved
@aaitor
Removing unused parameter didRegistryAddress
c5d3e04
@aaitor
using _vaultAddress as parameter to avoid passing unnecessary param…
111cc60 
…eters to hash and fulfill functions
@aaitor
Adding access control to approveBorrower function
3d0bf6b
@aaitor
Adding protection against receiving a second NFT in the vault missing…
5f18278 
… the previous one
@aaitor
removing unused parameter
4140313
@aaitor
using borrower address from the vault to avoid using a non-checked ad…
e4e3ba4 
…dress
@aaitor
using vaultAddress to get the borrower
4e4713d
@aaitor
removing unused parameter and using vault address
14537aa
@aaitor
adding lock condition as part of the vault to implement a better acce…
10f80b0 
…ss control
@aaitor
Adding protection to borrow function in the vault and using the vault…
d2a1645 
…Address as part of the conditions
@aaitor
linting
6173e62
@aaitor
Removing didRegistry address in the conditions initialization because…
71f1b1f 
… is not used
@aaitor
Using lock condition comming from the vault avoiding to pass this as …
642926c 
…a hash and fulfill parameter
@aaitor
Adding CONDITION_TYPE as part of the hashValues functions to prot…
ecd0c65 
…ect against hash collisions between condition ids
@aaitor aaitor merged commit ce9ce17 into master Oct 14, 2021
@aaitor aaitor deleted the feature/aave_integration branch October 14, 2021 10:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mrsmkl mrsmkl mrsmkl left review comments

Assignees
No one assigned
Labels
wip
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@aaitor @mrsmkl @ialberquilla @H34D