Omniscia Tangible Audit
NonblockingLzAppUpgradeable Code Style Findings
NonblockingLzAppUpgradeable Code Style Findings
NLA-01C: Inefficient mapping Lookups
| Type | Severity | Location |
|---|---|---|
| Gas Optimization |  | NonblockingLzAppUpgradeable.sol:L172, L176 |
Description:
The linked statements perform key-based lookup operations on mapping declarations from storage multiple times for the same key redundantly.
Example:
172bytes32 payloadHash = $.failedMessages[srcChainId][srcAddress][nonce];173require(payloadHash != bytes32(0), "NonblockingLzApp: no stored message");174require(keccak256(payload) == payloadHash, "NonblockingLzApp: invalid payload");175// clear the stored message176$.failedMessages[srcChainId][srcAddress][nonce] = bytes32(0);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 (47fbf62bbb):
A local _failedMessages mapping variable has been properly introduced as a cached lookup of the interim $.failedMessages[srcChainId][srcAddress] value, optimizing the code's gas cost.
It's usage in the code, however, is incorrect as the payloadHash assignment performed is done to memory rather than storage. We advise the code to write to the _failedMessages[nonce] entry instead of the payloadHash member as the latter represents a primitive type entirely in memory.
Alleviation (c98ea3cb77):
The _failedMessages data entry is properly cleared in the latest commit hash, rendering this exhibit fully alleviated.
NLA-02C: Redundant Representation of Literal Constant
| Type | Severity | Location |
|---|---|---|
| Code Style | | NonblockingLzAppUpgradeable.sol:L30 |
Description:
The NonblockingLzAppStorageLocation variable is declared as a bytes32 literal instead of utilizing its commented-out representation.
Example:
28// keccak256(abi.encode(uint256(keccak256("layerzero.storage.NonblockingLzApp")) - 1)) & ~bytes32(uint256(0xff))29bytes32 private constant NonblockingLzAppStorageLocation =30 0xe5a86fa43fa85f564c84895bd4f80ec5c29d03a57a0c1f7cb91d2cc05b4d8600;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.