Skip to content

Fix fetching of swap quotes when initial network was testnet#9726

Merged
Gudahtt merged 1 commit intodevelopfrom
fix-swaps-fetch-when-initially-on-testnet
Oct 27, 2020
Merged

Fix fetching of swap quotes when initial network was testnet#9726
Gudahtt merged 1 commit intodevelopfrom
fix-swaps-fetch-when-initially-on-testnet

Conversation

@Gudahtt
Copy link
Copy Markdown
Member

@Gudahtt Gudahtt commented Oct 26, 2020 •
edited
Loading

If the initial network when the extension is started is something other than Mainnet, the swaps controller will never successfully retrieve swap quotes. This is because the ethers provider used by the swaps controller doesn't allow network changes by default - it assumes that the network remains the same as when the provider was initialized.

This was fixed by hard-coding Mainnet as the initial chain ID for this ethers provider used by the swaps controller.

Some adjustments needed to be made to the provider stub to allow setting 1 as the network ID and chain ID in unit tests.

Manual testing steps:

  • Change the current network to Ropsten
  • Restart the extension (e.g. on Chrome, navigate to chrome://extensions/ and click the "reload" circular arrow button on the MetaMask extension).
  • Unlock the extension, and observe that the current network is Ropsten
  • Switch to Mainnet
  • Navigate to swaps and attempt to get quotes
  • Before this change, the attempt would always fail. After this change, it should succeed.

@Gudahtt Gudahtt requested a review from a team as a code owner October 26, 2020 22:59
@Gudahtt Gudahtt requested review from brad-decker and removed request for a team October 26, 2020 22:59
@Gudahtt Gudahtt marked this pull request as draft October 26, 2020 23:00
@Gudahtt
Copy link
Copy Markdown
Member Author

Gudahtt commented Oct 26, 2020 •
edited
Loading

This depends upon #9725 . I needed to update ganache-core to get support for setting a custom chainId, or the unit tests would fail.

Edit: This is now ready for review; #9725 has been merged.

@metamaskbot
Copy link
Copy Markdown
Collaborator

metamaskbot commented Oct 26, 2020

Builds ready [e69bc75]
Page Load Metrics (428 ± 53 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint298838126
domContentLoaded31068542611053
load31168542811053
domInteractive31068442611053

Base automatically changed from update-ganache to develop October 26, 2020 23:38
@Gudahtt Gudahtt force-pushed the fix-swaps-fetch-when-initially-on-testnet branch from e69bc75 to 05841e5 Compare October 26, 2020 23:39
@Gudahtt Gudahtt marked this pull request as ready for review October 26, 2020 23:39
@metamaskbot
Copy link
Copy Markdown
Collaborator

metamaskbot commented Oct 27, 2020

Builds ready [05841e5]
Page Load Metrics (416 ± 56 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint309538147
domContentLoaded30573941411656
load30774241611756
domInteractive30573841411656

tmashuang
tmashuang previously approved these changes Oct 27, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@tmashuang tmashuang left a comment

Choose a reason for hiding this comment

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

LGTM?

rekmarks
rekmarks reviewed Oct 27, 2020
View reviewed changes
@Gudahtt
Fix fetching of swap quotes when initial network was testnet
9b97505 
If the initial network when the extension is started is something other
than Mainnet, the swaps controller will never successfully retrieve
swap quotes. This is because the `ethers` provider used by the swaps
controller doesn't allow network changes by default - it assumes that
the network remains the same as when the provider was initialized.

This was fixed by hard-coding Mainnet as the initial chain ID for this
`ethers` provider used by the swaps controller.

Some adjustments needed to be made to the `provider` stub to allow
setting `1` as the network ID and chain ID in unit tests.
@Gudahtt Gudahtt force-pushed the fix-swaps-fetch-when-initially-on-testnet branch from 05841e5 to 9b97505 Compare October 27, 2020 15:26
rekmarks
rekmarks approved these changes Oct 27, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@rekmarks rekmarks left a comment

Choose a reason for hiding this comment

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

LGTM!

@metamaskbot
Copy link
Copy Markdown
Collaborator

metamaskbot commented Oct 27, 2020

Builds ready [9b97505]
Page Load Metrics (507 ± 71 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint31124512713
domContentLoaded30181950514771
load30282050714771
domInteractive30181950514771

@Gudahtt Gudahtt merged commit 3bbc1d1 into develop Oct 27, 2020
@Gudahtt Gudahtt deleted the fix-swaps-fetch-when-initially-on-testnet branch October 27, 2020 15:52
@github-actions github-actions bot locked and limited conversation to collaborators Oct 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@rekmarks rekmarks rekmarks approved these changes

@brad-decker brad-decker Awaiting requested review from brad-decker

+1 more reviewer

@tmashuang tmashuang tmashuang left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@Gudahtt @metamaskbot @tmashuang @rekmarks