ERC20Wrapper Manual Review Findings

ERC-01M: Improper Rounding of Initial Deposit

Type	Severity	Location
Mathematical Operations		ERC20Wrapper.sol:L117

Description:

The initial deposit for a wrapped EIP-20 asset will forcibly round down the shares minted to match the relevant decimals of the underlying HTS token.

In doing so, any surplus value provided will remain factored into the exchange rate of the token, thereby resulting in a drift from the one-to-one conversion rate accumulating.

Impact:

The system will forcibly truncate any surplus funds sent during the initial deposit of the system.

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);
121    shares -= MINIMUM_LIQUIDITY;
122} else {

Recommendation:

We advise the initial deposit in the system to be forcibly verified as being wholly divisible, preventing any truncation errors from manifesting.

Alleviation (6d473ba094):

The code was updated to utilize a fixed exchange rate between the underlying assets.

In the refactored implementation, the SaucerSwap Labs team has opted to simply emit a PrecisionLoss event if there is any precision loss in the exchange rate.

As the revised system still permits users to provide surplus funds, we advise a rescue function to be introduced that permits any tokens in the contract that do not back the HTS implementation to be withdrawn by an administrative member.

Alleviation (785446ee09):

The code was simplified further by truncating wrapping and unwrapping amounts to wholly divisible units, ensuring a user will supply the exact amount of funds necessary to back their wrapping operation rather than being able to provide a surplus.

As it is no longer possible to over-supply funds, we consider this exhibit fully alleviated.

ERC-02M: Incorrect Handling of Zero Supply in Conversions

Type	Severity	Location
Logical Fault		ERC20Wrapper.sol: • I-1: L293 • I-2: L305 • I-3: L317 • I-4: L329

Description:

All EIP-4626 style functions that rely on conversions between EIP-20 and HTS tokens will presently yield incorrect results when evaluating a yet-to-be-initialized HTS token due to assuming a one-to-one exchange rate between shares and assets in such a case.

Impact:

All estimation functions yield erroneous results as they do not account for initial deposits and the decimal conversion system that the ERC20Wrapper employs.

Example:

Copy
291function _convertToAssetsCeil(IHTS htsToken, IERC20 erc20Token, uint256 shares) internal view returns (uint256) {
292    uint256 supply = _totalSupply(htsToken);
293    return supply == 0 ? shares : shares.mulDiv(_totalAssets(erc20Token), supply, Math.Rounding.Ceil);
294}

Recommendation:

We advise the decimals of the EIP-20 token to be accounted for in the conversion mechanisms whenever no shares are detected in the system, to ensure a correct result is yielded to callers.

Alleviation (6d473ba09468c723cc379ee22cd29fba405bc94a):

The zero-value supply conversion mechanisms are no longer applicable in the revised one-to-one exchange mechanism.

ERC-03M: Inexplicable Share-Based System

Type	Severity	Location
Logical Fault		ERC20Wrapper.sol:L99-L134, L137-L161, L164-L185, L188-L206

Description:

The EIP-20 wrapper implementation will attach each EIP-20 instance to an HTS token using a share-based mechanism that can result in significant discrepancies such as improper exchange rates due to donation attacks.

Impact:

We consider the current design of the ERC20Wrapper contract to be redundantly over-engineered as a simpler one-to-one conversion rate will eliminate the MINIMUM_LIQUIDITY problem, will increase the system's usability, and will ultimately reduce gas costs across the board.

Example:

Copy
99function deposit(IERC20 erc20Token, uint256 assets, uint256 minCorrespondingAssets, address receiver)
100    external
101    override
102    nonReentrant
103    returns (uint256 shares)
104{
105    IHTS htsToken = _getCounterpart(erc20Token);
106
107    uint256 preBalance = erc20Token.balanceOf(address(this));
108    erc20Token.safeTransferFrom(msg.sender, address(this), assets);
109    uint256 postBalance = erc20Token.balanceOf(address(this));
110    assets = postBalance - preBalance;
111
112    uint256 supply = htsToken.totalSupply();
113    uint256 newSupply;
114    if (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);
121        shares -= MINIMUM_LIQUIDITY;
122    } else {
123        shares = assets.mulDiv(supply, preBalance);
124        require(shares != 0, "ZERO_SHARES");
125        newSupply = htsToken.mint(shares);
126    }
127
128    uint256 correspondingAssets = shares.mulDiv(postBalance, newSupply);
129    require(correspondingAssets >= minCorrespondingAssets, "SLIPPAGE_CHECK_FAIL");
130
131    htsToken.transfer(receiver, shares);
132
133    emit Deposit(erc20Token, msg.sender, receiver, assets, shares);
134}

Recommendation:

We advise the system to instead ensure ERC20Wrapper::deposit / ERC20Wrapper::mint operations provide wholly divisible numbers for wrapping, permitting the share system to be eliminated altogether and maintaining a constant and valid exchange rate between the EIP and HTS assets.

Alleviation (6d473ba094):

The system migrated to a one-to-one exchange rate mechanism, significantly reducing the logic involved in calculations whilst maintaining the core logic intact.

In the revised system, we have observed several inefficiencies as well as improper accounting of rounding errors. As all conversions have fixed outputs, we advise output slippage validation to be eliminated.

Additionally, the revised system will normalize EIP-20 tokens that have less than 18 decimals to 18 decimals. As such, precision loss can also occur on the HTS-to-EIP-20 conversion path which is not accounted for in the ERC20Wrapper::_convertHTSToERC20 function.

Alleviation (785446ee09):

All inefficiencies have been eliminated within the system as it was further simplified to perform a wrapping and unwrapping operation, permitting funds to be converted to their underlying wrapper counterpart in a straightforward manner.

Given that the revised version has minimized the codebase's footprint to the greatest extent possible, we consider this exhibit alleviated.