Frankencoin - EloiManuel's results

Judge has assessed an item in Issue #964 as 2 risk. The relevant finding follows:

L1

Summary

Note that this is my first submission at Code4rena and I am still figuring out how things work. I could not spend much time on this.

Issues

Low Risk Issues

Non-critical Risk Issues

[L-01] restructureCapTable() incorrect burning mechanism

The function restructureCapTable() in Equity.sol is supposed to burn the balance of all addresses in addressesToWipe[]. However, it would only do it for the first element as per line 313 shown below:

contracts/Equity.sol

309:    function restructureCapTable(address[] calldata helpers, address[] calldata addressesToWipe) public {
310:    require(zchf.equity() < MINIMUM_EQUITY);
311:    checkQualified(msg.sender, helpers);
312:    for (uint256 i = 0; i<addressesToWipe.length; i++){
313:        address current = addressesToWipe[0];
314:        _burn(current, balanceOf(current));
315:    }

Note that in the event using addressesToWipe with a considerable number of elements, it would only burn the tokens of the first address though using a lot of gas.

Recommended Mitigation Steps

Change address current = addressesToWipe[0]; to address current = addressesToWipe[i].

[NC-01] Missing or vague error messages

Some require statements do not have an error message or are the error messages they have are not clear.

contracts/Position.sol

53: require(initPeriod >= 3 days); // must be at least three days, recommended to use higher values

contracts/Equity.sol

276: require(canRedeem(msg.sender));

contracts/StablecoinBridge.sol

51:    require(block.timestamp <= horizon, "expired");
52:    require(chf.balanceOf(address(this)) <= limit, "limit");

[NC-02] Constants should be defined rather than using magic numbers

A magic number is a numeric literal that is used in the code without any explanation of its meaning. The use of magic numbers makes programs less readable and hence more difficult to maintain and update. Even assembly can benefit from using readable constants instead of hex/numeric literals

contracts/Frankencoin.sol

118:      return minterReserveE6 / 1000000;

166:      uint256 usableMint = (_amount * (1000_000 - _feesPPM - _reservePPM)) / 1000_000; // rounding down is fine

205:      uint256 theoreticalReserve = _reservePPM * mintedAmount / 1000000;

239:      return 1000000 * amountExcludingReserve / (1000000 - adjustedReservePPM); // 41 / (1-18%) = 50

[NC-03] Inconsistent Documentation

The documentation (https://docs.frankencoin.com/positions/open) says that this is the function to open a position:

	openPosition(
			address _collateralAddress, uint256 _minCollateral, uint256 _initialCollateral,
			uint256 _mintingMaximum, uint256 _expirationSeconds, uint256 _challengeSeconds,
			uint32 _mintingFeePPM, uint256 _liqPriceE18, uint32 _reservePPM)

It also says: "When opening a position, it is already provided with some collateral, but nothing is minted yet. Minting is not possible until the initialization period of seven days has passed." However, MintingHub also has:

    function openPosition(
        address _collateralAddress, uint256 _minCollateral, uint256 _initialCollateral,
        uint256 _mintingMaximum, uint256 _initPeriodSeconds, uint256 _expirationSeconds, uint256 _challengeSeconds,
        uint32 _mintingFeePPM, uint256 _liqPrice, uint32 _reservePPM) 

which let's the user choose another value other than 7 days (up to 3 days or more).

Furthermore, on Equity.sol, it says that equity could be negative when it is using a uint:

300:     * If there is less than 1000 ZCHF in equity left (maybe even negative),

[NC-04] | Unnecessary Import

The following import in PositionFactory.sol was not found to be necessary:

5:  import "./IFrankencoin.sol";