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

[Feature]: ERC-721 Enumerable protection #81

Open
1 task done
rsodre opened this issue Jul 2, 2024 · 2 comments
Open
1 task done

[Feature]: ERC-721 Enumerable protection #81

rsodre opened this issue Jul 2, 2024 · 2 comments

Comments

@rsodre
Copy link

rsodre commented Jul 2, 2024

Feature Request

For theerc721_enumerable_component to work, its logic must be included in the contract, adding sensitive code into mint(), burn(), and re-implementing functions transfer_from() and safe_transfer_from (). The erc721:: enumerable_mintable_burnable preset exemplifies it.

This logic should be insideerc721_enumerable_component, and not exposed to the contract creator. Any changes to that could make the components behave erratically.

Proposed Solution

By using existing SRC5 introspection, erc721_balance_component can understand if the contract is using erc721_enumerable_component or not, and ask it to do what it has to do inside transfer_from() and safe_transfer_from (). Contracts that do not require this component should still include them, but not expose its functions.

  • Enable SRC5 introspection on all ERC-721 components.
  • Include the enumerable component on all components that access erc721_balance_component.
  • Include the enumerable component on all contracts, even the ones that do not use it, like presets::erc721::mintable_burnable.cairo. Enumerable functions must not be embedded on these contracts.
  • Add initialise() function to erc721_enumerable_component, calling sr5.register_interface(IERC721_ENUMERABLE_ID). Contracts that make use of erc721_enumerable_component need to call that function at construction time.
  • Add after_transfer() function to erc721_enumerable_component, with the logic currently at the preset, updating its models accordingly.
  • Check if the contract makes use of erc721_enumerable_component by calling sr5.supports_interface(IERC721_ENUMERABLE_ID), and call erc721_enumerable_component::after_transfer() if it does.
  • Remove internal calls from the erc721:: enumerable_mintable_burnable preset.
  • Remove transfer_from() and safe_transfer_from () from the erc721:: enumerable_mintable_burnable preset, and expose the original functions from erc721_balance_component.

Alternatives

Adding before_update() and after_update() hooks to erc721_balance_component, like OpenZeppelin does. but implementing that inside another component is not possible, and should be included inside the token contract, which takes us back to the original problem we're trying to solve.

Related Code

No response

Additional context

No response

If the feature is accepted, would you be willing to contribute it?

  • Yes I would be willing to contribute
@rsodre
Copy link
Author

rsodre commented Jul 2, 2024

already working on this, will open PR as soon as #79 is closed.

@ScottyDavies
Copy link

Hi @rsodre can i work on this?

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

2 participants