feat(earn): Add enter amount screen #5399

finnian0826 · 2024-05-07T23:00:09Z

Description

Add EarnEnterAmount screen.

Test plan

Unit tests added.

Manual tests:

When a user has USDC on arbitrum:

enough.mp4

When a user doesn't have enough USDC on arbitrum:

not-enough.mp4

Clicking the info button:

info.mp4

Related issues

Fixes #ACT-1176

Backwards compatibility

Yes

Network scalability

If a new NetworkId and/or Network are added in the future, the changes in this PR will:

Continue to work without code changes, OR trigger a compilation error (guaranteeing we find it when a new network is added)

codecov · 2024-05-07T23:09:15Z

Codecov Report

Attention: Patch coverage is 80.63063% with 43 lines in your changes are missing coverage. Please review.

Project coverage is 86.29%. Comparing base (85a9d0c) to head (b1f5a6d).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5399      +/-   ##
==========================================
- Coverage   86.35%   86.29%   -0.06%     
==========================================
  Files         755      756       +1     
  Lines       30942    31162     +220     
  Branches     5281     5338      +57     
==========================================
+ Hits        26719    26891     +172     
- Misses       3996     4039      +43     
- Partials      227      232       +5

Files	Coverage Δ
src/analytics/Events.tsx	`100.00% <100.00%> (ø)`
src/analytics/Properties.tsx	`100.00% <ø> (ø)`
src/components/TokenIcon.tsx	`100.00% <100.00%> (ø)`
src/earn/EarnCard.tsx	`95.65% <100.00%> (ø)`
src/earn/EarnCta.tsx	`100.00% <100.00%> (ø)`
src/navigator/Screens.tsx	`100.00% <100.00%> (ø)`
src/send/EnterAmount.tsx	`96.22% <100.00%> (+0.01%)`	⬆️
src/statsig/constants.ts	`100.00% <ø> (ø)`
src/earn/prepareTransactions.ts	`89.33% <50.00%> (-6.06%)`	⬇️
src/earn/EarnEnterAmount.tsx	`81.00% <81.00%> (ø)`

... and 2 files with indirect coverage changes

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 85a9d0c...b1f5a6d. Read the comment docs.

finnian0826 · 2024-05-15T22:04:49Z

src/send/EnterAmount.tsx

