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: implement jupiter as a swapper #8120

Merged
merged 147 commits into from
Dec 2, 2024
Merged

feat: implement jupiter as a swapper #8120

merged 147 commits into from
Dec 2, 2024

Conversation

NeOMakinG
Copy link
Collaborator

@NeOMakinG NeOMakinG commented Nov 13, 2024

Description

Implementing the Jupiter swapper:

  • We are delegating the fee estimate at quote time to jupiter as they are building the TX instructions, it's safer and more opti in term of execution time
  • We need to use the wrapped SOL assetId instead of the SOL, jupited is unwrapping and wrapping directly in their program
  • Added affiliate fees account and BPS
  • Selecting wrapped sol shouldn't be possible as Jupiter is using it as underlying asset, if we send the wrapped sol balance to the Jupiter API, it will consider we want to swap SOL (as a follow up, we could display somewhere on the wrapped sol asset page or swapper that the user can unwrap these wrapped sol by clicking on a button)
  • ⚠️ Waiting for the DAO fee address for affiliate

Issue (if applicable)

closes #8017

Risk

High because swapper but behind a feature flag

High Risk PRs Require 2 approvals

What protocols, transaction types, wallets or contract interactions might be affected by this PR?

Testing

  • Try to do a swap from SOL to any SPL token
  • Try to do a swap from SPL to SPL
  • Try to do a swap from SPL to SOL
  • Try all of those but with a manual receive address (SPL to SOL shouldn't provide a quote because it's not effectively supported by Jupiter (it leave some wSOL in the receive address which is not acceptable for us))
  • Please try to swap to fresh accounts, and send again so it already have an associated token account, it's 2 different mechanisms, also try to swap a token you never had in your wallet
  • Try to swap some Token2022 tokens too (catwifhat is one of those), specially with a manual receive address
  • Smoke test swapper but shouldn't change anything to the rest

Engineering

n/a

Operations

n/a

  • 🏁 My feature is behind a flag and doesn't require operations testing (yet)

Screenshots (if applicable)

SOL to SPL

image

SPL to SOL

image https://jam.dev/c/ce54f3bb-43f1-4e1f-9f44-c8b44a19fb73

SPL to SPL

⚠️ It was a high volatility SPL token, I had to increase the slippage tolerange
https://explorer.solana.com/tx/3jfq6B3z44Y7ffJxfVm2wRAKFJMba8fANUitHSguzmiKgb8NYfyRfWRHyqvokxHbRe8Qi5mUyuESoAuUEZzwTau5
image
https://jam.dev/c/16c8e5f0-d5e2-4530-865b-b324a6980e25

TX history

image image image

Txs links

https://explorer.solana.com/tx/23xAVVo18sgD2ykyqH5Kx9C37j6bLNjbFXi2rRW4ge5YArm1ywsTFGXKzSRmAQV6Dx13GnXZP5W7f1UNjmrAqDau
https://explorer.solana.com/tx/5k2r33Dd2oPKf5NESmQZz2G2fQ23wSy7xfL82PhCKwZuT5kojmynuHrVDy3uzhsf2UFq13wKjcvJMr61cdsGDGBL
https://explorer.solana.com/tx/2ucg29ZHH58uVZ9MCbE3VCCfwgHVdcozcxG4NfELDmTQTew4T6HQKh6E8eBx7Xr12K2xjZEEHpHDfdi1fQxg7Tyv
https://explorer.solana.com/tx/4XgfBYeqrn5pq2vTGmYrktYmgii1EiUXR7vZNzmULNNQgePMQVMKZsVWPbChURtUrYN9w8cD2aFteVtP1fCet8LV

Copy link
Contributor

@gomesalexandre gomesalexandre left a comment

Choose a reason for hiding this comment

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

Tested locally:

  • Unable to get rates without wallet connected (see inline comments for the reason why) 🚫
image

Wallet connected - no custom receive addy

  • Able to get SOL sends rates with wallet connected ✅
image
  • Able to get and execute SOL sell quote with wallet connected 🚫

https://jam.dev/c/bb637228-1a2d-4b72-af70-0d8644693849

Tx simulation is failing on Solana popup

image

And actual Tx is failing too

image
  • Able to get SOL buy rates with wallet connected ✅
image
  • Able to execute SOL buy quotes with wallet connected 🚫

Same failure as above

https://jam.dev/c/0bed7f93-1d0e-4ab6-b8e3-977ee8c81457

Custom receive buy addy

  • Able to get SOL sends rates with wallet connected ✅
image
  • Able to get and execute SOL sell quote with wallet connected 🚫

Managed to get unchained to return a Txid once (though never seemed to hit the mempool), and got the same error as above afterwards

https://jam.dev/c/b69e29f8-ea76-4635-ba8f-7049c2040ba7

  • Should be unable to get SOL buy rates with wallet connected ✅ (cross-account execution not available for SOL buys)
image image
  • Able to get and execute SOL buy quote with wallet connected 🚫

https://jam.dev/c/1c567ab2-28ff-4cb4-b00f-dbe5a0478b21

Misc

  • Able to get rates for Wrapped SOL ✅
image image image image

@NeOMakinG
Copy link
Collaborator Author

@gomesalexandre we should be good for another review/runtime test 🙏

Copy link
Member

@woodenfurniture woodenfurniture left a comment

Choose a reason for hiding this comment

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

Testing notes

Looks like some flakienss that needs to be addressed before I go deep on testing.

Unable to get SOL -> Wrapped SOL rate ❌

Unable to execute SOL -> USDC trade ❌

https://jam.dev/c/0ab18444-b0f6-4702-b481-2691a88e3a5f

Tried 4x more times, saw 3 failures as above and eventually got a trade to go through

Unable to execute USDC -> Wrapped SOL trade ❌

4 attempts:
https://jam.dev/c/2aa2cd51-dddf-4a86-b1d7-87ecb66e20f4

Successful USDC -> SOL trade ✅

https://jam.dev/c/771cc054-79de-4631-8a61-bee44ef61737

Copy link
Contributor

@gomesalexandre gomesalexandre left a comment

Choose a reason for hiding this comment

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

Can confirm the flakiness spotted by @woodenfurniture here: https://gist.github.com/gomesalexandre/bbf9f37e179b1ecc31e7505171ed08bb

Main takeaways of this testing:

  • SOL <-> Wrapped SOL rates are borked (and their execution would probably be too if we could get to this stage with current diff)
  • Execution in general is flaky - either with reverts during simulation as can be seen in Phantom, and/or with no reverts, but not hitting the mempool seemingly (FYI, getting Txids from unchained's /api/v1/send endpoint is a red herring, it doesn't mean the Tx actually hit the mempool, it may still not. It only means got a deterministic Txid, and the Tx itself may or may not have hit the mempool)
  • Our ws seems flaky, though not related to this PR per se

q1. WRT execution @NeOMakinG: are we sure that the current way of keeping the rate from rate and only adding additional props to it yields consistent successes?
To me, it seems like getting a final quote the usual way with the right compute units is the way, and getting said additional data at quote time while keeping the original rate data is a recipe for flakiness, but this may or may not be the correct way for Solana, not sure.
q2.: how much of a lift would it be to add auto-slippige here for Jupiter as a first swapper to ensure better reliability (and rates while at that), and do we want to do that in this PR to mitigate failures here?

@NeOMakinG
Copy link
Collaborator Author

  • I disabled explicitly swapping between wrappedSOL to any other asset and any asset to wrappedSOL as it's considered as SOL from the Jupiter point of view, I updated the PR description because we may need to provide an unwrap feature to our users as Jupiter does
  • Fixed all the last comments

⚠️ I can't reproduce the USDC => SOL and SOL => USDC bugs, I tried a couple of swaps and none of them was not picked by the mempool, all the swaps were successful with good rates

q1 @gomes : Also tried to wait around 40 minutes after fetching the quote, it did fail, so I decided to fetch the prices again in the getTradeQuote handler in [5780800] (5780800)

Can you please guys test again and tell me if you see any more flaky behaviors?

q2: From the Jupiter point of view, the list is not so big: Update to be acceptingAuto, Default, Custom, add a prod acceptAuto`, that should be enabled for the jupiter swapper
Ensure all mathematics done with slippage are handling the "auto" mode, specially for the jupiter swaper

But please, we should try this in a follow up if we really want to, not on this particular PR which starts to be big

Copy link
Collaborator Author

@NeOMakinG NeOMakinG left a comment

Choose a reason for hiding this comment

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

Answered all the comments

Copy link
Contributor

@gomesalexandre gomesalexandre left a comment

Choose a reason for hiding this comment

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

Stamping as I ended up with the simulation failed on Phantom / Tx not broadcasted to the mempool only once, and couldn't repro after (which is why there is none in this Jam) despite many attempts). E2E testing below:

https://jam.dev/c/4bae293f-4fb2-47eb-85ab-1b53d231642e

Still noticing some Txs rarely failing for a diff. reason, though increasing slippeedge seems to solve it, so not too concerned about this as we will fast-follow with auto-slippage (#8193)

https://explorer.solana.com/tx/ajy2v17CFGZiuXvwC8NvKZqDeeeqgYLtGww39aRXBREwH4Bo9o2ZDGz859Fwy4SmKwu5GrUFHhjd59p6UD9XAHq

@NeOMakinG if you feel like it as a fast-follow, the ETA as " ms" where x < 100 is still looking very odd to me, looks like it got lost in translation in previous review (https://gist.github.com/gomesalexandre/bbf9f37e179b1ecc31e7505171ed08bb) but would be nice to tackle as a one-liner fast-follow, or in this PR if you feel like it, to make it no ETA similar to most EVM swappers (see 0x and Portals in the screenshot below missing it altogether, since swap is done within a block i.e effectively instantly, vs. Jup showing some ETA in the realm of ms):

image image

@NeOMakinG
Copy link
Collaborator Author

Stamping as I ended up with the simulation failed on Phantom / Tx not broadcasted to the mempool only once, and couldn't repro after (which is why there is none in this Jam) despite many attempts). E2E testing below:

https://jam.dev/c/4bae293f-4fb2-47eb-85ab-1b53d231642e

Still noticing some Txs rarely failing for a diff. reason, though increasing slippeedge seems to solve it, so not too concerned about this as we will fast-follow with auto-slippage (#8193)

https://explorer.solana.com/tx/ajy2v17CFGZiuXvwC8NvKZqDeeeqgYLtGww39aRXBREwH4Bo9o2ZDGz859Fwy4SmKwu5GrUFHhjd59p6UD9XAHq

@NeOMakinG if you feel like it as a fast-follow, the ETA as " ms" where x < 100 is still looking very odd to me, looks like it got lost in translation in previous review (https://gist.github.com/gomesalexandre/bbf9f37e179b1ecc31e7505171ed08bb) but would be nice to tackle as a one-liner fast-follow, or in this PR if you feel like it, to make it no ETA similar to most EVM swappers (see 0x and Portals in the screenshot below missing it altogether, since swap is done within a block i.e effectively instantly, vs. Jup showing some ETA in the realm of ms):

image image

Removed the time for jupiter in 561cd08

Copy link
Member

@woodenfurniture woodenfurniture left a comment

Choose a reason for hiding this comment

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

Confirmed swaps are looking much more stable now.

https://jam.dev/c/0f249b0c-7c8d-47b1-9a67-060da1fb2e16
https://jam.dev/c/6e14d3dd-137f-4beb-8cec-b7994d786d5e

Lets get this in and enjoy the follow-ups when they come 🎉

@gomesalexandre gomesalexandre enabled auto-merge (squash) December 2, 2024 02:41
@gomesalexandre gomesalexandre merged commit 4ab323c into develop Dec 2, 2024
3 checks passed
@gomesalexandre gomesalexandre deleted the jup-swapper branch December 2, 2024 02:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Jupiter Swapper Integration
4 participants