Lack of documentation
Description
While the white paper accurately describes the project and its mechanisms, there are certain areas in the code where a reaffirmation of the mechanisms would help with comprehensibility.
function newEra() external {
uint256 _era = latestEra.add(1);
require(currentEra() >= _era, "calEra not match");
// update era
latestEra = _era;
uint256 totalNewReward;
uint256 newTotalActive;
address[] memory poolList = getBondedPools();
for (uint256 i = 0; i < poolList.length; ++i) {
address poolAddress = poolList[i];
uint256[] memory validators = getValidatorIdsOf(poolAddress);
// newReward
uint256 poolNewReward = IStakePool(poolAddress).checkAndWithdrawRewards(validators);
emit NewReward(poolAddress, poolNewReward);
totalNewReward = totalNewReward.add(poolNewReward);
// unstakeClaimTokens
for (uint256 j = 0; j < validators.length; ++j) {
uint256 oldClaimedNonce = maxClaimedNonceOf[poolAddress][validators[j]];
uint256 newClaimedNonce = IStakePool(poolAddress).unstakeClaimTokens(validators[j], oldClaimedNonce);
if (newClaimedNonce > oldClaimedNonce) {
maxClaimedNonceOf[poolAddress][validators[j]] = newClaimedNonce;
emit NewClaimedNonce(poolAddress, validators[j], newClaimedNonce);
}
}
// ... 70 more lines
}
}
The newEra
function is a good example of this. The function is quite long (around 100 lines of code), and lacks clear documentation on its purpose. Moreover, it is shallowly commented, and the comments do not provide much insight into the function's purpose or what invariant it is trying to maintain.
Impact
Code maturity is very important in high-assurance projects. Undocumented code may result in developer confusion, potentially leading to future bugs should the code be modified in the future.
In general, a lack of documentation impedes the auditors' and external developers' abilities to read, understand, and extend the code. The problem is also carried over if the code is ever forked or reused.
Recommendations
We recommend adding more comments to the code --- especially comments that tie operations in code to locations in the white paper and brief comments to reaffirm developers' understanding.