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

[N09] Naming issues #403

Closed
cylon56 opened this issue Jun 15, 2022 · 1 comment
Closed

[N09] Naming issues #403

cylon56 opened this issue Jun 15, 2022 · 1 comment
Labels
openzeppelin PRs and issues related to OZ audit

Comments

@cylon56
Copy link

cylon56 commented Jun 15, 2022

To favor explicitness and readability, several parts of the contracts may benefit from better naming. Here are some examples:

  • The name CometProxyAdmin suggests that it is only the admin of the Comet proxy, but in reality, it will also have the same role on the Configurator proxy. Choose another name to avoid confusion.
  • TransparentUpgradeableConfiguratorProxy can be called ConfiguratorProxy. There is no need to use the inherited contract name as a prefix.
  • In CometMath, change InvalidUInt to InvalidUint and toUInt to toUint.
  • rescale and descale have different names but the same value, consider using one variable name scale.
@kevincheng96 kevincheng96 added the openzeppelin PRs and issues related to OZ audit label Jun 15, 2022
@jflatow
Copy link
Contributor

jflatow commented Jul 1, 2022

There are some good points made about the chosen names, however naming is a notoriously hard problem and we believe it is difficult if not impossible to make everyone happy. We adopted some of the proposed changes in #433.

@jflatow jflatow closed this as completed Jul 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
openzeppelin PRs and issues related to OZ audit
Projects
None yet
Development

No branches or pull requests

3 participants