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

Make Container a valid ERC721 and ERC1155 receiver #24

Merged
merged 10 commits into from
Sep 10, 2024

Conversation

gabrielstoica
Copy link
Collaborator

@gabrielstoica gabrielstoica commented Sep 2, 2024

Context

As we want our Container.sol contract to act as a true vault for users, we should allow anyone to also deposit ERC-721 or ERC-1155 tokens, apart from ERC-20 tokens and native ETH.

Changelog

This PR adds new functionalities as follows:

  • onERC721Received hook required for any contract to be IERC721Receiver compliant;
  • onERC1155Received and onERC1155BatchReceived hooks required for any contract to be IERC1155Receiver compliant;
  • withdrawERC721 method to allow ERC-721 token withdrawal by the Container owner;
  • withdrawERC1155 method to allow single and batch ERC-1155 token(s) withdrawal by the Container owner;
  • according events;
  • tests to handle the new functionalities;

Other unrelated add-ons:

  • fallback method on the Container.sol contract to emit NativeReceived event when ETH is sent to the contract with data or a function is called that doesn't exist in the contract;

task: none

@gabrielstoica gabrielstoica requested a review from V1d0r September 6, 2024 10:11
Copy link
Contributor

@V1d0r V1d0r left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great call! The container can be used as a vault so should support holding all native tokens.

@gabrielstoica gabrielstoica merged commit 28ec7de into main Sep 10, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants