From df9ec7cafdbe60f227c0caddd82b9f6a5e1f50bb Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Thu, 14 Mar 2019 13:38:16 -0700 Subject: [PATCH] Do not revert if value or amount are zero. Only if amount is non-zero and there is an overflow. --- .../contracts/src/ERC1155Proxy.sol | 23 ++++--------- contracts/asset-proxy/test/erc1155_proxy.ts | 33 ------------------- packages/types/src/index.ts | 2 -- 3 files changed, 7 insertions(+), 51 deletions(-) diff --git a/contracts/asset-proxy/contracts/src/ERC1155Proxy.sol b/contracts/asset-proxy/contracts/src/ERC1155Proxy.sol index 57862138e0..c847d07779 100644 --- a/contracts/asset-proxy/contracts/src/ERC1155Proxy.sol +++ b/contracts/asset-proxy/contracts/src/ERC1155Proxy.sol @@ -176,7 +176,7 @@ contract ERC1155Proxy is ) ////////// STEP 2/4 ////////// - let scaleAmount := calldataload(100) + let amount := calldataload(100) let valuesOffset := add(mload(100), 4) // add 4 for calldata offset let valuesLengthInBytes := mul( mload(valuesOffset), @@ -190,22 +190,13 @@ contract ERC1155Proxy is { // Load token value and generate scaled value let tokenValue := mload(tokenValueOffset) - let scaledTokenValue := mul(tokenValue, scaleAmount) - - // Check if scaled value is zero - if iszero(scaledTokenValue) { - // Revert with `Error("TRANSFER_GREATER_THAN_ZERO_REQUIRED")` - mstore(0, 0x08c379a000000000000000000000000000000000000000000000000000000000) - mstore(32, 0x0000002000000000000000000000000000000000000000000000000000000000) - mstore(64, 0x000000235452414e534645525f475245415445525f5448414e5f5a45524f5f52) - mstore(96, 0x4551554952454400000000000000000000000000000000000000000000000000) - mstore(128, 0) - revert(0, 132) - } + let scaledTokenValue := mul(tokenValue, amount) - // Check for multiplication overflow - let expectedTokenValue := div(scaledTokenValue, scaleAmount) - if iszero(eq(expectedTokenValue, tokenValue)) { + // Revert if `amount` != 0 and multiplication resulted in an overflow + if iszero(or( + iszero(amount), + eq(div(scaledTokenValue, amount), tokenValue) + )) { // Revert with `Error("UINT256_OVERFLOW")` mstore(0, 0x08c379a000000000000000000000000000000000000000000000000000000000) mstore(32, 0x0000002000000000000000000000000000000000000000000000000000000000) diff --git a/contracts/asset-proxy/test/erc1155_proxy.ts b/contracts/asset-proxy/test/erc1155_proxy.ts index 01368f7833..e69bfb6900 100644 --- a/contracts/asset-proxy/test/erc1155_proxy.ts +++ b/contracts/asset-proxy/test/erc1155_proxy.ts @@ -540,39 +540,6 @@ describe('ERC1155Proxy', () => { RevertReason.NFTNotOwnedByFromAddress, ); }); - it('should revert if tansferring 0 amount of any token', async () => { - // setup test parameters - const tokenHolders = [spender, receiver]; - const tokensToTransfer = nonFungibleTokensOwnedBySpender.slice(0, 3); - const valuesToTransfer = [nonFungibleValueToTransfer, new BigNumber(0), nonFungibleValueToTransfer]; - const perUnitValue = perUnitValueNft; - // check balances before transfer - const expectedInitialBalances = [ - // spender - nftOwnerBalance, - nftOwnerBalance, - nftOwnerBalance, - // receiver - nftNotOwnerBalance, - nftNotOwnerBalance, - nftNotOwnerBalance, - ]; - await erc1155Wrapper.assertBalancesAsync(tokenHolders, tokensToTransfer, expectedInitialBalances); - // execute transfer - await expectTransactionFailedAsync( - erc1155ProxyWrapper.transferFromAsync( - spender, - receiver, - erc1155Contract.address, - tokensToTransfer, - valuesToTransfer, - perUnitValue, - receiverCallbackData, - authorized, - ), - RevertReason.TransferGreaterThanZeroRequired, - ); - }); it('should revert if there is a multiplication overflow', async () => { // setup test parameters const tokenHolders = [spender, receiver]; diff --git a/packages/types/src/index.ts b/packages/types/src/index.ts index 49bf741e48..23cc118b2e 100644 --- a/packages/types/src/index.ts +++ b/packages/types/src/index.ts @@ -309,8 +309,6 @@ export enum RevertReason { TriedToMintNonFungibleForFungibleToken = 'TRIED_TO_MINT_NON_FUNGIBLE_FOR_FUNGIBLE_TOKEN', TransferRejected = 'TRANSFER_REJECTED', Uint256Underflow = 'UINT256_UNDERFLOW', - // ERC1155 Proxy - TransferGreaterThanZeroRequired = 'TRANSFER_GREATER_THAN_ZERO_REQUIRED', } export enum StatusCodes {