-
-
Notifications
You must be signed in to change notification settings - Fork 142
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/XRPL connector v2 #128
Feat/XRPL connector v2 #128
Conversation
Steps:
Actual:
|
@mlguys Could you please add any updates on this PR? |
@mlguys Could you please resolve branch conflicts? |
hi @mlguys. |
Hey @nikspz, sorry for being unresponsive. I've been busy fixing the order tracker logic since I found some bugs during making integration tests. Fortunately I have resolved the logic issue and pushed integration tests which should cover more than 75% cov of connector code. I will now proceed to fix the bug you mentioned above and make sure this PR is conflict free. |
Hey guys, could you re-run the unit tests again? I have pushed fixes to address the test errors. |
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 prelim comments
|
Hey @nikspz, thanks for testing, currently I'm busy with moving to new home so I'm unable to work on this yet. Will resolve this next week. |
hi @mlguys Please give updates on this bounty when available |
@mlguys if you have any updates please post here |
Hey @nikspz, I'm still working on this but due to tight schedule with other connector and limited work hours during my moving to another living space, I'm unable to finish this yet. However, I get more time for this week so I will push this, hopefully will be able to push some fixes by today. |
hi @mlguys Could you please provide updates and fix branch conflicts? |
Hey @nikspz, I've resolved the conflicts and added fix to your issue, could you check it? |
|
Hey @nikspz, Thanks for testing. I've added a fix on hummingbot/hummingbot#6535 to fix your issue, could you check? |
Steps to reproduce:
Actual:
|
hi @mlguys, kindly Please add your updates here |
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.
LGTM
- Ran tests with the Feat/XRPL connector v2 hummingbot#6535
- Most of tests are done on testnet
- Added orders on orderbook using account provided by dev to simulate a real exchange scenario
- Added xrpl wallet successfully and balance display ok
- Setup simple PMM strategy and observe connector:
- Create, cancel and fetch orders ok (failed fetch update happens but thats due to nature of exchange)
- Setup multiple order levels, all ok
- Filled order events ok
Note: Current version does have issues with balance not updated orders are created on testnet, a separate issue will be created as part of improvement for the connector. Mainnet has not been tested.
LGTM! |
Merged to development and part of release version 1.23.0 |
Before submitting this PR, please make sure:
A description of the changes proposed in the pull request:
CLOBish
classTests performed by the developer:
USD/VND
andXRP/VND
on testnetTips for QA testing:
USD/VND
andXRP/VND
on XRP DEX on testnet:Tests added:
test/chains/xrpl
test/connectors/xrpl
Test coverage:
Passed all tests on 22 of August using this command:
Connector endpoints coverage:
Chain endpoints coverage: