MegaETH: MegaSaleRefund

Description

The natspec documentation above _processSingleRefund() function says =>

// PREREQUISITE: sale contract must grant TOKEN_RECOVERER_ROLE to this contract// @dev This is checked in _processRefund() via sale.hasRole(sale.TOKEN_RECOVERER_ROLE(), address(this))

But this is incorrect description. The mentioned check is actually done in refund() and batchRefund() functions, and _processRefund() function is not a part of the current codebase.

Recommendation

Consider changing the given natspec comment.

Description

The TokenSale contract provided a way for users to bid with USDT, and the MegaSaleRefund contract is aimed at providing 10 % rebates for users who opted in to lock the acquired $MEGA tokens for a year.

The refund() function here uses the input "to" address to check user entry in the merkle tree (list of all user entities that are eligible for refund), and sends the refunded USDT back to this "to" address. The user does not have a way to receive the refund at a different address.

This works fine if all entities that interacted with TokenSale contract were EOAs. But if they were contracts with hardcoded pathways, they might not be able to pull those USDT out after receiving the refund at their own end.

Recommendation

Consider ensuring that there are no entities who used a smart contract to participate in the public token sale. If such entities are eligible for refunds, they should be able to handle the received USDT tokens.

Description

The batchRefund() function emits an event BatchRefundProcessed(uint256 usersCount, uint256 totalAmount); and makes use of the following logic to track usersCount and totalAmount refunded in the batch :

uint256 batchTotalAmount = 0;        uint256 successfulRefunds = 0; // for tracking user count
        for (uint256 i = 0; i < usersCount; i++) {            _processSingleRefund(users[i], amounts[i], merkleProofs[i]);            batchTotalAmount += amounts[i];            successfulRefunds++;        }

But this local variable successfulRefunds is not required. The batchRefund only ever executes in full, it succeeds only if all refunds succeed as per the input, and we already have the user count from the input users array.

Recommendation

Consider using the already known users count for event data, and removing the successfulRefunds variable.