Omniscia Tangible Audit

RebaseTokenUpgradeable Code Style Findings

RebaseTokenUpgradeable Code Style Findings

RTU-01C: Improper Evaluation of Assert

Description:

The RebaseTokenUpgradeable::_checkRebaseOverflow function will evaluate an assert statement that can never evaluate to false.

Example:

src/RebaseTokenUpgradeable.sol
241function _checkRebaseOverflow(uint256 shares, uint256 index) private view {
242    // The condition inside `assert()` can never evaluate `false`, but `toTokens()` would throw an arithmetic
243    // exception in case we overflow, and that's all we need.
244    assert(shares.toTokens(index) + ERC20Upgradeable.totalSupply() <= type(uint256).max);
245}

Recommendation:

While this is detailed in the assert statement's comments, we still advise the code to be wrapped in an unchecked code block and to instead ensure that the addition between shares.toTokens(index) and ERC20Upgradeable::totalSupply does not overflow thus ensuring that no code is unreachable.

Alleviation (47fbf62bbbf2409ff0baf9a18a2945466cb2a576):

The assert statement was converted to an if-revert construct that properly performs the addition overflow check in an unchecked code block, addressing this exhibit.

RTU-02C: Redundant Conditional Clause

Description:

The RebaseTokenUpgradeable::_disableRebase function is solely invoked within the USTB token's disable rebase function whereby the new disable state of the account is guaranteed to be different.

Example:

src/RebaseTokenUpgradeable.sol
76function _disableRebase(address account, bool disable) internal {
77    RebaseTokenStorage storage $ = _getRebaseTokenStorage();
78    if ($.optOut[account] != disable) {
79        uint256 balance = balanceOf(account);
80        $.optOut[account] = disable;
81        if (balance != 0) {
82            if (disable) {
83                RebaseTokenUpgradeable._update(account, address(0), balance);
84                ERC20Upgradeable._update(address(0), account, balance);
85            } else {
86                ERC20Upgradeable._update(account, address(0), balance);
87                RebaseTokenUpgradeable._update(address(0), account, balance);
88            }
89        }
90        if (disable) emit RebaseDisabled(account);
91        else emit RebaseEnabled(account);
92    }
93}

Recommendation:

We advise the referenced conditional to be omitted from the codebase, optimizing the gas cost of the function.

Alleviation (47fbf62bbbf2409ff0baf9a18a2945466cb2a576):

The Tangible team evaluated this exhibit but opted to retain its current behaviour in place as the RebaseTokenUpgradeable::_disableRebase function may be invoked outside the context of USTB::disableRebase in derivative implementations.

As such, we consider this exhibit nullified given that the purpose of the RebaseTokenUpgradeable implementation is meant to be generic and inherited by other contracts outside the Tangible ecosystem.

RTU-03C: Redundant Representation of Literal Constant

Description:

The RebaseTokenStorageLocation variable is declared as a bytes32 literal instead of utilizing its commented-out representation.

Example:

src/RebaseTokenUpgradeable.sol
34// keccak256(abi.encode(uint256(keccak256("tangible.storage.RebaseToken")) - 1)) & ~bytes32(uint256(0xff))
35bytes32 private constant RebaseTokenStorageLocation =
36    0x8a0c9d8ec1d9f8b365393c36404b40a33f47675e34246a2e186fbefd5ecd3b00;

Recommendation:

We advise the commented-out representation to replace the value literal, ensuring that the value can be easily maintained while ensuring that no external verification of equivalence must be performed.

To note, it is possible to perform literal-based calculations in constant declarations and keccak256 evaluations of string values are correctly evaluated during compile-time.

Alleviation (47fbf62bbbf2409ff0baf9a18a2945466cb2a576):

The Tangible team has specified that the representation they currently utilize is aligned with best-practices as established by the likes of OpenZeppelin.

The OpenZeppelin standard adheres to the EIP-7201 specification and as such, we advised the codebase of the Tangible team to also align with the standard by introducing the relevant custom annotations described within it to the struct entries of the codebase.

The Tangible team evaluated our recommendation and opted to not introduce the notation as they wish to utilize external toolkits to evaluate each storage slot's validity.

To this end, we consider this exhibit safely acknowledged as the Tangible team has been informed of the relevant standards and has made an informed decision to retain the codebase as is.

Prev

OFTCoreUpgradeable.sol (OFC-C)

Next

Finding Types

On this page