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 eth-json-rpc-middleware to v9.0.1 #16096

Merged
merged 1 commit into from
Oct 10, 2022

Conversation

Gudahtt
Copy link
Member

@Gudahtt Gudahtt commented Oct 5, 2022

This update includes fixes for our block-ref and retry-on-empty middleware.

The block-ref middleware resolves the block reference latest to a specific block number, the latest one we are aware of. This is meant to protect against situations where the network gives inconsistent answers for what the latest block number is due to some nodes being out-of-sync with each other (this was a frequent problem years ago with Infura).

It was broken in that the latest resolution was failing, and we were submitting an additional redundant request to Infura for each request.

The retry-on-empty middleware is meant to retry certain methods when they return an empty response. This was also meant to deal with network synchronization issues that were more common years ago. This middleware works by making a "child" request over and over until either a retry limit is reached, or a non-empty response is received.

It was broken in that the final response recieved was thrown away, so it's as though the middleware was not used. Except that it did result in additional redundant network requests.

As a result of this update we should see that the extension is more resilient to certain network synchronization issues. But this is difficult to test, and these issues may not happen in production anymore today.

We should see a reduction in requests to Infura as well. This should be easier to test.

Manual Testing Steps

  1. Checkout the develop branch and create a build.
  2. Open the background console to the network tab
  3. Perform some action that interacts with the network (e.g. confirm a transaction).
  4. Take note of the Infura requests that your interaction caused.
  5. Perform steps 2-4 again on this branch, again taking note of the Infura requests. See that fewer requests are made.

Aside from testing that some redundant network requests have been eliminated, this should have no functional impact that we can practically test for.

Pre-Merge Checklist

  • PR template is filled out
  • IF this PR fixes a bug, a test that would have caught the bug has been added
  • PR is linked to the appropriate GitHub issue
  • PR has been added to the appropriate release Milestone

+ If there are functional changes:

  • Manual testing complete & passed
  • "Extension QA Board" label has been applied

@Gudahtt Gudahtt force-pushed the update-eth-json-rpc-middleware branch 4 times, most recently from 21e2fbc to 1bb2679 Compare October 5, 2022 21:28
@Gudahtt Gudahtt marked this pull request as ready for review October 5, 2022 21:34
@Gudahtt Gudahtt requested a review from a team as a code owner October 5, 2022 21:34
@Gudahtt Gudahtt requested review from Gtonizuka and mcmire October 5, 2022 21:34
@Gudahtt Gudahtt added this to the v10.21.0 milestone Oct 6, 2022
@metamaskbot
Copy link
Collaborator

Builds ready [1bb2679]
Page Load Metrics (2183 ± 54 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint88161105199
domContentLoaded18162371217311756
load18362371218311354
domInteractive18162371217311756

@mcmire
Copy link
Contributor

mcmire commented Oct 7, 2022

Looks good, pending a rebase.

This update includes fixes for our `block-ref` and `retry-on-empty`
middleware.

The `block-ref` middleware resolves the block reference `latest` to a
specific block number, the latest one we are aware of. This is meant to
protect against situations where the network gives inconsistent answers
for what the latest block number is due to some nodes being out-of-sync
with each other (this was a frequent problem years ago with Infura).

It was broken in that the `latest` resolution was failing, and we were
submitting an additional redundant request to Infura for each request.

The `retry-on-empty` middleware is meant to retry certain methods
when they return an empty response. This was also meant to deal with
network synchronization issues that were more common years ago. This
middleware works by making a "child" request over and over until either
a retry limit is reached, or a non-empty response is received.

It was broken in that the final response recieved was thrown away, so
it's as though the middleware was not used. Except that it did result
in additional redundant network requests.

As a result of this update we should see that the extension is more
resilient to certain network synchronization issues. But this is
difficult to test, and these issues may not happen in production
anymore today.

We should see a reduction in requests to Infura as well. This should
be easier to test.
@Gudahtt Gudahtt force-pushed the update-eth-json-rpc-middleware branch from 1bb2679 to 8c066d2 Compare October 10, 2022 13:04
@metamaskbot
Copy link
Collaborator

Builds ready [8c066d2]
Page Load Metrics (2380 ± 135 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint892421213517
domContentLoaded206333032376284136
load206333032380282135
domInteractive206333032376284136

@Gudahtt Gudahtt merged commit f6f8edf into develop Oct 10, 2022
@Gudahtt Gudahtt deleted the update-eth-json-rpc-middleware branch October 10, 2022 15:46
@github-actions github-actions bot locked and limited conversation to collaborators Oct 10, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants