Skip to content

fix(transaction-controller): Return global ethQuery when !isMultichainEnabled - #4390

Merged
legobeat merged 1 commit into
MetaMask:mainfrom
legobeat:fix-multichaintrackinghelper-use-global-when-no-multi
Jun 10, 2024
Merged

fix(transaction-controller): Return global ethQuery when !isMultichainEnabled#4390
legobeat merged 1 commit into
MetaMask:mainfrom
legobeat:fix-multichaintrackinghelper-use-global-when-no-multi

Conversation

@legobeat

@legobeatlegobeat commented Jun 7, 2024

Copy link
Copy Markdown
Contributor

Explanation

There is a test stating that "MultichainTrackingHelper.getEthQuery should always return ethQuery for global provider when !isMultichainEnabled`".

It was not, but the test was passing anyway due to the mock being hardcoded to do so. This fixes that.

Also refactors tests to actually test for it (separately in #4391).


(Aside: I'm not sure why there even is a distinction between "global provider" and "chain-specific providers" in the first place - seems like there should never be a reason for both to exist simultaneously?) Answer: This is transitory and the "global provider" us due to be refactored away.

References

Related

Blocking

Changelog

@metamask/transaction-controller

  • FIXED: MultichainTrackingHelper.getEthQuery now returns global ethQuery with

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

@legobeatlegobeat changed the title fix(transaction-controller): fix(transaction-controller): Return global ethQuery when !isMultichainEnabledJun 7, 2024
@legobeat
legobeat marked this pull request as ready for review June 7, 2024 00:52
@legobeat
legobeat requested a review from a teamJune 7, 2024 00:52
dbrans
dbrans previously approved these changes Jun 7, 2024
@legobeat
legobeatforce-pushed the fix-multichaintrackinghelper-use-global-when-no-multi branch from 5a05293 to 31b2ea0CompareJune 7, 2024 01:46
@legobeat
legobeat requested review from a teamJune 7, 2024 02:35
mcmire
mcmire previously approved these changes Jun 7, 2024

@mcmiremcmire left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense.

@legobeat
legobeatforce-pushed the fix-multichaintrackinghelper-use-global-when-no-multi branch from 31b2ea0 to a365586CompareJune 8, 2024 00:48
@legobeat

Copy link
Copy Markdown
ContributorAuthor

(Conflict resolution)

@legobeat
legobeat requested review from a teamJune 8, 2024 00:49
@legobeat
legobeatforce-pushed the fix-multichaintrackinghelper-use-global-when-no-multi branch from a365586 to 5a311c5CompareJune 10, 2024 00:23
@legobeat
legobeat requested review from dbrans and mcmireJune 10, 2024 00:33
@dbrans

Copy link
Copy Markdown
Contributor

Also refactors tests to actually test for it (separately in #4391).

Hi @legobeat, I couldn’t find the test for !this.#isMultichainEnabled in #4391. It’s usually clearer to include tests in the same PR, especially for future reference.

@legobeat

legobeat commented Jun 10, 2024

Copy link
Copy Markdown
ContributorAuthor

Also refactors tests to actually test for it (separately in #4391).

Hi @legobeat, I couldn’t find the test for !this.#isMultichainEnabled in #4391. It’s usually clearer to include tests in the same PR, especially for future reference.

The existing test is here.

See also the multichainTrackerHelperMock, which always uses and returns the same provider unconditionally for getEthQuery and getProvider.

@legobeat

legobeat commented Jun 10, 2024

Copy link
Copy Markdown
ContributorAuthor

The test-related changes have now been separated into #4391. This PR now contains the fix in isolation.

…eturn ethQuery for global provider when !isMultichainEnabled
@legobeat
legobeatforce-pushed the fix-multichaintrackinghelper-use-global-when-no-multi branch from 3f23743 to bb80e40CompareJune 10, 2024 17:26
@legobeat
legobeat merged commit 316035e into MetaMask:mainJun 10, 2024
legobeat added a commit to legobeat/metamask-controllers that referenced this pull request Jun 12, 2024
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

3 participants

@legobeat@dbrans@mcmire