Neo Tokyo contest - 0x6980's results

1. for modern and more readable code; update import usages

Context: All Contracts.

• BYTES2.sol#L4-L9

//File:contracts/staking/BYTES2.sol
4:  import "@openzeppelin/contracts/token/ERC20/ERC20.sol";
5:  import "@openzeppelin/contracts/token/ERC20/IERC20.sol";

7:  import "../access/PermitControl.sol";
8:  import "../interfaces/IByteContract.sol";
9:  import "../interfaces/IStaker.sol";

• NeoTokyoStaker.sol#L4-L8

//File:contracts/staking/NeoTokyoStaker.sol
4:  import "@openzeppelin/contracts/security/ReentrancyGuard.sol";

6:  import "../access/PermitControl.sol";
7:  import "../interfaces/IByteContract.sol";
8:  import "../interfaces/IGenericGetter.sol";

Recommendation: import {contract1 , contract2} from "filename.sol";

2. Function writing that does not comply with the Solidity Style Guide

Order of Functions; ordering helps readers identify which functions they can call and to find the constructor and fallback definitions easier. But there are contracts in the project that do not comply with this. soliditylang-style-guide Functions should be grouped according to their visibility and ordered:

constructor

receive function (if exists)

fallback function (if exists)

external

public

internal

private

within a grouping, place the view and pure functions last

3. Empty blocks should be removed or Emit something

Code contains empty block

• BYTES2.sol#L193-L194
• BYTES2.sol#L207-L208

193:	) external {
194:	}

207:	) external {
208:	}

Recommended Mitigation Steps

The code should be refactored such that they no longer exist, or the block should do something useful, such as emitting an event or reverting.

4. Showing the actual values of numbers in natspec comments makes checking and reading code easier.

• NeoTokyoStaker.sol#L203
• NeoTokyoStaker.sol#L197

197:	uint256 constant private _PRECISION = 1e12; // 1_000_000_000_000

203:	uint256 constant private _BYTES_PER_POINT = 200 * 1e18; 200_000_000_000_000_000_000

5. Use delete to clear variables instead of zero assignment.

You can use the delete keyword instead of setting the variable as zero.

• NeoTokyoStaker.sol#L1517-L1521

1517:		stakedCitizen.stakedBytes = 0;
1518:		stakedCitizen.timelockEndTime = 0;
1519:		stakedCitizen.points = 0;
1520:		stakedCitizen.hasVault = false;
1521:		stakedCitizen.stakedVaultId = 0;

6. Generate perfect code headers every time.

Description: I recommend using header for Solidity code layout and readability

Headers

/*//////////////////////////////////////////////////////////////
                           TESTING 123
//////////////////////////////////////////////////////////////*/

7. Missing ReEntrancy Guard to upgradeBytes function.

• BYTES2.sol#L93

93: 	function upgradeBytes (

8. Missing ReEntrancy Guard to getReward function

• BYTES2.sol#L114

114:	function getReward (