Smart Contract Audit Report

SolidInspect: Static Analyzer Version: 1.0.0​

# # # # # # # # # #  
#    _.._..,_,_   #
#   (          )  #
#    ]~,"-.-~~[   #
#  .=])' (;  ([   #
#  | ]:: '    [   #
#  '=]): .)  ([   #
#    |:: '    |   #
#     ~~----~~    #
# # # # # # # # # #  

by: Kaan Caglan​

Audit Summary

Audit Date: 2023-11-26​

The static analysis has been performed to ensure adherence to established coding standards, identify security vulnerabilities, and target areas for optimization and improved code cleanliness. This automated assessment is pivotal in upholding high standards of code quality and dependability throughout the software development process.

Findings

Medium Risk Issues​

Total: 2 instances over 2 issues.

Low Risk Issues​

Total: 18 instances over 10 issues.

Gas Optimizations​

Total: 33 instances over 20 issues.

Non-Critical Issues​

Total: 62 instances over 21 issues.

Medium Risk Issues​

[M-01] Ownership Irrevocability Vulnerability​

Severity​

• Impact: Medium
• Likelihood: Low

Description​

The smart contract under inspection inherits from the Ownable library, which provides basic authorization control functions, simplifying the implementation of user permissions. The contract in question allows the owner to adjust parameters such as feeOwnerPercentageBuy. However, the contract does not provide a mechanism to transfer ownership to another address or account, and it retains the default renounceOwnership function from Ownable. Given this, once the owner renounces ownership using the renounceOwnership function, the contract becomes ownerless. As evidenced in the provided transaction logs, after the renounceOwnership function is called, attempts to call functions that require owner permissions fail with the error message: "Ownable: caller is not the owner." This state renders the contract's adjustable parameters immutable and potentially makes the contract useless for any future administrative changes that might be necessary.

Number Of Instances Found​

1

Code Location​

Path: ./contracts/ERC20MultiDelegate.sol

