Read the findings of the 2022 Web3 Security Report

The contract has unlocked compiler version. An unlocked compiler version in the source code of the contract permits the user to compile it at or above a particular version. This, in turn, leads to differences in the generated bytecode between compilations due to differing compiler version numbers. This can lead to an ambiguity when debugging as compiler specific bugs may occur in the codebase that would be hard to identify over a span of multiple compiler versions rather than a specific one.

Unlocked Compiler Version

`public` functions that are never called by the contract could be declared as `external`. `external` functions are more efficient than `public` functions and can save gas.

Function Visibility Optimization

There should always be events emitted in the sensitive functions that are controlled by centralization roles.

Missing Emit Events

The linked variables could be declared as `constant` since these state variables are never modified.

Variables That Could Be Declared As `constant`

In the contract, `GPO`, the role, `_owner`, has the authority over the functions shown in the diagram below.

Any compromise to the privileged account which has access to `_owner` may allow the hacker to take advantage of this and can pause the contract, switch on "Free Trade" or change pool parameters among other functionalities.

![](https://accelerator-tasks-prod.s3.amazonaws.com/29cf4418866d48d8a75e78310432b6ba/GPO-_owner.svg)

Centralization Risk in GPO.sol

Function `distributeFee()` has external calls inside a loop. In case any of the calls fail, it can cause the entire loop to revert. It is advice to change the reward distribution strategy.

External Call Inside a Loop

The function `setPoolParameters()` assigns the `addrUSDC` variable to the value obtained from the parameter without checking if it is the Zero address.

Missing Zero Address Validation

The comment inside function `setFeeSplits()` refers to a temporary functionality of allowing 0 fee splits but it is the default behavior.

Unimplemented Functionality

`transfer()`/`transferFrom()` invocations do not check the return value of the function call which should yield a `true` result in case of proper ERC-20 implementation.

Unchecked Value of ERC-20 `transfer()` Call

On the `_beforeTokenTransfer()` function, the `require` statement doesn't follow the comment above. If the user is on the whitelist, the `or` condition will suffice and it will ignore the `to != address(0x0)` condition, meaning that a user on the whitelist can burn tokens. Although this wouldn't take place as the `_transfer()` function of the ERC20 contract checks whether the `to` or `from` addresses are not the zero address before calling `_beforeTokenTransfer()`.

Inconsistent Comments and Code

The **require** can be used to check for conditions and throw an exception if the condition is not met. It is better to provide a string message containing details about the error that will be passed back to the caller.

Missing Error Messages

Function `setFeeSplits()` recieves an array of FeeSplits struct and adds their `fee` field to the variable `grandTotal`, which is then forced to equal the value `100` on the `require` statement. Given that the data type of the `fee` field is `uint16`, this condition will make the max amount of participants who can get rewards of the fees to be 100. There are no comments of such functionality.

Undocumented Functionality

On the `distributeFee()` function, every remnant the distribution is sent to the first address of the `feeSplits` array but given that  the array could have been given using the `setFeeSplits()` function, the first address of the array could be anything, including the Zero Address. This is not very helpful for transparency as users would not know if some address may be preferred over others and thus, placing it first on the array. 

Lack Of Transparency On Fees Distribution

The contract is serving as the underlying entity to interact with third party Uniswap protocols. The scope of the audit treats 3rd party entities as black boxes and assume their functional correctness. However, in the real world, 3rd parties can be compromised and this may lead to lost or stolen assets. In addition, upgrades of 3rd parties can possibly create severe impacts, such as increasing fees of 3rd parties, migrating to new LP pools, etc.

Third Party Dependencies

On functions `swapToExactOutput()`, `swapToExactInput()`, `swapFromExactOutput()` and `swapFromExactInput()` there is no validation of the variables containing the amounts of tokens to be swaped. If a user makes a swap with all the amounts to be swapped equal to zero, this would still be valid and would not only cause to waste gas on the swap but also on every transaction made by the `distributeFee()` function, which could lead to potential big loses.

Lack of Amount Check Before Token Swap

Functions `swapToExactOutput()`, `swapToExactInput()`, `swapFromExactOutput()` and `swapFromExactInput()` make an external call to the `addrUSDC` variable. Although the address should be pointing to the USDC Smart Contract address which should be trusted, it is assign via the `setPoolParameters()`. 

There is no mechanisms on the aforementioned functions to avoid the external call make another call to the same functions. In this case, this would be the same as interacting with the functions more than once, which doesn't have a great impact for the contract.

GoldPesa

Highlights & Alerts

This is just a placeholder

highlight placeholder lorem Ipsum...

highlight lorem

This is a alert demo

Alert demo

Code Audit History

16

0

12

1

3