frontend: Integrate Safe #1755
Merged
frontend: Integrate Safe #1755
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Added a commit for showing the warning if the same address as the connected Safe is chosen for the target address. |
GabrielBuragev
suggested changes
May 3, 2023
frontend/tests/unit/services/transactions/request-manager.spec.ts
Outdated
Show resolved
Hide resolved
GabrielBuragev
approved these changes
May 8, 2023
Moves any functionality but token adding and switching chains into a new abstract class `BasicEthereumProvider`. Enables to have providers which don't support these functionalities.
…er` optional Not all providers necessarily support these, like for example the Safe provider.
… switching chains In this case only the connected chain should be shown.
…so available There were some issues with the Safe integration. A safe could for example have setup multiple required signatures. Before a tx could fail when continued because the provider was not available. With multisignature safes it must be possible to close the app come back later.
Enables to connect to a gnosis Safe. It adds new autoconnect function for connecting as it is required by Safe.
Enables to use provider specific functionalities in these functions. Also makes the coupling with ethers JsonRPCSigner less. In the future we should instead use our own Signer interface to distinguish between simple providers for read operations and wallets which are able to sign tx.
Safe's use a special internal safeTxHash which is returned from the provider. We cannot use it to get the transaction events or show it in the explorer.
We assumed that our event is always the first one. In cases such as when using a safe provider the transaction call is delegated, therefore other events of the SafeProxy are also included in the transaction.
Safes are only autoconnected and they could not get connected again.
The issue is that a Safe is a contract which only exists on one chain.
… chosen as the target
…nectable and being a contract wallet Makes these features reusable when we introduce other providers
manuelwedler
force-pushed
the
frontend/safe-integration
branch
from
7bad9ca to
2207b1e
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
see #1507
(we still need to open an issue against their repo to get listed in the Safe App)
This was much more work than expected. Some function declarations and interfaces of our internal architecture needed to get changed. A few things are still missing:
The warning if the target address is the same as the Safe's one is missing. Some tests are missing.Maybe it would be nicer to use a flag on theIEthereumProviderinterface if the disconnect should be shown or if the target address should be prefilled.