From b8925baa885390f5c292e9240528a00770f65d9d Mon Sep 17 00:00:00 2001 From: Lawrence Date: Tue, 19 Mar 2019 00:22:55 -0400 Subject: [PATCH] Add unit tests for contracts/utils/LibAddressArray. Fix `LibAddressArray.indexOf` returning wrong index. --- contracts/utils/CHANGELOG.json | 6 + contracts/utils/compiler.json | 1 + .../utils/contracts/src/LibAddressArray.sol | 4 +- .../contracts/test/TestLibAddressArray.sol | 114 +++++++++++++ contracts/utils/package.json | 2 +- contracts/utils/src/artifacts.ts | 4 + contracts/utils/src/wrappers.ts | 2 + contracts/utils/test/lib_address_array.ts | 156 ++++++++++++++++++ contracts/utils/tsconfig.json | 2 + 9 files changed, 288 insertions(+), 3 deletions(-) create mode 100644 contracts/utils/contracts/test/TestLibAddressArray.sol create mode 100644 contracts/utils/test/lib_address_array.ts diff --git a/contracts/utils/CHANGELOG.json b/contracts/utils/CHANGELOG.json index ecec7db642..9f08c82897 100644 --- a/contracts/utils/CHANGELOG.json +++ b/contracts/utils/CHANGELOG.json @@ -13,6 +13,12 @@ { "note": "Added Address.sol with test for whether or not an address is a contract", "pr": 1657 + }, + { + "note": "Add unit tests for `LibAddressArray`" + }, + { + "note": "Fix `LibAddressArray.indexOf` returning incorrect index." } ] }, diff --git a/contracts/utils/compiler.json b/contracts/utils/compiler.json index 4632bf57b7..8cab311419 100644 --- a/contracts/utils/compiler.json +++ b/contracts/utils/compiler.json @@ -30,6 +30,7 @@ "src/SafeMath.sol", "src/interfaces/IOwnable.sol", "test/TestConstants.sol", + "test/TestLibAddressArray.sol", "test/TestLibBytes.sol" ] } diff --git a/contracts/utils/contracts/src/LibAddressArray.sol b/contracts/utils/contracts/src/LibAddressArray.sol index 0d4ebb544e..259dc558aa 100644 --- a/contracts/utils/contracts/src/LibAddressArray.sol +++ b/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 } diff --git a/contracts/utils/contracts/test/TestLibAddressArray.sol b/contracts/utils/contracts/test/TestLibAddressArray.sol new file mode 100644 index 0000000000..3783442ac3 --- /dev/null +++ b/contracts/utils/contracts/test/TestLibAddressArray.sol @@ -0,0 +1,114 @@ +/* + + 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 Creates an in-memory copy of `addressArray`, + /// 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 + ) + { + // Create a copy of the array. + result = new address[](addressArray.length); + assembly { + oldArrayMemStart := result + let length := mload(addressArray) + for { let i := 0 } lt(i, length) { i := add(i, 1) } { + mstore(add(result, mul(add(i, 1), 32)), + mload(add(addressArray, mul(add(i, 1), 32)))) + } + + // Move the free memory pointer. + mstore(0x40, add(mload(0x40), freeMemOffset)) + } + + // Call append. + result = result.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); + } + +} diff --git a/contracts/utils/package.json b/contracts/utils/package.json index 3d83834965..5f5098cf0c 100644 --- a/contracts/utils/package.json +++ b/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": { diff --git a/contracts/utils/src/artifacts.ts b/contracts/utils/src/artifacts.ts index 15a1abedfc..04ebb5b525 100644 --- a/contracts/utils/src/artifacts.ts +++ b/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, }; diff --git a/contracts/utils/src/wrappers.ts b/contracts/utils/src/wrappers.ts index 8748cb074d..91a90bec6c 100644 --- a/contracts/utils/src/wrappers.ts +++ b/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'; diff --git a/contracts/utils/test/lib_address_array.ts b/contracts/utils/test/lib_address_array.ts new file mode 100644 index 0000000000..a7d462f374 --- /dev/null +++ b/contracts/utils/test/lib_address_array.ts @@ -0,0 +1,156 @@ +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(); + return expectContractCallFailedAsync( + lib.testAppendRealloc.callAsync(arr, new BigNumber(-1), 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 expected = [...arr, addr]; + const [result, oldArrayMemStart, newArrayMemStart] = await lib.testAppendRealloc.callAsync( + arr, + new BigNumber(0), + 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 expected = [...arr, addr]; + const [result, oldArrayMemStart, newArrayMemStart] = await lib.testAppendRealloc.callAsync( + arr, + new BigNumber(1), + addr, + ); + expect(result).to.deep.equal(expected); + expect(newArrayMemStart).bignumber.to.not.be.equal(oldArrayMemStart); + }); + }); + + 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 diff --git a/contracts/utils/tsconfig.json b/contracts/utils/tsconfig.json index efd71ff937..dce54b101e 100644 --- a/contracts/utils/tsconfig.json +++ b/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"]