Skip to content
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

Update EIP-1559 compatibility during network lookup #1236

Merged
merged 2 commits into from
Apr 21, 2023

Conversation

Gudahtt
Copy link
Member

@Gudahtt Gudahtt commented Apr 20, 2023

Description

The EIP-1559 compatibility state is now updated during the network lookup, rather than during the provider configuration step. This matches the extension more closely. This check made more sense during the network lookup anyway; it's part of checking the status of the network, not of configuring the provider.

Changes

  • CHANGED: lookupNetwork will now check the latest block and update the network details state.

References

Closes #1203

Checklist

  • I've updated the test suite for new or updated code as appropriate
  • I've updated documentation for new or updated code as appropriate (note: this will usually be JSDoc)
  • I've highlighted breaking changes using the "BREAKING" category above as appropriate

@Gudahtt Gudahtt force-pushed the update-eip-1559-compatibility-during-network-lookup branch 2 times, most recently from 2e7594c to 7d1d9a6 Compare April 21, 2023 00:20
@Gudahtt

This comment was marked as resolved.

@Gudahtt Gudahtt force-pushed the refresh-network-when-connection-is-reset branch from c88d819 to b319771 Compare April 21, 2023 00:57
@Gudahtt Gudahtt force-pushed the update-eip-1559-compatibility-during-network-lookup branch from 7d1d9a6 to 9ad143c Compare April 21, 2023 00:57
Base automatically changed from refresh-network-when-connection-is-reset to main April 21, 2023 01:38
@Gudahtt Gudahtt force-pushed the update-eip-1559-compatibility-during-network-lookup branch from 9ad143c to 92d373a Compare April 21, 2023 01:38
@Gudahtt Gudahtt marked this pull request as ready for review April 21, 2023 01:38
@Gudahtt Gudahtt requested a review from a team as a code owner April 21, 2023 01:38
@@ -5445,7 +5576,7 @@ describe('NetworkController', () => {

await waitForStateChanges(messenger, {
propertyPath: ['networkId'],
count: 1,
count: 2,
Copy link
Member Author

@Gudahtt Gudahtt Apr 21, 2023

Choose a reason for hiding this comment

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

It seems that the network ID we're checking for here was always the second state change. But the second state change was made synchronously after the first one, so the test still passed.

@Gudahtt Gudahtt mentioned this pull request Apr 21, 2023
3 tasks
The EIP-1559 compatibility state is now updated during the network
lookup, rather than during the provider configuration step.

Closes #1203
@Gudahtt Gudahtt force-pushed the update-eip-1559-compatibility-during-network-lookup branch from eb4fae4 to 4e1f13c Compare April 21, 2023 12:51
Copy link
Member

@mikesposito mikesposito left a comment

Choose a reason for hiding this comment

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

LGTM!

@Gudahtt Gudahtt merged commit 383bcea into main Apr 21, 2023
@Gudahtt Gudahtt deleted the update-eip-1559-compatibility-during-network-lookup branch April 21, 2023 13:27
@legobeat legobeat mentioned this pull request Apr 25, 2023
Gudahtt added a commit that referenced this pull request May 2, 2023
An unnecessary mock has been removed from a helper function used by
network client tests. The `getEIP1559Compatibility` function is only
called as part of `lookupNetwork` (which is also mocked) as of #1236,
so this has been redundant since then.

Relates to #1116
Gudahtt added a commit that referenced this pull request May 2, 2023
An unnecessary mock has been removed from a helper function used by
network client tests. The `getEIP1559Compatibility` function is only
called as part of `lookupNetwork` (which is also mocked) as of #1236,
so this has been redundant since then.

Relates to #1116
MajorLift pushed a commit that referenced this pull request Oct 11, 2023
The EIP-1559 compatibility state is now updated during the network
lookup, rather than during the provider configuration step.

Closes #1203
MajorLift pushed a commit that referenced this pull request Oct 11, 2023
An unnecessary mock has been removed from a helper function used by
network client tests. The `getEIP1559Compatibility` function is only
called as part of `lookupNetwork` (which is also mocked) as of #1236,
so this has been redundant since then.

Relates to #1116
MajorLift pushed a commit that referenced this pull request Oct 11, 2023
The EIP-1559 compatibility state is now updated during the network
lookup, rather than during the provider configuration step.

Closes #1203
MajorLift pushed a commit that referenced this pull request Oct 11, 2023
An unnecessary mock has been removed from a helper function used by
network client tests. The `getEIP1559Compatibility` function is only
called as part of `lookupNetwork` (which is also mocked) as of #1236,
so this has been redundant since then.

Relates to #1116
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

NetworkController: Capture EIP-1559 support for the current network within lookupNetwork
2 participants