code-423n4 / 2021-10-pooltogether-findings

0 stars 0 forks source link

Style issues #52

Open code423n4 opened 3 years ago

code423n4 commented 3 years ago

Handle

pauliax

Vulnerability details

Impact

Style issues that you may want to apply or reject, no impact on security. Grouping them together as one submission to reduce waste. Consider fixing or ignoring them, up to you.

function controllerBurnFrom could also skip _approve decrease if the current approval is uint max (unlimited).
hardcoded number 1000 in PrizeSplit could be extracted to a constant variable to improve readability and maintainability.

asselstine commented 3 years ago

function controllerBurnFrom could also skip _approve decrease if the current approval is uint max (unlimited).

That doesn't make sense to me....you still want to decrease the allowance if their approval is max uint? It seems this suggestion re-interprets uint max as a "special value".

hardcoded number 1000 in PrizeSplit could be extracted to a constant variable to improve readability and maintainability.

Makes sense to me.

https://github.com/pooltogether/v4-core/pull/241

GalloDaSballo commented 3 years ago

The first statement may have been true at a time, for certain types of tokens These tokens wouldn't reduce the allowance if set to type(uint256).max

Removing magic numbers and instead using constants is best practice

GalloDaSballo commented 3 years ago

Sponsor has applied the change in a subsequent PR