Chainlink for ERC4626 factorized

[QGarchery]

ONLY THE VAULT TESTS:

WITHOUT THE VAULT TESTS (and without the WBTC tests, because they get skipped for some reason):

[MerlinEgalite]

I'm not 100% sure that it's better.

Btw do you any gas cost study on this change?

[Rubilmax]

I'm not 100% sure that it's better.

Btw do you any gas cost study on this change?

Beyond gas cost, it's just less lines of code for the same complexity. And I do mean the same complexity: if separated, there's is still the possibility to set a ERC4626 and no quote or base feed so it's really just the same mechanics. I don't get why we'd not do this multi-purpose oracle wrapper

[MerlinEgalite]

Beyond gas cost, it's just less lines of code for the same complexity. And I do mean the same complexity: if separated, there's is still the possibility to set a ERC4626 and no quote or base feed so it's really just the same mechanics. I don't get why we'd not do this multi-purpose oracle wrapper

It's at least more confusing for reviewers and auditors to me. So I still think it adds a bit of complexity for them.

I'm not against it though.

A gas cost study would have been nice to add least know what is the total cost of triggering the oracle compared to the "yaourt nature" version (without vaults and without multiple oracles).

[Jean-Grimal]

I'm in favor of this implementation (having all in one contract) but I think we should add a comment to explain that the vault parameter is related to the case where the collateral asset is a ERC4626 token.
Currently it's not clear enough imo

[MerlinEgalite]

Wait the difference is more than 10k ? I'm not sure how to interpret it 😅

[QGarchery]

Wait the difference is more than 10k ? I'm not sure how to interpret it 😅

I changed the description multiple times (won't do it anymore), so maybe you are referring to an older version. Keep in mind that we should compare the PRs, for the purpose of this choice.

As for why this is so costly to take into account the price of a vault, I'm assuming it is because the accounting warms multiple slots (things like totals shares, total assets, ...)

[MerlinEgalite]

I changed the description multiple times (won't do it anymore), so maybe you are referring to an older version. Keep in mind that we should compare the PRs, for the purpose of this choice.

As for why this is so costly to take into account the price of a vault, I'm assuming it is because the accounting warms multiple slots (things like totals shares, total assets, ...)

Ok now I understand it better!

[QGarchery]

Can't approve but LGTM