Display CIP8 profile info in Transaction Feed #37

isabellewei · 2021-02-20T01:56:58Z

migrated from celo-org/celo-monorepo#6605

Description

This PR includes a refactor of how we handle recipients. When handling recipient info, the paradigm is to put all information into a Recipient object which is passed between components/functions. This parameter is made mandatory, instead of individually passing address/number/name/thumbnail variables individually.

When a user sends a transaction, it uploads symmetric keys which grants the recipient permission to access their profile information. When the recipient receives the transaction, it downloads and decrypts this profile info, saving it in redux.

Transactions in the feed displays CIP8 profile information if it is available for the recipient. This PR does not handle contact list and send flow UI.

Other/Drive-by Changes

Removed recipients from the persist blacklist, because I want to persist valoraRecipientsCache. Previously the only variable in the recipients reducer was the recipientCache (renamed to phoneRecipientCache ), which I guess was blacklisted because we reconstruct it at the start of each app session. Not sure what the impact of this is

Tested

Sending transactions between Android device and iOS emulator

Related issues

Fixes #18 partially

…rg/wallet into isabellewei/displayTransactionProfiles

gnardini

Really excited for this changes to land 🙌

I tried building to see this in action but got some errors:

When running yarn build:wallet from the repo root I get a bunch of errors and it gets stuck there.
After that, I install the ios pods manually but when I run yarn dev:ios or try to install from XCode I get the following errors:
- /Users/repoDirectory/wallet/node_modules/react-native-geth/ios/RNGeth.swift:462:39: Value of type 'GethKeyStore' has no member 'computeECDHSharedSecret'
- /Users/repoDirectory/wallet/node_modules/react-native-geth/ios/RNGeth.swift:486:37: Value of type 'GethKeyStore' has no member 'decrypt'
  Do you know what may be the cause or how to fix it?

packages/mobile/src/components/AvatarSelf.tsx

packages/mobile/src/recipients/recipient.ts

packages/mobile/src/firebase/notifications.test.ts

packages/mobile/src/identity/reducer.ts

packages/mobile/src/send/Send.tsx

packages/mobile/src/transactions/saga.ts

jeanregisser

Wow this is looking great! 👍
Thanks for the big effort on this.
And I'm so glad we are using TypeScript 😄

packages/mobile/src/account/selectors.ts

jeanregisser · 2021-02-26T14:40:22Z

packages/mobile/src/analytics/Properties.tsx

+    // TODO: decide what recipient info to collect, now that RecipientKind doesn't exist
    usedSearchBar: boolean


cc @tarikbellamine

jeanregisser · 2021-02-26T15:31:26Z

packages/mobile/src/components/Avatar.tsx

