Omniscia rain protocol Audit

ERC20TransferTier Code Style Findings

ERC20TransferTier Code Style Findings

ERT-01C: Optimized Value Usage

Description:

The linked safeTransfer invocation transfers the value differece to the account_ which is guaranteed to be equal to the msg.sender.

Example:

contracts/tier/ERC20TransferTier.sol
102// As _anyone_ can call `setTier` we require that `msg.sender` and
103// `account_` are the same if the end tier is not an improvement.
104// Anyone can increase anyone else's tier as the `msg.sender` is
105// responsible to pay the difference.
106if (endTier_ <= startTier_) {
107 require(msg.sender == account_, "DELEGATED_TIER_LOSS");
108}
109
110uint256[8] memory tierValues_ = tierValues();
111
112// Handle the erc20 transfer.
113// Convert the start tier to an erc20 amount.
114uint256 startValue_ = tierToValue(tierValues_, startTier_);
115// Convert the end tier to an erc20 amount.
116uint256 endValue_ = tierToValue(tierValues_, endTier_);
117
118// Short circuit if the values are the same for both tiers.
119if (endValue_ == startValue_) {
120 return;
121}
122if (endValue_ > startValue_) {
123 // Going up, take ownership of erc20 from the `msg.sender`.
124 erc20.safeTransferFrom(
125 msg.sender,
126 address(this),
127 endValue_.saturatingSub(startValue_)
128 );
129} else {
130 // Going down, process a refund for the tiered account.
131 erc20.safeTransfer(account_, startValue_.saturatingSub(endValue_));
132}

Recommendation:

We advise the function to utilize msg.sender instead as it is more gas efficient than reading a local variable.

Alleviation:

The code now properly utilizes the msg.sender instead of the account_ in the transfer invocation.

ERT-02C: Redundant Usage of Saturating Subtractions

Description:

The linked subtractions are both enclosed in logical if clauses that guarantee their safety and non-saturatedness.

Example:

contracts/tier/ERC20TransferTier.sol
118// Short circuit if the values are the same for both tiers.
119if (endValue_ == startValue_) {
120 return;
121}
122if (endValue_ > startValue_) {
123 // Going up, take ownership of erc20 from the `msg.sender`.
124 erc20.safeTransferFrom(
125 msg.sender,
126 address(this),
127 endValue_.saturatingSub(startValue_)
128 );
129} else {
130 // Going down, process a refund for the tiered account.
131 erc20.safeTransfer(account_, startValue_.saturatingSub(endValue_));
132}

Recommendation:

We advise them to be performed normally (i.e. with the subtraction - character) and we advise them to be set in unchecked code blocks as a further gas optimization.

Alleviation:

The saturatingSub instructions were properly replaced by simple subtractions (-) and wrapped in an unchecked code block further optimizing the gas use of the function.