Jerms/fix restore streak #238
Conversation
yogurtandjam
commented
Oct 17, 2025
yogurtandjam
commented
- emit currentStreak on spin complete
- restoreSTreak also sets last spin timestamp
Summary by OctaneNew ContractsNo new contracts were added in this PR. Updated Contracts
🔗 Commit Hash: 70962a7 |
There was a problem hiding this comment.
Pull Request Overview
Adds current streak information to the SpinCompleted event and ensures admin streak restoration updates last spin timestamp.
- Extend SpinCompleted event with currentStreak parameter and emit it.
- Update restoreStreak and restoreStreaks to also set lastSpinTimestamp.
- No input validation added for new/changed behaviors.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| event SpinCompleted( | ||
| address indexed walletAddress, string rewardCategory, uint256 rewardAmount, uint256 currentStreak | ||
| ); |
There was a problem hiding this comment.
Changing the SpinCompleted event signature (adding currentStreak) is a breaking change for off-chain consumers relying on the previous ABI. Consider adding a new event (e.g., SpinCompletedV2) and emitting both during a transition period, or clearly documenting the migration requirement.
|
|
||
| function restoreStreak(address user, uint256 streak) external onlyRole(ADMIN_ROLE) { | ||
| userData[user].streakCount = streak; | ||
| userData[user].lastSpinTimestamp = block.timestamp; | ||
| } | ||
|
|
There was a problem hiding this comment.
[nitpick] restoreStreak now also updates lastSpinTimestamp but the function name and (presumed) existing documentation suggest only streak restoration. Add a comment clarifying this side-effect or rename if broader state restoration is intended.
| function restoreStreak(address user, uint256 streak) external onlyRole(ADMIN_ROLE) { | |
| userData[user].streakCount = streak; | |
| userData[user].lastSpinTimestamp = block.timestamp; | |
| } | |
| /** | |
| * @notice Restores a user's streak count and also updates their lastSpinTimestamp to the current block timestamp. | |
| * This function has the side-effect of resetting the user's last spin time. | |
| */ | |
| function restoreStreak(address user, uint256 streak) external onlyRole(ADMIN_ROLE) { | |
| userData[user].streakCount = streak; | |
| userData[user].lastSpinTimestamp = block.timestamp; | |
| } | |
| /** | |
| * @notice Restores multiple users' streak counts and also updates their lastSpinTimestamp to the current block timestamp. | |
| * This function has the side-effect of resetting each user's last spin time. | |
| */ |
| function restoreStreaks(address[] memory users, uint256[] memory streaks) external onlyRole(ADMIN_ROLE) { | ||
| for (uint256 i = 0; i < users.length; i++) { | ||
| userData[users[i]].streakCount = streaks[i]; | ||
| userData[users[i]].lastSpinTimestamp = block.timestamp; | ||
| } | ||
| } |
There was a problem hiding this comment.
Missing validation that users and streaks arrays have equal length; a mismatch will either revert (out-of-bounds) or silently ignore extra streak entries. Add a require(users.length == streaks.length, 'Length mismatch') before the loop for clearer input validation.
| function restoreStreaks(address[] memory users, uint256[] memory streaks) external onlyRole(ADMIN_ROLE) { | ||
| for (uint256 i = 0; i < users.length; i++) { | ||
| userData[users[i]].streakCount = streaks[i]; | ||
| userData[users[i]].lastSpinTimestamp = block.timestamp; | ||
| } | ||
| } |
There was a problem hiding this comment.
[nitpick] Similar to restoreStreak, this batch function now alters both streakCount and lastSpinTimestamp; add a comment explaining why timestamp is reset during restoration to avoid confusion for maintainers.
Overview
Detailed findings
|
