feat: add error messages and tests to reserve

Author: efecarranza

src/contracts/facilitators/gsm/GhoReserve.sol

@@ -23,7 +23,7 @@ contract GhoReserve is Ownable, VersionedInitializable, IGhoReserve {
   mapping(address => GhoUsage) private _ghoUsage;
 
   /// Set of entities with a GHO limit available
-  EnumerableSet.AddressSet private entities;
+  EnumerableSet.AddressSet private _entities;
 
   /**
    * @dev Constructor
@@ -68,30 +68,29 @@ contract GhoReserve is Ownable, VersionedInitializable, IGhoReserve {
 
   /// @inheritdoc IGhoReserve
   function addEntity(address entity) external onlyOwner {
-    entities.add(entity);
+    require(_entities.add(entity), 'ENTITY_ALREADY_EXISTS');
     emit EntityAdded(entity);
   }
 
   /// @inheritdoc IGhoReserve
   function removeEntity(address entity) external onlyOwner {
-    GhoUsage memory usage = _ghoUsage[entity];
-    require(usage.used == 0, 'CANNOT_REMOVE_ENTITY_WITH_BALANCE');
-    entities.remove(entity);
+    require(_ghoUsage[entity].used == 0, 'ENTITY_GHO_USED_NOT_ZERO');
+    require(_entities.remove(entity), 'ENTITY_NOT_REMOVED');
 
     emit EntityRemoved(entity);
   }
 
   /// @inheritdoc IGhoReserve
   function setLimit(address entity, uint256 limit) external onlyOwner {
-    require(entities.contains(entity), 'ENTITY_NOT_ALLOWED');
+    require(_entities.contains(entity), 'ENTITY_DOES_NOT_EXIST');
     _ghoUsage[entity].limit = uint128(limit);
 
     emit GhoLimitUpdated(entity, limit);
   }
 
   /// @inheritdoc IGhoReserve
   function getEntities() external view returns (address[] memory) {
-    return entities.values();
+    return _entities.values();
   }
 
   /// @inheritdoc IGhoReserve
@@ -112,12 +111,12 @@ contract GhoReserve is Ownable, VersionedInitializable, IGhoReserve {
 
   /// @inheritdoc IGhoReserve
   function isEntity(address entity) external view returns (bool) {
-    return entities.contains(entity);
+    return _entities.contains(entity);
   }
 
   /// @inheritdoc IGhoReserve
   function totalEntities() external view returns (uint256) {
-    return entities.length();
+    return _entities.length();
   }
 
   /// @inheritdoc IGhoReserve

src/test/TestGhoReserve.t.sol

@@ -104,17 +104,19 @@ contract TestGhoReserve is TestGhoBase {
   }
 
   function testAddEntityAlreadyInSet() public {
+    uint256 entitiesCount = GHO_RESERVE.totalEntities();
     address alice = makeAddr('alice');
     vm.expectEmit(true, true, true, true, address(GHO_RESERVE));
     emit EntityAdded(alice);
     GHO_RESERVE.addEntity(address(alice));
 
     // Set already contains two entities from constructor
-    assertEq(GHO_RESERVE.totalEntities(), 3);
+    assertEq(GHO_RESERVE.totalEntities(), entitiesCount + 1);
 
+    vm.expectRevert('ENTITY_ALREADY_EXISTS');
     GHO_RESERVE.addEntity(address(alice));
 
-    assertEq(GHO_RESERVE.totalEntities(), 3);
+    assertEq(GHO_RESERVE.totalEntities(), entitiesCount + 1);
   }
 
   function testRemoveEntity() public {
@@ -133,14 +135,16 @@ contract TestGhoReserve is TestGhoBase {
   }
 
   function testRemoveEntityNotInSet() public {
+    uint256 entitiesCount = GHO_RESERVE.totalEntities();
     address alice = makeAddr('alice');
     assertFalse(GHO_RESERVE.isEntity(alice));
-    assertEq(GHO_RESERVE.totalEntities(), 2);
+    assertEq(GHO_RESERVE.totalEntities(), entitiesCount);
 
+    vm.expectRevert('ENTITY_NOT_REMOVED');
     GHO_RESERVE.removeEntity(address(alice));
 
     assertFalse(GHO_RESERVE.isEntity(alice));
-    assertEq(GHO_RESERVE.totalEntities(), 2);
+    assertEq(GHO_RESERVE.totalEntities(), entitiesCount);
   }
 
   function testRevertRemoveEntityBalanceOutstanding() public {
@@ -152,7 +156,7 @@ contract TestGhoReserve is TestGhoBase {
     vm.prank(alice);
     GHO_RESERVE.use(5_000 ether);
 
-    vm.expectRevert('CANNOT_REMOVE_ENTITY_WITH_BALANCE');
+    vm.expectRevert('ENTITY_GHO_USED_NOT_ZERO');
     GHO_RESERVE.removeEntity(alice);
   }
 
@@ -166,6 +170,11 @@ contract TestGhoReserve is TestGhoBase {
     GHO_RESERVE.setLimit(alice, capacity);
   }
 
+  function testSetLimitEntityDoesNotExist() public {
+    vm.expectRevert('ENTITY_DOES_NOT_EXIST');
+    GHO_RESERVE.setLimit(makeAddr('no-entity'), 100_000 ether);
+  }
+
   function testTransfer() public {
     GhoReserve reserve = _deployReserve();
     address facilitator = makeAddr('facilitator');