In the StableSwapRouter contract, the constructor accepts addresses for key contracts such as the stableSwapFactory and stableSwapInfo. However, using the address type does not provide compile-time safety, leading to potential risks if a contract passed to the constructor does not conform to the expected interface. By accepting generic addresses, there is no guarantee that the assigned contracts implement the necessary functions for the StableSwapRouter to operate correctly. This can result in runtime errors or unexpected behavior when interacting with these contracts. Furthermore, the contract imports the IWROSE interface and does not use it for WROSE.
Attachments
Revised Code
It is recommended to replace the use of the address type with the respective interface types for key contracts like stableSwapFactory and stableSwapInfo. Furthermore, use the IWROSE that is imported and never used. This ensures that the contracts passed to the constructor are verified at compile-time to implement the required interface, reducing the likelihood of runtime errors and improving overall code robustness. Additionally, this approach enhances code readability and enforces stricter type checks, which contribute to better contract security and reliability.
import "@openzeppelin/contracts/token/ERC20/IERC20.sol";
import "./interfaces/IStableSwapRouter.sol";
import "./interfaces/IStableSwap.sol";
+ import "./interfaces/IStableSwapFactory.sol"; // Import the factory interface
+ import "./interfaces/IStableSwapInfo.sol"; // Import the info interface
import "./interfaces/IWROSE.sol";
import "./libraries/SmartRouterHelper.sol";
import "./libraries/Constants.sol";
import "@openzeppelin/contracts/access/Ownable.sol";
import "@openzeppelin/contracts/security/ReentrancyGuard.sol";
import "hardhat/console.sol";
/// @title Stable Swap Router
/// @notice A router contract for excuting stable swaps between different stablecoins pairs through mutiple pools
/// It allows users to swap stable coins efficiently
/// @dev This contract manages stable swap functionality, including executing swaps and caculating swap amounts
contract StableSwapRouter is
IStableSwapRouter,
Ownable,
ReentrancyGuard
{
- address public WROSE;
+ IWROSE public WROSE;
address public stableSwapFactory;
address public stableSwapInfo;
address public constant ROSE=0xEeeeeEeeeEeEeeEeEeEeeEEEeeeeEeeeeeeeEEeE;
bool public isKill;
/*╔══════════════════════════════╗
║ EVENT ║
╚══════════════════════════════╝*/
event SetStableSwap(address indexed factory, address indexed info);
event StableExchange(
address indexed buyer,
uint256 amountIn,
address indexed token1,
uint256 amountOut,
address indexed token2,
address recipient
);
receive() external payable {}
/*╔══════════════════════════════╗
║ CONSTRUCTOR ║
╚══════════════════════════════╝*/
constructor(
- address _stableSwapFactory,
- address _stableSwapInfo
+ IStableSwapFactory _stableSwapFactory,
+ IStableSwapInfo _stableSwapInfo
){
stableSwapFactory = _stableSwapFactory;
stableSwapInfo = _stableSwapInfo;
isKill=false;
}
- function setStableSwap(address _factory, address _info) external onlyOwner {
+ function setStableSwap(IStableSwapFactory _factory, IStableSwapInfo _info) external onlyOwner {
require(_factory != address(0) && _info != address(0));
stableSwapFactory = _factory;
stableSwapInfo = _info;
emit SetStableSwap(stableSwapFactory, stableSwapInfo);
}
Changing the type definitions of function arguments from address to the expected contract interfaces does not add any run-time checks when passing these arguments, as far as I know.
Github username: -- Twitter username: -- Submission hash (on-chain): 0xd65f86398f17c88bb2f6add723893e43349b8e7b58c5f67a0bc607b43fb97324 Severity: low
Description: Description
In the
StableSwapRouter
contract, the constructor accepts addresses for key contracts such as thestableSwapFactory
andstableSwapInfo
. However, using theaddress
type does not provide compile-time safety, leading to potential risks if a contract passed to the constructor does not conform to the expected interface. By accepting generic addresses, there is no guarantee that the assigned contracts implement the necessary functions for theStableSwapRouter
to operate correctly. This can result in runtime errors or unexpected behavior when interacting with these contracts. Furthermore, the contract imports theIWROSE
interface and does not use it forWROSE
.Attachments
It is recommended to replace the use of the
address
type with the respective interface types for key contracts likestableSwapFactory
andstableSwapInfo
. Furthermore, use theIWROSE
that is imported and never used. This ensures that the contracts passed to the constructor are verified at compile-time to implement the required interface, reducing the likelihood of runtime errors and improving overall code robustness. Additionally, this approach enhances code readability and enforces stricter type checks, which contribute to better contract security and reliability.