-
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: SOL-46 Adds the multichain transactions controller #29129
base: main
Are you sure you want to change the base?
feat: SOL-46 Adds the multichain transactions controller #29129
Conversation
CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes. |
@metamaskbot update-policies |
Policies updated. 🧠 Learn how: https://lavamoat.github.io/guides/policy-diff/#what-to-look-for-when-reviewing-a-policy-diff |
…github.com:MetaMask/metamask-extension into SOL-46-adds-the-multichain-transactions-controller
Builds ready [a1f3d21]
Page Load Metrics (1701 ± 108 ms)
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
Builds ready [43a89fb]
Page Load Metrics (1850 ± 76 ms)
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
Fusión |
Builds ready [6ad4665]
Page Load Metrics (1662 ± 53 ms)
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
Builds ready [b0f3d1d]
Page Load Metrics (1688 ± 81 ms)
|
Builds ready [10f0669]
Page Load Metrics (1704 ± 83 ms)
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
Builds ready [d68226e]
Page Load Metrics (1662 ± 53 ms)
|
Builds ready [f1a0a22]
Page Load Metrics (1839 ± 152 ms)
|
app/scripts/lib/transaction/MultichainTransactionsController.ts
Outdated
Show resolved
Hide resolved
…github.com:MetaMask/metamask-extension into SOL-46-adds-the-multichain-transactions-controller
Builds ready [ac7c72c]
Page Load Metrics (1740 ± 80 ms)
|
app/scripts/lib/transaction/MultichainTransactionsController.ts
Outdated
Show resolved
Hide resolved
app/scripts/lib/transaction/MultichainTransactionsController.ts
Outdated
Show resolved
Hide resolved
ui/components/app/transaction-list/transaction-list.component.js
Outdated
Show resolved
Hide resolved
ui/components/app/transaction-list/transaction-list.component.js
Outdated
Show resolved
Hide resolved
app/scripts/lib/transaction/MultichainTransactionsController.ts
Outdated
Show resolved
Hide resolved
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.
Can you move the controller/tracker to MetaMask/core
, so it can be reused for mobile as well? I think generally we shouldn't add new controllers to the extension repo directly.
@Mrtenz this will happen for sure, it will all be moved to Core repo as follow-up work, it's currently in the extension to speed up our development and make iteration faster. |
Builds ready [f91ef32]
Page Load Metrics (1604 ± 47 ms)
|
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.
This should probably be based on one of the polling controllers available here: https://github.com/MetaMask/core/tree/main/packages/polling-controller/src
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.
Will look into that now that it's being moved to Core 👍🏼
Description
This is the Part 1 of the work to have SOL/BTC transactions history showing in the activity tab, this PR adds the
MultichainTransactionsController
that will be responsible for retrieving Txs from the snap. On the snap side, the followup logic is under development as well.It adds already some of the necessary UI to display the Txs in the activity list.
Also bumps keyring-api so that the new keyring method
listAccountTransactions
is found.Related issues
Adds: https://consensyssoftware.atlassian.net/browse/SOL-46
Manual testing steps
Testing this is very extensive, but it you still want to give it a go these are the steps:
yarn
shared/lib/accounts/solana-wallet-snap.ts
with:export const SOLANA_WALLET_SNAP_ID: SnapId = 'local:http://localhost:8080' as SnapId;
app/scripts/lib/transaction/MultichainTransactionsController.ts
in L251 with:We need to add the devnet/testnet here just for testing purposes.
yarn start:flask
yarn
yarn start
Screenshots/Recordings
Before
Didn't exist.
After
Note: The video already shows the Tx detail modal, but thats out of the scope of this task and is in another PR.
Screen.Recording.2024-12-17.at.12.27.40.1.mov
Pre-merge author checklist
Pre-merge reviewer checklist