OpenZeppelin / OpenZeppelin/cairo-contracts

Consider moving "private" functions to a `PrivateImpl`

Open
#994 0 comments 1 reaction 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Rust
Stars
909
Forks
397
Avg merge
2d 4h
Merged PRs (30d)
4

Description

For functions meant to be private, it might make sense to move them out of a component's InternalImpl and into a PrivateImpl. I think this will better communicate that the moved functions are meant to be private. Here's an example of this idea: https://github.com/andrew-fleming/cairo-contracts/blob/ffe01ad9949e4e10ed21df69041bbeb673c404ba/src/token/erc721/extensions/erc721_enumerable/erc721_enumerable.cairo#L128-L136

Furthermore, this separation makes it more difficult to accidentally misuse sensitive functions. Most contracts bring a component's InternalImpl into scope to call the initializer which means that private functions are also immediately accessible

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Review the linked erc721_enumerable.cairo example, especially the referenced lines, then inspect how components expose InternalImpl and how contracts use initializer. Compare the current visibility of private functions with the proposed PrivateImpl separation. Done means the approach is defined consistently enough to apply across relevant components without making initializer usage unsafe.

Written by the indexing model from the issue text.

Assessment

Domain
blockchain, security
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
30/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.