Skip to content

Commit

Permalink
chore(swaps): add more analytics for debugging (#3810)
Browse files Browse the repository at this point in the history
### Description

We are still getting many errors that indicate that we are allowing the
user to proceed with a swap amount greater than their balance. It's not
clear why this still happens, because we have a (high) fee estimation in
place already.

Ideally I'd like to have data to be able to say "oh yes, this swap was
expected to fail because the swap amount was X, required fee was Y, and
X + Y > the total balance". i think then we could go back and tweak the
logic required for X, when we have a better idea of what X and Y are.

Sadly during my debugging today I think some errors are being thrown at
the calculation for the gas fee 🫠 so we may not have the value for Y.

This PR adds data for 
- total "from" token balance at the time of executing the swap
- context id's used for the swap transactions so that we can also check
the TransactionEvents which contains some information about gas / errors
etc. If an error is thrown when calculating the gas fee, then we will be
able to tell (from the lack of the fee event).
- add fee currency to the estimation and error TransactionEvents, to see
if there's any correlation of token with errors.

### Test plan

n/a

### Related issues

n/a

### Backwards compatibility

Y
  • Loading branch information
kathaypacific committed May 26, 2023
1 parent e66b1ed commit daa8973
Show file tree
Hide file tree
Showing 5 changed files with 120 additions and 85 deletions.
13 changes: 11 additions & 2 deletions src/analytics/Properties.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@ import {
AppEvents,
AssetsEvents,
AuthenticationEvents,
CICOEvents,
CeloExchangeEvents,
CeloNewsEvents,
CICOEvents,
CoinbasePayEvents,
ContractKitEvents,
DappExplorerEvents,
Expand Down Expand Up @@ -695,6 +695,7 @@ interface TransactionEventsProperties {
txId: string
estimatedGas: number
prefilled: boolean
feeCurrencyAddress?: string
}
[TransactionEvents.transaction_hash_received]: {
txId: string
Expand All @@ -713,6 +714,7 @@ interface TransactionEventsProperties {
[TransactionEvents.transaction_exception]: {
txId: string
error: string
feeCurrencyAddress?: string
}
}

Expand Down Expand Up @@ -1217,9 +1219,16 @@ interface SwapEventsProperties {
toToken: string
fromToken: string
}
[SwapEvents.swap_execute_success]: SwapQuoteEvent
[SwapEvents.swap_execute_success]: SwapQuoteEvent & {
fromTokenBalance: string
swapExecuteTxId: string
swapApproveTxId: string
}
[SwapEvents.swap_execute_error]: SwapQuoteEvent & {
error: string
fromTokenBalance: string
swapExecuteTxId: string
swapApproveTxId: string
}
[SwapEvents.swap_learn_more]: undefined
}
Expand Down
78 changes: 46 additions & 32 deletions src/swap/saga.test.ts
Original file line number Diff line number Diff line change
@@ -1,17 +1,26 @@
import BigNumber from 'bignumber.js'
import { expectSaga } from 'redux-saga-test-plan'
import { EffectProviders, StaticProvider } from 'redux-saga-test-plan/providers'
import { call, select } from 'redux-saga/effects'
import { SwapEvents } from 'src/analytics/Events'
import ValoraAnalytics from 'src/analytics/ValoraAnalytics'
import { store } from 'src/redux/store'
import { swapSubmitSaga } from 'src/swap/saga'
import { swapApprove, swapError, swapExecute, swapPriceChange } from 'src/swap/slice'
import { getERC20TokenContract } from 'src/tokens/saga'
import { swappableTokensSelector } from 'src/tokens/selectors'
import { sendTransaction } from 'src/transactions/send'
import Logger from 'src/utils/Logger'
import { getContractKit } from 'src/web3/contracts'
import { getConnectedUnlockedAccount } from 'src/web3/saga'
import { walletAddressSelector } from 'src/web3/selectors'
import { mockAccount, mockContract } from 'test/values'
import {
mockAccount,
mockCeloAddress,
mockCeurAddress,
mockContract,
mockTokenBalances,
} from 'test/values'

const loggerErrorSpy = jest.spyOn(Logger, 'error')

Expand All @@ -31,8 +40,8 @@ jest.mock('src/transactions/send', () => ({

const mockSwapTransaction = {
buyAmount: '10000000000000000',
buyTokenAddress: '0xd8763cba276a3738e6de85b4b3bf5fded6d6ca73',
sellTokenAddress: '0xe8537a3d056da446677b9e9d6c5db704eaab4787',
buyTokenAddress: mockCeloAddress,
sellTokenAddress: mockCeurAddress,
price: '1',
guaranteedPrice: '1.02',
from: '0x078e54ad49b0865fff9086fd084b92b3dac0857d',
Expand Down Expand Up @@ -64,18 +73,29 @@ describe(swapSubmitSaga, () => {
jest.clearAllMocks()
})

const defaultProviders: (EffectProviders | StaticProvider)[] = [
[select(walletAddressSelector), mockAccount],
[call(getContractKit), contractKit],
[call(getConnectedUnlockedAccount), mockAccount],
[
select(swappableTokensSelector),
[
{
...mockTokenBalances[mockCeurAddress],
balance: new BigNumber('10'),
},
],
],
[
call(getERC20TokenContract, mockSwap.payload.unvalidatedSwapTransaction.sellTokenAddress),
mockContract,
],
]

it('should complete swap', async () => {
await expectSaga(swapSubmitSaga, mockSwap)
.withState(store.getState())
.provide([
[select(walletAddressSelector), mockAccount],
[call(getContractKit), contractKit],
[call(getConnectedUnlockedAccount), mockAccount],
[
call(getERC20TokenContract, mockSwap.payload.unvalidatedSwapTransaction.sellTokenAddress),
mockContract,
],
])
.provide(defaultProviders)
.put(swapApprove())
.put(swapExecute())
.run()
Expand All @@ -84,14 +104,17 @@ describe(swapSubmitSaga, () => {

expect(ValoraAnalytics.track).toHaveBeenCalledTimes(1)
expect(ValoraAnalytics.track).toHaveBeenCalledWith(SwapEvents.swap_execute_success, {
toToken: '0xd8763cba276a3738e6de85b4b3bf5fded6d6ca73',
fromToken: '0xe8537a3d056da446677b9e9d6c5db704eaab4787',
toToken: mockCeloAddress,
fromToken: mockCeurAddress,
amount: '10000000000000000',
amountType: 'buyAmount',
price: '1',
allowanceTarget: '0xdef1c0ded9bec7f1a1670819833240f027b25eff',
estimatedPriceImpact: '0.1',
provider: '0x',
fromTokenBalance: '10000000000000000000',
swapApproveTxId: 'a uuid',
swapExecuteTxId: 'a uuid',
})
})

Expand All @@ -101,50 +124,41 @@ describe(swapSubmitSaga, () => {
})
await expectSaga(swapSubmitSaga, mockSwap)
.withState(store.getState())
.provide([
[select(walletAddressSelector), mockAccount],
[call(getContractKit), contractKit],
[call(getConnectedUnlockedAccount), mockAccount],
[
call(getERC20TokenContract, mockSwap.payload.unvalidatedSwapTransaction.sellTokenAddress),
mockContract,
],
])
.provide(defaultProviders)
.put(swapApprove())
.put(swapError())
.run()
expect(ValoraAnalytics.track).toHaveBeenCalledTimes(1)
expect(ValoraAnalytics.track).toHaveBeenCalledWith(SwapEvents.swap_execute_error, {
error: 'fake error',
toToken: '0xd8763cba276a3738e6de85b4b3bf5fded6d6ca73',
fromToken: '0xe8537a3d056da446677b9e9d6c5db704eaab4787',
toToken: mockCeloAddress,
fromToken: mockCeurAddress,
amount: '10000000000000000',
amountType: 'buyAmount',
price: '1',
allowanceTarget: '0xdef1c0ded9bec7f1a1670819833240f027b25eff',
estimatedPriceImpact: '0.1',
provider: '0x',
fromTokenBalance: '10000000000000000000',
swapApproveTxId: 'a uuid',
swapExecuteTxId: 'a uuid',
})
})

it('should set swap state correctly on price change', async () => {
mockSwap.payload.unvalidatedSwapTransaction.guaranteedPrice = '1.021'
await expectSaga(swapSubmitSaga, mockSwap)
.withState(store.getState())
.provide([
[select(walletAddressSelector), mockAccount],
[call(getContractKit), contractKit],
[call(getConnectedUnlockedAccount), mockAccount],
])
.provide(defaultProviders)
.put(swapPriceChange())
.run()

expect(ValoraAnalytics.track).toHaveBeenCalledTimes(1)
expect(ValoraAnalytics.track).toHaveBeenCalledWith(SwapEvents.swap_execute_price_change, {
price: '1',
guaranteedPrice: '1.021',
toToken: '0xd8763cba276a3738e6de85b4b3bf5fded6d6ca73',
fromToken: '0xe8537a3d056da446677b9e9d6c5db704eaab4787',
toToken: mockCeloAddress,
fromToken: mockCeurAddress,
})
})
})
96 changes: 52 additions & 44 deletions src/swap/saga.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,12 @@ import {
swapStart,
swapSuccess,
} from 'src/swap/slice'
import { ApproveTransaction, Field, SwapInfo, SwapTransaction } from 'src/swap/types'
import { Field, SwapInfo, SwapTransaction } from 'src/swap/types'
import { getERC20TokenContract } from 'src/tokens/saga'
import { swappableTokensSelector } from 'src/tokens/selectors'
import { TokenBalance } from 'src/tokens/slice'
import { sendTransaction } from 'src/transactions/send'
import { newTransactionContext } from 'src/transactions/types'
import { newTransactionContext, TransactionContext } from 'src/transactions/types'
import Logger from 'src/utils/Logger'
import { safely } from 'src/utils/safely'
import { getContractKit } from 'src/web3/contracts'
Expand All @@ -36,8 +38,8 @@ function getPercentageDifference(price1: number, price2: number) {
}

function* handleSendSwapTransaction(
rawTx: ApproveTransaction | SwapTransaction,
tagDescription: string
rawTx: SwapTransaction,
transactionContext: TransactionContext
) {
const kit: ContractKit = yield call(getContractKit)
const walletAddress: string = yield call(getConnectedUnlockedAccount)
Expand All @@ -47,15 +49,7 @@ function* handleSendSwapTransaction(
const tx: CeloTx = yield call(normalizer.populate.bind(normalizer), rawTx)
const txo = buildTxo(kit, tx)

yield call(
sendTransaction,
txo,
walletAddress,
newTransactionContext(TAG, tagDescription),
undefined,
undefined,
undefined
)
yield call(sendTransaction, txo, walletAddress, transactionContext)
}

export function* swapSubmitSaga(action: PayloadAction<SwapInfo>) {
Expand All @@ -69,9 +63,35 @@ export function* swapSubmitSaga(action: PayloadAction<SwapInfo>) {
allowanceTarget,
estimatedPriceImpact,
} = action.payload.unvalidatedSwapTransaction
const amountType = action.payload.userInput.updatedField === Field.TO ? 'buyAmount' : 'sellAmount'
const amountType =
action.payload.userInput.updatedField === Field.TO
? ('buyAmount' as const)
: ('sellAmount' as const)
const amount = action.payload.unvalidatedSwapTransaction[amountType]

const tokenBalances: TokenBalance[] = yield select(swappableTokensSelector)
const fromToken = tokenBalances.find((token) => token.address === sellTokenAddress)
const fromTokenBalance = fromToken
? fromToken.balance.shiftedBy(fromToken.decimals).toString()
: ''

const swapApproveContext = newTransactionContext(TAG, 'Swap/Approve')
const swapExecuteContext = newTransactionContext(TAG, 'Swap/Execute')

const defaultSwapExecuteProps = {
toToken: buyTokenAddress,
fromToken: sellTokenAddress,
amount,
amountType,
price,
allowanceTarget,
estimatedPriceImpact,
provider: action.payload.details.swapProvider,
fromTokenBalance,
swapExecuteTxId: swapExecuteContext.id,
swapApproveTxId: swapApproveContext.id,
}

try {
// Navigate to swap pending screen
yield call(navigate, Screens.SwapExecuteScreen)
Expand Down Expand Up @@ -104,40 +124,31 @@ export function* swapSubmitSaga(action: PayloadAction<SwapInfo>) {
TAG,
`Approving ${amountToApprove} of ${sellTokenAddress} for address: ${allowanceTarget}`
)
yield call(sendApproveTx, sellTokenAddress, amountToApprove, allowanceTarget)
yield call(
sendApproveTx,
sellTokenAddress,
amountToApprove,
allowanceTarget,
swapApproveContext
)

// Execute transaction
yield put(swapExecute())
Logger.debug(TAG, `Starting to swap execute for address: ${walletAddress}`)

yield call(
handleSendSwapTransaction,
{ ...action.payload.unvalidatedSwapTransaction },
'Swap/Execute'
swapExecuteContext
)
yield put(swapSuccess())
vibrateSuccess()
ValoraAnalytics.track(SwapEvents.swap_execute_success, {
toToken: buyTokenAddress,
fromToken: sellTokenAddress,
amount,
amountType,
price,
allowanceTarget,
estimatedPriceImpact,
provider: action.payload.details.swapProvider,
})
ValoraAnalytics.track(SwapEvents.swap_execute_success, defaultSwapExecuteProps)
} catch (error) {
Logger.error(TAG, 'Error while swapping', error)
ValoraAnalytics.track(SwapEvents.swap_execute_error, {
...defaultSwapExecuteProps,
error: error.message,
toToken: buyTokenAddress,
fromToken: sellTokenAddress,
amount,
amountType,
price,
allowanceTarget,
estimatedPriceImpact,
provider: action.payload.details.swapProvider,
})
yield put(swapError())
vibrateError()
Expand All @@ -148,7 +159,12 @@ export function* swapSaga() {
yield takeLatest(swapStart.type, safely(swapSubmitSaga))
}

export function* sendApproveTx(tokenAddress: string, amount: string, recipientAddress: string) {
export function* sendApproveTx(
tokenAddress: string,
amount: string,
recipientAddress: string,
transactionContext: TransactionContext
) {
const kit: ContractKit = yield call(getContractKit)
const contract: Contract = yield call(getERC20TokenContract, tokenAddress)
const walletAddress: string = yield call(getConnectedUnlockedAccount)
Expand All @@ -158,13 +174,5 @@ export function* sendApproveTx(tokenAddress: string, amount: string, recipientAd
contract.methods.approve(recipientAddress, amount)
)

yield call(
sendTransaction,
tx.txo,
walletAddress,
newTransactionContext(TAG, 'Swap/Approve'),
undefined,
undefined,
undefined
)
yield call(sendTransaction, tx.txo, walletAddress, transactionContext)
}
Loading

0 comments on commit daa8973

Please sign in to comment.