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

protocol: Superchain ERC721 Design #194

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

lukema95
Copy link

Description

Introduces new SuperchainERC721 contract standard and SuperchainERC721Bridge predeploy to enable seamless NFT transfers between Superchain L2s. The solution leverages Superchain's native cross-chain messaging system to ensure secure and efficient NFT cross-chain transfers.

Tests

Additional context

Metadata

}

//// @inheritdoc IERC165
function supportsInterface(bytes4 _interfaceId) public view virtual returns (bool) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I like the simplicity and how this conforms to ERC-7802. I don't think anything would go horribly wrong if somebody called this contract with an ERC20.

Copy link
Author

Choose a reason for hiding this comment

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

Thank you for the feedback! You raise a good point about interface handling.
I propose adding explicit ERC20 interface rejection for better clarity:

function supportsInterface(bytes4 interfaceId) public view virtual override returns (bool) {
return interfaceId == type(IERC165).interfaceId
|| interfaceId == type(IERC721).interfaceId
|| interfaceId == type(IL2ERC721CrossChain).interfaceId
|| (interfaceId == type(IERC20).interfaceId ? false : super.supportsInterface(_interfaceId));
}

This change would:

  1. Maintain the current simple and ERC-7802 compliant approach
  2. Explicitly handle ERC20 interface queries
  3. Delegate other unknown interfaces to the parent implementation

What do you think about this approach?

@skidipapo
Copy link

Xl0NVceZIQPu8kyHbcfhm5ErQQNGzUGUqqsqq1A

Copy link

@skidipapo skidipapo left a comment

Choose a reason for hiding this comment

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

protocol/superchain-erc721/superchain-erc721-design.md

@@ -0,0 +1,239 @@
# Purpose

Choose a reason for hiding this comment

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

protocol/superchain-erc721/superchain-erc721-design.md

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.

3 participants