From 0b9e9eb0e4fb58ffbda84f0e77c3b0c83902d669 Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Mon, 26 Nov 2018 17:52:16 -0800 Subject: Don't load assetProxy if currentProxyid is equal to the last seen proxyid --- .../protocol/AssetProxy/MultiAssetProxy.sol | 26 ++++++++++++++-------- packages/contracts/test/asset_proxy/proxies.ts | 1 + 2 files changed, 18 insertions(+), 9 deletions(-) (limited to 'packages/contracts') diff --git a/packages/contracts/contracts/protocol/AssetProxy/MultiAssetProxy.sol b/packages/contracts/contracts/protocol/AssetProxy/MultiAssetProxy.sol index 6db42e44f..42231e73b 100644 --- a/packages/contracts/contracts/protocol/AssetProxy/MultiAssetProxy.sol +++ b/packages/contracts/contracts/protocol/AssetProxy/MultiAssetProxy.sol @@ -18,13 +18,11 @@ pragma solidity 0.4.24; -import "../../utils/SafeMath/SafeMath.sol"; import "../Exchange/MixinAssetProxyDispatcher.sol"; import "./MixinAuthorizable.sol"; contract MultiAssetProxy is - SafeMath, MixinAssetProxyDispatcher, MixinAuthorizable { @@ -172,6 +170,10 @@ contract MultiAssetProxy is // Calculate number of bytes in `amounts` contents let amountsByteLen := mul(amountsLen, 32) + // Initialize `assetProxyId` and `assetProxy` to 0 + let assetProxyId := 0 + let assetProxy := 0 + // Loop through `amounts` and `nestedAssetData`, calling `transferFrom` for each respective element for {let i := 0} lt(i, amountsByteLen) {i := add(i, 32)} { @@ -221,16 +223,22 @@ contract MultiAssetProxy is } // Load AssetProxy id - let assetProxyId := and( + let currentAssetProxyId := and( calldataload(nestedAssetDataElementContentsStart), 0xffffffff00000000000000000000000000000000000000000000000000000000 ) - // To lookup a value in a mapping, we load from the storage location keccak256(k, p), - // where k is the key left padded to 32 bytes and p is the storage slot - mstore(132, assetProxyId) - mstore(164, assetProxies_slot) - let assetProxy := sload(keccak256(132, 64)) + // Only load `assetProxy` if `currentAssetProxyId` does not equal `assetProxyId` + // We do not need to check if `currentAssetProxyId` is 0 since `assetProxy` is also initialized to 0 + if iszero(eq(currentAssetProxyId, assetProxyId)) { + // Update `assetProxyId` + assetProxyId := currentAssetProxyId + // To lookup a value in a mapping, we load from the storage location keccak256(k, p), + // where k is the key left padded to 32 bytes and p is the storage slot + mstore(132, assetProxyId) + mstore(164, assetProxies_slot) + assetProxy := sload(keccak256(132, 64)) + } // Revert if AssetProxy with given id does not exist if iszero(assetProxy) { @@ -257,7 +265,7 @@ contract MultiAssetProxy is 0, // pointer to start of input add(164, nestedAssetDataElementLen), // length of input 0, // write output over memory that won't be reused - 0 // reserve 512 bytes for output + 0 // don't copy output to memory ) // Revert with reason given by AssetProxy if `transferFrom` call failed diff --git a/packages/contracts/test/asset_proxy/proxies.ts b/packages/contracts/test/asset_proxy/proxies.ts index face0b92b..8fa1e602a 100644 --- a/packages/contracts/test/asset_proxy/proxies.ts +++ b/packages/contracts/test/asset_proxy/proxies.ts @@ -695,6 +695,7 @@ describe('Asset Transfer Proxies', () => { }); it('should have an id of 0x94cfcdd7', async () => { const proxyId = await multiAssetProxy.getProxyId.callAsync(); + // first 4 bytes of `keccak256('MultiAsset(uint256[],bytes[])')` const expectedProxyId = '0x94cfcdd7'; expect(proxyId).to.equal(expectedProxyId); }); -- cgit v1.2.3