Feat Aave v2

[pakim249CAL]

Pull Request

Issue(s) fixed

This pull request fixes #90

[Rubilmax]

Great job! Small changes for coherence but non-blocking

[Rubilmax]

Added the CI in #94

[MerlinEgalite]

Some changes to apply but overall LGTM

[MerlinEgalite]

One question

[MerlinEgalite]

LGTM

[Rubilmax]

I like having additional endpoints defaulting values so that when calling such endpoints, the user always has the highest possible access to their funds, e.g.:

• deposit(uint256 _amount) => _deposit(msg.sender, msg.sender, msg.sender, _amount, shares)
• deposit(uint256 _amount, address _receiver) => _deposit(msg.sender, _receiver, msg.sender, _amount, shares)

etc...

But it's a plus

[Rubilmax]

But it's a plus

And I think it'd be cleaner to add them in another PR, to every vault (via ERC4626UpgradeableSafe)

[MerlinEgalite]

But it's a plus

And I think it'd be cleaner to add them in another PR, to every vault (via ERC4626UpgradeableSafe)

Yes let's do this in another PR. Integrators are waiting for the ERC4625 interfaces. So even if your suggestion seems to ease integrations it's clearly not in practice with the previous argument.

[Rubilmax]

But it's a plus

And I think it'd be cleaner to add them in another PR, to every vault (via ERC4626UpgradeableSafe)

Yes let's do this in another PR. Integrators are waiting for the ERC4625 interfaces. So even if your suggestion seems to ease integrations it's clearly not in practice with the previous argument.

Proposal in #97