Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/liquidity provider sandbox #16

Merged
merged 18 commits into from
Nov 13, 2020

Conversation

moodlezoup
Copy link
Contributor

@moodlezoup moodlezoup commented Oct 27, 2020

Description

  • Updates LiquidityProviderFeature to use an off-chain registry instead of an on-chain one
  • Removes MultiBridge stuff from AS
  • Updates the ILiquidityProvider interface
  • Updates the ProtocolFeeUnfunded event in FQT

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.

public
FixinCommon()
{
weth = weth_;
sandbox = new LiquidityProviderSandbox(zeroEx);
Copy link
Contributor

Choose a reason for hiding this comment

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

Wouldn't we rather pass in the sandbox so we can reuse an older one and avoid having to whitelist a new one every time we make a change to this feature?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah I was thinking in the next iteration of this feature, sandbox would be passed in as a constructor arg. no need to pass it in for this first version though, might as well deploy it in the same txn

@dorothy-zbornak
Copy link
Contributor

think a rebase will fix the test-publish 🤞

@moodlezoup moodlezoup force-pushed the feature/liquidity_provider_sandbox branch from 8849fc3 to ca8e066 Compare October 28, 2020 17:32
@moodlezoup moodlezoup force-pushed the feature/liquidity_provider_sandbox branch from b93330e to c4458bb Compare October 28, 2020 23:23
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.

👏 👏 👏

@dorothy-zbornak
Copy link
Contributor

Has this been simbotted btw?

@moodlezoup moodlezoup force-pushed the feature/liquidity_provider_sandbox branch from ace05ce to 32e1bd0 Compare October 30, 2020 20:07
@dorothy-zbornak dorothy-zbornak mentioned this pull request Nov 3, 2020
4 tasks
@moodlezoup moodlezoup force-pushed the feature/liquidity_provider_sandbox branch 2 times, most recently from d09a6ab to b40eee5 Compare November 9, 2020 21:40
@dorothy-zbornak dorothy-zbornak force-pushed the feature/liquidity_provider_sandbox branch from b40eee5 to 06295c4 Compare November 10, 2020 17:33
@moodlezoup moodlezoup merged commit 7403c02 into development Nov 13, 2020
@moodlezoup moodlezoup deleted the feature/liquidity_provider_sandbox branch November 13, 2020 20:22
moodlezoup added a commit that referenced this pull request Nov 14, 2020
moodlezoup added a commit that referenced this pull request Nov 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants