Inconsistent use of uint256 typecasts, either remove all or add all.

[ghost]

Description

I had originally believed that these typecasts were necessary. However, I was disproven in issues #9, #11, #13, #14.

There are nine unnecessary typecasts to uint256. If you would prefer to keep them, then there are four places where they are missing.

It would improve readability of code to either consistently include or remove these typecasts.

Scenario

It appears that typecasting smaller uint's into uint256 is unnecessary. In issues #9, #11, #13, #14, I pointed out four places where Offers.sol failed to typecast uint128 and uint64 variables into uint256. However, it was pointed out that these are unnecessary, because:

Such cast is not required as it's converting from a smaller magnitude to a bigger one

If this is so, then there are nine unnecessary uint256 typecast conversions in Offers.sol.

Impact

Low/Note:
(1) Better consistency/readability of code.
(2) I do not know if removing unnecessary typecasts in this circumstance saves any gas. If it does, there's an additional benefit there, but I do not think that it does.

Reproduction

The following nine lines have unnecessary typecasts up to uint256:

uint256 previousOfferTotal = uint256(previousOffer.total);
uint256 previousPriceForOwner = _computeOfferPrice(previousOfferTotal, uint256(previousOffer.offerCut));
uint256 total = uint256(offer.total);
uint256 offerCut = uint256(offer.offerCut);
uint256 total = uint256(offer.total);
uint256 offerCut = uint256(offer.offerCut);
uint256 cfoEarnings = uint256(offer.unsuccessfulFee);
uint256 toRefund = uint256(offer.total) - cfoEarnings;
uint256 total = uint256(offer.total);

If you would prefer to keep them, there are four missing typecasts to uint256:

uint256 expiresAt = offer.expiresAt;
uint256 cfoEarnings = previousOffer.unsuccessfulFee;
uint256 expiresAt = offer.expiresAt;
uint256 _offerCut = offerCut;

Fix

If you would prefer to remove the unnecessary uint256 typecasts, remove them like so:

uint256 previousOfferTotal = previousOffer.total;
uint256 previousPriceForOwner = _computeOfferPrice(previousOfferTotal, previousOffer.offerCut);
uint256 total = offer.total;
uint256 offerCut = offer.offerCut;
uint256 total = offer.total;
uint256 offerCut = offer.offerCut;
uint256 cfoEarnings = offer.unsuccessfulFee;
uint256 toRefund = offer.total - cfoEarnings;
uint256 total = offer.total;

If you would like to keep all uint256 typecasts where they are not strictly needed, add them like so:

uint256 expiresAt =  uint256(offer.expiresAt);
uint256 cfoEarnings =  uint256(previousOffer.unsuccessfulFee);
uint256 expiresAt =  uint256(offer.expiresAt);
uint256 _offerCut =  uint256(offerCut);

[hwrdtm]

Thanks @michaelKim4736 for your feedback! We will take it into consideration.

[arthcmr]

Thanks for your participation, @michaelKim4736! Our team has reviewed your submission, and we are pleased to reward you for your report.

Severity: Note
Points: 10