aave-v3-core icon indicating copy to clipboard operation
aave-v3-core copied to clipboard

feat/enable disable flashloan config

Open stevenvaleri opened this issue 2 years ago • 1 comments

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 avatar Aug 29 '22 11:08 stevenvaleri

Comments addressed - ready for re-review

stevenvaleri avatar Oct 18 '22 18:10 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

stevenvaleri avatar Oct 25 '22 14:10 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 avatar Oct 27 '22 12:10 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 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.

stevenvaleri avatar Oct 27 '22 13:10 stevenvaleri

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 https://github.com/aave/aave-v3-core/pull/710#issuecomment-1293551917, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEFXTWXL4SYNRX7UCB2WU3LWFKBVDANCNFSM575K7BAQ . You are receiving this because you were mentioned.Message ID: @.***>

kyzia551 avatar Oct 27 '22 14:10 kyzia551

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.

stevenvaleri avatar Oct 27 '22 16:10 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.

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

miguelmtzinf avatar Oct 27 '22 16:10 miguelmtzinf