-
Notifications
You must be signed in to change notification settings - Fork 5k
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
feat: enable STX by default with migration and notification #28854
Open
httpJunkie
wants to merge
135
commits into
main
Choose a base branch
from
feat/enable-stx-migration
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
135 commits
Select commit
Hold shift + click to select a range
dba315a
Add migration and tests for enabling STX by default, update preferenc…
httpJunkie 59b94a5
add logging to trace state and metadata change in preferences control…
httpJunkie 606f293
add logging in setPreference in preferences controller and actions.ts
httpJunkie 629b2a2
update `VersionedData` in the migration to have preferences:
httpJunkie dac7203
Merge branch 'main' into feat/enable-stx-migration
httpJunkie 3010831
Add UI for legacy transaction flow, creation of STX Banner Alert for …
httpJunkie 0987ad2
Merge branch 'main' into feat/enable-stx-migration
httpJunkie f89ab51
update `stx-migration.js` to include `extraReducers`, added test and …
httpJunkie 73a0f19
Add logs and alet to ensure component is mounting and state is as int…
httpJunkie a3daae9
remove alert and update logs
httpJunkie 85073df
registering our stxMigration reducer in ducks index
httpJunkie 9b906cd
Remove logs from migration script; update stx-migration logs to debug…
httpJunkie cf23f7b
update stx-migration in ducks to dismissAndDIsable alert state once t…
httpJunkie a24e963
revert changes to wrong file
httpJunkie 89fde17
update stx-migration ducks file to consider dismiss and disable alert.
httpJunkie 3a6e424
update path to store in test file and directly tests the thunk behav…
httpJunkie 3af6e58
add logs to check extraReducer logic and state.
httpJunkie ed3be1f
Add permanent alert dismissal for MetaMask Smart Transactions onboard…
httpJunkie 724d3e6
With changes to permanent alert dismissal, we needed to update our te…
httpJunkie 24c1d6c
Merge branch 'main' into feat/enable-stx-migration
httpJunkie 1df568e
First stab at trying to implement alert for new confirmation/transact…
httpJunkie a6fa239
added an action to dismiss
httpJunkie 21a45a8
feat: add STX migration toast to new confirmation flow
httpJunkie da793a6
move toast to top of page and try to default to darkmode
httpJunkie cea779c
Merge branch 'main' into feat/enable-stx-migration
danjm 5df15e0
chore: cleanup STX migration implementation
httpJunkie 8162b8c
Merge branch 'feat/enable-stx-migration' of github.com:MetaMask/metam…
httpJunkie d2ccb44
Merge branch 'main' into feat/enable-stx-migration
httpJunkie a220f50
Add Banner Alert to the Swaps page (top)
httpJunkie 32d4822
chore: remove 'smartTransactionsLearnMore' for use of 'learnMoreUpper…
httpJunkie 5ec03ae
Change `smartTransactionsLearnMore` to 'learnMore' on STXBannerAlert
httpJunkie 04f21e7
fix: Add 'transaction-alerts' style to wrap component as it's not mad…
httpJunkie 2c74335
chore: remove 'Message" postfix
httpJunkie f836749
fix: This change ensures that the STX opt-in status is only affected …
httpJunkie 1b59f9c
chore: Rename any and all use of word: 'stx or STX' for variables and…
httpJunkie d34a1c5
Merge branch 'main' into feat/enable-stx-migration
httpJunkie 657255a
chore: address issues in PR (use constants), remove zendesk url, remo…
httpJunkie 62ece63
fix: Update SmartTransactionsBannerAlert to encompass styling needed …
httpJunkie 9abcd56
fix: update the index file for SmartTransactionsBannerAlert as we are…
httpJunkie 5814520
Merge branch 'main' into feat/enable-stx-migration
httpJunkie b40018b
Update Smart Transactions Migration redux/ducks test to ensure we are…
httpJunkie 2823d1d
fix: test needed to be updated after changing naming in `SmartTransac…
httpJunkie 7771d5d
chore: run prettier fix on `messages.json`
httpJunkie 77ba466
Merge branch 'main' into feat/enable-stx-migration
httpJunkie 036a3a8
fix: fitness function failure, convert our JS files to TypeScript sin…
httpJunkie 958ae8f
fix: fitness function failure, convert our JS files to TypeScript
httpJunkie bd9cdf0
test: update snapshots to include STX banner container
httpJunkie ab95ca9
fix: remove unused d.ts file no longer needed as SmartTransactionsBan…
httpJunkie 73b4662
Merge branch 'main' into feat/enable-stx-migration
httpJunkie 94f920b
Merge branch 'main' into feat/enable-stx-migration
httpJunkie 398bd78
fix: These are old tests planned for disable. The quicker way to solv…
httpJunkie 7ff3b71
Merge branch 'feat/enable-stx-migration' of github.com:MetaMask/metam…
httpJunkie 8388d4d
Merge branch 'main' into feat/enable-stx-migration
httpJunkie ebe57a4
Merge branch 'main' into feat/enable-stx-migration
httpJunkie 796b1a2
Merge branch 'main' into feat/enable-stx-migration
httpJunkie 11fe290
fix: removed console.log accidentally left in.
httpJunkie 35123de
Merge branch 'main' into feat/enable-stx-migration
httpJunkie 50be87a
This seems to be the correct way to skip these tests rather than `xde…
httpJunkie b87bd4c
Merge branch 'feat/enable-stx-migration' of github.com:MetaMask/metam…
httpJunkie 5368950
refactor: simplify smart transactions migration alert
httpJunkie 2936c72
fix: let's make the test descriptions more accurate to what we're act…
httpJunkie bf6234d
Merge branch 'main' into feat/enable-stx-migration
httpJunkie 8acd1ec
Merge branch 'main' into feat/enable-stx-migration
httpJunkie a7f208f
fix: update snapshot to include smartTransactionsOptInStatus
httpJunkie e79a067
Merge branch 'main' into feat/enable-stx-migration
httpJunkie 2af5d8f
Merge branch 'feat/enable-stx-migration' of github.com:MetaMask/metam…
httpJunkie e8c85fd
added snapshots for Banner Alert
httpJunkie b265893
chore: updates snapshots for the connect-account-modal test
georgeweiler 941d34b
Merge branch 'main' into feat/enable-stx-migration
georgeweiler 57125a2
Merge branch 'feat/enable-stx-migration' of github.com:MetaMask/metam…
georgeweiler 6fab703
Revert "chore: updates snapshots for the connect-account-modal test"
georgeweiler 7d29763
fix: revert to ^HEAD on problematic test.
httpJunkie ffcf811
Merge branch 'feat/enable-stx-migration' of github.com:MetaMask/metam…
httpJunkie c10e55d
Merge branch 'main' into feat/enable-stx-migration
httpJunkie f6473a3
disable STX for these tests: adding the baseFixtureOptions with smar…
httpJunkie 81631b9
Merge branch 'feat/enable-stx-migration' of github.com:MetaMask/metam…
httpJunkie 577cdee
match the structure where metamask should ibe nested under data in ou…
httpJunkie f24f686
revert changes to `swaps-notifications.spec.ts` as test results did n…
httpJunkie a53cb5e
Add logic to determine confirmationType in Wrapper
httpJunkie 32ad253
chore: Move all logic for conditionally rendering the SmartTransactio…
httpJunkie cf244b5
Merge branch 'main' into feat/enable-stx-migration
httpJunkie 0971419
chore: update SmartTransactionsBannerAlert to maintain backward compa…
httpJunkie fb4fe4a
fix: update smart-transaction-banner-alert test so it verifies our ne…
httpJunkie 628563d
fix: auto-dismiss STX migration banner when user disables STX
httpJunkie 82b40d7
fix: handle STX banner dismiss when toggling STX off
httpJunkie e65e33d
Merge branch 'main' into feat/enable-stx-migration
httpJunkie eb955dd
fix: linting / TS errors
httpJunkie 688f049
fix: update snapshots for `snap-ui-address`, `connect-accounts-modal`…
httpJunkie cf008ed
fix: add `as string` back to satisfy linting
httpJunkie 775fc85
Merge branch 'main' into feat/enable-stx-migration
httpJunkie e7ca076
Merge branch 'main' into feat/enable-stx-migration
httpJunkie fdb9f00
fix: update snapshot for connect-account-modal to have the same trans…
httpJunkie 986592e
fix: update snapshot to match CI transform for `snap-ui-address`
httpJunkie d529b61
Merge branch 'main' into feat/enable-stx-migration
httpJunkie 7c6757e
fix: update STX Banner Alert text to match new design mockups, ensure…
httpJunkie 6e1f73e
fix: improve STX banner display logic for new installs
httpJunkie ccb2130
Merge branch 'main' into feat/enable-stx-migration
httpJunkie 46f9ef5
fix: update preferences-controller test because we added smartTransac…
httpJunkie 62dd4e8
Merge branch 'main' into feat/enable-stx-migration
httpJunkie 8c875b0
fix: update snapshot for `confirm-send-ether.test.js`
httpJunkie 35d12d3
Merge branch 'main' into feat/enable-stx-migration
httpJunkie 0ed49c5
fix: add test for prepare-swap-page.test.js
httpJunkie e743a15
fix: updated snapshots from failing tests.
httpJunkie 0f6e770
fix: snapshot update for connect-accounts-modal
httpJunkie b7f6313
fix: update snapshot for `snap-ui-address.test.tsx`
httpJunkie 07680bd
fix: add `smartTransactionsMigrationApplied` to `expectedMissingState`
httpJunkie 3c29e52
Merge branch 'main' into feat/enable-stx-migration
httpJunkie 258b0a0
fix: try adding withPreferencesController with smartTransactionsMigra…
httpJunkie ae6d64e
fix: fix formatting on error.spec.js
httpJunkie c6b81f8
fix: reset `errors.spec.js` to version on main
httpJunkie ba8f557
fix: remove comments in preference-controller
httpJunkie 80f264a
feat: update how we check for transaction confirmations
httpJunkie 51b633f
fix: update `confirm.test.tsx` and snapshot.
httpJunkie 9fd62b6
Merge branch 'main' into feat/enable-stx-migration
httpJunkie 067fc9b
Merge branch 'main' into feat/enable-stx-migration
httpJunkie 202e7d6
Update snapshots for E2E tests
dan437 4377a1b
Update a Jest snapshot
dan437 fd00aa9
Update an E2E test
dan437 b7ed2d1
fix: remove root level duplicated `smartTransactionsOptInStatus` (we …
httpJunkie 49ab97d
Merge branch 'main' into feat/enable-stx-migration
httpJunkie b445df7
fix: cleaned up test for migration after removing `smartTransactionsO…
httpJunkie d9f06a1
Merge branch 'main' into feat/enable-stx-migration
httpJunkie 9a51557
fix: remove root level `smartTransactionsOptInStatus` from snapshots.
httpJunkie 7123914
Merge branch 'feat/enable-stx-migration' of github.com:MetaMask/metam…
httpJunkie 3cda0ac
Merge branch 'main' into feat/enable-stx-migration
httpJunkie 20cca27
Merge branch 'main' into feat/enable-stx-migration
httpJunkie ab5c1d7
fix: Remove redundant test for an impossible state (alert enabled bef…
httpJunkie 105d90d
refactor: streamline SmartTransactionsBannerAlert logic
httpJunkie f520d7c
refactor: simplify logic in migration 135 and fix formatting in smart…
httpJunkie dcf814c
Merge branch 'main' into feat/enable-stx-migration
httpJunkie 28d135a
fix: remove skipped tests
httpJunkie b58381f
fix: close SmartTransactionsMigrationNotification on `swap-eth.spec.ts`
httpJunkie e92d4dd
fix: Scroll "View all quotes" button into view before clicking in STX…
httpJunkie 8385fce
Merge branch 'main' into feat/enable-stx-migration
httpJunkie f4b14dc
Merge branch 'main' into feat/enable-stx-migration
httpJunkie File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,187 @@ | ||
import { SmartTransaction } from '@metamask/smart-transactions-controller/dist/types'; | ||
import { migrate, VersionedData } from './135'; | ||
|
||
const prevVersion = 134; | ||
|
||
describe('migration #135', () => { | ||
const mockSmartTransaction: SmartTransaction = { | ||
uuid: 'test-uuid', | ||
}; | ||
|
||
it('should update the version metadata', async () => { | ||
const oldStorage: VersionedData = { | ||
meta: { version: prevVersion }, | ||
data: {}, | ||
}; | ||
|
||
const newStorage = await migrate(oldStorage); | ||
expect(newStorage.meta).toStrictEqual({ version: 135 }); | ||
}); | ||
|
||
it('should set stx opt-in to true and migration flag when stx opt-in status is null', async () => { | ||
const oldStorage: VersionedData = { | ||
meta: { version: prevVersion }, | ||
data: { | ||
PreferencesController: { | ||
preferences: { | ||
smartTransactionsOptInStatus: null, | ||
}, | ||
}, | ||
}, | ||
}; | ||
|
||
const newStorage = await migrate(oldStorage); | ||
expect( | ||
newStorage.data.PreferencesController?.preferences | ||
?.smartTransactionsOptInStatus, | ||
).toBe(true); | ||
expect( | ||
newStorage.data.PreferencesController?.preferences | ||
?.smartTransactionsMigrationApplied, | ||
).toBe(true); | ||
}); | ||
|
||
it('should set stx opt-in to true and migration flag when stx opt-in status is undefined', async () => { | ||
const oldStorage: VersionedData = { | ||
meta: { version: prevVersion }, | ||
data: { | ||
PreferencesController: {}, | ||
}, | ||
}; | ||
|
||
const newStorage = await migrate(oldStorage); | ||
expect( | ||
newStorage.data.PreferencesController?.preferences | ||
?.smartTransactionsOptInStatus, | ||
).toBe(true); | ||
expect( | ||
newStorage.data.PreferencesController?.preferences | ||
?.smartTransactionsMigrationApplied, | ||
).toBe(true); | ||
}); | ||
|
||
it('should set stx opt-in to true and migration flag when stx opt-in is false and no existing mainnet smart transactions', async () => { | ||
const oldStorage: VersionedData = { | ||
meta: { version: prevVersion }, | ||
data: { | ||
PreferencesController: { | ||
preferences: { | ||
smartTransactionsOptInStatus: false, | ||
}, | ||
}, | ||
SmartTransactionsController: { | ||
smartTransactionsState: { | ||
smartTransactions: { | ||
'0x1': [], // Empty mainnet transactions | ||
'0xAA36A7': [mockSmartTransaction], // Sepolia has transactions | ||
}, | ||
}, | ||
}, | ||
}, | ||
}; | ||
|
||
const newStorage = await migrate(oldStorage); | ||
expect( | ||
newStorage.data.PreferencesController?.preferences | ||
?.smartTransactionsOptInStatus, | ||
).toBe(true); | ||
expect( | ||
newStorage.data.PreferencesController?.preferences | ||
?.smartTransactionsMigrationApplied, | ||
).toBe(true); | ||
}); | ||
|
||
it('should not change stx opt-in when stx opt-in is false but has existing smart transactions, but should set migration flag', async () => { | ||
const oldStorage: VersionedData = { | ||
meta: { version: prevVersion }, | ||
data: { | ||
PreferencesController: { | ||
preferences: { | ||
smartTransactionsOptInStatus: false, | ||
}, | ||
}, | ||
SmartTransactionsController: { | ||
smartTransactionsState: { | ||
smartTransactions: { | ||
'0x1': [mockSmartTransaction], | ||
}, | ||
}, | ||
}, | ||
}, | ||
}; | ||
|
||
const newStorage = await migrate(oldStorage); | ||
expect( | ||
newStorage.data.PreferencesController?.preferences | ||
?.smartTransactionsOptInStatus, | ||
).toBe(false); | ||
expect( | ||
newStorage.data.PreferencesController?.preferences | ||
?.smartTransactionsMigrationApplied, | ||
).toBe(true); | ||
}); | ||
|
||
it('should not change stx opt-in when stx opt-in is already true, but should set migration flag', async () => { | ||
const oldStorage: VersionedData = { | ||
meta: { version: prevVersion }, | ||
data: { | ||
PreferencesController: { | ||
preferences: { | ||
smartTransactionsOptInStatus: true, | ||
}, | ||
}, | ||
}, | ||
}; | ||
|
||
const newStorage = await migrate(oldStorage); | ||
expect( | ||
newStorage.data.PreferencesController?.preferences | ||
?.smartTransactionsOptInStatus, | ||
).toBe(true); | ||
expect( | ||
newStorage.data.PreferencesController?.preferences | ||
?.smartTransactionsMigrationApplied, | ||
).toBe(true); | ||
}); | ||
|
||
it('should initialize preferences object if it does not exist', async () => { | ||
const oldStorage: VersionedData = { | ||
meta: { version: prevVersion }, | ||
data: { | ||
PreferencesController: { | ||
preferences: { | ||
smartTransactionsOptInStatus: true, | ||
}, | ||
}, | ||
}, | ||
}; | ||
|
||
const newStorage = await migrate(oldStorage); | ||
expect(newStorage.data.PreferencesController?.preferences).toBeDefined(); | ||
expect( | ||
newStorage.data.PreferencesController?.preferences | ||
?.smartTransactionsMigrationApplied, | ||
).toBe(true); | ||
}); | ||
|
||
it('should capture exception if PreferencesController state is invalid', async () => { | ||
const sentryCaptureExceptionMock = jest.fn(); | ||
global.sentry = { | ||
captureException: sentryCaptureExceptionMock, | ||
}; | ||
|
||
const oldStorage = { | ||
meta: { version: prevVersion }, | ||
data: { | ||
PreferencesController: 'invalid', | ||
}, | ||
} as unknown as VersionedData; | ||
|
||
await migrate(oldStorage); | ||
|
||
expect(sentryCaptureExceptionMock).toHaveBeenCalledTimes(1); | ||
expect(sentryCaptureExceptionMock).toHaveBeenCalledWith( | ||
new Error('Invalid PreferencesController state: string'), | ||
); | ||
}); | ||
}); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,84 @@ | ||
import { hasProperty, isObject } from '@metamask/utils'; | ||
import { cloneDeep } from 'lodash'; | ||
import type { SmartTransaction } from '@metamask/smart-transactions-controller/dist/types'; | ||
import { CHAIN_IDS } from '@metamask/transaction-controller'; | ||
|
||
export type VersionedData = { | ||
meta: { | ||
version: number; | ||
}; | ||
data: { | ||
PreferencesController?: { | ||
preferences?: { | ||
smartTransactionsOptInStatus?: boolean | null; | ||
smartTransactionsMigrationApplied?: boolean; | ||
}; | ||
}; | ||
SmartTransactionsController?: { | ||
smartTransactionsState: { | ||
smartTransactions: Record<string, SmartTransaction[]>; | ||
}; | ||
}; | ||
}; | ||
}; | ||
|
||
export const version = 135; | ||
|
||
function transformState(state: VersionedData['data']) { | ||
if ( | ||
!hasProperty(state, 'PreferencesController') || | ||
!isObject(state.PreferencesController) | ||
) { | ||
global.sentry?.captureException?.( | ||
new Error( | ||
`Invalid PreferencesController state: ${typeof state.PreferencesController}`, | ||
), | ||
); | ||
return state; | ||
} | ||
|
||
const { PreferencesController } = state; | ||
|
||
const currentOptInStatus = | ||
PreferencesController.preferences?.smartTransactionsOptInStatus; | ||
|
||
if ( | ||
currentOptInStatus === undefined || | ||
currentOptInStatus === null || | ||
(currentOptInStatus === false && !hasExistingSmartTransactions(state)) | ||
) { | ||
state.PreferencesController.preferences = { | ||
...state.PreferencesController.preferences, | ||
smartTransactionsOptInStatus: true, | ||
smartTransactionsMigrationApplied: true, | ||
}; | ||
} else { | ||
state.PreferencesController.preferences = { | ||
...state.PreferencesController.preferences, | ||
smartTransactionsMigrationApplied: true, | ||
}; | ||
} | ||
|
||
return state; | ||
} | ||
|
||
function hasExistingSmartTransactions(state: VersionedData['data']): boolean { | ||
const smartTransactions = | ||
state?.SmartTransactionsController?.smartTransactionsState | ||
?.smartTransactions; | ||
|
||
if (!isObject(smartTransactions)) { | ||
return false; | ||
} | ||
|
||
return (smartTransactions[CHAIN_IDS.MAINNET] || []).length > 0; | ||
} | ||
|
||
export async function migrate( | ||
originalVersionedData: VersionedData, | ||
): Promise<VersionedData> { | ||
const versionedData = cloneDeep(originalVersionedData); | ||
versionedData.meta.version = version; | ||
transformState(versionedData.data); | ||
return versionedData; | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is the
smartTransactionsOptInStatus
line here twice?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the heads up, I will most likely be removing the root level property in the
controllerMetadata
insidepreferences-controller
and that should fix this. I was initially trying to figure out the right place for it and this is a valid point.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a reminder that
smartTransactionsOptInStatus
is still here twice.