Add revert requirement for Attribute Registry implementations

Author: 0age

contracts/AttributeRegistryInterface.sol

@@ -1,16 +1,17 @@
 pragma solidity ^0.4.25;
 
-
 /**
  * @title Attribute Registry interface. EIP-165 ID: 0x5f46473f
  */
 interface AttributeRegistryInterface {
   /**
    * @notice Check if an attribute of the type with ID `attributeTypeID` has
-   * been assigned to the account at `account` and is still valid.
+   * been assigned to the account at `account` and is currently valid.
    * @param account address The account to check for a valid attribute.
    * @param attributeTypeID uint256 The ID of the attribute type to check for.
    * @return True if the attribute is assigned and valid, false otherwise.
+   * @dev This function MUST return either true or false - i.e. calling this
+   * function MUST NOT cause the caller to revert.
    */
   function hasAttribute(
     address account,
@@ -23,6 +24,9 @@ interface AttributeRegistryInterface {
    * @param account address The account to check for the given attribute value.
    * @param attributeTypeID uint256 The ID of the attribute type to check for.
    * @return The attribute value if the attribute is valid, reverts otherwise.
+   * @dev This function MUST revert if a directly preceding or subsequent
+   * function call to `hasAttribute` with identical `account` and
+   * `attributeTypeID` parameters would return false.
    */
   function getAttributeValue(
     address account,
@@ -32,13 +36,19 @@ interface AttributeRegistryInterface {
   /**
    * @notice Count the number of attribute types defined by the registry.
    * @return The number of available attribute types.
+   * @dev This function MUST return a positive integer value  - i.e. calling
+   * this function MUST NOT cause the caller to revert.
    */
   function countAttributeTypes() external view returns (uint256);
 
   /**
    * @notice Get the ID of the attribute type at index `index`.
    * @param index uint256 The index of the attribute type in question.
    * @return The ID of the attribute type.
+   * @dev This function MUST revert if the provided `index` value falls outside
+   * of the range of the value returned from a directly preceding or subsequent
+   * function call to `countAttributeTypes`. It MUST NOT revert if the provided
+   * `index` value falls inside said range.
    */
   function getAttributeTypeID(uint256 index) external view returns (uint256);
 }

contracts/BasicJurisdiction.sol

@@ -398,10 +398,12 @@ contract BasicJurisdiction is Ownable, Pausable, AttributeRegistryInterface, Bas
 
   /**
    * @notice Check if an attribute of the type with ID `attributeTypeID` has
-   * been assigned to the account at `account` and is still valid.
+   * been assigned to the account at `account` and is currently valid.
    * @param account address The account to check for a valid attribute.
    * @param attributeTypeID uint256 The ID of the attribute type to check for.
    * @return True if the attribute is assigned and valid, false otherwise.
+   * @dev This function MUST return either true or false - i.e. calling this
+   * function MUST NOT cause the caller to revert.
    */
   function hasAttribute(
     address account, 
@@ -421,6 +423,9 @@ contract BasicJurisdiction is Ownable, Pausable, AttributeRegistryInterface, Bas
    * @param account address The account to check for the given attribute value.
    * @param attributeTypeID uint256 The ID of the attribute type to check for.
    * @return The attribute value if the attribute is valid, reverts otherwise.
+   * @dev This function MUST revert if a directly preceding or subsequent
+   * function call to `hasAttribute` with identical `account` and
+   * `attributeTypeID` parameters would return false.
    */
   function getAttributeValue(
     address account,
@@ -503,8 +508,10 @@ contract BasicJurisdiction is Ownable, Pausable, AttributeRegistryInterface, Bas
   }
 
   /**
-   * @notice Count the number of attribute types defined by the jurisdiction.
+   * @notice Count the number of attribute types defined by the registry.
    * @return The number of available attribute types.
+   * @dev This function MUST return a positive integer value  - i.e. calling
+   * this function MUST NOT cause the caller to revert.
    */
   function countAttributeTypes() external view returns (uint256) {
     return _attributeIDs.length;
@@ -514,8 +521,17 @@ contract BasicJurisdiction is Ownable, Pausable, AttributeRegistryInterface, Bas
    * @notice Get the ID of the attribute type at index `index`.
    * @param index uint256 The index of the attribute type in question.
    * @return The ID of the attribute type.
+   * @dev This function MUST revert if the provided `index` value falls outside
+   * of the range of the value returned from a directly preceding or subsequent
+   * function call to `countAttributeTypes`. It MUST NOT revert if the provided
+   * `index` value falls inside said range.
    */
   function getAttributeTypeID(uint256 index) external view returns (uint256) {
+    require(
+      index < _attributeIDs.length,
+      "provided index is outside of the range of defined attribute type IDs"
+    );
+
     return _attributeIDs[index];
   }
 

contracts/ExtendedJurisdiction.sol

@@ -1071,10 +1071,12 @@ contract ExtendedJurisdiction is Ownable, Pausable, AttributeRegistryInterface,
 
   /**
    * @notice Check if an attribute of the type with ID `attributeTypeID` has
-   * been assigned to the account at `account` and is still valid.
+   * been assigned to the account at `account` and is currently valid.
    * @param account address The account to check for a valid attribute.
    * @param attributeTypeID uint256 The ID of the attribute type to check for.
    * @return True if the attribute is assigned and valid, false otherwise.
+   * @dev This function MUST return either true or false - i.e. calling this
+   * function MUST NOT cause the caller to revert.
    */
   function hasAttribute(
     address account, 
@@ -1103,6 +1105,9 @@ contract ExtendedJurisdiction is Ownable, Pausable, AttributeRegistryInterface,
    * @param account address The account to check for the given attribute value.
    * @param attributeTypeID uint256 The ID of the attribute type to check for.
    * @return The attribute value if the attribute is valid, reverts otherwise.
+   * @dev This function MUST revert if a directly preceding or subsequent
+   * function call to `hasAttribute` with identical `account` and
+   * `attributeTypeID` parameters would return false.
    */
   function getAttributeValue(
     address account,
@@ -1247,8 +1252,10 @@ contract ExtendedJurisdiction is Ownable, Pausable, AttributeRegistryInterface,
   }
 
   /**
-   * @notice Count the number of attribute types defined by the jurisdiction.
+   * @notice Count the number of attribute types defined by the registry.
    * @return The number of available attribute types.
+   * @dev This function MUST return a positive integer value  - i.e. calling
+   * this function MUST NOT cause the caller to revert.
    */
   function countAttributeTypes() external view returns (uint256) {
     return _attributeIDs.length;
@@ -1258,8 +1265,17 @@ contract ExtendedJurisdiction is Ownable, Pausable, AttributeRegistryInterface,
    * @notice Get the ID of the attribute type at index `index`.
    * @param index uint256 The index of the attribute type in question.
    * @return The ID of the attribute type.
+   * @dev This function MUST revert if the provided `index` value falls outside
+   * of the range of the value returned from a directly preceding or subsequent
+   * function call to `countAttributeTypes`. It MUST NOT revert if the provided
+   * `index` value falls inside said range.
    */
   function getAttributeTypeID(uint256 index) external view returns (uint256) {
+    require(
+      index < _attributeIDs.length,
+      "provided index is outside of the range of defined attribute type IDs"
+    );
+
     return _attributeIDs[index];
   }
 

scripts/testBasic.js

@@ -945,6 +945,19 @@ module.exports = {test: async function (provider, testingContext) {
     failed++    
   })
 
+  await Jurisdiction.methods.getAttributeTypeID(
+    2
+  ).call({
+    from: address,
+    gas: 5000000,
+    gasPrice: 10 ** 1
+  }).catch(error => {
+    console.log(
+      " ✓  - Out-of-range attribute type IDs revert"
+    )
+    passed++
+  })
+
   await Jurisdiction.methods.getValidatorDescription(
     validator.address
   ).call({

scripts/testExtended.js

@@ -2005,6 +2005,19 @@ module.exports = {test: async function (provider, testingContext) {
     failed++    
   })
 
+  await Jurisdiction.methods.getAttributeTypeID(
+    2
+  ).call({
+    from: address,
+    gas: 5000000,
+    gasPrice: 10 ** 1
+  }).catch(error => {
+    console.log(
+      " ✓  - Out-of-range attribute type IDs revert"
+    )
+    passed++
+  })
+
   await Jurisdiction.methods.removeAttributeType(
     attribute.attributeId
   ).send({