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 Defira connector #5422

Merged
merged 81 commits into from Aug 11, 2022

Conversation

navijation
Copy link
Contributor

@navijation navijation commented Jun 19, 2022

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:

  • Add Defira gateway connector and manual curl tests
  • Add schema and validation tests for Defira config file
  • Fix bug in Harmony chain which messed up gas estimates
  • Add WONE74 token used by Defira testnet to sushiswap testnet file to allow manual tests to pass
  • Add dependency on fixed commit of https://github.com/zuzu-cat/defira-sdk for Defira pairs and trades

Tests performed by the developer:

  • Get sell / buy price on Defira mainnet using USDC and FIRA pair (should be roughly 2.365 FIRA for USDC)
  • Perform sell / buy trade on Defira testnet using WONE74 (the token used to bootstrap testnet DEX) and 1ETH
  • Added unit tests for Defira routes, core, and schema validation

Tips for QA testing:

  • Make sure that token config file is setup to accomodate Defira properly (default is Sushiswap tokens; add more tokens
    as needed or switch to the Defira testnet tokens JSON in harmony.yaml)

navijation and others added 30 commits May 10, 2022 21:09
…nectors-api

feat/Add Defira to `GET /connectors` API
Feat/Add Defira mainnet tokens to config
Rename Defira connector, fix testnet router address
…hema

Feat/Add Defira config schema and init code hash
@arnelhbot
Copy link
Contributor

PR Update: Upon testing we have noticed that an Unhandled error in background task: 'harmony' error happens when we try to connect an existing wallet with Mainnet and Testnet.

2022-07-14 04:25:29 | error | 	unhandledRejection: Client network socket disconnected before secure TLS connection was established
Error: Client network socket disconnected before secure TLS connection was established
    at connResetException (node:internal/errors:683:14)
    at TLSSocket.onConnectEnd (node:_tls_wrap:1577:19)
    at TLSSocket.emit (node:events:406:35)
    at endReadableNT (node:internal/streams/readable:1329:12)
    at processTicksAndRejections (node:internal/process/task_queues:83:21) | Error: Client network socket disconnected before secure TLS connection was established
    at connResetException (node:internal/errors:683:14)
    at TLSSocket.onConnectEnd (node:_tls_wrap:1577:19)
    at TLSSocket.emit (node:events:406:35)
    at endReadableNT (node:internal/streams/readable:1329:12)
    at processTicksAndRejections (node:internal/process/task_queues:83:21)

Steps to reproduce:

  1. Delete docker instance in .hummingbot-gateway
  2. Stop and Delete docker instance Hummingbot-gateway-xxxxxx
  3. Start client and call gateway create and enter Infura API
  4. Enter gateway connect and select Defira/mainnet
  5. Enter gateway connect and select Defira/testnet
  6. When prompted to connect with existing wallet, type yes

image
logs_gateway_app.log.2022-07-14.zip

@navijation
Copy link
Contributor Author

@arnelhbot Are you sure this is related to my PR? I didn't make any changes to the Harmony chain aside from the gas price calculation and the getSpender stuff.

@rapcmia
Copy link
Contributor

rapcmia commented Jul 18, 2022

@arnelhbot Are you sure this is related to my PR? I didn't make any changes to the Harmony chain aside from the gas price calculation and the getSpender stuff.

Hi @navneethjayendran yes, looks like it has something to do with Harmony chain and outside of the PR. We are investigating this atm since this is the first PR connector for the Harmony chain.

@arnelhbot
Copy link
Contributor

PR Update: Up testing we noticed that when starting the AMM_Arbitrage strategy connected to Harmony mainnet wallet, we receive an Unhandled error in background task.

