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

Add Security Testing Requirements and Update Documentation #331

Merged
merged 2 commits into from
Dec 10, 2024

Conversation

savvar9991
Copy link

@savvar9991 savvar9991 commented Dec 4, 2024

Pull Request: Comprehensive Documentation Update

Changes Overview

This PR includes comprehensive updates to both Engineering Guidelines and Licensing documentation.

Files Modified

  • GUIDELINES.md
  • .test/foundry/modules/licensing/README.md

Documentation Improvements

Fixed typos and grammar:

  • ✅ "Test should use" → "Tests should use"
  • ✅ "upgreadeability" → "upgradeability"
  • ✅ "methods an events" → "methods and events"
  • ✅ "worklows" → "workflows"

- Standardize comment formatting across documentation using /* */ style
- Improve readability of edge cases and TODO comments
- Add clear visual separation between code sections
- Maintain consistent indentation and structure
- Update test case status indicators (✅/🚧)

This improves documentation clarity and makes it easier to follow the licensing system behavior and test cases.
- Add Security Testing section to Engineering Guidelines with critical aspects:
  - Access control and permission systems
  - Token economics and balance accounting
  - Upgrade mechanisms and storage layouts
  - External contract interactions and reentrancy guards
  - Event emission for state changes

This addition helps developers focus on key security aspects when testing smart contracts.
Copy link

@kingster-will kingster-will left a comment

Choose a reason for hiding this comment

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

LGTM!

@kingster-will kingster-will merged commit 231401e into storyprotocol:main Dec 10, 2024
2 checks passed
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.

2 participants