Feat/59 add natspec descriptions

[mrscottyrose]

Adds NatSpec descriptions to generated functions in the Contracts Wizard. This improves code documentation and maintainability by providing clear documentation for all generated functions.

Closes #59

[github-actions[bot]]

All contributors have signed the CLA ✍️ ✅
_{Posted by the CLA Assistant Lite bot.}

[mrscottyrose]

I confirm that I have read and hereby agree to the OpenZeppelin Contributor License Agreement

[CoveMB]

Hey @mrscottyrose thanks for your MR ✨

Some considerations, when adding NatSpec we qwould probably want to align both content and formatting with what is found on the contract library here is an example We might not want to add comments to overrides.

Let's wait for @OpenZeppelin/contracts input for defining the content of the comments.

[mrscottyrose]

I've tried to resolve it according to the requirement could you please check and tell me if there's anything i could add or change? Thank you.

[CoveMB]

Thanks for making changes, please revert any type / number of arguments changes from the MR. In it's current state it does not build the package properly.

[mrscottyrose]

@CoveMB what about now? I couldn't understand why the netlify checks failed.

[mrscottyrose]

@CoveMB are these two changes are enough to merge the mr?? Or I have to change other things also?

[mrscottyrose]

@CoveMB is there anything I need to change?

[CoveMB]

@ernestognw could you validate/correct the comment's content?

[mrscottyrose]

@ericglau sorry I was messing everything again and again. Could you please review the new commit? If there are any changes let me know thanks

[mrscottyrose]

@CoveMB can you review it again?

[mrscottyrose]

@CoveMB can you check now? if everything is alright or not?

[CoveMB]

It seem that this comment has not been addressed? (seem like the specialised comment is deleted instead of overriding the general comment)

[CoveMB]

Thanks for making the code changes 🦾 you will also need to update the tests as the contract got modified (with the added natspecs)

[mrscottyrose]

@CoveMB I ran the test and some of the tests fails cuz I'm in windows. What should I do? Will I commit the generated test files?

[CoveMB]

humm interesting could you share the issues here?

[mrscottyrose]

The log is huge. When I run yarn prepare some tests fails. @CoveMB

[ObjectPlayer]

Hey @mrscottyrose @CoveMB

I pulled the package locally and ran the project to debug the errors. Here’s what I found and possible solutions:

1. yarn prepare engine error

• CI requires Node ^18.18.0 || ^20.9.0 || >=21.1.0.
• On my machine, Node v22.14.0 works fine.
• ✅ Suggestion: update .nvmrc / engines field to include 22.x or pin CI to a specific version.

---

2. yarn format:check failures

Errors appeared on:

• packages/core/solidity/remappings.txt
• scripts/release/publish.sh
• scripts/release/version.sh

These aren’t actual formatting problems — Prettier just doesn’t know which parser to use.

✅ Fix options:

• Add these files to .prettierignore, or
• Extend Prettier config with overrides:

  "overrides": [
    { "files": "*.sh", "options": { "parser": "sh" } },
    { "files": "remappings.txt", "options": { "parser": "markdown" } }
  ]
  

• and install prettier-plugin-sh to support shell scripts.

---

3. yarn lint failures

Errors appeared on:

• Fails due to autofixable issues.
• ✅ Can be solved by running:

   yarn lint --fix
  

---

4. Changesets issue

• Current changeset file references @openzeppelin/contracts-wizard, which doesn’t exist.
• Correct package name is @openzeppelin/wizard (see packages/core/solidity/package.json).

✅ Fix: update .changeset/six-yaks-accept.md to:

- "@openzeppelin/contracts-wizard": patch
+ "@openzeppelin/wizard": patch

---

✅ Conclusion

With these adjustments, CI should pass smoothly.

If you’re open to it, could you give me contribution access to your PR branch? That way I can push these fixes directly and help move it forward. 🚀

[CoveMB]

Thanks for landing a hand @ObjectPlayer 🌾 Let us know if you can move forward or maybe you could also pull this commit from a fork of yours

[ObjectPlayer]

Thanks @CoveMB Thanks for the guidance 🌱 Since I don’t have access to the contributor’s repo, I’ll go ahead and fork the branch, apply the fixes I tested locally, and open a new PR that builds on #549 . I’ll make sure to credit @mrscottyrose for their work.

[ericglau]

Hi @mrscottyrose, thanks a lot for your efforts here. However, from reviewing the proposed changes for these NatSpec descriptions, we’ve determined that these may not be beneficial for users, and instead they could make the code harder to read.

For function overrides due to multiple inheritance, of which there are many, these do not really benefit from NatSpec since users should just look at the imported (overridden) library functions' NatSpec. For other functions, many have just one line implementations with potentially a modifier like onlyOwner. Users should inspect these implementations rather than rely on NatSpec descriptions. There are some cases where comments are useful, but these should be handled case-by-case, and comments are already included in some areas where needed (for example, in the Account tab).

For these reasons, we will close this PR and the original issue as won’t fix.