Skip to content

fix: remove use of magic numbers in __ProvisionManager_init_unchained function (OZ N-11) #1041

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.

Sign up for GitHub

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

Merged
merged 8 commits into from
Oct 1, 2024
Merged

fix: remove use of magic numbers in __ProvisionManager_init_unchained function (OZ N-11) #1041

Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
ffe9b89
fix: address magic number use. (OZ N-11)
MoonBoi9001 Sep 18, 2024
76e4940
fix: update constant names. (OZ N-11)
MoonBoi9001 Sep 20, 2024
8912125
Update packages/horizon/contracts/data-service/utilities/ProvisionMan…
MoonBoi9001 Sep 20, 2024
2729a37
fix: use new constants in subgraphservice.sol
MoonBoi9001 Sep 30, 2024
f8e39df
Merge branch 'horizon' into fix_oz_n-11
MoonBoi9001 Sep 30, 2024
a598eca
fix: set all ProvisionManager constants internal.
MoonBoi9001 Oct 1, 2024
de6c822
fix: remove ProvisionManager inheritance.
MoonBoi9001 Oct 1, 2024
ffd1f79
fix: remove whitespace.
MoonBoi9001 Oct 1, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 11 additions & 3 deletions packages/horizon/contracts/data-service/utilities/ProvisionManager.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,14 @@ import { ProvisionManagerV1Storage } from "./ProvisionManagerStorage.sol";
abstract contract ProvisionManager is Initializable, GraphDirectory, ProvisionManagerV1Storage {
using UintRange for uint256;

// Constants
uint32 private constant DEFAULT_MIN_UINT32 = 0;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think better names for these would be tied to the range they are describing. For example, instead of DEFAULT_MIN_UINT256 and DEFAULT_MAX_UINT256 you would have DEFAULT_MIN_PROVISION_TOKENS and DEFAULT_MAX_PROVISION_TOKENS and so forth.

uint64 private constant DEFAULT_MIN_UINT64 = 0;
uint256 private constant DEFAULT_MIN_UINT256 = 0;
uint64 private constant DEFAULT_MAX_UINT64 = type(uint64).max;
uint256 private constant DEFAULT_MAX_UINT256 = type(uint256).max;
uint32 private constant MAX_PPM = uint32(PPMMath.MAX_PPM);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here I would use DEFAULT_MAX_VERIFIER_CUT


/**
* @notice Emitted when the provision tokens range is set.
* @param min The minimum allowed value for the provision tokens.
Expand Down Expand Up @@ -115,9 +123,9 @@ abstract contract ProvisionManager is Initializable, GraphDirectory, ProvisionMa
*/
// solhint-disable-next-line func-name-mixedcase
function __ProvisionManager_init_unchained() internal onlyInitializing {
_setProvisionTokensRange(type(uint256).min, type(uint256).max);
_setVerifierCutRange(type(uint32).min, uint32(PPMMath.MAX_PPM));
_setThawingPeriodRange(type(uint64).min, type(uint64).max);
_setProvisionTokensRange(DEFAULT_MIN_UINT256, DEFAULT_MAX_UINT256);
_setVerifierCutRange(DEFAULT_MIN_UINT32, MAX_PPM);
_setThawingPeriodRange(DEFAULT_MIN_UINT64, DEFAULT_MAX_UINT64);
}

/**
Expand Down