2022-07-19 01:30:26 | error | 	The spender param is not a valid Ethereum address (0x followed by 40 hexidecimal characters). | Error: The spender param is not a valid Ethereum address (0x followed by 40 hexidecimal characters).
    at throwIfErrorsExist (/usr/src/app/dist/src/services/validators.js:38:15)
    at /usr/src/app/dist/src/services/validators.js:85:40
    at /usr/src/app/dist/src/evm/evm.routes.js:32:61
    at Generator.next (<anonymous>)
    at /usr/src/app/dist/src/evm/evm.routes.js:8:71
    at new Promise (<anonymous>)
    at __awaiter (/usr/src/app/dist/src/evm/evm.routes.js:4:12)
    at /usr/src/app/dist/src/evm/evm.routes.js:31:90
    at /usr/src/app/dist/src/services/error-handler.js:37:28
    at Layer.handle [as handle_request] (/usr/src/app/node_modules/express/lib/router/layer.js:95:5)

image

Steps to reproduce:

  1. Connect Harmony mainnet wallet
  2. Create AMM_Arbitrage strategy
  3. Start strategy and observe log pane for error

logs_gateway_app.log.2022-07-18.zip

@Faouzijedidi1
Copy link
Contributor

@navneethjayendran To fix the current error that @arnelhbot faced. You need to add defira to the validated spenders in the harmony.validators.ts lines 40-46. Similar to how sushiswap and viperswap are validated add defira there. This File

@navijation
Copy link
Contributor Author

@navneethjayendran To fix the current error that @arnelhbot faced. You need to add defira to the validated spenders in the harmony.validators.ts lines 40-46. Similar to how sushiswap and viperswap are validated add defira there. This File

Thanks for the reminder. I forgot to test this step E2E. Added unit test coverage.

@arnelhbot
Copy link
Contributor

arnelhbot commented Jul 21, 2022

PR Update: Upon testing, we have noticed we are still getting the same The spender param is not a valid Ethereum address error. We are also seeing a new error missing response when running bot and waiting for allowance

2022-07-22 02:48:54 | error | 	missing response (requestBody="{\"method\":\"eth_getBalance\",\"params\":[\"0x08940dc9b5a19fab9319b77c61dda7b8067e6843\",\"latest\"],\"id\":45,\"jsonrpc\":\"2.0\"}", requestMethod="POST", serverError={"code":"ECONNRESET","path":null,"host":"api.s0.t.hmny.io","port":443}, url="https://api.s0.t.hmny.io/", code=SERVER_ERROR, version=web/5.6.1) | Error: missing response (requestBody="{\"method\":\"eth_getBalance\",\"params\":[\"0x08940dc9b5a19fab9319b77c61dda7b8067e6843\",\"latest\"],\"id\":45,\"jsonrpc\":\"2.0\"}", requestMethod="POST", serverError={"code":"ECONNRESET","path":null,"host":"api.s0.t.hmny.io","port":443}, url="https://api.s0.t.hmny.io/", code=SERVER_ERROR, version=web/5.6.1)
    at Logger.makeError (/usr/src/app/node_modules/@ethersproject/logger/lib/index.js:233:21)
    at Logger.throwError (/usr/src/app/node_modules/@ethersproject/logger/lib/index.js:242:20)
    at /usr/src/app/node_modules/@ethersproject/web/lib/index.js:252:36
    at step (/usr/src/app/node_modules/@ethersproject/web/lib/index.js:33:23)
    at Object.throw (/usr/src/app/node_modules/@ethersproject/web/lib/index.js:14:53)
    at rejected (/usr/src/app/node_modules/@ethersproject/web/lib/index.js:6:65)
    at processTicksAndRejections (node:internal/process/task_queues:96:5)

image

Steps to reproduce:

  1. Connect Harmony mainnet wallet
  2. Create AMM_Arbitrage strategy
  3. Start strategy and observe log pane for error

logs_gateway_app.log.2022-07-21.zip

@navijation
Copy link
Contributor Author

@Faouzijedidi1 It seems like the Harmony chain integration is problematic since things are failing on operations that have nothing to do with the connector, e.g. reusing an existing wallet. Can we meet and discuss the best solution to these issues?

@arnelhbot
Copy link
Contributor

Upon testing new updates, we are getting a async_utils - Unhandled error in background task message once we starting the bot.

2022-08-02 07:39:29,419 - 1466638 - hummingbot.core.utils.async_utils - ERROR - Unhandled error in background task: Error on POST https://localhost:21701/evm/allowances Error: {'message': 'The spender param is not a valid Ethereum address (0x followed by 40 hexidecimal characters).', 'httpErrorCode': 404, 'errorCode': -1, 'stack': 'Error: The spender param is not a valid Ethereum address (0x followed by 40 hexidecimal characters).\n    at throwIfErrorsExist (/usr/src/app/dist/src/services/validators.js:38:15)\n    at /usr/src/app/dist/src/services/validators.js:85:40\n    at /usr/src/app/dist/src/evm/evm.routes.js:32:61\n    at Generator.next (<anonymous>)\n    at /usr/src/app/dist/src/evm/evm.routes.js:8:71\n    at new Promise (<anonymous>)\n    at __awaiter (/usr/src/app/dist/src/evm/evm.routes.js:4:12)\n    at /usr/src/app/dist/src/evm/evm.routes.js:31:90\n    at /usr/src/app/dist/src/services/error-handler.js:37:28\n    at Layer.handle [as handle_request] (/usr/src/app/node_modules/express/lib/router/layer.js:95:5)'}
Traceback (most recent call last):
  File "/home/ubuntu/Hummingbot/pull_request/5422/hummingbot/core/utils/async_utils.py", line 9, in safe_wrapper
    return await c
  File "/home/ubuntu/Hummingbot/pull_request/5422/hummingbot/connector/gateway_EVM_AMM.py", line 290, in auto_approve
    await self.update_allowances()
  File "/home/ubuntu/Hummingbot/pull_request/5422/hummingbot/connector/gateway_EVM_AMM.py", line 340, in update_allowances
    self._allowances = await self.get_allowances()
  File "/home/ubuntu/Hummingbot/pull_request/5422/hummingbot/connector/gateway_EVM_AMM.py", line 348, in get_allowances
    resp: Dict[str, Any] = await self._get_gateway_instance().get_allowances(
  File "/home/ubuntu/Hummingbot/pull_request/5422/hummingbot/core/gateway/gateway_http_client.py", line 341, in get_allowances
    return await self.api_request("post", "evm/allowances", {
  File "/home/ubuntu/Hummingbot/pull_request/5422/hummingbot/core/gateway/gateway_http_client.py", line 194, in api_request
    raise e
  File "/home/ubuntu/Hummingbot/pull_request/5422/hummingbot/core/gateway/gateway_http_client.py", line 182, in api_request
    raise ValueError(f"Error on {method.upper()} {url} Error: {parsed_response}")

image

Steps to reproduce:

  1. Cloned Feature branch with no issues
  2. Manually created Docker image in gateway folder
  3. Generated-cert and gateway create with no issues
  4. Successfully connected Defira mainnet/testnet
  5. Configured Harmony NodeURL https://api.harmony.one
  6. Created simple AMM_Arbitrage strategy and started bot
  7. Observe log pane for error

Defira_conf_amm_arb_1.log

@navijation
Copy link
Contributor Author

@arnelhbot I see the problem. There are red herring validators for the Harmony chain that don't actually do anything. I edited the spender validator there instead of the actually used Ethereum validators. It is now fixed and I am able to do AMM_ARB with Defira against itself.

Screen Shot 2022-08-02 at 10 17 42 AM

arnelhbot
arnelhbot previously approved these changes Aug 3, 2022
Copy link
Contributor

@arnelhbot arnelhbot 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 Feature branch with no issues
  • Created Docker image in gateway folder
  • Generated-cert and gateway create with no issues
  • Successfully connected Defira mainnet/testnet
  • Created simple AMM_Arbitrage strategy and started bot
  • Confirmed arbitrage opportunity and filled order
  • Txhash visible in ledger
  • History confirmed displaying order filled

Copy link
Contributor

@arnelhbot arnelhbot 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 Feature branch with no issues
  • Created Docker image in gateway folder
  • Generated-cert and gateway create with no issues
  • Successfully connected Defira mainnet/testnet
  • DefiKingdoms visible as a connector
  • Created simple AMM_Arbitrage strategy and started bot
  • Confirmed arbitrage opportunity and filled order
  • Txhash visible in ledger
  • History confirmed displaying order filled

@JeremyKono JeremyKono merged commit 6a4f759 into hummingbot:development Aug 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

7 participants