Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

store raw values in config #18

Open
sakulstra opened this issue Nov 1, 2023 · 1 comment
Open

store raw values in config #18

sakulstra opened this issue Nov 1, 2023 · 1 comment

Comments

@sakulstra
Copy link
Contributor

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:
#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.

@sakulstra sakulstra changed the title reformat config store raw values in config Nov 1, 2023
@reem
Copy link
Contributor

reem commented Nov 1, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants