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

testpoolswap RPC should check for a direct swap first when auto is specified. #1114

Closed
surangap opened this issue Feb 22, 2022 · 3 comments · Fixed by #1115
Closed

testpoolswap RPC should check for a direct swap first when auto is specified. #1114

surangap opened this issue Feb 22, 2022 · 3 comments · Fixed by #1115

Comments

@surangap
Copy link
Contributor

What happened:

  1. add a CAT-DFI pool.
  2. call testpoolswap from CAT -> DFI with path = auto, verbose = true
  3. response have as follows. note pools field.
    { path: 'auto', pools: [ '2', '2', '2' ], amount: '48.48437068@0' }

The above should be a direct swap since CAT-DFI pool is present.
RCA
even in compositeswap check for a direct swap is done first and then goes to the CalculateSwaps() and compositeswap()
ref ->

if (!pcustomcsview->GetPoolPair(poolSwapMsg.idTokenFrom, poolSwapMsg.idTokenTo)) {

but in the scenario mentioned above, it will call CalculateSwaps() straight. ref ->

poolIds = compositeSwap.CalculateSwaps(mnview_dummy, true);

it could be that the CalculateSwaps() is unable to handle this scenario well.
But simply adding a check for a direct swap first should solve the issue(same as the compositeswap flow. ).

poolIds = compositeSwap.CalculateSwaps(mnview_dummy, true);

What you expected to happen:

How to reproduce it (as minimally and precisely as possible):

What are your environment parameters:

Anything else we need to know?:

@defichain-bot
Copy link
Member

@surangap: Thanks for opening an issue, it is currently awaiting triage.

The triage/accepted label can be added by foundation members by writing /triage accepted in a comment.

Details

I am a bot created to help the DeFiCh developers manage community feedback and contributions. You can check out my manifest file to understand my behavior and what I can do. If you want to use this for your project, you can check out the DeFiCh/oss-governance-bot repository.

@surangap
Copy link
Contributor Author

relates to BirthdayResearch/jellyfishsdk#1074

@eli-lim
Copy link

eli-lim commented Feb 22, 2022

It seems like CPoolSwap::CalculateSwaps / CalculatePoolPaths doesn't look for a direct path.

Just curious, which of the following are we looking to achieve for testpoolswap?

  • Always use direct path if available
  • Use best path (implying some composite path may yield an amount greater than direct - assuming that's even possible)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants