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.

Sign up for GitHub

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

Jump to bottom

[Bug]: Connect - Queuing connect requests from different dapps causes to display all domains as the first domain that made the request #29594

Closed
seaona opened this issue Jan 9, 2025 · 2 comments · Fixed by #29653
Closed

[Bug]: Connect - Queuing connect requests from different dapps causes to display all domains as the first domain that made the request #29594

seaona opened this issue Jan 9, 2025 · 2 comments · Fixed by #29653
Assignees
@matthewwalsh0
Labels
regression-RC-12.10.0 Regression bug that was found in release candidate (RC) for release 12.10.0 release-12.11.0 Issue or pull request that will be included in release 12.11.0 release-blocker This bug is blocking the next release Sev1-high High severity; partial loss of service with severe impact upon users, with no workaround. team-confirmations Push issues to confirmations team team-wallet-api-platform type-bug

Comments

@seaona
Copy link
Contributor

seaona commented Jan 9, 2025

Describe the bug

Queuing connect requests from different dapps causes to display all domains as the first domain that made the request.

Note that in current production 12.9.3, the requests are not queued, so the issue is not present there.

Expected behavior

Display the correct domain for each request

Screenshots/Recordings

connect-incorrect-domain.mp4

Steps to reproduce

  1. Open one browser tab and go to a dapp ie https://metamask.github.io/test-dapp/
  2. Open another browser tab and go to Uniswap https://app.uniswap.org/swap
  3. Click Connect to the test dapp
  4. Without accepting or rejecting, go to Uniswap
  5. Click Connect to Uniswap
  6. Navigate to both requests and see how both domains display the test-dapp one
  7. Cancel the first request, the one coming from the test dapp
  8. See how the second request still displays the incorrect domain (test dapp) instead of uniswap

Error messages or log output



Detection stage


During release testing


Version


12.10.0


Build type


None


Browser


Chrome


Operating system


Linux


Hardware wallet


No response


Additional context


No response


Severity


No response

      		
    

      
  





        



            

              
            

        

      


      
    








      

    



      

  
            

    

      
    

    


      

          @seaona
seaona




            added
      

  regression-RC-12.10.0

  Regression bug that was found in release candidate (RC) for release 12.10.0

      

  release-blocker

  This bug is blocking the next release

      

  Sev1-high

  High severity; partial loss of service with severe impact upon users, with no workaround.

      

  type-bug


  labels


      Jan 9, 2025

    

  



    


    

    

      
    

    


      

          @github-project-automation
github-project-automation
bot



          moved this to To be fixed
  in Bugs by severity


      Jan 9, 2025

    

  



    

    

      
    

    


      

          @github-project-automation
github-project-automation
bot



          moved this to To be fixed
  in Bugs by team


      Jan 9, 2025

    

  








      

  
      



  

  @seaona





  


    

      

  

    

        
    
    

  


      
          
  
      
            Copy link

  

      
    

  


  

      

  
    Contributor


      

  Author


  


  

    

      

      
            seaona
  

      

      

      commented


        Jan 9, 2025


      
    


  





      


        


  
    

      
          
Note: I added the sev1 label since this seems like a security issue, but feel free to re-assess if needed @adonesky1 🙏

      		
    

  





        


            

            

              
            

        

      


      

            
  

    
  
    
    

  

  



    












      

  
            







      

  
      



  

  @adonesky1





  


    

      

  

    

        
    
    

  


      
          
  
      
            Copy link

  

      
    

  


  

      

  
    Contributor


      

  


  

    

  





      


        


  
    

      
          
I believe the issue is introduced by this commit, this PR, which makes some updates to the confirmation navigation system

      		
    

  





        


            

            

              
            

        

      


      

            
  

    
  
    
    

  

  



    












      

  
            

    

      
    

    


      

          @adonesky1
adonesky1




          added
  the 

  team-confirmations

  Push issues to confirmations team
 label


      Jan 9, 2025

    

  



    

    

      
    

    


      

          @matthewwalsh0
matthewwalsh0




              self-assigned this


      Jan 13, 2025

    

  



  

  

    
  


  

      

    @matthewwalsh0
