Skip to content
This repository was archived by the owner on Oct 4, 2023. It is now read-only.

[PAY-456] Add sagas for exchanging SOL to AUDIO#1651

Closed
rickyrombo wants to merge 12 commits into
mainfrom
mjp-cb-onramp-sagas
Closed

[PAY-456] Add sagas for exchanging SOL to AUDIO#1651
rickyrombo wants to merge 12 commits into
mainfrom
mjp-cb-onramp-sagas

Conversation

@rickyrombo

Copy link
Copy Markdown
Contributor

Description

(don't worry, it's not actually as big as it looks - the 1k+ lines are from the package-lock.json)

Adds the sagas responsible for using the Jupiter Aggregator Core SDK to exchange a user's SOL for AUDIO. Uses the following actions:

  • ui/buy-audio/quote - get the quote to/from any pair of USDC (SPL), AUDIO or SOL
  • ui/buy-audio/exchange - exchange from SOL to AUDIO
  • ui/buy-audio/exchangeAfterBalanceChange - watches for a balance change in the user's wallet, then puts a ui/buy-audio/exchange action with the entire balance of the wallet less the min balance required for a swap

Dragons

Is there anything the reviewer should be on the lookout for? Are there any dangerous changes?

So, so many... ❗

  • Waiting for a balance change can time out!
  • Exchanges are limited to SOL => AUDIO at the moment, and include another transaction for transferring the result into the userbank

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide repro instructions & any configuration.

Tested using some prototype UI w/ Coinbase Pay and the Redux dev tools. It works most of the time, though lately it isn't working due to potentially some state issue with my user wallet.

How will this change be monitored?

For features that are critical or could fail silently please describe the monitoring/alerting being added.

Feature Flags

Are all new features properly feature flagged? Describe added feature flags.

Completely uncalled from anywhere in code. Can add feature flags as an extra safety if wanted?

inputUiAmount?: string
outputUiAmount?: string
quoteStatus: QuoteStatus
exchangeStatus: ExchangeStatus

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

arguably we'll only ever support an exchange from SOL => AUDIO, so this could potentially be a top level status and swaps can be quotes

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

did this. lmk what think

return `0x${addr.substring(2, 4)}...${addr.substr(addr.length - 5)}`
}

export const convertJSBIToUiString = (amount: JSBI, decimals: number) => {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

this JSBI thing is a new one. I think it belongs here, but let me know if not

Comment thread packages/web/src/store/application/ui/buy-audio/sagas.ts Outdated
@@ -0,0 +1,40 @@
export type JupiterTokenListing = {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I might add a comment that these are copy/pasted from the results of getting the token listens via the Jupiter API. I figured they're not going to change, why query for them?

@audius-infra

Copy link
Copy Markdown
Collaborator

Preview this change https://demo.audius.co/mjp-cb-onramp-sagas

@dylanjeffers dylanjeffers left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

wow really amazing job, prob makes sense for another monetization dev to review

Comment thread packages/web/src/common/store/buy-audio/slice.ts Outdated
Comment thread packages/web/src/common/store/buy-audio/slice.ts Outdated
Comment thread packages/web/src/common/store/buy-audio/slice.ts Outdated
state.swaps[inputTokenSymbol][outputTokenSymbol].exchangeStatus =
ExchangeStatus.WAITING
},
exchange: (

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

i think this is fine, but would it be helpful to know what we are exchanging? seems like... exchanging tokens? i guess exchanging is fine, but yeah just thinkin

state.swaps[inputTokenSymbol][outputTokenSymbol].exchangeStatus =
ExchangeStatus.FAILED
},
quote: (

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

getQuote?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

RE: exchange and quote, I left these as verbs intentionally to differentiate from selectors or fetchers, and to indicate that they're more like commands than actions

Comment thread packages/web/src/store/application/ui/buy-audio/sagas.ts Outdated
Comment thread packages/web/src/store/application/ui/buy-audio/sagas.ts
Comment thread packages/web/src/store/application/ui/buy-audio/sagas.ts
Comment thread packages/web/src/store/application/ui/buy-audio/sagas.ts Outdated

export default function sagas() {
return [watchQuote, watchExchange, watchExchangeAfterBalanceChange]
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

wildness, awesome job!

@audius-infra

Copy link
Copy Markdown
Collaborator

Preview this change https://demo.audius.co/mjp-cb-onramp-sagas

@audius-infra

Copy link
Copy Markdown
Collaborator

Preview this change https://demo.audius.co/mjp-cb-onramp-sagas

@audius-infra

Copy link
Copy Markdown
Collaborator

Preview this change https://demo.audius.co/mjp-cb-onramp-sagas

@sddioulde sddioulde left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

looks great and pristine!

Comment thread packages/web/package.json Outdated
"@fingerprintjs/fingerprintjs-pro": "3.5.6",
"@hcaptcha/react-hcaptcha": "0.3.6",
"@juggle/resize-observer": "^3.3.1",
"@jup-ag/core": "^2.0.0-beta.3",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

should we drop the caret and use the exact version?

Comment thread packages/web/package.json Outdated
"@reduxjs/toolkit": "1.3.4",
"@sentry/browser": "6.16.1",
"@sentry/integrations": "6.16.1",
"@solana/spl-token": "^0.1.6",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

same comment as above regarding version

transactionHandler
})
const account = yield* select(getAccountUser)
if (!account?.userBank) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

maybe do this check in the first few lines of the function so we don't have to init TransactionHandler or call doSwap

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

very good catch

@piazzatron piazzatron left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

sickkkk. only comment is that it might be nice to hide Jupiter behind an interace (i.e. SwapProvider, or even SwapProvider & QuoteProvider) so we're not tightly bound to this particular swap/quote implementation

return `0x${addr.substring(2, 4)}...${addr.substr(addr.length - 5)}`
}

export const convertJSBIToUiString = (amount: JSBI, decimals: number) => {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Maybe a comment about what this is / why we need it?

* @returns the result of the transaction handler handleTransaction call
*/
function* sendTransaction(
name: string,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: would be nice to have object args for this guy

@audius-infra

Copy link
Copy Markdown
Collaborator

Preview this change https://demo.audius.co/mjp-cb-onramp-sagas

@rickyrombo rickyrombo mentioned this pull request Aug 2, 2022
2 tasks
@rickyrombo

Copy link
Copy Markdown
Contributor Author

Superceded by #1666

@rickyrombo rickyrombo closed this Aug 16, 2022
@dylanjeffers dylanjeffers deleted the mjp-cb-onramp-sagas branch August 25, 2022 23:43
@AudiusProject AudiusProject deleted a comment from linear Bot Sep 11, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants