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

✨ Ownership detection for ERC721 side #42

Open
Vectorized opened this issue Feb 13, 2024 · 11 comments
Open

✨ Ownership detection for ERC721 side #42

Vectorized opened this issue Feb 13, 2024 · 11 comments

Comments

@Vectorized
Copy link
Owner

Vectorized commented Feb 13, 2024

For marketplaces like Opensea

  • Upon initialization handshake, the mirror will try to get the owner from the base contract. If there is an owner, the owner will be updated to that owner, and a OwnershipTransferred(address indexed oldOwner, address indexed newOwner) event will be emitted. Store the current owner in the deployer slot.

  • Upon logging a batch NFT mint / burn, try to get the owner from the base contract. If the owner is different from that of the deployer slot, a OwnershipTransferred(address indexed oldOwner, address indexed newOwner) will be emitted, and the owner in the deployer slot will be updated.

Not sure if this will work, someone help me try.

@lambdalf-dev
Copy link
Collaborator

Why not just have an owner() function that returns the owner of the DN404 base contract?

@wwhchung
Copy link

Opensea does not rely on owner or OwnershipTransferred because it’s easily spoofable.

@lambdalf-dev
Copy link
Collaborator

Event easily spoofable for sure, owner is what the contract considers owner.

@lambdalf-dev
Copy link
Collaborator

Proposed solution here:
#43

@wwhchung
Copy link

Event easily spoofable for sure, owner is what the contract considers owner.

Yup, but OpenSea typically doesn't use this to group things under an address's 'created tokens' because that can also be faked. i.e. I can hardcode the owner() to return as you, and have these tokens as 'created by you' if OpenSea trusted the contract return values as the source of provenance.

@lambdalf-dev
Copy link
Collaborator

Well, if I'm the owner of the contract, it makes sense that the items were created by me...

@wwhchung
Copy link

Well, if I'm the owner of the contract, it makes sense that the items were created by me...

Not necessarily. I could create a contract with a fake owner function and inject tokens into your collection. Similar exploit has been used in the past to make fake beeples which is why rarible and opensea moved away from relying on owner.

@cygaar
Copy link
Collaborator

cygaar commented Feb 13, 2024

Yeah @wwhchung is correct here - it's extremely easy to game owner() to make it look like someone else deployed your contract

@TechnicallyWeb3
Copy link

TechnicallyWeb3 commented Feb 19, 2024

Why is the owner needed at all? I’ve got a version of ERC404 on my TW3404 repo that uses ERC1155 instead of ERC721 which allows for batch transfers natively.

@lambdalf-dev
Copy link
Collaborator

Why is the owner needed at all?

For marketplaces collection management

@sonicsmith
Copy link

Looks like a good solution would be to implement ERC-7015 since Opensea uses this for proof of ownership. (https://docs.opensea.io/docs/contract-level-metadata)

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

No branches or pull requests

6 participants