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

[Wallet] Fix slow activity feed tx confirmations #2290

Merged
merged 17 commits into from
Jan 8, 2020
Merged
Show file tree
Hide file tree
Changes from 8 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
54 changes: 21 additions & 33 deletions packages/mobile/src/apollo/__mockData__.ts
Original file line number Diff line number Diff line change
@@ -1,93 +1,81 @@
import {
EventTypeNames,
HomeExchangeFragment,
HomeTransferFragment,
UserTransactionsData,
} from 'src/apollo/types'
import { HomeExchangeFragment, HomeTransferFragment, UserTransactionsData } from 'src/apollo/types'
import { CURRENCY_ENUM } from 'src/geth/consts'
import { SENTINEL_INVITE_COMMENT } from 'src/invite/actions'
import { TransactionTypes } from 'src/transactions/reducer'
import { TransactionTypes, TransferTransactionTypes } from 'src/transactions/reducer'

export const invitedAddress = '0x1b173'

const exchangeDollar: HomeExchangeFragment = {
__typename: EventTypeNames.Exchange,
type: 'EXCHANGE' as TransactionTypes,
type: 'EXCHANGE' as TransactionTypes.EXCHANGE,
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not just use the enum's actual value? (not sure who did this originally)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this is a more realistic mock of what would be returned?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think Connors point is that TransactionTypes.Exchange === 'EXCHANGE' so why hardcode the value here?
And I think the reason it was originally hard coded in the first place is that the auto-gen typing tool did it this way

hash: '1',
inValue: 19080,
timestamp: Date.now(),
inSymbol: 'Celo Dollar',
outSymbol: 'Celo Gold',
inSymbol: 'Celo Dollar' as CURRENCY_ENUM,
outSymbol: 'Celo Gold' as CURRENCY_ENUM,
outValue: 62252,
}

const exchangeGold: HomeExchangeFragment = {
__typename: EventTypeNames.Exchange,
type: 'EXCHANGE' as TransactionTypes,
type: 'EXCHANGE' as TransactionTypes.EXCHANGE,
hash: '1',
inValue: 190,
timestamp: Date.now(),
inSymbol: 'Celo Gold',
outSymbol: 'Celo Dollar',
inSymbol: 'Celo Gold' as CURRENCY_ENUM,
outSymbol: 'Celo Dollar' as CURRENCY_ENUM,
outValue: 62,
}

const sent: HomeTransferFragment = {
__typename: EventTypeNames.Transfer,
type: 'SENT' as TransactionTypes,
type: 'SENT' as TransferTransactionTypes,
value: 987161,
symbol: 'Celo Gold',
symbol: 'Celo Gold' as CURRENCY_ENUM,
timestamp: Date.now(),
address: '0x423043cca38e75d7913504fedfd1dd4539cc55b3',
comment: 'FAKE FAKE FAKE',
hash: '01010',
}

const sentInvite: HomeTransferFragment = {
__typename: EventTypeNames.Transfer,
type: 'SENT' as TransactionTypes,
type: 'SENT' as TransferTransactionTypes,
value: 0.33,
symbol: 'Celo Dollar',
symbol: 'Celo Dollar' as CURRENCY_ENUM,
timestamp: Date.now(),
address: invitedAddress,
comment: SENTINEL_INVITE_COMMENT,
hash: '0x1010',
}

const recieved: HomeTransferFragment = {
__typename: EventTypeNames.Transfer,
type: 'RECEIVED' as TransactionTypes,
type: 'RECEIVED' as TransferTransactionTypes,
value: 587161,
symbol: 'Celo Gold',
symbol: 'Celo Gold' as CURRENCY_ENUM,
timestamp: Date.now(),
address: '0x423043cca38e75d7913504fedfd1dd4539cc55b3',
comment: 'FAKE FAKE FAKE',
hash: '01010',
}
const faucet: HomeTransferFragment = {
__typename: EventTypeNames.Transfer,
type: 'FAUCET' as TransactionTypes,
type: 'FAUCET' as TransferTransactionTypes,
value: 387161,
symbol: 'Celo Dollar',
symbol: 'Celo Dollar' as CURRENCY_ENUM,
timestamp: Date.now(),
address: '0x423043cca38e75d7913504fedfd1dd4539cc55b3',
comment: 'FAKE FAKE FAKE',
hash: '01010',
}
const verificationFee: HomeTransferFragment = {
__typename: EventTypeNames.Transfer,
type: 'VERIFICATION_FEE' as TransactionTypes,
type: 'VERIFICATION_FEE' as TransferTransactionTypes,
value: 0.3,
symbol: 'Celo Gold',
symbol: 'Celo Gold' as CURRENCY_ENUM,
timestamp: Date.now(),
address: '0x423043cca38e75d7913504fedfd1dd4539cc55b3',
comment: 'FAKE FAKE FAKE',
hash: '01010',
}
const verificationReward: HomeTransferFragment = {
__typename: EventTypeNames.Transfer,
Copy link
Contributor

Choose a reason for hiding this comment

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

Why don't we need these graphQL provided __typename properties anymore? Is it cause @jeanregisser split the home feed and exchange feed queries in bc-api?

Copy link
Contributor Author

@annakaz annakaz Dec 18, 2019

Choose a reason for hiding this comment

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

Yeah we don't need them since we know the EventType from the transaction type

type: 'VERIFICATION_REWARD' as TransactionTypes,
type: 'VERIFICATION_REWARD' as TransferTransactionTypes,
value: 9371,
symbol: 'Celo Dollar',
symbol: 'Celo Dollar' as CURRENCY_ENUM,
timestamp: Date.now(),
address: '0x423043cca38e75d7913504fedfd1dd4539cc55b3',
comment: 'FAKE FAKE FAKE',
Expand Down
48 changes: 29 additions & 19 deletions packages/mobile/src/apollo/types.ts
Original file line number Diff line number Diff line change
@@ -1,41 +1,51 @@
import { Query } from 'react-apollo'
import { TransactionTypes } from 'src/transactions/reducer'
import { CURRENCY_ENUM } from 'src/geth/consts'
Copy link
Contributor

Choose a reason for hiding this comment

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

I just tried it and it doesn't work right now, but i think this file originally came from running yarn run build:gen-graphql-types with the blockchain api running locally

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Kk are you saying we should avoid modifying this generated file?

Copy link
Contributor

Choose a reason for hiding this comment

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

This file was originally made that way but most of that auto gen code has been changed over time. I've made modifications here too.

Copy link
Contributor

Choose a reason for hiding this comment

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

I believe we diverged away from the auto gen types because of the differing shapes of our standby tx and graphql txs

import {
TransactionStatus,
TransactionTypes,
TransferTransactionTypes,
} from 'src/transactions/reducer'

export interface UserTransactionsVariables {
address: string
}

export enum EventTypeNames {
Exchange = 'Exchange',
Transfer = 'Transfer',
}

export interface HomeExchangeFragment {
__typename: EventTypeNames.Exchange
type: TransactionTypes
hash: string
export interface ExchangeTransaction {
type: TransactionTypes.EXCHANGE
id?: string
hash?: string
inValue: number
outValue: number
inSymbol: string
outSymbol: string
inSymbol: CURRENCY_ENUM
outSymbol: CURRENCY_ENUM
timestamp: number
status?: TransactionStatus
}

export interface HomeTransferFragment {
__typename: EventTypeNames.Transfer
type: TransactionTypes
hash: string
export interface TransferTransaction {
type: TransferTransactionTypes
id?: string
hash?: string
value: number
symbol: string
symbol: CURRENCY_ENUM
timestamp: number
address: string
comment: string
status?: TransactionStatus
// TODO: fee needs to be added here
}

export type Event = HomeExchangeFragment | HomeTransferFragment
export interface HomeExchangeFragment extends ExchangeTransaction {
hash: string
}

export interface HomeTransferFragment extends TransferTransaction {
hash: string
}

export type Transaction = ExchangeTransaction | TransferTransaction
export interface UserTransactionsData {
events?: Event[] | null
events?: Transaction[] | null
}

export default class UserTransactionsQuery extends Query<
Expand Down
2 changes: 1 addition & 1 deletion packages/mobile/src/escrow/saga.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ function* registerStandbyTransaction(id: string, value: string, address: string)
id,
type: TransactionTypes.ESCROW_SENT,
status: TransactionStatus.Pending,
value,
value: +value,
Copy link
Contributor Author

@annakaz annakaz Dec 17, 2019

Choose a reason for hiding this comment

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

Note that storing StandbyTransaction as number instead of string does not impact precision, as the confirmed transaction value is stored as a number anyway. Did not want to modify graphQL to change both to strings so as not to overlap with @jeanregisser 's local currency work

symbol: CURRENCY_ENUM.DOLLAR,
timestamp: Math.floor(Date.now() / 1000),
address,
Expand Down
4 changes: 2 additions & 2 deletions packages/mobile/src/exchange/saga.ts
Original file line number Diff line number Diff line change
Expand Up @@ -235,9 +235,9 @@ function* createStandbyTx(
type: TransactionTypes.EXCHANGE,
status: TransactionStatus.Pending,
inSymbol: makerToken,
inValue: makerAmount.toString(),
inValue: makerAmount.toNumber(),
outSymbol: makerToken === CURRENCY_ENUM.DOLLAR ? CURRENCY_ENUM.GOLD : CURRENCY_ENUM.DOLLAR,
outValue: takerAmount.toString(),
outValue: takerAmount.toNumber(),
timestamp: Math.floor(Date.now() / 1000),
})
)
Expand Down
4 changes: 2 additions & 2 deletions packages/mobile/src/home/TransactionsList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import * as React from 'react'
import { WithNamespaces, withNamespaces } from 'react-i18next'
import { connect } from 'react-redux'
import componentWithAnalytics from 'src/analytics/wrapper'
import UserTransactionsQuery, { Event, UserTransactionsData } from 'src/apollo/types'
import UserTransactionsQuery, { Transaction, UserTransactionsData } from 'src/apollo/types'
import { Namespaces } from 'src/i18n'
import { RootState } from 'src/redux/reducers'
import { removeStandbyTransaction } from 'src/transactions/actions'
Expand Down Expand Up @@ -77,7 +77,7 @@ export class TransactionsList extends React.PureComponent<Props> {
}

const events = data.events
const queryDataTxIDs = new Set(events.map((event: Event) => event.hash))
const queryDataTxIDs = new Set(events.map((event: Transaction) => event.hash))
const inQueryTxs = (tx: StandbyTransaction) =>
tx.hash && queryDataTxIDs.has(tx.hash) && tx.status !== TransactionStatus.Failed
const filteredStandbyTxs = this.props.standbyTransactions.filter(inQueryTxs)
Expand Down
4 changes: 2 additions & 2 deletions packages/mobile/src/stableToken/saga.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ describe('stableToken saga', () => {
type: TransactionTypes.SENT,
comment: COMMENT,
status: TransactionStatus.Pending,
value: BALANCE,
value: +BALANCE,
symbol: CURRENCY_ENUM.DOLLAR,
timestamp: Math.floor(Date.now() / 1000),
address: mockAccount,
Expand All @@ -97,7 +97,7 @@ describe('stableToken saga', () => {
type: TransactionTypes.SENT,
comment: COMMENT,
status: TransactionStatus.Pending,
value: BALANCE,
value: +BALANCE,
symbol: CURRENCY_ENUM.DOLLAR,
timestamp: Math.floor(Date.now() / 1000),
address: mockAccount,
Expand Down
2 changes: 1 addition & 1 deletion packages/mobile/src/tokens/saga.ts
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ export function tokenTransferFactory({
type: TransactionTypes.SENT,
comment,
status: TransactionStatus.Pending,
value: amount.toString(),
value: +amount,
symbol: currency,
timestamp: Math.floor(Date.now() / 1000),
address: recipientAddress,
Expand Down
4 changes: 1 addition & 3 deletions packages/mobile/src/transactions/ExchangeFeedItem.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import * as React from 'react'
import 'react-native'
import { Provider } from 'react-redux'
import * as renderer from 'react-test-renderer'
import { EventTypeNames } from 'src/apollo/types'
import { CURRENCY_ENUM } from 'src/geth/consts'
import { ExchangeFeedItem } from 'src/transactions/ExchangeFeedItem'
import { TransactionStatus, TransactionTypes } from 'src/transactions/reducer'
Expand All @@ -28,8 +27,7 @@ describe('ExchangeFeedItem', () => {
<Provider store={createMockStore({})}>
<ExchangeFeedItem
status={TransactionStatus.Complete}
__typename={EventTypeNames.Exchange}
type={'EXCHANGE' as TransactionTypes}
type={'EXCHANGE' as TransactionTypes.EXCHANGE}
Copy link
Contributor

Choose a reason for hiding this comment

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

same enum comment

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same as above

hash={'0x'}
inValue={1}
outValue={10}
Expand Down
15 changes: 7 additions & 8 deletions packages/mobile/src/transactions/TransactionFeed.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import * as React from 'react'
import 'react-native'
import { Provider } from 'react-redux'
import * as renderer from 'react-test-renderer'
import { EventTypeNames, HomeExchangeFragment } from 'src/apollo/types'
import { HomeExchangeFragment } from 'src/apollo/types'
import { CURRENCY_ENUM } from 'src/geth/consts'
import { StandbyTransaction, TransactionStatus, TransactionTypes } from 'src/transactions/reducer'
import TransactionFeed, {
Expand All @@ -20,7 +20,7 @@ const standbyTransactions: StandbyTransaction[] = [
type: TransactionTypes.SENT,
comment: 'Eye for an Eye',
status: TransactionStatus.Pending,
value: '100',
value: 100,
symbol: CURRENCY_ENUM.DOLLAR,
timestamp: 1542406112,
address: '0072bvy2o23u',
Expand All @@ -30,17 +30,17 @@ const standbyTransactions: StandbyTransaction[] = [
type: TransactionTypes.EXCHANGE,
status: TransactionStatus.Pending,
inSymbol: CURRENCY_ENUM.DOLLAR,
inValue: '20',
inValue: 20,
outSymbol: CURRENCY_ENUM.GOLD,
outValue: '30',
outValue: 30,
timestamp: 1542409112,
},
{
id: '0113',
type: TransactionTypes.NETWORK_FEE,
comment: '',
status: TransactionStatus.Pending,
value: '0.0001',
value: 0.0001,
symbol: CURRENCY_ENUM.DOLLAR,
timestamp: 1542406112,
address: '0072bvy2o23u',
Expand All @@ -52,17 +52,16 @@ const failedExchange: StandbyTransaction[] = [
type: TransactionTypes.EXCHANGE,
status: TransactionStatus.Failed,
inSymbol: CURRENCY_ENUM.DOLLAR,
inValue: '20',
inValue: 20,
outSymbol: CURRENCY_ENUM.GOLD,
outValue: '30',
outValue: 30,
timestamp: 1542409112,
id: '0x00000000000000000001',
},
]

const exchangeEvents: HomeExchangeFragment[] = [
{
__typename: EventTypeNames.Exchange,
type: TransactionTypes.EXCHANGE,
inValue: 30,
outValue: 200,
Expand Down
Loading