Fix/tx request decoding

app/tray/Account/Requests/TransactionRequest/TxMainNew/overview.js

@@ -4,14 +4,13 @@ import link from '../../../../../../resources/link'
 import EnsOverview from '../../Ens'
 
 import svg from '../../../../../../resources/svg'
+import { isNonZeroHex } from '../../../../../../resources/utils'
 
 import { Cluster, ClusterRow, ClusterValue } from '../../../../../../resources/Components/Cluster'
 import { DisplayValue } from '../../../../../../resources/Components/DisplayValue'
 import RequestHeader from '../../../../../../resources/Components/RequestHeader'
 
-const isNonZeroHex = (hex) => !!hex && !['0x', '0x0'].includes(hex)
-
-const ContractCallOverview = ({ method }) => {
+const SimpleContractCallOverview = ({ method }) => {
   const body = method ? `Calling Contract Method ${method}` : 'Calling Contract'
 
   return <div className='_txDescriptionSummaryLine'>{body}</div>
@@ -32,7 +31,8 @@ const ApproveOverview = ({ amount, decimals, symbol }) => {
   )
 }
 
-const SendOverview = ({ amount, decimals, symbol }) => {
+const SendOverview = ({ req, symbol, decimals, amount: ammt }) => {
+  const amount = ammt || req.data.value
   return (
     <div>
       <span>{'Send'}</span>
@@ -50,6 +50,11 @@ const SendOverview = ({ amount, decimals, symbol }) => {
 const DeployContractOverview = () => <div>Deploying Contract</div>
 const DataOverview = () => <div>Sending data</div>
 
+const ContractCallOverview = ({ req }) => {
+  const { decodedData: { method } = {} } = req
+  return renderRecognizedActions(req) || <SimpleContractCallOverview method={method} />
+}
+
 const actionOverviews = {
   'erc20:transfer': SendOverview,
   'erc20:approve': ApproveOverview,
@@ -60,12 +65,12 @@ const renderActionOverview = (action, index) => {
   const { id = '', data } = action
   const key = id + index
   const [actionClass, actionType] = id.split(':')
-  const ActionOverview = actionOverviews[id] || ContractCallOverview
+  const ActionOverview = actionOverviews[id] || SimpleContractCallOverview
 
   return <ActionOverview key={key} type={actionType} {...{ ...data }} />
 }
 
-function renderRecognizedAction(req) {
+function renderRecognizedActions(req) {
   const { recognizedActions: actions = [] } = req
 
   return !actions.length ? (
@@ -75,6 +80,13 @@ function renderRecognizedAction(req) {
   )
 }
 
+const BaseOverviews = {
+  CONTRACT_DEPLOY: DeployContractOverview,
+  CONTRACT_CALL: ContractCallOverview,
+  SEND_DATA: DataOverview,
+  NATIVE_TRANSFER: SendOverview
+}
+
 const TxOverview = ({
   req,
   chainName,
@@ -85,35 +97,17 @@ const TxOverview = ({
   simple,
   valueColor
 }) => {
-  const { recipientType, decodedData: { method } = {}, data: tx = {} } = req
-  const { to, value, data: calldata } = tx
-
-  const isContractDeploy = !to && isNonZeroHex(calldata)
-  const isSend = recipientType === 'external' || isNonZeroHex(value)
-  const isContractCall = recipientType !== 'external' && isNonZeroHex(calldata)
+  const { data: tx = {}, classification } = req
+  const { data: calldata } = tx
 
-  let description
-
-  // TODO: empty vs unknown transactions
-
-  if (isContractDeploy) {
-    description = <DeployContractOverview />
-  } else if (isContractCall) {
-    description = renderRecognizedAction(req)
-
-    if (!description && !!method) {
-      description = <ContractCallOverview method={method} />
-    }
-  } else if (isSend) {
-    description = <SendOverview amount={value} decimals={18} symbol={symbol} />
-  } else if (isNonZeroHex(calldata)) {
-    description = <DataOverview />
-  }
+  const Description = BaseOverviews[classification]
 
   if (simple) {
     return (
       <div className='txDescriptionSummaryStandalone'>
-        <span className='txDescriptionSummaryStandaloneWrap'>{description}</span>
+        <span className='txDescriptionSummaryStandaloneWrap'>
+          <Description req={req} decimals={18} symbol={symbol} />
+        </span>
       </div>
     )
   } else {
@@ -132,7 +126,9 @@ const TxOverview = ({
                   <div className='requestItemTitleSubIcon'>{svg.window(10)}</div>
                   <div className='requestItemTitleSubText'>{originName}</div>
                 </div>
-                <div className='_txDescriptionSummaryMain'>{description}</div>
+                <div className='_txDescriptionSummaryMain'>
+                  <Description req={req} decimals={18} symbol={symbol} />
+                </div>
               </RequestHeader>
             </div>
           </ClusterValue>

main/accounts/Account/index.ts

@@ -284,12 +284,11 @@ class FrameAccount {
     if (to) {
       // Get recipient identity
       try {
-        const recipient = await reveal.identity(to, parseInt(chainId, 16))
+        const recipient = await reveal.identity(to)
         const knownTxRequest = this.requests[req.handlerId] as TransactionRequest
 
         if (recipient && knownTxRequest) {
           knownTxRequest.recipient = recipient.ens
-          knownTxRequest.recipientType = recipient.type
           this.update()
         }
       } catch (e) {

main/accounts/index.ts

@@ -33,7 +33,6 @@ import { ActionType } from '../transaction/actions'
 import { openBlockExplorer } from '../windows/window'
 import { ApprovalType } from '../../resources/constants'
 import { accountNS } from '../../resources/domain/account'
-import { TokenData } from '../contracts/erc20'
 
 function notify(title: string, body: string, action: (event: Electron.Event) => void) {
   const notification = new Notification({ title, body })

main/accounts/types.ts

@@ -89,6 +89,13 @@ export interface Permit {
   nonce: string | number
 }
 
+export enum TxClassification {
+  CONTRACT_DEPLOY = 'CONTRACT_DEPLOY',
+  CONTRACT_CALL = 'CONTRACT_CALL',
+  SEND_DATA = 'SEND_DATA',
+  NATIVE_TRANSFER = 'NATIVE_TRANSFER'
+}
+
 export interface TransactionRequest extends AccountRequest<'transaction'> {
   payload: RPC.SendTransaction.Request
   data: TransactionData
@@ -110,6 +117,7 @@ export interface TransactionRequest extends AccountRequest<'transaction'> {
   feesUpdatedByUser: boolean
   recipientType: string
   recognizedActions: Action<unknown>[]
+  classification: TxClassification
 }
 
 export type TypedData<T extends MessageTypes = MessageTypes> = BaseTypedMessage<T>

main/provider/index.ts

@@ -20,12 +20,13 @@ import accounts, {
 import Chains, { Chain } from '../chains'
 import { getSignerType, Type as SignerType } from '../../resources/domain/signer'
 import { TransactionData } from '../../resources/domain/transaction'
-import { populate as populateTransaction, maxFee } from '../transaction'
+import { populate as populateTransaction, maxFee, classifyTransaction } from '../transaction'
 import FrameAccount from '../accounts/Account'
 import { capitalize } from '../../resources/utils'
 import { ApprovalType } from '../../resources/constants'
 import { createObserver as AssetsObserver, loadAssets } from './assets'
 import { getVersionFromTypedData } from './typedData'
+import reveal from '../reveal'
 
 import {
   checkExistingNonceGas,
@@ -45,10 +46,11 @@ import {
   createOriginChainObserver as OriginChainObserver,
   getActiveChains
 } from './chains'
-import type {
+import {
   EIP2612TypedData,
   LegacyTypedData,
   PermitSignatureRequest,
+  TxClassification,
   TypedData,
   TypedMessage
 } from '../accounts/types'
@@ -448,33 +450,38 @@ export class Provider extends EventEmitter {
       const gas = gasFees(rawTx)
       const chainConfig = this.connection.connections['ethereum'][parseInt(rawTx.chainId)].chainConfig
 
-      const estimateGas = rawTx.gasLimit
-        ? Promise.resolve(rawTx)
-        : this.getGasEstimate(rawTx)
-            .then((gasLimit) => ({ ...rawTx, gasLimit }))
-            .catch((err) => {
-              approvals.push({
-                type: ApprovalType.GasLimitApproval,
-                data: {
-                  message: err.message,
-                  gasLimit: '0x00'
-                }
-              })
+      const estimateGasLimit = async () => {
+        try {
+          return await this.getGasEstimate(rawTx)
+        } catch (error) {
+          approvals.push({
+            type: ApprovalType.GasLimitApproval,
+            data: {
+              message: (error as Error).message,
+              gasLimit: '0x00'
+            }
+          })
+          return '0x00'
+        }
+      }
 
-              return { ...rawTx, gasLimit: '0x00' }
-            })
+      const [gasLimit, recipientType] = await Promise.all([
+        rawTx.gasLimit ?? estimateGasLimit(),
+        rawTx.to ? reveal.resolveEntityType(rawTx.to, parseInt(rawTx.chainId, 16)) : ''
+      ])
 
-      estimateGas
-        .then((tx) => {
-          const populatedTransaction = populateTransaction(tx, chainConfig, gas)
+      const tx = { ...rawTx, gasLimit, recipientType }
 
-          log.info({ populatedTransaction })
+      try {
+        const populatedTransaction = populateTransaction(tx, chainConfig, gas)
+        const checkedTransaction = checkExistingNonceGas(populatedTransaction)
 
-          return populatedTransaction
-        })
-        .then((tx) => checkExistingNonceGas(tx))
-        .then((tx) => cb(null, { tx, approvals }))
-        .catch(cb)
+        log.verbose('Succesfully populated transaction', checkedTransaction)
+
+        cb(null, { tx: checkedTransaction, approvals })
+      } catch (error) {
+        return cb(error as Error)
+      }
     } catch (e) {
       log.error('error creating transaction', e)
       cb(e as Error)
@@ -515,9 +522,10 @@ export class Provider extends EventEmitter {
         }
 
         const handlerId = this.addRequestHandler(res)
-        const { feesUpdated, ...data } = txMetadata.tx
 
-        const req = {
+        const { feesUpdated, recipientType, ...data } = txMetadata.tx
+
+        const unclassifiedReq = {
           handlerId,
           type: 'transaction',
           data,
@@ -526,9 +534,16 @@ export class Provider extends EventEmitter {
           origin: payload._origin,
           approvals: [],
           feesUpdatedByUser: feesUpdated || false,
-          recipientType: '',
+          recipientType,
           recognizedActions: []
-        } as TransactionRequest
+        } as Omit<TransactionRequest, 'classification'>
+
+        const classification = classifyTransaction(unclassifiedReq)
+
+        const req = {
+          ...unclassifiedReq,
+          classification
+        }
 
         accounts.addRequest(req, res)
 

main/reveal/index.ts

@@ -167,6 +167,7 @@ const surface = {
     // TODO: Check the address against previously verified contracts
     return { type, ens }
   },
+  resolveEntityType,
   decode: async (contractAddress: string = '', chainId: number, calldata: string) => {
     // Decode calldata
     const contractSources: ContractSource[] = [{ name: 'ERC-20', source: 'Generic ERC-20', abi: erc20Abi }]

main/transaction/index.ts

@@ -5,7 +5,9 @@ import { Common } from '@ethereumjs/common'
 
 import { AppVersion, SignerSummary } from '../signers/Signer'
 import { GasFeesSource, TransactionData, typeSupportsBaseFee } from '../../resources/domain/transaction'
+import { isNonZeroHex } from '../../resources/utils'
 import chainConfig from '../chains/config'
+import { TransactionRequest, TxClassification } from '../accounts/types'
 
 const londonHardforkSigners: SignerCompatibilityByVersion = {
   seed: () => true,
@@ -173,4 +175,16 @@ async function sign(rawTx: TransactionData, signingFn: (tx: TypedTransaction) =>
   })
 }
 
-export { maxFee, populate, sign, signerCompatibility, londonToLegacy }
+function classifyTransaction({
+  payload: { params },
+  recipientType
+}: Omit<TransactionRequest, 'classification'>): TxClassification {
+  const { to, data = '0x' } = params[0]
+
+  if (!to) return TxClassification.CONTRACT_DEPLOY
+  if (recipientType === 'external' && data.length > 2) return TxClassification.SEND_DATA
+  if (isNonZeroHex(data) && recipientType !== 'external') return TxClassification.CONTRACT_CALL
+  return TxClassification.NATIVE_TRANSFER
+}
+
+export { maxFee, populate, sign, signerCompatibility, londonToLegacy, classifyTransaction }

resources/domain/transaction/index.ts

@@ -13,6 +13,7 @@ export interface TransactionData extends Omit<JsonTx, 'chainId' | 'type'> {
   chainId: string
   type: string
   gasFeesSource: GasFeesSource
+  recipientType?: string
 }
 
 export function typeSupportsBaseFee(type: string) {

resources/utils/index.ts

@@ -79,6 +79,9 @@ function getAddress(address: Address) {
     return lowerCaseAddress
   }
 }
+function isNonZeroHex(hex: string) {
+  return !!hex && !['0x', '0x0'].includes(hex)
+}
 
 export {
   getErrorCode,
@@ -98,5 +101,6 @@ export {
   gweiToWeiHex,
   getAddress,
   stripHexPrefix,
-  matchFilter
+  matchFilter,
+  isNonZeroHex
 }

test/app/tray/Account/Requests/TransactionRequest/index.test.js

@@ -4,6 +4,7 @@ import Restore from 'react-restore'
 import store from '../../../../../../main/store'
 import { screen, render } from '../../../../../componentSetup'
 import TxRequestComponent from '../../../../../../app/tray/Account/Requests/TransactionRequest'
+import { TxClassification } from '../../../../../../main/accounts/types'
 
 jest.mock('../../../../../../main/store/persist')
 jest.mock('../../../../../../resources/link', () => ({ rpc: jest.fn() }))
@@ -30,7 +31,8 @@ describe('confirm', () => {
       status: 'confirming',
       data: {
         chainId: '0x89'
-      }
+      },
+      classification: TxClassification.NATIVE_TRANSFER
     }
 
     addRequest(req)
@@ -50,7 +52,8 @@ describe('confirm', () => {
       recipientType: 'external',
       data: {
         chainId: '0x89'
-      }
+      },
+      classification: TxClassification.NATIVE_TRANSFER
     }
 
     addRequest(req)

test/main/provider/index.test.js

@@ -20,6 +20,9 @@ let accountRequests = []
 jest.mock('../../../main/store')
 jest.mock('../../../main/chains', () => ({ send: jest.fn(), syncDataEmit: jest.fn(), on: jest.fn() }))
 jest.mock('../../../main/accounts', () => ({}))
+jest.mock('../../../main/reveal', () => ({
+  resolveEntityType: jest.fn().mockResolvedValue('external')
+}))
 jest.mock('../../../main/provider/helpers', () => {
   const helpers = jest.requireActual('../../../main/provider/helpers')