Delay variable initialization in the transaction controller until network provider is available

[pedronfigueiredo]

Explanation

In the future, the network provider and block tracker may no longer be defined when the Transactions Controller is instantiated. As such, this PR creates a new method that can be called by the client when the provider and block tracker are finally available.

For more details on the reasoning behind this change, please see the linked PRs and issues.

References

• Fixes #2120 on the planning repo (see also #881).
• Related to: PR #23005 on the extension repo and PR #274 on the smart transactions repo.

Changelog

@metamask/transaction-controller

• BREAKING: Added a delayedInit method that should be called only once the provider and blockTracker are available in the client.

Checklist

• I've updated the test suite for new or updated code as appropriate
• I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate
• I've highlighted breaking changes using the "BREAKING" category above as appropriate

[matthewwalsh0]

My main concern is that these helpers are fundamental to the operation of this controller as they are required not only to monitor transactions but to even add them.

Meaning an instance without them feels very limited in utility and we now have to be conscious of these two alternate states of initialisation which adds additional complexity to an already complex controller.

If the client doesn't have the necessary dependencies to initialise the controller, is that not by definition a client problem and so would a cleaner and simpler solution be to delay the initialisation of the controller on the client side until all the dependencies are available? This would also have the benefit of making the flows easier to predict and handle as the client would be able to create suitable errors if the controller isn't yet initialised but an action is triggered that requires it.

Ultimately I worry this is over-coupling the client to the controller as the intent of the controllers to begin with is to encapsulate state and logic out of the clients, using the constructors to define the explicit dependencies they require to function.

[mcmire]

I agree. We should do our best to not place things in an invalid state on purpose, and this would include initializing a controller but leaving its provider, block tracker, or EthQuery instance unset until some future time. Delaying the initialization of the controller until we can pass a provider seems like a better approach. There's probably some context I'm missing here, however, so I'll see if I can take some time to study the requirements that prompted these changes this week.

[matthewwalsh0]

We still have all the original initialisations so I worry that makes the solution even more complex since we don't know if we have a functional helper (nonce tracker, multi-chain tracker etc) or a functioning one?

Plus having these helpers initialised only once allows us to maintain stability by being confident we are dealing with single instances with one set of event listeners for example, and not being as concerned about deconstruction.

[matthewwalsh0]

If this is solely to support the global network blockTracker and provider instances, then this should no longer be an issue once this controller goes full multi-chain as these options will be removed since all the provider and block tracker instances will be dynamically retrieved based on the networkClientId provided when adding transactions.

[Gudahtt]

Looks like you committed a few extra files accidentally