-
-
Notifications
You must be signed in to change notification settings - Fork 246
feat/ upgrade traderjoe #131
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/ upgrade traderjoe #131
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tests performed:
- Cloned and install feature branch and Client on development
- Set up gateway and reviewed its ONLINE
- successfully connected traderjoe
- review client shows correct balance
- created/started amm_Arb strategy using traderjoe avalanche
- Review client shows profitability rates
- Set low profitability and reviewed order execution successful
- Tests Curl: allowances/price/trade etc. return expected responses
david-hummingbot
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Docker Ubuntu 20.04 (amd64)
- build docker image locally successful
- run client and connect traderjoe ok
- balance command works
- run amm_arb strategy ok
- run curl tests ok
fengtality
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See question about MAX_HOPS
| import { createPublicClient, http } from 'viem'; | ||
| import { avalanche, avalancheFuji } from 'viem/chains'; | ||
|
|
||
| const MAX_HOPS = 2; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would users need to modify this parameter? if so, I think it should be a param in traderjoe.yml
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not a parameter regular users should modify in this case. Changing that value without adding more path tokens would have no effect on the final route. It would be a different case if there is an auto/smart router.
Before submitting this PR, please make sure:
A description of the changes proposed in the pull request:
This pr upgrade TraderJoe to use sdk V2.
Tests performed by the developer:
Tips for QA testing: