search

ethereum / consensus-specs

Ethereum Proof-of-Stake Consensus Specifications
Creative Commons Zero v1.0 Universal
3.56k stars 971 forks source link

EIP4844 : Remove mathematical/one-word variables from the public APIs in cryptography module #3160

Open kevaundray opened 1 year ago

kevaundray commented 1 year ago

Motivation

Due to cryptography being closely tied to mathematics, there is sometimes a tendency to use one word variables like z, x, y for conciseness. In and of itself, its not a bad thing, however since code being written usually copies this verbatim, it makes the code harder to read and maintain.

Suggestion

Remove one word variables from the public API of cryptography modules. ie anywhere a non-cryptographer needs to read the code, there should be descriptive words to describe the meaning of arguments.

Reference

Example of this is z and y used here which propagates to the precompile

asn-d6 commented 1 year ago

I respect the sentiment and I think it's a reasonable suggestion. I also think moderation is key, especially when it comes to establishing new codebase rules that might be annoying to enforce in the future.

If you submit a PR, I will gladly review and merge, since the cases of such single-letter variables on the public API are so few and worth improving!

However, if we ever want to expand this "rule" to the private API as well, the patch will grow quite a bit.

kevaundray commented 1 year ago

Yeah I was contemplating this rule for the private API, however the change is larger and has less tangible benefits than for the public API