LzAppUpgradeable Code Style Findings

LAU-01C: Event Usability Enhancement

Type	Severity	Location
Code Style		LzAppUpgradeable.sol:L45-L47

Description:

The referenced events do not contain any indexed arguments declared.

Example:

Copy
45event SetTrustedRemote(uint16 _remoteChainId, bytes _path);
46event SetTrustedRemoteAddress(uint16 _remoteChainId, bytes _remoteAddress);
47event SetMinDstGas(uint16 _dstChainId, uint16 _type, uint256 _minDstGas);

Recommendation:

We advise the indexed keyword to be introduced to the _remoteChainId and _dstChainId variables, optimizing off-chain filters of these events in relation to their chain ID.

Alleviation (47fbf62bbbf2409ff0baf9a18a2945466cb2a576):

The Tangible team evaluated this exhibit but opted to retain the original code's behaviour in place to ensure that the events in LzAppUpgradeable closely correlate with the tried-and-true events present in the LayerZero implementation. As such, we consider this exhibit safely acknowledged.

LAU-02C: Redundant Representation of Literal Constant

Type	Severity	Location
Code Style		LzAppUpgradeable.sol:L59

Description:

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

Example:

Copy
58// keccak256(abi.encode(uint256(keccak256("layerzero.storage.LzApp")) - 1)) & ~bytes32(uint256(0xff))
59bytes32 private constant LzAppStorageLocation = 0x111388274dd962a0529050efb131321f60015c2ab1a99387d94540f430037b00;

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.

LAU-03C: Repetitive Value Literal

Type	Severity	Location
Code Style		LzAppUpgradeable.sol:L265, L268

Description:

The linked value literal is repeated across the codebase multiple times.

Example:

Copy
265require(_adapterParams.length >= 34, "LzApp: invalid adapterParams");

Recommendation:

We advise it to be set to a constant variable instead optimizing the legibility of the codebase.

Alleviation (47fbf62bbbf2409ff0baf9a18a2945466cb2a576):

The Tangible team evaluated this exhibit but opted to retain the original code's behaviour in place to ensure that the code of LzAppUpgradeable mimics its original LzApp counterpart by LayerZero as closely as possible. As such, we consider this exhibit safely acknowledged.

LAU-04C: Significant Optimization of Call Relays

Type	Severity	Location
Gas Optimization		LzAppUpgradeable.sol:L51, L93

Description:

The LzAppUpgradeable contract is meant to be an upgradeable version of the LzApp by LayerZero. To achieve this, its storage space has been migrated to a pointer-based LzAppStorage struct causing the lzEndpoint to no longer be immutable and require a full SLOAD operation per-use.

Example:

Copy
49/// @custom:storage-location erc7201:layerzero.storage.LzApp
50struct LzAppStorage {
51    ILayerZeroEndpoint lzEndpoint;
52    mapping(uint16 => bytes) trustedRemoteLookup;
53    mapping(uint16 => mapping(uint16 => uint256)) minDstGasLookup;
54    mapping(uint16 => uint256) payloadSizeLimitLookup;
55    address precrime;
56}

Recommendation:

We advise the lzEndpoint to be relocated to the contract itself and set as an immutable variable. Immutable variables are upgradeability-compatible as they are placed within the bytecode of the contract rather than its storage space, meaning that they can safely be set in the LzAppUpgradeable::constructor and utilized in the other functions of the contract via delegatecall operations.

Alleviation (47fbf62bbbf2409ff0baf9a18a2945466cb2a576):

The lzEndpoint variable was relocated to a contract-level immutable declaration as advised with a public visibility specifier, addressing this exhibit.