ENS - nmirchev8's results

QA Analysis

Low Vulnerabilities

1. Low: The emit DelegationProcessed(source, target, amount) event could be spammed if the provided amount is set to 0. Consider implementing a threshold or additional checks to prevent spamming.

2. Low: Funds sent directly to the proxy contract will be stuck and used only for voting.

3. Low: Best practices suggest checking addresses for address(0) as the current implementation allows delegating funds to/from address(0)(if inside the target array there are address(0)s), which will only lock delegators' funds and lead to useless ERC1155 accounting, as the result of balances[address(0)][delegator] may arrise, but address votesOf(address(0)) will remain 0

Analysis

• using Address for address Address library from OZ util package is not used, so it could be removed
• Caching Math.min() could save gas.
• Consider marking private/internal functions with a prefix like "_" (e.g., _getBalanceForDelegate()) to clearly distinguish them from externally accessible functions.

Test File Feedback

• Optimization: The code for delegate multi-calls could be extracted into a fixture as it is used in multiple tests to avoid redundancy and improve code maintainability. Example:

  const delegatorTokenAmount = await token.balanceOf(deployer);
  const delegates = [deployer, alice];
  const amounts = delegates.map(() => delegatorTokenAmount.div(delegates.length));
  
  // ... rest of the code ...

• Inaccurate Test Name: The test named it('should be able to delegate to already delegated delegates') doesn't perform the behavior described. Only deployer has funds to delegate and all other tests of different delegators are irrelevant to real world scenarios. To simulate the intended behavior, assets should be transferred to the secondDelegator in the beforeEach section:

  const [dep, second] = await ethers.getSigners();
  const halfBalance = mintAmount.div(2);
  await token.transfer(second.address, halfBalance);