Roundabout way of calculating L2GraphToken address
In L2GraphTokenGateway at L156 and L236 the L2GraphToken address is calculated as calculateL2TokenAddress(l1GRT), but this always just returns address(graphToken()). Consider skipping the check performed in calculateL2TokenAddress by replacing calculateL2TokenAddress(l1GRT) with address(graphToken()) at L156 and L236.
No need for SafeMath for user protection
SafeMath is not needed here as it is only a check to protect the user against his own invalid inputs.
Don't allow invalid parameters with a unique valid value
outboundTransfer() and finalizeInboundTransfer(), in both L1GraphTokenGateway.sol and L2GraphTokenGateway.sol, have as first parameter _l1Token which is immediately checked for its only acceptable value. Instead of checking whether its value is correct, leave the parameter void and simply declare its value inside the function.
Cache variable in memory
Move Governed.sol#L60 to before L54 so you can use the cached oldPendingGovernor instead of pendingGovernor at L55. (Then also emit oldPendingGovernor instead of governor at L65.)
Avoid accessing a storage variable when a cheaper alternative is available
Roundabout way of calculating L2GraphToken address
In L2GraphTokenGateway at L156 and L236 the L2GraphToken address is calculated as
calculateL2TokenAddress(l1GRT)
, but this always just returnsaddress(graphToken())
. Consider skipping the check performed incalculateL2TokenAddress
by replacingcalculateL2TokenAddress(l1GRT)
withaddress(graphToken())
at L156 and L236.No need for SafeMath for user protection
SafeMath is not needed here as it is only a check to protect the user against his own invalid inputs.
Comparing to constant bool
Replace e.g.
foo == false
with!foo
.callhookWhitelist[msg.sender] == true
Unused function
Consider removing the unused function
Managed.graphTokenGateway()
.Unnecessary address(0)-check
_pendingImplementation != address(0) && msg.sender == _pendingImplementation
can be simplified tomsg.sender == _pendingImplementation
asmsg.sender
cannot beaddress(0)
.pendingGovernor != address(0) && msg.sender == pendingGovernor
can be simplified tomsg.sender == pendingGovernor
asmsg.sender
cannotaddress(0)
._escrow != address(0) && Address.isContract(_escrow)
can be simplified toAddress.isContract(_escrow)
asaddress(0)
cannot be a contract.Don't allow invalid parameters with a unique valid value
outboundTransfer()
andfinalizeInboundTransfer()
, in both L1GraphTokenGateway.sol and L2GraphTokenGateway.sol, have as first parameter_l1Token
which is immediately checked for its only acceptable value. Instead of checking whether its value is correct, leave the parameter void and simply declare its value inside the function.Cache variable in memory
Move Governed.sol#L60 to before L54 so you can use the cached
oldPendingGovernor
instead ofpendingGovernor
at L55. (Then also emitoldPendingGovernor
instead ofgovernor
at L65.)Avoid accessing a storage variable when a cheaper alternative is available
Replace
pendingGovernor
with_newGovernor
at Governed.sol#L46.Replace
pendingGovernor
withaddress(0)
at Governed.sol#L66.Replace
_partialPaused
with_toPause
at Pausable.sol#L31 and L34.Replace
_paused
with_toPause
at Pausable.sol#L45 and L48.Replace
pauseGuardian
withnewPauseGuardian
at Pausable.sol#L58Replace
gateway
with_gw
at L2GraphToken.sol#L62<
is cheaper than<=
https://github.com/code-423n4/2022-10-thegraph/blob/309a188f7215fa42c745b136357702400f91b4ff/contracts/l2/token/GraphTokenUpgradeable.sol#L95