feat: add valdation when reading mapping values

Author: rackstar

contracts/interfaces/IPermissionedAssessment.sol

@@ -132,6 +132,8 @@ interface IPermissionedAssessment {
   error ClaimIdsCidsLengthMismatch();
   error ClaimAssessmentNotFinished();
   error EmptyAssessorGroup();
-  error InvalidVote();
   error InvalidAssessor();
+  error InvalidClaimId();
+  error InvalidVote();
+  error InvalidProductType();
 }

contracts/modules/assessment/PermissionedAssessment.sol

@@ -48,17 +48,26 @@ contract PermissionedAssessment is IPermissionedAssessment, MasterAwareV2, Multi
   }
 
   function payoutCooldown(uint256 productTypeId) external view returns (uint256) {
-    return _assessmentData[productTypeId].cooldownPeriod;
+    // TODO: call CoverProduct to validate productTypeId?\
+    AssessmentData storage assessmentData = _assessmentData[productTypeId];
+    require(assessmentData.assessingGroupId != 0, InvalidProductType());
+    return assessmentData.cooldownPeriod;
   }
 
   function assessorGroupOf(bytes32 claimId) external view returns (uint32) {
-    return _assessments[claimId].assessorGroupId;
+    Assessment storage assessment = _assessments[claimId];
+    require(assessment.start != 0, InvalidClaimId());
+    return assessment.assessorGroupId;
   }
 
   function getAssessmentInfo(bytes32 claimId) external view returns (uint32 start, uint32 end, uint256 accepts, uint256 denies) {
 
     Assessment storage assessment = _assessments[claimId];
+    require(assessment.start != 0, InvalidClaimId());
+
     EnumerableSet.UintSet storage assessorGroup = _assessorGroups[assessment.assessorGroupId];
+    require(assessorGroup.length() > 0, EmptyAssessorGroup());
+
     (accepts, denies) = _getVoteTally(claimId, assessorGroup);
 
     return (assessment.start, assessment.end, accepts, denies);
@@ -86,9 +95,12 @@ contract PermissionedAssessment is IPermissionedAssessment, MasterAwareV2, Multi
   function getOutcome(bytes32 claimId) external view returns (bool accepted) {
 
     Assessment storage assessment = _assessments[claimId];
-    EnumerableSet.UintSet storage assessorGroup = _assessorGroups[assessment.assessorGroupId];
+    require(assessment.start != 0, InvalidClaimId());
 
     // Check if the assessment has been decided (has votes, not a draw and voting period has ended)
+    EnumerableSet.UintSet storage assessorGroup = _assessorGroups[assessment.assessorGroupId];
+    require(assessorGroup.length() > 0, EmptyAssessorGroup());
+
     (uint256 acceptCount, uint256 denyCount) = _getVoteTally(claimId, assessorGroup);
     require(_isAssessmentDecided(acceptCount, denyCount, assessment), ClaimAssessmentNotFinished());
 
@@ -103,7 +115,11 @@ contract PermissionedAssessment is IPermissionedAssessment, MasterAwareV2, Multi
   function isAssessmentDecided(bytes32 claimId) external view returns (bool) {
 
     Assessment storage assessment = _assessments[claimId];
+    require(assessment.start != 0, InvalidClaimId());
+
     EnumerableSet.UintSet storage assessorGroup = _assessorGroups[assessment.assessorGroupId];
+    require(assessorGroup.length() > 0, EmptyAssessorGroup());
+
     (uint256 acceptCount, uint256 denyCount) = _getVoteTally(claimId, assessorGroup);
 
     return _isAssessmentDecided(acceptCount, denyCount, assessment);
@@ -116,7 +132,11 @@ contract PermissionedAssessment is IPermissionedAssessment, MasterAwareV2, Multi
   /// @dev This function considers only votes from current assessors in the group
   function getVoteTally(bytes32 claimId) external view returns (uint256 acceptCount, uint256 denyCount) {
     Assessment storage assessment = _assessments[claimId];
+    require(assessment.start != 0, InvalidClaimId());
+
     EnumerableSet.UintSet storage assessorGroup = _assessorGroups[assessment.assessorGroupId];
+    require(assessorGroup.length() > 0, EmptyAssessorGroup());
+
     return _getVoteTally(claimId, assessorGroup);
   }
 
@@ -128,10 +148,14 @@ contract PermissionedAssessment is IPermissionedAssessment, MasterAwareV2, Multi
   /// @dev Only callable by internal contracts
   /// @dev Reverts if an assessment already exists for the given claimId
   function startAssessment(bytes32 claimId, uint16 productTypeId) external onlyInternal {
+    // TODO: call CoverProduct to validate productTypeId?
 
     Assessment storage assessment = _assessments[claimId];
     require(assessment.start == 0, AssessmentAlreadyExists());
 
+    AssessmentData storage assessmentData = _assessmentData[productTypeId];
+    require(assessmentData.assessingGroupId != 0, InvalidProductType());
+
     assessment.start = uint32(block.timestamp);
     assessment.end = uint32(block.timestamp + MIN_VOTING_PERIOD);
     assessment.assessorGroupId = _assessmentData[productTypeId].assessingGroupId;
@@ -152,6 +176,7 @@ contract PermissionedAssessment is IPermissionedAssessment, MasterAwareV2, Multi
     // Validate assessor and get assessment data
     (uint256 assessorMemberId, Assessment storage assessment) = _validateAssessor(claimId, msg.sender);
     EnumerableSet.UintSet storage assessorGroup = _assessorGroups[assessment.assessorGroupId];
+    require(assessorGroup.length() > 0, EmptyAssessorGroup());
 
     // Only allow voting if the poll is not yet decided (no votes, a draw or voting period hasn't ended)
     (uint256 acceptCount, uint256 denyCount) = _getVoteTally(claimId, assessorGroup);
@@ -168,6 +193,7 @@ contract PermissionedAssessment is IPermissionedAssessment, MasterAwareV2, Multi
     (acceptCount, denyCount) = _getVoteTally(claimId, assessorGroup);
 
     // Check if we can close the poll early
+    // NOTE: the check against assessorGroup being empty is done by _validateAssessory
     bool allVoted = acceptCount + denyCount == assessorGroup.length();
     bool notADraw = acceptCount != denyCount;
     bool canCloseEarly = allVoted && notADraw;
@@ -197,7 +223,7 @@ contract PermissionedAssessment is IPermissionedAssessment, MasterAwareV2, Multi
   /// @param assessment The assessment data for the claim
   /// @return true if the assessment is decided, false otherwise
   function _isAssessmentDecided(uint256 acceptCount, uint256 denyCount, Assessment storage assessment) internal view returns (bool) {
-    // The assessment is considered still open if it's a draw (includes case of no votes where 0 == 0)
+    // The assessment is considered still open if it's a draw, or no votes (0 == 0)
     if (acceptCount == denyCount) return false;
 
     // The assessment is considered decided if there is at least 1 vote and the voting period has ended
@@ -215,11 +241,12 @@ contract PermissionedAssessment is IPermissionedAssessment, MasterAwareV2, Multi
   ) internal view returns (uint256 acceptCount, uint256 denyCount) {
 
     Assessment storage assessment = _assessments[claimId];
+    require(assessment.start != 0, InvalidClaimId());
+
     acceptCount = 0;
     denyCount = 0;
 
     uint256 length = assessorGroup.length();
-    require(length > 0, EmptyAssessorGroup());
 
     for (uint i = 0; i < length;) {
         uint256 assessorMemberId = assessorGroup.at(i);
@@ -242,6 +269,7 @@ contract PermissionedAssessment is IPermissionedAssessment, MasterAwareV2, Multi
     // TODO: implement memberRoles.getMemberId - can be memberId be 0?
     assessorMemberId = _memberRoles().getMemberId(assessor);
     assessment = _assessments[claimId];
+    require(assessment.start != 0, InvalidClaimId());
     require(_assessorGroups[assessment.assessorGroupId].contains(assessorMemberId), InvalidAssessor());
     return (assessorMemberId, assessment);
   }