-
-
Notifications
You must be signed in to change notification settings - Fork 252
feat: Add transaction emulation actions #6935
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
Changes from all commits
8cc4d27
4ac55db
a191ae8
4cef7ac
e813e32
a41e1ab
2131a02
b247d57
91d04dd
357add4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -357,6 +357,26 @@ export type TransactionControllerAddTransactionBatchAction = { | |
| handler: TransactionController['addTransactionBatch']; | ||
| }; | ||
|
|
||
| /** | ||
| * Emulate a new transaction. | ||
| * | ||
| * @param transactionId - The transaction ID. | ||
| */ | ||
| export type TransactionControllerEmulateNewTransaction = { | ||
| type: `${typeof controllerName}:emulateNewTransaction`; | ||
| handler: TransactionController['emulateNewTransaction']; | ||
| }; | ||
|
|
||
| /** | ||
| * Emmulate a transaction update. | ||
| * | ||
| * @param transactionMeta - Transaction metadata. | ||
| */ | ||
| export type TransactionControllerEmulateTransactionUpdate = { | ||
| type: `${typeof controllerName}:emulateTransactionUpdate`; | ||
| handler: TransactionController['emulateTransactionUpdate']; | ||
| }; | ||
|
|
||
| /** | ||
| * The internal actions available to the TransactionController. | ||
| */ | ||
|
|
@@ -369,7 +389,9 @@ export type TransactionControllerActions = | |
| | TransactionControllerGetStateAction | ||
| | TransactionControllerGetTransactionsAction | ||
| | TransactionControllerUpdateCustodialTransactionAction | ||
| | TransactionControllerUpdateTransactionAction; | ||
| | TransactionControllerUpdateTransactionAction | ||
| | TransactionControllerEmulateNewTransaction | ||
| | TransactionControllerEmulateTransactionUpdate; | ||
|
|
||
| /** | ||
| * Configuration options for the PendingTransactionTracker | ||
|
|
@@ -2786,6 +2808,68 @@ export class TransactionController extends BaseController< | |
| }); | ||
| } | ||
|
|
||
| /** | ||
| * Emulate a new transaction. | ||
| * | ||
| * @param transactionId - The transaction ID. | ||
| */ | ||
| emulateNewTransaction(transactionId: string) { | ||
| const transactionMeta = this.state.transactions.find( | ||
| (tx) => tx.id === transactionId, | ||
| ); | ||
|
|
||
| if (!transactionMeta) { | ||
| return; | ||
| } | ||
|
|
||
| if (transactionMeta.type === TransactionType.swap) { | ||
| this.messagingSystem.publish('TransactionController:transactionNewSwap', { | ||
| transactionMeta, | ||
| }); | ||
| } else if (transactionMeta.type === TransactionType.swapApproval) { | ||
| this.messagingSystem.publish( | ||
| 'TransactionController:transactionNewSwapApproval', | ||
| { transactionMeta }, | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Emulate a transaction update. | ||
| * | ||
| * @param transactionMeta - Transaction metadata. | ||
| */ | ||
| emulateTransactionUpdate(transactionMeta: TransactionMeta) { | ||
| const updatedTransactionMeta = { | ||
| ...transactionMeta, | ||
| txParams: { | ||
| ...transactionMeta.txParams, | ||
| from: this.messagingSystem.call('AccountsController:getSelectedAccount') | ||
| .address, | ||
| }, | ||
| }; | ||
|
|
||
| const transactionExists = this.state.transactions.some( | ||
| (tx) => tx.id === updatedTransactionMeta.id, | ||
| ); | ||
|
|
||
| if (!transactionExists) { | ||
| this.update((state) => { | ||
| state.transactions.push(updatedTransactionMeta); | ||
| }); | ||
| } | ||
|
|
||
| this.updateTransaction( | ||
| updatedTransactionMeta, | ||
| 'Generated from user operation', | ||
| ); | ||
|
|
||
| this.messagingSystem.publish( | ||
| 'TransactionController:transactionStatusUpdated', | ||
| { transactionMeta: updatedTransactionMeta }, | ||
| ); | ||
| } | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bug: Event Publishes Incomplete Transaction DataIn There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This does seem kinda broken, but, this is the pre-existing behavior There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Again, likely not used, so low risk we can address in a future PR. |
||
|
|
||
| #addMetadata(transactionMeta: TransactionMeta) { | ||
| validateTxParams(transactionMeta.txParams); | ||
| this.update((state) => { | ||
|
|
@@ -4392,6 +4476,16 @@ export class TransactionController extends BaseController< | |
| `${controllerName}:updateTransaction`, | ||
| this.updateTransaction.bind(this), | ||
| ); | ||
|
|
||
| this.messagingSystem.registerActionHandler( | ||
| `${controllerName}:emulateNewTransaction`, | ||
| this.emulateNewTransaction.bind(this), | ||
| ); | ||
|
|
||
| this.messagingSystem.registerActionHandler( | ||
| `${controllerName}:emulateTransactionUpdate`, | ||
| this.emulateTransactionUpdate.bind(this), | ||
| ); | ||
| } | ||
|
|
||
| #deleteTransaction(transactionId: string) { | ||
|
|
||
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.
Bug: Transaction History Limit Bypass
When adding a transaction that doesn't exist, the code directly pushes
updatedTransactionMetato state without using#trimTransactionsForState(). This bypasses the transaction history limit enforcement that is used everywhere else in the controller (e.g., in#addMetadataat line 2876-2880). This could cause the state to exceed the configuredtransactionHistoryLimit, leading to unbounded memory growth.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.
Again, this does seem like a legitimate bug, but it's a pre-existing problem
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.
Likely not used at all currently so very very low risk.