Skip to content

Conversation

@matthewwalsh0
Copy link
Member

@matthewwalsh0 matthewwalsh0 commented Mar 12, 2024

Description

Fix hanging smart swaps by including chainId parameter in unsigned transactions when signing.

This is now required by the multi-chain support in TransactionController.

Open in GitHub Codespaces

Related issues

Fixes: #23373

Manual testing steps

See issue.

Screenshots/Recordings

Before

After

Pre-merge author checklist

  • I’ve followed MetaMask Coding Standards.
  • I've clearly explained what problem this PR is solving and how it is solved.
  • I've linked related issues
  • I've included manual testing steps
  • I've included screenshots/recordings if applicable
  • 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.
  • I’ve properly set the pull request status:
    • In case it's not yet "ready for review", I've set it to "draft".
    • In case it's "ready for review", I've changed it from "draft" to "non-draft".

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.

@matthewwalsh0 matthewwalsh0 added the team-confirmations-system-deprecated DEPRECATED: please use "team-confirmations" instead label Mar 12, 2024
@matthewwalsh0 matthewwalsh0 marked this pull request as ready for review March 12, 2024 13:30
@matthewwalsh0 matthewwalsh0 requested a review from a team as a code owner March 12, 2024 13:30
@metamaskbot
Copy link
Collaborator

Builds ready [928bf5a]
Page Load Metrics (1382 ± 499 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint752531544421
domContentLoaded11115402512
load59244513821040499
domInteractive11115402512
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 24 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

@codecov
Copy link

codecov bot commented Mar 12, 2024

Codecov Report

Attention: Patch coverage is 0% with 2 lines in your changes are missing coverage. Please review.

Project coverage is 68.71%. Comparing base (171b6c7) to head (928bf5a).
Report is 2 commits behind head on develop.

Files Patch % Lines
ui/ducks/swaps/swaps.js 0.00% 2 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop   #23425      +/-   ##
===========================================
- Coverage    68.72%   68.71%   -0.00%     
===========================================
  Files         1122     1122              
  Lines        43577    43579       +2     
  Branches     11661    11661              
===========================================
  Hits         29945    29945              
- Misses       13632    13634       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@matthewwalsh0 matthewwalsh0 merged commit 4fedbb5 into develop Mar 12, 2024
@matthewwalsh0 matthewwalsh0 deleted the fix/23373-smart-swaps-chain-id branch March 12, 2024 16:45
@github-actions github-actions bot locked and limited conversation to collaborators Mar 12, 2024
@metamaskbot metamaskbot added release-11.14.0 Issue or pull request that will be included in release 11.14.0 release-11.12.0 Issue or pull request that will be included in release 11.12.0 and removed release-11.14.0 Issue or pull request that will be included in release 11.14.0 labels Mar 12, 2024
@metamaskbot
Copy link
Collaborator

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

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

Labels

QA Passed release-11.12.0 Issue or pull request that will be included in release 11.12.0 team-confirmations-system-deprecated DEPRECATED: please use "team-confirmations" instead

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Swap is no longer working when Smart Transaction is turned ON

5 participants