ForecasterReward audit review

AuditReview.md

Note: No critical issue found that can cause loss of funds or locking of funds.

Section 5 - Audit findings

5.1 Note Issues

5.1.1 Event parameters not indexed

Type: Enhancement
Status: fixed
Comment: Indexed parameters are used for searching logs. Contract was no relying on logs but store records explicitly in storage. According to auditors I must rely on logs so from this perspective indexing is important.

5.1.2 Use of if(...) throw; instead of require/assert

Type: Voilation of Good Practice
Status: fixed
Comment: Auditors suggested to use require / assert becuase it increase readibility and help in audit.

5.1.3 Inline precondition checks in place of modifiers

Type: Voilation of Good Practice
Status: fixed
Comment: Redundant conditions are now replaced with a modifier.

5.1.4 Constructor does not check start/end preconditions

Type: Precondition missing
Status: fixed
Comment: This issue make possible to input start and end block from past at time of deployment.

5.1.5 Inconsistent division of responsibilities between internal and external funding methods

Type: Usability Issue
Status: fixed
Comment: Now investor can invest without calling a specific function.

5.1.6 Pointless fundingRaised / availableFunding methods

Type: Redundant Code
Status: fixed
Comment: There were getters for two public variables.

5.2 Low Issues

5.2.1 Use of block numbers in place of timestamps

Type: Auditors Suggestion
Status: fixed
Comment: According to ConsenSys smart contract best practices guide and remix IDE warnings it is not recommended to use blocks timestamps because it can be influenced by miners. But auditors believe that miner can influence timestamps it is very limited.

5.2.2 Use of redundant contract storage

Type: Storage Wast
Status: fixed
Comment: I could not estimate to the power and reliability of event logs so decided to store event in contract but according to investors I must rely on logs.

5.2.3 setEndsAt impedes terminating the crowdsale early

Type: Bug
Status: fixed
Comment: This issues could cause a delay of 10 minutes in closing of crowdsale.

5.3 Medium Issues

5.3.1 setEndsAt permits extension or restart of crowdsale

Type: Voilation of Good Practice
Status: Not Fixed
Comment: According to requirements owner must be able to close or extened crowdsale duration.

5.3.2 Fallback function does not permit crowdsale contribution

Type: Usability Issue
Status: fixed
Comment: Now investor can invest without calling a specific function.

5.3.3 Finalize is subject to extortion attack by contract owner

Type: Due to requirments
Status: fixed
Comment: This is no more relevant because of 5.3.4 fix.

5.3.4 Unnecessary accumulation of funds in contract

Type: Due to requirments
Status: fixed
Comment: Forecaster address is not available at time of deployment or even by the end of pre ico.

5.3.5 Incorrect use of Transfer event in finalize

Type: Error
Status: fixed
Comment: Incorrect address was getting logged in an event.

5.3.6 setFinalize permits changes to funding targets at any time before the crowdsale begins

Type: Due to requirments
Status: fixed
Comment: Forecaster address is not available at time of deployment or even by the end of pre ico.