-
-export function getE164Number(e164Number?: string, recipient?: Recipient) {
-  return e164Number || (recipient && recipient.e164PhoneNumber)
+export function getE164Number(recipient: Recipient, e164Number?: string) {


From the outside, the ordering of the parameters seems to indicate the recipient number would be used first.
But the implementation does the opposite.
Can we keep the existing ordering?

The order of the parameters has to be recipient first, because it's a required parameter. I flipped the OR statement instead

packages/mobile/src/components/ContactCircle.test.tsx

jeanregisser · 2021-02-26T15:39:42Z

packages/mobile/src/components/ContactCircle.tsx

@@ -52,9 +38,8 @@ function ContactCircle({ contact, size, thumbnailPath, name, address, style }: P
    }

    const fontColor = getAddressForegroundColor(address || '0x0')
-    // Mobile # is what default display name when contact isn't saved
-    if (displayName && displayName !== 'Mobile #') {


Is this case still handled somewhere else?

Yeah! This is just checking that we don't have an actual name for the recipient, because the name field used to be mandatory and we would save display info in the name variable. Now we can directly check if the name variable exists or not

jeanregisser · 2021-02-26T16:42:26Z

packages/mobile/src/recipients/recipient.ts

+  const nameA = a.name!.toUpperCase()
+  const nameB = b.name!.toUpperCase()


Any way to avoid the !?

packages/mobile/src/send/SendConfirmation.tsx

packages/mobile/src/send/ValidateRecipientIntro.tsx

packages/mobile/src/identity/secureSend.ts

packages/mobile/src/paymentRequest/IncomingPaymentRequestListItem.tsx

…P8valoraIntegration

…n fatal

…rg/wallet into isabellewei/displayTransactionProfiles

isabellewei · 2021-04-28T14:41:00Z

Added the account field to TransferFragment, as the account address of the transaction is needed to fetch CIP8 info. When constructing the StandbyTransaction however, there is currently no way to get the account address of the recipient from their wallet address, so the wallet address is used as a placeholder for the account field. This shouldn't affect the functionality of the standby transaction

isabellewei · 2021-04-28T14:50:07Z

Does this address #36?

No currently you can still only look up the recipient from their address

…rg/wallet into isabellewei/displayTransactionProfiles

…splayTransactionProfiles

gnardini

Amazing work on this!

There is one more issue which is that when a user creates an account if they don't verify their phone number they don't have any funds so they can't register their DEK and therefore a user sending them money won't be able to give them access.

There's no perfect way around this I think but I guess it's just something to keep in mind.

packages/mobile/src/account/selectors.ts

gnardini · 2021-04-29T16:26:51Z

packages/mobile/src/apollo/types.ts

@@ -272,7 +272,8 @@ export interface TransferItemFragment {
  type: TokenTransactionType
  hash: string
  timestamp: number
-  address: string
+  address: string // wallet address (EOA)
+  account: string // account address (MTW)


This naming from the blockchain-api is really confusing 😅

haha definitely, hopefully these comments help

gnardini · 2021-04-29T16:32:29Z

packages/mobile/src/recipients/recipient.ts

+  name?: string
+  contactId?: string // unique ID given by phone OS
+  thumbnailPath?: string
+  displayNumber?: string


If we also add to the inner recipient

e164PhoneNumber?: string address?: string

It will allow us to query using recipient.address or recipient. e164PhoneNumber and will still require one of address and e164PhoneNumber to be present.

Oh this is a great solution, thanks!

packages/mobile/src/recipients/recipient.ts

packages/mobile/src/escrow/EscrowedPaymentListItem.tsx

packages/mobile/src/send/utils.ts

gnardini · 2021-04-29T17:37:20Z

packages/mobile/src/transactions/saga.ts

+function* addProfile(transaction: TransferItemFragment) {
+  const address = transaction.account
+  const newProfile: AddressToRecipient = {}
+  if (transaction.type === TokenTransactionType.Received) {


If we have many incoming transactions from the same address we will fetch their profile info many times. Something we could do is to add a timestamp to the recipient with the date that the data was last refreshed and only read again of more than X time has passed (an hour? a day?)

What do you think? Also, do you think it makes sense to do it in this PR or create a new issue for this?

filed an issue! :) #300

gnardini

Amazing job with this!

ValoraQA · 2021-05-11T12:03:02Z

Hey @isabellewei I have verified this issue using latest released Android internal build V1.14.0 (1004294350) & Test Flight build V1.14.0(58) & observed following:
Observations:

It is observed that, while withdrawing CELO user A(Sender) able to see profile name & profile text as profile picture in Recent Feed when user perform transaction through scanning QR code even user has updated profile particular photo. Where for user B (Receiver) only see random profile picture with phone number. Attachment
Also in above scenario it is observed that, either Receiver or Sender profile name displayed in respective device but not able to see for both. Attachment
It is observe that, user not able to see profile picture when user withdraw or perform transaction without scanning QR code. Only profile name displayed in Activity feed.

Verified On Devices: iPhone SE (14.2), Redmi Note 8 (9.0)

Can you please take a look & let us know if unchecked points are expected or we need to raise tickets for same ?
Thanks..!!

isabellewei requested a review from gnardini February 20, 2021 01:57

migrating PR

9b752fc

isabellewei force-pushed the isabellewei/CIP8valoraIntegration branch from 7e46eb3 to 9b752fc Compare February 20, 2021 02:17

isabellewei requested review from asaj, cmcewen and timmoreton as code owners February 20, 2021 02:17

migrating PR

6ba7a7f

isabellewei force-pushed the isabellewei/displayTransactionProfiles branch from 64bafd3 to 6ba7a7f Compare February 20, 2021 02:28

isabellewei added 6 commits February 22, 2021 18:00

missed changes

39a4b7f

Merge branch 'isabellewei/CIP8valoraIntegration' of github.com:celo-o…

5583ebb

…rg/wallet into isabellewei/displayTransactionProfiles

missed changes

b0d73e9

throw error when uploading

1f468b4

get working in new repo :)

68ec1fa

Merge branch 'isabellewei/CIP8valoraIntegration' of github.com:celo-o…

c273a3f

…rg/wallet into isabellewei/displayTransactionProfiles

gnardini reviewed Feb 25, 2021

View reviewed changes

PR comments

fb9e154

jeanregisser reviewed Feb 26, 2021

View reviewed changes

isabellewei added 13 commits February 26, 2021 17:45

more PR comments

c2609cf

tests

63e5284

stokado changes

dc47a82

Merge branch 'main' of github.com:celo-org/wallet into isabellewei/CI…

e5a5117

…P8valoraIntegration

tests

6b243ca

use DEK as signer and add graphQL account type

044ebde

update tests

dd5fcb3

register DEK on app open

b570f55

use EIP712 and expiration for writing data

5ced14a

Addressing PR comments

44a92a7

merging changes

d590f1a

PR comments

084a6af

merging from main

972e181

isabellewei and others added 5 commits April 27, 2021 14:04

register DEK on account import, failed uploading symmetric keys is no…

e6610db

…n fatal

upload name and picture when restoring account

73be650

update import test

037383d

Merge branch 'isabellewei/CIP8valoraIntegration' of github.com:celo-o…

9bc41dd

…rg/wallet into isabellewei/displayTransactionProfiles

Merge branch 'main' into isabellewei/CIP8valoraIntegration

aa19829

isabellewei and others added 5 commits April 28, 2021 10:52

only add firebase info to addressToDisplayInfo mapping

d66a688

Merge branch 'main' into isabellewei/CIP8valoraIntegration

a332b4b

update snapshots

60bf04a

Merge branch 'isabellewei/CIP8valoraIntegration' of github.com:celo-o…

679e2e9

…rg/wallet into isabellewei/displayTransactionProfiles

Merge branch 'main' of github.com:celo-org/wallet into isabellewei/di…

b629003

…splayTransactionProfiles

isabellewei changed the base branch from isabellewei/CIP8valoraIntegration to main April 29, 2021 00:35

isabellewei changed the base branch from main to isabellewei/CIP8valoraIntegration April 29, 2021 00:36

isabellewei changed the base branch from isabellewei/CIP8valoraIntegration to main April 29, 2021 00:36

gnardini reviewed Apr 29, 2021

View reviewed changes

fix tests

ce31c29

isabellewei mentioned this pull request May 3, 2021

Wait to download CIP8 info for the same address #300

Closed

isabellewei and others added 6 commits May 3, 2021 00:05

PR comments

5f8cbf9

Merge branch 'main' into isabellewei/displayTransactionProfiles

9b76323

Merge conflicts and other small fixes

97e1f70

Fix lint

c38a91e

Fix test

cc36813

Fix linter

d1e2bf6

gnardini approved these changes May 3, 2021

View reviewed changes

isabellewei added the automerge Have PR merge automatically when checks pass label May 3, 2021

mergify bot merged commit 3aff7b6 into main May 3, 2021

isabellewei deleted the isabellewei/displayTransactionProfiles branch May 3, 2021 21:48

jeanregisser mentioned this pull request Mar 31, 2022

Display invitee from contact list in home feed, rather than "unknown" #2265

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Display CIP8 profile info in Transaction Feed #37

Display CIP8 profile info in Transaction Feed #37

isabellewei commented Feb 20, 2021

gnardini left a comment

jeanregisser left a comment

jeanregisser Feb 26, 2021

jeanregisser Feb 26, 2021

isabellewei Apr 16, 2021 •

edited

Loading

jeanregisser Feb 26, 2021

isabellewei Apr 16, 2021 •

edited

Loading

jeanregisser Feb 26, 2021

isabellewei commented Apr 28, 2021 •

edited

Loading

isabellewei commented Apr 28, 2021

gnardini left a comment

gnardini Apr 29, 2021

isabellewei May 3, 2021

gnardini Apr 29, 2021

isabellewei May 3, 2021

gnardini Apr 29, 2021

isabellewei May 3, 2021

gnardini left a comment

ValoraQA commented May 11, 2021 •

edited

Loading

		// TODO: decide what recipient info to collect, now that RecipientKind doesn't exist
		usedSearchBar: boolean

		const nameA = a.name!.toUpperCase()
		const nameB = b.name!.toUpperCase()

Display CIP8 profile info in Transaction Feed #37

Display CIP8 profile info in Transaction Feed #37

Conversation

isabellewei commented Feb 20, 2021

Description

Other/Drive-by Changes

Tested

Related issues

gnardini left a comment

Choose a reason for hiding this comment

jeanregisser left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

isabellewei Apr 16, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

isabellewei Apr 16, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

isabellewei commented Apr 28, 2021 • edited Loading

isabellewei commented Apr 28, 2021

gnardini 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

gnardini left a comment

Choose a reason for hiding this comment

ValoraQA commented May 11, 2021 • edited Loading

isabellewei Apr 16, 2021 •

edited

Loading

isabellewei Apr 16, 2021 •

edited

Loading

isabellewei commented Apr 28, 2021 •

edited

Loading

ValoraQA commented May 11, 2021 •

edited

Loading