search

code-423n4 / 2022-06-connext-findings

1 stars 0 forks source link

QA Report #207

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Non-critical: No event emitted for some important state changes

A number of functions make important updates to the state of the protocol but don't emit events to log the change.

For users and indexers of the protocol it would be beneficial to emit events in the following functions:

InfinityExchange

InfinityStaker

Non-critical: Native ETH represented by the zero address

The zero address is also the default value assigned to an address that is uninitialized. So there is some risk that buggy client code that does not properly setup the MakerOrder struct could default to native eth when it wasn't intended.

A safer option would be to designate some other non-zero address for native ETH to ensure it was explicitly intended.

An example of this can be found in the Kyber Network where 0xEeeeeEeeeEeEeeEeEeEeeEEEeeeeEeeeeeeeEEeE is used to represent native ETH. See the docs here https://docs.kyberswap.com/api/#tag/swap.

Documenting this on addCurrency and _currencies would be helpful too. This will make it immediately clear to the reader that native ETH is represented with a special address.

Non-critical: Mutable state variable names in CAPS

It's confusing for readers of the code and auditors to see mutable variables names in all caps.

The following variable names should change to mixed / camel case so it's clear they are mutable:

InfinityExchange

InfinityStaker

Non-critical: INFINITY_TOKEN should be marked immutable

It appears INFINITY_TOKEN won't change and if that is the case marking it immutable will guarantee it can't be changed.

Non-critical: Typos in natspec comments

"Storate" (Storage) on InfinityExchange._currencies "mesage" (message) on SignatureChecker.recover

jakekidd commented 2 years ago

This seems to be a report for something else