feat/enable disable flashloan config

[stevenvaleri]

Adds a configuration to enable and disable flashloans for a specific asset.

All existing assets will continue to have flashloans enabled. In the future this will allow for explicit errors when attempting to flashloan gho.

This is dependent on an update to the deploy repo. The update enables flashloans for all assets during the reserve configuration process.

[stevenvaleri]

Comments addressed - ready for re-review

[miguelmtzinf]

Missing a couple of test cases, but lgtm in general

[stevenvaleri]

Additional updates added - ready for re-review

• switch to using bit 63 in reserve configuration
• add reserve configuration unit tests
• minor test case updates

[kyzia551]

All existing assets will continue to have flashloans enabled. In the future this will allow for explicit errors when attempting to flashloan gho.

Hey @stevenvaleri ! How is it achieved in PR? Maybe I'm missing something, but by default this bit will be 0 and an asset will become flashloanable only when you set it to 1 (for existing deployments, I mean)

[kyzia551]

the same require missing in the validateFlashloanSimple in some lines above. Because of that looks like a good idea to reuse simple ie:

   ...
    require(assets.length == amounts.length, Errors.INCONSISTENT_FLASHLOAN_PARAMS);
    for (uint256 i = 0; i < assets.length; i++) {
      validateFlashloanSimple(reservesData[assets[i]].configuration);
    }

[miguelmtzinf]

Good catch. Let's add tests and evaluate this change!

[stevenvaleri]

Agreed - will update

[stevenvaleri]

All existing assets will continue to have flashloans enabled. In the future this will allow for explicit errors when attempting to flashloan gho.

Hey @stevenvaleri ! How is it achieved in PR? Maybe I'm missing something, but by default this bit will be 0 and an asset will become flashloanable only when you set it to 1 (for existing deployments, I mean)

@kyzia551 It's updated in the deploy repo. If you re-install dependencies in this repo, it will pull in beta version of the deploy repo with this update.

[kyzia551]

Why don't make it enabled by default? Because it sounds more like expected behaviour Best regards, Andrei

…

On Thu, 27 Oct 2022, 15:46 Steven Valeri, ***@***.***> wrote: All existing assets will continue to have flashloans enabled. In the future this will allow for explicit errors when attempting to flashloan gho. Hey @stevenvaleri <https://github.com/stevenvaleri> ! How is it achieved in PR? Maybe I'm missing something, but by default this bit will be 0 and an asset will become flashloanable only when you set it to 1 (for existing deployments, I mean) @kyzia551 <https://github.com/kyzia551> It's updated in the deploy repo. If you re-install dependencies in this repo, it will pull in beta version of the deploy repo with this update. — Reply to this email directly, view it on GitHub <#710 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AEFXTWXL4SYNRX7UCB2WU3LWFKBVDANCNFSM575K7BAQ> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[stevenvaleri]

I think it makes more sense to enable separately after the reserve is added - this would be consistent with making something borrowable, or enabled as collateral.

If you want an asset not flashloanable, but it is configured to be enabled by default, you'd need to remember to disable it in the same transaction when listing it, or we'd need to change the initReserve function to have a flag to override the default. I think it is less risky / prone to error to have all disabled by default and I don't think we'd consider updating the initReserve params.

[miguelmtzinf]

I think it makes more sense to enable separately after the reserve is added - this would be consistent with making something borrowable, or enabled as collateral.

If you want an asset not flashloanable, but it is configured to be enabled by default, you'd need to remember to disable it in the same transaction when listing it, or we'd need to change the initReserve function to have a flag to override the default. I think it is less risky / prone to error to have all disabled by default and I don't think we'd consider updating the initReserve params.

Totally agree. Having it enabled by default would lead to errors because breaks the consistency across the reserve configuration.