validPeriods in StagedCrowdsale contract

[shingonu]

1. I think this line should uncommented and put it in L50. https://github.com/Onther-Tech/tokyo/blob/3c6f03243d5c8c4d28c730c40aaa365b42e793c0/packages/tokyo-reusable-crowdsale/audit/full-features/contracts/base/crowdsale/StagedCrowdsale.sol#L71
2. startTime[i] < endTime[i] condition check is required in the validPeriods function. https://github.com/Onther-Tech/tokyo/blob/3c6f03243d5c8c4d28c730c40aaa365b42e793c0/packages/tokyo-reusable-crowdsale/audit/full-features/contracts/base/crowdsale/StagedCrowdsale.sol#L74 like this:

   for (uint256 i = 0; i < periods.length; i++) {
     if (periods[i].endTime < periods[i].startTime) {
       return false;
     }
   }

3. Although the index(i) value is expected to be a small value, uint256 is safer than uint8 in for loop. https://github.com/Onther-Tech/tokyo/blob/3c6f03243d5c8c4d28c730c40aaa365b42e793c0/packages/tokyo-reusable-crowdsale/audit/full-features/contracts/base/crowdsale/StagedCrowdsale.sol#L80

4. this require statement does not seem necessary. This is because we already checked when we created the initPeriods function.

   if this require statement needs, it is need to check not only require(periods.length == numPeriods); but also startTime, endTime condition. So I think It's better to delete require(periods.length == numPeriods); , or to write require(validPeriods()) instead of require(periods.length == numPeriods); statement . https://github.com/Onther-Tech/tokyo/blob/3c6f03243d5c8c4d28c730c40aaa365b42e793c0/packages/tokyo-reusable-crowdsale/audit/full-features/contracts/base/crowdsale/StagedCrowdsale.sol#L110

[4000D]

1. should be fixed
2. should be false if endTime <= startTime
3. there is no reason uint256 is safer than uint8. periods.length is equal to uint8 numPeriods. no overflow occurs.
4. require(periods.length == numPeriods); makes sure that finishing sale before initPeriods() is invoked