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

Replace many const with var #813

Closed
ethanfrey opened this issue Apr 25, 2022 · 2 comments · Fixed by #826
Closed

Replace many const with var #813

ethanfrey opened this issue Apr 25, 2022 · 2 comments · Fixed by #826
Milestone

Comments

@ethanfrey
Copy link
Member

This would address many of the points raised in #202 and also be a cleaner solution than fully removing checks as in #809

Using a var for these values will allow a chain importing wasmd (or compiling a custom version) to easily override the values compile time. It will not allow a param change proposal to fix it, but any chain upgrade could update these values as well, so it does make them somewhat dynamic (or at least adjustable by governance without forking wasmd).

This should be a minimally intrusive change that will remove a number of headaches for integrations.

@ethanfrey ethanfrey added this to the v0.27.0 milestone Apr 25, 2022
@alpe
Copy link
Member

alpe commented Apr 25, 2022

With the gas register interface and extension point, custom values or logic can be integrated easily. Switching to vars for them with compiler flags may cause more confusion as we have 2 ways now to set values.

MaxWasmSize and MaxLabelSize are a different story though. They are not customizable for chains, currently. Either we make them both vars or use a custom ante handler that can read from params.
The ante handler would be a cleaner solution for me as it removes duplication but adds on gas cost or requires caching.
Wondering if there is a use case for custom MaxLabelSize 🤷

@alpe alpe mentioned this issue Apr 25, 2022
4 tasks
@ethanfrey
Copy link
Member Author

Great. If it is just those two left to do, let's make them vars and document that can call this done.

I don't see the need to customize MaxLabelSize, but someone will surely come up with one. If we just make it var and keep the same default, it should be fine.

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

Successfully merging a pull request may close this issue.

2 participants