From 5ec2e2d89824a63598da9889e7b46972d68dfb07 Mon Sep 17 00:00:00 2001
From: Hadrien Croubois <hadrien.croubois@gmail.com>
Date: Fri, 8 Jan 2021 10:41:29 +0100
Subject: [PATCH 01/16] Refactor SafeMath to avoid memory leaks

FIxes #2413, remove the string allocation in SafeMath  default functions: caused unrecovered memory allocation even for successfull calls
---
 contracts/math/SafeMath.sol      | 63 ++++++++++++++++++++++++++++----
 contracts/mocks/SafeMathMock.sol | 36 ++++++++++++++++++
 test/math/SafeMath.test.js       | 22 +++++++++++
 3 files changed, 114 insertions(+), 7 deletions(-)

diff --git a/contracts/math/SafeMath.sol b/contracts/math/SafeMath.sol
index 7975c634cca..68ef98b2615 100644
--- a/contracts/math/SafeMath.sol
+++ b/contracts/math/SafeMath.sol
@@ -33,6 +33,23 @@ library SafeMath {
         return c;
     }
 
+    /**
+     * @dev Returns the addition of two unsigned integers, reverting with custom message on
+     * overflow.
+     *
+     * Counterpart to Solidity's `+` operator.
+     *
+     * Requirements:
+     *
+     * - Addition cannot overflow.
+     */
+    function add(uint256 a, uint256 b, string memory errorMessage) internal pure returns (uint256) {
+        uint256 c = a + b;
+        require(c >= a, errorMessage);
+
+        return c;
+    }
+
     /**
      * @dev Returns the subtraction of two unsigned integers, reverting on
      * overflow (when the result is negative).
@@ -44,7 +61,10 @@ library SafeMath {
      * - Subtraction cannot overflow.
      */
     function sub(uint256 a, uint256 b) internal pure returns (uint256) {
-        return sub(a, b, "SafeMath: subtraction overflow");
+        require(b <= a, "SafeMath: subtraction overflow");
+        uint256 c = a - b;
+
+        return c;
     }
 
     /**
@@ -89,7 +109,31 @@ library SafeMath {
     }
 
     /**
-     * @dev Returns the integer division of two unsigned integers. Reverts on
+     * @dev Returns the multiplication of two unsigned integers, reverting with custom message on
+     * overflow.
+     *
+     * Counterpart to Solidity's `*` operator.
+     *
+     * Requirements:
+     *
+     * - Multiplication cannot overflow.
+     */
+    function mul(uint256 a, uint256 b, string memory errorMessage) internal pure returns (uint256) {
+        // Gas optimization: this is cheaper than requiring 'a' not being zero, but the
+        // benefit is lost if 'b' is also tested.
+        // See: https://github.com/OpenZeppelin/openzeppelin-contracts/pull/522
+        if (a == 0) {
+            return 0;
+        }
+
+        uint256 c = a * b;
+        require(c / a == b, errorMessage);
+
+        return c;
+    }
+
+    /**
+     * @dev Returns the integer division of two unsigned integers, reverting on
      * division by zero. The result is rounded towards zero.
      *
      * Counterpart to Solidity's `/` operator. Note: this function uses a
@@ -101,11 +145,15 @@ library SafeMath {
      * - The divisor cannot be zero.
      */
     function div(uint256 a, uint256 b) internal pure returns (uint256) {
-        return div(a, b, "SafeMath: division by zero");
+        require(b > 0, "SafeMath: division by zero");
+        uint256 c = a / b;
+        // assert(a == b * c + a % b); // There is no case in which this doesn't hold
+
+        return c;
     }
 
     /**
-     * @dev Returns the integer division of two unsigned integers. Reverts with custom message on
+     * @dev Returns the integer division of two unsigned integers, reverting with custom message on
      * division by zero. The result is rounded towards zero.
      *
      * Counterpart to Solidity's `/` operator. Note: this function uses a
@@ -126,7 +174,7 @@ library SafeMath {
 
     /**
      * @dev Returns the remainder of dividing two unsigned integers. (unsigned integer modulo),
-     * Reverts when dividing by zero.
+     * reverting when dividing by zero.
      *
      * Counterpart to Solidity's `%` operator. This function uses a `revert`
      * opcode (which leaves remaining gas untouched) while Solidity uses an
@@ -137,12 +185,13 @@ library SafeMath {
      * - The divisor cannot be zero.
      */
     function mod(uint256 a, uint256 b) internal pure returns (uint256) {
-        return mod(a, b, "SafeMath: modulo by zero");
+        require(b != 0, "SafeMath: modulo by zero");
+        return a % b;
     }
 
     /**
      * @dev Returns the remainder of dividing two unsigned integers. (unsigned integer modulo),
-     * Reverts with custom message when dividing by zero.
+     * reverting with custom message when dividing by zero.
      *
      * Counterpart to Solidity's `%` operator. This function uses a `revert`
      * opcode (which leaves remaining gas untouched) while Solidity uses an
diff --git a/contracts/mocks/SafeMathMock.sol b/contracts/mocks/SafeMathMock.sol
index 5d2b8d8b2da..627c68cfcf5 100644
--- a/contracts/mocks/SafeMathMock.sol
+++ b/contracts/mocks/SafeMathMock.sol
@@ -24,4 +24,40 @@ contract SafeMathMock {
     function mod(uint256 a, uint256 b) public pure returns (uint256) {
         return SafeMath.mod(a, b);
     }
+
+    function mulMemoryCheck() public pure returns (uint256 mem) {
+        uint256 length = 32;
+        assembly { mem := mload(0x40) }
+        for (uint256 i = 0; i < length; ++i) { SafeMath.mul(1, 1); }
+        assembly { mem := sub(mload(0x40), mem) }
+    }
+
+    function divMemoryCheck() public pure returns (uint256 mem) {
+        uint256 length = 32;
+        assembly { mem := mload(0x40) }
+        for (uint256 i = 0; i < length; ++i) { SafeMath.div(1, 1); }
+        assembly { mem := sub(mload(0x40), mem) }
+    }
+
+    function subMemoryCheck() public pure returns (uint256 mem) {
+        uint256 length = 32;
+        assembly { mem := mload(0x40) }
+        for (uint256 i = 0; i < length; ++i) { SafeMath.sub(1, 1); }
+        assembly { mem := sub(mload(0x40), mem) }
+    }
+
+    function addMemoryCheck() public pure returns (uint256 mem) {
+        uint256 length = 32;
+        assembly { mem := mload(0x40) }
+        for (uint256 i = 0; i < length; ++i) { SafeMath.add(1, 1); }
+        assembly { mem := sub(mload(0x40), mem) }
+    }
+
+    function modMemoryCheck() public pure returns (uint256 mem) {
+        uint256 length = 32;
+        assembly { mem := mload(0x40) }
+        for (uint256 i = 0; i < length; ++i) { SafeMath.mod(1, 1); }
+        assembly { mem := sub(mload(0x40), mem) }
+    }
+
 }
diff --git a/test/math/SafeMath.test.js b/test/math/SafeMath.test.js
index 44cc1ba8e7c..cda5c9ff2c0 100644
--- a/test/math/SafeMath.test.js
+++ b/test/math/SafeMath.test.js
@@ -143,4 +143,26 @@ contract('SafeMath', function (accounts) {
       await expectRevert(this.safeMath.mod(a, b), 'SafeMath: modulo by zero');
     });
   });
+
+  describe('memory leakage', function () {
+    it('add does not leak', async function () {
+      expect(await this.safeMath.addMemoryCheck()).to.be.bignumber.equal('0');
+    });
+
+    it('sub does not leak', async function () {
+      expect(await this.safeMath.subMemoryCheck()).to.be.bignumber.equal('0');
+    });
+
+    it('mul does not leak', async function () {
+      expect(await this.safeMath.mulMemoryCheck()).to.be.bignumber.equal('0');
+    });
+
+    it('div does not leak', async function () {
+      expect(await this.safeMath.divMemoryCheck()).to.be.bignumber.equal('0');
+    });
+
+    it('mod does not leak', async function () {
+      expect(await this.safeMath.modMemoryCheck()).to.be.bignumber.equal('0');
+    });
+  });
 });

From 9c789b0516599551d4b887c1588d1c0e95466b7d Mon Sep 17 00:00:00 2001
From: Hadrien Croubois <hadrien.croubois@gmail.com>
Date: Fri, 8 Jan 2021 10:45:23 +0100
Subject: [PATCH 02/16] silence inline assembly warning in mock

---
 contracts/mocks/SafeMathMock.sol | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/contracts/mocks/SafeMathMock.sol b/contracts/mocks/SafeMathMock.sol
index 627c68cfcf5..e2262e5877d 100644
--- a/contracts/mocks/SafeMathMock.sol
+++ b/contracts/mocks/SafeMathMock.sol
@@ -27,36 +27,46 @@ contract SafeMathMock {
 
     function mulMemoryCheck() public pure returns (uint256 mem) {
         uint256 length = 32;
+        // solhint-disable-next-line no-inline-assembly
         assembly { mem := mload(0x40) }
         for (uint256 i = 0; i < length; ++i) { SafeMath.mul(1, 1); }
+        // solhint-disable-next-line no-inline-assembly
         assembly { mem := sub(mload(0x40), mem) }
     }
 
     function divMemoryCheck() public pure returns (uint256 mem) {
         uint256 length = 32;
+        // solhint-disable-next-line no-inline-assembly
         assembly { mem := mload(0x40) }
         for (uint256 i = 0; i < length; ++i) { SafeMath.div(1, 1); }
+        // solhint-disable-next-line no-inline-assembly
         assembly { mem := sub(mload(0x40), mem) }
     }
 
     function subMemoryCheck() public pure returns (uint256 mem) {
         uint256 length = 32;
+        // solhint-disable-next-line no-inline-assembly
         assembly { mem := mload(0x40) }
         for (uint256 i = 0; i < length; ++i) { SafeMath.sub(1, 1); }
+        // solhint-disable-next-line no-inline-assembly
         assembly { mem := sub(mload(0x40), mem) }
     }
 
     function addMemoryCheck() public pure returns (uint256 mem) {
         uint256 length = 32;
+        // solhint-disable-next-line no-inline-assembly
         assembly { mem := mload(0x40) }
         for (uint256 i = 0; i < length; ++i) { SafeMath.add(1, 1); }
+        // solhint-disable-next-line no-inline-assembly
         assembly { mem := sub(mload(0x40), mem) }
     }
 
     function modMemoryCheck() public pure returns (uint256 mem) {
         uint256 length = 32;
+        // solhint-disable-next-line no-inline-assembly
         assembly { mem := mload(0x40) }
         for (uint256 i = 0; i < length; ++i) { SafeMath.mod(1, 1); }
+        // solhint-disable-next-line no-inline-assembly
         assembly { mem := sub(mload(0x40), mem) }
     }
 

From 42e6cfac51b0e1cb2e0c902b93c13522c0a0189f Mon Sep 17 00:00:00 2001
From: Hadrien Croubois <hadrien.croubois@gmail.com>
Date: Fri, 8 Jan 2021 19:05:42 +0100
Subject: [PATCH 03/16] improved coverage

---
 contracts/mocks/SafeMathMock.sol |  20 +++
 test/math/SafeMath.test.js       | 294 ++++++++++++++++++++++---------
 2 files changed, 231 insertions(+), 83 deletions(-)

diff --git a/contracts/mocks/SafeMathMock.sol b/contracts/mocks/SafeMathMock.sol
index e2262e5877d..e958eff674e 100644
--- a/contracts/mocks/SafeMathMock.sol
+++ b/contracts/mocks/SafeMathMock.sol
@@ -25,6 +25,26 @@ contract SafeMathMock {
         return SafeMath.mod(a, b);
     }
 
+    function mulWithMessage(uint256 a, uint256 b, string memory errorMessage) public pure returns (uint256) {
+        return SafeMath.mul(a, b, errorMessage);
+    }
+
+    function divWithMessage(uint256 a, uint256 b, string memory errorMessage) public pure returns (uint256) {
+        return SafeMath.div(a, b, errorMessage);
+    }
+
+    function subWithMessage(uint256 a, uint256 b, string memory errorMessage) public pure returns (uint256) {
+        return SafeMath.sub(a, b, errorMessage);
+    }
+
+    function addWithMessage(uint256 a, uint256 b, string memory errorMessage) public pure returns (uint256) {
+        return SafeMath.add(a, b, errorMessage);
+    }
+
+    function modWithMessage(uint256 a, uint256 b, string memory errorMessage) public pure returns (uint256) {
+        return SafeMath.mod(a, b, errorMessage);
+    }
+
     function mulMemoryCheck() public pure returns (uint256 mem) {
         uint256 length = 32;
         // solhint-disable-next-line no-inline-assembly
diff --git a/test/math/SafeMath.test.js b/test/math/SafeMath.test.js
index cda5c9ff2c0..4b3b59a9cb2 100644
--- a/test/math/SafeMath.test.js
+++ b/test/math/SafeMath.test.js
@@ -10,158 +10,286 @@ contract('SafeMath', function (accounts) {
     this.safeMath = await SafeMathMock.new();
   });
 
-  async function testCommutative (fn, lhs, rhs, expected) {
-    expect(await fn(lhs, rhs)).to.be.bignumber.equal(expected);
-    expect(await fn(rhs, lhs)).to.be.bignumber.equal(expected);
+  async function testCommutative (fn, lhs, rhs, expected, ...extra) {
+    expect(await fn(lhs, rhs, ...extra)).to.be.bignumber.equal(expected);
+    expect(await fn(rhs, lhs, ...extra)).to.be.bignumber.equal(expected);
   }
 
-  async function testFailsCommutative (fn, lhs, rhs, reason) {
-    await expectRevert(fn(lhs, rhs), reason);
-    await expectRevert(fn(rhs, lhs), reason);
+  async function testFailsCommutative (fn, lhs, rhs, reason, ...extra) {
+    await expectRevert(fn(lhs, rhs, ...extra), reason);
+    await expectRevert(fn(rhs, lhs, ...extra), reason);
   }
 
-  describe('add', function () {
-    it('adds correctly', async function () {
-      const a = new BN('5678');
-      const b = new BN('1234');
+  describe('with default revert message', function () {
+    describe('add', function () {
+      it('adds correctly', async function () {
+        const a = new BN('5678');
+        const b = new BN('1234');
 
-      await testCommutative(this.safeMath.add, a, b, a.add(b));
-    });
+        await testCommutative(this.safeMath.add, a, b, a.add(b));
+      });
 
-    it('reverts on addition overflow', async function () {
-      const a = MAX_UINT256;
-      const b = new BN('1');
+      it('reverts on addition overflow', async function () {
+        const a = MAX_UINT256;
+        const b = new BN('1');
 
-      await testFailsCommutative(this.safeMath.add, a, b, 'SafeMath: addition overflow');
+        await testFailsCommutative(this.safeMath.add, a, b, 'SafeMath: addition overflow');
+      });
     });
-  });
 
-  describe('sub', function () {
-    it('subtracts correctly', async function () {
-      const a = new BN('5678');
-      const b = new BN('1234');
+    describe('sub', function () {
+      it('subtracts correctly', async function () {
+        const a = new BN('5678');
+        const b = new BN('1234');
 
-      expect(await this.safeMath.sub(a, b)).to.be.bignumber.equal(a.sub(b));
-    });
+        expect(await this.safeMath.sub(a, b)).to.be.bignumber.equal(a.sub(b));
+      });
 
-    it('reverts if subtraction result would be negative', async function () {
-      const a = new BN('1234');
-      const b = new BN('5678');
+      it('reverts if subtraction result would be negative', async function () {
+        const a = new BN('1234');
+        const b = new BN('5678');
 
-      await expectRevert(this.safeMath.sub(a, b), 'SafeMath: subtraction overflow');
+        await expectRevert(this.safeMath.sub(a, b), 'SafeMath: subtraction overflow');
+      });
     });
-  });
 
-  describe('mul', function () {
-    it('multiplies correctly', async function () {
-      const a = new BN('1234');
-      const b = new BN('5678');
+    describe('mul', function () {
+      it('multiplies correctly', async function () {
+        const a = new BN('1234');
+        const b = new BN('5678');
+
+        await testCommutative(this.safeMath.mul, a, b, a.mul(b));
+      });
+
+      it('multiplies by zero correctly', async function () {
+        const a = new BN('0');
+        const b = new BN('5678');
 
-      await testCommutative(this.safeMath.mul, a, b, a.mul(b));
+        await testCommutative(this.safeMath.mul, a, b, '0');
+      });
+
+      it('reverts on multiplication overflow', async function () {
+        const a = MAX_UINT256;
+        const b = new BN('2');
+
+        await testFailsCommutative(this.safeMath.mul, a, b, 'SafeMath: multiplication overflow');
+      });
     });
 
-    it('multiplies by zero correctly', async function () {
-      const a = new BN('0');
-      const b = new BN('5678');
+    describe('div', function () {
+      it('divides correctly', async function () {
+        const a = new BN('5678');
+        const b = new BN('5678');
+
+        expect(await this.safeMath.div(a, b)).to.be.bignumber.equal(a.div(b));
+      });
+
+      it('divides zero correctly', async function () {
+        const a = new BN('0');
+        const b = new BN('5678');
+
+        expect(await this.safeMath.div(a, b)).to.be.bignumber.equal('0');
+      });
+
+      it('returns complete number result on non-even division', async function () {
+        const a = new BN('7000');
+        const b = new BN('5678');
 
-      await testCommutative(this.safeMath.mul, a, b, '0');
+        expect(await this.safeMath.div(a, b)).to.be.bignumber.equal('1');
+      });
+
+      it('reverts on division by zero', async function () {
+        const a = new BN('5678');
+        const b = new BN('0');
+
+        await expectRevert(this.safeMath.div(a, b), 'SafeMath: division by zero');
+      });
     });
 
-    it('reverts on multiplication overflow', async function () {
-      const a = MAX_UINT256;
-      const b = new BN('2');
+    describe('mod', function () {
+      describe('modulos correctly', async function () {
+        it('when the dividend is smaller than the divisor', async function () {
+          const a = new BN('284');
+          const b = new BN('5678');
+
+          expect(await this.safeMath.mod(a, b)).to.be.bignumber.equal(a.mod(b));
+        });
+
+        it('when the dividend is equal to the divisor', async function () {
+          const a = new BN('5678');
+          const b = new BN('5678');
+
+          expect(await this.safeMath.mod(a, b)).to.be.bignumber.equal(a.mod(b));
+        });
+
+        it('when the dividend is larger than the divisor', async function () {
+          const a = new BN('7000');
+          const b = new BN('5678');
+
+          expect(await this.safeMath.mod(a, b)).to.be.bignumber.equal(a.mod(b));
+        });
 
-      await testFailsCommutative(this.safeMath.mul, a, b, 'SafeMath: multiplication overflow');
+        it('when the dividend is a multiple of the divisor', async function () {
+          const a = new BN('17034'); // 17034 == 5678 * 3
+          const b = new BN('5678');
+
+          expect(await this.safeMath.mod(a, b)).to.be.bignumber.equal(a.mod(b));
+        });
+      });
+
+      it('reverts with a 0 divisor', async function () {
+        const a = new BN('5678');
+        const b = new BN('0');
+
+        await expectRevert(this.safeMath.mod(a, b), 'SafeMath: modulo by zero');
+      });
     });
   });
 
-  describe('div', function () {
-    it('divides correctly', async function () {
-      const a = new BN('5678');
-      const b = new BN('5678');
+  describe('with custom revert message', function () {
+    describe('add', function () {
+      it('adds correctly', async function () {
+        const a = new BN('5678');
+        const b = new BN('1234');
 
-      expect(await this.safeMath.div(a, b)).to.be.bignumber.equal(a.div(b));
-    });
+        await testCommutative(this.safeMath.addWithMessage, a, b, a.add(b), 'MyErrorMessage');
+      });
 
-    it('divides zero correctly', async function () {
-      const a = new BN('0');
-      const b = new BN('5678');
+      it('reverts on addition overflow', async function () {
+        const a = MAX_UINT256;
+        const b = new BN('1');
 
-      expect(await this.safeMath.div(a, b)).to.be.bignumber.equal('0');
+        await testFailsCommutative(this.safeMath.addWithMessage, a, b, 'MyErrorMessage', 'MyErrorMessage');
+      });
     });
 
-    it('returns complete number result on non-even division', async function () {
-      const a = new BN('7000');
-      const b = new BN('5678');
+    describe('sub', function () {
+      it('subtracts correctly', async function () {
+        const a = new BN('5678');
+        const b = new BN('1234');
 
-      expect(await this.safeMath.div(a, b)).to.be.bignumber.equal('1');
-    });
+        expect(await this.safeMath.subWithMessage(a, b, 'MyErrorMessage')).to.be.bignumber.equal(a.sub(b));
+      });
 
-    it('reverts on division by zero', async function () {
-      const a = new BN('5678');
-      const b = new BN('0');
+      it('reverts if subtraction result would be negative', async function () {
+        const a = new BN('1234');
+        const b = new BN('5678');
 
-      await expectRevert(this.safeMath.div(a, b), 'SafeMath: division by zero');
+        await expectRevert(this.safeMath.subWithMessage(a, b, 'MyErrorMessage'), 'MyErrorMessage');
+      });
     });
-  });
 
-  describe('mod', function () {
-    describe('modulos correctly', async function () {
-      it('when the dividend is smaller than the divisor', async function () {
-        const a = new BN('284');
+    describe('mul', function () {
+      it('multiplies correctly', async function () {
+        const a = new BN('1234');
         const b = new BN('5678');
 
-        expect(await this.safeMath.mod(a, b)).to.be.bignumber.equal(a.mod(b));
+        await testCommutative(this.safeMath.mulWithMessage, a, b, a.mul(b), 'MyErrorMessage');
       });
 
-      it('when the dividend is equal to the divisor', async function () {
+      it('multiplies by zero correctly', async function () {
+        const a = new BN('0');
+        const b = new BN('5678');
+
+        await testCommutative(this.safeMath.mulWithMessage, a, b, '0', 'MyErrorMessage');
+      });
+
+      it('reverts on multiplication overflow', async function () {
+        const a = MAX_UINT256;
+        const b = new BN('2');
+
+        await testFailsCommutative(this.safeMath.mulWithMessage, a, b, 'MyErrorMessage', 'MyErrorMessage');
+      });
+    });
+
+    describe('div', function () {
+      it('divides correctly', async function () {
         const a = new BN('5678');
         const b = new BN('5678');
 
-        expect(await this.safeMath.mod(a, b)).to.be.bignumber.equal(a.mod(b));
+        expect(await this.safeMath.divWithMessage(a, b, 'MyErrorMessage')).to.be.bignumber.equal(a.div(b));
       });
 
-      it('when the dividend is larger than the divisor', async function () {
-        const a = new BN('7000');
+      it('divides zero correctly', async function () {
+        const a = new BN('0');
         const b = new BN('5678');
 
-        expect(await this.safeMath.mod(a, b)).to.be.bignumber.equal(a.mod(b));
+        expect(await this.safeMath.divWithMessage(a, b, 'MyErrorMessage')).to.be.bignumber.equal('0');
       });
 
-      it('when the dividend is a multiple of the divisor', async function () {
-        const a = new BN('17034'); // 17034 == 5678 * 3
+      it('returns complete number result on non-even division', async function () {
+        const a = new BN('7000');
         const b = new BN('5678');
 
-        expect(await this.safeMath.mod(a, b)).to.be.bignumber.equal(a.mod(b));
+        expect(await this.safeMath.divWithMessage(a, b, 'MyErrorMessage')).to.be.bignumber.equal('1');
+      });
+
+      it('reverts on division by zero', async function () {
+        const a = new BN('5678');
+        const b = new BN('0');
+
+        await expectRevert(this.safeMath.divWithMessage(a, b, 'MyErrorMessage'), 'MyErrorMessage');
       });
     });
 
-    it('reverts with a 0 divisor', async function () {
-      const a = new BN('5678');
-      const b = new BN('0');
+    describe('mod', function () {
+      describe('modulos correctly', async function () {
+        it('when the dividend is smaller than the divisor', async function () {
+          const a = new BN('284');
+          const b = new BN('5678');
+
+          expect(await this.safeMath.modWithMessage(a, b, 'MyErrorMessage')).to.be.bignumber.equal(a.mod(b));
+        });
 
-      await expectRevert(this.safeMath.mod(a, b), 'SafeMath: modulo by zero');
+        it('when the dividend is equal to the divisor', async function () {
+          const a = new BN('5678');
+          const b = new BN('5678');
+
+          expect(await this.safeMath.modWithMessage(a, b, 'MyErrorMessage')).to.be.bignumber.equal(a.mod(b));
+        });
+
+        it('when the dividend is larger than the divisor', async function () {
+          const a = new BN('7000');
+          const b = new BN('5678');
+
+          expect(await this.safeMath.modWithMessage(a, b, 'MyErrorMessage')).to.be.bignumber.equal(a.mod(b));
+        });
+
+        it('when the dividend is a multiple of the divisor', async function () {
+          const a = new BN('17034'); // 17034 == 5678 * 3
+          const b = new BN('5678');
+
+          expect(await this.safeMath.modWithMessage(a, b, 'MyErrorMessage')).to.be.bignumber.equal(a.mod(b));
+        });
+      });
+
+      it('reverts with a 0 divisor', async function () {
+        const a = new BN('5678');
+        const b = new BN('0');
+
+        await expectRevert(this.safeMath.modWithMessage(a, b, 'MyErrorMessage'), 'MyErrorMessage');
+      });
     });
   });
 
   describe('memory leakage', function () {
-    it('add does not leak', async function () {
+    it('add', async function () {
       expect(await this.safeMath.addMemoryCheck()).to.be.bignumber.equal('0');
     });
 
-    it('sub does not leak', async function () {
+    it('sub', async function () {
       expect(await this.safeMath.subMemoryCheck()).to.be.bignumber.equal('0');
     });
 
-    it('mul does not leak', async function () {
+    it('mul', async function () {
       expect(await this.safeMath.mulMemoryCheck()).to.be.bignumber.equal('0');
     });
 
-    it('div does not leak', async function () {
+    it('div', async function () {
       expect(await this.safeMath.divMemoryCheck()).to.be.bignumber.equal('0');
     });
 
-    it('mod does not leak', async function () {
+    it('mod', async function () {
       expect(await this.safeMath.modMemoryCheck()).to.be.bignumber.equal('0');
     });
   });

From 28ae57fa3a2c55245e020473ecbf2c4b986cbbcc Mon Sep 17 00:00:00 2001
From: Hadrien Croubois <hadrien.croubois@gmail.com>
Date: Mon, 11 Jan 2021 19:47:41 +0100
Subject: [PATCH 04/16] using flags to handle safemath

---
 contracts/math/SafeMath.sol      | 193 +++++++++++++++----------------
 contracts/mocks/SafeMathMock.sol |  46 +++-----
 test/math/SafeMath.test.js       |  39 -------
 3 files changed, 114 insertions(+), 164 deletions(-)

diff --git a/contracts/math/SafeMath.sol b/contracts/math/SafeMath.sol
index 68ef98b2615..075d3efa71d 100644
--- a/contracts/math/SafeMath.sol
+++ b/contracts/math/SafeMath.sol
@@ -17,58 +17,71 @@ pragma solidity >=0.6.0 <0.8.0;
  */
 library SafeMath {
     /**
-     * @dev Returns the addition of two unsigned integers, reverting on
-     * overflow.
-     *
-     * Counterpart to Solidity's `+` operator.
-     *
-     * Requirements:
-     *
-     * - Addition cannot overflow.
+     * @dev Returns the addition of two unsigned integers, with an overflow flag.
      */
-    function add(uint256 a, uint256 b) internal pure returns (uint256) {
+    function addX(uint256 a, uint256 b) internal pure returns (bool, uint256) {
         uint256 c = a + b;
-        require(c >= a, "SafeMath: addition overflow");
+        if (c < a) return (false, 0);
+        return (true, c);
+    }
 
-        return c;
+    /**
+     * @dev Returns the substraction of two unsigned integers, with an overflow flag.
+     */
+    function subX(uint256 a, uint256 b) internal pure returns (bool, uint256) {
+        if (b > a) return (false, 0);
+        return (true, a - b);
     }
 
     /**
-     * @dev Returns the addition of two unsigned integers, reverting with custom message on
-     * overflow.
-     *
-     * Counterpart to Solidity's `+` operator.
-     *
-     * Requirements:
-     *
-     * - Addition cannot overflow.
+     * @dev Returns the multiplication of two unsigned integers, with an overflow flag.
      */
-    function add(uint256 a, uint256 b, string memory errorMessage) internal pure returns (uint256) {
-        uint256 c = a + b;
-        require(c >= a, errorMessage);
+    function mulX(uint256 a, uint256 b) internal pure returns (bool, uint256) {
+        // Gas optimization: this is cheaper than requiring 'a' not being zero, but the
+        // benefit is lost if 'b' is also tested.
+        // See: https://github.com/OpenZeppelin/openzeppelin-contracts/pull/522
+        if (a == 0) {
+            return (true, 0);
+        }
+        uint256 c = a * b;
+        if (c / a != b) return (false, 0);
+        return (true, c);
+    }
 
-        return c;
+    /**
+     * @dev Returns the division of two unsigned integers, with a division by zero flag.
+     */
+    function divX(uint256 a, uint256 b) internal pure returns (bool, uint256) {
+        if (b == 0) return (false, 0);
+        return (true, a / b);
     }
 
     /**
-     * @dev Returns the subtraction of two unsigned integers, reverting on
-     * overflow (when the result is negative).
+     * @dev Returns the remainder of dividing two unsigned integers, with a division by zero flag.
+     */
+    function modX(uint256 a, uint256 b) internal pure returns (bool, uint256) {
+        if (b == 0) return (false, 0);
+        return (true, a % b);
+    }
+
+    /**
+     * @dev Returns the addition of two unsigned integers, reverting on
+     * overflow.
      *
-     * Counterpart to Solidity's `-` operator.
+     * Counterpart to Solidity's `+` operator.
      *
      * Requirements:
      *
-     * - Subtraction cannot overflow.
+     * - Addition cannot overflow.
      */
-    function sub(uint256 a, uint256 b) internal pure returns (uint256) {
-        require(b <= a, "SafeMath: subtraction overflow");
-        uint256 c = a - b;
-
-        return c;
+    function add(uint256 a, uint256 b) internal pure returns (uint256) {
+        (bool success, uint256 result) = addX(a, b);
+        require(success, "SafeMath: addition overflow");
+        return result;
     }
 
     /**
-     * @dev Returns the subtraction of two unsigned integers, reverting with custom message on
+     * @dev Returns the subtraction of two unsigned integers, reverting on
      * overflow (when the result is negative).
      *
      * Counterpart to Solidity's `-` operator.
@@ -77,11 +90,10 @@ library SafeMath {
      *
      * - Subtraction cannot overflow.
      */
-    function sub(uint256 a, uint256 b, string memory errorMessage) internal pure returns (uint256) {
-        require(b <= a, errorMessage);
-        uint256 c = a - b;
-
-        return c;
+    function sub(uint256 a, uint256 b) internal pure returns (uint256) {
+        (bool success, uint256 result) = subX(a, b);
+        require(success, "SafeMath: subtraction overflow");
+        return result;
     }
 
     /**
@@ -95,41 +107,9 @@ library SafeMath {
      * - Multiplication cannot overflow.
      */
     function mul(uint256 a, uint256 b) internal pure returns (uint256) {
-        // Gas optimization: this is cheaper than requiring 'a' not being zero, but the
-        // benefit is lost if 'b' is also tested.
-        // See: https://github.com/OpenZeppelin/openzeppelin-contracts/pull/522
-        if (a == 0) {
-            return 0;
-        }
-
-        uint256 c = a * b;
-        require(c / a == b, "SafeMath: multiplication overflow");
-
-        return c;
-    }
-
-    /**
-     * @dev Returns the multiplication of two unsigned integers, reverting with custom message on
-     * overflow.
-     *
-     * Counterpart to Solidity's `*` operator.
-     *
-     * Requirements:
-     *
-     * - Multiplication cannot overflow.
-     */
-    function mul(uint256 a, uint256 b, string memory errorMessage) internal pure returns (uint256) {
-        // Gas optimization: this is cheaper than requiring 'a' not being zero, but the
-        // benefit is lost if 'b' is also tested.
-        // See: https://github.com/OpenZeppelin/openzeppelin-contracts/pull/522
-        if (a == 0) {
-            return 0;
-        }
-
-        uint256 c = a * b;
-        require(c / a == b, errorMessage);
-
-        return c;
+        (bool success, uint256 result) = mulX(a, b);
+        require(success, "SafeMath: multiplication overflow");
+        return result;
     }
 
     /**
@@ -145,51 +125,67 @@ library SafeMath {
      * - The divisor cannot be zero.
      */
     function div(uint256 a, uint256 b) internal pure returns (uint256) {
-        require(b > 0, "SafeMath: division by zero");
-        uint256 c = a / b;
-        // assert(a == b * c + a % b); // There is no case in which this doesn't hold
-
-        return c;
+        (bool success, uint256 result) = divX(a, b);
+        require(success, "SafeMath: division by zero");
+        return result;
     }
 
     /**
-     * @dev Returns the integer division of two unsigned integers, reverting with custom message on
-     * division by zero. The result is rounded towards zero.
+     * @dev Returns the remainder of dividing two unsigned integers. (unsigned integer modulo),
+     * reverting when dividing by zero.
      *
-     * Counterpart to Solidity's `/` operator. Note: this function uses a
-     * `revert` opcode (which leaves remaining gas untouched) while Solidity
-     * uses an invalid opcode to revert (consuming all remaining gas).
+     * Counterpart to Solidity's `%` operator. This function uses a `revert`
+     * opcode (which leaves remaining gas untouched) while Solidity uses an
+     * invalid opcode to revert (consuming all remaining gas).
      *
      * Requirements:
      *
      * - The divisor cannot be zero.
      */
-    function div(uint256 a, uint256 b, string memory errorMessage) internal pure returns (uint256) {
-        require(b > 0, errorMessage);
-        uint256 c = a / b;
-        // assert(a == b * c + a % b); // There is no case in which this doesn't hold
+    function mod(uint256 a, uint256 b) internal pure returns (uint256) {
+        (bool success, uint256 result) = modX(a, b);
+        require(success, "SafeMath: modulo by zero");
+        return result;
+    }
 
-        return c;
+    /**
+     * @notice DEPRECATED: causes memory leak
+     * @dev Returns the subtraction of two unsigned integers, reverting with custom message on
+     * overflow (when the result is negative).
+     *
+     * Counterpart to Solidity's `-` operator.
+     *
+     * Requirements:
+     *
+     * - Subtraction cannot overflow.
+     */
+    function sub(uint256 a, uint256 b, string memory errorMessage) internal pure returns (uint256) {
+        (bool success, uint256 result) = subX(a, b);
+        require(success, errorMessage);
+        return result;
     }
 
     /**
-     * @dev Returns the remainder of dividing two unsigned integers. (unsigned integer modulo),
-     * reverting when dividing by zero.
+     * @notice DEPRECATED: causes memory leak
+     * @dev Returns the integer division of two unsigned integers, reverting with custom message on
+     * division by zero. The result is rounded towards zero.
      *
-     * Counterpart to Solidity's `%` operator. This function uses a `revert`
-     * opcode (which leaves remaining gas untouched) while Solidity uses an
-     * invalid opcode to revert (consuming all remaining gas).
+     * Counterpart to Solidity's `/` operator. Note: this function uses a
+     * `revert` opcode (which leaves remaining gas untouched) while Solidity
+     * uses an invalid opcode to revert (consuming all remaining gas).
      *
      * Requirements:
      *
      * - The divisor cannot be zero.
      */
-    function mod(uint256 a, uint256 b) internal pure returns (uint256) {
-        require(b != 0, "SafeMath: modulo by zero");
-        return a % b;
+    function div(uint256 a, uint256 b, string memory errorMessage) internal pure returns (uint256) {
+        (bool success, uint256 result) = divX(a, b);
+        require(success, errorMessage);
+        return result;
     }
 
     /**
+     * @notice DEPRECATED: causes memory leak
      * @dev Returns the remainder of dividing two unsigned integers. (unsigned integer modulo),
      * reverting with custom message when dividing by zero.
      *
@@ -202,7 +198,8 @@ library SafeMath {
      * - The divisor cannot be zero.
      */
     function mod(uint256 a, uint256 b, string memory errorMessage) internal pure returns (uint256) {
-        require(b != 0, errorMessage);
-        return a % b;
+        (bool success, uint256 result) = modX(a, b);
+        require(success, errorMessage);
+        return result;
     }
 }
diff --git a/contracts/mocks/SafeMathMock.sol b/contracts/mocks/SafeMathMock.sol
index e958eff674e..e8f393fd9d2 100644
--- a/contracts/mocks/SafeMathMock.sol
+++ b/contracts/mocks/SafeMathMock.sol
@@ -5,82 +5,74 @@ pragma solidity >=0.6.0 <0.8.0;
 import "../math/SafeMath.sol";
 
 contract SafeMathMock {
-    function mul(uint256 a, uint256 b) public pure returns (uint256) {
-        return SafeMath.mul(a, b);
-    }
-
-    function div(uint256 a, uint256 b) public pure returns (uint256) {
-        return SafeMath.div(a, b);
+    function add(uint256 a, uint256 b) public pure returns (uint256) {
+        return SafeMath.add(a, b);
     }
 
     function sub(uint256 a, uint256 b) public pure returns (uint256) {
         return SafeMath.sub(a, b);
     }
 
-    function add(uint256 a, uint256 b) public pure returns (uint256) {
-        return SafeMath.add(a, b);
-    }
-
-    function mod(uint256 a, uint256 b) public pure returns (uint256) {
-        return SafeMath.mod(a, b);
+    function mul(uint256 a, uint256 b) public pure returns (uint256) {
+        return SafeMath.mul(a, b);
     }
 
-    function mulWithMessage(uint256 a, uint256 b, string memory errorMessage) public pure returns (uint256) {
-        return SafeMath.mul(a, b, errorMessage);
+    function div(uint256 a, uint256 b) public pure returns (uint256) {
+        return SafeMath.div(a, b);
     }
 
-    function divWithMessage(uint256 a, uint256 b, string memory errorMessage) public pure returns (uint256) {
-        return SafeMath.div(a, b, errorMessage);
+    function mod(uint256 a, uint256 b) public pure returns (uint256) {
+        return SafeMath.mod(a, b);
     }
 
     function subWithMessage(uint256 a, uint256 b, string memory errorMessage) public pure returns (uint256) {
         return SafeMath.sub(a, b, errorMessage);
     }
 
-    function addWithMessage(uint256 a, uint256 b, string memory errorMessage) public pure returns (uint256) {
-        return SafeMath.add(a, b, errorMessage);
+    function divWithMessage(uint256 a, uint256 b, string memory errorMessage) public pure returns (uint256) {
+        return SafeMath.div(a, b, errorMessage);
     }
 
     function modWithMessage(uint256 a, uint256 b, string memory errorMessage) public pure returns (uint256) {
         return SafeMath.mod(a, b, errorMessage);
     }
 
-    function mulMemoryCheck() public pure returns (uint256 mem) {
+    function addMemoryCheck() public pure returns (uint256 mem) {
         uint256 length = 32;
         // solhint-disable-next-line no-inline-assembly
         assembly { mem := mload(0x40) }
-        for (uint256 i = 0; i < length; ++i) { SafeMath.mul(1, 1); }
+        for (uint256 i = 0; i < length; ++i) { SafeMath.add(1, 1); }
         // solhint-disable-next-line no-inline-assembly
         assembly { mem := sub(mload(0x40), mem) }
     }
 
-    function divMemoryCheck() public pure returns (uint256 mem) {
+    function subMemoryCheck() public pure returns (uint256 mem) {
         uint256 length = 32;
         // solhint-disable-next-line no-inline-assembly
         assembly { mem := mload(0x40) }
-        for (uint256 i = 0; i < length; ++i) { SafeMath.div(1, 1); }
+        for (uint256 i = 0; i < length; ++i) { SafeMath.sub(1, 1); }
         // solhint-disable-next-line no-inline-assembly
         assembly { mem := sub(mload(0x40), mem) }
     }
 
-    function subMemoryCheck() public pure returns (uint256 mem) {
+    function mulMemoryCheck() public pure returns (uint256 mem) {
         uint256 length = 32;
         // solhint-disable-next-line no-inline-assembly
         assembly { mem := mload(0x40) }
-        for (uint256 i = 0; i < length; ++i) { SafeMath.sub(1, 1); }
+        for (uint256 i = 0; i < length; ++i) { SafeMath.mul(1, 1); }
         // solhint-disable-next-line no-inline-assembly
         assembly { mem := sub(mload(0x40), mem) }
     }
 
-    function addMemoryCheck() public pure returns (uint256 mem) {
+    function divMemoryCheck() public pure returns (uint256 mem) {
         uint256 length = 32;
         // solhint-disable-next-line no-inline-assembly
         assembly { mem := mload(0x40) }
-        for (uint256 i = 0; i < length; ++i) { SafeMath.add(1, 1); }
+        for (uint256 i = 0; i < length; ++i) { SafeMath.div(1, 1); }
         // solhint-disable-next-line no-inline-assembly
         assembly { mem := sub(mload(0x40), mem) }
     }
-
+    
     function modMemoryCheck() public pure returns (uint256 mem) {
         uint256 length = 32;
         // solhint-disable-next-line no-inline-assembly
diff --git a/test/math/SafeMath.test.js b/test/math/SafeMath.test.js
index 4b3b59a9cb2..12c63853b22 100644
--- a/test/math/SafeMath.test.js
+++ b/test/math/SafeMath.test.js
@@ -147,22 +147,6 @@ contract('SafeMath', function (accounts) {
   });
 
   describe('with custom revert message', function () {
-    describe('add', function () {
-      it('adds correctly', async function () {
-        const a = new BN('5678');
-        const b = new BN('1234');
-
-        await testCommutative(this.safeMath.addWithMessage, a, b, a.add(b), 'MyErrorMessage');
-      });
-
-      it('reverts on addition overflow', async function () {
-        const a = MAX_UINT256;
-        const b = new BN('1');
-
-        await testFailsCommutative(this.safeMath.addWithMessage, a, b, 'MyErrorMessage', 'MyErrorMessage');
-      });
-    });
-
     describe('sub', function () {
       it('subtracts correctly', async function () {
         const a = new BN('5678');
@@ -179,29 +163,6 @@ contract('SafeMath', function (accounts) {
       });
     });
 
-    describe('mul', function () {
-      it('multiplies correctly', async function () {
-        const a = new BN('1234');
-        const b = new BN('5678');
-
-        await testCommutative(this.safeMath.mulWithMessage, a, b, a.mul(b), 'MyErrorMessage');
-      });
-
-      it('multiplies by zero correctly', async function () {
-        const a = new BN('0');
-        const b = new BN('5678');
-
-        await testCommutative(this.safeMath.mulWithMessage, a, b, '0', 'MyErrorMessage');
-      });
-
-      it('reverts on multiplication overflow', async function () {
-        const a = MAX_UINT256;
-        const b = new BN('2');
-
-        await testFailsCommutative(this.safeMath.mulWithMessage, a, b, 'MyErrorMessage', 'MyErrorMessage');
-      });
-    });
-
     describe('div', function () {
       it('divides correctly', async function () {
         const a = new BN('5678');

From 39bd25d2846517dfac94921e5bf1c953faacfbc0 Mon Sep 17 00:00:00 2001
From: Hadrien Croubois <hadrien.croubois@gmail.com>
Date: Mon, 11 Jan 2021 22:05:44 +0100
Subject: [PATCH 05/16] reduce gas usage by avoiding double if

---
 contracts/math/SafeMath.sol      |  47 +++++-----
 contracts/mocks/SafeMathMock.sol |  22 ++++-
 test/math/SafeMath.test.js       | 143 +++++++++++++++++++++++++++++++
 3 files changed, 184 insertions(+), 28 deletions(-)

diff --git a/contracts/math/SafeMath.sol b/contracts/math/SafeMath.sol
index 075d3efa71d..aed69437ce3 100644
--- a/contracts/math/SafeMath.sol
+++ b/contracts/math/SafeMath.sol
@@ -40,9 +40,7 @@ library SafeMath {
         // Gas optimization: this is cheaper than requiring 'a' not being zero, but the
         // benefit is lost if 'b' is also tested.
         // See: https://github.com/OpenZeppelin/openzeppelin-contracts/pull/522
-        if (a == 0) {
-            return (true, 0);
-        }
+        if (a == 0) return (true, 0);
         uint256 c = a * b;
         if (c / a != b) return (false, 0);
         return (true, c);
@@ -75,9 +73,9 @@ library SafeMath {
      * - Addition cannot overflow.
      */
     function add(uint256 a, uint256 b) internal pure returns (uint256) {
-        (bool success, uint256 result) = addX(a, b);
-        require(success, "SafeMath: addition overflow");
-        return result;
+        uint256 c = a + b;
+        require(c >= a, "SafeMath: addition overflow");
+        return c;
     }
 
     /**
@@ -91,9 +89,8 @@ library SafeMath {
      * - Subtraction cannot overflow.
      */
     function sub(uint256 a, uint256 b) internal pure returns (uint256) {
-        (bool success, uint256 result) = subX(a, b);
-        require(success, "SafeMath: subtraction overflow");
-        return result;
+        require(b <= a, "SafeMath: subtraction overflow");
+        return a - b;
     }
 
     /**
@@ -107,9 +104,10 @@ library SafeMath {
      * - Multiplication cannot overflow.
      */
     function mul(uint256 a, uint256 b) internal pure returns (uint256) {
-        (bool success, uint256 result) = mulX(a, b);
-        require(success, "SafeMath: multiplication overflow");
-        return result;
+        if (a == 0) return 0;
+        uint256 c = a * b;
+        require(c / a == b, "SafeMath: multiplication overflow");
+        return c;
     }
 
     /**
@@ -125,9 +123,8 @@ library SafeMath {
      * - The divisor cannot be zero.
      */
     function div(uint256 a, uint256 b) internal pure returns (uint256) {
-        (bool success, uint256 result) = divX(a, b);
-        require(success, "SafeMath: division by zero");
-        return result;
+        require(b > 0, "SafeMath: division by zero");
+        return a / b;
     }
 
     /**
@@ -143,9 +140,8 @@ library SafeMath {
      * - The divisor cannot be zero.
      */
     function mod(uint256 a, uint256 b) internal pure returns (uint256) {
-        (bool success, uint256 result) = modX(a, b);
-        require(success, "SafeMath: modulo by zero");
-        return result;
+        require(b > 0, "SafeMath: modulo by zero");
+        return a % b;
     }
 
     /**
@@ -160,9 +156,8 @@ library SafeMath {
      * - Subtraction cannot overflow.
      */
     function sub(uint256 a, uint256 b, string memory errorMessage) internal pure returns (uint256) {
-        (bool success, uint256 result) = subX(a, b);
-        require(success, errorMessage);
-        return result;
+        require(b <= a, errorMessage);
+        return a - b;
     }
 
     /**
@@ -179,9 +174,8 @@ library SafeMath {
      * - The divisor cannot be zero.
      */
     function div(uint256 a, uint256 b, string memory errorMessage) internal pure returns (uint256) {
-        (bool success, uint256 result) = divX(a, b);
-        require(success, errorMessage);
-        return result;
+        require(b > 0, errorMessage);
+        return a / b;
     }
 
     /**
@@ -198,8 +192,7 @@ library SafeMath {
      * - The divisor cannot be zero.
      */
     function mod(uint256 a, uint256 b, string memory errorMessage) internal pure returns (uint256) {
-        (bool success, uint256 result) = modX(a, b);
-        require(success, errorMessage);
-        return result;
+        require(b > 0, errorMessage);
+        return a % b;
     }
 }
diff --git a/contracts/mocks/SafeMathMock.sol b/contracts/mocks/SafeMathMock.sol
index e8f393fd9d2..394640d63a3 100644
--- a/contracts/mocks/SafeMathMock.sol
+++ b/contracts/mocks/SafeMathMock.sol
@@ -5,6 +5,26 @@ pragma solidity >=0.6.0 <0.8.0;
 import "../math/SafeMath.sol";
 
 contract SafeMathMock {
+    function addWithFlag(uint256 a, uint256 b) public pure returns (bool flag, uint256 value) {
+        return SafeMath.addX(a, b);
+    }
+
+    function subWithFlag(uint256 a, uint256 b) public pure returns (bool flag, uint256 value) {
+        return SafeMath.subX(a, b);
+    }
+
+    function mulWithFlag(uint256 a, uint256 b) public pure returns (bool flag, uint256 value) {
+        return SafeMath.mulX(a, b);
+    }
+
+    function divWithFlag(uint256 a, uint256 b) public pure returns (bool flag, uint256 value) {
+        return SafeMath.divX(a, b);
+    }
+
+    function modWithFlag(uint256 a, uint256 b) public pure returns (bool flag, uint256 value) {
+        return SafeMath.modX(a, b);
+    }
+
     function add(uint256 a, uint256 b) public pure returns (uint256) {
         return SafeMath.add(a, b);
     }
@@ -72,7 +92,7 @@ contract SafeMathMock {
         // solhint-disable-next-line no-inline-assembly
         assembly { mem := sub(mload(0x40), mem) }
     }
-    
+
     function modMemoryCheck() public pure returns (uint256 mem) {
         uint256 length = 32;
         // solhint-disable-next-line no-inline-assembly
diff --git a/test/math/SafeMath.test.js b/test/math/SafeMath.test.js
index 12c63853b22..ba063a81b77 100644
--- a/test/math/SafeMath.test.js
+++ b/test/math/SafeMath.test.js
@@ -5,6 +5,18 @@ const { expect } = require('chai');
 
 const SafeMathMock = artifacts.require('SafeMathMock');
 
+async function deepExpect (value, expected) {
+  for (const key in expected) {
+    if (BN.isBN(value[key])) {
+      expect(value[key]).to.be.bignumber.equal(expected[key]);
+    } else if (Symbol.iterator in Object(value)) {
+      deepExpect(value[key], expected[key]);
+    } else {
+      expect(value[key]).to.be.equal(expected[key]);
+    }
+  }
+}
+
 contract('SafeMath', function (accounts) {
   beforeEach(async function () {
     this.safeMath = await SafeMathMock.new();
@@ -20,6 +32,137 @@ contract('SafeMath', function (accounts) {
     await expectRevert(fn(rhs, lhs, ...extra), reason);
   }
 
+  async function testCommutativeIterable (fn, lhs, rhs, expected, ...extra) {
+    deepExpect(await fn(lhs, rhs, ...extra), expected);
+    deepExpect(await fn(rhs, lhs, ...extra), expected);
+  }
+
+  describe('with flag', function () {
+    describe('add', function () {
+      it('adds correctly', async function () {
+        const a = new BN('5678');
+        const b = new BN('1234');
+
+        testCommutativeIterable(this.safeMath.addWithFlag, a, b, { flag: true, value: a.add(b) });
+      });
+
+      it('reverts on addition overflow', async function () {
+        const a = MAX_UINT256;
+        const b = new BN('1');
+
+        testCommutativeIterable(this.safeMath.addWithFlag, a, b, { flag: false, value: '0' });
+      });
+    });
+
+    describe('sub', function () {
+      it('subtracts correctly', async function () {
+        const a = new BN('5678');
+        const b = new BN('1234');
+
+        deepExpect(await this.safeMath.subWithFlag(a, b), { flag: true, value: a.sub(b) });
+      });
+
+      it('reverts if subtraction result would be negative', async function () {
+        const a = new BN('1234');
+        const b = new BN('5678');
+
+        deepExpect(await this.safeMath.subWithFlag(a, b), { flag: false, value: '0' });
+      });
+    });
+
+    describe('mul', function () {
+      it('multiplies correctly', async function () {
+        const a = new BN('1234');
+        const b = new BN('5678');
+
+        testCommutativeIterable(this.safeMath.mulWithFlag, a, b, { flag: true, value: a.mul(b) });
+      });
+
+      it('multiplies by zero correctly', async function () {
+        const a = new BN('0');
+        const b = new BN('5678');
+
+        testCommutativeIterable(this.safeMath.mulWithFlag, a, b, { flag: true, value: a.mul(b) });
+      });
+
+      it('reverts on multiplication overflow', async function () {
+        const a = MAX_UINT256;
+        const b = new BN('2');
+
+        testCommutativeIterable(this.safeMath.mulWithFlag, a, b, { flag: false, value: '0' });
+      });
+    });
+
+    describe('div', function () {
+      it('divides correctly', async function () {
+        const a = new BN('5678');
+        const b = new BN('5678');
+
+        deepExpect(await this.safeMath.divWithFlag(a, b), { flag: true, value: a.div(b) });
+      });
+
+      it('divides zero correctly', async function () {
+        const a = new BN('0');
+        const b = new BN('5678');
+
+        deepExpect(await this.safeMath.divWithFlag(a, b), { flag: true, value: a.div(b) });
+      });
+
+      it('returns complete number result on non-even division', async function () {
+        const a = new BN('7000');
+        const b = new BN('5678');
+
+        deepExpect(await this.safeMath.divWithFlag(a, b), { flag: true, value: a.div(b) });
+      });
+
+      it('reverts on division by zero', async function () {
+        const a = new BN('5678');
+        const b = new BN('0');
+
+        deepExpect(await this.safeMath.divWithFlag(a, b), { flag: false, value: '0' });
+      });
+    });
+
+    describe('mod', function () {
+      describe('modulos correctly', async function () {
+        it('when the dividend is smaller than the divisor', async function () {
+          const a = new BN('284');
+          const b = new BN('5678');
+
+          deepExpect(await this.safeMath.modWithFlag(a, b), { flag: true, value: a.mod(b) });
+        });
+
+        it('when the dividend is equal to the divisor', async function () {
+          const a = new BN('5678');
+          const b = new BN('5678');
+
+          deepExpect(await this.safeMath.modWithFlag(a, b), { flag: true, value: a.mod(b) });
+        });
+
+        it('when the dividend is larger than the divisor', async function () {
+          const a = new BN('7000');
+          const b = new BN('5678');
+
+          deepExpect(await this.safeMath.modWithFlag(a, b), { flag: true, value: a.mod(b) });
+        });
+
+        it('when the dividend is a multiple of the divisor', async function () {
+          const a = new BN('17034'); // 17034 == 5678 * 3
+          const b = new BN('5678');
+
+          deepExpect(await this.safeMath.modWithFlag(a, b), { flag: true, value: a.mod(b) });
+        });
+      });
+
+      it('reverts with a 0 divisor', async function () {
+        const a = new BN('5678');
+        const b = new BN('0');
+
+        deepExpect(await this.safeMath.modWithFlag(a, b), { flag: false, value: '0' });
+      });
+    });
+  });
+
   describe('with default revert message', function () {
     describe('add', function () {
       it('adds correctly', async function () {

From 23b65b20fb3343ef5619ea1cf5aabdb39b9cbe22 Mon Sep 17 00:00:00 2001
From: Hadrien Croubois <hadrien.croubois@gmail.com>
Date: Fri, 15 Jan 2021 10:26:46 +0100
Subject: [PATCH 06/16] addX renamed to tryAdd

---
 contracts/math/SafeMath.sol      | 10 +++++-----
 contracts/mocks/SafeMathMock.sol | 20 ++++++++++----------
 test/math/SafeMath.test.js       | 32 ++++++++++++++++----------------
 3 files changed, 31 insertions(+), 31 deletions(-)

diff --git a/contracts/math/SafeMath.sol b/contracts/math/SafeMath.sol
index aed69437ce3..8c04f9e9879 100644
--- a/contracts/math/SafeMath.sol
+++ b/contracts/math/SafeMath.sol
@@ -19,7 +19,7 @@ library SafeMath {
     /**
      * @dev Returns the addition of two unsigned integers, with an overflow flag.
      */
-    function addX(uint256 a, uint256 b) internal pure returns (bool, uint256) {
+    function tryAdd(uint256 a, uint256 b) internal pure returns (bool, uint256) {
         uint256 c = a + b;
         if (c < a) return (false, 0);
         return (true, c);
@@ -28,7 +28,7 @@ library SafeMath {
     /**
      * @dev Returns the substraction of two unsigned integers, with an overflow flag.
      */
-    function subX(uint256 a, uint256 b) internal pure returns (bool, uint256) {
+    function trySub(uint256 a, uint256 b) internal pure returns (bool, uint256) {
         if (b > a) return (false, 0);
         return (true, a - b);
     }
@@ -36,7 +36,7 @@ library SafeMath {
     /**
      * @dev Returns the multiplication of two unsigned integers, with an overflow flag.
      */
-    function mulX(uint256 a, uint256 b) internal pure returns (bool, uint256) {
+    function tryMul(uint256 a, uint256 b) internal pure returns (bool, uint256) {
         // Gas optimization: this is cheaper than requiring 'a' not being zero, but the
         // benefit is lost if 'b' is also tested.
         // See: https://github.com/OpenZeppelin/openzeppelin-contracts/pull/522
@@ -49,7 +49,7 @@ library SafeMath {
     /**
      * @dev Returns the division of two unsigned integers, with a division by zero flag.
      */
-    function divX(uint256 a, uint256 b) internal pure returns (bool, uint256) {
+    function tryDiv(uint256 a, uint256 b) internal pure returns (bool, uint256) {
         if (b == 0) return (false, 0);
         return (true, a / b);
     }
@@ -57,7 +57,7 @@ library SafeMath {
     /**
      * @dev Returns the remainder of dividing two unsigned integers, with a division by zero flag.
      */
-    function modX(uint256 a, uint256 b) internal pure returns (bool, uint256) {
+    function tryMod(uint256 a, uint256 b) internal pure returns (bool, uint256) {
         if (b == 0) return (false, 0);
         return (true, a % b);
     }
diff --git a/contracts/mocks/SafeMathMock.sol b/contracts/mocks/SafeMathMock.sol
index 394640d63a3..c29296a4e63 100644
--- a/contracts/mocks/SafeMathMock.sol
+++ b/contracts/mocks/SafeMathMock.sol
@@ -5,24 +5,24 @@ pragma solidity >=0.6.0 <0.8.0;
 import "../math/SafeMath.sol";
 
 contract SafeMathMock {
-    function addWithFlag(uint256 a, uint256 b) public pure returns (bool flag, uint256 value) {
-        return SafeMath.addX(a, b);
+    function tryAdd(uint256 a, uint256 b) public pure returns (bool flag, uint256 value) {
+        return SafeMath.tryAdd(a, b);
     }
 
-    function subWithFlag(uint256 a, uint256 b) public pure returns (bool flag, uint256 value) {
-        return SafeMath.subX(a, b);
+    function trySub(uint256 a, uint256 b) public pure returns (bool flag, uint256 value) {
+        return SafeMath.trySub(a, b);
     }
 
-    function mulWithFlag(uint256 a, uint256 b) public pure returns (bool flag, uint256 value) {
-        return SafeMath.mulX(a, b);
+    function tryMul(uint256 a, uint256 b) public pure returns (bool flag, uint256 value) {
+        return SafeMath.tryMul(a, b);
     }
 
-    function divWithFlag(uint256 a, uint256 b) public pure returns (bool flag, uint256 value) {
-        return SafeMath.divX(a, b);
+    function tryDiv(uint256 a, uint256 b) public pure returns (bool flag, uint256 value) {
+        return SafeMath.tryDiv(a, b);
     }
 
-    function modWithFlag(uint256 a, uint256 b) public pure returns (bool flag, uint256 value) {
-        return SafeMath.modX(a, b);
+    function tryMod(uint256 a, uint256 b) public pure returns (bool flag, uint256 value) {
+        return SafeMath.tryMod(a, b);
     }
 
     function add(uint256 a, uint256 b) public pure returns (uint256) {
diff --git a/test/math/SafeMath.test.js b/test/math/SafeMath.test.js
index ba063a81b77..fab0b544cdc 100644
--- a/test/math/SafeMath.test.js
+++ b/test/math/SafeMath.test.js
@@ -43,14 +43,14 @@ contract('SafeMath', function (accounts) {
         const a = new BN('5678');
         const b = new BN('1234');
 
-        testCommutativeIterable(this.safeMath.addWithFlag, a, b, { flag: true, value: a.add(b) });
+        testCommutativeIterable(this.safeMath.tryAdd, a, b, { flag: true, value: a.add(b) });
       });
 
       it('reverts on addition overflow', async function () {
         const a = MAX_UINT256;
         const b = new BN('1');
 
-        testCommutativeIterable(this.safeMath.addWithFlag, a, b, { flag: false, value: '0' });
+        testCommutativeIterable(this.safeMath.tryAdd, a, b, { flag: false, value: '0' });
       });
     });
 
@@ -59,14 +59,14 @@ contract('SafeMath', function (accounts) {
         const a = new BN('5678');
         const b = new BN('1234');
 
-        deepExpect(await this.safeMath.subWithFlag(a, b), { flag: true, value: a.sub(b) });
+        deepExpect(await this.safeMath.trySub(a, b), { flag: true, value: a.sub(b) });
       });
 
       it('reverts if subtraction result would be negative', async function () {
         const a = new BN('1234');
         const b = new BN('5678');
 
-        deepExpect(await this.safeMath.subWithFlag(a, b), { flag: false, value: '0' });
+        deepExpect(await this.safeMath.trySub(a, b), { flag: false, value: '0' });
       });
     });
 
@@ -75,21 +75,21 @@ contract('SafeMath', function (accounts) {
         const a = new BN('1234');
         const b = new BN('5678');
 
-        testCommutativeIterable(this.safeMath.mulWithFlag, a, b, { flag: true, value: a.mul(b) });
+        testCommutativeIterable(this.safeMath.tryMul, a, b, { flag: true, value: a.mul(b) });
       });
 
       it('multiplies by zero correctly', async function () {
         const a = new BN('0');
         const b = new BN('5678');
 
-        testCommutativeIterable(this.safeMath.mulWithFlag, a, b, { flag: true, value: a.mul(b) });
+        testCommutativeIterable(this.safeMath.tryMul, a, b, { flag: true, value: a.mul(b) });
       });
 
       it('reverts on multiplication overflow', async function () {
         const a = MAX_UINT256;
         const b = new BN('2');
 
-        testCommutativeIterable(this.safeMath.mulWithFlag, a, b, { flag: false, value: '0' });
+        testCommutativeIterable(this.safeMath.tryMul, a, b, { flag: false, value: '0' });
       });
     });
 
@@ -98,28 +98,28 @@ contract('SafeMath', function (accounts) {
         const a = new BN('5678');
         const b = new BN('5678');
 
-        deepExpect(await this.safeMath.divWithFlag(a, b), { flag: true, value: a.div(b) });
+        deepExpect(await this.safeMath.tryDiv(a, b), { flag: true, value: a.div(b) });
       });
 
       it('divides zero correctly', async function () {
         const a = new BN('0');
         const b = new BN('5678');
 
-        deepExpect(await this.safeMath.divWithFlag(a, b), { flag: true, value: a.div(b) });
+        deepExpect(await this.safeMath.tryDiv(a, b), { flag: true, value: a.div(b) });
       });
 
       it('returns complete number result on non-even division', async function () {
         const a = new BN('7000');
         const b = new BN('5678');
 
-        deepExpect(await this.safeMath.divWithFlag(a, b), { flag: true, value: a.div(b) });
+        deepExpect(await this.safeMath.tryDiv(a, b), { flag: true, value: a.div(b) });
       });
 
       it('reverts on division by zero', async function () {
         const a = new BN('5678');
         const b = new BN('0');
 
-        deepExpect(await this.safeMath.divWithFlag(a, b), { flag: false, value: '0' });
+        deepExpect(await this.safeMath.tryDiv(a, b), { flag: false, value: '0' });
       });
     });
 
@@ -129,28 +129,28 @@ contract('SafeMath', function (accounts) {
           const a = new BN('284');
           const b = new BN('5678');
 
-          deepExpect(await this.safeMath.modWithFlag(a, b), { flag: true, value: a.mod(b) });
+          deepExpect(await this.safeMath.tryMod(a, b), { flag: true, value: a.mod(b) });
         });
 
         it('when the dividend is equal to the divisor', async function () {
           const a = new BN('5678');
           const b = new BN('5678');
 
-          deepExpect(await this.safeMath.modWithFlag(a, b), { flag: true, value: a.mod(b) });
+          deepExpect(await this.safeMath.tryMod(a, b), { flag: true, value: a.mod(b) });
         });
 
         it('when the dividend is larger than the divisor', async function () {
           const a = new BN('7000');
           const b = new BN('5678');
 
-          deepExpect(await this.safeMath.modWithFlag(a, b), { flag: true, value: a.mod(b) });
+          deepExpect(await this.safeMath.tryMod(a, b), { flag: true, value: a.mod(b) });
         });
 
         it('when the dividend is a multiple of the divisor', async function () {
           const a = new BN('17034'); // 17034 == 5678 * 3
           const b = new BN('5678');
 
-          deepExpect(await this.safeMath.modWithFlag(a, b), { flag: true, value: a.mod(b) });
+          deepExpect(await this.safeMath.tryMod(a, b), { flag: true, value: a.mod(b) });
         });
       });
 
@@ -158,7 +158,7 @@ contract('SafeMath', function (accounts) {
         const a = new BN('5678');
         const b = new BN('0');
 
-        deepExpect(await this.safeMath.modWithFlag(a, b), { flag: false, value: '0' });
+        deepExpect(await this.safeMath.tryMod(a, b), { flag: false, value: '0' });
       });
     });
   });

From 2b69162313c35402606c8df5200beffc91743c56 Mon Sep 17 00:00:00 2001
From: Hadrien Croubois <hadrien.croubois@gmail.com>
Date: Fri, 15 Jan 2021 10:40:01 +0100
Subject: [PATCH 07/16] addressing the memory leak issue in EnumerableMap

---
 contracts/mocks/EnumerableMapMock.sol |  8 +++++
 contracts/utils/EnumerableMap.sol     | 23 ++++++++++++++-
 test/utils/EnumerableMap.test.js      | 42 ++++++++++++++++++++++++++-
 3 files changed, 71 insertions(+), 2 deletions(-)

diff --git a/contracts/mocks/EnumerableMapMock.sol b/contracts/mocks/EnumerableMapMock.sol
index 79226d739ec..28472b91787 100644
--- a/contracts/mocks/EnumerableMapMock.sol
+++ b/contracts/mocks/EnumerableMapMock.sol
@@ -34,7 +34,15 @@ contract EnumerableMapMock {
     }
 
 
+    function tryGet(uint256 key) public view returns (bool, address) {
+        return _map.tryGet(key);
+    }
+
     function get(uint256 key) public view returns (address) {
         return _map.get(key);
     }
+
+    function getWithMessage(uint256 key, string calldata errorMessage) public view returns (address) {
+        return _map.get(key, errorMessage);
+    }
 }
diff --git a/contracts/utils/EnumerableMap.sol b/contracts/utils/EnumerableMap.sol
index 2daa131905a..3eca02d0a7f 100644
--- a/contracts/utils/EnumerableMap.sol
+++ b/contracts/utils/EnumerableMap.sol
@@ -143,6 +143,16 @@ library EnumerableMap {
         return (entry._key, entry._value);
     }
 
+    /**
+     * @dev Tries to returns the value associated with `key`.  O(1).
+     * Does not revert if `key` is not in the map.
+     */
+    function _tryGet(Map storage map, bytes32 key) private view returns (bool, bytes32) {
+        uint256 keyIndex = map._indexes[key];
+        if (keyIndex == 0) return (false, 0); // Equivalent to contains(map, key)
+        return (true, map._entries[keyIndex - 1]._value); // All indexes are 1-based
+    }
+
     /**
      * @dev Returns the value associated with `key`.  O(1).
      *
@@ -151,7 +161,9 @@ library EnumerableMap {
      * - `key` must be in the map.
      */
     function _get(Map storage map, bytes32 key) private view returns (bytes32) {
-        return _get(map, key, "EnumerableMap: nonexistent key");
+        uint256 keyIndex = map._indexes[key];
+        require(keyIndex != 0, "EnumerableMap: nonexistent key"); // Equivalent to contains(map, key)
+        return map._entries[keyIndex - 1]._value; // All indexes are 1-based
     }
 
     /**
@@ -217,6 +229,15 @@ library EnumerableMap {
         return (uint256(key), address(uint160(uint256(value))));
     }
 
+    /**
+     * @dev Tries to returns the value associated with `key`.  O(1).
+     * Does not revert if `key` is not in the map.
+     */
+    function tryGet(UintToAddressMap storage map, uint256 key) internal view returns (bool, address) {
+        (bool success, bytes32 value) = _tryGet(map._inner, bytes32(key));
+        return (success, address(uint160(uint256(value))));
+    }
+
     /**
      * @dev Returns the value associated with `key`.  O(1).
      *
diff --git a/test/utils/EnumerableMap.test.js b/test/utils/EnumerableMap.test.js
index fcbdba6d7fd..29052d0f3a7 100644
--- a/test/utils/EnumerableMap.test.js
+++ b/test/utils/EnumerableMap.test.js
@@ -1,4 +1,4 @@
-const { BN, expectEvent } = require('@openzeppelin/test-helpers');
+const { BN, constants, expectEvent, expectRevert } = require('@openzeppelin/test-helpers');
 const { expect } = require('chai');
 
 const zip = require('lodash.zip');
@@ -139,4 +139,44 @@ contract('EnumerableMap', function (accounts) {
       expect(await this.map.contains(keyB)).to.equal(false);
     });
   });
+
+  describe('read', function () {
+    beforeEach(async function () {
+      await this.map.set(keyA, accountA);
+    });
+
+    describe('get', function () {
+      it('existing value', async function () {
+        expect(await this.map.get(keyA)).to.be.equal(accountA);
+      });
+      it('missing value', async function () {
+        await expectRevert(this.map.get(keyB), "EnumerableMap: nonexistent key");
+      });
+    });
+
+    describe('get with message', function () {
+      it('existing value', async function () {
+        expect(await this.map.getWithMessage(keyA, "custom error string")).to.be.equal(accountA);
+      });
+      it('missing value', async function () {
+        await expectRevert(this.map.getWithMessage(keyB, "custom error string"), "custom error string");
+      });
+    });
+
+    describe('tryGet', function () {
+      it('existing value', async function () {
+        expect(await this.map.tryGet(keyA)).to.be.deep.equal({
+          '0': true,
+          '1': accountA
+        });
+      });
+      it('missing value', async function () {
+        expect(await this.map.tryGet(keyB)).to.be.deep.equal({
+          '0': false,
+          '1': constants.ZERO_ADDRESS
+        });
+      });
+    });
+
+  });
 });

From b75705e8ffc93316891e8d2d0994f6ce1894cfe7 Mon Sep 17 00:00:00 2001
From: Hadrien Croubois <hadrien.croubois@gmail.com>
Date: Fri, 15 Jan 2021 10:51:10 +0100
Subject: [PATCH 08/16] test lint + Changelog entry

---
 CHANGELOG.md                     |  6 ++++--
 test/utils/EnumerableMap.test.js | 15 +++++++--------
 2 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 6f2576c72a0..ca70567e691 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -7,9 +7,11 @@
  * `ERC20Permit`: added an implementation of the ERC20 permit extension for gasless token approvals. ([#2237](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2237))
  * Presets: added token presets with preminted fixed supply `ERC20PresetFixedSupply` and `ERC777PresetFixedSupply`. ([#2399](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2399))
  * `Address`: added `functionDelegateCall`, similar to the existing `functionCall`. ([#2333](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2333))
- * `Context`: moved from `contracts/GSN` to `contracts/utils`. ([#2453](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2453)) 
+ * `Context`: moved from `contracts/GSN` to `contracts/utils`. ([#2453](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2453))
  * `PaymentSplitter`: replace usage of `.transfer()` with `Address.sendValue` for improved compatibility with smart wallets. ([#2455](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2455))
- * `UpgradeableProxy`: bubble revert reasons from initialization calls. ([#2454](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2454)) 
+ * `UpgradeableProxy`: bubble revert reasons from initialization calls. ([#2454](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2454))
+ * `SafeMath`: fix a memory allocation issue by adding new `SafeMath.tryXXX(uint,uint)→(bool,uint)` function. `SafeMath.XXX(uint,uint,string)→(uint)` are not deprecated. ([#2462](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2462))
+ * `EnumerableMap`: fix a memory allocation issue by adding new `EnumerableMap.tryGet(uint)→(bool,address)` function. `SafeMath.get(uint)→(string)` is now deprecated. ([#2462](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2462))
 
 ## 3.3.0 (2020-11-26)
 
diff --git a/test/utils/EnumerableMap.test.js b/test/utils/EnumerableMap.test.js
index 29052d0f3a7..9dc700b510b 100644
--- a/test/utils/EnumerableMap.test.js
+++ b/test/utils/EnumerableMap.test.js
@@ -150,33 +150,32 @@ contract('EnumerableMap', function (accounts) {
         expect(await this.map.get(keyA)).to.be.equal(accountA);
       });
       it('missing value', async function () {
-        await expectRevert(this.map.get(keyB), "EnumerableMap: nonexistent key");
+        await expectRevert(this.map.get(keyB), 'EnumerableMap: nonexistent key');
       });
     });
 
     describe('get with message', function () {
       it('existing value', async function () {
-        expect(await this.map.getWithMessage(keyA, "custom error string")).to.be.equal(accountA);
+        expect(await this.map.getWithMessage(keyA, 'custom error string')).to.be.equal(accountA);
       });
       it('missing value', async function () {
-        await expectRevert(this.map.getWithMessage(keyB, "custom error string"), "custom error string");
+        await expectRevert(this.map.getWithMessage(keyB, 'custom error string'), 'custom error string');
       });
     });
 
     describe('tryGet', function () {
       it('existing value', async function () {
         expect(await this.map.tryGet(keyA)).to.be.deep.equal({
-          '0': true,
-          '1': accountA
+          0: true,
+          1: accountA,
         });
       });
       it('missing value', async function () {
         expect(await this.map.tryGet(keyB)).to.be.deep.equal({
-          '0': false,
-          '1': constants.ZERO_ADDRESS
+          0: false,
+          1: constants.ZERO_ADDRESS,
         });
       });
     });
-
   });
 });

From ce3a5c56a5c7a760530ebe46c780f01445dfb686 Mon Sep 17 00:00:00 2001
From: Hadrien Croubois <hc@iex.ec>
Date: Sat, 16 Jan 2021 00:46:02 +0100
Subject: [PATCH 09/16] Update CHANGELOG.md

Co-authored-by: Francisco Giordano <frangio.1@gmail.com>
---
 CHANGELOG.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index ca70567e691..5ed2e80520f 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -10,7 +10,7 @@
  * `Context`: moved from `contracts/GSN` to `contracts/utils`. ([#2453](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2453))
  * `PaymentSplitter`: replace usage of `.transfer()` with `Address.sendValue` for improved compatibility with smart wallets. ([#2455](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2455))
  * `UpgradeableProxy`: bubble revert reasons from initialization calls. ([#2454](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2454))
- * `SafeMath`: fix a memory allocation issue by adding new `SafeMath.tryXXX(uint,uint)→(bool,uint)` function. `SafeMath.XXX(uint,uint,string)→(uint)` are not deprecated. ([#2462](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2462))
+ * `SafeMath`: fix a memory allocation issue by adding new `SafeMath.tryOp(uint,uint)→(bool,uint)` functions. `SafeMath.op(uint,uint,string)→uint` are now deprecated. ([#2462](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2462))
  * `EnumerableMap`: fix a memory allocation issue by adding new `EnumerableMap.tryGet(uint)→(bool,address)` function. `SafeMath.get(uint)→(string)` is now deprecated. ([#2462](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2462))
 
 ## 3.3.0 (2020-11-26)

From 796e7b0f568ab96fa6c3b7552aa76d7790478d34 Mon Sep 17 00:00:00 2001
From: Hadrien Croubois <hc@iex.ec>
Date: Sat, 16 Jan 2021 00:46:14 +0100
Subject: [PATCH 10/16] Update CHANGELOG.md

Co-authored-by: Francisco Giordano <frangio.1@gmail.com>
---
 CHANGELOG.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 5ed2e80520f..d3d794b4026 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -11,7 +11,7 @@
  * `PaymentSplitter`: replace usage of `.transfer()` with `Address.sendValue` for improved compatibility with smart wallets. ([#2455](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2455))
  * `UpgradeableProxy`: bubble revert reasons from initialization calls. ([#2454](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2454))
  * `SafeMath`: fix a memory allocation issue by adding new `SafeMath.tryOp(uint,uint)→(bool,uint)` functions. `SafeMath.op(uint,uint,string)→uint` are now deprecated. ([#2462](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2462))
- * `EnumerableMap`: fix a memory allocation issue by adding new `EnumerableMap.tryGet(uint)→(bool,address)` function. `SafeMath.get(uint)→(string)` is now deprecated. ([#2462](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2462))
+ * `EnumerableMap`: fix a memory allocation issue by adding new `EnumerableMap.tryGet(uint)→(bool,address)` functions. `EnumerableMap.get(uint)→string` is now deprecated. ([#2462](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2462))
 
 ## 3.3.0 (2020-11-26)
 

From f09506b615d5cdf4040e0059ca02417e70bbb0c1 Mon Sep 17 00:00:00 2001
From: Hadrien Croubois <hc@iex.ec>
Date: Sat, 16 Jan 2021 00:46:39 +0100
Subject: [PATCH 11/16] Update contracts/math/SafeMath.sol

Co-authored-by: Francisco Giordano <frangio.1@gmail.com>
---
 contracts/math/SafeMath.sol | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/contracts/math/SafeMath.sol b/contracts/math/SafeMath.sol
index 8c04f9e9879..89c0ec3d529 100644
--- a/contracts/math/SafeMath.sol
+++ b/contracts/math/SafeMath.sol
@@ -145,10 +145,12 @@ library SafeMath {
     }
 
     /**
-     * @notice DEPRECATED: causes memory leak
      * @dev Returns the subtraction of two unsigned integers, reverting with custom message on
      * overflow (when the result is negative).
      *
+     * CAUTION: This function is deprecated because it requires allocating memory for the error
+     * message unnecessarily. For custom revert reasons use {trySub}.
+     *
      * Counterpart to Solidity's `-` operator.
      *
      * Requirements:

From 241f5d59aecdfca15286f73f848dc286451b1da5 Mon Sep 17 00:00:00 2001
From: Hadrien Croubois <hc@iex.ec>
Date: Sat, 16 Jan 2021 00:47:02 +0100
Subject: [PATCH 12/16] Update contracts/math/SafeMath.sol

Co-authored-by: Francisco Giordano <frangio.1@gmail.com>
---
 contracts/math/SafeMath.sol | 1 -
 1 file changed, 1 deletion(-)

diff --git a/contracts/math/SafeMath.sol b/contracts/math/SafeMath.sol
index 89c0ec3d529..d7be6cdf582 100644
--- a/contracts/math/SafeMath.sol
+++ b/contracts/math/SafeMath.sol
@@ -181,7 +181,6 @@ library SafeMath {
     }
 
     /**
-     * @notice DEPRECATED: causes memory leak
      * @dev Returns the remainder of dividing two unsigned integers. (unsigned integer modulo),
      * reverting with custom message when dividing by zero.
      *

From 05e66186d43d58af9c4210b809b5d2ee68f5ae16 Mon Sep 17 00:00:00 2001
From: Hadrien Croubois <hc@iex.ec>
Date: Sat, 16 Jan 2021 00:47:11 +0100
Subject: [PATCH 13/16] Update contracts/math/SafeMath.sol

Co-authored-by: Francisco Giordano <frangio.1@gmail.com>
---
 contracts/math/SafeMath.sol | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/contracts/math/SafeMath.sol b/contracts/math/SafeMath.sol
index d7be6cdf582..1f2e24d0530 100644
--- a/contracts/math/SafeMath.sol
+++ b/contracts/math/SafeMath.sol
@@ -167,6 +167,9 @@ library SafeMath {
      * @dev Returns the integer division of two unsigned integers, reverting with custom message on
      * division by zero. The result is rounded towards zero.
      *
+     * CAUTION: This function is deprecated because it requires allocating memory for the error
+     * message unnecessarily. For custom revert reasons use {tryDiv}.
+     *
      * Counterpart to Solidity's `/` operator. Note: this function uses a
      * `revert` opcode (which leaves remaining gas untouched) while Solidity
      * uses an invalid opcode to revert (consuming all remaining gas).

From d39f5d46ce64b73c36b5dd2c4bfe41560d1be2f3 Mon Sep 17 00:00:00 2001
From: Hadrien Croubois <hc@iex.ec>
Date: Sat, 16 Jan 2021 00:47:32 +0100
Subject: [PATCH 14/16] Update contracts/math/SafeMath.sol

Co-authored-by: Francisco Giordano <frangio.1@gmail.com>
---
 contracts/math/SafeMath.sol | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/contracts/math/SafeMath.sol b/contracts/math/SafeMath.sol
index 1f2e24d0530..9a2220874a0 100644
--- a/contracts/math/SafeMath.sol
+++ b/contracts/math/SafeMath.sol
@@ -187,6 +187,9 @@ library SafeMath {
      * @dev Returns the remainder of dividing two unsigned integers. (unsigned integer modulo),
      * reverting with custom message when dividing by zero.
      *
+     * CAUTION: This function is deprecated because it requires allocating memory for the error
+     * message unnecessarily. For custom revert reasons use {tryMod}.
+     *
      * Counterpart to Solidity's `%` operator. This function uses a `revert`
      * opcode (which leaves remaining gas untouched) while Solidity uses an
      * invalid opcode to revert (consuming all remaining gas).

From 80466e3e2a6d40080956b3d1e6a3a11c47118fc8 Mon Sep 17 00:00:00 2001
From: Hadrien Croubois <hadrien.croubois@gmail.com>
Date: Sat, 16 Jan 2021 01:13:24 +0100
Subject: [PATCH 15/16] notices + addressing comment about testign helper

---
 contracts/math/SafeMath.sol       |  1 -
 contracts/utils/EnumerableMap.sol |  6 ++++++
 test/math/SafeMath.test.js        | 30 ++++++++++++++----------------
 3 files changed, 20 insertions(+), 17 deletions(-)

diff --git a/contracts/math/SafeMath.sol b/contracts/math/SafeMath.sol
index 9a2220874a0..413251e5891 100644
--- a/contracts/math/SafeMath.sol
+++ b/contracts/math/SafeMath.sol
@@ -163,7 +163,6 @@ library SafeMath {
     }
 
     /**
-     * @notice DEPRECATED: causes memory leak
      * @dev Returns the integer division of two unsigned integers, reverting with custom message on
      * division by zero. The result is rounded towards zero.
      *
diff --git a/contracts/utils/EnumerableMap.sol b/contracts/utils/EnumerableMap.sol
index 3eca02d0a7f..57bdabc4005 100644
--- a/contracts/utils/EnumerableMap.sol
+++ b/contracts/utils/EnumerableMap.sol
@@ -168,6 +168,9 @@ library EnumerableMap {
 
     /**
      * @dev Same as {_get}, with a custom error message when `key` is not in the map.
+     *
+     * CAUTION: This function is deprecated because it requires allocating memory for the error
+     * message unnecessarily. For custom revert reasons use {_tryGet}.
      */
     function _get(Map storage map, bytes32 key, string memory errorMessage) private view returns (bytes32) {
         uint256 keyIndex = map._indexes[key];
@@ -251,6 +254,9 @@ library EnumerableMap {
 
     /**
      * @dev Same as {get}, with a custom error message when `key` is not in the map.
+     *
+     * CAUTION: This function is deprecated because it requires allocating memory for the error
+     * message unnecessarily. For custom revert reasons use {tryGet}.
      */
     function get(UintToAddressMap storage map, uint256 key, string memory errorMessage) internal view returns (address) {
         return address(uint160(uint256(_get(map._inner, bytes32(key), errorMessage))));
diff --git a/test/math/SafeMath.test.js b/test/math/SafeMath.test.js
index fab0b544cdc..19feb041f3a 100644
--- a/test/math/SafeMath.test.js
+++ b/test/math/SafeMath.test.js
@@ -5,12 +5,10 @@ const { expect } = require('chai');
 
 const SafeMathMock = artifacts.require('SafeMathMock');
 
-async function deepExpect (value, expected) {
+function expectStruct(value, expected) {
   for (const key in expected) {
     if (BN.isBN(value[key])) {
       expect(value[key]).to.be.bignumber.equal(expected[key]);
-    } else if (Symbol.iterator in Object(value)) {
-      deepExpect(value[key], expected[key]);
     } else {
       expect(value[key]).to.be.equal(expected[key]);
     }
@@ -33,8 +31,8 @@ contract('SafeMath', function (accounts) {
   }
 
   async function testCommutativeIterable (fn, lhs, rhs, expected, ...extra) {
-    deepExpect(await fn(lhs, rhs, ...extra), expected);
-    deepExpect(await fn(rhs, lhs, ...extra), expected);
+    expectStruct(await fn(lhs, rhs, ...extra), expected);
+    expectStruct(await fn(rhs, lhs, ...extra), expected);
   }
 
   describe('with flag', function () {
@@ -59,14 +57,14 @@ contract('SafeMath', function (accounts) {
         const a = new BN('5678');
         const b = new BN('1234');
 
-        deepExpect(await this.safeMath.trySub(a, b), { flag: true, value: a.sub(b) });
+        expectStruct(await this.safeMath.trySub(a, b), { flag: true, value: a.sub(b) });
       });
 
       it('reverts if subtraction result would be negative', async function () {
         const a = new BN('1234');
         const b = new BN('5678');
 
-        deepExpect(await this.safeMath.trySub(a, b), { flag: false, value: '0' });
+        expectStruct(await this.safeMath.trySub(a, b), { flag: false, value: '0' });
       });
     });
 
@@ -98,28 +96,28 @@ contract('SafeMath', function (accounts) {
         const a = new BN('5678');
         const b = new BN('5678');
 
-        deepExpect(await this.safeMath.tryDiv(a, b), { flag: true, value: a.div(b) });
+        expectStruct(await this.safeMath.tryDiv(a, b), { flag: true, value: a.div(b) });
       });
 
       it('divides zero correctly', async function () {
         const a = new BN('0');
         const b = new BN('5678');
 
-        deepExpect(await this.safeMath.tryDiv(a, b), { flag: true, value: a.div(b) });
+        expectStruct(await this.safeMath.tryDiv(a, b), { flag: true, value: a.div(b) });
       });
 
       it('returns complete number result on non-even division', async function () {
         const a = new BN('7000');
         const b = new BN('5678');
 
-        deepExpect(await this.safeMath.tryDiv(a, b), { flag: true, value: a.div(b) });
+        expectStruct(await this.safeMath.tryDiv(a, b), { flag: true, value: a.div(b) });
       });
 
       it('reverts on division by zero', async function () {
         const a = new BN('5678');
         const b = new BN('0');
 
-        deepExpect(await this.safeMath.tryDiv(a, b), { flag: false, value: '0' });
+        expectStruct(await this.safeMath.tryDiv(a, b), { flag: false, value: '0' });
       });
     });
 
@@ -129,28 +127,28 @@ contract('SafeMath', function (accounts) {
           const a = new BN('284');
           const b = new BN('5678');
 
-          deepExpect(await this.safeMath.tryMod(a, b), { flag: true, value: a.mod(b) });
+          expectStruct(await this.safeMath.tryMod(a, b), { flag: true, value: a.mod(b) });
         });
 
         it('when the dividend is equal to the divisor', async function () {
           const a = new BN('5678');
           const b = new BN('5678');
 
-          deepExpect(await this.safeMath.tryMod(a, b), { flag: true, value: a.mod(b) });
+          expectStruct(await this.safeMath.tryMod(a, b), { flag: true, value: a.mod(b) });
         });
 
         it('when the dividend is larger than the divisor', async function () {
           const a = new BN('7000');
           const b = new BN('5678');
 
-          deepExpect(await this.safeMath.tryMod(a, b), { flag: true, value: a.mod(b) });
+          expectStruct(await this.safeMath.tryMod(a, b), { flag: true, value: a.mod(b) });
         });
 
         it('when the dividend is a multiple of the divisor', async function () {
           const a = new BN('17034'); // 17034 == 5678 * 3
           const b = new BN('5678');
 
-          deepExpect(await this.safeMath.tryMod(a, b), { flag: true, value: a.mod(b) });
+          expectStruct(await this.safeMath.tryMod(a, b), { flag: true, value: a.mod(b) });
         });
       });
 
@@ -158,7 +156,7 @@ contract('SafeMath', function (accounts) {
         const a = new BN('5678');
         const b = new BN('0');
 
-        deepExpect(await this.safeMath.tryMod(a, b), { flag: false, value: '0' });
+        expectStruct(await this.safeMath.tryMod(a, b), { flag: false, value: '0' });
       });
     });
   });

From 28b4b53cc26bd4e7efba65af18d38b06e29b22da Mon Sep 17 00:00:00 2001
From: Hadrien Croubois <hadrien.croubois@gmail.com>
Date: Sat, 16 Jan 2021 01:20:59 +0100
Subject: [PATCH 16/16] fix lint

---
 test/math/SafeMath.test.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/math/SafeMath.test.js b/test/math/SafeMath.test.js
index 19feb041f3a..f7e5f91d069 100644
--- a/test/math/SafeMath.test.js
+++ b/test/math/SafeMath.test.js
@@ -5,7 +5,7 @@ const { expect } = require('chai');
 
 const SafeMathMock = artifacts.require('SafeMathMock');
 
-function expectStruct(value, expected) {
+function expectStruct (value, expected) {
   for (const key in expected) {
     if (BN.isBN(value[key])) {
       expect(value[key]).to.be.bignumber.equal(expected[key]);