From b770d89adeb68041ba322b2389563d60aca39b20 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alberto=20Cuesta=20Ca=C3=B1ada?= <mail@albertocuesta.es>
Date: Thu, 23 Jan 2020 21:17:49 +0000
Subject: [PATCH] Transactions now return false on failure.

---
 contracts/mocks/EnumerableSetMock.sol |  9 ++++++---
 contracts/utils/EnumerableSet.sol     |  6 ++++--
 test/utils/EnumerableSet.test.js      | 24 ++++++++++++++++--------
 3 files changed, 26 insertions(+), 13 deletions(-)

diff --git a/contracts/mocks/EnumerableSetMock.sol b/contracts/mocks/EnumerableSetMock.sol
index e4456fba5..0f77fc1e8 100644
--- a/contracts/mocks/EnumerableSetMock.sol
+++ b/contracts/mocks/EnumerableSetMock.sol
@@ -8,9 +8,10 @@ import "../utils/EnumerableSet.sol";
  * @author Alberto Cuesta Cañada
  */
 contract EnumerableSetMock{
-
     using EnumerableSet for EnumerableSet.AddressSet;
 
+    event TransactionResult(bool result);
+
     EnumerableSet.AddressSet private set;
 
     constructor() public {
@@ -34,7 +35,8 @@ contract EnumerableSetMock{
     function testAdd(address value)
         public
     {
-        EnumerableSet.add(set, value);
+        bool result = EnumerableSet.add(set, value);
+        emit TransactionResult(result);
     }
 
     /**
@@ -43,7 +45,8 @@ contract EnumerableSetMock{
     function testRemove(address remove)
         public
     {
-        EnumerableSet.remove(set, remove);
+        bool result = EnumerableSet.remove(set, remove);
+        emit TransactionResult(result);
     }
 
     /**
diff --git a/contracts/utils/EnumerableSet.sol b/contracts/utils/EnumerableSet.sol
index fe6eb2a96..76eb8d55c 100644
--- a/contracts/utils/EnumerableSet.sol
+++ b/contracts/utils/EnumerableSet.sol
@@ -39,7 +39,7 @@ library EnumerableSet {
     }
 
     /**
-     * @dev Add a value. O(1).
+     * @dev Add a value. O(1). Returns false if the value was already in the set.
      */
     function add(AddressSet storage set, address value)
         internal
@@ -47,13 +47,14 @@ library EnumerableSet {
     {
         if (!contains(set, value)){
             set.index[value] = set.values.push(value);
+            return true;
         } else {
             return false;
         }
     }
 
     /**
-     * @dev Remove a value. O(1).
+     * @dev Remove a value. O(1). Returns false if the calue was not present in the set.
      */
     function remove(AddressSet storage set, address value)
         internal
@@ -64,6 +65,7 @@ library EnumerableSet {
             set.values[set.index[value] - 1] = set.values[set.values.length - 1];
             set.values.pop();
             delete set.index[value];
+            return true;
         } else {
             return false;
         }
diff --git a/test/utils/EnumerableSet.test.js b/test/utils/EnumerableSet.test.js
index b0996b8fe..824b8b523 100644
--- a/test/utils/EnumerableSet.test.js
+++ b/test/utils/EnumerableSet.test.js
@@ -18,7 +18,9 @@ describe('EnumerableSet', function () {
   });
 
   it('adds a value.', async function () {
-    await this.set.testAdd(a);
+    const transaction = await this.set.testAdd(a);
+    expect(transaction.logs[0].event).to.equal('TransactionResult');
+    expect(transaction.logs[0].args.result).to.equal(true);
     expect(await this.set.testContains(a)).to.equal(true);
   });
 
@@ -30,20 +32,26 @@ describe('EnumerableSet', function () {
     expect(await this.set.testContains(c)).to.equal(false);
   });
 
-  /* it('adding same value twice returns false.', async function () {
+  it('adding same value twice returns false.', async function () {
     await this.set.testAdd(a);
-    expect(await this.set.testAdd(a)).to.equal(false);
-  }); */
+    const transaction = (await this.set.testAdd(a));
+    expect(transaction.logs[0].event).to.equal('TransactionResult');
+    expect(transaction.logs[0].args.result).to.equal(false);
+  });
 
   it('removes values.', async function () {
     await this.set.testAdd(a);
-    await this.set.testRemove(a);
+    const transaction = await this.set.testRemove(a);
+    expect(transaction.logs[0].event).to.equal('TransactionResult');
+    expect(transaction.logs[0].args.result).to.equal(true);
     expect(await this.set.testContains(a)).to.equal(false);
   });
 
-  /* it('removing values that are not in the set returns false.', async function () {
-    expect(await this.set.testRemove(a)).to.equal(false);
-  }); */
+  it('removing values that are not in the set returns false.', async function () {
+    const transaction = await this.set.testRemove(a);
+    expect(transaction.logs[0].event).to.equal('TransactionResult');
+    expect(transaction.logs[0].args.result).to.equal(false);
+  });
 
   it('enumerates values as an empty array', async function () {
     expect(await this.set.testEnumerate()).to.eql([]);