Omniscia rain protocol Audit
ERC20TransferTier Code Style Findings
ERC20TransferTier Code Style Findings
ERT-01C: Optimized Value Usage
Type | Severity | Location |
---|---|---|
Gas Optimization | ERC20TransferTier.sol:L131 |
Description:
The linked safeTransfer
invocation transfers the value differece to the account_
which is guaranteed to be equal to the msg.sender
.
Example:
102// As _anyone_ can call `setTier` we require that `msg.sender` and103// `account_` are the same if the end tier is not an improvement.104// Anyone can increase anyone else's tier as the `msg.sender` is105// 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
Type | Severity | Location |
---|---|---|
Gas Optimization | ERC20TransferTier.sol:L127, L131 |
Description:
The linked subtractions are both enclosed in logical if
clauses that guarantee their safety and non-saturatedness.
Example:
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.