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

Add fee estimation for Withdrawing CELO and pay for Withdrawing and Selling CELO fees in CELO #5345

Merged
merged 11 commits into from
Oct 20, 2020
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion packages/mobile/src/components/CeloAmountInput.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import TextInputWithButtons from '@celo/react-components/components/TextInputWithButtons'
import colors from '@celo/react-components/styles/colors'
import fontStyles from '@celo/react-components/styles/fonts'
import BigNumber from 'bignumber.js'
import React from 'react'
import { useTranslation } from 'react-i18next'
import { StyleSheet, Text, TextInputProps, TouchableOpacity, ViewStyle } from 'react-native'
Expand All @@ -14,6 +15,7 @@ interface Props {
celo: string
onCeloChanged: (address: string) => void
color?: string
feeEstimate: BigNumber
}

export default function CeloAmountInput({
Expand All @@ -22,13 +24,15 @@ export default function CeloAmountInput({
celo,
onCeloChanged,
color = colors.goldUI,
feeEstimate,
}: Props) {
const { t } = useTranslation(Namespaces.exchangeFlow9)
const goldBalance = useSelector((state: RootState) => state.goldToken.balance)

const setMaxAmount = () => {
if (goldBalance) {
onCeloChanged(goldBalance)
const maxValue = new BigNumber(goldBalance).minus(feeEstimate)
onCeloChanged(maxValue.isPositive() ? maxValue.toString() : '0')
}
}

Expand Down
8 changes: 6 additions & 2 deletions packages/mobile/src/exchange/WithdrawCeloReviewScreen.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ import DisconnectBanner from 'src/shared/DisconnectBanner'
type Props = StackScreenProps<StackParamList, Screens.WithdrawCeloReviewScreen>

function WithdrawCeloReviewScreen({ route }: Props) {
const { amount, recipientAddress } = route.params
const { amount, recipientAddress, feeEstimate } = route.params
const { t } = useTranslation(Namespaces.exchangeFlow9)
// loading is never set to false, when the withdrawal is complete or after a short while,
// withdrawCelo saga will navigate to |ExchangeHomeScreen|.
Expand All @@ -56,7 +56,11 @@ function WithdrawCeloReviewScreen({ route }: Props) {
amount={<ShortenedAddress style={styles.withdrawAddress} address={recipientAddress} />}
/>
<HorizontalLine />
<WithdrawCeloSummary amount={amount} recipientAddress={recipientAddress} />
<WithdrawCeloSummary
amount={amount}
recipientAddress={recipientAddress}
feeEstimate={feeEstimate}
/>
</View>
<Button
onPress={onConfirmWithdraw}
Expand Down
29 changes: 28 additions & 1 deletion packages/mobile/src/exchange/WithdrawCeloScreen.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import fontStyles from '@celo/react-components/styles/fonts'
import variables from '@celo/react-components/styles/variables'
import { StackScreenProps } from '@react-navigation/stack'
import BigNumber from 'bignumber.js'
import React, { useState } from 'react'
import React, { useEffect, useState } from 'react'
import { useTranslation } from 'react-i18next'
import { StyleSheet, Text } from 'react-native'
import { SafeAreaView } from 'react-native-safe-area-context'
Expand All @@ -25,14 +25,20 @@ import { HeaderTitleWithBalance, headerWithBackButton } from 'src/navigator/Head
import { Screens } from 'src/navigator/Screens'
import { StackParamList } from 'src/navigator/types'
import useSelector from 'src/redux/useSelector'
import { getSendFee } from 'src/send/saga'
import { useDailyTransferLimitValidator } from 'src/send/utils'
import DisconnectBanner from 'src/shared/DisconnectBanner'
import { divideByWei } from 'src/utils/formatting'
import Logger from 'src/utils/Logger'

type Props = StackScreenProps<StackParamList, Screens.WithdrawCeloScreen>

const RANDOM_ADDRESS = '0xDCE9762d6C1fe89FF4f3857832131Ca18eE15C66'
Copy link
Contributor Author

@gnardini gnardini Oct 15, 2020

Choose a reason for hiding this comment

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

We don't have an address here yet to do the estimation, and estimating requires an address, so I used a random one. Could that cause any issues?

Copy link
Contributor

Choose a reason for hiding this comment

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

This is fine, we already do this here as well.


function WithdrawCeloScreen({ navigation }: Props) {
const [accountAddress, setAccountAddress] = useState('')
const [celoInput, setCeloToTransfer] = useState('')
const [feeEstimate, setFeeEstimate] = useState(new BigNumber(0))

const goldBalance = useSelector((state) => state.goldToken.balance)
const goldBalanceNumber = new BigNumber(goldBalance || 0)
Expand All @@ -51,6 +57,25 @@ function WithdrawCeloScreen({ navigation }: Props) {
CURRENCY_ENUM.GOLD
)

useEffect(() => {
getSendFee(
accountAddress,
CURRENCY_ENUM.GOLD,
{
recipientAddress: RANDOM_ADDRESS,
amount: goldBalanceNumber,
comment: '',
},
false
)
.then((estimate: BigNumber) => {
setFeeEstimate(divideByWei(estimate))
})
.catch((error) => {
Logger.warn('WithdrawCeloScreen', `Error found while estimating gas fee: ${error}`)
})
}, [])

Copy link
Contributor

Choose a reason for hiding this comment

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

Async functions should be avoided in general in useEffect because they can trigger actions after the component has been unmounted.

useAsync from react-async-hook solves this.

Now we have CalculateFee which uses useAsync under the hood and mostly does what you want here.

I originally created it as a component because most components we had at the time, were class components.
But we could export a useSendFee hook from it now.

Added benefit is it provides loading/error states so we can display a spinner while it's calculating and it displays an error if it fails.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for this! I had run into this many times and always solved it in this inelegant way, the useAsync hook looks great.

const onConfirm = async () => {
if (isTransferLimitReached) {
showLimitReachedBanner()
Expand All @@ -63,6 +88,7 @@ function WithdrawCeloScreen({ navigation }: Props) {
navigation.navigate(Screens.WithdrawCeloReviewScreen, {
amount: celoToTransfer,
recipientAddress: accountAddress,
feeEstimate,
})
}

Expand All @@ -86,6 +112,7 @@ function WithdrawCeloScreen({ navigation }: Props) {
inputStyle={styles.input}
onCeloChanged={setCeloToTransfer}
celo={celoInput}
feeEstimate={feeEstimate}
/>
</KeyboardAwareScrollView>
<Button
Expand Down
8 changes: 4 additions & 4 deletions packages/mobile/src/exchange/WithdrawCeloSummary.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,16 +16,16 @@ interface WithdrawCeloProps {
style?: ViewStyle
amount: BigNumber
recipientAddress: string
feeEstimate: BigNumber
}

export default function WithdrawCeloSummary({
style,
amount,
recipientAddress,
feeEstimate,
}: WithdrawCeloProps) {
const { t } = useTranslation(Namespaces.exchangeFlow9)
// TODO: Estimate real fee.
const fee = new BigNumber(0)

return (
<View style={style}>
Expand All @@ -46,8 +46,8 @@ export default function WithdrawCeloSummary({
currency={CURRENCY_ENUM.GOLD}
isExchange={false}
isEstimate={true}
securityFee={fee}
totalFee={fee}
securityFee={feeEstimate}
totalFee={feeEstimate}
/>
<TotalLineItem
amount={{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -295,35 +295,6 @@ exports[`WithdrawCeloReviewScreen renders correctly 1`] = `

</Text>
</View>
<Text
style={
Array [
Object {
"color": "#2E3338",
"fontFamily": "Inter-Regular",
"fontSize": 16,
"lineHeight": 22,
},
undefined,
]
}
>
<Text
numberOfLines={1}
style={
Array [
undefined,
Object {
"color": undefined,
},
]
}
>


0
</Text>
</Text>
</View>
</View>
</View>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -187,35 +187,6 @@ exports[`WithdrawCeloSummary renders correctly 1`] = `

</Text>
</View>
<Text
style={
Array [
Object {
"color": "#2E3338",
"fontFamily": "Inter-Regular",
"fontSize": 16,
"lineHeight": 22,
},
undefined,
]
}
>
<Text
numberOfLines={1}
style={
Array [
undefined,
Object {
"color": undefined,
},
]
}
>


0
</Text>
</Text>
</View>
</View>
</View>
Expand Down
14 changes: 12 additions & 2 deletions packages/mobile/src/exchange/saga.ts
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,10 @@ export function* exchangeGoldAndStableTokens(action: ExchangeTokensAction) {
sendTransaction,
approveTx.txo,
account,
newTransactionContext(TAG, `Approve exchange of ${makerToken}`)
newTransactionContext(TAG, `Approve exchange of ${makerToken}`),
undefined, // staticGas
undefined, // cancelAction
makerToken
)
Logger.debug(TAG, `Transaction approved: ${util.inspect(approveTx.txo.arguments)}`)

Expand Down Expand Up @@ -395,7 +398,14 @@ function* withdrawCelo(action: WithdrawCeloAction) {
}
)

yield call(sendAndMonitorTransaction, tx, account, context, CURRENCY_ENUM.GOLD)
yield call(
sendAndMonitorTransaction,
tx,
account,
context,
CURRENCY_ENUM.GOLD,
CURRENCY_ENUM.GOLD
)

const dollarAmount = yield call(celoToDollarAmount, amount)
yield put(sendPaymentOrInviteSuccess(dollarAmount))
Expand Down
15 changes: 12 additions & 3 deletions packages/mobile/src/fees/CalculateFee.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ interface SendProps extends CommonProps {
amount: BigNumber
comment?: string
includeDekFee: boolean
currency?: CURRENCY_ENUM
}

interface ExchangeProps extends CommonProps {
Expand Down Expand Up @@ -95,19 +96,27 @@ const CalculateSendFee: FunctionComponent<SendProps> = (props) => {
recipientAddress: string,
amount: BigNumber,
comment: string = MAX_PLACEHOLDER_COMMENT,
includeDekFee: boolean = false
includeDekFee: boolean = false,
currency: CURRENCY_ENUM = CURRENCY_ENUM.DOLLAR
) =>
getSendFee(
account,
CURRENCY_ENUM.DOLLAR,
currency,
{
recipientAddress,
amount: amount.valueOf(),
comment,
},
includeDekFee
),
[props.account, props.recipientAddress, props.amount, props.comment, props.includeDekFee]
[
props.account,
props.recipientAddress,
props.amount,
props.comment,
props.includeDekFee,
props.currency,
]
)
return props.children(asyncResult) as React.ReactElement
}
Expand Down
1 change: 1 addition & 0 deletions packages/mobile/src/navigator/types.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,7 @@ export type StackParamList = {
[Screens.WithdrawCeloReviewScreen]: {
amount: BigNumber
recipientAddress: string
feeEstimate: BigNumber
}
[Screens.WithdrawCeloScreen]: undefined
}
Expand Down
7 changes: 5 additions & 2 deletions packages/mobile/src/send/saga.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,12 +45,15 @@ export async function getSendTxGas(
try {
Logger.debug(`${TAG}/getSendTxGas`, 'Getting gas estimate for send tx')
const tx = await createTokenTransferTransaction(currency, params)
const txParams = { from: account, feeCurrency: await getCurrencyAddress(currency) }
const txParams = {
from: account,
feeCurrency: currency === CURRENCY_ENUM.GOLD ? undefined : await getCurrencyAddress(currency),
}
const gas = await estimateGas(tx.txo, txParams)
Logger.debug(`${TAG}/getSendTxGas`, `Estimated gas of ${gas.toString()}`)
return gas
} catch (error) {
throw Error(ErrorMessages.INSUFFICIENT_BALANCE)
throw Error(ErrorMessages.CALCULATE_FEE_FAILED)
}
}

Expand Down
4 changes: 3 additions & 1 deletion packages/mobile/src/transactions/saga.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,8 @@ export function* sendAndMonitorTransaction<T>(
tx: CeloTransactionObject<T>,
account: string,
context: TransactionContext,
currency?: CURRENCY_ENUM
currency?: CURRENCY_ENUM,
feeCurrency?: CURRENCY_ENUM
) {
try {
Logger.debug(TAG + '@sendAndMonitorTransaction', `Sending transaction with id: ${context.id}`)
Expand All @@ -74,6 +75,7 @@ export function* sendAndMonitorTransaction<T>(
tx.txo,
account,
context,
feeCurrency,
nonce
)
const hash = yield transactionHash
Expand Down
Loading