Refactor to use UUID instead of random number for txId #20952

OGPoyraz · 2023-09-19T11:18:32Z

Explanation

Core TransactionController creates UUID strings as an id for transactions. On the otherside extension creates random number while creating transactions using createRandomId.

While we are on a mission for TransactionController alignment for both clients, for the sake of standardisation the use of UUIDs is preferred.

So this PR aims to make transaction ids to be UUID instead of random number. And also creates a migration for persisted transactions to migrate from random numbers to UUIDs.

Here is the redux state dump for three completed tx after migration ran over the old ones.
3 transaction state dump.txt

Screenshots/Screencaps

No functional change.

Manual Testing Steps

This PRs manual testing could be tricky since it's using a migration and needs a history of transactions persisted in previous commit. So testing could be done in these steps:

Checkout develop
Send any transaction on any network (just to have some transaction history on the home page)
Checkout 1102-use-uuids-for-transaction-ids-in-extension-transaction-controller
See history persist there without any issue (history should be clickable and detail modal should render without any issue)

Apart from the case above, we need to be able to navigate between transactions on multiple transaction notification:

Try to send some transaction, don't close notification
Open test-dapp on another tab, and do "Personal Sign" (basically creates another transaction)
Try to navigate with the arrows on top of navigation without any issue

Pre-merge author checklist

I've clearly explained:
- What problem this PR is solving
- How this problem was solved
- How reviewers can test my changes
Sufficient automated test coverage has been added

Pre-merge reviewer checklist

Manual testing (e.g. pull and build branch, run in browser, test code being changed)
PR is linked to the appropriate GitHub issue
IF this PR fixes a bug in the release milestone, add this PR to the release milestone

If further QA is required (e.g. new feature, complex testing steps, large refactor), add the Extension QA Board label.

In this case, a QA Engineer approval will be be required.

metamaskbot · 2023-09-19T12:13:02Z

Builds ready [b7ae5e6]

builds: chrome, firefox
builds (beta): chrome
builds (flask): chrome, firefox
builds (MMI): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4
- common: 0, 1, 2, 3, 4, 5, 6
- content-script: 0
- contentscript: contentscript
- disable-console: disable-console
- inpage: inpage
- policy-load: policy-load
- sentry-install: sentry-install
- ui: 0, 1, 10, 11, 12, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1675 ± 64 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	118	166	139	15	7
		domContentLoaded	1463	2076	1675	134	64
		load	1463	2077	1675	134	64
		domInteractive	1463	2076	1675	134	64

Bundle size diffs [🚨 Warning! Bundle size has increased!]

background: 760 Bytes (0.02%)
ui: 107 Bytes (0.00%)
common: 0 Bytes (0.00%)

codecov · 2023-09-19T12:13:11Z

Codecov Report

Attention: 4 lines in your changes are missing coverage. Please review.

Comparison is base (8a54f65) 68.35% compared to head (d889ece) 68.36%.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop   #20952      +/-   ##
===========================================
+ Coverage    68.35%   68.36%   +0.01%     
===========================================
  Files         1007     1008       +1     
  Lines        40269    40285      +16     
  Branches     10773    10776       +3     
===========================================
+ Hits         27524    27537      +13     
- Misses       12745    12748       +3

Files	Coverage Δ
...s/transactions/EtherscanRemoteTransactionSource.ts	`89.74% <100.00%> (ø)`
app/scripts/controllers/transactions/index.js	`72.64% <ø> (ø)`
...ripts/controllers/transactions/tx-state-manager.js	`92.31% <ø> (ø)`
app/scripts/migrations/099.ts	`100.00% <100.00%> (ø)`
app/scripts/migrations/index.js	`100.00% <ø> (ø)`
shared/constants/transaction.ts	`100.00% <ø> (ø)`
ui/ducks/app/app.ts	`56.82% <ø> (ø)`
...saction-base/confirm-transaction-base.container.js	`80.36% <100.00%> (ø)`
...nfirm-transaction/confirm-transaction.component.js	`75.00% <100.00%> (ø)`
ui/pages/swaps/awaiting-swap/awaiting-swap.js	`96.94% <ø> (ø)`
... and 3 more

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

