Merge pull request #18 from KaelSensei/fix/slither-alerts-clean

Fix/slither alerts clean

Author: KaelSensei

Dockerfile

@@ -3,6 +3,7 @@ ENV DEBIAN_FRONTEND=noninteractive
 
 RUN apt-get update && apt-get install -y \
     curl git build-essential pkg-config libssl-dev \
+    python3 python3-pip \
     && curl -fsSL https://deb.nodesource.com/setup_20.x | bash - \
     && apt-get install -y nodejs \
     && rm -rf /var/lib/apt/lists/*
@@ -11,4 +12,9 @@ RUN curl -L https://foundry.paradigm.xyz | bash
 ENV PATH="/root/.foundry/bin:$PATH"
 RUN foundryup
 
+# Slither (static analyzer) expects `solc` available on PATH
+RUN pip3 install --no-cache-dir slither-analyzer solc-select \
+    && solc-select install 0.8.26 \
+    && solc-select use 0.8.26
+
 WORKDIR /workspace

PROGRESS.md

@@ -45,6 +45,8 @@ A hands-on Solidity training ground based on solidity-by-example.org.
 - [x] Fuzz tests: 256 runs per test
 - [x] Invariant tests: 64 runs, 2048 calls
 - [x] All contracts have NatSpec documentation
+- [x] Slither findings: enforce ERC20 `transfer` / `transferFrom` return checks in AMM examples
+- [x] Slither findings: enforce ERC20 `transfer` / `transferFrom` return checks in DeFi examples (vault, lockers, staking, crowdfunding, swaps)
 
 ### ✅ Phase 5: Remaining Basic Topics (29/29 COMPLETE)
 - [x] DataLocations - storage, memory, calldata

src/defi/ConstantProductAMM.sol

@@ -37,6 +37,7 @@ contract ConstantProductAMM {
     error InsufficientLiquidity();
     error InsufficientShares();
     error InvalidRatio();
+    error TransferFailed();
 
     constructor(address _token0, address _token1) {
         token0 = IERC20AMM(_token0);
@@ -56,7 +57,7 @@ contract ConstantProductAMM {
             ? (token0, token1, reserve0, reserve1)
             : (token1, token0, reserve1, reserve0);
 
-        _tokenIn.transferFrom(msg.sender, address(this), amountIn);
+        if (!_tokenIn.transferFrom(msg.sender, address(this), amountIn)) revert TransferFailed();
 
         // 0.3% fee: amountInWithFee = amountIn * 997 / 1000
         uint256 amountInWithFee = (amountIn * 997) / 1000;
@@ -66,7 +67,7 @@ contract ConstantProductAMM {
 
         if (amountOut == 0) revert InsufficientLiquidity();
 
-        _tokenOut.transfer(msg.sender, amountOut);
+        if (!_tokenOut.transfer(msg.sender, amountOut)) revert TransferFailed();
 
         _updateReserves();
         emit Swap(msg.sender, tokenIn, amountIn, amountOut);
@@ -84,8 +85,8 @@ contract ConstantProductAMM {
             if (amount0 * reserve1 != amount1 * reserve0) revert InvalidRatio();
         }
 
-        token0.transferFrom(msg.sender, address(this), amount0);
-        token1.transferFrom(msg.sender, address(this), amount1);
+        if (!token0.transferFrom(msg.sender, address(this), amount0)) revert TransferFailed();
+        if (!token1.transferFrom(msg.sender, address(this), amount1)) revert TransferFailed();
 
         if (totalSupply == 0) {
             shares = _sqrt(amount0 * amount1);
@@ -121,8 +122,8 @@ contract ConstantProductAMM {
         balanceOf[msg.sender] -= shares;
         totalSupply -= shares;
 
-        token0.transfer(msg.sender, amount0);
-        token1.transfer(msg.sender, amount1);
+        if (!token0.transfer(msg.sender, amount0)) revert TransferFailed();
+        if (!token1.transfer(msg.sender, amount1)) revert TransferFailed();
 
         _updateReserves();
         emit RemoveLiquidity(msg.sender, shares, amount0, amount1);

src/defi/ConstantSumAMM.sol

@@ -40,6 +40,7 @@ contract ConstantSumAMM {
     error ZeroAmount();
     error InsufficientLiquidity();
     error InsufficientShares();
+    error TransferFailed();
 
     /// @param _token0 Address of the first token
     /// @param _token1 Address of the second token
@@ -64,7 +65,7 @@ contract ConstantSumAMM {
             ? (token0, token1, reserve1)
             : (token1, token0, reserve0);
 
-        _tokenIn.transferFrom(msg.sender, address(this), amountIn);
+        if (!_tokenIn.transferFrom(msg.sender, address(this), amountIn)) revert TransferFailed();
 
         // 0.3% fee: amountOut = amountIn * 997 / 1000
         // Constant sum means 1:1 exchange rate — no price impact
@@ -73,7 +74,7 @@ contract ConstantSumAMM {
         if (amountOut > _resOut) revert InsufficientLiquidity();
         if (amountOut == 0) revert ZeroAmount();
 
-        _tokenOut.transfer(msg.sender, amountOut);
+        if (!_tokenOut.transfer(msg.sender, amountOut)) revert TransferFailed();
 
         _updateReserves();
         emit Swap(msg.sender, tokenIn, amountIn, amountOut);
@@ -89,10 +90,10 @@ contract ConstantSumAMM {
         if (amount0 == 0 && amount1 == 0) revert ZeroAmount();
 
         if (amount0 > 0) {
-            token0.transferFrom(msg.sender, address(this), amount0);
+            if (!token0.transferFrom(msg.sender, address(this), amount0)) revert TransferFailed();
         }
         if (amount1 > 0) {
-            token1.transferFrom(msg.sender, address(this), amount1);
+            if (!token1.transferFrom(msg.sender, address(this), amount1)) revert TransferFailed();
         }
 
         if (totalSupply == 0) {
@@ -129,10 +130,10 @@ contract ConstantSumAMM {
         totalSupply -= shares;
 
         if (amount0 > 0) {
-            token0.transfer(msg.sender, amount0);
+            if (!token0.transfer(msg.sender, amount0)) revert TransferFailed();
         }
         if (amount1 > 0) {
-            token1.transfer(msg.sender, amount1);
+            if (!token1.transfer(msg.sender, amount1)) revert TransferFailed();
         }
 
         _updateReserves();

src/defi/CrowdFund.sol

@@ -48,6 +48,8 @@ contract CrowdFund {
     /// @notice Emitted when goal not met and pledges are refunded
     event Refunded(address pledger, uint256 amount);
 
+    error TransferFailed();
+
     /// @param _token Token address for contributions
     /// @param _creator Campaign creator
     /// @param _goal Funding goal
@@ -74,7 +76,7 @@ contract CrowdFund {
         require(amount > 0, "Cannot pledge 0");
 
         // Transfer tokens from pledger
-        token.transferFrom(msg.sender, address(this), amount);
+        if (!token.transferFrom(msg.sender, address(this), amount)) revert TransferFailed();
 
         // Update state
         pledges[msg.sender] += amount;
@@ -93,7 +95,7 @@ contract CrowdFund {
         totalPledged -= amount;
 
         // Transfer tokens back to pledger
-        token.transfer(msg.sender, amount);
+        if (!token.transfer(msg.sender, amount)) revert TransferFailed();
 
         emit Unpledged(msg.sender, amount);
     }
@@ -108,7 +110,7 @@ contract CrowdFund {
         claimed = true;
 
         // Transfer all pledged tokens to creator
-        token.transfer(creator, totalPledged);
+        if (!token.transfer(creator, totalPledged)) revert TransferFailed();
 
         emit Claimed(creator, totalPledged);
     }
@@ -126,7 +128,7 @@ contract CrowdFund {
         totalPledged -= amount;
 
         // Transfer tokens back
-        token.transfer(msg.sender, amount);
+        if (!token.transfer(msg.sender, amount)) revert TransferFailed();
 
         emit Refunded(msg.sender, amount);
     }

src/defi/DiscreteStakingRewards.sol

@@ -50,6 +50,7 @@ contract DiscreteStakingRewards {
     error InsufficientBalance();
     error NotOwner();
     error NoRewards();
+    error TransferFailed();
 
     /// @param _stakingToken Address of the token users stake
     /// @param _rewardToken Address of the token distributed as rewards
@@ -72,7 +73,7 @@ contract DiscreteStakingRewards {
         // Settle any pending rewards before changing the user's stake
         _updateRewards(msg.sender);
 
-        IERC20Staking(stakingToken).transferFrom(msg.sender, address(this), amount);
+        if (!IERC20Staking(stakingToken).transferFrom(msg.sender, address(this), amount)) revert TransferFailed();
 
         stakedBalance[msg.sender] += amount;
         totalStaked += amount;
@@ -93,7 +94,7 @@ contract DiscreteStakingRewards {
         stakedBalance[msg.sender] -= amount;
         totalStaked -= amount;
 
-        IERC20Staking(stakingToken).transfer(msg.sender, amount);
+        if (!IERC20Staking(stakingToken).transfer(msg.sender, amount)) revert TransferFailed();
 
         emit Withdrawn(msg.sender, amount);
     }
@@ -108,7 +109,7 @@ contract DiscreteStakingRewards {
 
         userRewards[msg.sender] = 0;
 
-        IERC20Staking(rewardToken).transfer(msg.sender, amount);
+        if (!IERC20Staking(rewardToken).transfer(msg.sender, amount)) revert TransferFailed();
 
         emit RewardClaimed(msg.sender, amount);
     }
@@ -129,7 +130,7 @@ contract DiscreteStakingRewards {
         if (amount == 0) revert ZeroAmount();
         if (totalStaked == 0) revert InsufficientBalance(); // No stakers to receive rewards
 
-        IERC20Staking(rewardToken).transferFrom(msg.sender, address(this), amount);
+        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;

src/defi/StableSwapAMM.sol

@@ -52,6 +52,7 @@ contract StableSwapAMM {
     error InsufficientLiquidity();
     error InsufficientShares();
     error ConvergenceFailed();
+    error TransferFailed();
 
     /// @param _token0 Address of the first token
     /// @param _token1 Address of the second token
@@ -77,7 +78,7 @@ contract StableSwapAMM {
             ? (token0, token1, reserve0, reserve1)
             : (token1, token0, reserve1, reserve0);
 
-        _tokenIn.transferFrom(msg.sender, address(this), amountIn);
+        if (!_tokenIn.transferFrom(msg.sender, address(this), amountIn)) revert TransferFailed();
 
         // Apply 0.3% fee to the input amount
         uint256 amountInWithFee = (amountIn * 997) / 1000;
@@ -94,7 +95,7 @@ contract StableSwapAMM {
         amountOut = _resOut - newResOut;
         if (amountOut == 0) revert InsufficientLiquidity();
 
-        _tokenOut.transfer(msg.sender, amountOut);
+        if (!_tokenOut.transfer(msg.sender, amountOut)) revert TransferFailed();
 
         _updateReserves();
         emit Swap(msg.sender, tokenIn, amountIn, amountOut);
@@ -108,10 +109,10 @@ contract StableSwapAMM {
         if (amount0 == 0 && amount1 == 0) revert ZeroAmount();
 
         if (amount0 > 0) {
-            token0.transferFrom(msg.sender, address(this), amount0);
+            if (!token0.transferFrom(msg.sender, address(this), amount0)) revert TransferFailed();
         }
         if (amount1 > 0) {
-            token1.transferFrom(msg.sender, address(this), amount1);
+            if (!token1.transferFrom(msg.sender, address(this), amount1)) revert TransferFailed();
         }
 
         if (totalSupply == 0) {
@@ -150,10 +151,10 @@ contract StableSwapAMM {
         totalSupply -= shares;
 
         if (amount0 > 0) {
-            token0.transfer(msg.sender, amount0);
+            if (!token0.transfer(msg.sender, amount0)) revert TransferFailed();
         }
         if (amount1 > 0) {
-            token1.transfer(msg.sender, amount1);
+            if (!token1.transfer(msg.sender, amount1)) revert TransferFailed();
         }
 
         _updateReserves();

src/defi/StakingRewards.sol

@@ -33,7 +33,7 @@ library SafeERC20 {
     }
 
     function _callOptionalReturn(IERC20Minimal token, bytes memory data) private {
-        (bool success, bytes memory result) = address(token).call(data);
+        (bool success, ) = address(token).call(data);
         require(success, "SafeERC20: call failed");
     }
 }
@@ -162,7 +162,7 @@ contract StakingRewards {
     }
 
     /// @notice Stake with permit
-    function stakeWithPermit(uint256 amount, uint256 deadline, uint8 v, bytes32 r, bytes32 s) external {
+    function stakeWithPermit(uint256 amount, uint256, uint8, bytes32, bytes32) external {
         // Note: In production, implement permit signature
         _stake(msg.sender, amount);
     }
@@ -175,7 +175,7 @@ contract StakingRewards {
         _updateReward(_user);
 
         // Transfer tokens from user
-        stakingToken.transferFrom(_user, address(this), amount);
+        stakingToken.safeTransferFrom(_user, address(this), amount);
 
         // Update state
         _balances[_user] += amount;
@@ -196,7 +196,7 @@ contract StakingRewards {
         _totalSupply -= amount;
 
         // Transfer tokens to user
-        stakingToken.transfer(msg.sender, amount);
+        stakingToken.safeTransfer(msg.sender, amount);
 
         emit Withdrawn(msg.sender, amount);
     }
@@ -208,7 +208,7 @@ contract StakingRewards {
         uint256 reward = rewards[msg.sender];
         if (reward > 0) {
             rewards[msg.sender] = 0;
-            rewardsToken.transfer(msg.sender, reward);
+            rewardsToken.safeTransfer(msg.sender, reward);
             emit RewardPaid(msg.sender, reward);
         }
     }
@@ -247,12 +247,12 @@ contract StakingRewards {
         _balances[msg.sender] = 0;
         _totalSupply = 0;
 
-        stakingToken.transfer(msg.sender, amount);
+        stakingToken.safeTransfer(msg.sender, amount);
 
         uint256 reward = rewards[msg.sender];
         if (reward > 0) {
             rewards[msg.sender] = 0;
-            rewardsToken.transfer(msg.sender, reward);
+            rewardsToken.safeTransfer(msg.sender, reward);
             emit RewardPaid(msg.sender, reward);
         }
 

src/defi/TokenLocker.sol

@@ -41,6 +41,7 @@ contract TokenLocker {
     error NotBeneficiary();
     error NotYetUnlocked();
     error AlreadyWithdrawn();
+    error TransferFailed();
 
     /// @notice Create a new token lock
     /// @dev Transfers tokens from the caller into this contract and records the lock.
@@ -61,7 +62,7 @@ contract TokenLocker {
         if (beneficiary == address(0)) revert ZeroAddress();
         if (unlockTime <= block.timestamp) revert UnlockTimeInPast();
 
-        IERC20Locker(token).transferFrom(msg.sender, address(this), amount);
+        if (!IERC20Locker(token).transferFrom(msg.sender, address(this), amount)) revert TransferFailed();
 
         lockId = locks.length;
         locks.push(Lock({
@@ -87,7 +88,7 @@ contract TokenLocker {
 
         lock.withdrawn = true;
 
-        IERC20Locker(lock.token).transfer(msg.sender, lock.amount);
+        if (!IERC20Locker(lock.token).transfer(msg.sender, lock.amount)) revert TransferFailed();
 
         emit Withdrawn(lockId, msg.sender, lock.amount);
     }

src/defi/UniswapV3Swap.sol

@@ -64,6 +64,8 @@ contract UniswapV3Swap {
     /// @notice Emitted when swap completes
     event SwapCompleted(address tokenIn, address tokenOut, uint256 amountIn, uint256 amountOut);
 
+    error TransferFailed();
+
     /// @param _router Address of Uniswap V3 Router
     constructor(address _router) {
         router = ISwapRouter(_router);
@@ -84,7 +86,7 @@ contract UniswapV3Swap {
         uint256 amountOutMinimum
     ) external returns (uint256 amountOut) {
         // Transfer tokens from caller
-        IERC20(tokenIn).transferFrom(msg.sender, address(this), amountIn);
+        if (!IERC20(tokenIn).transferFrom(msg.sender, address(this), amountIn)) revert TransferFailed();
 
         // Approve router
         IERC20(tokenIn).approve(address(router), amountIn);
@@ -121,7 +123,7 @@ contract UniswapV3Swap {
         uint256 amountInMaximum
     ) external returns (uint256 amountIn) {
         // Transfer max tokens from caller
-        IERC20(tokenIn).transferFrom(msg.sender, address(this), amountInMaximum);
+        if (!IERC20(tokenIn).transferFrom(msg.sender, address(this), amountInMaximum)) revert TransferFailed();
 
         // Approve router
         IERC20(tokenIn).approve(address(router), amountInMaximum);
@@ -142,7 +144,7 @@ contract UniswapV3Swap {
 
         // Refund unused tokens
         if (amountInMaximum > amountIn) {
-            IERC20(tokenIn).transfer(msg.sender, amountInMaximum - amountIn);
+            if (!IERC20(tokenIn).transfer(msg.sender, amountInMaximum - amountIn)) revert TransferFailed();
         }
 
         emit SwapCompleted(tokenIn, tokenOut, amountIn, amountOut);
@@ -160,7 +162,7 @@ contract UniswapV3Swap {
     ) external returns (uint256 amountOut) {
         // Transfer tokens from caller
         address tokenIn = extractTokenInFromPath(path);
-        IERC20(tokenIn).transferFrom(msg.sender, address(this), amountIn);
+        if (!IERC20(tokenIn).transferFrom(msg.sender, address(this), amountIn)) revert TransferFailed();
 
         // Approve router
         IERC20(tokenIn).approve(address(router), amountIn);