Merge pull request #1712 from merklejerk/fix/feature/contracts/LibAddressArray-tests

Fix LibAddressArray.indexOf() and add LibAddressArray tests.

contracts/utils/CHANGELOG.json

@@ -13,6 +13,14 @@
             {
                 "note": "Added Address.sol with test for whether or not an address is a contract",
                 "pr": 1657
+            },
+            {
+                "note": "Add unit tests for `LibAddressArray`",
+                "pr": 1712
+            },
+            {
+                "note": "Fix `LibAddressArray.indexOf` returning incorrect index.",
+                "pr": 1712
             }
         ],
         "timestamp": 1553091633

contracts/utils/compiler.json

@@ -30,6 +30,7 @@
         "src/SafeMath.sol",
         "src/interfaces/IOwnable.sol",
         "test/TestConstants.sol",
+        "test/TestLibAddressArray.sol",
         "test/TestLibBytes.sol"
     ]
 }

contracts/utils/contracts/src/LibAddressArray.sol

@@ -30,7 +30,7 @@ library LibAddressArray {
     /// @param addressArray Array of addresses.
     /// @param addressToAppend  Address to append.
     /// @return Array of addresses: [... addressArray, addressToAppend]
-    function append(address[] memory addressArray, address addressToAppend) 
+    function append(address[] memory addressArray, address addressToAppend)
         internal
         pure
         returns (address[] memory)
@@ -148,7 +148,7 @@ library LibAddressArray {
                 if eq(target, arrayElement) {
                     // Set success and index
                     success := 1
-                    index := div(i, 32)
+                    index := div(sub(i, arrayContentsStart), 32)
                     // Break loop
                     i := arrayContentsEnd
                 }

contracts/utils/contracts/test/TestLibAddressArray.sol

@@ -0,0 +1,106 @@
+/*
+
+  Copyright 2018 ZeroEx Intl.
+
+  Licensed under the Apache License, Version 2.0 (the "License");
+  you may not use this file except in compliance with the License.
+  You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+  Unless required by applicable law or agreed to in writing, software
+  distributed under the License is distributed on an "AS IS" BASIS,
+  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+  See the License for the specific language governing permissions and
+  limitations under the License.
+
+*/
+
+pragma solidity ^0.5.5;
+
+import "../src/LibAddressArray.sol";
+
+
+contract TestLibAddressArray {
+
+    using LibAddressArray for address[];
+
+    /// @dev Append a new address to an array of addresses.
+    ///      The `addressArray` may need to be reallocated to make space
+    ///      for the new address. Because of this we return the resulting
+    ///      memory location of `addressArray`.
+    /// @param addressArray Array of addresses.
+    /// @param addressToAppend  Address to append.
+    /// @return Array of addresses: [... addressArray, addressToAppend]
+    function publicAppend(address[] memory addressArray, address addressToAppend)
+        public
+        pure
+        returns (address[] memory)
+    {
+        return addressArray.append(addressToAppend);
+    }
+
+    /// @dev Moves the free memory pointer by `freeMemOffset` bytes,
+    ///      then performs the append.
+    ///      This tests the behavior of the address array being reallocated if
+    ///      the memory immediately after the old array is claimed.
+    /// @param addressArray Array of addresses.
+    /// @param freeMemOffset Number of (signed) bytes to offset the free memory pointer (0x40).
+    /// @param addressToAppend Address to append.
+    /// @return The new address array.
+    /// @return The memory address of the old address array.
+    /// @return The memory address of the new address array.
+    function testAppendRealloc(
+        address[] memory addressArray,
+        int256 freeMemOffset,
+        address addressToAppend
+    )
+        public
+        pure
+        returns (
+            address[] memory result,
+            uint256 oldArrayMemStart,
+            uint256 newArrayMemStart
+        )
+    {
+        assembly {
+            // Remember the original memory address of the array.
+            oldArrayMemStart := addressArray
+            // Move the free memory pointer.
+            mstore(0x40, add(mload(0x40), freeMemOffset))
+        }
+
+        // Call append.
+        result = addressArray.append(addressToAppend);
+
+        // Get the new array memory address.
+        assembly {
+            newArrayMemStart := result
+        }
+    }
+
+    /// @dev Checks if an address array contains the target address.
+    /// @param addressArray Array of addresses.
+    /// @param target Address to search for in array.
+    /// @return True if the addressArray contains the target.
+    function publicContains(address[] memory addressArray, address target)
+        public
+        pure
+        returns (bool success)
+    {
+        return addressArray.contains(target);
+    }
+
+    /// @dev Finds the index of an address within an array.
+    /// @param addressArray Array of addresses.
+    /// @param target Address to search for in array.
+    /// @return Existence and index of the target in the array.
+    function publicIndexOf(address[] memory addressArray, address target)
+        public
+        pure
+        returns (bool success, uint256 index)
+    {
+        (success, index) = addressArray.indexOf(target);
+    }
+
+}

contracts/utils/package.json

@@ -33,7 +33,7 @@
         "lint-contracts": "solhint -c ../.solhint.json contracts/**/**/**/**/*.sol"
     },
     "config": {
-        "abis": "./generated-artifacts/@(IOwnable|LibBytes|Ownable|ReentrancyGuard|SafeMath|TestConstants|TestLibBytes).json",
+        "abis": "./generated-artifacts/@(Address|IOwnable|LibBytes|Ownable|ReentrancyGuard|SafeMath|TestConstants|TestLibAddressArray|TestLibBytes).json",
         "abis:comment": "This list is auto-generated by contracts-gen. Don't edit manually."
     },
     "repository": {

contracts/utils/src/artifacts.ts

@@ -5,14 +5,17 @@
  */
 import { ContractArtifact } from 'ethereum-types';
 
+import * as Address from '../generated-artifacts/Address.json';
 import * as IOwnable from '../generated-artifacts/IOwnable.json';
 import * as LibBytes from '../generated-artifacts/LibBytes.json';
 import * as Ownable from '../generated-artifacts/Ownable.json';
 import * as ReentrancyGuard from '../generated-artifacts/ReentrancyGuard.json';
 import * as SafeMath from '../generated-artifacts/SafeMath.json';
 import * as TestConstants from '../generated-artifacts/TestConstants.json';
+import * as TestLibAddressArray from '../generated-artifacts/TestLibAddressArray.json';
 import * as TestLibBytes from '../generated-artifacts/TestLibBytes.json';
 export const artifacts = {
+    Address: Address as ContractArtifact,
     LibBytes: LibBytes as ContractArtifact,
     Ownable: Ownable as ContractArtifact,
     ReentrancyGuard: ReentrancyGuard as ContractArtifact,
@@ -20,4 +23,5 @@ export const artifacts = {
     IOwnable: IOwnable as ContractArtifact,
     TestConstants: TestConstants as ContractArtifact,
     TestLibBytes: TestLibBytes as ContractArtifact,
+    TestLibAddressArray: TestLibAddressArray as ContractArtifact,
 };

contracts/utils/src/wrappers.ts

@@ -3,10 +3,12 @@
  * Warning: This file is auto-generated by contracts-gen. Don't edit manually.
  * -----------------------------------------------------------------------------
  */
+export * from '../generated-wrappers/address';
 export * from '../generated-wrappers/i_ownable';
 export * from '../generated-wrappers/lib_bytes';
 export * from '../generated-wrappers/ownable';
 export * from '../generated-wrappers/reentrancy_guard';
 export * from '../generated-wrappers/safe_math';
 export * from '../generated-wrappers/test_constants';
+export * from '../generated-wrappers/test_lib_address_array';
 export * from '../generated-wrappers/test_lib_bytes';

contracts/utils/test/lib_address_array.ts

@@ -0,0 +1,161 @@
+import {
+    addressUtils,
+    chaiSetup,
+    expectContractCallFailedAsync,
+    provider,
+    txDefaults,
+    web3Wrapper,
+} from '@0x/contracts-test-utils';
+import { BlockchainLifecycle } from '@0x/dev-utils';
+import { RevertReason } from '@0x/types';
+import { BigNumber } from '@0x/utils';
+import * as chai from 'chai';
+import * as _ from 'lodash';
+
+import { artifacts, TestLibAddressArrayContract } from '../src';
+
+chaiSetup.configure();
+const expect = chai.expect;
+const blockchainLifecycle = new BlockchainLifecycle(web3Wrapper);
+
+describe('LibAddressArray', () => {
+    let lib: TestLibAddressArrayContract;
+
+    before(async () => {
+        await blockchainLifecycle.startAsync();
+    });
+    after(async () => {
+        await blockchainLifecycle.revertAsync();
+    });
+    before(async () => {
+        // Deploy LibAddressArray
+        lib = await TestLibAddressArrayContract.deployFrom0xArtifactAsync(
+            artifacts.TestLibAddressArray,
+            provider,
+            txDefaults,
+        );
+    });
+    beforeEach(async () => {
+        await blockchainLifecycle.startAsync();
+    });
+    afterEach(async () => {
+        await blockchainLifecycle.revertAsync();
+    });
+
+    describe('append', () => {
+        it('should append to empty array', async () => {
+            const addr = addressUtils.generatePseudoRandomAddress();
+            const result = await lib.publicAppend.callAsync([], addr);
+            const expected = [addr];
+            expect(result).to.deep.equal(expected);
+        });
+
+        it('should append to non-empty array', async () => {
+            const arr = _.times(3, () => addressUtils.generatePseudoRandomAddress());
+            const addr = addressUtils.generatePseudoRandomAddress();
+            const expected = [...arr, addr];
+            const result = await lib.publicAppend.callAsync(arr, addr);
+            expect(result).to.deep.equal(expected);
+        });
+
+        it('should revert if the free memory pointer was moved to before the end of the array', async () => {
+            const arr = _.times(3, () => addressUtils.generatePseudoRandomAddress());
+            const addr = addressUtils.generatePseudoRandomAddress();
+            const freeMemOffset = new BigNumber(-1);
+            return expectContractCallFailedAsync(
+                lib.testAppendRealloc.callAsync(arr, freeMemOffset, addr),
+                RevertReason.InvalidFreeMemoryPtr,
+            );
+        });
+
+        it('should keep the same memory address if free memory pointer does not move', async () => {
+            const arr = _.times(3, () => addressUtils.generatePseudoRandomAddress());
+            const addr = addressUtils.generatePseudoRandomAddress();
+            const freeMemOffset = new BigNumber(0);
+            const expected = [...arr, addr];
+            const [result, oldArrayMemStart, newArrayMemStart] = await lib.testAppendRealloc.callAsync(
+                arr,
+                freeMemOffset,
+                addr,
+            );
+            expect(result).to.deep.equal(expected);
+            expect(newArrayMemStart).bignumber.to.be.equal(oldArrayMemStart);
+        });
+
+        it('should change memory address if free memory pointer advances', async () => {
+            const arr = _.times(3, () => addressUtils.generatePseudoRandomAddress());
+            const addr = addressUtils.generatePseudoRandomAddress();
+            const freeMemOffset = new BigNumber(1);
+            const expectedArray = [...arr, addr];
+            const [result, oldArrayMemStart, newArrayMemStart] = await lib.testAppendRealloc.callAsync(
+                arr,
+                freeMemOffset,
+                addr,
+            );
+            // The new location should be the end of the old array + freeMemOffset.
+            const expectedNewArrayMemStart = oldArrayMemStart.plus((arr.length + 1) * 32).plus(freeMemOffset);
+            expect(result).to.deep.equal(expectedArray);
+            expect(newArrayMemStart).bignumber.to.be.equal(expectedNewArrayMemStart);
+        });
+    });
+
+    describe('contains', () => {
+        it('should return false on an empty array', async () => {
+            const addr = addressUtils.generatePseudoRandomAddress();
+            const isFound = await lib.publicContains.callAsync([], addr);
+            expect(isFound).to.equal(false);
+        });
+
+        it('should return false on a missing item', async () => {
+            const arr = _.times(3, () => addressUtils.generatePseudoRandomAddress());
+            const addr = addressUtils.generatePseudoRandomAddress();
+            const isFound = await lib.publicContains.callAsync(arr, addr);
+            expect(isFound).to.equal(false);
+        });
+
+        it('should return true on an included item', async () => {
+            const arr = _.times(4, () => addressUtils.generatePseudoRandomAddress());
+            const addr = _.sample(arr) as string;
+            const isFound = await lib.publicContains.callAsync(arr, addr);
+            expect(isFound).to.equal(true);
+        });
+
+        it('should return true on the only item in the array', async () => {
+            const arr = _.times(1, () => addressUtils.generatePseudoRandomAddress());
+            const isFound = await lib.publicContains.callAsync(arr, arr[0]);
+            expect(isFound).to.equal(true);
+        });
+    });
+
+    describe('indexOf', () => {
+        it('should fail on an empty array', async () => {
+            const addr = addressUtils.generatePseudoRandomAddress();
+            const [isSuccess] = await lib.publicIndexOf.callAsync([], addr);
+            expect(isSuccess).to.equal(false);
+        });
+
+        it('should fail on a missing item', async () => {
+            const arr = _.times(3, () => addressUtils.generatePseudoRandomAddress());
+            const addr = addressUtils.generatePseudoRandomAddress();
+            const [isSuccess] = await lib.publicIndexOf.callAsync(arr, addr);
+            expect(isSuccess).to.equal(false);
+        });
+
+        it('should succeed on an included item', async () => {
+            const arr = _.times(4, () => addressUtils.generatePseudoRandomAddress());
+            const expectedIndexOf = _.random(0, arr.length - 1);
+            const addr = arr[expectedIndexOf];
+            const [isSuccess, index] = await lib.publicIndexOf.callAsync(arr, addr);
+            expect(isSuccess).to.equal(true);
+            expect(index).bignumber.to.equal(expectedIndexOf);
+        });
+
+        it('should succeed on the only item in the array', async () => {
+            const arr = _.times(1, () => addressUtils.generatePseudoRandomAddress());
+            const [isSuccess, index] = await lib.publicIndexOf.callAsync(arr, arr[0]);
+            expect(isSuccess).to.equal(true);
+            expect(index).bignumber.to.equal(0);
+        });
+    });
+});
+// tslint:disable:max-file-line-count

contracts/utils/tsconfig.json

@@ -3,12 +3,14 @@
     "compilerOptions": { "outDir": "lib", "rootDir": ".", "resolveJsonModule": true },
     "include": ["./src/**/*", "./test/**/*", "./generated-wrappers/**/*"],
     "files": [
+        "generated-artifacts/Address.json",
         "generated-artifacts/IOwnable.json",
         "generated-artifacts/LibBytes.json",
         "generated-artifacts/Ownable.json",
         "generated-artifacts/ReentrancyGuard.json",
         "generated-artifacts/SafeMath.json",
         "generated-artifacts/TestConstants.json",
+        "generated-artifacts/TestLibAddressArray.json",
         "generated-artifacts/TestLibBytes.json"
     ],
     "exclude": ["./deploy/solc/solc_bin"]

packages/types/CHANGELOG.json

@@ -17,6 +17,10 @@
             {
                 "note": "Add `RevertReason.SignatureInvalid` thrown by Coordinator",
                 "pr": 1705
+            },
+            {
+                "note": "Add `RevertReason.InvalidFreeMemoryPtr` thrown by LibAddressArray",
+                "pr": 1712
             }
         ],
         "timestamp": 1553091633

packages/types/src/index.ts

@@ -283,6 +283,8 @@ export enum RevertReason {
     TxFullyConfirmed = 'TX_FULLY_CONFIRMED',
     TxNotFullyConfirmed = 'TX_NOT_FULLY_CONFIRMED',
     TimeLockIncomplete = 'TIME_LOCK_INCOMPLETE',
+    // LibAddressArray
+    InvalidFreeMemoryPtr = 'INVALID_FREE_MEMORY_PTR',
     // DutchAuction
     AuctionInvalidAmount = 'INVALID_AMOUNT',
     AuctionExpired = 'AUCTION_EXPIRED',