-    disableProceed || (disableBalanceCheck ? !!tokenAmount?.isZero() : !transactionIsPossible)
+    disableProceed ||
+    (disableBalanceCheck
+      ? !!tokenAmount?.isZero() || (!!tokenAmount?.lte(token.balance) && !transactionIsPossible)


We want to disable the continue button when checking if the prepared transaction is possible, even if we are disabling the warnings.

this is getting super hard to read, either consider breaking this up or leave a comment explaining the different scenarios

finnian0826 · 2024-05-15T22:05:48Z

src/send/EnterAmount.tsx

@@ -441,14 +465,13 @@ function EnterAmount({
          />
        )}

-        {children}


With children here, the bottom sheets were acting super weird. Need to go through and double check that this doesn't mess anything else up.

Pretty sure this will affect the InLineNotification in the jumpstart screen. I guess one option would be to pass bottomsheets in a separate prop.. Not ideal but should work for now

finnian0826 · 2024-05-15T22:07:46Z

locales/base/translation.json

+      "info": "This pool is powered by Aave",
+      "infoBottomSheet": {
+        "title": "Why this pool?",
+        "description": "This Aave pool has over $150M TVL and more than 6,000 contributors, indicating strong liquidity and trustworthiness. It also uses USDC as its token and Abritrum as its network, which reduces volatility and ensures users will receive inexpensive gas rates.\n\nAll together this makes it a safe and attractive option with competitive APY, ensuring a beneficial investment opportunity for our users.\n\nYou can explore other Aave pools here.",


The "here" is supposed to be a link, but I'm not sure what it is supposed to link to. Also, this is used as the description param for a bottom sheet so I'm not sure how to format links for translations when it is passed as a param.

maybe something like https://app.aave.com/markets/? Would be good to check with product. And re description, can we render this as a content inside the bottom sheet instead of using the existing prop?

Confirmed with John that that link is good so I'll add it in

satish-ravi

some padding looks off

satish-ravi · 2024-05-15T22:42:02Z

src/analytics/Properties.tsx

@@ -1583,6 +1583,8 @@ interface EarnEventsProperties {
  [EarnEvents.earn_deposit_complete]: undefined
  [EarnEvents.earn_deposit_cancel]: undefined
  [EarnEvents.earn_view_pools_press]: undefined
+  [EarnEvents.earn_enter_amount_info_press]: undefined
+  [EarnEvents.earn_enter_amount_continue_press]: { userHasFunds: boolean }


would be good to include some additional props here like the amount the user entered, tokenId etc.

satish-ravi · 2024-05-15T22:55:38Z

src/send/EnterAmount.tsx

@@ -441,14 +465,13 @@ function EnterAmount({
          />
        )}

-        {children}


Pretty sure this will affect the InLineNotification in the jumpstart screen. I guess one option would be to pass bottomsheets in a separate prop.. Not ideal but should work for now

satish-ravi · 2024-05-15T23:02:54Z

src/send/EnterAmount.tsx

  disableProceed?: boolean
  children?: React.ReactNode
  ProceedComponent: ComponentType<ProceedComponentProps>
  disableBalanceCheck?: boolean
+  proceedComponentStatic?: boolean


can we document what this prop means, doesn't seem super obvious

satish-ravi · 2024-05-15T23:06:04Z

src/send/EnterAmount.tsx

-    disableProceed || (disableBalanceCheck ? !!tokenAmount?.isZero() : !transactionIsPossible)
+    disableProceed ||
+    (disableBalanceCheck
+      ? !!tokenAmount?.isZero() || (!!tokenAmount?.lte(token.balance) && !transactionIsPossible)


this is getting super hard to read, either consider breaking this up or leave a comment explaining the different scenarios

satish-ravi · 2024-05-15T23:07:33Z

src/send/EnterAmount.tsx

+        contentContainerStyle={
+          proceedComponentStatic
+            ? styles.contentContainer
+            : { ...styles.contentContainer, flexGrow: 1 }


Suggested change

contentContainerStyle={

proceedComponentStatic

? styles.contentContainer

: { ...styles.contentContainer, flexGrow: 1 }

contentContainerStyle=[styles.contentContainer, !proceedComponentStatic && {flexGrow: 1}]

satish-ravi · 2024-05-15T23:15:46Z

locales/base/translation.json

+      "info": "This pool is powered by Aave",
+      "infoBottomSheet": {
+        "title": "Why this pool?",
+        "description": "This Aave pool has over $150M TVL and more than 6,000 contributors, indicating strong liquidity and trustworthiness. It also uses USDC as its token and Abritrum as its network, which reduces volatility and ensures users will receive inexpensive gas rates.\n\nAll together this makes it a safe and attractive option with competitive APY, ensuring a beneficial investment opportunity for our users.\n\nYou can explore other Aave pools here.",


maybe something like https://app.aave.com/markets/? Would be good to check with product. And re description, can we render this as a content inside the bottom sheet instead of using the existing prop?

satish-ravi · 2024-05-15T23:17:20Z

src/earn/EarnEnterAmount.tsx

+        token={token}
+        tokenAmount={tokenAmount.minus(token.balance)}
+      />
+      {prepareTransactionsResult && prepareTransactionsResult.type === 'possible' && (


Suggested change

{prepareTransactionsResult && prepareTransactionsResult.type === 'possible' && (

{prepareTransactionsResult?.type === 'possible' && (

does this work?

src/earn/EarnEnterAmount.tsx

satish-ravi · 2024-05-16T03:40:57Z

src/send/EnterAmount.tsx

@@ -150,10 +154,13 @@ function EnterAmount({
  prepareTransactionError,
  tokenSelectionDisabled = false,
  onPressProceed,
+  onPressInfo,
+  onSetTokenAmount,


Suggested change

onSetTokenAmount,

onChangeTokenAmount,

satish-ravi · 2024-05-16T03:47:46Z

src/send/EnterAmount.tsx

@@ -71,10 +72,13 @@ interface Props {
  prepareTransactionError?: Error
  tokenSelectionDisabled?: boolean
  onPressProceed(args: ProceedArgs): void
+  onPressInfo?(): void


is onPressInfo needed in EnterAmount? Seems like it just gets it as a prop and passes in back to proceed component, which can all be done within EarnEnterAmount

no, I'll update it to just be within the proceed component

actually, I think to do this I would have to have the EarnProceed function inside EarnEnterAmount, and this causes issues with the token amount state value and also rendering APY.

So I think EarnProceed (which needs access to onPressInfo) needs to be outside of EarnEnterAmount so I'm not sure then how it would have access to onPressInfo. @satish-ravi is there some way to access it from outside the function it is declared without passing as a prop?

emerge-tools · 2024-05-16T21:48:23Z

1 build increased size

Name	Version	Download	Change	Install	Change	Approval
Celo (test) org.celo.mobile.test	1.85.0 (150)	27.1 MB	⬆️ 13.0 kB (0.05%)	64.5 MB	⬆️ 32.8 kB (0.05%)	N/A

Celo (test) 1.85.0 (150)
org.celo.mobile.test

⚖️ Compare build
📦 Install build
⏱️ Analyze build performance

Total install size change: ⬆️ 32.8 kB (0.05%)
Total download size change: ⬆️ 13.0 kB (0.05%)

Largest size changes

Item	Install Size Change
main.jsbundle	⬆️ 32.8 kB

🛸 Powered by Emerge Tools

finnian0826 · 2024-05-17T21:35:35Z

src/earn/EarnEnterAmount.tsx

+          token={token}
+          amountEnteredIn={enteredIn}
+          onPressProceed={onPressContinue}
+          onPressInfo={onPressInfo}


Still need to pass this here. I tried having EarnProceed be a const inside of EarnEnterAmount rather than a function outside of it, but this was causing it to re-render everytime the input amount was changed and so the APY was flashing a lot.

finnian0826 · 2024-05-17T21:39:59Z

Follow up issue to use EnterAmount: https://linear.app/valora/issue/ACT-1199/placeholder-clean-up-have-earnenteramount-use-enteramount-component

satish-ravi · 2024-05-17T22:22:27Z

src/earn/prepareTransactions.ts

+export function usePrepareSupplyTransactions() {
+  const prepareTransactions = useAsyncCallback(prepareSupplyTransactions, {
+    onError: (error) => {
+      Logger.error(TAG, `usePrepareSupplyTransactions: ${error}`)


Suggested change

Logger.error(TAG, `usePrepareSupplyTransactions: ${error}`)

Logger.error(TAG, 'usePrepareSupplyTransactions', error)

so the error is logged with trace, you may need an ensureError potentially

satish-ravi · 2024-05-17T22:23:52Z

src/earn/EarnEnterAmount.tsx

+    prepareTransactionsResult.transactions.length > 0
+
+  const disabled =
+    !!tokenAmount?.isZero() || (!!tokenAmount?.lte(token.balance) && !transactionIsPossible) // Should disable if the user enters 0 or has enough balance but the transaction is not possible, shouldn't disable if they enter an amount larger than their balance as they will go to add flow


nit: this won't be readable in the editor, move the comment to one line above and break it up to multiple lines. would be nice if prettier can do this.

satish-ravi · 2024-05-17T22:31:13Z

src/earn/EarnEnterAmount.tsx

+    prepareTransactionError,
+  } = usePrepareSupplyTransactions()
+
+  const walletAddress = useSelector(walletAddressSelector) as Address


can we avoid the as Address, I think we've been doing a isAddress check where required

satish-ravi · 2024-05-17T22:34:49Z

src/analytics/Properties.tsx

+    tokenAmount: string | null
+    amountInUsd: string | null
+    amountEnteredIn: AmountEnteredIn
+    tokenId: string | null
+    networkId: string | null


do these need to be nulls?

no, they should all have values, I'll update to not take null

satish-ravi · 2024-05-17T22:36:02Z

src/earn/EarnEnterAmount.tsx

+
+  const onPressContinue = ({ tokenAmount, token, amountEnteredIn }: ProceedArgs) => {
+    ValoraAnalytics.track(EarnEvents.earn_enter_amount_continue_press, {
+      userHasFunds: token.balance?.gte(tokenAmount),


reuse isAmountLessThanBalance

satish-ravi · 2024-05-17T22:42:21Z

src/earn/EarnEnterAmount.test.tsx

+    })
+  })
+
+  it('should handle navigating to the deposit bottom sheet', async () => {


can this assert the appropriate bottom sheet was opened?

satish-ravi · 2024-05-17T22:42:29Z

src/earn/EarnEnterAmount.test.tsx

+      }
+    )
+  })
+  it('should handle navigating to the add crypto bottom sheet', async () => {


same comment as above

satish-ravi · 2024-05-17T22:44:29Z

src/earn/EarnEnterAmount.tsx

+        <EarnDepositBottomSheet
+          forwardedRef={reviewBottomSheetRef}
+          preparedTransaction={prepareTransactionsResult}
+          amount={tokenAmount ? tokenAmount.toString() : '0'}


tokenAmount should be defined with tx is possible right? You can just add this as another condition to the conditional rendering check above instead of the ternary here.

satish-ravi · 2024-05-17T22:45:50Z

src/earn/EarnEnterAmount.tsx

+    paddingVertical: Spacing.Thick24,
+  },
+  valuesText: {
+    ...typeScale.labelSemiBoldSmall,


add color to all text styles

satish-ravi · 2024-05-17T22:50:47Z

src/earn/EarnEnterAmount.tsx

+  }
+  const onPressMorePools = () => {
+    ValoraAnalytics.track(EarnEvents.earn_enter_amount_info_more_pools)
+    navigate(Screens.WebViewScreen, { uri: 'https://app.aave.com/markets/' })


can we move this to the earn_stablecoin_config dynamic config?

MuckT · 2024-05-18T00:03:31Z

src/earn/prepareTransactions.ts

 import { fetchWithTimeout } from 'src/utils/fetchWithTimeout'
 import { publicClient } from 'src/viem'
 import { TransactionRequest, prepareTransactions } from 'src/viem/prepareTransactions'
 import networkConfig, { networkIdToNetwork } from 'src/web3/networkConfig'
 import { Address, encodeFunctionData, isAddress, parseUnits } from 'viem'

+const TAG = 'src/earn/prepareTransactions'


Suggested change

const TAG = 'src/earn/prepareTransactions'

const TAG = 'earn/prepareTransactions'

nit: usually we don't include src in the tags.

feat(earn): Add enter amount screen

a32008d

finnian0826 added 20 commits May 8, 2024 10:38

refactor

73638e4

more changes

2f25009

Merge branch 'main' into finnian0826/earn-enter-amount

5bcbded

implementing more

bf49cc5

implementing

ef4b3be

more work

78b0573

Merge branch 'main' into finnian0826/earn-enter-amount

2d259d4

fix merge

7aa5e88

refactor

63cc9bf

updates

73dbd00

more updates

bd96e77

janky fix

fa87d6c

almost working

7b05075

fix statsig thing

9b8f2de

add apy

d7e784a

earn up to

2bbf334

fix hooks

02914bd

add analytics and update root state schema

2044fa3

add unit tests

9d01515

fix tests

3780fd1

finnian0826 commented May 15, 2024

View reviewed changes

finnian0826 marked this pull request as ready for review May 15, 2024 22:07

cleanup

32d8f50

satish-ravi reviewed May 16, 2024

View reviewed changes

feedback

beb1c3c

finnian0826 added 3 commits May 18, 2024 07:50

Merge branch 'main' into finnian0826/earn-enter-amount

9606ff3

fix merge

f5ef6de

clean up

9dd057c

finnian0826 commented May 17, 2024

View reviewed changes

finnian0826 added 2 commits May 17, 2024 15:36

fix hook issue

2e816a6

Merge branch 'main' into finnian0826/earn-enter-amount

ffe3533

try to fix hooks again

050774d

satish-ravi reviewed May 17, 2024

View reviewed changes

MuckT reviewed May 18, 2024

View reviewed changes

finnian0826 added 8 commits May 20, 2024 10:03

feedback

4ba0031

fix error

b8859d1

update gas warning

69c8a6c

fix warning

bfdba62

fix translation

e55120f

add cta to warning

3095b7c

update copy

5da4802

fix

780104f

silasbw approved these changes May 20, 2024

View reviewed changes

finnian0826 added 3 commits May 20, 2024 13:32

add analytics for no gas warning press

38cd886

Merge branch 'main' into finnian0826/earn-enter-amount

5d87f86

fix swapped check for balance

244fd9a

finnian0826 enabled auto-merge May 20, 2024 19:48

finnian0826 added 3 commits May 21, 2024 08:05

Merge branch 'main' into finnian0826/earn-enter-amount

18d2ea6

Merge branch 'main' into finnian0826/earn-enter-amount

69cc044

Merge branch 'main' into finnian0826/earn-enter-amount

b1f5a6d

finnian0826 added this pull request to the merge queue May 20, 2024

Merged via the queue into main with commit 7421745 May 20, 2024
16 checks passed

finnian0826 deleted the finnian0826/earn-enter-amount branch May 20, 2024 21:42

	{prepareTransactionsResult && prepareTransactionsResult.type === 'possible' && (
	{prepareTransactionsResult?.type === 'possible' && (

	Logger.error(TAG, `usePrepareSupplyTransactions: ${error}`)
	Logger.error(TAG, 'usePrepareSupplyTransactions', error)

	const TAG = 'src/earn/prepareTransactions'
	const TAG = 'earn/prepareTransactions'

feat(earn): Add enter amount screen #5399

feat(earn): Add enter amount screen #5399

Conversation

finnian0826 commented May 7, 2024 • edited

Description

Test plan

Related issues

Backwards compatibility

Network scalability

codecov bot commented May 7, 2024 • edited

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

satish-ravi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

emerge-tools bot commented May 16, 2024 • edited

1 build increased size

Celo (test) 1.85.0 (150)org.celo.mobile.test

Choose a reason for hiding this comment

finnian0826 commented May 17, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

finnian0826 commented May 7, 2024 •

edited

codecov bot commented May 7, 2024 •

edited

emerge-tools bot commented May 16, 2024 •

edited

Celo (test) 1.85.0 (150)
org.celo.mobile.test