The following methods have a lack checks if the received argument is an address, it's good practice in order to reduce human error to check that the address specified in the constructor or initialize is different than address(0).
As stated in the OpenZeppelin source comments, the OpenZeppeling ERC20 safeApprove() function has been deprecated.
Using this deprecated function could result in accidental reverts and possibly fund locking. The deprecation of this function is discussed in greater depth in the OZ issue #2219.
As suggested by the OpenZeppelin comment, replace safeApprove() with safeIncreaseAllowance() or safeDecreaseAllowance() instead.
The following code doesn't check the result of the transferFrom or approve calls. ERC20 standard specify that the token can return false if these calls fails, so it's mandatory to check the result of these ERC20 methods.
The modification of an owner is a delicate procedure, as it may jeopardize the governance of our contract and therefore the project. As a result, it is advised that the owner's modification logic be changed to one that allows for verification that the new owner is valid and exists.
It is required to establish a logic for the owner's modification in which a new owner is proposed first, the owner approves the suggestion, and we ensure that the new owner's address is written correctly.
The owner of the WETHGateway contract has the ability to extract all existing WETH from the contract via the refreshAllowance method.
These capabilities must be limited in favor of decentralization, in order to avoid loss of reputation or user confidence.
Lack of empty address checks
The following methods have a lack checks if the received argument is an address, it's good practice in order to reduce human error to check that the address specified in the constructor or initialize is different than
address(0)
.Source code:
As stated in the OpenZeppelin source comments, the OpenZeppeling ERC20
safeApprove()
function has been deprecated.Using this deprecated function could result in accidental reverts and possibly fund locking. The deprecation of this function is discussed in greater depth in the OZ issue #2219.
As suggested by the OpenZeppelin comment, replace
safeApprove()
withsafeIncreaseAllowance()
orsafeDecreaseAllowance()
instead.Source code:
Unsafe ERC20 calls
The following code doesn't check the result of the
transferFrom
orapprove
calls. ERC20 standard specify that the token can return false if these calls fails, so it's mandatory to check the result of these ERC20 methods.Source code:
Unsecure Ownership change
The modification of an owner is a delicate procedure, as it may jeopardize the governance of our contract and therefore the project. As a result, it is advised that the owner's modification logic be changed to one that allows for verification that the new owner is valid and exists.
It is required to establish a logic for the owner's modification in which a new owner is proposed first, the owner approves the suggestion, and we ensure that the new owner's address is written correctly.
Source code:
Possible Rogue pool
The owner of the
WETHGateway
contract has the ability to extract all existing WETH from the contract via therefreshAllowance
method. These capabilities must be limited in favor of decentralization, in order to avoid loss of reputation or user confidence.Source code: