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

Swaps deeplink #6178

Merged
merged 15 commits into from
Dec 4, 2024
Merged

Swaps deeplink #6178

merged 15 commits into from
Dec 4, 2024

Conversation

greg-schrammel
Copy link
Contributor

@greg-schrammel greg-schrammel commented Oct 8, 2024

Fixes APP-1234

What changed (plus any additional context for devs)

inputAsset: UniqueId
outputAsset: UniqueId

inputAmount: Number
outputAmount: Number
percentageToSell: Number // value between 0 and 1

gasSpeed: 'normal' | 'fast' | 'urgent'
slippage: Number
flashbots: Boolean

from: Address

with assets

  • inputAsset and outputAsset expects a UniqueId (address_chainId)
    ex: rainbow://swaps?inputAsset=0xa0b86991c6218b36c1d19d4a2e9eb0ce3606eb48_1

with prefilled values

  • percentageToSell > inputAmount > outputAmount only one is used, in this order of importance
  • percentageToSell expects a value between 0 and 1
    ex: rainbow://swaps?inputAsset=eth_1&percentageToSell=0.2, 20% of wallet swappable ETH

with gasSpeed

  • values are normal,fast or urgent
    ex: rainbow://swaps?gasSpeed=urgent

with flashbots

  • param not included does not mean false, keeps user preferences
  • flashbots=true any other value besides true means false
    ex: rainbow://swaps?flashbots=true

with slipagge

  • in bips slippage=100 = 1% slippage
    ex: rainbow://swaps?slippage=100

with from

  • let you navigate to another wallet, param is silently ignored if you don't own the wallet
    ex: rainbow://swaps?from=0x507F0daA42b215273B8a063B092ff3b6d27767aF

Screen recordings / screenshots

What to test

  • multiple weird combinations of params
  • with app open and app closed

to test in ios you can run xcrun simctl openurl booted 'rainbow://swap/?outputAsset=0xa0b86991c6218b36c1d19d4a2e9eb0ce3606eb48_1&gasSpeed=urgent&flashbots=true&inputAmount=1&inputAsset=eth_1'

branch is blocking some vpns, when trying to open deeplinks while connected to a vpn it would fail to connect to branch servers and return an empty params object, not undefined, empty `params: {}`, in turn making the app ignore the deeplink and not open it
Copy link

linear bot commented Oct 8, 2024

@brunobar79
Copy link
Member

Launch in simulator or device for afa7d2e

Copy link
Contributor

@walmat walmat left a comment

Choose a reason for hiding this comment

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

Mostly comments that I need clarity on, a few actual changes that I think could make the flow even better:

  1. parallelize some of the network requests
  2. safeguards against division by 0, undefined cases, etc.
  3. make sure the default 50% slider position if no inputAmount is defined hasn't regressed.

Otherwise, great work!

ios/Rainbow.xcodeproj/project.pbxproj Outdated Show resolved Hide resolved
src/handlers/deeplinks.ts Outdated Show resolved Hide resolved
src/handlers/deeplinks.ts Outdated Show resolved Hide resolved
src/handlers/deeplinks.ts Outdated Show resolved Hide resolved
src/handlers/deeplinks.ts Outdated Show resolved Hide resolved
src/handlers/deeplinks.ts Outdated Show resolved Hide resolved
src/state/swaps/swapsStore.ts Show resolved Hide resolved
Comment on lines 596 to 599
if (internalSelectedInputAsset.value && internalSelectedOutputAsset.value) {
fetchQuoteAndAssetPrices();
}

Copy link
Contributor

Choose a reason for hiding this comment

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

i don't think this is necessary since it should be handled via a useAnimatedReaction.

@@ -657,6 +691,7 @@ export function useSwapInputsController({
assetToSellNetwork: internalSelectedInputAsset.value?.chainId,
}),
(current, previous) => {
if (!previous) return;
Copy link
Contributor

Choose a reason for hiding this comment

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

why was this added?

Copy link
Contributor

Choose a reason for hiding this comment

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

still curious about this

Copy link
Member

Choose a reason for hiding this comment

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

+1

@brunobar79
Copy link
Member

Launch in simulator or device for d51e895

@brunobar79
Copy link
Member

Launch in simulator or device for 7e95ffb

@derHowie
Copy link
Member

hey @greg-schrammel, did we end up getting blocked / deprioritizing this?

@greg-schrammel
Copy link
Contributor Author

greg-schrammel commented Nov 25, 2024

hey @greg-schrammel, did we end up getting blocked / deprioritizing this?

no I moved to other stuff and forgot about it, just fixed the conflicts

@brunobar79
Copy link
Member

Launch in simulator or device for 4c3d7cf

@brunobar79
Copy link
Member

Launch in simulator or device for dc3803c

@brunobar79 brunobar79 added release for release blockers and release candidate branches and removed nice for release labels Dec 2, 2024
@brunobar79 brunobar79 requested review from derHowie and removed request for benisgold December 2, 2024 18:29
@@ -657,6 +691,7 @@ export function useSwapInputsController({
assetToSellNetwork: internalSelectedInputAsset.value?.chainId,
}),
(current, previous) => {
if (!previous) return;
Copy link
Member

Choose a reason for hiding this comment

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

+1

Comment on lines +40 to +55
export function getSwapsNavigationParams() {
const params = (Navigation.getActiveRoute().params || {}) as SwapsParams;

const inputMethod = getInputMethod(params);
const lastTypedInput = inputMethod === 'slider' ? 'inputAmount' : inputMethod;

const state = useSwapsStore.getState();
return {
inputMethod,
lastTypedInput,
focusedInput: lastTypedInput,
inputAsset: params.inputAsset || state.inputAsset,
outputAsset: params.outputAsset || state.outputAsset,
...params,
} as const;
}
Copy link
Member

Choose a reason for hiding this comment

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

consumers of this function will think getSwapsNavigationParams().inputAsset and getSwapsNavigationParams().outputAsset are guaranteed.

can you tweak this fn to reflect that they're optional?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

they are typed as ParsedSearchAsset | null

src/handlers/deeplinks.ts Outdated Show resolved Hide resolved
src/handlers/deeplinks.ts Outdated Show resolved Hide resolved
@brunobar79
Copy link
Member

Launch in simulator or device for 60f5eff

@greg-schrammel greg-schrammel merged commit 03bfb03 into develop Dec 4, 2024
8 checks passed
@greg-schrammel greg-schrammel deleted the swaps-deeplink branch December 4, 2024 21:50
benisgold added a commit that referenced this pull request Dec 5, 2024
* Browser upgrades (#6269)

* Dapp browser upgrades

* Remove unneeded whitelisted native text prop

* Clean up SwipeNavigator tab bar components

* Remove BPA wrapper from GestureHandlerButton

Also adds turbo-haptics and an internal scale animation, giving GestureHandlerButton full platform parity

* freezeStore → navigationStore

* Restructure browser gesture handlers, code cleanup

* Make useKeyboardHeight more efficient

* TextShadow: Add support for AnimatedText

* Remove commented code

* Fix lint error

* Remove commented code

* Force a new WebView when the content process dies

Reloading the page by calling reload() on the ref doesn't successfully restart the WebView.

This was causing tabs to occasionally become blank after the app sat backgrounded for long enough, and upon returning to the app they'd be stuck.

This change forces a new WebView by providing a new key, which should resolve the issue.

* Minor gesture cleanup

* Add common gesture options to GestureHandlerButton

* Bump packages

* Fix manual activation bug in RNGH

Fixes a bug that exists with manually activated gestures where when the app returns to the foreground, the first interaction with the manually activated handler doesn't trigger any events

* Fix type issue in react-native-view-shot

RN was complaining that this iOS code was not Android compliant

* Only render WebViewBorder on iOS, misc. cleanup

* Adjust ScrollView/tabs hierarchy, props cleanup

* Bump Skia

* Remove activeTabInfo.value.tabId

No longer needed as there's a dedicated activeTabId Shared Value

* Make tab switching lighter, add getTabInfo helper

A few tab switching styles were applied for all tabs when tab switching, when only the active tab and the tabs to the left and right need full-size tab styles

* Smooth out height animations

* Clean up scripts, clamp tab height

* Clean up WebView handlers

* Improve tab animation performance

- Removed unnecessary isSwitchingTabs (`tabViewGestureState` serves the same purpose)

- Removed a wrapper view around each tab and merged most animated tab styles into `animatedWebViewStyle`

- In `animatedWebViewStyle`, now passing values to the style generation worklets instead of passing the Shared Values themselves

* Make screenshot flicker less likely, improve tab switching reset

- Now setting the JS-side active tab index first so it gets a tiny head start (the JS and SV active indexes inform which tab activates and when tab screenshots are shown/hidden)

- Now resetting tab switching state *before* updating `activeTabId` and `animatedActiveTabIndex` (instead of after) — this fixes an edge case where the switch animation would occasionally be skipped, if you switched tabs again at the exact moment a previous tab switch ended

* Make isNewIndexValid a boolean

* Fix bad worklet pattern

Because getTabInfo is stable and the SVs used within getTabInfo weren't visible to the useAnimatedStyle hooks, styles weren't recomputing for changes in getTabInfo's input values — same idea applies to useDerivedValue, useAnimatedReaction prepare functions, etc.

* revert third party positions enabled (#6298)

* chain badges

* Swaps deeplink (#6178)

* fix branch

branch is blocking some vpns, when trying to open deeplinks while connected to a vpn it would fail to connect to branch servers and return an empty params object, not undefined, empty `params: {}`, in turn making the app ignore the deeplink and not open it

* aaa

* aaaaaa

* remove unused imports

* fix type

* ops

* better validations

* opsi inverted the conditions

* revert pbxproj changes

* setFromWallet

* fix typo & code style

* remove uncessary stuff

---------

Co-authored-by: Matthew Wall <[email protected]>

* add eth rewards to claimables and make it discoverable by watchers

---------

Co-authored-by: Christian Baroni <[email protected]>
Co-authored-by: Kane Thomas <[email protected]>
Co-authored-by: gregs <[email protected]>
Co-authored-by: Matthew Wall <[email protected]>
benisgold added a commit that referenced this pull request Dec 10, 2024
* progress

* progress

* progress

* testing

* progress

* progress

* crosschain quote working

* claim + crosschainswap rap worked

* raps logic is done

* delete v2

* cleanup

* working on dropdowns

* little dropdown refactor

* useDropdownMenu hook

* refactoring

* working on dropdown

* basically done with ui refactor

* fixes

* claim as x is working

* context work

* progress

* gas

* enormous refactor, need to fix some things

* gas fixes

* pretty sure i fixed quotes

* functional

* wrapping things up

* cleaning up

* fixes

* more fixes

* fix formatting

* lint

* gas and quote state fixes

* rm logs

* error messages

* error messages + i18n

* fix bug that allowed button press w/o token/chain selected

* disable watched wallets

* error logging in executeClaim

* i18n

* slippage

* add comments to transaction context

* comments

* rainbow fee

* adjust rainbow fee

* gas improvements

* forgot to undo this

* fix gas race condition

* fix swap data assetToSell bug

* rm comment

* improve error logging in raps

* nit

* temp, merging develop

* fix gas estimation

* rm unused imports

* rm console log

* rm dai, wbtc, add usdc

* fix quote amount + fix gas estimation throttling

* rename txState to gasState

* more gas fixes

* network icons for dropdown menu

* fix populateSwap util and some other minor things

* fix analyticsId

* fix scenario where user does not have native asset for gas calculations

* fix context menu ordering on android

* fix gas estimation

* undo obsolete change

* fix analytics

* Claimables followups (#6301)

* Browser upgrades (#6269)

* Dapp browser upgrades

* Remove unneeded whitelisted native text prop

* Clean up SwipeNavigator tab bar components

* Remove BPA wrapper from GestureHandlerButton

Also adds turbo-haptics and an internal scale animation, giving GestureHandlerButton full platform parity

* freezeStore → navigationStore

* Restructure browser gesture handlers, code cleanup

* Make useKeyboardHeight more efficient

* TextShadow: Add support for AnimatedText

* Remove commented code

* Fix lint error

* Remove commented code

* Force a new WebView when the content process dies

Reloading the page by calling reload() on the ref doesn't successfully restart the WebView.

This was causing tabs to occasionally become blank after the app sat backgrounded for long enough, and upon returning to the app they'd be stuck.

This change forces a new WebView by providing a new key, which should resolve the issue.

* Minor gesture cleanup

* Add common gesture options to GestureHandlerButton

* Bump packages

* Fix manual activation bug in RNGH

Fixes a bug that exists with manually activated gestures where when the app returns to the foreground, the first interaction with the manually activated handler doesn't trigger any events

* Fix type issue in react-native-view-shot

RN was complaining that this iOS code was not Android compliant

* Only render WebViewBorder on iOS, misc. cleanup

* Adjust ScrollView/tabs hierarchy, props cleanup

* Bump Skia

* Remove activeTabInfo.value.tabId

No longer needed as there's a dedicated activeTabId Shared Value

* Make tab switching lighter, add getTabInfo helper

A few tab switching styles were applied for all tabs when tab switching, when only the active tab and the tabs to the left and right need full-size tab styles

* Smooth out height animations

* Clean up scripts, clamp tab height

* Clean up WebView handlers

* Improve tab animation performance

- Removed unnecessary isSwitchingTabs (`tabViewGestureState` serves the same purpose)

- Removed a wrapper view around each tab and merged most animated tab styles into `animatedWebViewStyle`

- In `animatedWebViewStyle`, now passing values to the style generation worklets instead of passing the Shared Values themselves

* Make screenshot flicker less likely, improve tab switching reset

- Now setting the JS-side active tab index first so it gets a tiny head start (the JS and SV active indexes inform which tab activates and when tab screenshots are shown/hidden)

- Now resetting tab switching state *before* updating `activeTabId` and `animatedActiveTabIndex` (instead of after) — this fixes an edge case where the switch animation would occasionally be skipped, if you switched tabs again at the exact moment a previous tab switch ended

* Make isNewIndexValid a boolean

* Fix bad worklet pattern

Because getTabInfo is stable and the SVs used within getTabInfo weren't visible to the useAnimatedStyle hooks, styles weren't recomputing for changes in getTabInfo's input values — same idea applies to useDerivedValue, useAnimatedReaction prepare functions, etc.

* revert third party positions enabled (#6298)

* chain badges

* Swaps deeplink (#6178)

* fix branch

branch is blocking some vpns, when trying to open deeplinks while connected to a vpn it would fail to connect to branch servers and return an empty params object, not undefined, empty `params: {}`, in turn making the app ignore the deeplink and not open it

* aaa

* aaaaaa

* remove unused imports

* fix type

* ops

* better validations

* opsi inverted the conditions

* revert pbxproj changes

* setFromWallet

* fix typo & code style

* remove uncessary stuff

---------

Co-authored-by: Matthew Wall <[email protected]>

* add eth rewards to claimables and make it discoverable by watchers

---------

Co-authored-by: Christian Baroni <[email protected]>
Co-authored-by: Kane Thomas <[email protected]>
Co-authored-by: gregs <[email protected]>
Co-authored-by: Matthew Wall <[email protected]>

* fix hook usage

* lint

* wallet screen fix

* dont throw errors in rap creation

* remove claimable asset from dropdown if network is not compatible

* rm unnecessary useCallback dep

* show wallet error alert if can't load wallet

* fix approval target

---------

Co-authored-by: Christian Baroni <[email protected]>
Co-authored-by: Kane Thomas <[email protected]>
Co-authored-by: gregs <[email protected]>
Co-authored-by: Matthew Wall <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release for release blockers and release candidate branches
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants