ERC20Wrapper Code Style Findings

ERC-01C: Ineffectual Usage of Safe Arithmetics

Type	Severity	Location
Language Specific		ERC20Wrapper.sol: • I-1: L116 • I-2: L121 • I-3: L388

Description:

The linked mathematical operations are guaranteed to be performed safely by surrounding conditionals evaluated in either require checks or if-else constructs.

Example:

Copy
116uint256 decimalDiff = erc20Decimals > MAX_DECIMALS ? erc20Decimals - MAX_DECIMALS : 0;

Recommendation:

Given that safe arithmetics are toggled on by default in pragma versions of 0.8.X, we advise the linked statements to be wrapped in unchecked code blocks thereby optimizing their execution cost.

Alleviation (785446ee095db8d42f50665408df09339b4513d8):

While the code has been refactored to a significant degree, this particular optimization is still applicable.

The SaucerSwap Labs team has opted to acknowledge it as they do not wish to apply it to their current iteration of the codebase.

ERC-02C: Inefficient Assignment of Revised Allowance

Type	Severity	Location
Gas Optimization		ERC20Wrapper.sol:L388

Description:

The referenced statement will perform a subtraction-and-assignment (-=) even though the allowed entry has already been loaded into memory.

Example:

Copy
385uint256 allowed = withdrawalAllowances[token][owner][msg.sender];
386if (allowed != type(uint256).max) {
387    require(allowed >= shares, "INSUFFICIENT_WITHDRAWAL_ALLOWANCE");
388    withdrawalAllowances[token][owner][msg.sender] -= shares;
389}

Recommendation:

We advise the code to instead assign the result of allowed - shares, optimizing its gas cost.

Alleviation (785446ee095db8d42f50665408df09339b4513d8):

The codebase has undergone a significant refactor rendering this exhibit no longer applicable.

ERC-03C: Inefficient Contract Existence Checks

Type	Severity	Location
Gas Optimization		ERC20Wrapper.sol: • I-1: L212 • I-2: L218

Description:

The referenced checks are meant to ensure that the input token has been enrolled in the system, however, they will inefficiently fetch the data entry from the internal call as well.

Example:

Copy
211function totalAssets(IERC20 token) external view override nonReentrantBefore returns (uint256) {
212    _getCounterpart(token);
213    return _totalAssets(token);
214}

Recommendation:

We advise the relevant mapping entries to be validated directly, optimizing the code's gas cost.

Alleviation (785446ee095db8d42f50665408df09339b4513d8):

The codebase has undergone a significant refactor rendering this exhibit no longer applicable.

ERC-04C: Inefficient mapping Lookups

Type	Severity	Location
Gas Optimization		ERC20Wrapper.sol:L385, L388

Description:

The linked statements perform key-based lookup operations on mapping declarations from storage multiple times for the same key redundantly.

Example:

Copy
382function _spendWithdrawalAllowance(IHTS token, address owner, uint256 shares) internal {
383    if (owner != msg.sender) {
384        if (!withdrawalOperators[owner][msg.sender]) {
385            uint256 allowed = withdrawalAllowances[token][owner][msg.sender];
386            if (allowed != type(uint256).max) {
387                require(allowed >= shares, "INSUFFICIENT_WITHDRAWAL_ALLOWANCE");
388                withdrawalAllowances[token][owner][msg.sender] -= shares;
389            }
390        }
391    }
392}

Recommendation:

As the lookups internally perform an expensive keccak256 operation, we advise the lookups to be cached wherever possible to a single local declaration that either holds the value of the mapping in case of primitive types or holds a storage pointer to the struct contained.

As the compiler's optimizations may take care of these caching operations automatically at-times, we advise the optimization to be selectively applied, tested, and then fully adopted to ensure that the proposed caching model indeed leads to a reduction in gas costs.

Alleviation (785446ee095db8d42f50665408df09339b4513d8):

The codebase has undergone a significant refactor rendering this exhibit no longer applicable.

ERC-05C: Misleading Invariant Conditional

Type	Severity	Location
Code Style		ERC20Wrapper.sol:L120

Description:

The referenced conditional is meant to evaluate whether newSupply equates the shares to be minted, however, it is structured as newSupply - shares == supply which is misleading.

Example:

Copy
114if (supply == 0) {
115    uint256 erc20Decimals = IERC20Metadata(address(erc20Token)).decimals();
116    uint256 decimalDiff = erc20Decimals > MAX_DECIMALS ? erc20Decimals - MAX_DECIMALS : 0;
117    shares = assets / 10 ** (decimalDiff);
118    require(shares >= MINIMUM_LIQUIDITY, "INSUFFICIENT_SHARES");
119    newSupply = htsToken.mint(shares);
120    assert(newSupply - shares == supply);

Recommendation:

We advise the assert check to be adjusted as advised, ensuring it properly illustrates what it is meant to enforce.

Alleviation (6d473ba09468c723cc379ee22cd29fba405bc94a):

The referenced optimization is no longer applicable.