Skip to content
This repository has been archived by the owner on Jul 9, 2021. It is now read-only.

@0x/contracts-erc20-bridge-sampler: LiquidityProviderRegistry #2499

Merged

Conversation

moodlezoup
Copy link
Contributor

@moodlezoup moodlezoup commented Feb 27, 2020

Description

Updates the way the ERC20BridgeSampler samples from generic liquidity providers to route through a registry. This allows the owner of the registry to swap out the underlying liquidity providers without interruption for 0x API

Testing instructions

Types of changes

Checklist:

  • Prefix PR title with [WIP] if necessary.
  • Add tests to cover changes as needed.
  • Update documentation as needed.
  • Add new entries to the relevant CHANGELOG.jsons.

@moodlezoup moodlezoup force-pushed the feature/erc20-bridge-sampler/liquidity-provider-registry branch from d70ac86 to 4bc5106 Compare February 27, 2020 01:02
@moodlezoup moodlezoup marked this pull request as ready for review February 27, 2020 01:16
@moodlezoup moodlezoup changed the title Get liquidity provider from registry in the sampler @0x/contracts-erc20-bridge-sampler: LiquidityProviderRegistry Feb 27, 2020
@coveralls
Copy link

coveralls commented Feb 27, 2020

Coverage Status

Coverage increased (+0.2%) to 79.739% when pulling a47c031 on feature/erc20-bridge-sampler/liquidity-provider-registry into e1a0617 on development.

takerToken,
makerToken
);

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if a liquidity pool is not found, we should return an array of 0s

@@ -435,14 +436,14 @@ contract ERC20BridgeSampler is
}

/// @dev Sample sell quotes from an arbitrary on-chain liquidity provider.
/// @param providerAddress Address of the liquidity provider contract.
/// @param registryAddress Address of the liquidity provider registry contract.
/// @param takerToken Address of the taker token (what to sell).
/// @param makerToken Address of the maker token (what to buy).
/// @param takerTokenAmounts Taker token sell amount for each sample.
/// @return makerTokenAmounts Maker amounts bought at each taker token
/// amount.
function sampleSellsFromLiquidityProvider(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed, this should be sample(Sells|Buys)FromLiquidityProviderRegistry()

@0xProject 0xProject deleted a comment from PirosB3 Feb 27, 2020
@0xProject 0xProject deleted a comment from PirosB3 Feb 27, 2020
@moodlezoup moodlezoup force-pushed the feature/erc20-bridge-sampler/liquidity-provider-registry branch from 99d94b3 to 52bd73a Compare February 27, 2020 19:12
@moodlezoup moodlezoup force-pushed the feature/erc20-bridge-sampler/liquidity-provider-registry branch from 52bd73a to a47c031 Compare February 27, 2020 19:23
)
public
view
returns (address providerAddress)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

by default, is providerAddress set to address(0)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep

Copy link
Contributor

@dorothy-zbornak dorothy-zbornak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Maybe regenerate contract wrappers to make Daniel's life a little easier.

@PirosB3 PirosB3 merged commit 68fb6c2 into development Feb 27, 2020
@PirosB3 PirosB3 deleted the feature/erc20-bridge-sampler/liquidity-provider-registry branch February 27, 2020 20:37
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants