Quality Assurance Report

[L-01] PUSH0 opcode is not supported on Arbitrum when using solc v0.8.0 or higher

This would cause deployment errors and transaction reverts on the Arbitrum L2.

[L-02] Use Ownable2Step in MOR contract

Currently the MOR token contract uses Ownable.sol. This would cause issues when ownership is transferred to another address since an incorrect input would lead to permanent loss of the owner role. Due to this, it is advised to use a two step mechanism to avoid this issue.

[L-03] Missing event emission in setter functions

The setter functions below should include event emissions to ensure any offchain tracking mechanism is alerted of critical changes by the owner role.

[L-04] Pass adapterParams as a configurable bytes parameter instead of using bytes("")

See 7th point in the LayerZero integration list. Hardcoding adapterParams to bytes("") is not recommended since it limits the protocol from future configurability or added features.

[L-05] Do not hardcode zroPaymentAddress to address(0)

On Line 149, we can see that the zroPaymentAddress field is hardcoded to address(0). This makes the protocol devoid of using the future ZRO token (confirmed) as a payment option for the cross-chain fees. See 5th point in the LayerZero integration list

[L-06] Consider using mapping over array to avoid DOS

Whenever a pool is created, it is pushed to the pools array. The issue is that if this pools array grows too large, it could cause DOS or excessive gas consumption for other contract trying to retrieve all the pools. Although this does not occur in the current protocol, it is an issue to know about in case external protocols or additional contracts use the TCM model to expand on some functionality.

[L-07] Use CEI pattern in function retryMessage()

Similar to how the retryPayload() function in the Endpoint contract deletes the stored payload before making a call, consider deleting the failedMessage payload stored before making a call to _nonBlockingLzReceive() on Line 64. This would adhere to the best practice to using the CEI pattern as well as prevent any reentrancy risks that could arise (though there aren't any currently).

[L-08] Return value of approve() not checked

The unwrappedToken (StETH) returns a bool in its approve() function. This value is not checked in the current code. Although it returns true, it is best practice to check these values as true.

[L-09] Use outboundTransferCustomRefund() instead of outboundTransfer() as recommended by Arbitrum documentation

On Line 116, we can see that function outboundTransfer() is used to make the cross-chain call from Ethereum L1 to Arbitrum L2. The Arbitrum documentation though recommends using outboundTransferCustomRefund() when bridging from Ethereum to Arbitrum. (See first point at the bottom here).

[L-10] Anyone can call claim() function on behalf of any other user

Currently, the claim() function in the Distribution.sol contract takes in user_ as a parameter instead of using msg.sender directly. This allows anyone to claim on behalf of the user_, which might not be what the user wanted. The user_ may have wanted to accumulate more rewards and then finally bridge to mint MOR tokens on Arbitrum. But since an attacker or anyone has already bridged the amount, this would affect the user's strategy. This does not affect the user in any way at first glance since the attacker pays the bridging fees but external applications or strategies would be affected due to this issue.

Solution: Ensure user_ is msg.sender and/or implement an approval mapping which stores approvals the user_ has given to certain external applications/contracts to call the claim() function on behalf.