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.

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

ERC777 support in MultiSigWallet and gnosis safe #104

Closed
guylando opened this issue May 11, 2019 · 4 comments
Closed

ERC777 support in MultiSigWallet and gnosis safe #104

guylando opened this issue May 11, 2019 · 4 comments

Comments

@guylando
Copy link

guylando commented May 11, 2019

After reviewing open zepplin implementation of ERC777 (OpenZeppelin/openzeppelin-contracts#1749 question number 7) it seems that it requires multisig wallets contracts to implement tokensReceived (IERC777Recipient interface) in order to be able to deposit ERC777 tokens in the multisig wallet.

What is the status of ERC777 support in MultiSigWallet and gnosis safe?

This question is relevant for both gnosis safe and MultiSigWallet since it was recommended for now to still use MultiSigWallet for new tokens: #103

Seems that the question is even more urgent for MultiSigWallet because since we plan to create an ERC777 token, if there is no support for it in the multisig wallet then we will not be able to use it or we will need to fork it.

UPDATE: At another look at the ERC777 implementation it seems the ERC20 functions in the contract DO NOT enforce the IERC777Recipient interface implementation on the multisig wallet. Will just need to confirm / test that it indeed works at MultiSigWallet. Did you have the chance of checking it? The open zepplin ERC777 implementation is not in draft stage anymore, its public.
Also of course it is better if it would be possible to use the new ERC777 functions although it is not a must, so it IS interesting to know the status regarding implementing the NEW functions.

Thanks

@rmeissner
Copy link
Member

As ERC777 also allow multiple ways to use it without implementing this method we will not do this right now. Something like this is in consideration for ERC1155 for the Safe, but this is less likely that we make adjustments to the MultiSigWallet.

ERC777 should pose not issue, either use the "unsecure" transfer or register a delegate implementation for the check (see ERC1820)

@guylando
Copy link
Author

What would you suggest as a basic implementation of IERC777Recipient for the multisig wallet? Just a stub which does nothing (or possibly emits log although seems unnecessary since logs are already emitted for transfers)?
I am guessing the benefits of setting the delegate implementation (even stub) would be for working with tools/contracts/systems which do not fallback to ERC20 and only support the new ERC777 functions. So if such tools/contracts/systems would start to exist (not sure if they exist right now, probably not) then it would probably be good if there was some recommended implementation of IERC777Recipient for the multisig wallet published. If someone will want to implement additional validation on ERC777 receival then he will implement his own IERC777Recipient. Another benefit is if someone will want to use operators with the multisig wallet. Seems that in that case the open zepplin implementation forces to use IERC777Recipient even if it just has a stub implementation.
What do you think?

@rmeissner
Copy link
Member

The simplest implementation would just be a stub. But it probably would make sense to allow the user to configure the multisig to limit the tokens it wants to receive (Blacklisting or whitelisting).

@rmeissner
Copy link
Member

Closed with introduction of https://github.com/gnosis/safe-contracts/blob/development/contracts/handler/DefaultCallbackHandler.sol in version 1.1.0 of the contracts

fdarian pushed a commit to fdarian/safe-contracts that referenced this issue Jan 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants