From 41cb93593059e4592dbacf70f2a3a7398e08984c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alberto=20Cuesta=20Ca=C3=B1ada?= <mail@albertocuesta.es>
Date: Thu, 23 Jan 2020 20:43:57 +0000
Subject: [PATCH] Revert on useless operations.

---
 contracts/utils/EnumerableSet.sol | 14 ++++++--------
 test/utils/EnumerableSet.test.js  | 17 ++++++++++++++++-
 2 files changed, 22 insertions(+), 9 deletions(-)

diff --git a/contracts/utils/EnumerableSet.sol b/contracts/utils/EnumerableSet.sol
index 2beaa9f9c..b206da5fc 100644
--- a/contracts/utils/EnumerableSet.sol
+++ b/contracts/utils/EnumerableSet.sol
@@ -33,9 +33,8 @@ library EnumerableSet {
     function add(AddressSet storage set, address value)
         internal
     {
-        if (!contains(set, value)){
-            set.index[value] = set.values.push(value);
-        }
+        require(!contains(set, value), "EnumerableSet: value already in set");
+        set.index[value] = set.values.push(value);
     }
 
     /**
@@ -44,11 +43,10 @@ library EnumerableSet {
     function remove(AddressSet storage set, address value)
         internal
     {
-        if (contains(set, value)) {
-            set.values[set.index[value] - 1] = set.values[set.values.length - 1];
-            set.values.pop();
-            delete set.index[value];
-        }
+        require(contains(set, value), "EnumerableSet: value not in set");
+        set.values[set.index[value] - 1] = set.values[set.values.length - 1];
+        set.values.pop();
+        delete set.index[value];
     }
 
     /**
diff --git a/test/utils/EnumerableSet.test.js b/test/utils/EnumerableSet.test.js
index 148212a23..b6d3aed78 100644
--- a/test/utils/EnumerableSet.test.js
+++ b/test/utils/EnumerableSet.test.js
@@ -1,5 +1,5 @@
 const { contract } = require('@openzeppelin/test-environment');
-
+const { expectRevert } = require('@openzeppelin/test-helpers');
 const { expect } = require('chai');
 
 const EnumerableSetMock = contract.fromArtifact('EnumerableSetMock');
@@ -31,12 +31,27 @@ describe('EnumerableSet', function () {
     expect(await this.set.testContains(c)).to.equal(false);
   });
 
+  it('can\'t add same value twice.', async function () {
+    await this.set.testAdd(a);
+    await expectRevert(
+      this.set.testAdd(a),
+      'EnumerableSet: value already in set',
+    );
+  });
+
   it('removes values.', async function () {
     await this.set.testAdd(a);
     await this.set.testRemove(a);
     expect(await this.set.testContains(a)).to.equal(false);
   });
 
+  it('can\'t remove values that are not in the set.', async function () {
+    await expectRevert(
+      this.set.testRemove(a),
+      'EnumerableSet: value not in set',
+    );
+  });
+
   it('enumerates values as an empty array', async function () {
     expect(await this.set.testEnumerate()).to.eql([]);
   });