MetaMask / metamask-sdk

The simplest yet most secure way to connect your blockchain-based applications to millions of MetaMask Wallet users.
https://metamask.io/sdk/
Other
188 stars 115 forks source link

fix: improper implementation of handleBatchMethod + more robust tests #1065

Closed ecp4224 closed 1 month ago

ecp4224 commented 1 month ago

Explanation

This PR fixes a bug where the extension provider invokes the metamask_batch request logic correctly, but then also sends a metamask_batch request again. This (at best) causes the requests to be sent individually, then all at once or (at worst) causing metamask_batch to break entirely if the extension doesn't support the RPC

Currently, the latter case happens. The requests are sent individually, but then errors because it then tries to send metamask_batch and the extension doesn't support that

Screenshot 2024-10-04 at 2 17 14 PM

With the fixed logic in handleBatchMethod, the extension and SDK return the correct result

Screenshot 2024-10-04 at 2 38 59 PM

This PR also updates the unit tests for handleBatchMethod to be more robust to ensure this behavior doesn't regress.

References

This likely closes #1045

Checklist

codecov[bot] commented 1 month ago

Codecov Report

Attention: Patch coverage is 80.00000% with 1 line in your changes missing coverage. Please review.

Project coverage is 78.14%. Comparing base (d2c776d) to head (bc466f6). Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...ider/extensionProviderHelpers/handleBatchMethod.ts 80.00% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #1065 +/- ## ========================================== - Coverage 78.16% 78.14% -0.02% ========================================== Files 179 179 Lines 4153 4155 +2 Branches 1020 1022 +2 ========================================== + Hits 3246 3247 +1 - Misses 907 908 +1 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

deeeed commented 1 month ago

@christopherferreira9 Can you take a look at this PR to make sure everything is good with metamask_batch since we recently had regression?

christopherferreira9 commented 1 month ago

Working nicely. Checked:

We have some unit tests failing, can you give it a look @ecp4224 ? We'll then be creating a new release with this change targeting 0.29.1.

sonarcloud[bot] commented 1 month ago

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud