Frankencoin - lukris02's results

QA Report for Frankencoin contest

Overview

During the audit, 3 low and 7 non-critical issues were found.

Low Risk Findings(3)

L-1. Function transferAndCall is not safe

Description

The function transferAndCall is vulnerable to reentrancy.

Instances

• function transferAndCall(address recipient, uint256 amount, bytes calldata data) external override returns (bool) {

Recommendation

Avoid using it.

L-2. Use SafeCast Library

Description

Downcasting from uint256/int256 in Solidity does not revert on overflow. This can easily result in undesired exploitation or bugs, since developers usually assume that overflows raise errors. SafeCast restores this intuition by reverting the transaction when such an operation overflows.

Instances

• totalVotesAtAnchor = uint192(totalVotes() - roundingLoss - lostVotes);

Recommendation

It is better to use safe casting library.

L-3. Check that amount to mint > 0

Instances

• _mint(_target, usableMint);
• _mint(address(reserve), _amount - usableMint); // rest goes to equity as reserves or as fees
• _mint(_target, _amount);
• _mint(msg.sender, _amount - reserveLeft);
• mint(msg.sender, amount);

Recommendation

For example, add the check:

if (usableMint == 0) revert ZeroAmount();
_mint(_target, usableMint);

Non-Critical Risk Findings(7)

NC-1. Order of Layout

Description

According to Order of Layout, inside each contract, library or interface, use the following order:

1. Type declarations
2. State variables
3. Events
4. Modifiers
5. Functions

Instances

• https://github.com/code-423n4/2023-04-frankencoin/blob/main/contracts/Position.sol#L366
• https://github.com/code-423n4/2023-04-frankencoin/blob/main/contracts/Position.sol#L373
• https://github.com/code-423n4/2023-04-frankencoin/blob/main/contracts/Position.sol#L380
• https://github.com/code-423n4/2023-04-frankencoin/blob/main/contracts/Position.sol#L387
• https://github.com/code-423n4/2023-04-frankencoin/blob/main/contracts/MintingHub.sol#L115
• https://github.com/code-423n4/2023-04-frankencoin/blob/main/contracts/Frankencoin.sol#L266
• https://github.com/code-423n4/2023-04-frankencoin/blob/main/contracts/Ownable.sol#L49

Recommendation

Place modifiers before constructor.

NC-2. Inconsistency when using the number 1000_000

Description

In some cases, 1000000 is used, and in some - 1000_000.

Instances

1000000:

• return minterReserveE6 / 1000000;
• uint256 theoreticalReserve = _reservePPM * mintedAmount / 1000000;
• return 1000000 * amountExcludingReserve / (1000000 - adjustedReservePPM); // 41 / (1-18%) = 50

1000_000:

• return totalMint * (1000_000 - reserveContribution - calculateCurrentFee()) / 1000_000;
• return totalMint * (1000_000 - reserveContribution) / 1000_000;
• uint256 reward = (volume * CHALLENGER_REWARD) / 1000_000;
• uint256 usableMint = (_amount * (1000_000 - _feesPPM - _reservePPM)) / 1000_000; // rounding down is fine

Recommendation

Stick to one style.

NC-3. Inconsistency when using uint and uint256

Description

Some variables is declared as uint and some as uint256.

Instances

There are 4 cases with uint when the rest are with uint256:

• event Trade(address who, int amount, uint totPrice, uint newprice);
• for (uint i=0; i<helpers.length; i++){
• for (uint j=i+1; j<helpers.length; j++){

Recommendation

Stick to one style.

NC-4. Prevent zero transfers

Description

Check that amount to transfer > 0.

Instances

• https://github.com/code-423n4/2023-04-frankencoin/blob/main/contracts/MintingHub.sol#L142
• https://github.com/code-423n4/2023-04-frankencoin/blob/main/contracts/Equity.sol#L279

NC-5. Natspec is incomplete

Instances

Parameter roundingLoss is missing.

* @notice Decrease the total votes anchor when tokens lose their voting power due to being moved
* @param from      sender
* @param amount    amount to be sent
*/

• function adjustTotalVotes(address from, uint256 amount, uint256 roundingLoss) internal {

Recommendation

Add the description for the roundingLoss.

NC-6. No space between the control structures

Description

According to Style Guide, there should be a single space between the control structures if, while, and for and the parenthetic block representing the conditional.

Instances

• if(_coll < minimumCollateral) revert InsufficientCollateral();

Recommendation

Change:

if(...) {
    ...
}

to:

if (...) {
    ...
}

NC-7. Missing leading underscores

Description

Internal and private constants, immutables and functions should have a leading underscore.

Instances

• function collateralBalance() internal view returns (uint256){
• function mintInternal(address target, uint256 amount, uint256 collateral_) internal {
• function restrictMinting(uint256 period) internal {
• function repayInternal(uint256 burnable) internal {
• function notifyRepaidInternal(uint256 amount) internal {
• function internalWithdrawCollateral(address target, uint256 amount) internal returns (uint256) {
• function checkCollateral(uint256 collateralReserve, uint256 atPrice) internal view {
• function emitUpdate() internal {
• IPositionFactory private immutable POSITION_FACTORY; // position contract to clone
• function minBid(Challenge storage challenge) internal view returns (uint256) {
• function returnCollateral(Challenge storage challenge, bool postpone) internal {
• uint256 private constant MINIMUM_EQUITY = 1000 * ONE_DEC18;
• uint32 private constant QUORUM = 300;
• uint8 private constant BLOCK_TIME_RESOLUTION_BITS = 24;
• uint192 private totalVotesAtAnchor; // Total number of votes at the anchor time, see comment on the um
• uint64 private totalVotesAnchorTime; // 40 Bit for the block number, 24 Bit sub-block time resolution
• mapping (address => uint64) private voteAnchor; // 40 Bit for the block number, 24 Bit sub-block time resolution
• function adjustTotalVotes(address from, uint256 amount, uint256 roundingLoss) internal {
• function adjustRecipientVoteAnchor(address to, uint256 amount) internal returns (uint256){
• function anchorTime() internal view returns (uint64){
• function canVoteFor(address delegate, address owner) internal view returns (bool) {
• function calculateSharesInternal(uint256 capitalBefore, uint256 investment) internal view returns (uint256) {
• uint256 private minterReserveE6;
• function allowanceInternal(address owner, address spender) internal view override returns (uint256) {
• uint256 internal constant INFINITY = (1 << 255);
• function allowanceInternal(address owner, address spender) internal view virtual returns (uint256) {
• function mintInternal(address target, uint256 amount) internal {
• function burnInternal(address zchfHolder, address target, uint256 amount) internal {
• function createClone(address target) internal returns (address result) {
• uint256 internal constant ONE_DEC18 = 10**18;
• uint256 internal constant THRESH_DEC18 = 10000000000000000;//0.01
• function setOwner(address newOwner) internal {
• function requireOwner(address sender) internal view {

Recommendation

Add leading underscores where needed.