Category: Code Maturity
Redundant code in the fill function
Informational Impact
Informational Severity
N/A Likelihood
Description
The fill function handles the transfer of native output tokens and ERC-20 tokens separately.
function fill(Order calldata order) external payable nonReentrant whenNotPaused onlySolver {
[...]
// Handle native or ERC20 output
if (order.outputToken.isNativeToken()) {
// For native tokens, solver must send exact amount via msg.value
require(msg.value == order.outputAmount, "Incorrect native amount sent");
order.outputToken.safeTransfer(order.recipient, order.outputAmount);
} else {
// For ERC20 tokens, ensure no native tokens were sent
require(msg.value == 0, "No native tokens should be sent for ERC20 fills");
IERC20(order.outputToken).safeTransferFrom(msg.sender, order.recipient, order.outputAmount);
}
[...]
}However, the safeTransfer function from the NativeTokenUtils library, which is used for native token transfers, already supports transferring both native and ERC-20 tokens. As a result, the separate logic in the fill function is redundant, and the safeTransfer function could be used to handle this logic.
function safeTransfer(address token, address to, uint256 amount) internal {
if (isNativeToken(token)) {
(bool success, ) = payable(to).call{value: amount}("");
require(success, "Native transfer failed");
} else {
IERC20(token).safeTransfer(to, amount);
}
}Impact
This redundant code adds unnecessary complexity.
Recommendations
We recommend removing the duplicated logic in the fill function and using the safeTransfer function for handling both ERC-20 and native token transfers.
Remediation
This issue has been acknowledged by Aori, and a fix was implemented in commit ff76040c↗.