Omniscia Pirex Audit
ERC20SnapshotSolmate Code Style Findings
ERC20SnapshotSolmate Code Style Findings
ERS-01C: Inefficient mapping
Lookups
Type | Severity | Location |
---|---|---|
Gas Optimization | ERC20SnapshotSolmate.sol:L126, L129 |
Description:
The linked statements perform key-based lookup operations on mapping
declarations from storage multiple times for the same key redundantly.
Example:
119function transferFrom(120 address from,121 address to,122 uint256 amount123) public virtual returns (bool) {124 _beforeTokenTransfer(from, to, amount);125
126 uint256 allowed = allowance[from][msg.sender]; // Saves gas for limited approvals.127
128 if (allowed != type(uint256).max)129 allowance[from][msg.sender] = allowed - amount;130
131 balanceOf[from] -= amount;132
133 // Cannot overflow because the sum of all user134 // balances can't exceed the max uint256 value.135 unchecked {136 balanceOf[to] += amount;137 }138
139 emit Transfer(from, to, amount);140
141 return true;142}
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.
Alleviation:
While the Pirex team acknowledges the gas optimization this change brings, they do not utilize the transferFrom
function in their protocol and as such they will not apply the optimization in favor of code clarity.
ERS-02C: Variable Read Optimization
Type | Severity | Location |
---|---|---|
Gas Optimization | ERC20SnapshotSolmate.sol:L486, L489 |
Description:
The linked length
variable is read twice from storage
off the ids
input argument.
Example:
481function _lastSnapshotId(uint256[] storage ids)482 private483 view484 returns (uint256)485{486 if (ids.length == 0) {487 return 0;488 } else {489 return ids[ids.length - 1];490 }491}
Recommendation:
We advise the ids.length
lookup to be cached to a local variable instead optimizing the read-access gas cost of the function.
Alleviation:
The ids.length
evaluation is now properly cached to an idsLen
variable that is consequently utilized for the if
clause and the array indice thereby optimizing the codebase.