matthewwalsh0



    mentioned this issue
    
      Jan 13, 2025
    





  


  

      
   Merged

  





  
  

      
        
      
      

  
    
    
  


    7 tasks
  





  




  


  
    
  
  

    
  github-merge-queue bot

      pushed a commit
      that referenced
      this issue

    
      Jan 13, 2025
    
    

      


  

    

      


  

      
        @matthewwalsh0
  





      

        
          fix: duplicate content in multiple pending connect confirmations (#29653
        

          
            
          

      


      

          

    
    
    

  

    


      


      

          

    
    
    

  

  

    
  



      


      
      

        
          38b7bc3
        
      

    

  

    

      
)

## **Description**

Fix duplicate content when there are multiple pending connect
confirmations.

Caused by props being copied to state but not updated.

## **Related issues**

Fixes: #29594 

## **Manual testing steps**

See issue.

## **Screenshots/Recordings**

### **Before**

### **After**

## **Pre-merge author checklist**

- [x] I've followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask
Extension Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [x] I've completed the PR template to the best of my ability
- [x] I’ve included tests if applicable
- [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [x] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)).
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-project-automation
github-project-automation
bot



          moved this from To be fixed
  to Fixed
  in Bugs by team


      Jan 13, 2025

    

  



    

    

      
    

    


      

          @metamaskbot
metamaskbot




          added
  the 

  release-12.11.0

  Issue or pull request that will be included in release 12.11.0
 label


      Jan 13, 2025

    

  



  


  
    
  
  

    
  matthewwalsh0 

      added a commit
      that referenced
      this issue

    
      Jan 13, 2025
    
    

      


  

    

      


  

      
        @matthewwalsh0
  





      

        
          fix: duplicate content in multiple pending connect confirmations (#29653
        

          
            
          

      


      

          

    
    
    

  

      


      

          

    
    
    

  

  

    
  



      


      
      

        
          7a31865
        
      

    

  

    

      
)

## **Description**

Fix duplicate content when there are multiple pending connect
confirmations.

Caused by props being copied to state but not updated.

## **Related issues**

Fixes: #29594 

## **Manual testing steps**

See issue.

## **Screenshots/Recordings**

### **Before**

### **After**

## **Pre-merge author checklist**

- [x] I've followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask
Extension Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [x] I've completed the PR template to the best of my ability
- [x] I’ve included tests if applicable
- [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [x] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)).
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.

    




    

  




  

  

    
  


  

      

    @FrederikBolding
FrederikBolding



    mentioned this issue
    
      Jan 24, 2025
    





  


  

      
   Merged

  





  
  

      
        
      
      

  
    
    
  


    7 tasks
  





  













  
  

    

      

  





  




  
  

              

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


  



  






  
                  




    

  


      
  

    Assignees
  



        

      

        

  
      @matthewwalsh0
  
    matthewwalsh0
    

      








      


  


  

    Labels
  



    

      

    regression-RC-12.10.0

  Regression bug that was found in release candidate (RC) for release 12.10.0
  

    release-12.11.0

  Issue or pull request that will be included in release 12.11.0
  

    release-blocker

  This bug is blocking the next release
  

    Sev1-high

  High severity; partial loss of service with severe impact upon users, with no workaround.
  

    team-confirmations

  Push issues to confirmations team
  

    team-wallet-api-platform

  

    type-bug









      

  

    

        

    Projects
  


        




        
  

    

      
          
        Bugs by severity
        
    

    

      

          

  

    
      Archived in project



      

    

    

  



        
  

    

      
          
        Bugs by team
        
    

    

      

            
    Status: Fixed

              
      

    

    

  







  



      


  

    
  

    Milestone
  


      No milestone





    
      
          


  

    
      

        
    

    Development
  




          
  
Successfully merging a pull request may close this issue.





    


  
          
      fix: duplicate content in multiple pending connect confirmations

    
      MetaMask/metamask-extension
    





    
  




      

    

  
      

  

    

      4 participants
    

    

        
          @matthewwalsh0 
        
          @adonesky1 
        
          @metamaskbot 
        
          @seaona