Skip to content
This repository was archived by the owner on Nov 10, 2023. It is now read-only.

(Fix) Remove the provider when the site is disconnected#1585

Merged
fernandomg merged 4 commits intodevelopmentfrom
fix/provider-connected-without-account
Nov 12, 2020
Merged

(Fix) Remove the provider when the site is disconnected#1585
fernandomg merged 4 commits intodevelopmentfrom
fix/provider-connected-without-account

Conversation

@fernandomg
Copy link
Contributor

When a user disconnected the Safe from the wallet, the Safe didn't remove the provider leading to the error: Provided address "" is invalid, the capitalization checksum test failed, or it's an indirect IBAN address which can't be converted.

This PR solves that issue by disconnecting the provider when there's no address provided by the connected wallet.

remove-provider

@github-actions
Copy link

CLA Assistant Lite All Contributors have signed the CLA.

@github-actions
Copy link

github-actions bot commented Nov 10, 2020

ESLint Summary View Full Report

Annotations are provided inline on the Files Changed tab. You can also see all annotations that were generated on the annotations page.

Type Occurrences Fixable
Errors 0 0
Warnings 1 0
Ignored 0 N/A
  • Result: ✅ success

  • Annotations: 1 total


[warning] @typescript-eslint/explicit-module-boundary-types

Require explicit return and argument types on exported functions' and classes' public class methods


Report generated by eslint-plus-action

@ghost
Copy link

ghost commented Nov 10, 2020

Travis automatic deployment:
https://pr1585--safereact.review.gnosisdev.com/volta/app

@ghost
Copy link

ghost commented Nov 10, 2020

Travis automatic deployment:
https://pr1585--safereact.review.gnosisdev.com/rinkeby/app

@dasanra
Copy link
Collaborator

dasanra commented Nov 11, 2020

@fernandomg Nice catch. I found one scenario where this still happen. It's not that critical because it's only once. If you Lock Metamask the Safe Multisig disconnects, but then you unlock Metamask again (without clicking anything in the Safe) and you get the samer error

@ghost
Copy link

ghost commented Nov 11, 2020

Travis automatic deployment:
https://pr1585--safereact.review.gnosisdev.com/rinkeby/app

@ghost
Copy link

ghost commented Nov 11, 2020

Travis automatic deployment:
https://pr1585--safereact.review.gnosisdev.com/volta/app

@ghost
Copy link

ghost commented Nov 11, 2020

Travis automatic deployment:
https://pr1585--safereact.review.gnosisdev.com/volta/app

@ghost
Copy link

ghost commented Nov 11, 2020

Travis automatic deployment:
https://pr1585--safereact.review.gnosisdev.com/rinkeby/app

@fernandomg
Copy link
Contributor Author

I found one scenario where this still happen. It's not that critical because it's only once. If you Lock Metamask the Safe Multisig disconnects, but then you unlock Metamask again (without clicking anything in the Safe) and you get the samer error

@dasanra, just created #1589, to keep track of this finding and a possible refactor of the connection management in the Safe.

@ghost
Copy link

ghost commented Nov 12, 2020

Travis automatic deployment:
https://pr1585--safereact.review.gnosisdev.com/volta/app

@ghost
Copy link

ghost commented Nov 12, 2020

Travis automatic deployment:
https://pr1585--safereact.review.gnosisdev.com/rinkeby/app

@francovenica
Copy link
Contributor

Never thought about this one.

Tried disconnecting from the MM prompt the connection to gnosis
If there are more than 1 MM address from the same wallet connected to gnosis, when you disconnect one it will jump to the next. When the last one is disconnected gnosis will cut the connection with MM completely
If you use the "Disconnect all accounts" from the MM prompt the same will happen, gnosis will cut connection with MM completely.

I had no problems reconnecting to the site after all this.

Looks good to me

@fernandomg fernandomg merged commit da1e39c into development Nov 12, 2020
@fernandomg fernandomg deleted the fix/provider-connected-without-account branch November 12, 2020 14:40
@github-actions github-actions bot locked and limited conversation to collaborators Nov 12, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants