Merge pull request #26 from KaelSensei/fix/slither-low-nonbasic

fix: reduce Slither low findings (non-hacks)

Author: KaelSensei

PROGRESS.md

@@ -6,6 +6,7 @@ A hands-on Solidity training ground based on solidity-by-example.org.
 
 ## Completed
 
+- [x] **Slither low cleanup (non-hacks, non-basic, 2026-04)**: Reduced low-severity Slither code scanning noise outside `src/hacks/` and `src/basic/` by adding a `RewardNotified` event to `StakingRewards.notifyRewardAmount`, applying CEI-ordering to `DiscreteStakingRewards.notifyReward`, auctions (`DutchAuction.purchase`, `EnglishAuction.end`), `CrowdFund.pledge`, and `TokenLocker.createLock`, and adding narrowly scoped `slither-disable-next-line timestamp` annotations where `block.timestamp` comparisons are intentional (permit deadlines, timelock scheduling, auctions, oracle staleness, swap router deadlines). Full suite green via `forge test` in Docker.
 - [x] **Naming / NatSpec cleanup (2026-04)**: After Slither-oriented renames, fixed shadowing bugs (`value = value`, `num = num`), aligned bodies with new parameter names, removed drift-prone `/// @param` lines under `src/` (excluding `src/hacks/`), added `scripts/strip_natspec_params.py`, and updated tests (`ERC20Permit`, `GasGolf`, `Immutable`). Full suite green via `forge test` in Docker.
 - [x] **Slither / Code Scanning (zero address)**: Added `require(... != address(0), "Zero address")` on low-level call entry points flagged in PR review (`Call`, `SendingEther`, `TryCatch`, `Payable.withdrawTo`, `Immutable` constructor, `Delegatecall` proxy + demo), with matching revert tests.
 - [x] **Codecov**: Upgraded `codecov/codecov-action` to v5, added root `codecov.yml` (informational checks, PR comment layout), documented installing the [Codecov GitHub App](https://github.com/marketplace/codecov) in `CI.md` for reliable uploads and PR comments.

src/applications/ERC20Permit.sol

@@ -134,6 +134,7 @@ contract ERC20Permit {
         bytes32 s
     ) external {
         // Check deadline first
+        // slither-disable-next-line timestamp
         if (block.timestamp > deadline) {
             revert ExpiredDeadline(deadline, block.timestamp);
         }

src/applications/PaymentChannel.sol

@@ -75,6 +75,7 @@ contract PaymentChannel {
     /// @dev Only callable by the sender after the expiration timestamp.
     function timeout() external {
         if (msg.sender != sender) revert NotSender();
+        // slither-disable-next-line timestamp
         if (block.timestamp < expiration) revert ChannelNotExpired();
         if (closed) revert ChannelAlreadyClosed();
 

src/applications/TimeLock.sol

@@ -62,6 +62,7 @@ contract TimeLock {
         txId = getTxId(target, value, data, executeTime);
 
         if (queued[txId]) revert AlreadyQueued(txId);
+        // slither-disable-next-line timestamp
         if (executeTime < block.timestamp + MIN_DELAY || executeTime > block.timestamp + MAX_DELAY) {
             revert TimestampNotInRange(executeTime, block.timestamp + MIN_DELAY, block.timestamp + MAX_DELAY);
         }
@@ -81,7 +82,9 @@ contract TimeLock {
         bytes32 txId = getTxId(target, value, data, executeTime);
 
         if (!queued[txId]) revert NotQueued(txId);
+        // slither-disable-next-line timestamp
         if (block.timestamp < executeTime) revert TimestampNotPassed(executeTime, block.timestamp);
+        // slither-disable-next-line timestamp
         if (block.timestamp > executeTime + GRACE_PERIOD) revert TimestampExpired(executeTime, executeTime + GRACE_PERIOD);
 
         delete queued[txId];

src/defi/ChainlinkPriceFeed.sol

@@ -64,12 +64,14 @@ contract ChainlinkPriceFeed {
             uint256 updatedAt,
             uint80 answeredInRound
         ) = priceFeed.latestRoundData();
+        (startedAt); // intentionally unused
 
         require(answer > 0, "Invalid price");
         require(updatedAt > 0, "Round not complete");
         require(answeredInRound >= roundId, "Stale data");
 
         // Check staleness
+        // slither-disable-next-line timestamp
         uint256 timeSinceUpdate = block.timestamp - updatedAt;
         require(timeSinceUpdate <= staleThreshold, "Price is stale");
 
@@ -94,6 +96,7 @@ contract ChainlinkPriceFeed {
             return (0, true);
         }
 
+        // slither-disable-next-line timestamp
         uint256 timeSinceUpdate = block.timestamp - updatedAt;
         isStale = timeSinceUpdate > staleThreshold;
 

src/defi/CrowdFund.sol

@@ -69,15 +69,16 @@ contract CrowdFund {
 
     /// @notice Pledge tokens to the campaign
     function pledge(uint256 amount) external {
+        // slither-disable-next-line timestamp
         require(block.timestamp < deadline, "Campaign ended");
         require(amount > 0, "Cannot pledge 0");
-        
-        // Transfer tokens from pledger
-        if (!token.transferFrom(msg.sender, address(this), amount)) revert TransferFailed();
-        
-        // Update state
+
+        // Effects before interactions (CEI)
         pledges[msg.sender] += amount;
         totalPledged += amount;
+
+        // Transfer tokens from pledger
+        if (!token.transferFrom(msg.sender, address(this), amount)) revert TransferFailed();
 
         emit Pledged(msg.sender, amount);
     }
@@ -100,6 +101,7 @@ contract CrowdFund {
     /// @notice Claim funds if goal is met (only creator)
     function claim() external {
         require(msg.sender == creator, "Only creator");
+        // slither-disable-next-line timestamp
         require(block.timestamp >= deadline, "Campaign ongoing");
         require(!claimed, "Already claimed");
         require(totalPledged >= goal, "Goal not met");
@@ -114,6 +116,7 @@ contract CrowdFund {
 
     /// @notice Get refund if goal not met (only after deadline)
     function refund() external {
+        // slither-disable-next-line timestamp
         require(block.timestamp >= deadline, "Campaign ongoing");
         require(totalPledged < goal, "Goal met");
 

src/defi/DiscreteStakingRewards.sol

@@ -135,11 +135,11 @@ contract DiscreteStakingRewards {
         if (amount == 0) revert ZeroAmount();
         if (totalStaked == 0) revert InsufficientBalance(); // No stakers to receive rewards
 
-        if (!IERC20Staking(rewardToken).transferFrom(msg.sender, address(this), amount)) revert TransferFailed();
-
         // Increase the global reward index: each staked token earns (amount / totalStaked)
         rewardIndex += (amount * 1e18) / totalStaked;
 
+        if (!IERC20Staking(rewardToken).transferFrom(msg.sender, address(this), amount)) revert TransferFailed();
+
         emit RewardNotified(amount, rewardIndex);
     }
 

src/defi/DutchAuction.sol

@@ -81,10 +81,12 @@ contract DutchAuction {
 
     /// @notice Get current price based on time elapsed
     function currentPrice() public view returns (uint256) {
+        // slither-disable-next-line timestamp
         if (block.timestamp >= endsAt) {
             return minimumPrice;
         }
 
+        // slither-disable-next-line timestamp
         uint256 timePassed = block.timestamp - startsAt;
         uint256 maxDiscount = startingPrice - minimumPrice;
         // Avoid overflow: timePassed * discountRate
@@ -102,6 +104,7 @@ contract DutchAuction {
     /// @notice Purchase the NFT at current price
     function purchase() external payable {
         require(!ended, "Auction ended");
+        // slither-disable-next-line timestamp
         require(block.timestamp < endsAt, "Auction expired");
 
         uint256 price = currentPrice();
@@ -110,15 +113,15 @@ contract DutchAuction {
         // Mark as ended
         ended = true;
         buyer = msg.sender;
-        
-        // Transfer NFT to buyer
-        nft.safeTransferFrom(address(this), msg.sender, tokenId);
 
         // Pull-payment accounting (avoids direct ETH sends during purchase)
         pendingWithdrawals[seller] += price;
         if (msg.value > price) {
             pendingWithdrawals[msg.sender] += (msg.value - price);
         }
+        
+        // Transfer NFT to buyer
+        nft.safeTransferFrom(address(this), msg.sender, tokenId);
 
         emit AuctionEnded(msg.sender, price, block.timestamp);
     }

src/defi/EnglishAuction.sol

@@ -79,6 +79,7 @@ contract EnglishAuction {
         require(!started, "Already started");
 
         started = true;
+        // slither-disable-next-line timestamp
         endAt = block.timestamp + duration;
 
         emit Started(block.timestamp, endAt);
@@ -87,6 +88,7 @@ contract EnglishAuction {
     /// @notice Place a bid
     function bid() external payable {
         require(started, "Not started");
+        // slither-disable-next-line timestamp
         require(block.timestamp < endAt, "Ended");
         uint256 minRequired = highestBidder == address(0)
             ? highestBid
@@ -120,16 +122,17 @@ contract EnglishAuction {
     function end() external {
         require(started, "Not started");
         require(!ended, "Already ended");
+        // slither-disable-next-line timestamp
         require(block.timestamp >= endAt, "Not yet ended");
 
         ended = true;
 
         if (highestBidder != address(0)) {
-            // Transfer NFT to winner
-            nft.safeTransferFrom(address(this), highestBidder, tokenId);
-
             // Pull-payment for seller proceeds (avoids direct ETH send in end())
             pendingWithdrawals[seller] += highestBid;
+
+            // Transfer NFT to winner
+            nft.safeTransferFrom(address(this), highestBidder, tokenId);
         } else {
             // No bids - return NFT to seller
             nft.safeTransferFrom(address(this), seller, tokenId);

src/defi/StakingRewards.sol

@@ -91,6 +91,9 @@ contract StakingRewards {
     /// @notice Emitted when rewards are paid out
     event RewardPaid(address indexed user, uint256 reward);
 
+    /// @notice Emitted when rewards are notified and the distribution parameters are updated
+    event RewardNotified(uint256 reward, uint256 rewardRate, uint256 periodFinish);
+
     uint256 private _unlocked = 1;
     modifier nonReentrant() {
         require(_unlocked == 1, "Reentrancy");
@@ -126,6 +129,7 @@ contract StakingRewards {
 
     /// @notice Last time reward was applicable
     function lastTimeRewardApplicable() public view returns (uint256) {
+        // slither-disable-next-line timestamp
         return block.timestamp < periodFinish ? block.timestamp : periodFinish;
     }
 
@@ -225,9 +229,11 @@ contract StakingRewards {
 
         _updateReward(address(0));
 
+        // slither-disable-next-line timestamp
         if (block.timestamp >= periodFinish) {
             rewardRate = reward / rewardsDuration;
         } else {
+            // slither-disable-next-line timestamp
             uint256 remainingRewards = (periodFinish - block.timestamp) * rewardRate;
             rewardRate = (reward + remainingRewards) / rewardsDuration;
         }
@@ -236,8 +242,12 @@ contract StakingRewards {
         uint256 balance = rewardsToken.balanceOf(address(this));
         require(rewardRate <= balance / rewardsDuration, "Reward amount > balance");
 
+        // slither-disable-next-line timestamp
         periodFinish = block.timestamp + rewardsDuration;
+        // slither-disable-next-line timestamp
         lastUpdateTime = block.timestamp;
+
+        emit RewardNotified(reward, rewardRate, periodFinish);
     }
 
     /// @notice Exit the staking (withdraw + claim)