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

Remove Unnecessary Use of Integer Types in Prysm #9958

Closed
2 tasks
rauljordan opened this issue Dec 1, 2021 · 1 comment · Fixed by #10318
Closed
2 tasks

Remove Unnecessary Use of Integer Types in Prysm #9958

rauljordan opened this issue Dec 1, 2021 · 1 comment · Fixed by #10318
Assignees
Labels
Good First Issue Good for newcomers Help Wanted Extra attention is needed Priority: Medium Medium priority item Security Security Related Issues

Comments

@rauljordan
Copy link
Contributor

rauljordan commented Dec 1, 2021

💎 Issue

Background

Thank you @uncdr for bringing this to our attention. As Prysm is becoming a more complex project, we should place a lot of care into how we use primitive types. The danger of using integer types where they are not needed is problematic, as there could be future changes to the codebase that lead to bugs. Artem from our team has mentioned we should only use integers as a type when we really need signed numbers. Otherwise, everything should be unsigned and enforced by the compiler.

Description

This is a tracking issue to refactor all uses of integer types in Prysm into uints (either uint, uint16, 32, or 64) and audit them carefully. Ideally, this should be enforced by a static analyzer or some Github action script which checks for uses of integers, casting of values into integer types, and more. Wherever we need integers, which is rare, we should add exceptions, which can be done via comment pragmas, such as //ignore:int which can be parsed by the analyzer.

In the meantime, there a few low-hanging fruit we can take of, such as refactoring all uses of cliCtx.IntFlag into clicCtx.Uint64Flag for flag definitions of uint values.

TODOs

  • Create some kind of static analyzer or Github action which checks for uses of the int type in Prysm, running as part of every pull request
  • Refactor unnecessary uses of int into uint with the respective size, typically uint64
@rauljordan rauljordan added Good First Issue Good for newcomers Help Wanted Extra attention is needed Priority: Medium Medium priority item Security Security Related Issues labels Dec 1, 2021
@hmrtn
Copy link

hmrtn commented Feb 3, 2022

I'd like to work on this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Good First Issue Good for newcomers Help Wanted Extra attention is needed Priority: Medium Medium priority item Security Security Related Issues
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants