Skip to content

fix: missing walletConnect prop#9283

Merged
christopherferreira9 merged 3 commits intomainfrom
fix/rpcmiddleware-9246
Apr 17, 2024
Merged

fix: missing walletConnect prop#9283
christopherferreira9 merged 3 commits intomainfrom
fix/rpcmiddleware-9246

Conversation

@abretonc7s
Copy link
Copy Markdown
Contributor

@abretonc7s abretonc7s commented Apr 17, 2024 •
edited
Loading

Description

  • Issue with generateRawSignature that didn't include the isWalletConnect property.
  • Issue undetected because of a missing type (set as any) in the method signature.

Related issues

Fixes: #9246

Manual testing steps

  1. Go to this page...

Screenshots/Recordings

Before

After

Pre-merge author checklist

  • I’ve followed MetaMask Coding Standards.
  • I've completed the PR template to the best of my ability
  • I’ve included tests if applicable
  • I’ve documented my code using JSDoc format if applicable
  • I’ve applied the right labels on the PR (see labeling guidelines). Not required for external contributors.

Pre-merge reviewer checklist

  • I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed).
  • I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 17, 2024

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

@abretonc7s abretonc7s added the WalletConnect WalletConnect related issue or bug label Apr 17, 2024
@abretonc7s abretonc7s marked this pull request as ready for review April 17, 2024 12:23
@abretonc7s abretonc7s requested a review from a team as a code owner April 17, 2024 12:23
@andreahaku andreahaku added team-sdk-deprecated DEPRECATED: please use "team-wallet-integrations" instead needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) labels Apr 17, 2024
@abretonc7s abretonc7s mentioned this pull request Apr 17, 2024
Closed
andreahaku
andreahaku previously approved these changes Apr 17, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@andreahaku andreahaku left a comment

Choose a reason for hiding this comment

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

LGTM

@christopherferreira9
Copy link
Copy Markdown
Contributor

christopherferreira9 commented Apr 17, 2024

Test case: #9246

Screen.Recording.2024-04-17.at.14.40.53.mov

@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud bot commented Apr 17, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@christopherferreira9 christopherferreira9 added QA Passed QA testing has been completed and passed Run Smoke E2E and removed needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) labels Apr 17, 2024
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 17, 2024 •
edited by metamaskbot
Loading

https://bitrise.io/ Bitrise

✅✅✅ pr_smoke_e2e_pipeline passed on Bitrise! ✅✅✅

Commit hash: 848fd07
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/e3b433af-96e0-4209-837e-689bd1bbb0c9

Note

  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

andreahaku
andreahaku approved these changes Apr 17, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@andreahaku andreahaku left a comment

Choose a reason for hiding this comment

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

LGTM

@christopherferreira9 christopherferreira9 merged commit c8c78d6 into main Apr 17, 2024
@christopherferreira9 christopherferreira9 deleted the fix/rpcmiddleware-9246 branch April 17, 2024 14:54
@github-actions github-actions bot locked and limited conversation to collaborators Apr 17, 2024
@metamaskbot metamaskbot added release-7.22.0 Issue or pull request that will be included in release 7.22.0 release-7.21.0 Issue or pull request that will be included in release 7.21.0 and removed release-7.22.0 Issue or pull request that will be included in release 7.22.0 labels Apr 17, 2024
@metamaskbot
Copy link
Copy Markdown
Collaborator

metamaskbot commented Apr 17, 2024

Missing release label release-7.21.0 on PR. Adding release label release-7.21.0 on PR and removing other release labels(release-7.22.0), as PR was cherry-picked in branch 7.21.0.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer

@andreahaku andreahaku andreahaku approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@christopherferreira9 christopherferreira9

Labels

QA Passed QA testing has been completed and passed release-7.21.0 Issue or pull request that will be included in release 7.21.0 team-sdk-deprecated DEPRECATED: please use "team-wallet-integrations" instead WalletConnect WalletConnect related issue or bug

Projects

PR review queue
Archived in project

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Bug]: Signatures - Triggering signatures with WalletConnect throws error Invalid parameters: must provide an Ethereum address."}

4 participants

@abretonc7s @christopherferreira9 @metamaskbot @andreahaku