Skip to content

feat: manage recent pending transactions - #6055

Merged
guanbinrui merged 5 commits into
developfrom
feat/pending-tx
Apr 21, 2022
Merged

feat: manage recent pending transactions#6055
guanbinrui merged 5 commits into
developfrom
feat/pending-tx

Conversation

@UncleBill

Copy link
Copy Markdown
Contributor

@github-actions

github-actions Bot commented Apr 11, 2022

Copy link
Copy Markdown
Contributor

@github-actions
github-actions Bot temporarily deployed to pull request April 11, 2022 01:59 Inactive
@UncleBill UncleBill changed the title feat: recent transactions feat: manage recent pending transactions Apr 11, 2022
@UncleBill
UncleBill force-pushed the feat/pending-tx branch 2 times, most recently from 5461e1d to 1b3ea84 Compare April 11, 2022 02:06
@github-actions
github-actions Bot temporarily deployed to pull request April 11, 2022 02:11 Inactive
@github-actions
github-actions Bot temporarily deployed to pull request April 11, 2022 04:55 Inactive
Comment thread packages/mask/src/components/shared/WalletStatusBox/TransactionList.tsx Outdated
}

const domainAddressMap: Record<string, string[]> = {
'uniswap.org': collect(Trader, [

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Will these links use for opening in the browser? Lost protocol.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

No, there is no such requirement yet.

@UncleBill
UncleBill force-pushed the feat/pending-tx branch 2 times, most recently from 0db471a to 19d9af7 Compare April 12, 2022 02:15
@github-actions
github-actions Bot temporarily deployed to pull request April 12, 2022 02:20 Inactive
@UncleBill

Copy link
Copy Markdown
Contributor Author

@DimensionDev/qa Please help to verify.

Comment thread packages/mask/src/components/shared/WalletStatusBox/TransactionList.tsx Outdated
Comment thread packages/mask/src/components/shared/WalletStatusBox/TransactionList.tsx Outdated
const txData = tx.payload?.params?.[0].data as string | undefined
const { value: functionName } = useAsync(async () => {
const name = await getContractFunctionName(txData)
if (name === TransactionType.CREATE_RED_PACKET) return 'Create Lucky Drop'

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What about other TrasnactionType?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It's naming migration.
I have rename the enum, no need to do such fixing anymore.

Comment thread packages/mask/src/components/shared/WalletStatusBox/TransactionList.tsx Outdated
Comment thread packages/mask/src/components/shared/WalletStatusBox/TransactionList.tsx Outdated
Comment thread packages/mask/src/components/shared/WalletStatusBox/TransactionList.tsx Outdated
Comment thread packages/mask/src/components/shared/WalletStatusBox/TransactionList.tsx Outdated
Comment thread packages/mask/src/plugins/Wallet/hooks/useClearRecentTransactions.ts Outdated
Comment thread packages/mask/src/plugins/Wallet/hooks/useRemoveRecentTransaction.ts Outdated
@github-actions
github-actions Bot temporarily deployed to pull request April 12, 2022 12:59 Inactive
@UncleBill
UncleBill requested a review from guanbinrui April 13, 2022 02:23
@UncleBill

Copy link
Copy Markdown
Contributor Author

@DimensionDev/qa Please help to verify.

Comment thread packages/mask/src/components/shared/WalletStatusBox/TransactionList.tsx Outdated
Comment thread packages/mask/src/components/shared/WalletStatusBox/TransactionList.tsx Outdated
@github-actions
github-actions Bot temporarily deployed to pull request April 19, 2022 03:56 Inactive
frozenTxes.current = pendingTransactions.slice(0, 2)
// trigger rerender in next loop
Promise.resolve().then(() => {
setMeltedTxHashes([])

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
setMeltedTxHashes([])
setMeltedTxHashes(EMPTY_LIST)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Need to setState a new array to trigger rerendering.

className={classes.transactionList}
transactions={transactions}
onClear={(tx) => {
setMeltedTxHashes((list) => [...list, tx.hash])

@guanbinrui guanbinrui Apr 20, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Use uniqBy to remove duplicate items.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

No need, meltedTxHashes just for checking existing.

Comment thread packages/mask/src/components/shared/WalletStatusBox/TransactionList.tsx Outdated
}
return (transaction.type ?? '').replace(/_/g, ' ')
}, [transaction.type])
const transactionType = (transaction.type ?? '').replace(/_/g, ' ')

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Maybe extract format function?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Or maybe we should add a uiName field to Transaction

@github-actions
github-actions Bot temporarily deployed to pull request April 20, 2022 04:54 Inactive
const web3 = useWeb3()
const { value: targetAddress } = useAsync(async () => {
if (tx.receipt?.contractAddress) return tx.receipt.contractAddress
if (tx.payload?.params?.[0].to) return tx.payload.params[0].to as string

@nuanyang233 nuanyang233 Apr 20, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You can use the lodash head or first

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Access tx.payload.params twice, don't wanna use head twice.

setTxStatus(TransactionStatusType.SUCCEED)
}
break
case TransactionStateType.FAILED:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

you're missing break, and look like the if(tx.hash === data.receipt.transactionHash) move outside the switch be better.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

you're missing break

We can omit it at the final branch

look like the if(tx.hash === data.receipt.transactionHash) move outside the switch be better.

When a TX receipt, it also has receipt.hash

    | {
          type: TransactionStateType.RECEIPT
          receipt: TransactionReceipt
      }
    | {
          type: TransactionStateType.CONFIRMED
          no: number
          receipt: TransactionReceipt
          reason?: string
      }

</Grid>
<Grid item className={classes.cell} flexGrow={1} md={4} justifyContent="right">
<Typography variant="body1">
{address && isSameAddress(domain, address) ? Utils?.formatAddress?.(address, 4) : domain || address}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

So the domain is an address? If so, I think the name is easy to cause misunderstanding

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

domainOrAddress

@UncleBill
UncleBill force-pushed the feat/pending-tx branch 3 times, most recently from 82b7c6a to c0f77ce Compare April 20, 2022 16:09
@github-actions
github-actions Bot temporarily deployed to pull request April 20, 2022 16:16 Inactive
@github-actions
github-actions Bot temporarily deployed to pull request April 21, 2022 01:53 Inactive
@UncleBill
UncleBill requested a review from nuanyang233 April 21, 2022 01:54
@guanbinrui
guanbinrui merged commit 1454211 into develop Apr 21, 2022
@guanbinrui
guanbinrui deleted the feat/pending-tx branch April 21, 2022 01:57
yanzhihong23 added a commit that referenced this pull request Apr 24, 2022
* develop: (143 commits)
  feat: tips entrance (#6012)
  refactor: remove unused plugin entries
  chore: patch @solana/web3.js (#6125)
  New translations en-US.json (Quenya) (#6131)
  New translations en-US.json (Quenya) (#6126)
  feat: new intial persona page (#6122)
  chore: turn on fortmatic (#6127)
  fix: if phantom is locked, might not able to open its popup (#6115)
  fix: reduce jsx nested (#6124)
  fix: duplicate render by unique key (#6123)
  docs(i18n): sync i18n files from Crowdin (#6119)
  feat: manage recent pending transactions (#6055)
  chore: add conflux config (#6112)
  fix: type error (#6113)
  feat: improve mask popup (#5986)
  feat: new app board (#6018)
  fix(tip): set significant to 6 for formatting max amount (#6062)
  fix: ITO history default icon (#6078)
  feat: add <ConcealableTabs /> (#6073)
  fix: if gulp biuld-ci fails, ci should fail as well (#6107)
  ...
yanzhihong23 added a commit that referenced this pull request Apr 24, 2022
* develop: (94 commits)
  fix: mnemonic words exports (#6136)
  feat: tips entrance (#6012)
  refactor: remove unused plugin entries
  chore: patch @solana/web3.js (#6125)
  New translations en-US.json (Quenya) (#6131)
  New translations en-US.json (Quenya) (#6126)
  feat: new intial persona page (#6122)
  chore: turn on fortmatic (#6127)
  fix: if phantom is locked, might not able to open its popup (#6115)
  fix: reduce jsx nested (#6124)
  fix: duplicate render by unique key (#6123)
  docs(i18n): sync i18n files from Crowdin (#6119)
  feat: manage recent pending transactions (#6055)
  chore: add conflux config (#6112)
  fix: type error (#6113)
  feat: improve mask popup (#5986)
  feat: new app board (#6018)
  fix(tip): set significant to 6 for formatting max amount (#6062)
  fix: ITO history default icon (#6078)
  feat: add <ConcealableTabs /> (#6073)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants