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/tinyman #90

Merged
merged 31 commits into from
May 22, 2023
Merged

Feat/tinyman #90

merged 31 commits into from
May 22, 2023

Conversation

vic-en
Copy link
Collaborator

@vic-en vic-en commented Apr 21, 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:
[ch 39263]
This pr adds the tiny man connector on top of the Algorand chain.

Tests performed by the developer:

  • Price endpoint test
  • Trade endpoint test

Tips for QA testing:

  • Test price endpoint using curl
  • Test trade endpoint using curl

Algorand generic connector - hummingbot/hummingbot#6277
PRP - https://snapshot.org/#/hbot-prp.eth/proposal/0x08a53dead66043ee8e8b0e65229a99b52be2e212a640e992eb72c9b4cd388701

petioptrv and others added 18 commits April 13, 2023 13:27
- This commit also includes odd fixes for Injective's RLU cache handling
- Also includes some formatting changes
# Conflicts:
#	src/services/wallet/wallet.controllers.ts
Co-authored-by: vic-en <31972210+vic-en@users.noreply.github.com>
Co-authored-by: vic-en <31972210+vic-en@users.noreply.github.com>
Co-authored-by: vic-en <31972210+vic-en@users.noreply.github.com>
@vic-en vic-en closed this Apr 21, 2023
@vic-en vic-en reopened this May 2, 2023
@vic-en vic-en changed the base branch from main to development May 2, 2023 20:41
async getAccountFromAddress(address: string): Promise<Account> {
const path = `${walletPath}/${this._chain}`;
const encryptedMnemonic: string = await fse.readFile(
`${path}/${address}.json`,

Check failure

Code scanning / CodeQL

Uncontrolled data used in path expression

This path depends on a [user-provided value](1). This path depends on a [user-provided value](2). This path depends on a [user-provided value](3).
@rapcmia
Copy link
Contributor

rapcmia commented May 12, 2023

PR update:

  • Test on local WSL ubuntu20.04 with Feat / add algorand amm hummingbot#6277
  • Connect successfully wallet using Pera wallet and balance displayed ok
  • Setup thunder client for postman related tests {port, connectors}
  • Ran test endpoints using curl
    • Port and connectors ✅
    • Add/Remove wallet ✅
    • Get balance ✅
    • Get poll ✅
    • Get assets ✅
    • Post opt-in {GALGO,USDC} ✅
    • Check prices ✅
    • Trade {BUY,SELL} ✅
      • Compare trade data from endpoint and txhash from https://algoexplorer.io/
      • If not enough balance, getting error of overspend both client and gateway logs

Pending:

  • Client + Gateway test with connector

rapcmia
rapcmia previously approved these changes May 15, 2023
fengtality
fengtality previously approved these changes May 17, 2023
@nikspz
Copy link
Contributor

nikspz commented May 18, 2023

@vic-en
Failed to yarn build on this PR:

Steps:

  1. git clone -b feat/tinyman https://github.com/CoinAlpha/gateway.git gw90
  2. cd gw90
  3. yarn && yarn build

Actual:
Found 1 error in src/connectors/connectors.routes.ts:146
error Command failed with exit code 2.

src/connectors/connectors.routes.ts:146:13 - error TS2322: Type '{ name: string; trading_type: string[]; chain_type: string; available_networks: AvailableN
etworks[]; }' is not assignable to type '{ name: string; trading_type: string[]; available_networks: AvailableNetworks[]; additional_spenders?: string[] |
undefined; additional_add_wallet_prompts?: Record<string, string> | undefined; }'.
  Object literal may only specify known properties, and 'chain_type' does not exist in type '{ name: string; trading_type: string[]; available_networks: Av
ailableNetworks[]; additional_spenders?: string[] | undefined; additional_add_wallet_prompts?: Record<string, string> | undefined; }'.

146             chain_type: TinymanConfig.config.chainType,
                ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~


Found 1 error in src/connectors/connectors.routes.ts:146

error Command failed with exit code 2.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

image

@vic-en
Copy link
Collaborator Author

vic-en commented May 18, 2023

@nikspz PR #105 needs to be merged first.

@vic-en
Copy link
Collaborator Author

vic-en commented May 18, 2023

Pull again. I just merged with development on this pr.

@nikspz
Copy link
Contributor

nikspz commented May 18, 2023

curl -s -X POST -k --key $GATEWAY_KEY --cert $GATEWAY_CERT -H "Content-Type: application/json" -d "$(envsubst < ./requests/algorand_balances.json)" https://localhost:15888/algorand/balances | jq

Display balance for GOBTC in request ✅

Failed to get balance for goBTC ❌

The issue is current Tokenlist is remote, and its using goBTC in here (so you can't create amm_arb using GOBTC pairs)

image

@nikspz
Copy link
Contributor

nikspz commented May 18, 2023

Test performed:

  • Test on local WSL ubuntu20.04 with Feat / add algorand amm hummingbot#6277

  • Connect successfully wallet using Pera wallet seed phrase, correct balance displayed

  • Added connector-tokens goBTC, balance not displayed after that (reported in comment above)

  • Ran test endpoints using curl

    • Status and /connectors ✅
    • /network/status?chain=algorand ✅
    • /wallet/add and /wallet/remove ✅
    • Get balance ✅ Failed to get balance for goBTC ❌
    • Get poll ✅
    • Get assets ✅
    • Post opt-in {GALGO,USDC} ✅
    • Check prices ✅ Failed to check price for ALGO-USDt using USDt with lowerkey letter ❌
    • Trade {BUY,SELL} ✅
      • Compare trade data from endpoint and txhash from https://algoexplorer.io/
      • If not enough balance, getting error of overspend both client and gateway logs
      • If not approved receiver error: must op-tin, asset 123456 missing from PZLY....UPZI, after opt-in request is resolved
      • Failed to trade with error (Trade query failed) when using pairs that cointain lowkey letters (goBTC, goETH, USDt etc.) ❌
  • Client + Gateway: Failed to start strategy using pairs with lowkey letters (goBTC, goETH, USDt etc.) ❌

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:
Test on local WSL ubuntu20.04 with hummingbot/hummingbot#6277

  • Connect successfully wallet using Pera wallet seed phrase, correct balance displayed

  • Ran test endpoints using curl

    • Status and /connectors ✅
    • /network/status?chain=algorand ✅
    • /wallet/add and /wallet/remove ✅
    • Get balance ✅
    • Get poll ✅
    • Get assets ✅ response with list of assets (GOBTC there use capital letters)
    • Post opt-in ✅
    • Check prices ✅
    • Trade {BUY,SELL} ✅
      • Compare trade data from endpoint and txhash from https://algoexplorer.io/
      • If not enough balance, getting error of overspend both client and gateway logs
      • If not approved receiver error: must op-tin, asset 123456 missing from PZLY....UPZI, after opt-in request is resolved
  • Client + Gateway: reported issue on PR6277

    Note:

    • Current Tokenlist(AssetList for algorand) is using remote URL (not local file)
    • Gateway connector-tokens for this connector must be added on all uppercase, sample GOBTC
    • Failed to trade with error (Trade query failed) when using pairs that cointain lowkey letters (goBTC, goETH, USDt etc.)

@nikspz nikspz requested a review from cardosofede May 20, 2023 15:19
@nikspz nikspz merged commit f07bcae into hummingbot:development May 22, 2023
@nikspz
Copy link
Contributor

nikspz commented May 22, 2023

Merged the PR in behalf of foundation team to development and will be available on the next version 1.16.0

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.

5 participants