Phuture Finance contest - sseefried's results

QA Report

Remarks/Recommendations

• A test suite would have been very useful. A comprehensive test suite has two major benefits:

  • it serves as documentation for how the system is used in practice. It has the added benefit of being more precise than describing the system in English.
  • it can be extended by auditors to write Proofs of Concept of exploits. I like to do this myself and then provide links to my GitHub to help the sponsors. Here is an example of a small bug I found during the Paladin contest.

• In general, it was quite hard to work out how all these contracts were used together in practice. Tests would have aided my understanding. Failing that, a worked example of just which functions were called in each contract would have been useful. The documentation provided here was imprecise. A more precise example of what would have been useful appears below. (Be aware, I don't know if this is correct. It's just an example to give a flavour of the level of precision that would be useful)

  1. User deposits ERC20 tokens to the address of an instance of the IndexLogic contract.
  2. User calls mint function passing their address as _recipient parameter.
  3. ... etc ...

Low Risk: Weights can be calculated in such a way as to not add up to 255

Impact

A weight between 0 and 255 is too low precision in order to guarantee that the weights in an index always add up to 255. This can lead to incorrect calculations of the value/price of the index.

Proof of Concept

Consider the following scenario:

• token A capitalization = 1
• token B capitalization = 9999

Then we would get weights:

• (1 * 255) / 10000 = 0
• (9999 * 255) / 1000 = 254

These do not add up to 255

Tools used

Manual inspection

Recommended Mitigation Steps

Use a value larger than uint8 to represent weights. There is little reason to restrict oneself to a small integer. Also, consider using a fractional value instead.