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

Refactor / chain endpoints #147

Merged
merged 3 commits into from
Jul 6, 2023

Conversation

vic-en
Copy link
Collaborator

@vic-en vic-en commented Jul 5, 2023

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:
This PR refactors chain related endpoints to a new /chain/* endpoint.
List of refactored endpoints is as follows:

  • /evm/allowances ====> /chain/allowances
  • /evm/cancel ====> /chain/cancel
  • /evm/approve ====> /chain/approve
  • /evm/nonce ====> /chain/nonce
  • /algorand/opt-in====> /chain/approve
  • /algorand/assets====> /chain/tokens
  • /algorand/balances ====> /chain/balances
  • /algorand/poll ====> /chain/poll
  • /cosmos/poll====> /chain/poll
  • /cosmos/balances ====> /chain/balances
  • /injective/poll====> /chain/poll
  • /injective/balances ====> /chain/balances
  • /injective/transfer/to/bank ====> /chain/transfer
  • /injective/transfer/fom/bank ====> /chain/transfer
  • /near/poll====> /chain/poll
  • /near/balances ====> /chain/balances
  • /near/tokens====> /chain/balances
  • /network/balances ====> /chain/balances
  • /network/status ====> /chain/status
  • /network/poll ====> /chain/poll
  • /network/tokens ====> /chain/tokens
  • /network/config ====> /chain/config

Tests performed by the developer:

Tips for QA testing:

@vic-en vic-en requested a review from fengtality July 5, 2023 13:16
@vic-en vic-en changed the title refactor chains Refactor / chain endpoints Jul 5, 2023
@vic-en vic-en self-assigned this Jul 5, 2023
@fengtality
Copy link
Sponsor Contributor

The nonce Swagger docs are wrong, as it expects address, not privateKey
Screen Shot 2023-07-05 at 2 19 28 PM

Copy link
Sponsor Contributor

@fengtality fengtality left a comment

Choose a reason for hiding this comment

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

LGTM - I think the Swagger docs need updating, but that shouldn't block merge.

QA: Please test this PR on the following chains / networks:

  • Ethereum Goerli
  • BSC Mainnet
  • Polygon Mainnet
  • Avalanche Mainnet

For each chain, using a new wallet:

  1. Connect wallet to a DEX on that chain
  2. Run approve-tokens command for a token pair
  3. Run AMM-ARB strategy between that pair and a CEX pair

This was referenced Jul 5, 2023
@rapcmia
Copy link
Contributor

rapcmia commented Jul 6, 2023

PR update:

  • Setup this PR and 6445 gateway connected successfully
  • Add old and new wallet on Ethereum-Goerli and BSC-mainnet
  • Run curl tests:
    • Port and connectors ✅
    • Add wallet and remove, list all added wallet ✅
    • Network balance ✅
    • Allowance ✅
    • Approve ✅
    • Check poll ✅
    • Check price and trade ✅
  • Observe client behavior when running AMM-Arbitrage using new wallet (uniswap-goerli)
    • Approve tokens ✅
    • Connector-tokens ✅
    • Start the strategy and check arbitrage opportunity

Pending:

  • Client : Observe behavior when doin arbitrage/trade
  • Polygon and Avalanche mainnet

@fengtality fengtality merged commit 8971533 into hummingbot:development Jul 6, 2023
2 of 3 checks passed
@fengtality fengtality mentioned this pull request Sep 2, 2023
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants