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 all 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
30 changes: 24 additions & 6 deletions packages/mobile/src/components/CeloAmountInput.tsx
Original file line number Diff line number Diff line change
@@ -1,9 +1,18 @@
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'
import {
ActivityIndicator,
StyleSheet,
Text,
TextInputProps,
TouchableOpacity,
View,
ViewStyle,
} from 'react-native'
import { useSelector } from 'react-redux'
import { Namespaces } from 'src/i18n'
import { RootState } from 'src/redux/reducers'
Expand All @@ -14,6 +23,7 @@ interface Props {
celo: string
onCeloChanged: (address: string) => void
color?: string
feeEstimate: BigNumber | undefined
}

export default function CeloAmountInput({
Expand All @@ -22,13 +32,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)
if (goldBalance && feeEstimate) {
const maxValue = new BigNumber(goldBalance).minus(feeEstimate)
onCeloChanged(maxValue.isPositive() ? maxValue.toString() : '0')
}
}

Expand All @@ -43,9 +55,15 @@ export default function CeloAmountInput({
value={celo}
testID={'CeloAmount'}
>
<TouchableOpacity testID={'MaxAmount'} onPress={setMaxAmount}>
<Text style={[styles.maxAmount, { color }]}>{t('maxSymbol')}</Text>
</TouchableOpacity>
{feeEstimate ? (
<TouchableOpacity testID={'MaxAmount'} onPress={setMaxAmount}>
<Text style={[styles.maxAmount, { color }]}>{t('maxSymbol')}</Text>
</TouchableOpacity>
) : (
<View>
<ActivityIndicator size="small" color={colors.goldUI} />
</View>
)}
</TextInputWithButtons>
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ const SAMPLE_AMOUNT = new BigNumber(5.001)
const mockScreenProps = getMockStackScreenProps(Screens.WithdrawCeloReviewScreen, {
recipientAddress: SAMPLE_ADDRESS,
amount: SAMPLE_AMOUNT,
feeEstimate: new BigNumber(1),
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this 1 wei, and if so, is that displayed to the user at all? If the fee was actually 1 wei, it would likely not be significant enough to warrant mentioning (unless they are trying to withdraw 100% of their balance, in which case we might help craft a request that won't leave any dust behind.)

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 reviewing! The wei to CELO mapping happens in WithdrawCeloScreen, line 67. The converted fee is then passed to this review screen and this is a test of it where I put a != 0 number just to have something there.

})

const store = createMockStore()
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
11 changes: 10 additions & 1 deletion packages/mobile/src/exchange/WithdrawCeloScreen.test.tsx
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import BigNumber from 'bignumber.js'
import * as React from 'react'
import 'react-native'
import { fireEvent, render } from 'react-native-testing-library'
Expand All @@ -16,6 +17,14 @@ const store = createMockStore({
goldToken: { balance: SAMPLE_BALANCE },
})

const mockResult = new BigNumber(10)
jest.mock('src/fees/CalculateFee', () => ({
useSendFee: () => ({
result: mockResult,
loading: false,
}),
}))

describe('WithdrawCeloScreen', () => {
it('renders correctly', () => {
const tree = render(
Expand Down Expand Up @@ -55,7 +64,7 @@ describe('WithdrawCeloScreen', () => {

expect(getByTestId('CeloAmount').props.value).toBe('')
fireEvent.press(getByTestId('MaxAmount'))
expect(getByTestId('CeloAmount').props.value).toBe(SAMPLE_BALANCE)
expect(parseFloat(getByTestId('CeloAmount').props.value).toFixed(5)).toBe(SAMPLE_BALANCE)
expect(getByTestId('WithdrawReviewButton').props.disabled).toBe(false)
})

Expand Down
17 changes: 17 additions & 0 deletions packages/mobile/src/exchange/WithdrawCeloScreen.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ import ValoraAnalytics from 'src/analytics/ValoraAnalytics'
import AccountAddressInput from 'src/components/AccountAddressInput'
import CeloAmountInput from 'src/components/CeloAmountInput'
import { exchangeRatePairSelector } from 'src/exchange/reducer'
import { FeeType } from 'src/fees/actions'
import { useSendFee } from 'src/fees/CalculateFee'
import { CURRENCY_ENUM } from 'src/geth/consts'
import i18n, { Namespaces } from 'src/i18n'
import { HeaderTitleWithBalance, headerWithBackButton } from 'src/navigator/Headers'
Expand All @@ -27,9 +29,12 @@ import { StackParamList } from 'src/navigator/types'
import useSelector from 'src/redux/useSelector'
import { useDailyTransferLimitValidator } from 'src/send/utils'
import DisconnectBanner from 'src/shared/DisconnectBanner'
import { divideByWei } from 'src/utils/formatting'

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('')
Expand All @@ -51,6 +56,16 @@ function WithdrawCeloScreen({ navigation }: Props) {
CURRENCY_ENUM.GOLD
)

const { result } = useSendFee({
feeType: FeeType.SEND,
account: RANDOM_ADDRESS,
currency: CURRENCY_ENUM.GOLD,
recipientAddress: RANDOM_ADDRESS,
amount: goldBalance || '0',
includeDekFee: false,
})
const feeEstimate = result && divideByWei(result)

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

Expand All @@ -86,6 +102,7 @@ function WithdrawCeloScreen({ navigation }: Props) {
inputStyle={styles.input}
onCeloChanged={setCeloToTransfer}
celo={celoInput}
feeEstimate={feeEstimate}
/>
</KeyboardAwareScrollView>
<Button
Expand Down
6 changes: 5 additions & 1 deletion packages/mobile/src/exchange/WithdrawCeloSummary.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,11 @@ describe('WithdrawCeloSummary', () => {
it('renders correctly', () => {
const tree = render(
<Provider store={store}>
<WithdrawCeloSummary amount={SAMPLE_AMOUNT} recipientAddress={SAMPLE_ADDRESS} />
<WithdrawCeloSummary
amount={SAMPLE_AMOUNT}
recipientAddress={SAMPLE_ADDRESS}
feeEstimate={new BigNumber(1)}
/>
</Provider>
)
expect(tree).toMatchSnapshot()
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 @@ -321,7 +321,7 @@ exports[`WithdrawCeloReviewScreen renders correctly 1`] = `
>


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


0
1
</Text>
</Text>
</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 @@ -284,7 +284,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 @@ -397,7 +400,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
29 changes: 21 additions & 8 deletions packages/mobile/src/fees/CalculateFee.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,10 @@ interface SendProps extends CommonProps {
feeType: FeeType.SEND
account: string
recipientAddress: string
amount: BigNumber
amount: string
comment?: string
includeDekFee: boolean
currency?: CURRENCY_ENUM
}

interface ExchangeProps extends CommonProps {
Expand Down Expand Up @@ -88,27 +89,39 @@ const CalculateInviteFee: FunctionComponent<InviteProps> = (props) => {
return props.children(asyncResult) as React.ReactElement
}

const CalculateSendFee: FunctionComponent<SendProps> = (props) => {
const asyncResult = useAsyncShowError(
export const useSendFee = (props: Omit<SendProps, 'children'>): UseAsyncReturn<BigNumber> => {
return useAsyncShowError(
(
account: string,
recipientAddress: string,
amount: BigNumber,
amount: string,
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(),
amount,
comment,
},
includeDekFee
),
[props.account, props.recipientAddress, props.amount, props.comment, props.includeDekFee]
[
props.account,
props.recipientAddress,
props.amount,
props.comment,
props.includeDekFee,
props.currency,
]
)
}

const CalculateSendFee: FunctionComponent<SendProps> = (props) => {
const asyncResult = useSendFee(props)
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
2 changes: 1 addition & 1 deletion packages/mobile/src/send/SendConfirmation.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -467,7 +467,7 @@ export class SendConfirmation extends React.Component<Props, State> {
feeType: FeeType.SEND,
account,
recipientAddress,
amount,
amount: amount.valueOf(),
includeDekFee: !isDekRegistered,
}
: { feeType: FeeType.INVITE, account, amount }
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