vinistevam · 2023-09-21T14:26:34Z

Great PR! Small question, shouldn't we change here too?

app/scripts/migrations/099.test.ts

app/scripts/migrations/099.ts

test/e2e/fixture-builder.js

ui/store/actions.ts

OGPoyraz · 2023-09-22T09:00:35Z

Great PR! Small question, shouldn't we change here too?

Thanks @vinistevam, it's fixed in: 520f9c4

metamaskbot · 2023-09-22T10:28:36Z

Builds ready [a5bd456]

builds: chrome, firefox
builds (beta): chrome
builds (flask): chrome, firefox
builds (MMI): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4
- common: 0, 1, 2, 3, 4, 5, 6, 7
- content-script: 0
- contentscript: contentscript
- disable-console: disable-console
- inpage: inpage
- policy-load: policy-load
- sentry-install: sentry-install
- ui: 0, 1, 10, 11, 12, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1663 ± 74 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	120	199	155	19	9
		domContentLoaded	1368	1957	1662	153	74
		load	1368	1957	1663	153	74
		domInteractive	1368	1957	1662	153	74

Bundle size diffs [🚨 Warning! Bundle size has increased!]

background: 760 Bytes (0.02%)
ui: 107 Bytes (0.00%)
common: 0 Bytes (0.00%)

sleepytanya · 2023-09-22T17:57:31Z

When we create the second transaction (signature) the error occurs:

Steps to reproduce:

Create regular 'send' transaction.
Do not close confirmation.
Go to test-dapp and create 'signature' transaction.
Try to navigate between two confirmations.
Try to confirm 'send' transaction.
See the error message.
Chrome (same error on Firefox):

111.mov

sleepytanya · 2023-09-26T15:41:53Z

Confirmed that transactions history persists after correctly done migration.
On develop branch:

After migrating to current branch:

…ension-transaction-controller-2

metamaskbot · 2023-09-26T16:50:32Z

Builds ready [de4f958]

builds: chrome, firefox
builds (beta): chrome
builds (flask): chrome, firefox
builds (MMI): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4
- common: 0, 1, 2, 3, 4, 5, 6
- content-script: 0
- contentscript: contentscript
- disable-console: disable-console
- inpage: inpage
- policy-load: policy-load
- sentry-install: sentry-install
- ui: 0, 1, 10, 11, 12, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (977 ± 414 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	92	161	119	19	9
		domContentLoaded	73	148	108	20	10
		load	85	1992	977	863	414
		domInteractive	73	148	108	20	10

Bundle size diffs [🚨 Warning! Bundle size has increased!]

background: 760 Bytes (0.02%)
ui: 107 Bytes (0.00%)
common: 0 Bytes (0.00%)

…ension-transaction-controller-2

metamaskbot · 2023-09-27T07:55:38Z

Builds ready [d889ece]

builds: chrome, firefox
builds (beta): chrome
builds (flask): chrome, firefox
builds (MMI): chrome, firefox
builds (test): chrome, firefox
builds (test-flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4
- common: 0, 1, 2, 3, 4, 5, 6
- content-script: 0
- contentscript: contentscript
- disable-console: disable-console
- inpage: inpage
- policy-load: policy-load
- sentry-install: sentry-install
- ui: 0, 1, 10, 11, 12, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (508 ± 305 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	80	142	97	15	7
		domContentLoaded	70	134	89	17	8
		load	77	1571	508	635	305
		domInteractive	69	134	89	17	8

Bundle size diffs [🚨 Warning! Bundle size has increased!]

background: 760 Bytes (0.02%)
ui: 107 Bytes (0.00%)
common: 0 Bytes (0.00%)

Refactor to use UUID instead of random number

abe514a

OGPoyraz requested a review from a team as a code owner September 19, 2023 11:18

OGPoyraz added the team-confirmations-system DEPRECATED: please use "team-confirmations" label instead label Sep 19, 2023

update test

b7ae5e6