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

feat/ add poolId with unit test. #306

Merged
merged 1 commit into from
Apr 11, 2024

Conversation

vic-en
Copy link
Collaborator

@vic-en vic-en commented Apr 6, 2024

Before submitting this PR, please make sure:

  • Your code builds clean without any errors or warnings
  • You are using approved title ("feat/", "fix/", "docs/", "refactor/")

A description of the changes proposed in the pull request:
Adds PoolId as described in #302
This pr will me merged into Balancer pr to further confirm it works.

Tests performed by the developer:

  • Unit test
  • Curl test to ensure exisiting implementation isn't broken

Tips for QA testing:

  • Perform curl test to ensure existing implemetion isn't broken.

@rapcmia rapcmia requested review from fengtality and nikspz April 8, 2024 10:07
Copy link
Contributor

@nikspz nikspz left a comment

Choose a reason for hiding this comment

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

Test performed:

  • Cloned and installed latest development and gwPR306
  • connected successfully uniswap_ethereum_arbitrum
  • Created amm_arb using uniswap_ethereum_arbitrum successfully
  • started amm_arb, Review order execution is okay
  • Created amm_v3_lp using uniswap_ethereum_arbitrum
  • started amm_v3_lp, review position created
  • Curl:
    • connectors: ok
    • /chain/status: ok
    • /chain/config: ok
    • wallet/add: ok
    • wallet/remove: ok
    • amm/price: ok
    • amm/trade: ok
    • amm/liquidity/add: ok
    • amm/liquidity/price: ok
    • amm/liquidity/position: ok
    • amm/liquidity/remove: ok

Copy link

@nikspz nikspz merged commit 0fe8449 into hummingbot:development Apr 11, 2024
3 checks passed
Copy link

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