From 807904bb8654dafe1fa1661122254bca464ea8c2 Mon Sep 17 00:00:00 2001 From: Daniel Pyrathon Date: Wed, 4 Mar 2020 07:59:09 -0800 Subject: [PATCH] addressed PR comments --- .../contracts/src/DummyLiquidityProvider.sol | 19 +++++------- .../src/DummyLiquidityProviderRegistry.sol | 30 ++++++++----------- .../contracts/src/DeploymentConstants.sol | 2 +- .../src/utils/market_operation_utils/index.ts | 14 ++++----- .../test/market_operation_utils_test.ts | 6 ++-- 5 files changed, 32 insertions(+), 39 deletions(-) diff --git a/contracts/erc20-bridge-sampler/contracts/src/DummyLiquidityProvider.sol b/contracts/erc20-bridge-sampler/contracts/src/DummyLiquidityProvider.sol index 05b7c6e500..fb7d44b9f4 100644 --- a/contracts/erc20-bridge-sampler/contracts/src/DummyLiquidityProvider.sol +++ b/contracts/erc20-bridge-sampler/contracts/src/DummyLiquidityProvider.sol @@ -4,11 +4,6 @@ pragma experimental ABIEncoderV2; contract DummyLiquidityProvider { - constructor() - public - // solhint-disable-next-line no-empty-blocks - {} - /// @dev Quotes the amount of `makerToken` that would be obtained by /// selling `sellAmount` of `takerToken`. /// @param sellAmount Amount of `takerToken` to sell. @@ -20,9 +15,10 @@ contract DummyLiquidityProvider ) external view - returns (uint256 makerTokenAmount) { - makerTokenAmount = sellAmount - 1; - } + returns (uint256 makerTokenAmount) + { + makerTokenAmount = sellAmount - 1; + } /// @dev Quotes the amount of `takerToken` that would need to be sold in /// order to obtain `buyAmount` of `makerToken`. @@ -35,7 +31,8 @@ contract DummyLiquidityProvider ) external view - returns (uint256 takerTokenAmount) { - takerTokenAmount = buyAmount + 1; - } + returns (uint256 takerTokenAmount) + { + takerTokenAmount = buyAmount + 1; + } } \ No newline at end of file diff --git a/contracts/erc20-bridge-sampler/contracts/src/DummyLiquidityProviderRegistry.sol b/contracts/erc20-bridge-sampler/contracts/src/DummyLiquidityProviderRegistry.sol index e4b3a55a88..e7e1244425 100644 --- a/contracts/erc20-bridge-sampler/contracts/src/DummyLiquidityProviderRegistry.sol +++ b/contracts/erc20-bridge-sampler/contracts/src/DummyLiquidityProviderRegistry.sol @@ -6,40 +6,36 @@ contract DummyLiquidityProviderRegistry { address private constant NULL_ADDRESS = address(0x0); - constructor() - public - // solhint-disable-next-line no-empty-blocks - {} - mapping (address => mapping (address => address)) internal _gAddressBook; /// @dev Sets address of pool for a market given market (xAsset, yAsset). - /// @param takerToken First asset managed by pool. - /// @param makerToken Second asset managed by pool. + /// @param xToken First asset managed by pool. + /// @param yToken Second asset managed by pool. /// @param poolAddress Address of pool. function setLiquidityProviderForMarket( - address takerToken, - address makerToken, + address xToken, + address yToken, address poolAddress - ) external + ) + external { - _gAddressBook[takerToken][makerToken] = poolAddress; - _gAddressBook[makerToken][takerToken] = poolAddress; + _gAddressBook[xToken][yToken] = poolAddress; + _gAddressBook[yToken][xToken] = poolAddress; } /// @dev Returns the address of pool for a market given market (xAsset, yAsset), or reverts if pool does not exist. - /// @param takerToken First asset managed by pool. - /// @param makerToken Second asset managed by pool. + /// @param xToken First asset managed by pool. + /// @param yToken Second asset managed by pool. /// @return Address of pool. function getLiquidityProviderForMarket( - address takerToken, - address makerToken + address xToken, + address yToken ) external view returns (address poolAddress) { - poolAddress = _gAddressBook[takerToken][makerToken]; + poolAddress = _gAddressBook[xToken][yToken]; require( poolAddress != NULL_ADDRESS, "Registry/MARKET_PAIR_NOT_SET" diff --git a/contracts/utils/contracts/src/DeploymentConstants.sol b/contracts/utils/contracts/src/DeploymentConstants.sol index ad310d388a..05f7b69f4f 100644 --- a/contracts/utils/contracts/src/DeploymentConstants.sol +++ b/contracts/utils/contracts/src/DeploymentConstants.sol @@ -49,7 +49,7 @@ contract DeploymentConstants { /// @dev Mainnet address of the 0x DevUtils contract. address constant private DEV_UTILS_ADDRESS = 0x74134CF88b21383713E096a5ecF59e297dc7f547; // /// @dev Kovan address of the 0x DevUtils contract. - // address constant private DEV_UTILS_ADDRESS = 0x161793Cdca4fF9E766A706c2C49c36AC1340bbcd; + // address constant private DEV_UTILS_ADDRESS = 0x9402639A828BdF4E9e4103ac3B69E1a6E522eB59; /// @dev Kyber ETH pseudo-address. address constant internal KYBER_ETH_ADDRESS = 0xEeeeeEeeeEeEeeEeEeEeeEEEeeeeEeeeeeeeEEeE; /// @dev Mainnet address of the dYdX contract. diff --git a/packages/asset-swapper/src/utils/market_operation_utils/index.ts b/packages/asset-swapper/src/utils/market_operation_utils/index.ts index 4fdaac04f2..9e34fb378c 100644 --- a/packages/asset-swapper/src/utils/market_operation_utils/index.ts +++ b/packages/asset-swapper/src/utils/market_operation_utils/index.ts @@ -75,7 +75,7 @@ export class MarketOperationUtils { const [makerToken, takerToken] = getOrderTokens(nativeOrders[0]); const [ fillableAmounts, - liquidityPoolAddress, + liquidityProviderAddress, ethToMakerAssetRate, dexQuotes, ] = await this._sampler.executeAsync( @@ -89,7 +89,7 @@ export class MarketOperationUtils { ? DexOrderSampler.ops.constant(new BigNumber(1)) : DexOrderSampler.ops.getMedianSellRate( difference(FEE_QUOTE_SOURCES, _opts.excludedSources).concat( - this._liquidityPoolSourceIfAvailable(), + this._liquidityProviderSourceIfAvailable(), ), makerToken, this._wethAddress, @@ -97,7 +97,7 @@ export class MarketOperationUtils { this._liquidityProviderRegistry, ), DexOrderSampler.ops.getSellQuotes( - difference(SELL_SOURCES, _opts.excludedSources).concat(this._liquidityPoolSourceIfAvailable()), + difference(SELL_SOURCES, _opts.excludedSources).concat(this._liquidityProviderSourceIfAvailable()), makerToken, takerToken, getSampleAmounts(takerAmount, _opts.numSamples, _opts.sampleDistributionBase), @@ -146,7 +146,7 @@ export class MarketOperationUtils { makerToken, collapsePath(optimalPath, false), _opts.bridgeSlippage, - liquidityPoolAddress, + liquidityProviderAddress, ); } @@ -187,7 +187,7 @@ export class MarketOperationUtils { ? DexOrderSampler.ops.constant(new BigNumber(1)) : DexOrderSampler.ops.getMedianSellRate( difference(FEE_QUOTE_SOURCES, _opts.excludedSources).concat( - this._liquidityPoolSourceIfAvailable(), + this._liquidityProviderSourceIfAvailable(), ), takerToken, this._wethAddress, @@ -195,7 +195,7 @@ export class MarketOperationUtils { this._liquidityProviderRegistry, ), DexOrderSampler.ops.getBuyQuotes( - difference(BUY_SOURCES, _opts.excludedSources).concat(this._liquidityPoolSourceIfAvailable()), + difference(BUY_SOURCES, _opts.excludedSources).concat(this._liquidityProviderSourceIfAvailable()), makerToken, takerToken, getSampleAmounts(makerAmount, _opts.numSamples, _opts.sampleDistributionBase), @@ -275,7 +275,7 @@ export class MarketOperationUtils { ); } - private _liquidityPoolSourceIfAvailable(): ERC20BridgeSource[] { + private _liquidityProviderSourceIfAvailable(): ERC20BridgeSource[] { return this._liquidityProviderRegistry !== NULL_ADDRESS ? [ERC20BridgeSource.LiquidityProvider] : []; } diff --git a/packages/asset-swapper/test/market_operation_utils_test.ts b/packages/asset-swapper/test/market_operation_utils_test.ts index f29b528235..d5874b76de 100644 --- a/packages/asset-swapper/test/market_operation_utils_test.ts +++ b/packages/asset-swapper/test/market_operation_utils_test.ts @@ -141,7 +141,7 @@ describe('MarketOperationUtils tests', () => { makerToken: string, takerToken: string, fillAmounts: BigNumber[], - liquidityProviderAddress?: string | undefined, + liquidityProviderAddress?: string, ) => DexSample[][]; function createGetMultipleSellQuotesOperationFromRates(rates: RatesBySource): GetMultipleQuotesOperation { @@ -169,7 +169,7 @@ describe('MarketOperationUtils tests', () => { makerToken: string, takerToken: string, fillAmounts: BigNumber[], - liquidityProviderAddress: string | undefined, + liquidityProviderAddress?: string, ) => { liquidityPoolParams.liquidityProviderAddress = liquidityProviderAddress; liquidityPoolParams.sources = sources; @@ -195,7 +195,7 @@ describe('MarketOperationUtils tests', () => { makerToken: string, takerToken: string, fillAmounts: BigNumber[], - liquidityProviderAddress?: string | undefined, + liquidityProviderAddress?: string, ) => BigNumber; type GetLiquidityProviderFromRegistryOperation = (