Merge pull request #162 from 1inch/feature/increase-coverage

[SC-1321] Increase coverage

contracts/mixins/BySig.sol

@@ -205,16 +205,13 @@ abstract contract BySig is Context, EIP712 {
         uint256 nonce = traits.nonce();
         if (nonceType == BySigTraits.NonceType.Account) {
             return nonce == _bySigAccountNonces[signer]++;
-        }
-        if (nonceType == BySigTraits.NonceType.Selector) {
+        } else if (nonceType == BySigTraits.NonceType.Selector) {
             return nonce == _bySigSelectorNonces[signer][bytes4(data)]++;
-        }
-        if (nonceType == BySigTraits.NonceType.Unique) {
+        } else { // nonceType == BySigTraits.NonceType.Unique
             mapping(uint256 => uint256) storage map = _bySigUniqueNonces[signer];
             uint256 cache = map[nonce >> 8];
             map[nonce >> 8] |= 1 << (nonce & 0xff);
             return cache != map[nonce >> 8];
         }
-        return false;
     }
 }

contracts/tests/mocks/AddressArrayMock.sol

@@ -27,6 +27,10 @@ contract AddressArrayMock {
         return AddressArray.get(_self);
     }
 
+    function getAndProvideArr(address[] memory providedArr) external view returns (address[] memory, address[] memory) {
+        return (AddressArray.get(_self, providedArr), providedArr);
+    }
+
     function push(address account) external returns (uint256) {
         return AddressArray.push(_self, account);
     }
@@ -38,4 +42,8 @@ contract AddressArrayMock {
     function set(uint256 index, address account) external {
         AddressArray.set(_self, index, account);
     }
+
+    function erase() external {
+        AddressArray.erase(_self);
+    }
 }

contracts/tests/mocks/AddressSetMock.sol

@@ -29,11 +29,19 @@ contract AddressSetMock {
         return AddressSet.get(_self);
     }
 
+    function getAndProvideSet(address[] memory providedArr) external view returns (address[] memory, address[] memory) {
+        return (AddressSet.get(_self, providedArr), providedArr);
+    }
+
     function add(address item) external returns (bool) {
         return AddressSet.add(_self, item);
     }
 
     function remove(address item) external returns (bool) {
         return AddressSet.remove(_self, item);
     }
+
+    function erase() external {
+        AddressSet.erase(_self);
+    }
 }

contracts/tests/mocks/RevertReasonForwarderMock.sol

@@ -0,0 +1,43 @@
+// SPDX-License-Identifier: MIT
+// solhint-disable one-contract-per-file
+
+pragma solidity ^0.8.0;
+
+import "../../libraries/RevertReasonForwarder.sol";
+
+contract RevertReasonForwarderHelper {
+    error RevertReason();
+
+    function revertWithReason() external pure {
+        revert RevertReason();
+    }
+}
+
+contract RevertReasonForwarderMock {
+    address payable private _helper;
+
+    constructor(address helper) {
+        _helper = payable(helper);
+    }
+
+    function reRevert() external payable {
+        // solhint-disable-next-line avoid-low-level-calls
+        (bool success, ) = _helper.call{value: msg.value}(
+            abi.encodeWithSignature("revertWithReason()")
+        );
+        if (!success) {
+            RevertReasonForwarder.reRevert();
+        }
+    }
+
+    function reReason() external payable returns (bytes memory) {
+        // solhint-disable-next-line avoid-low-level-calls
+        (bool success, ) = _helper.call{value: msg.value}(
+            abi.encodeWithSignature("revertWithReason()")
+        );
+        if (!success) {
+            return RevertReasonForwarder.reReason();
+        }
+        return "";
+    }
+}

contracts/tests/mocks/SafeERC20Helper.sol

