Revert if amount is zero, not if value is zero

0xProject · Mar 14, 2019 · 32b0b0b · 32b0b0b
1 parent cc4796f
commit 32b0b0b
Show file tree

Hide file tree

Showing 3 changed files with 18 additions and 19 deletions.
diff --git a/contracts/asset-proxy/contracts/src/ERC1155Proxy.sol b/contracts/asset-proxy/contracts/src/ERC1155Proxy.sol
@@ -176,7 +176,17 @@ contract ERC1155Proxy is
                 )
 
                 ////////// STEP 2/4 //////////
-                let scaleAmount := calldataload(100)
+                // load amount and ensure it's non-zero (we divide by amount below)
+                let amount := calldataload(100)
+                if iszero(amount) {
+                    // Revert with `Error("AMOUNT_GREATER_THAN_ZERO_REQUIRED")`
+                    mstore(0, 0x08c379a000000000000000000000000000000000000000000000000000000000)
+                    mstore(32, 0x0000002000000000000000000000000000000000000000000000000000000000)
+                    mstore(64, 0x00000021414d4f554e545f475245415445525f5448414e5f5a45524f5f524551)
+                    mstore(96, 0x5549524544000000000000000000000000000000000000000000000000000000)
+                    mstore(128, 0)
+                    revert(0, 132)
+                }
                 let valuesOffset := add(mload(100), 4) // add 4 for calldata offset
                 let valuesLengthInBytes := mul(
                     mload(valuesOffset),
@@ -190,21 +200,10 @@ 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)
+                    let expectedTokenValue := div(scaledTokenValue, amount)
                     if iszero(eq(expectedTokenValue, tokenValue)) {
                         // Revert with `Error("UINT256_OVERFLOW")`
                         mstore(0, 0x08c379a000000000000000000000000000000000000000000000000000000000)

diff --git a/contracts/asset-proxy/test/erc1155_proxy.ts b/contracts/asset-proxy/test/erc1155_proxy.ts
@@ -540,12 +540,12 @@ describe('ERC1155Proxy', () => {
                 RevertReason.NFTNotOwnedByFromAddress,
             );
         });
-        it('should revert if tansferring 0 amount of any token', async () => {
+        it('should revert if amount = 0', async () => {
             // setup test parameters
             const tokenHolders = [spender, receiver];
             const tokensToTransfer = nonFungibleTokensOwnedBySpender.slice(0, 3);
-            const valuesToTransfer = [nonFungibleValueToTransfer, new BigNumber(0), nonFungibleValueToTransfer];
-            const perUnitValue = perUnitValueNft;
+            const valuesToTransfer = [nonFungibleValueToTransfer, new BigNumber(1), nonFungibleValueToTransfer];
+            const perUnitValue = new BigNumber(0);
             // check balances before transfer
             const expectedInitialBalances = [
                 // spender
@@ -570,7 +570,7 @@ describe('ERC1155Proxy', () => {
                     receiverCallbackData,
                     authorized,
                 ),
-                RevertReason.TransferGreaterThanZeroRequired,
+                RevertReason.AmountGreaterThanZeroRequired,
             );
         });
         it('should revert if there is a multiplication overflow', async () => {

diff --git a/packages/types/src/index.ts b/packages/types/src/index.ts
@@ -310,7 +310,7 @@ export enum RevertReason {
     TransferRejected = 'TRANSFER_REJECTED',
     Uint256Underflow = 'UINT256_UNDERFLOW',
     // ERC1155 Proxy
-    TransferGreaterThanZeroRequired = 'TRANSFER_GREATER_THAN_ZERO_REQUIRED',
+    AmountGreaterThanZeroRequired = 'AMOUNT_GREATER_THAN_ZERO_REQUIRED',
 }
 
 export enum StatusCodes {