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.

Sign up for GitHub

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

Jump to bottom

Skip collateral factor check #1515

Merged
merged 4 commits into from
Oct 25, 2022
Merged

Skip collateral factor check #1515

merged 4 commits into from
Oct 25, 2022

Conversation

shohamc1
Copy link
Contributor

What kind of PR is this?:

/kind feature

Which issue(s) does this PR fixes?:

Fixes #1509

Additional comments?:

Supersedes #1473

@fuxingloh
Copy link
Contributor

Adding myself to the approval list, I will checkout this branch and test it.

@fuxingloh fuxingloh self-requested a review October 18, 2022 06:35
fuxingloh added a commit to BirthdayResearch/jellyfishsdk that referenced this pull request Oct 18, 2022
@fuxingloh
chore(docker): bump to use HEAD-aaaec97e8 from DeFiCh/ain#1515
cf7ebee
@fuxingloh fuxingloh mentioned this pull request Oct 18, 2022
Merged
@fuxingloh
Copy link
Contributor

@shohamc1 do you mind taking a look at this test https://github.com/JellyfishSDK/jellyfish/actions/runs/3271588607/jobs/5381935157

I think a recent change to the master branch might have affected it. Is it an issue?

@shohamc1
Copy link
Contributor Author

@fuxingloh they all seem to be related to changes in #1481. I believe the tests need to be updated.

@fuxingloh
Copy link
Contributor

@fuxingloh they all seem to be related to changes in #1481. I believe the tests need to be updated.

They're safe to update?

Given that it was a drastically different outcome for expected and received. Do you have better insights into this? Do you mind sharing?

Expected substring: "Price is higher than indicated"
Received message:   "RpcApiError: 'Cannot find usable pool pair.', code: -32600, method: testpoolswap"

Expected substring: "Lack of liquidity"
Received message:   "RpcApiError: 'Cannot find usable pool pair.', code: -32600, method: testpoolswap"

@shohamc1 shohamc1 mentioned this pull request Oct 18, 2022
Merged
fuxingloh
fuxingloh approved these changes Oct 19, 2022
View reviewed changes
Copy link
Contributor

@fuxingloh fuxingloh left a comment

Choose a reason for hiding this comment

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

LGTM

Tested on BirthdayResearch/jellyfishsdk#1814 with all test passing.

fuxingloh added a commit to BirthdayResearch/jellyfishsdk that referenced this pull request Oct 19, 2022
@fuxingloh
chore(docker): bump to use HEAD-49fba65ce from DeFiCh/ain#1515 (#1814)
cb8aee7 
#### What this PR does / why we need it:

As per the title.

Bump to use HEAD-49fba65ce from DeFiCh/ain#1515
to address issue from DeFiCh/ain#1509
@prasannavl prasannavl merged commit 812d3df into master Oct 25, 2022
@prasannavl prasannavl deleted the feature/skip-collateral-factor-check branch October 25, 2022 08:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dcorral dcorral dcorral approved these changes

@Bushstar Bushstar Bushstar approved these changes

@fuxingloh fuxingloh fuxingloh approved these changes

@Mixa84 Mixa84 Awaiting requested review from Mixa84

@wafflespeanut wafflespeanut Awaiting requested review from wafflespeanut

@prasannavl prasannavl Awaiting requested review from prasannavl

@mambisi mambisi Awaiting requested review from mambisi

@Jouzo Jouzo Awaiting requested review from Jouzo

Assignees

@shohamc1 shohamc1

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

skip collateral_factor check when -jellyfish_regtest args are present
6 participants
@shohamc1 @fuxingloh @Bushstar @dcorral @prasannavl @defichain-bot