From 626d0dfa93f24cfbdc11b2c4d6c05fe14e66c7f6 Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Mon, 20 May 2019 18:02:17 -0700 Subject: [PATCH] Store scaled values and end of calldata in ERC1155 Asset Proxy --- .../contracts/src/ERC1155Proxy.sol | 64 +++++++--- contracts/asset-proxy/test/erc1155_proxy.ts | 120 +++++++++++++++++- .../test/utils/erc1155_proxy_wrapper.ts | 27 ++-- .../erc1155/contracts/src/ERC1155Mintable.sol | 26 ++++ yarn.lock | 26 +++- 5 files changed, 229 insertions(+), 34 deletions(-) diff --git a/contracts/asset-proxy/contracts/src/ERC1155Proxy.sol b/contracts/asset-proxy/contracts/src/ERC1155Proxy.sol index 43fa0800ea..0597c364f5 100644 --- a/contracts/asset-proxy/contracts/src/ERC1155Proxy.sol +++ b/contracts/asset-proxy/contracts/src/ERC1155Proxy.sol @@ -59,7 +59,7 @@ contract ERC1155Proxy is // | | 100 | | 4. offset to data (*) | // | Data | | | ids: | // | | 132 | 32 | 1. ids Length | - // | | 164 | a | 2. ids Contents | + // | | 164 | a | 2. ids Contents | // | | | | values: | // | | 164 + a | 32 | 1. values Length | // | | 196 + a | b | 2. values Contents | @@ -77,31 +77,40 @@ contract ERC1155Proxy is // | | 4 | | 1. from address | // | | 36 | | 2. to address | // | | 68 | | 3. offset to ids (*) | - // | | 100 | | 4. offset to values (*) | + // | | 100 | | 4. offset to scaledValues (*) | // | | 132 | | 5. offset to data (*) | // | Data | | | ids: | // | | 164 | 32 | 1. ids Length | - // | | 196 | a | 2. ids Contents | + // | | 196 | a | 2. ids Contents | // | | | | values: | // | | 196 + a | 32 | 1. values Length | // | | 228 + a | b | 2. values Contents | // | | | | data | // | | 228 + a + b | 32 | 1. data Length | // | | 260 + a + b | c | 2. data Contents | + // | | | | scaledValues: (***) | + // | | 260 + a+b+c | 32 | 1. scaledValues Length | + // | | 292 + a+b+c | b | 2. scaledValues Contents | // // // (*): offset is computed from start of function parameters, so offset // by an additional 4 bytes in the calldata. - // + // // (**): the `Offset` column is computed assuming no calldata compression; // offsets in the Data Area are dynamic and should be evaluated in // real-time. // + // (***): The contents of `values` are modified and stored separately, as `scaledValues`. + // The `values` array cannot be overwritten, as other dynamically allocated fields + // (`ids` and `data`) may resolve to the same array contents. For example, if + // `ids` = [1,2] and `values` = [1,2], the asset data may be optimized + // such that both arrays resolve to same entry of [1,2]. + // // WARNING: The ABIv2 specification allows additional padding between // the Params and Data section. This will result in a larger // offset to assetData. // - // Note: Table #1 and Table #2 exists in Calldata. We construct Table #3 in memory. + // Note: Table #1 and Table #2 exist in Calldata. We construct Table #3 in memory. // // assembly { @@ -134,7 +143,7 @@ contract ERC1155Proxy is // Construct Table #3 in memory, starting at memory offset 0. // The algorithm below maps asset data from Table #1 and Table #2 to Table #3, while // scaling the `values` (Table #2) by `amount` (Table #1). Once Table #3 has - // been constructed in memory, the destination erc1155 contract is called using this + // been constructed in memory, the destination erc1155 contract is called using this // as its calldata. This process is divided into four steps, below. ////////// STEP 1/4 ////////// @@ -177,17 +186,31 @@ contract ERC1155Proxy is ) ////////// STEP 2/4 ////////// - let amount := calldataload(100) + // Setup iterators for `values` array (Table #3) let valuesOffset := add(mload(100), 4) // add 4 for calldata offset - let valuesLengthInBytes := mul( - mload(valuesOffset), - 32 - ) + let valuesLength := mload(valuesOffset) + let valuesLengthInBytes := mul(valuesLength, 32) let valuesBegin := add(valuesOffset, 32) let valuesEnd := add(valuesBegin, valuesLengthInBytes) - for { let tokenValueOffset := valuesBegin } + + // Setup iterators for `scaledValues` array (Table #3). + // This array is placed at the end of the regular ERC1155 calldata, + // which is 32 bytes longer than `assetData` (Table #2). + let scaledValuesOffset := add(assetDataLength, 32) + let scaledValuesBegin := add(scaledValuesOffset, 32) + let scaledValuesEnd := add(scaledValuesBegin, valuesLengthInBytes) + + // Scale `values` by `amount` and store the output in `scaledValues` + let amount := calldataload(100) + for { + let tokenValueOffset := valuesBegin + let scaledTokenValueOffset := scaledValuesBegin + } lt(tokenValueOffset, valuesEnd) - { tokenValueOffset := add(tokenValueOffset, 32) } + { + tokenValueOffset := add(tokenValueOffset, 32) + scaledTokenValueOffset := add(scaledTokenValueOffset, 32) + } { // Load token value and generate scaled value let tokenValue := mload(tokenValueOffset) @@ -206,10 +229,17 @@ contract ERC1155Proxy is revert(0, 100) } - // There was no overflow, update `tokenValue` with its scaled counterpart - mstore(tokenValueOffset, scaledTokenValue) + // There was no overflow, store the scaled token value + mstore(scaledTokenValueOffset, scaledTokenValue) } + // Store length of `scaledValues` (which is the same as `values`) + mstore(scaledValuesOffset, valuesLength) + + // Point `values` to `scaledValues` (see Table #3); + // subtract 4 from memory location to account for selector + mstore(100, sub(scaledValuesOffset, 4)) + ////////// STEP 3/4 ////////// // Store the safeBatchTransferFrom function selector, // and copy `from`/`to` fields from Table #1 to Table #3. @@ -232,7 +262,7 @@ contract ERC1155Proxy is assetAddress, // call address of erc1155 asset 0, // don't send any ETH 0, // pointer to start of input - add(assetDataLength, 32), // length of input (Table #3) is 32 bytes longer than `assetData` (Table #2) + scaledValuesEnd, // length of input (Table #3) is the end of the `scaledValues` 0, // write output over memory that won't be reused 0 // don't copy output to memory ) @@ -246,7 +276,7 @@ contract ERC1155Proxy is ) revert(0, returndatasize()) } - + // Return if call was successful return(0, 0) } diff --git a/contracts/asset-proxy/test/erc1155_proxy.ts b/contracts/asset-proxy/test/erc1155_proxy.ts index 9aaba81429..db5243e9c5 100644 --- a/contracts/asset-proxy/test/erc1155_proxy.ts +++ b/contracts/asset-proxy/test/erc1155_proxy.ts @@ -15,6 +15,7 @@ import { web3Wrapper, } from '@0x/contracts-test-utils'; import { BlockchainLifecycle } from '@0x/dev-utils'; +import { assetDataUtils } from '@0x/order-utils'; import { AssetProxyId, RevertReason } from '@0x/types'; import { BigNumber } from '@0x/utils'; import * as chai from 'chai'; @@ -483,7 +484,15 @@ describe('ERC1155Proxy', () => { const totalValuesTransferred = _.map(valuesToTransfer, (value: BigNumber) => { return value.times(valueMultiplier); }); + const erc1155ContractAddress = erc1155Wrapper.getContract().address; + const assetData = assetDataUtils.encodeERC1155AssetData( + erc1155ContractAddress, + tokensToTransfer, + valuesToTransfer, + receiverCallbackData, + ); const extraData = '0102030405060708'; + const assetDataWithExtraData = `${assetData}${extraData}`; // check balances before transfer const expectedInitialBalances = [spenderInitialFungibleBalance, receiverContractInitialFungibleBalance]; await erc1155Wrapper.assertBalancesAsync(tokenHolders, tokensToTransfer, expectedInitialBalances); @@ -497,7 +506,7 @@ describe('ERC1155Proxy', () => { valueMultiplier, receiverCallbackData, authorized, - extraData, + assetDataWithExtraData, ); // check receiver log ignored extra asset data expect(txReceipt.logs.length).to.be.equal(2); @@ -519,6 +528,115 @@ describe('ERC1155Proxy', () => { ]; await erc1155Wrapper.assertBalancesAsync(tokenHolders, tokensToTransfer, expectedFinalBalances); }); + it('should successfully transfer if token ids and values are abi encoded to same entry in calldata', async () => { + /** + * Suppose the `tokensToTransfer` and `valuesToTransfer` are identical; their offsets in + * the ABI-encoded asset data may be the same. E.g. token IDs [1, 2] and values [1, 2]. + * Suppose we scale by a factor of 2, then we expect to trade token IDs [1, 2] and values [2, 4]. + * This test ensures that scaling the values does not simultaneously scale the token IDs. + */ + ///// Step 1/5 ///// + // Create tokens with ids [1, 2, 3, 4] and mint a balance of 4 for the `spender` + const tokensToCreate = [new BigNumber(1), new BigNumber(2), new BigNumber(3), new BigNumber(4)]; + const spenderInitialBalance = new BigNumber(4); + const receiverInitialBalance = new BigNumber(0); + const tokenUri = ''; + for (const tokenToCreate of tokensToCreate) { + // create token + await erc1155Wrapper.getContract().createWithType.awaitTransactionSuccessAsync( + tokenToCreate, + tokenUri, + { + from: owner, + }, + constants.AWAIT_TRANSACTION_MINED_MS, + ); + + // mint balance for spender + await erc1155Wrapper.getContract().mintFungible.awaitTransactionSuccessAsync( + tokenToCreate, + [spender], + [spenderInitialBalance], + { + from: owner, + }, + constants.AWAIT_TRANSACTION_MINED_MS, + ); + } + ///// Step 2/5 ///// + // Check balances before transfer + const balanceHolders = [spender, spender, spender, spender, receiver, receiver, receiver, receiver]; + const balanceTokens = tokensToCreate.concat(tokensToCreate); + const initialBalances = await erc1155Wrapper.getBalancesAsync(balanceHolders, balanceTokens); + const expectedInitialBalances = [ + spenderInitialBalance, // Token ID 1 / Spender Balance + spenderInitialBalance, // Token ID 2 / Spender Balance + spenderInitialBalance, // Token ID 3 / Spender Balance + spenderInitialBalance, // Token ID 4 / Spender Balance + receiverInitialBalance, // Token ID 1 / Receiver Balance + receiverInitialBalance, // Token ID 2 / Receiver Balance + receiverInitialBalance, // Token ID 3 / Receiver Balance + receiverInitialBalance, // Token ID 4 / Receiver Balance + ]; + expect(initialBalances).to.be.deep.equal(expectedInitialBalances); + ///// Step 3/5 ///// + // Create optimized calldata. We expect it to be formatted like the table below. + // 0x 0000000000000000000000000b1ba0af832d7c05fd64161e0db78e85978e8082 // ERC1155 contract address + // 0x20 0000000000000000000000000000000000000000000000000000000000000080 // Offset to token IDs + // 0x40 0000000000000000000000000000000000000000000000000000000000000080 // Offset to token values (same as IDs) + // 0x60 00000000000000000000000000000000000000000000000000000000000000e0 // Offset to data + // 0x80 0000000000000000000000000000000000000000000000000000000000000002 // Length of token Ids / token values + // 0xA0 0000000000000000000000000000000000000000000000000000000000000001 // First Token ID / Token value + // 0xC0 0000000000000000000000000000000000000000000000000000000000000002 // Second Token ID / Token value + // 0xE0 0000000000000000000000000000000000000000000000000000000000000004 // Length of callback data + // 0x100 0102030400000000000000000000000000000000000000000000000000000000 // Callback data + const erc1155ContractAddress = erc1155Wrapper.getContract().address; + const tokensToTransfer = [new BigNumber(1), new BigNumber(2)]; + const valuesToTransfer = tokensToTransfer; + const valueMultiplier = new BigNumber(2); + const assetData = assetDataUtils.encodeERC1155AssetData( + erc1155ContractAddress, + tokensToTransfer, + valuesToTransfer, + receiverCallbackData, + ); + const offsetToTokenIds = 74; + const assetDataWithoutContractAddress = assetData.substr(offsetToTokenIds); + const expectedAssetDataWithoutContractAddress = + '0000000000000000000000000000000000000000000000000000000000000080000000000000000000000000000000000000000000000000000000000000008000000000000000000000000000000000000000000000000000000000000000e000000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000001000000000000000000000000000000000000000000000000000000000000000200000000000000000000000000000000000000000000000000000000000000040102030400000000000000000000000000000000000000000000000000000000'; + expect(assetDataWithoutContractAddress).to.be.equal(expectedAssetDataWithoutContractAddress); + ///// Step 4/5 ///// + // Transfer token IDs [1, 2] and amounts [1, 2] with a multiplier of 2; + // the expected trade will be token IDs [1, 2] and amounts [2, 4] + await erc1155ProxyWrapper.transferFromAsync( + spender, + receiver, + erc1155Contract.address, + tokensToTransfer, + valuesToTransfer, + valueMultiplier, + receiverCallbackData, + authorized, + assetData, + ); + ///// Step 5/5 ///// + // Validate final balances + const finalBalances = await erc1155Wrapper.getBalancesAsync(balanceHolders, balanceTokens); + const expectedAmountsTransferred = _.map(valuesToTransfer, value => { + return value.times(valueMultiplier); + }); + const expectedFinalBalances = [ + spenderInitialBalance.minus(expectedAmountsTransferred[0]), // Token ID 1 / Spender Balance + spenderInitialBalance.minus(expectedAmountsTransferred[1]), // Token ID 2 / Spender Balance + spenderInitialBalance, // Token ID 3 / Spender Balance + spenderInitialBalance, // Token ID 4 / Spender Balance + receiverInitialBalance.plus(expectedAmountsTransferred[0]), // Token ID 1 / Receiver Balance + receiverInitialBalance.plus(expectedAmountsTransferred[1]), // Token ID 2 / Receiver Balance + receiverInitialBalance, // Token ID 3 / Receiver Balance + receiverInitialBalance, // Token ID 4 / Receiver Balance + ]; + expect(finalBalances).to.be.deep.equal(expectedFinalBalances); + }); it('should transfer nothing if value is zero', async () => { // setup test parameters const tokenHolders = [spender, receiver]; diff --git a/contracts/asset-proxy/test/utils/erc1155_proxy_wrapper.ts b/contracts/asset-proxy/test/utils/erc1155_proxy_wrapper.ts index e17c166379..a84d2368db 100644 --- a/contracts/asset-proxy/test/utils/erc1155_proxy_wrapper.ts +++ b/contracts/asset-proxy/test/utils/erc1155_proxy_wrapper.ts @@ -106,20 +106,20 @@ export class ERC1155ProxyWrapper { valueMultiplier: BigNumber, receiverCallbackData: string, authorizedSender: string, - extraData?: string, + assetData_?: string, ): Promise { this._validateProxyContractExistsOrThrow(); - let encodedAssetData = assetDataUtils.encodeERC1155AssetData( - contractAddress, - tokensToTransfer, - valuesToTransfer, - receiverCallbackData, - ); - if (extraData !== undefined) { - encodedAssetData = `${encodedAssetData}${extraData}`; - } + const assetData = + assetData_ === undefined + ? assetDataUtils.encodeERC1155AssetData( + contractAddress, + tokensToTransfer, + valuesToTransfer, + receiverCallbackData, + ) + : assetData_; const data = this._assetProxyInterface.transferFrom.getABIEncodedTransactionData( - encodedAssetData, + assetData, from, to, valueMultiplier, @@ -128,6 +128,7 @@ export class ERC1155ProxyWrapper { to: (this._proxyContract as ERC1155ProxyContract).address, data, from: authorizedSender, + gas: 300000, }); return txHash; } @@ -153,7 +154,7 @@ export class ERC1155ProxyWrapper { valueMultiplier: BigNumber, receiverCallbackData: string, authorizedSender: string, - extraData?: string, + assetData?: string, ): Promise { const txReceipt = await this._logDecoder.getTxWithDecodedLogsAsync( await this.transferFromAsync( @@ -165,7 +166,7 @@ export class ERC1155ProxyWrapper { valueMultiplier, receiverCallbackData, authorizedSender, - extraData, + assetData, ), ); return txReceipt; diff --git a/contracts/erc1155/contracts/src/ERC1155Mintable.sol b/contracts/erc1155/contracts/src/ERC1155Mintable.sol index 021f46095d..073234f59a 100644 --- a/contracts/erc1155/contracts/src/ERC1155Mintable.sol +++ b/contracts/erc1155/contracts/src/ERC1155Mintable.sol @@ -63,6 +63,32 @@ contract ERC1155Mintable is } } + /// @dev creates a new token + /// @param type_ of token + /// @param uri URI of token + function createWithType( + uint256 type_, + string calldata uri + ) + external + { + // This will allow restricted access to creators. + creators[type_] = msg.sender; + + // emit a Transfer event with Create semantic to help with discovery. + emit TransferSingle( + msg.sender, + address(0x0), + address(0x0), + type_, + 0 + ); + + if (bytes(uri).length > 0) { + emit URI(uri, type_); + } + } + /// @dev mints fungible tokens /// @param id token type /// @param to beneficiaries of minted tokens diff --git a/yarn.lock b/yarn.lock index c7c6617199..0bd6bd4f39 100644 --- a/yarn.lock +++ b/yarn.lock @@ -13274,7 +13274,7 @@ prebuild-install@^2.2.2: pump "^2.0.1" rc "^1.1.6" simple-get "^2.7.0" - tar-fs "~1.16.3" + tar-fs "^1.13.0" tunnel-agent "^0.6.0" which-pm-runs "^1.0.0" @@ -13854,7 +13854,7 @@ rc@^1.0.1, rc@^1.1.6, rc@^1.1.7: version "1.2.6" resolved "https://registry.yarnpkg.com/rc/-/rc-1.2.6.tgz#eb18989c6d4f4f162c399f79ddd29f3835568092" dependencies: - deep-extend "^0.6.0" + deep-extend "~0.4.0" ini "~1.3.0" minimist "^1.2.0" strip-json-comments "~2.0.1" @@ -13884,6 +13884,15 @@ react-dom@^16.3.2: object-assign "^4.1.1" prop-types "^15.6.0" +react-dom@^16.4.2: + version "16.8.6" + resolved "https://registry.yarnpkg.com/react-dom/-/react-dom-16.8.6.tgz#71d6303f631e8b0097f56165ef608f051ff6e10f" + dependencies: + loose-envify "^1.1.0" + object-assign "^4.1.1" + prop-types "^15.6.2" + scheduler "^0.13.6" + react-dom@^16.5.2: version "16.5.2" resolved "https://registry.yarnpkg.com/react-dom/-/react-dom-16.5.2.tgz#b69ee47aa20bab5327b2b9d7c1fe2a30f2cfa9d7" @@ -13941,6 +13950,8 @@ react-highlight@0xproject/react-highlight#react-peer-deps: dependencies: highlight.js "^9.11.0" highlightjs-solidity "^1.0.5" + react "^16.4.2" + react-dom "^16.4.2" react-hot-loader@^4.3.3: version "4.3.4" @@ -14185,6 +14196,15 @@ react@^16.3.2: object-assign "^4.1.1" prop-types "^15.6.0" +react@^16.4.2: + version "16.8.6" + resolved "https://registry.yarnpkg.com/react/-/react-16.8.6.tgz#ad6c3a9614fd3a4e9ef51117f54d888da01f2bbe" + dependencies: + loose-envify "^1.1.0" + object-assign "^4.1.1" + prop-types "^15.6.2" + scheduler "^0.13.6" + react@^16.5.2: version "16.5.2" resolved "https://registry.yarnpkg.com/react/-/react-16.5.2.tgz#19f6b444ed139baa45609eee6dc3d318b3895d42" @@ -15066,7 +15086,7 @@ schedule@^0.5.0: scheduler@^0.13.6: version "0.13.6" - resolved "https://registry.npmjs.org/scheduler/-/scheduler-0.13.6.tgz#466a4ec332467b31a91b9bf74e5347072e4cd889" + resolved "https://registry.yarnpkg.com/scheduler/-/scheduler-0.13.6.tgz#466a4ec332467b31a91b9bf74e5347072e4cd889" dependencies: loose-envify "^1.1.0" object-assign "^4.1.1"