Omniscia Evergon Labs Audit

OmnichainERC721Transfers Code Style Findings

OmnichainERC721Transfers Code Style Findings

OES-01C: Optimization of Conditionals

Description:

The OmnichainERC721Transfers::_update implementation which is purpose-built for the OmnichainAddress data type will redundantly apply several if conditionals which are guaranteed to be true.

Example:

contracts/dataManagers/omnichain/ERC721/OmnichainERC721Transfers.sol
28function _safeTransferFrom(address from, OmnichainAddress to, uint256 tokenId, bytes memory) internal {
29 (uint32 chainId, ) = OmnichainAddresses.decode(to);
30 ChainidTools.requireNotCurrentChain(chainId);
31
32 // Setting an "auth" arguments enables the `_isAuthorized` check which verifies that the token exists
33 // (from != 0). Therefore, it is not needed to verify that the return value is not 0 here.
34 address previousOwner = _update(to, tokenId, _msgSender());
35 if (previousOwner != from) {
36 revert ERC721IncorrectOwner(from, tokenId, previousOwner);
37 }
38}
39
40function _update(OmnichainAddress to, uint256 tokenId, address auth) internal virtual returns (address) {
41 address from = _ownerOf(tokenId);
42
43 // Perform (optional) operator check
44 if (auth != address(0)) {
45 _checkAuthorized(from, auth, tokenId);
46 }
47
48 if (from != address(0)) {
49 // Clear approval. No need to re-authorize or emit the Approval event
50 _approve(address(0), tokenId, address(0), false);
51 }
52
53 _writeTransfer(from, to, tokenId);
54
55 (uint32 toChainId, ) = OmnichainAddresses.decode(to);
56 address toAddressPlaceholder = address(uint160(toChainId));
57
58 emit Transfer(from, toAddressPlaceholder, tokenId);
59
60 return from;
61}

Recommendation:

We advise the referenced if branches to be omitted as the auth argument is guaranteed to be non-zero whilst the from argument would fail in the ERC721Transfers::_checkAuthorized call if it were zero.

Alleviation (c6b23c23d8bcd8cce85049ad959cbd711a37126b):

The Evergon Labs team evaluated this exhibit and stated that while they acknowledge the optimization in the code's current state, they intend the code to be compatible with future upgrades that introduce burn operations etc. and thus wish to remain flexible.