25:contract ERC20MultiDelegate is ERC1155, Ownable {    // @audit-issue

[M-02] Centralization risk for privileged functions​

Severity​

• Impact: Medium
• Likelihood: Low

Description​

Contracts with privileged functions need owner/admin to be trusted not to perform malicious updates or drain funds. This may also cause a single point failure.

Number Of Instances Found​

1

Code Location​

Path: ./contracts/ERC20MultiDelegate.sol

151:    function setUri(string memory uri) external onlyOwner { // @audit-issue

Low Risk Issues​

[L-01] Missing checks for address(0) in constructor/initializers​

Severity​

• Impact: Medium
• Likelihood: Low

Description​

In Solidity, the Ethereum address 0x0000000000000000000000000000000000000000 is known as the "zero address". This address has significance because it's the default value for uninitialized address variables and is often used to represent an invalid or non-existent address. The " Missing zero address control" issue arises when a Solidity smart contract does not properly check or prevent interactions with the zero address, leading to unintended behavior. For instance, a contract might allow tokens to be sent to the zero address without any checks, which essentially burns those tokens as they become irretrievable. While sometimes this is intentional, without proper control or checks, accidental transfers could occur.

Number Of Instances Found​

1

Code Location​

Path: ./contracts/ERC20MultiDelegate.sol

48:        token = _token;  // @audit-issue

[L-02] Floating Pragma​

Severity​

• Impact: Low
• Likelihood: Medium

Description​

A "floating pragma" in Solidity refers to the practice of using a pragma statement that does not specify a fixed compiler version but instead allows the contract to be compiled with any compatible compiler version. This issue arises when pragma statements like pragma solidity ^0.8.0; are used without a specific version number, allowing the contract to be compiled with the latest available compiler version. This can lead to various compatibility and stability issues.

Number Of Instances Found​

1

Code Location​

Path: ./contracts/ERC20MultiDelegate.sol

2:pragma solidity ^0.8.2;   // @audit-issue

[L-03] Use Ownable2Step rather than Ownable​

Severity​

• Impact: Low
• Likelihood: Medium

Description​

Ownable2Step and Ownable2StepUpgradeable prevent the contract ownership from mistakenly being transferred to an address that cannot handle it (e.g. due to a typo in the address), by requiring that the recipient of the owner permissions actively accept via a contract call of its own.

Number Of Instances Found​

1

Code Location​

Path: ./contracts/ERC20MultiDelegate.sol

25:contract ERC20MultiDelegate is ERC1155, Ownable {    // @audit-issue

[L-04] Avoid Double casting​

Severity​

• Impact: Low
• Likelihood: Low

Description​

Consider refactoring the following code, as double casting may introduce unexpected truncations and/or rounding issues.

Furthermore, double type casting can make the code less readable and harder to maintain, increasing the likelihood of errors and misunderstandings during development and debugging.

Number Of Instances Found​

2

Code Location​

Path: ./contracts/ERC20MultiDelegate.sol

195:        return ERC1155(this).balanceOf(msg.sender, uint256(uint160(delegate))); // @audit-issue

214:        return address(uint160(uint256(hash))); // @audit-issue

[L-05] Consider using OpenZeppelin’s SafeCast library to prevent unexpected overflows when casting from various type int/uint values​

Severity​

• Impact: Low
• Likelihood: Low

Description​

In Solidity, when casting from int to uint or vice versa, there is a risk of unexpected overflow or underflow, especially when dealing with large values. To mitigate this risk and ensure safe type conversions, you can consider using OpenZeppelin’s SafeCast library. This library provides functions that check for overflow/underflow conditions before performing the cast, helping you prevent potential issues related to type conversion in your smart contracts.

Number Of Instances Found​

5

Code Location​

Path: ./contracts/ERC20MultiDelegate.sol

91:                ? address(uint160(sources[transferIndex]))   // @audit-issue

94:                ? address(uint160(targets[transferIndex]))   // @audit-issue

195:        return ERC1155(this).balanceOf(msg.sender, uint256(uint160(delegate))); // @audit-issue

210:                uint256(0), // salt // @audit-issue

214:        return address(uint160(uint256(hash))); // @audit-issue

[L-06] Numbers downcast to addresses may result in collisions​

Severity​

• Impact: Low
• Likelihood: Low

Description​

If a number is downcast to an address the upper bytes are truncated, which may mean that more than one value will map to the address

Number Of Instances Found​

3

Code Location​

Path: ./contracts/ERC20MultiDelegate.sol

91:                ? address(uint160(sources[transferIndex]))   // @audit-issue

94:                ? address(uint160(targets[transferIndex]))   // @audit-issue

214:        return address(uint160(uint256(hash))); // @audit-issue

[L-07] External calls in an unbounded loop can result in a DoS​

Severity​

• Impact: Low
• Likelihood: Low

Description​

Consider limiting the number of iterations in loops that make external calls, as just a single one of them failing will result in a revert.

Number Of Instances Found​

2

Code Location​

Path: ./contracts/ERC20MultiDelegate.sol

98:            if (transferIndex < Math.min(sourcesLength, targetsLength)) {    // @audit-issue

87:            transferIndex < Math.max(sourcesLength, targetsLength);  // @audit-issue

[L-08] Solidity version 0.8.20 might not work on all chains due to PUSH0​

Severity​

• Impact: Low
• Likelihood: Low

Description​

Solidity version 0.8.20 employs the recently introduced PUSH0 opcode in the Shanghai EVM, this opcode might not be universally supported across all blockchain networks and Layer 2 solutions.It is advisable to use an earlier version of solidity to ensure compatibility.

Number Of Instances Found​

1

Code Location​

Path: ./contracts/ERC20MultiDelegate.sol

2:pragma solidity ^0.8.2;   // @audit-issue

[L-09] abi.encodePacked() should not be used with dynamic types when passing the result to a hash function such as keccak256()​

Severity​

• Impact: Low
• Likelihood: Low

Description​

Use abi.encode() instead which will pad items to 32 bytes, which will prevent hash collisions (e.g. abi.encodePacked(0x123,0x456) => 0x123456 => abi.encodePacked(0x1,0x23456), but abi.encode(0x123,0x456) => 0x0...1230...456). "Unless there is a compelling reason, abi.encode should be preferred". If there is only one argument to abi.encodePacked() it can often be cast to bytes() or bytes32() instead. If all arguments are strings and or bytes, bytes.concat() should be used instead

Number Of Instances Found​

1

Code Location​

Path: ./contracts/ERC20MultiDelegate.sol

206:        bytes32 hash = keccak256(
207:            abi.encodePacked(   // @audit-issue
208:                bytes1(0xff),
209:                address(this),
210:                uint256(0), // salt
211:                keccak256(bytecode)
212:            )
213:        );

[L-10] Vulnerable versions of packages are being used​

Severity​

• Impact: Low
• Likelihood: Medium

Description​

This project is using specific package versions which are vulnerable to the specific CVEs listed below. Consider switching to more recent versions of these packages that don't have these vulnerabilities.

• CVE-2022-39384 - MEDIUM - (openzeppelin/contracts >=3.2.0 <4.4.1): OpenZeppelin Contracts is a library for secure smart contract development. Before version 4.4.1 but after 3.2.0, initializer functions that are invoked separate from contract creation (the most prominent example being minimal proxies) may be reentered if they make an untrusted non-view external call. Once an initializer has finished running it can never be re-executed. However, an exception put in place to support multiple inheritance made reentrancy possible in the scenario described above, breaking the expectation that there is a single execution. Note that upgradeable proxies are commonly initialized together with contract creation, where reentrancy is not feasible, so the impact of this issue is believed to be minor. This issue has been patched, please upgrade to version 4.4.1. As a workaround, avoid untrusted external calls during initialization.

• CVE-2022-35961 - MEDIUM - (openzeppelin/contracts >=4.1.0 <4.7.3): OpenZeppelin Contracts is a library for secure smart contract development. The functions ECDSA.recover and ECDSA.tryRecover are vulnerable to a kind of signature malleability due to accepting EIP-2098 compact signatures in addition to the traditional 65 byte signature format. This is only an issue for the functions that take a single bytes argument, and not the functions that take r, v, s or r, vs as separate arguments. The potentially affected contracts are those that implement signature reuse or replay protection by marking the signature itself as used rather than the signed message or a nonce included in it. A user may take a signature that has already been submitted, submit it again in a different form, and bypass this protection. The issue has been patched in 4.7.3.

• CVE-2022-35915 - MEDIUM - (openzeppelin/contracts >=2.0.0 <4.7.2): OpenZeppelin Contracts is a library for secure smart contract development. The target contract of an EIP-165 supportsInterface query can cause unbounded gas consumption by returning a lot of data, while it is generally assumed that this operation has a bounded cost. The issue has been fixed in v4.7.2. Users are advised to upgrade. There are no known workarounds for this issue.

• CVE-2022-31198 - HIGH - (openzeppelin/contracts >=4.3.0 <4.7.2): OpenZeppelin Contracts is a library for secure smart contract development. This issue concerns instances of Governor that use the module GovernorVotesQuorumFraction, a mechanism that determines quorum requirements as a percentage of the voting token's total supply. In affected instances, when a proposal is passed to lower the quorum requirements, past proposals may become executable if they had been defeated only due to lack of quorum, and the number of votes it received meets the new quorum requirement. Analysis of instances on chain found only one proposal that met this condition, and we are actively monitoring for new occurrences of this particular issue. This issue has been patched in v4.7.2. Users are advised to upgrade. Users unable to upgrade should consider avoiding lowering quorum requirements if a past proposal was defeated for lack of quorum.

• CVE-2022-31172 - HIGH - (openzeppelin/contracts >=4.1.0 <4.7.1): OpenZeppelin Contracts is a library for smart contract development. Versions 4.1.0 until 4.7.1 are vulnerable to the SignatureChecker reverting. SignatureChecker.isValidSignatureNow is not expected to revert. However, an incorrect assumption about Solidity 0.8's abi.decode allows some cases to revert, given a target contract that doesn't implement EIP-1271 as expected. The contracts that may be affected are those that use SignatureChecker to check the validity of a signature and handle invalid signatures in a way other than reverting. The issue was patched in version 4.7.1.

• CVE-2022-31170 - HIGH - (openzeppelin/contracts >=4.0.0 <4.7.1): OpenZeppelin Contracts is a library for smart contract development. Versions 4.0.0 until 4.7.1 are vulnerable to ERC165Checker reverting instead of returning false. ERC165Checker.supportsInterface is designed to always successfully return a boolean, and under no circumstance revert. However, an incorrect assumption about Solidity 0.8's abi.decode allows some cases to revert, given a target contract that doesn't implement EIP-165 as expected, specifically if it returns a value other than 0 or 1. The contracts that may be affected are those that use ERC165Checker to check for support for an interface and then handle the lack of support in a way other than reverting. The issue was patched in version 4.7.1.

Number Of Instances Found​

1

Code Location​

/// @audit Global finding.

GitHub: 0

Gas Optimizations​

[G-01] Unneeded initializations of uint256 and bool variable to 0/false.​

Severity​

• Impact: Low
• Likelihood: Low

Description​

In Solidity, it is common practice to initialize variables with default values when declaring them. However, initializing uint256 variables to 0 and boolvariables to false when they are not subsequently used in the code can lead to unnecessary gas consumption and code clutter. This issue points out instances where such initializations are present but serve no functional purpose.

Number Of Instances Found​

1

Code Location​

Path: ./contracts/ERC20MultiDelegate.sol

86:            uint transferIndex = 0;  // @audit-issue

[G-02] Unnecessary Assert​

Severity​

• Impact: Low
• Likelihood: Low

Description​

Unnecessary assert statements in Solidity contracts can have several detrimental impacts on code readability, gas efficiency, and overall contract functionality. assert statements are typically used to check for conditions that should never occur under normal circumstances. While they can be valuable for catching unexpected errors and halting contract execution, their misuse can lead to unnecessary complexity and increased gas costs.

Number Of Instances Found​

1

Code Location​

Path: ./contracts/ERC20MultiDelegate.sol

131:        assert(amount <= balance);  // @audit-issue

[G-03] Using Prefix Operators Costs Less Gas Than Postfix Operators in Loops​

Severity​

• Impact: Low
• Likelihood: Low

Description​

Conditions can be optimized issues in Solidity refer to situations where smart contract developers write conditional statements that can be simplified or optimized for better gas efficiency, readability, and maintainability. Optimizing conditions can lead to more cost-effective and secure smart contracts.

Number Of Instances Found​

1

Code Location​

Path: ./contracts/ERC20MultiDelegate.sol

88:            transferIndex++  // @audit-issue

[G-04] Reduce Gas Usage by Moving to Solidity 0.8.19 or Later​

Severity​

• Impact: Low
• Likelihood: Low

Description​

This issue highlights the opportunity to reduce gas consumption in a smart contract by upgrading the Solidity compiler version to 0.8.19 or a later release. Gas usage is a critical consideration in Ethereum smart contracts, as it directly affects transaction costs and contract execution efficiency. Newer compiler versions often come with optimizations and improvements that can lead to reduced gas costs for certain operations and contract structures.

Number Of Instances Found​

1

Code Location​

Path: ./contracts/ERC20MultiDelegate.sol

2:pragma solidity ^0.8.2;   // @audit-issue

[G-05] Constructor Can Be Marked As Payable​

Severity​

• Impact: Low
• Likelihood: Low

Description​

payable functions cost less gas to execute, since the compiler does not have to add extra checks to ensure that a payment wasn't provided.

A constructor can safely be marked as payable, since only the deployer would be able to pass funds, and the project itself would not pass any funds.

Number Of Instances Found​

2

Code Location​

Path: ./contracts/ERC20MultiDelegate.sol

16:    constructor(ERC20Votes _token, address _delegate) {  // @audit-issue

44:    constructor( // @audit-issue

[G-06] Revert String Size Optimization​

Severity​

• Impact: Low
• Likelihood: Low

Description​

Shortening the revert strings to fit within 32 bytes will decrease deployment time gas and decrease runtime gas when the revert condition is met.

Revert strings that are longer than 32 bytes require at least one additional mstore, along with additional overhead to calculate memory offset, etc.

Number Of Instances Found​

2

Code Location​

Path: ./contracts/ERC20MultiDelegate.sol

74:        require( // @audit-issue
75:            sourcesLength > 0 || targetsLength > 0,
76:            "Delegate: You should provide at least one source or one target delegate"
77:        );

79:        require( // @audit-issue
80:            Math.max(sourcesLength, targetsLength) == amountsLength,
81:            "Delegate: The number of amounts must be equal to the greater of the number of sources or targets"
82:        );

[G-07] Custom Errors in Solidity for Gas Efficiency​

Severity​

• Impact: Low
• Likelihood: Low

Description​

Starting from Solidity version 0.8.4, the language introduced a feature known as "custom errors". These custom errors provide a way for developers to define more descriptive and semantically meaningful error conditions without relying on string messages. Prior to this version, developers often used the require statement with string error messages to handle specific conditions or validations. However, every unique string used as a revert reason consumes gas, making transactions more expensive.

Custom errors, on the other hand, are identified by their name and the types of their parameters only, and they don't have the overhead of string storage. This means that, when using custom errors instead of require statements with string messages, the gas consumption can be significantly reduced, leading to more gas-efficient contracts.

Number Of Instances Found​

2

Code Location​

Path: ./contracts/ERC20MultiDelegate.sol

74:        require( // @audit-issue
75:            sourcesLength > 0 || targetsLength > 0,
76:            "Delegate: You should provide at least one source or one target delegate"
77:        );

79:        require( // @audit-issue
80:            Math.max(sourcesLength, targetsLength) == amountsLength,
81:            "Delegate: The number of amounts must be equal to the greater of the number of sources or targets"
82:        );

[G-08] Increments can be unchecked in for-loops​

Severity​

• Impact: Low
• Likelihood: Low

Description​

Newer versions of the Solidity compiler will check for integer overflows and underflows automatically. This provides safety but increases gas costs. When an unsigned integer is guaranteed to never overflow, the unchecked feature of Solidity can be used to save gas costs.A common case for this is for-loops using a strictly-less-than comparision in their conditional statement.

Number Of Instances Found​

1

Code Location​

Path: ./contracts/ERC20MultiDelegate.sol

88:            transferIndex++

[G-09] Use calldata instead of memory for function arguments that do not get mutated​

Severity​

• Impact: Low
• Likelihood: Low

Description​

Mark data types as calldata instead of memory where possible. This makes it so that the data is not automatically loaded into memory. If the data passed into the function does not need to be changed (like updating values in an array), it can be passed in as calldata. The one exception to this is if the argument must later be passed into another function that takes an argument that specifies memory storage.

Number Of Instances Found​

2

Code Location​

Path: ./contracts/ERC20MultiDelegate.sol

44:    constructor(
45:        ERC20Votes _token,
46:        string memory _metadata_uri  // @audit-issue
47:    ) ERC1155(_metadata_uri) {

151:    function setUri(string memory uri) external onlyOwner { // @audit-issue

[G-10] Use != 0 Instead of > 0 for Unsigned Integer Comparison​

Severity​

• Impact: Low
• Likelihood: Low

Description​

In Solidity, unsigned integers (e.g., uint256, uint8, etc.) represent non-negative whole numbers, ranging from 0 to a maximum value determined by their bit size. When performing comparisons on these numbers, especially to check if they are non-zero, developers have options. A common practice is to compare against zero using the > operator, as in value > 0. However, given the nature of unsigned integers, a more straightforward and slightly gas-efficient comparison is to use the != operator, as in value != 0.

The primary rationale is that the != comparison directly checks for non-equality, whereas the > comparison checks if one value is strictly greater than another. For unsigned integers, where negative values don't exist, the != 0 check is both semantically clearer and potentially more efficient at the EVM bytecode level.

Number Of Instances Found​

1

Code Location​

Path: ./contracts/ERC20MultiDelegate.sol

75:            sourcesLength > 0 || targetsLength > 0,  // @audit-issue

[G-11] Operator >=/<= costs less gas than operator >/<​

Severity​

• Impact: Low
• Likelihood: Low

Description​

The compiler uses opcodes GT and ISZERO for code that uses >, but only requires LT for >=. A similar behaviour applies for >, which uses opcodes LT and ISZERO, but only requires GT for <=.

Number Of Instances Found​

3

Code Location​

Path: ./contracts/ERC20MultiDelegate.sol

75:            sourcesLength > 0 || targetsLength > 0,  // @audit-issue

110:        if (sourcesLength > 0) {    // @audit-issue

113:        if (targetsLength > 0) {    // @audit-issue

[G-12] State Variables Only Set in The Constructor Should Be Declared immutable​

Severity​

• Impact: Low
• Likelihood: Low

Description​

Avoids a Gsset( 20000 gas) in the constructor, and replaces the first access in each transaction(Gcoldsload - 2100 gas ) and each access thereafter(Gwarmacces - 100 gas ) with aPUSH32( 3 gas ).

Whilestrings are not value types, and therefore cannot beimmutable / constant if not hard - coded outside of the constructor, the same behavior can be achieved by making the current contract abstract with virtual functions for thestring accessors, and having a child contract override the functions with the hard - coded implementation - specific values.

Number Of Instances Found​

1

Code Location​

Path: ./contracts/ERC20MultiDelegate.sol

28:    ERC20Votes public token; // @audit-issue

[G-13] State Variables That Are Used Multiple Times In a Function Should Be Cached In Stack Variables​

Severity​

• Impact: Low
• Likelihood: Low

Description​

When performing multiple operations on a state variable in a function, it is recommended to cache it first. Either multiple reads or multiple writes to a state variable can save gas by caching it on the stack. Caching of a state variable replaces each Gwarmaccess (100 gas) with a much cheaper stack read. Other less obvious fixes/optimizations include having local memory caches of state variable structs, or having local caches of state variable contracts/addresses. Saves 100 gas per instance.

Number Of Instances Found​

1

Code Location​

Path: ./contracts/ERC20MultiDelegate.sol

163:    function transferBetweenDelegators(
164:        address from,
165:        address to,
166:        uint256 amount
167:    ) internal {
168:        address proxyAddressFrom = retrieveProxyContractAddress(token, from);   // @audit-issue
169:        address proxyAddressTo = retrieveProxyContractAddress(token, to);
170:        token.transferFrom(proxyAddressFrom, proxyAddressTo, amount);
171:    }

[G-14] Functions guaranteed to revert when called by normal users can be marked payable​

Severity​

• Impact: Low
• Likelihood: Low

Description​

If a function modifier such as onlyOwner is used, the function will revert if a normal user tries to pay the function. Marking the function as payable will lower the gas cost for legitimate callers because the compiler will not include checks for whether a payment was provided.

Number Of Instances Found​

1

Code Location​

Path: ./contracts/ERC20MultiDelegate.sol

151:    function setUri(string memory uri) external onlyOwner { // @audit-issue

[G-15] abi.encode() is less efficient than abi.encodepacked()​

Severity​

• Impact: Low
• Likelihood: Low

Description​

When working with Solidity, it is important to pay attention to gas efficiency in order to optimize smart contracts. In this blog post, we will compare the gas costs of abi.encode() and abi.encodepacked() and explore why the latter is more efficient.Source: reference

Number Of Instances Found​

1

Code Location​

Path: ./contracts/ERC20MultiDelegate.sol

204:            abi.encode(_token, _delegate)   // @audit-issue

[G-16] Optimize names to save gas​

Severity​

• Impact: Low
• Likelihood: Low

Description​

public/external function names and public member variable names can be optimized to save gas. Below are the interfaces/abstract contracts that can be optimized so that the most frequently-called functions use the least amount of gas possible during method lookup. Method IDs that have two leading zero bytes can save 128 gas each during deployment, and renaming functions to have lower method IDs will save 22 gas per call, per sorted position shifted.

Number Of Instances Found​

1

Code Location​

Path: ./contracts/ERC20MultiDelegate.sol

25:contract ERC20MultiDelegate is ERC1155, Ownable {    // @audit-issue

[G-17] Internal functions only called once can be inlined to save gas​

Severity​

• Impact: Low
• Likelihood: Low

Description​

If an internal function is only used once, there is no need to modularize it, unless the function calling it would otherwise be too long and complex. Not inlining costs 20 to 40 gas because of two extra JUMP instructions and additional stack operations needed for function calls.

Number Of Instances Found​

6

Code Location​

Path: ./contracts/ERC20MultiDelegate.sol

65:    function _delegateMulti( // @audit-issue

124:    function _processDelegation(    // @audit-issue

144:    function _reimburse(address source, uint256 amount) internal {  // @audit-issue

155:    function createProxyDelegatorAndTransfer(   // @audit-issue

163:    function transferBetweenDelegators( // @audit-issue

192:    function getBalanceForDelegate( // @audit-issue

[G-18] Use assembly to check for 0​

Severity​

• Impact: Low
• Likelihood: Low

Description​

Using assembly to check for zero can save gas by allowing more direct access to the evm and reducing some of the overhead associated with high-level operations in solidity.

Number Of Instances Found​

1

Code Location​

Path: ./contracts/ERC20MultiDelegate.sol

185:        if (bytecodeSize == 0) {    // @audit-issue

[G-19] Use assembly to emit an event​

Severity​

• Impact: Low
• Likelihood: Low

Description​

To efficiently emit events, it's possible to utilize assembly by making use of scratch space and the free memory pointer. This approach has the advantage of potentially avoiding the costs associated with memory expansion.

However, it's important to note that in order to safely optimize this process, it is preferable to cache and restore the free memory pointer.

A good example of such practice can be seen in Solady's codebase.

Number Of Instances Found​

2

Code Location​

Path: ./contracts/ERC20MultiDelegate.sol

136:        emit DelegationProcessed(source, target, amount);   // @audit-issue

187:            emit ProxyDeployed(delegate, proxyAddress); // @audit-issue

[G-20] Use assembly to write hashes​

Severity​

• Impact: Low
• Likelihood: Low

Description​

Considering using assembly to write hashes, as it's possible to save a considerable amount of gas.

Number Of Instances Found​

2

Code Location​

Path: ./contracts/ERC20MultiDelegate.sol

206:        bytes32 hash = keccak256(   // @audit-issue

211:                keccak256(bytecode) // @audit-issue

Non-Critical Issues​

[N-01] Non-external/public variable and function names should begin with an underscore​

Severity​

• Impact: Low
• Likelihood: Low

Description​

According to the Solidity Style Guide, Non-external/public variable and function names should begin with an underscore

Number Of Instances Found​

5

Code Location​

Path: ./contracts/ERC20MultiDelegate.sol

155:    function createProxyDelegatorAndTransfer(   // @audit-issue
156:        address target,
157:        uint256 amount
158:    ) internal {

163:    function transferBetweenDelegators( // @audit-issue
164:        address from,
165:        address to,
166:        uint256 amount
167:    ) internal {

173:    function deployProxyDelegatorIfNeeded(  // @audit-issue
174:        address delegate
175:    ) internal returns (address) {

192:    function getBalanceForDelegate( // @audit-issue
193:        address delegate
194:    ) internal view returns (uint256) {

198:    function retrieveProxyContractAddress(  // @audit-issue
199:        ERC20Votes _token,
200:        address _delegate
201:    ) private view returns (address) {

[N-02] Too long functions should be refactored​

Severity​

• Impact: Low
• Likelihood: Low

Description​

Functions with too many lines are difficult to understand. It is recommended to refactor complex functions into multiple shorter and easier to understand functions.

Number Of Instances Found​

1

Code Location​

Path: ./contracts/ERC20MultiDelegate.sol

65:    function _delegateMulti( // @audit-issue 51 lines

[N-03] Use bytes.concat() on bytes instead of abi.encodePacked() for clearer semantic meaning​

Severity​

• Impact: Low
• Likelihood: Low

Description​

Starting with version 0.8.4, Solidity has the bytes.concat() function, which allows one to concatenate a list of bytes/strings, without extra padding. Using this function rather than abi.encodePacked() makes the intended operation more clear, leading to less reviewer confusion.

Number Of Instances Found​

2

Code Location​

Path: ./contracts/ERC20MultiDelegate.sol

202:        bytes memory bytecode = abi.encodePacked(   // @audit-issue
203:            type(ERC20ProxyDelegator).creationCode, 
204:            abi.encode(_token, _delegate)
205:        );

207:            abi.encodePacked(   // @audit-issue
208:                bytes1(0xff),
209:                address(this),
210:                uint256(0), // salt
211:                keccak256(bytecode)
212:            )

[N-04] Style guide: Function ordering does not follow the Solidity style guide​

Severity​

• Impact: Low
• Likelihood: Low

Description​

According to the Solidity style guide, functions should be laid out in the following order :constructor(), receive(), fallback(), external, public, internal, private, but the cases below do not follow this pattern

Number Of Instances Found​

1

Code Location​

Path: ./contracts/ERC20MultiDelegate.sol

151:    function setUri(string memory uri) external onlyOwner { // @audit-issue : setUri should come before than _reimburse

[N-05] Constants in comparisons should appear on the left side​

Severity​

• Impact: Low
• Likelihood: Low

Description​

Doing so will prevent typo bugs

Number Of Instances Found​

1

Code Location​

Path: ./contracts/ERC20MultiDelegate.sol

185:        if (bytecodeSize == 0) {    // @audit-issue

[N-06] Contract should expose an interface​

Severity​

• Impact: Low
• Likelihood: Low

Description​

All external/public functions should extend an interface. This is useful to make sure that the whole API is extracted.

Number Of Instances Found​

2

Code Location​

Path: ./contracts/ERC20MultiDelegate.sol

57:    function delegateMulti(
58:        uint256[] calldata sources,
59:        uint256[] calldata targets,
60:        uint256[] calldata amounts
61:    ) external { // @audit-issue

151:    function setUri(string memory uri) external onlyOwner { // @audit-issue

[N-07] Event is not properly indexed​

Severity​

• Impact: Low
• Likelihood: Low

Description​

Index event fields make the field more quickly accessible to off-chain tools that parse events. This is especially useful when it comes to filtering based on an address. However, note that each index field costs extra gas during emission, so it's not necessarily best to index the maximum allowed per event (three fields). Where applicable, each event should use three indexed fields if there are three or more fields, and gas usage is not particularly of concern for the events in question. If there are fewer than three applicable fields, all of the applicable fields should be indexed.

Number Of Instances Found​

1

Code Location​

Path: ./contracts/ERC20MultiDelegate.sol

32:    event ProxyDeployed(address indexed delegate, address proxyAddress); // @audit-issue

[N-08] Lack of specific import identifier​

Severity​

• Impact: Medium
• Likelihood: Low

Description​

It is better to use import {<identifier>} from "<file.sol>" instead of import "<file.sol>" to improve readability and speed up the compilation time.

Number Of Instances Found​

4

Code Location​

Path: ./contracts/ERC20MultiDelegate.sol

6:import "@openzeppelin/contracts/access/Ownable.sol";  // @audit-issue

7:import "@openzeppelin/contracts/token/ERC1155/ERC1155.sol";   // @audit-issue

8:import "@openzeppelin/contracts/token/ERC20/extensions/ERC20Votes.sol";   // @audit-issue

9:import "@openzeppelin/contracts/utils/math/Math.sol"; // @audit-issue

[N-09] Missing events in sensitive functions​

Severity​

• Impact: Low
• Likelihood: Low

Description​

Events should be emitted when sensitive changes are made to the contracts, but some functions lack them.

Number Of Instances Found​

1

Code Location​

Path: ./contracts/ERC20MultiDelegate.sol

48:        token = _token;  // @audit-issue

[N-10] Include sender information in events​

Severity​

• Impact: Low
• Likelihood: Low

Description​

When an action is triggered based on a user's action, not being able to filter based on who triggered the action makes event processing a lot more cumbersome. Including the msg.sender the events of these types of action will make events much more useful to end users, especially when msg.sender is not tx.origin.

Number Of Instances Found​

2

Code Location​

Path: ./contracts/ERC20MultiDelegate.sol

136:        emit DelegationProcessed(source, target, amount);   // @audit-issue

187:            emit ProxyDeployed(delegate, proxyAddress); // @audit-issue

[N-11] Consider activating via-ir for deploying​

Severity​

• Impact: Low
• Likelihood: Low

Description​

The IR-based code generator was developed to make code generation more performant by enabling optimization passes that can be applied across functions.

It is possible to activate the IR-based code generator through the command line by using the flag --via-iror by including the option {"viaIR": true}.

Keep in mind that compiling with this option may take longer. However, you can simply test it before deploying your code. If you find that it provides better performance, you can add the --via-ir flag to your deploy command.

Number Of Instances Found​

1

Code Location​

/// @audit Global finding.

GitHub: 0

[N-12] Contracts should have full test coverage​

Severity​

• Impact: Low
• Likelihood: Low

Description​

While 100% code coverage does not guarantee that there are no bugs, it often will catch easy-to-find bugs, and will ensure that there are fewer regressions when the code invariably has to be modified. Furthermore, in order to get full coverage, code authors will often have to re-organize their code so that it is more modular, so that each component can be tested separately, which reduces interdependencies between modules and layers, and makes for code that is easier to reason about and audit.

Number Of Instances Found​

1

Code Location​

/// @audit Global finding.

GitHub: 0

[N-13] Large or complicated code bases should implement invariant tests​

Severity​

• Impact: Low
• Likelihood: Low

Description​

This includes: large code bases, or code with lots of inline-assembly, complicated math, or complicated interactions between multiple contracts. Invariant fuzzers such as Echidna require the test writer to come up with invariants which should not be violated under any circumstances, and the fuzzer tests various inputs and function calls to ensure that the invariants always hold. Even code with 100% code coverage can still have bugs due to the order of the operations a user performs, and invariant fuzzers may help significantly.

Number Of Instances Found​

1

Code Location​

/// @audit Global finding.

GitHub: 0

[N-14] NatSpec: Contract declarations should have @author tag​

Severity​

• Impact: Low
• Likelihood: Low

Description​

In the world of decentralized code, giving credit is key. NatSpec's @author tag acknowledges the minds behind the code. It appears this Solidity contract omits the @author directive in its NatSpec annotations. Properly attributing code to its contributors not only recognizes effort but also aids in establishing trust and credibility. Dive Deeper into NatSpec Guidelines

Number Of Instances Found​

2

Code Location​

Path: ./contracts/ERC20MultiDelegate.sol

15:contract ERC20ProxyDelegator {   // @audit-issue missing `@author` tag

25:contract ERC20MultiDelegate is ERC1155, Ownable {    // @audit-issue missing `@author` tag

[N-15] NatSpec: Function @return tag is missing​

Severity​

• Impact: Low
• Likelihood: Low

Description​

Natural Specification (NatSpec) comments are crucial for understanding the role of function arguments in your Solidity code. Including @return tag will not only improve your code's readability but also its maintainability by clearly defining each argument's purpose. Dive Deeper into NatSpec Guidelines

Number Of Instances Found​

3

Code Location​

Path: ./contracts/ERC20MultiDelegate.sol

173:    function deployProxyDelegatorIfNeeded(  // @audit-issue missing `@return` tag

192:    function getBalanceForDelegate( // @audit-issue missing `@return` tag

198:    function retrieveProxyContractAddress(  // @audit-issue missing `@return` tag

[N-16] NatSpec: Function declarations should have @notice tag​

Severity​

• Impact: Low
• Likelihood: Low

Description​

In Solidity, the @notice tag in NatSpec comments is used to provide important explanations to end users about what a function does. It appears that this contract's function declarations are missing @notice tags in their NatSpec annotations.

The absence of @notice tags reduces the contract's transparency and could lead to misunderstandings about a function's purpose and behavior. Dive Deeper into NatSpec Guidelines

Number Of Instances Found​

12

Code Location​

Path: ./contracts/ERC20MultiDelegate.sol

16:    constructor(ERC20Votes _token, address _delegate) {  // @audit-issue missing `@notice` tag

44:    constructor( // @audit-issue missing `@notice` tag

57:    function delegateMulti(  // @audit-issue missing `@notice` tag

65:    function _delegateMulti( // @audit-issue missing `@notice` tag

124:    function _processDelegation(    // @audit-issue missing `@notice` tag

144:    function _reimburse(address source, uint256 amount) internal {  // @audit-issue missing `@notice` tag

151:    function setUri(string memory uri) external onlyOwner { // @audit-issue missing `@notice` tag

155:    function createProxyDelegatorAndTransfer(   // @audit-issue missing `@notice` tag

163:    function transferBetweenDelegators( // @audit-issue missing `@notice` tag

173:    function deployProxyDelegatorIfNeeded(  // @audit-issue missing `@notice` tag

192:    function getBalanceForDelegate( // @audit-issue missing `@notice` tag

198:    function retrieveProxyContractAddress(  // @audit-issue missing `@notice` tag

[N-17] NatSpec: Function declarations should have @dev tag​

Severity​

• Impact: Low
• Likelihood: Low

Description​

Some functions have an incomplete NatSpec: add a @dev notation to describe the function to improve the code documentation.

Number Of Instances Found​

8

Code Location​

Path: ./contracts/ERC20MultiDelegate.sol

16:    constructor(ERC20Votes _token, address _delegate) {  // @audit-issue missing `@dev` tag

65:    function _delegateMulti( // @audit-issue missing `@dev` tag

151:    function setUri(string memory uri) external onlyOwner { // @audit-issue missing `@dev` tag

155:    function createProxyDelegatorAndTransfer(   // @audit-issue missing `@dev` tag

163:    function transferBetweenDelegators( // @audit-issue missing `@dev` tag

173:    function deployProxyDelegatorIfNeeded(  // @audit-issue missing `@dev` tag

192:    function getBalanceForDelegate( // @audit-issue missing `@dev` tag

198:    function retrieveProxyContractAddress(  // @audit-issue missing `@dev` tag

[N-18] NatSpec: Function/Constructor @param tag is missing​

Severity​

• Impact: Low
• Likelihood: Low

Description​

Natural Specification (NatSpec) comments are crucial for understanding the role of function arguments in your Solidity code. Including @param tags will not only improve your code's readability but also its maintainability by clearly defining each argument's purpose. Dive Deeper into NatSpec Guidelines

Number Of Instances Found​

8

Code Location​

Path: ./contracts/ERC20MultiDelegate.sol

16:    constructor(ERC20Votes _token, address _delegate) {  // @audit-issue missing `@param` tag

65:    function _delegateMulti( // @audit-issue missing `@param` tag

151:    function setUri(string memory uri) external onlyOwner { // @audit-issue missing `@param` tag

155:    function createProxyDelegatorAndTransfer(   // @audit-issue missing `@param` tag

163:    function transferBetweenDelegators( // @audit-issue missing `@param` tag

173:    function deployProxyDelegatorIfNeeded(  // @audit-issue missing `@param` tag

192:    function getBalanceForDelegate( // @audit-issue missing `@param` tag

198:    function retrieveProxyContractAddress(  // @audit-issue missing `@param` tag

[N-19] NatSpec: Event declarations should have @notice tag​

Severity​

• Impact: Low
• Likelihood: Low

Description​

In Solidity, the @notice tag in NatSpec comments is used to provide important explanations to end users about what a event does. It appears that this contract's modifier declarations are missing @notice tags in their NatSpec annotations.

The absence of @notice tags reduces the contract's transparency and could lead to misunderstandings about a events's purpose and behavior. Dive Deeper into NatSpec Guidelines

Number Of Instances Found​

2

Code Location​

Path: ./contracts/ERC20MultiDelegate.sol

32:    event ProxyDeployed(address indexed delegate, address proxyAddress); // @audit-issue missing `@notice` tag

33:    event DelegationProcessed(   // @audit-issue missing `@notice` tag

[N-20] NatSpec: Event declarations should have @dev tag​

Severity​

• Impact: Low
• Likelihood: Low

Description​

Some events have an incomplete NatSpec: add a @dev notation to describe the event to improve the code documentation.

Number Of Instances Found​

2

Code Location​

Path: ./contracts/ERC20MultiDelegate.sol

32:    event ProxyDeployed(address indexed delegate, address proxyAddress); // @audit-issue missing `@dev` tag

33:    event DelegationProcessed(   // @audit-issue missing `@dev` tag

[N-21] NatSpec: Event @param tag is missing​

Severity​

• Impact: Low
• Likelihood: Low

Description​

Natural Specification (NatSpec) comments are crucial for understanding the role of event arguments in your Solidity code. Including @param tags will not only improve your code's readability but also its maintainability by clearly defining each argument's purpose. Dive Deeper into NatSpec Guidelines

Number Of Instances Found​

2

Code Location​

Path: ./contracts/ERC20MultiDelegate.sol

32:    event ProxyDeployed(address indexed delegate, address proxyAddress); // @audit-issue missing `@param` tag

33:    event DelegationProcessed(   // @audit-issue missing `@param` tag

Scope:​