From 08ee1ab2e636099344cade1c8bff67e9b3da2bb4 Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Tue, 26 Jun 2018 12:23:00 -0700 Subject: Remove redundant files, hard code function selector in dispatchTransferFrom, and modify revert reason --- packages/contracts/test/asset_proxy/decoder.ts | 84 ------------------- packages/contracts/test/asset_proxy/proxies.ts | 108 ------------------------- 2 files changed, 192 deletions(-) delete mode 100644 packages/contracts/test/asset_proxy/decoder.ts (limited to 'packages/contracts/test/asset_proxy') diff --git a/packages/contracts/test/asset_proxy/decoder.ts b/packages/contracts/test/asset_proxy/decoder.ts deleted file mode 100644 index 902255caf..000000000 --- a/packages/contracts/test/asset_proxy/decoder.ts +++ /dev/null @@ -1,84 +0,0 @@ -import { BlockchainLifecycle } from '@0xproject/dev-utils'; -import { assetProxyUtils, generatePseudoRandomSalt } from '@0xproject/order-utils'; -import { BigNumber } from '@0xproject/utils'; -import * as chai from 'chai'; -import ethUtil = require('ethereumjs-util'); - -import { TestAssetDataDecodersContract } from '../../src/generated_contract_wrappers/test_asset_data_decoders'; -import { artifacts } from '../../src/utils/artifacts'; -import { chaiSetup } from '../../src/utils/chai_setup'; -import { provider, txDefaults, web3Wrapper } from '../../src/utils/web3_wrapper'; - -chaiSetup.configure(); -const expect = chai.expect; -const blockchainLifecycle = new BlockchainLifecycle(web3Wrapper); - -describe('TestAssetDataDecoders', () => { - let testAssetProxyDecoder: TestAssetDataDecodersContract; - let testAddress: string; - - before(async () => { - await blockchainLifecycle.startAsync(); - }); - after(async () => { - await blockchainLifecycle.revertAsync(); - }); - before(async () => { - // Setup accounts & addresses - const accounts = await web3Wrapper.getAvailableAddressesAsync(); - testAddress = accounts[0]; - testAssetProxyDecoder = await TestAssetDataDecodersContract.deployFrom0xArtifactAsync( - artifacts.TestAssetDataDecoders, - provider, - txDefaults, - ); - }); - beforeEach(async () => { - await blockchainLifecycle.startAsync(); - }); - afterEach(async () => { - await blockchainLifecycle.revertAsync(); - }); - - describe('Asset Data Decoders', () => { - it('should correctly decode ERC721 asset data', async () => { - const tokenId = generatePseudoRandomSalt(); - const encodedAssetData = assetProxyUtils.encodeERC721AssetData(testAddress, tokenId); - const expectedDecodedAssetData = assetProxyUtils.decodeERC721AssetData(encodedAssetData); - let decodedTokenAddress: string; - let decodedTokenId: BigNumber; - let decodedData: string; - [ - decodedTokenAddress, - decodedTokenId, - decodedData, - ] = await testAssetProxyDecoder.publicDecodeERC721Data.callAsync(encodedAssetData); - expect(decodedTokenAddress).to.be.equal(expectedDecodedAssetData.tokenAddress); - expect(decodedTokenId).to.be.bignumber.equal(expectedDecodedAssetData.tokenId); - expect(decodedData).to.be.equal(expectedDecodedAssetData.receiverData); - }); - - it('should correctly decode ERC721 asset data with receiver data', async () => { - const tokenId = generatePseudoRandomSalt(); - const receiverDataFirst32Bytes = ethUtil.bufferToHex( - assetProxyUtils.encodeUint256(generatePseudoRandomSalt()), - ); - const receiverDataExtraBytes = 'FFFF'; - // We add extra bytes to generate a value that doesn't fit perfectly into one word - const receiverData = receiverDataFirst32Bytes + receiverDataExtraBytes; - const encodedAssetData = assetProxyUtils.encodeERC721AssetData(testAddress, tokenId, receiverData); - const expectedDecodedAssetData = assetProxyUtils.decodeERC721AssetData(encodedAssetData); - let decodedTokenAddress: string; - let decodedTokenId: BigNumber; - let decodedReceiverData: string; - [ - decodedTokenAddress, - decodedTokenId, - decodedReceiverData, - ] = await testAssetProxyDecoder.publicDecodeERC721Data.callAsync(encodedAssetData); - expect(decodedTokenAddress).to.be.equal(expectedDecodedAssetData.tokenAddress); - expect(decodedTokenId).to.be.bignumber.equal(expectedDecodedAssetData.tokenId); - expect(decodedReceiverData).to.be.equal(expectedDecodedAssetData.receiverData); - }); - }); -}); diff --git a/packages/contracts/test/asset_proxy/proxies.ts b/packages/contracts/test/asset_proxy/proxies.ts index 7960e46d2..5a5289508 100644 --- a/packages/contracts/test/asset_proxy/proxies.ts +++ b/packages/contracts/test/asset_proxy/proxies.ts @@ -5,7 +5,6 @@ import { BigNumber } from '@0xproject/utils'; import * as chai from 'chai'; import { LogWithDecodedArgs } from 'ethereum-types'; import ethUtil = require('ethereumjs-util'); -import * as _ from 'lodash'; import { DummyERC20TokenContract } from '../../src/generated_contract_wrappers/dummy_e_r_c20_token'; import { @@ -189,58 +188,6 @@ describe('Asset Transfer Proxies', () => { }); }); - describe('batchTransferFrom', () => { - it('should succesfully make multiple token transfers', async () => { - const erc20Balances = await erc20Wrapper.getBalancesAsync(); - - const encodedAssetData = assetProxyUtils.encodeERC20AssetData(zrxToken.address); - const amount = new BigNumber(10); - const numTransfers = 2; - const assetData = _.times(numTransfers, () => encodedAssetData); - const fromAddresses = _.times(numTransfers, () => makerAddress); - const toAddresses = _.times(numTransfers, () => takerAddress); - const amounts = _.times(numTransfers, () => amount); - - const txHash = await erc20Proxy.batchTransferFrom.sendTransactionAsync( - assetData, - fromAddresses, - toAddresses, - amounts, - { from: exchangeAddress }, - ); - const res = await web3Wrapper.awaitTransactionSuccessAsync( - txHash, - constants.AWAIT_TRANSACTION_MINED_MS, - ); - const newBalances = await erc20Wrapper.getBalancesAsync(); - - expect(res.logs.length).to.equal(numTransfers); - expect(newBalances[makerAddress][zrxToken.address]).to.be.bignumber.equal( - erc20Balances[makerAddress][zrxToken.address].minus(amount.times(numTransfers)), - ); - expect(newBalances[takerAddress][zrxToken.address]).to.be.bignumber.equal( - erc20Balances[takerAddress][zrxToken.address].add(amount.times(numTransfers)), - ); - }); - - it('should throw if not called by an authorized address', async () => { - const encodedAssetData = assetProxyUtils.encodeERC20AssetData(zrxToken.address); - const amount = new BigNumber(10); - const numTransfers = 2; - const assetData = _.times(numTransfers, () => encodedAssetData); - const fromAddresses = _.times(numTransfers, () => makerAddress); - const toAddresses = _.times(numTransfers, () => takerAddress); - const amounts = _.times(numTransfers, () => amount); - - return expectRevertReasonOrAlwaysFailingTransactionAsync( - erc20Proxy.batchTransferFrom.sendTransactionAsync(assetData, fromAddresses, toAddresses, amounts, { - from: notAuthorized, - }), - RevertReason.SenderNotAuthorized, - ); - }); - }); - it('should have an id of 0xf47261b0', async () => { const proxyId = await erc20Proxy.getProxyId.callAsync(); const expectedProxyId = '0xf47261b0'; @@ -440,61 +387,6 @@ describe('Asset Transfer Proxies', () => { }); }); - describe('batchTransferFrom', () => { - it('should succesfully make multiple token transfers', async () => { - const erc721TokensById = await erc721Wrapper.getBalancesAsync(); - const [makerTokenIdA, makerTokenIdB] = erc721TokensById[makerAddress][erc721Token.address]; - - const numTransfers = 2; - const assetData = [ - assetProxyUtils.encodeERC721AssetData(erc721Token.address, makerTokenIdA), - assetProxyUtils.encodeERC721AssetData(erc721Token.address, makerTokenIdB), - ]; - const fromAddresses = _.times(numTransfers, () => makerAddress); - const toAddresses = _.times(numTransfers, () => takerAddress); - const amounts = _.times(numTransfers, () => new BigNumber(1)); - - const txHash = await erc721Proxy.batchTransferFrom.sendTransactionAsync( - assetData, - fromAddresses, - toAddresses, - amounts, - { from: exchangeAddress }, - ); - const res = await web3Wrapper.awaitTransactionSuccessAsync( - txHash, - constants.AWAIT_TRANSACTION_MINED_MS, - ); - expect(res.logs.length).to.equal(numTransfers); - - const newOwnerMakerAssetA = await erc721Token.ownerOf.callAsync(makerTokenIdA); - const newOwnerMakerAssetB = await erc721Token.ownerOf.callAsync(makerTokenIdB); - expect(newOwnerMakerAssetA).to.be.bignumber.equal(takerAddress); - expect(newOwnerMakerAssetB).to.be.bignumber.equal(takerAddress); - }); - - it('should throw if not called by an authorized address', async () => { - const erc721TokensById = await erc721Wrapper.getBalancesAsync(); - const [makerTokenIdA, makerTokenIdB] = erc721TokensById[makerAddress][erc721Token.address]; - - const numTransfers = 2; - const assetData = [ - assetProxyUtils.encodeERC721AssetData(erc721Token.address, makerTokenIdA).slice(0, -2), - assetProxyUtils.encodeERC721AssetData(erc721Token.address, makerTokenIdB).slice(0, -2), - ]; - const fromAddresses = _.times(numTransfers, () => makerAddress); - const toAddresses = _.times(numTransfers, () => takerAddress); - const amounts = _.times(numTransfers, () => new BigNumber(1)); - - return expectRevertReasonOrAlwaysFailingTransactionAsync( - erc721Proxy.batchTransferFrom.sendTransactionAsync(assetData, fromAddresses, toAddresses, amounts, { - from: notAuthorized, - }), - RevertReason.SenderNotAuthorized, - ); - }); - }); - it('should have an id of 0x08e937fa', async () => { const proxyId = await erc721Proxy.getProxyId.callAsync(); const expectedProxyId = '0x08e937fa'; -- cgit From 7f233dcb15f897a4c7de29987e26143c9d192c01 Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Tue, 26 Jun 2018 17:34:43 -0700 Subject: Add more revert reasons to tests --- packages/contracts/test/asset_proxy/authorizable.ts | 17 +++++++++-------- packages/contracts/test/asset_proxy/proxies.ts | 8 +++----- 2 files changed, 12 insertions(+), 13 deletions(-) (limited to 'packages/contracts/test/asset_proxy') diff --git a/packages/contracts/test/asset_proxy/authorizable.ts b/packages/contracts/test/asset_proxy/authorizable.ts index c2295dda6..8c9d0495d 100644 --- a/packages/contracts/test/asset_proxy/authorizable.ts +++ b/packages/contracts/test/asset_proxy/authorizable.ts @@ -5,10 +5,7 @@ import * as chai from 'chai'; import { MixinAuthorizableContract } from '../../src/generated_contract_wrappers/mixin_authorizable'; import { artifacts } from '../../src/utils/artifacts'; -import { - expectRevertOrAlwaysFailingTransactionAsync, - expectRevertReasonOrAlwaysFailingTransactionAsync, -} from '../../src/utils/assertions'; +import { expectRevertReasonOrAlwaysFailingTransactionAsync } from '../../src/utils/assertions'; import { chaiSetup } from '../../src/utils/chai_setup'; import { constants } from '../../src/utils/constants'; import { provider, txDefaults, web3Wrapper } from '../../src/utils/web3_wrapper'; @@ -118,10 +115,11 @@ describe('Authorizable', () => { constants.AWAIT_TRANSACTION_MINED_MS, ); const index = new BigNumber(0); - return expectRevertOrAlwaysFailingTransactionAsync( + return expectRevertReasonOrAlwaysFailingTransactionAsync( authorizable.removeAuthorizedAddressAtIndex.sendTransactionAsync(address, index, { from: notOwner, }), + RevertReason.OnlyContractOwner, ); }); it('should throw if index is >= authorities.length', async () => { @@ -130,18 +128,20 @@ describe('Authorizable', () => { constants.AWAIT_TRANSACTION_MINED_MS, ); const index = new BigNumber(1); - return expectRevertOrAlwaysFailingTransactionAsync( + return expectRevertReasonOrAlwaysFailingTransactionAsync( authorizable.removeAuthorizedAddressAtIndex.sendTransactionAsync(address, index, { from: owner, }), + RevertReason.IndexOutOfBounds, ); }); it('should throw if owner attempts to remove an address that is not authorized', async () => { const index = new BigNumber(0); - return expectRevertOrAlwaysFailingTransactionAsync( + return expectRevertReasonOrAlwaysFailingTransactionAsync( authorizable.removeAuthorizedAddressAtIndex.sendTransactionAsync(address, index, { from: owner, }), + RevertReason.TargetNotAuthorized, ); }); it('should throw if address at index does not match target', async () => { @@ -156,10 +156,11 @@ describe('Authorizable', () => { constants.AWAIT_TRANSACTION_MINED_MS, ); const address1Index = new BigNumber(0); - return expectRevertOrAlwaysFailingTransactionAsync( + return expectRevertReasonOrAlwaysFailingTransactionAsync( authorizable.removeAuthorizedAddressAtIndex.sendTransactionAsync(address2, address1Index, { from: owner, }), + RevertReason.AuthorizedAddressMismatch, ); }); it('should allow owner to remove an authorized address', async () => { diff --git a/packages/contracts/test/asset_proxy/proxies.ts b/packages/contracts/test/asset_proxy/proxies.ts index 5a5289508..595839519 100644 --- a/packages/contracts/test/asset_proxy/proxies.ts +++ b/packages/contracts/test/asset_proxy/proxies.ts @@ -15,10 +15,7 @@ import { DummyERC721TokenContract } from '../../src/generated_contract_wrappers/ import { ERC20ProxyContract } from '../../src/generated_contract_wrappers/e_r_c20_proxy'; import { ERC721ProxyContract } from '../../src/generated_contract_wrappers/e_r_c721_proxy'; import { artifacts } from '../../src/utils/artifacts'; -import { - expectRevertOrAlwaysFailingTransactionAsync, - expectRevertReasonOrAlwaysFailingTransactionAsync, -} from '../../src/utils/assertions'; +import { expectRevertReasonOrAlwaysFailingTransactionAsync } from '../../src/utils/assertions'; import { chaiSetup } from '../../src/utils/chai_setup'; import { constants } from '../../src/utils/constants'; import { ERC20Wrapper } from '../../src/utils/erc20_wrapper'; @@ -298,7 +295,7 @@ describe('Asset Transfer Proxies', () => { expect(ownerMakerAsset).to.be.bignumber.equal(makerAddress); // Perform a transfer from makerAddress to takerAddress const amount = new BigNumber(1); - return expectRevertOrAlwaysFailingTransactionAsync( + return expectRevertReasonOrAlwaysFailingTransactionAsync( erc721Proxy.transferFrom.sendTransactionAsync( encodedAssetData, makerAddress, @@ -306,6 +303,7 @@ describe('Asset Transfer Proxies', () => { amount, { from: exchangeAddress }, ), + RevertReason.TransferFailed, ); }); -- cgit