search

bgd-labs / aave-proposals-v3

MIT License
22 stars 24 forks source link

store raw values in config #18

Open sakulstra opened 1 year ago

sakulstra commented 1 year ago

One thing i noticed is quite suboptimal with configs: When i initially created the generator, the goal was simple: ask some questions -> generate code but now as we added the config.json output(because gauntlet requested it and seems useful) idk if some old design decisions make sense

In the case of ACI proposal for example: https://github.com/bgd-labs/aave-proposals-v3/issues/3 they share the config.json, but if you would now use this same config.json with all issues patched, still wrong code would be generated. The reason is that the json does not store "raw" values, but already stores the formatted output.

"eModeCategory": "AaveV3EthereumEModes.NONE",
"optimalUtilizationRate": "_bpsToRay(45_00)",
"enabledToBorrow": "ENABLED",

So patches to the formatter don't have any effect.

I think make sense to remove the "automatic formating" inside the cli step and instead store the "raw value". The formatting should probably only happen on the build step.

reem commented 1 year ago

It can be helpful to store these values in the "human readable" format as the raw format like is already done in the diffs files. So in this case something like:

"eModeCategory": null,
"optimalUtilizationRte": "45.00",
"enabledToBorrow": true

This makes it a lot easier to review the config directly (as well as generate it with other tools which then don't have to implement their own serialization logic) and makes it easier to change the internal workings of the generator or the contracts later on.

For most proposals that can be done entirely from the config, I think it could actually be better not to commit the generated files at all and instead just generate on demand, this vastly reduces the surface area for reviews. Most proposals can be modeled as just plain data updates, no need for any custom code to review at all.

For custom proposals I agree it's not very helpful to actually include the config since it's not going to correspond with the proposal anyway, but we can also try to reduce the number of custom proposals over time by adding more features to the generator such as payment proposals.