Conversation
Summary by OctaneNew ContractsNo new contracts were added in this PR. Updated Contracts
🔗 Commit Hash: 413f3b6 |
There was a problem hiding this comment.
Pull Request Overview
This PR removes the prizeIsActive modifier from the updatePrize function, enabling administrators to edit prize details (name, value, quantity, and form ID) even after all winners have been drawn and the prize has been marked as inactive.
Key Changes:
- Removed the
prizeIsActivemodifier restriction from theupdatePrizefunction
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| uint256 quantity, | ||
| string calldata formId | ||
| ) external onlyRole(ADMIN_ROLE) prizeIsActive(prizeId) { | ||
| ) external onlyRole(ADMIN_ROLE) { |
There was a problem hiding this comment.
Removing the prizeIsActive check allows modification of prize details after the prize lifecycle has completed. This could lead to data integrity issues if prize information (name, value, quantity) is changed after winners have been determined, potentially creating inconsistencies in historical records or audit trails. Consider implementing a more granular permission system that allows editing only specific fields like formId for inactive prizes while restricting changes to core prize attributes.
| string calldata formId | ||
| ) external onlyRole(ADMIN_ROLE) prizeIsActive(prizeId) { | ||
| ) external onlyRole(ADMIN_ROLE) { | ||
| // Update prize details without affecting tickets or active status |
There was a problem hiding this comment.
The comment states 'without affecting tickets or active status' but this is now misleading since the function can now modify inactive prizes. Update the comment to clarify that this function can edit both active and inactive prizes, or specify which fields should be edited after a prize becomes inactive.
| // Update prize details without affecting tickets or active status | |
| // Update prize details (name, description, value, quantity, formId) for both active and inactive prizes without affecting tickets or the active status |
|
lgtm. |
Overview
Detailed findings
|
prizes are immediately marked as inactive and thus uneditable once all users are drawn but we often have to make updates such, such as tally form id