fix: Improve state management #353
Conversation
| @@ -460,6 +460,66 @@ describe('SmartTransactionsController', () => { | |||
| } as NetworkState); | |||
| expect(checkPollSpy).toHaveBeenCalled(); | |||
| }); | |||
|
|
|||
| it('calls "ensureUniqueSmartTransactions" on network change if it is a new chainId', () => { | |||
There was a problem hiding this comment.
I see that we check that this method is called, but we don't test what this method does. Do we want to add tests for that method in particular? Or, if the method isn't intended to be called on its own, should it be made private and then we test the behavior of the method in this test?
There was a problem hiding this comment.
So, there are tests to check that the function removed duplicities properly: https://github.com/MetaMask/smart-transactions-controller/pull/353/files#diff-750ce12ce9485eea01d3ee2172410c52fdd60e40cb3eef6c7b0909b8e8999284R496-R503
There was a problem hiding this comment.
Ah, you're right. Thanks for pointing that out.
There was a problem hiding this comment.
Does the method need to be public then, at least? Do we anticipate consumers needing to call it?
There was a problem hiding this comment.
It's only a private function and will be removed in one of the upcoming releases.
There was a problem hiding this comment.
I've marked the function as private in the code to make it clearer.
Description
We fixed this issue, eliminating an edge case related to network switching where we were unnecessarily polling for transaction status.
This PR improves state management to ensure unique smart transactions, and includes code to fix this issue for a small subset of users who have encountered this issue. The code to fix the issue for this subset of users will be removed in a future version once we have confirmed this is resolved.