-
-
Couldn't load subscription status.
- Fork 251
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
Conversation
c78a111 to
b0c8f59
Compare
These will be used indirectly by the UserOperationController to emulate adding and updating a transaction, in response to user operations.
b0c8f59 to
93ebaac
Compare
93ebaac to
a191ae8
Compare
| 'TransactionController:transactionStatusUpdated', | ||
| { transactionMeta: updatedTransactionMeta }, | ||
| ); | ||
| } |
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: Event Publishes Incomplete Transaction Data
In emulateTransactionUpdate, the transactionStatusUpdated event publishes transaction metadata before it's fully normalized by updateTransaction. This means event listeners receive data (e.g., missing value: '0x0') that differs from what's ultimately stored in the controller's state, leading to inconsistency.
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 does seem kinda broken, but, this is the pre-existing behavior
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, likely not used, so low risk we can address in a future PR.
| this.update((state) => { | ||
| state.transactions.push(updatedTransactionMeta); | ||
| }); | ||
| } |
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 updatedTransactionMeta to state without using #trimTransactionsForState(). This bypasses the transaction history limit enforcement that is used everywhere else in the controller (e.g., in #addMetadata at line 2876-2880). This could cause the state to exceed the configured transactionHistoryLimit, 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.
…r/multichain-transactions-controller * origin/main: (35 commits) feat: `JsonRpcEngineV2` (#6176) Release 641.0.0 (#6940) feat: Add transaction emulation actions (#6935) Release/640.0.0 (#6934) fix(core-backend): control randomness to fix flaky test (#6936) chore: Add `@metamask-previews/*` to NPM age gate exceptions (#6937) Release/639.0.0 (#6931) feat: make getCryptoApproveTransactionParams synchronous (#6930) feat: add new actions to `KeyringController` (#6928) feat: add `getAccounts` to `AccountsController` (#6927) chore: remove `Monad Mainnet` single call balance contract and add into account v4 (#6929) Release/638.0.0 (#6923) fix: Downgrade `multiformats` to `^9.9.0` to avoid ESM-only dependency (#6920) Release/637.0.0 (#6919) feat(account-tree-controller): add callbacks for hidden and pinned data (#6910) Release 636.0.0 (#6918) fix(core-backend): reconnection logic (#6861) fix: Tx state listener and signature coverage (#6906) Release/635.0.0 (#6917) fix(base-controller): add TypeScript declaration file for legacy module resolution (#6915) ...
Explanation
These will be used indirectly by the
UserOperationControllerto emulate adding and updating a transaction, in response to user operations.References
This came up as a blocker to MetaMask/metamask-extension#31843 because currently the extension is directly publishing
TransactionControllerevents, which will no longer be allowed with the new messenger.Checklist
Note
Add
emulateNewTransactionandemulateTransactionUpdateactions to TransactionController, emitting relevant swap/update events and updating state via messenger.TransactionController:emulateNewTransactionandTransactionController:emulateTransactionUpdate.emulateNewTransaction: PublishesTransactionController:transactionNewSwaporTransactionController:transactionNewSwapApprovalbased on the transaction type.emulateTransactionUpdate: SetstxParams.fromto the selected account, adds transaction if missing, updates it, and publishesTransactionController:transactionStatusUpdated.emulateNewTransactionandemulateTransactionUpdate.src/index.ts.Written by Cursor Bugbot for commit 357add4. This will update automatically on new commits. Configure here.