@@ -128,9 +128,23 @@ contract ERC20ThroughZeroApprove {
     error NonZeroToNonZeroApprove();
 
     mapping(address => mapping(address => uint256)) private _allowances;
+    bool private _failAfterZeroReset;
+    bool private _failFlag; // shows that last approve was failed and test should fail
+
+    function setFailAfterZeroReset(bool fail) external {
+        _failAfterZeroReset = fail;
+    }
+
+    function setFailFlag(bool fail) external {
+        _failAfterZeroReset = fail;
+    }
 
     function approve(address to, uint256 amount) external {
         if (_allowances[msg.sender][to] != 0 && amount != 0) revert NonZeroToNonZeroApprove();
+        if (_failFlag) revert NonZeroToNonZeroApprove();
+        if (_failAfterZeroReset && amount == 0) {
+            _failFlag = true;
+        }
         _allowances[msg.sender][to] = amount;
     }
 

src/bySig.ts

@@ -10,6 +10,7 @@ export enum NonceType {
     Account,    // Nonce for account
     Selector,   // Nonce for selector
     Unique,     // Nonce for unique
+    Invalid,    // Invalid Type
 }
 
 /**

test/contracts/AddressArray.test.ts

@@ -115,6 +115,24 @@ describe('AddressArray', function () {
             await signer1.sendTransaction(await addressArrayMock.get.populateTransaction());
             expect(await addressArrayMock.get()).to.be.deep.equal([signer1.address, signer2.address, signer3.address]);
         });
+
+        it('should get array with 2 elements and copies the addresses into the provided input array', async function () {
+            const { addressArrayMock } = await loadFixture(deployAddressArrayMock);
+            await addressArrayMock.push(signer1);
+            await addressArrayMock.push(signer2);
+            await signer1.sendTransaction(await addressArrayMock.getAndProvideArr.populateTransaction([constants.ZERO_ADDRESS, constants.ZERO_ADDRESS]));
+            expect(await addressArrayMock.getAndProvideArr([constants.ZERO_ADDRESS, constants.ZERO_ADDRESS])).to.be.deep.equal([
+                [signer1.address, signer2.address],
+                [signer1.address, signer2.address],
+            ]);
+        });
+
+        it('should reverted because provided input array size is too small', async function () {
+            const { addressArrayMock } = await loadFixture(deployAddressArrayMock);
+            await addressArrayMock.push(signer1);
+            await addressArrayMock.push(signer2);
+            await expect(addressArrayMock.getAndProvideArr([])).to.be.revertedWithCustomError(await ethers.getContractFactory('AddressArray'), 'OutputArrayTooSmall');
+        });
     });
 
     describe('push', function () {
@@ -233,4 +251,38 @@ describe('AddressArray', function () {
             await addressArrayMock.pop();
         });
     });
+
+    describe('erase', function () {
+        it('should not change empty array', async function () {
+            const { addressArrayMock } = await loadFixture(deployAddressArrayMock);
+            const arrayBefore = await addressArrayMock.get();
+            await addressArrayMock.erase();
+            expect(await addressArrayMock.get()).to.be.deep.equal(arrayBefore);
+        });
+
+        it('should reset non-zero array length', async function () {
+            const { addressArrayMock } = await loadFixture(deployAddressArrayMock);
+            await addressArrayMock.push(signer1);
+            expect(await addressArrayMock.length()).to.be.not.equal('0');
+            await addressArrayMock.erase();
+            expect(await addressArrayMock.length()).to.be.deep.equal('0');
+        });
+
+        it('should reset non-zero array', async function () {
+            const { addressArrayMock } = await loadFixture(deployAddressArrayMock);
+            await addressArrayMock.push(signer1);
+            expect(await addressArrayMock.get()).to.be.not.deep.equal([]);
+            await addressArrayMock.erase();
+            expect(await addressArrayMock.get()).to.be.deep.equal([]);
+        });
+
+        it('should not return item from array after reset', async function () {
+            const { addressArrayMock } = await loadFixture(deployAddressArrayMock);
+            await addressArrayMock.push(signer1);
+            await addressArrayMock.push(signer2);
+            expect(await addressArrayMock.get()).to.be.deep.equal([signer1.address, signer2.address]);
+            await addressArrayMock.erase();
+            await expect(addressArrayMock.at(1)).to.be.revertedWithCustomError(await ethers.getContractFactory('AddressArray'), 'IndexOutOfBounds');
+        });
+    });
 });

test/contracts/AddressSet.test.ts

@@ -86,6 +86,51 @@ describe('AddressSet', function () {
         });
     });
 
+    describe('get', function () {
+        it('should get empty array', async function () {
+            const { addressSetMock } = await loadFixture(deployAddressSetMock);
+            expect(await addressSetMock.get()).to.be.deep.equal([]);
+        });
+
+        it('should get array with 1 element', async function () {
+            const { addressSetMock } = await loadFixture(deployAddressSetMock);
+            await addressSetMock.add(signer1);
+            expect(await addressSetMock.get()).to.be.deep.equal([signer1.address]);
+        });
+
+        it('should get array with 2 elements', async function () {
+            const { addressSetMock } = await loadFixture(deployAddressSetMock);
+            await addressSetMock.add(signer1);
+            await addressSetMock.add(signer2);
+            expect(await addressSetMock.get()).to.be.deep.equal([signer1.address, signer2.address]);
+        });
+
+        it('should get from array with 3 elements', async function () {
+            const { addressSetMock } = await loadFixture(deployAddressSetMock);
+            await addressSetMock.add(signer1);
+            await addressSetMock.add(signer2);
+            await addressSetMock.add(signer3);
+            expect(await addressSetMock.get()).to.be.deep.equal([signer1.address, signer2.address, signer3.address]);
+        });
+
+        it('should get array with 2 elements and copies the addresses into the provided input array', async function () {
+            const { addressSetMock } = await loadFixture(deployAddressSetMock);
+            await addressSetMock.add(signer1);
+            await addressSetMock.add(signer2);
+            expect(await addressSetMock.getAndProvideSet([constants.ZERO_ADDRESS, constants.ZERO_ADDRESS])).to.be.deep.equal([
+                [signer1.address, signer2.address],
+                [signer1.address, signer2.address],
+            ]);
+        });
+
+        it('should reverted because provided input array size is too small', async function () {
+            const { addressSetMock } = await loadFixture(deployAddressSetMock);
+            await addressSetMock.add(signer1);
+            await addressSetMock.add(signer2);
+            await expect(addressSetMock.getAndProvideSet([])).to.be.revertedWithCustomError(await ethers.getContractFactory('AddressArray'), 'OutputArrayTooSmall');
+        });
+    });
+
     describe('contains', function () {
         it('should not contain in empty set', async function () {
             const { addressSetMock } = await loadFixture(deployAddressSetMock);
@@ -190,4 +235,38 @@ describe('AddressSet', function () {
             await addressSetMock.remove(signer1);
         });
     });
+
+    describe('erase', function () {
+        it('should not change empty array', async function () {
+            const { addressSetMock } = await loadFixture(deployAddressSetMock);
+            const arrayBefore = await addressSetMock.get();
+            await addressSetMock.erase();
+            expect(await addressSetMock.get()).to.be.deep.equal(arrayBefore);
+        });
+
+        it('should reset non-zero array length', async function () {
+            const { addressSetMock } = await loadFixture(deployAddressSetMock);
+            await addressSetMock.add(signer1);
+            expect(await addressSetMock.length()).to.be.not.equal('0');
+            await addressSetMock.erase();
+            expect(await addressSetMock.length()).to.be.deep.equal('0');
+        });
+
+        it('should reset non-zero array', async function () {
+            const { addressSetMock } = await loadFixture(deployAddressSetMock);
+            await addressSetMock.add(signer1);
+            expect(await addressSetMock.get()).to.be.not.deep.equal([]);
+            await addressSetMock.erase();
+            expect(await addressSetMock.get()).to.be.deep.equal([]);
+        });
+
+        it('should not return item from array after reset', async function () {
+            const { addressSetMock } = await loadFixture(deployAddressSetMock);
+            await addressSetMock.add(signer1);
+            await addressSetMock.add(signer2);
+            expect(await addressSetMock.get()).to.be.deep.equal([signer1.address, signer2.address]);
+            await addressSetMock.erase();
+            await expect(addressSetMock.at(1)).to.be.revertedWithCustomError(await ethers.getContractFactory('AddressArray'), 'IndexOutOfBounds');
+        });
+    });
 });

test/contracts/BySig.test.ts

@@ -78,6 +78,15 @@ describe('BySig', function () {
     });
 
     describe('bySig', function () {
+        it('should return false for an invalid nonce type', async function () {
+            const { addrs: { alice }, token } = await loadFixture(deployAddressArrayMock);
+            const sig = {
+                traits: buildBySigTraits({ deadline: 0xffffffffff, nonceType: NonceType.Invalid, nonce: 0 }),
+                data: '0x',
+            };
+            await expect(token.bySig(alice, sig, '0x')).to.be.revertedWithCustomError(token, 'WrongNonceType');
+        });
+
         it('should revert after traits deadline', async function () {
             const { addrs: { alice }, token } = await loadFixture(deployAddressArrayMock);
             const sig = {

test/contracts/RevertReasonForwarder.test.ts

@@ -0,0 +1,30 @@
+import { expect } from '../../src/expect';
+import { loadFixture } from '@nomicfoundation/hardhat-network-helpers';
+import { ethers } from 'hardhat';
+import {
+    RevertReasonForwarderHelper__factory as RevertReasonForwarderHelper,
+    RevertReasonForwarderMock__factory as RevertReasonForwarderMock,
+} from '../../typechain-types';
+
+describe('RevertReasonForwarder', function () {
+    let Helper: RevertReasonForwarderHelper;
+    let RevertReasonForwarderMock: RevertReasonForwarderMock;
+
+    async function deployRevertReasonForwarderMock() {
+        RevertReasonForwarderMock = await ethers.getContractFactory('RevertReasonForwarderMock');
+        Helper = await ethers.getContractFactory('RevertReasonForwarderHelper');
+        const helper = await Helper.deploy();
+        const mock = await RevertReasonForwarderMock.deploy(helper);
+        return { helper, mock };
+    }
+
+    it('should forward custom error from the helper contract using reRevert', async function () {
+        const { helper, mock } = await loadFixture(deployRevertReasonForwarderMock);
+        await expect(mock.reRevert()).to.be.revertedWithCustomError(helper, 'RevertReason');
+    });
+
+    it('should return custom error from the helper contract using reReason', async function () {
+        const { mock } = await loadFixture(deployRevertReasonForwarderMock);
+        expect(await mock.reReason.staticCall()).to.be.equal(ethers.id('RevertReason()').substring(0, 10));
+    });
+});

test/contracts/SafestERC20.test.ts

@@ -78,7 +78,7 @@ describe('SafeERC20', function () {
         await zeroApprove.waitForDeployment();
         const wrapper = await SafeERC20Wrapper.deploy(zeroApprove);
         await wrapper.waitForDeployment();
-        return { wrapper };
+        return { zeroApprove, wrapper };
     }
 
     async function deployPermitNoRevertAndSign() {
@@ -178,6 +178,13 @@ describe('SafeERC20', function () {
             await wrapper.approve(100);
         });
 
+        it('non-zero to non-zero approval should revert if approve failed', async function () {
+            const { zeroApprove, wrapper } = await loadFixture(deployWrapperZeroApprove);
+            await wrapper.approve(100);
+            await zeroApprove.setFailAfterZeroReset(true);
+            await expect(wrapper.approve(100)).to.be.revertedWithCustomError(wrapper, 'ForceApproveFailed');
+        });
+
         it('non-zero to zero to non-zero approval should pass', async function () {
             const { wrapper } = await loadFixture(deployWrapperZeroApprove);
             await wrapper.approve(100);
@@ -187,6 +194,17 @@ describe('SafeERC20', function () {
     });
 
     describe('safeBalanceOf', function () {
+        it('should return zero balance', async function () {
+            const { wrapper } = await loadFixture(deployERC20WithSafeBalance);
+            expect(await wrapper.safeBalanceOf(owner)).to.be.equal(0);
+        });
+
+        it('should return balance', async function () {
+            const { weth, wrapper } = await loadFixture(deployERC20WithSafeBalance);
+            await weth.deposit({ value: 123456 });
+            expect(await wrapper.safeBalanceOf(owner)).to.be.equal(123456);
+        });
+
         it('should be cheaper than balanceOf', async function () {
             if (hre.__SOLIDITY_COVERAGE_RUNNING === undefined) {
                 const { wrapper } = await loadFixture(deployERC20WithSafeBalance);