QA Report

[code423n4]

QA REPORT

[LOW] In the following functions consider verifying the fee parameter

Where the fee parameter validation is checking greater than 0% (which may happen by mistake) and less than 100%

Example: Staking.sol#L406

[LOW] Approve 0 first

At some tokens you can approve an amount (at USDT for instance) only after approving to 0. Consider using increase/decrease approve notation instead.

Proof of concept:

• AllocationExchange.sol#L77
• GRTWithdrawHelper.sol#L65

[LOW] Missing pause functionality

Proof of concept:

• Governed.sol
• AllocationExchange.sol

[LOW] Not verified input

At the following functions you should verify the parameters that are being assigned to a state variable.

Proof of concept:

• L1GraphTokenGateway.sol#L121
• L2GraphToken.sol#L69

[LOW] The project is compiled with different solidity versions

[LOW] Payable functions that should not be payable

The following functions are payable but doesn't record the sender transaction. Consider making them not payable instead.

Proof of concept:

• L2GraphTokenGateway.sol#L232
• L1GraphTokenGateway.sol#L269

[LOW] Missing nonReentrancy modifier

The following functions allows attackers to try reentrancy since they are calling to external contracts / transferring eth. Consider adding a nonReentrancy modifier.

Proof of concept:

• L1GraphTokenGateway.sol#L152
• Multicall.sol#L17

[LOW] Consider replacing assert with require

Assertions are a bad practice, use require instead.

Proof of concept:

• GraphProxy.sol#L50
• GraphProxy.sol#L47

[LOW] Use safeApprove

Use safeApprove in the following locations

Proof of concept:

• AllocationExchange.sol#L77
• BridgeEscrow.sol#L28

[NON CRITICAL] NonReentrancy should be the first modifier in order

Example: L2GraphTokenGateway.sol#L232

[NON CRITICAL] Floating pragma

Floating pragma is a bad practice, since it does not guaranty the same version at future deployments.

Proof of concept:

• Controller.sol
• L2ArbitrumMessenger.sol

[NON CRITICAL] Consider emitting an event at the following functions

Proof of concept:

• GraphToken.sol#L64
• GraphTokenUpgradeable.sol#L150

[NON CRITICAL] Missing function spec comments

Proof of concept:

• GNS.sol#L855
• SubgraphNFT.sol#L107

[NON CRITICAL] Unused function parameters should have name removed

If for any reason the following unused parameters are necessary then remove their naming (since only the type matters for function signature)

Example: L2ArbitrumMessenger.sol#L42

[NON CRITICAL] The following events are not indexed

Proof of concept:

• Pausable.sol#L20
• L2GraphToken.sol#L30

[pcarranzav]

Some of the QA issues reported here are valid as noted in other reports, but it's worth noting several of the reported issues are for files that are out of scope.