feat: rename variables and natspec updates

Author: efecarranza

docs/remote-facilitator.md

@@ -22,7 +22,7 @@ Remote facilitators function similarly to those on Ethereum, but the way GHO is
 
 [On Ethereum]
 
-1. An `GhoDirectFacilitator` contract is deployed to represent the minting strategy on the remote chain. It is granted the `Facilitator` role on the Ethereum `GhoToken` contract, with a defined bucket capacity X.
+1. A `GhoDirectFacilitator` contract is deployed to represent the minting strategy on the remote chain. It is granted the `Facilitator` role on the Ethereum `GhoToken` contract, with a defined bucket capacity of X.
 2. An amount X of GHO is minted via the `GhoDirectFacilitator` and bridged to the remote chain (typically via `CCIP`).
 
 [On the Remote Chain]

src/contracts/facilitators/gsm/GhoDirectFacilitator.sol

@@ -8,7 +8,7 @@ import {IGhoDirectFacilitator} from 'src/contracts/facilitators/gsm/interfaces/I
 /**
  * @title GhoDirectFacilitator
  * @author Aave/TokenLogic
- * @notice GHO Facilitator used to directly mint GHO to a given address.
+ * @notice Basic facilitator for minting and burning tokens to and from an account, controlled by an access control list.
  */
 contract GhoDirectFacilitator is AccessControl, IGhoDirectFacilitator {
   /// @inheritdoc IGhoDirectFacilitator
@@ -23,17 +23,17 @@ contract GhoDirectFacilitator is AccessControl, IGhoDirectFacilitator {
   /**
    * @dev Constructor
    * @param admin The address of the initial owner
-   * @param ghoAddress The address of GHO token
+   * @param gho The address of GHO token
    */
-  constructor(address admin, address ghoAddress) {
+  constructor(address admin, address gho) {
     require(admin != address(0), 'ZERO_ADDRESS_NOT_VALID');
-    require(ghoAddress != address(0), 'ZERO_ADDRESS_NOT_VALID');
+    require(gho != address(0), 'ZERO_ADDRESS_NOT_VALID');
 
     _grantRole(DEFAULT_ADMIN_ROLE, admin);
     _grantRole(MINTER_ROLE, admin);
     _grantRole(BURNER_ROLE, admin);
 
-    GHO_TOKEN = ghoAddress;
+    GHO_TOKEN = gho;
   }
 
   /// @inheritdoc IGhoDirectFacilitator

src/contracts/facilitators/gsm/GhoReserve.sol

@@ -19,10 +19,10 @@ contract GhoReserve is AccessControl, VersionedInitializable, IGhoReserve {
   using SafeCast for uint256;
 
   /// @inheritdoc IGhoReserve
-  bytes32 public constant MANAGE_ENTITY_ROLE = keccak256('MANAGE_ENTITY_ROLE');
+  bytes32 public constant ENTITY_MANAGER_ROLE = keccak256('ENTITY_MANAGER_ROLE');
 
   /// @inheritdoc IGhoReserve
-  bytes32 public constant SET_LIMIT_ROLE = keccak256('SET_LIMIT_ROLE');
+  bytes32 public constant LIMIT_MANAGER_ROLE = keccak256('LIMIT_MANAGER_ROLE');
 
   /// @inheritdoc IGhoReserve
   bytes32 public constant TRANSFER_ROLE = keccak256('TRANSFER_ROLE');
@@ -38,11 +38,11 @@ contract GhoReserve is AccessControl, VersionedInitializable, IGhoReserve {
 
   /**
    * @dev Constructor
-   * @param ghoAddress The address of the GHO token on the remote chain
+   * @param gho The address of the GHO token on the remote chain
    */
-  constructor(address ghoAddress) {
-    require(ghoAddress != address(0), 'ZERO_ADDRESS_NOT_VALID');
-    GHO_TOKEN = ghoAddress;
+  constructor(address gho) {
+    require(gho != address(0), 'ZERO_ADDRESS_NOT_VALID');
+    GHO_TOKEN = gho;
   }
 
   /**
@@ -53,8 +53,8 @@ contract GhoReserve is AccessControl, VersionedInitializable, IGhoReserve {
     require(admin != address(0), 'ZERO_ADDRESS_NOT_VALID');
 
     _grantRole(DEFAULT_ADMIN_ROLE, admin);
-    _grantRole(MANAGE_ENTITY_ROLE, admin);
-    _grantRole(SET_LIMIT_ROLE, admin);
+    _grantRole(ENTITY_MANAGER_ROLE, admin);
+    _grantRole(LIMIT_MANAGER_ROLE, admin);
     _grantRole(TRANSFER_ROLE, admin);
   }
 
@@ -82,13 +82,13 @@ contract GhoReserve is AccessControl, VersionedInitializable, IGhoReserve {
   }
 
   /// @inheritdoc IGhoReserve
-  function addEntity(address entity) external onlyRole(MANAGE_ENTITY_ROLE) {
+  function addEntity(address entity) external onlyRole(ENTITY_MANAGER_ROLE) {
     require(_entities.add(entity), 'ENTITY_ALREADY_EXISTS');
     emit EntityAdded(entity);
   }
 
   /// @inheritdoc IGhoReserve
-  function removeEntity(address entity) external onlyRole(MANAGE_ENTITY_ROLE) {
+  function removeEntity(address entity) external onlyRole(ENTITY_MANAGER_ROLE) {
     GhoUsage memory usage = _ghoUsage[entity];
     require(usage.used == 0, 'ENTITY_GHO_USED_NOT_ZERO');
     require(usage.limit == 0, 'ENTITY_GHO_LIMIT_NOT_ZERO');
@@ -98,7 +98,7 @@ contract GhoReserve is AccessControl, VersionedInitializable, IGhoReserve {
   }
 
   /// @inheritdoc IGhoReserve
-  function setLimit(address entity, uint256 limit) external onlyRole(SET_LIMIT_ROLE) {
+  function setLimit(address entity, uint256 limit) external onlyRole(LIMIT_MANAGER_ROLE) {
     require(_entities.contains(entity), 'ENTITY_DOES_NOT_EXIST');
     _ghoUsage[entity].limit = limit.toUint128();
 

src/contracts/facilitators/gsm/interfaces/IGhoDirectFacilitator.sol

@@ -1,23 +1,25 @@
 // SPDX-License-Identifier: MIT
 pragma solidity ^0.8.0;
 
+import {IAccessControl} from 'src/contracts/dependencies/openzeppelin-contracts/contracts/access/IAccessControl.sol';
+
 /**
  * @title IGhoDirectFacilitator
  * @author Aave/TokenLogic
  * @notice Defines the behaviour of an GhoDirectFacilitator
  */
-interface IGhoDirectFacilitator {
+interface IGhoDirectFacilitator is IAccessControl {
   /**
    * @notice Mint an amount of GHO to an address
-   * @dev Only callable by address with MINTER_ROLE.
+   @dev Only callable by addresses with `MINTER_ROLE` role..
    * @param account The address receiving GHO
    * @param amount The amount of GHO to be minted
    */
   function mint(address account, uint256 amount) external;
 
   /**
    * @notice Burns an amount of GHO
-   * @dev Only callable by address with BURNER_ROLE.
+   * @dev Only callable by addresses with `BURNER_ROLE` role.
    * @param amount The amount of GHO to be burned
    */
   function burn(uint256 amount) external;

src/contracts/facilitators/gsm/interfaces/IGhoReserve.sol

@@ -1,12 +1,14 @@
 // SPDX-License-Identifier: MIT
 pragma solidity ^0.8.0;
 
+import {IAccessControl} from 'src/contracts/dependencies/openzeppelin-contracts/contracts/access/IAccessControl.sol';
+
 /**
  * @title IGhoReserve
  * @author Aave/TokenLogic
  * @notice Defines the behaviour of a GhoReserve
  */
-interface IGhoReserve {
+interface IGhoReserve is IAccessControl {
   /**
    * @dev Struct data representing GHO usage.
    * @param limit The maximum amount of GHO that can be used
@@ -103,13 +105,13 @@ interface IGhoReserve {
    * @notice Returns the identifier of the ManageEntity Role
    * @return The bytes32 id hash of the ManageEntity role
    */
-  function MANAGE_ENTITY_ROLE() external pure returns (bytes32);
+  function ENTITY_MANAGER_ROLE() external pure returns (bytes32);
 
   /**
    * @notice Returns the identifier of the SetLimit Role
    * @return The bytes32 id hash of the SetLimit role
    */
-  function SET_LIMIT_ROLE() external pure returns (bytes32);
+  function LIMIT_MANAGER_ROLE() external pure returns (bytes32);
 
   /**
    * @notice Returns the identifier of the Transfer Role

tests/helpers/Constants.sol

@@ -31,8 +31,8 @@ contract Constants {
   bytes32 public constant BURNER_ROLE = keccak256('MINTER_ROLE');
 
   // admin role for GhoReserve
-  bytes32 public constant MANAGE_ENTITY_ROLE = keccak256('MANAGE_ENTITY_ROLE');
-  bytes32 public constant SET_LIMIT_ROLE = keccak256('SET_LIMIT_ROLE');
+  bytes32 public constant ENTITY_MANAGER_ROLE = keccak256('ENTITY_MANAGER_ROLE');
+  bytes32 public constant LIMIT_MANAGER_ROLE = keccak256('LIMIT_MANAGER_ROLE');
   bytes32 public constant TRANSFER_ROLE = keccak256('TRANSFER_ROLE');
 
   // signature typehash for GSM

tests/unit/TestGhoReserve.t.sol

@@ -19,15 +19,15 @@ contract TestGhoReserve is TestGhoBase {
     GhoReserve reserve = new GhoReserve(address(GHO_TOKEN));
 
     assertFalse(reserve.hasRole(DEFAULT_ADMIN_ROLE, address(this)));
-    assertFalse(reserve.hasRole(MANAGE_ENTITY_ROLE, address(this)));
-    assertFalse(reserve.hasRole(SET_LIMIT_ROLE, address(this)));
+    assertFalse(reserve.hasRole(ENTITY_MANAGER_ROLE, address(this)));
+    assertFalse(reserve.hasRole(LIMIT_MANAGER_ROLE, address(this)));
     assertFalse(reserve.hasRole(TRANSFER_ROLE, address(this)));
 
     reserve.initialize(address(this));
 
     assertTrue(reserve.hasRole(DEFAULT_ADMIN_ROLE, address(this)));
-    assertTrue(reserve.hasRole(MANAGE_ENTITY_ROLE, address(this)));
-    assertTrue(reserve.hasRole(SET_LIMIT_ROLE, address(this)));
+    assertTrue(reserve.hasRole(ENTITY_MANAGER_ROLE, address(this)));
+    assertTrue(reserve.hasRole(LIMIT_MANAGER_ROLE, address(this)));
     assertTrue(reserve.hasRole(TRANSFER_ROLE, address(this)));
   }
 
@@ -167,7 +167,7 @@ contract TestGhoReserve is TestGhoBase {
   }
 
   function testRevertAddEntityNoRole() public {
-    vm.expectRevert(AccessControlErrorsLib.MISSING_ROLE(MANAGE_ENTITY_ROLE, ALICE));
+    vm.expectRevert(AccessControlErrorsLib.MISSING_ROLE(ENTITY_MANAGER_ROLE, ALICE));
     vm.prank(ALICE);
     GHO_RESERVE.addEntity(ALICE);
   }
@@ -231,7 +231,7 @@ contract TestGhoReserve is TestGhoBase {
   }
 
   function testRevertRemoveEntityNoRole() public {
-    vm.expectRevert(AccessControlErrorsLib.MISSING_ROLE(MANAGE_ENTITY_ROLE, ALICE));
+    vm.expectRevert(AccessControlErrorsLib.MISSING_ROLE(ENTITY_MANAGER_ROLE, ALICE));
     vm.prank(ALICE);
     GHO_RESERVE.removeEntity(ALICE);
   }
@@ -261,7 +261,7 @@ contract TestGhoReserve is TestGhoBase {
   }
 
   function testRevertSetLimitNoRole() public {
-    vm.expectRevert(AccessControlErrorsLib.MISSING_ROLE(SET_LIMIT_ROLE, ALICE));
+    vm.expectRevert(AccessControlErrorsLib.MISSING_ROLE(LIMIT_MANAGER_ROLE, ALICE));
     vm.prank(ALICE);
     GHO_RESERVE.setLimit(ALICE, 1_000_000 ether);
   }