fix: safeTransfer check M-03

Author: unknownunknown1

contracts/src/arbitration/KlerosCore.sol

@@ -627,7 +627,7 @@ contract KlerosCore is IArbitratorV2, Initializable, UUPSProxiable {
     function transferBySortitionModule(address _account, uint256 _amount) external {
         if (msg.sender != address(sortitionModule)) revert SortitionModuleOnly();
         // Note eligibility is checked in SortitionModule.
-        pinakion.safeTransfer(_account, _amount);
+        if (!pinakion.safeTransfer(_account, _amount)) revert TransferFailed();
     }
 
     /// @inheritdoc IArbitratorV2

contracts/src/libraries/SafeERC20.sol

@@ -15,6 +15,12 @@ import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
 /// To use this library you can add a `using SafeERC20 for IERC20;` statement to your contract,
 /// which allows you to call the safe operations as `token.safeTransfer(...)`, etc.
 library SafeERC20 {
+    /// @notice Emits when safeTransfer fails.
+    /// @param _token Token to transfer.
+    /// @param _to Recipient address.
+    /// @param _value Amount transferred.
+    event SafeTransferFailed(IERC20 _token, address _to, uint256 _value);
+
     /// @notice Increases the allowance granted to `spender` by the caller.
     /// @param _token Token to transfer.
     /// @param _spender The address which will spend the funds.
@@ -31,7 +37,9 @@ library SafeERC20 {
     /// @return Whether transfer succeeded or not.
     function safeTransfer(IERC20 _token, address _to, uint256 _value) internal returns (bool) {
         (bool success, bytes memory data) = address(_token).call(abi.encodeCall(IERC20.transfer, (_to, _value)));
-        return (success && (data.length == 0 || abi.decode(data, (bool))));
+        bool ok = success && (data.length == 0 || abi.decode(data, (bool)));
+        if (!ok) emit SafeTransferFailed(_token, _to, _value);
+        return ok;
     }
 
     /// @notice Calls transferFrom() without reverting.

contracts/test/foundry/KlerosCore_Execution.t.sol

@@ -2,7 +2,7 @@
 pragma solidity ^0.8.24;
 
 import {KlerosCore_TestBase} from "./KlerosCore_TestBase.sol";
-import {KlerosCore} from "../../src/arbitration/KlerosCore.sol";
+import {KlerosCore, SafeERC20} from "../../src/arbitration/KlerosCore.sol";
 import {SortitionModule} from "../../src/arbitration/SortitionModule.sol";
 import {DisputeKitClassicBase} from "../../src/arbitration/dispute-kits/DisputeKitClassicBase.sol";
 import {IArbitratorV2, IArbitrableV2} from "../../src/arbitration/KlerosCore.sol";
@@ -487,6 +487,15 @@ contract KlerosCore_ExecutionTest is KlerosCore_TestBase {
         assertEq(pinakion.balanceOf(address(core)), 1000, "Wrong token balance of the core");
         assertEq(sortitionModule.totalStaked(), 1000, "Wrong totalStaked before withdrawal");
 
+        vm.prank(address(core));
+        pinakion.transfer(staker2, 1000); // Manually send the balance to trigger the revert
+
+        vm.expectRevert(KlerosCore.TransferFailed.selector);
+        sortitionModule.withdrawLeftoverPNK(staker1);
+
+        vm.prank(staker2);
+        pinakion.transfer(address(core), 1000); // Transfer the tokens back to execute withdrawal
+
         vm.expectEmit(true, true, true, true);
         emit SortitionModule.LeftoverPNKWithdrawn(staker1, 1000);
         sortitionModule.withdrawLeftoverPNK(staker1);
@@ -534,7 +543,7 @@ contract KlerosCore_ExecutionTest is KlerosCore_TestBase {
         voteIDs[1] = 1;
         voteIDs[2] = 2;
         vm.prank(staker1);
-        disputeKit.castVote(disputeID, voteIDs, 1, 0, "XYZ"); // Staker1 only got 1 vote because of low stake
+        disputeKit.castVote(disputeID, voteIDs, 1, 0, "XYZ");
 
         core.passPeriod(disputeID); // Appeal
 
@@ -554,6 +563,59 @@ contract KlerosCore_ExecutionTest is KlerosCore_TestBase {
         assertEq(feeToken.balanceOf(disputer), 0.82 ether, "Wrong fee token balance of disputer");
     }
 
+    function test_execute_feeToken_failedTransfer() public {
+        uint256 disputeID = 0;
+
+        feeToken.transfer(disputer, 1 ether);
+        vm.prank(disputer);
+        feeToken.approve(address(arbitrable), 1 ether);
+
+        vm.prank(owner);
+        core.changeAcceptedFeeTokens(feeToken, true);
+        vm.prank(owner);
+        core.changeCurrencyRates(feeToken, 500, 3);
+
+        vm.prank(disputer);
+        arbitrable.createDispute("Action", 0.18 ether);
+
+        vm.prank(staker1);
+        core.setStake(GENERAL_COURT, 20000);
+        vm.warp(block.timestamp + minStakingTime);
+        sortitionModule.passPhase(); // Generating
+        vm.warp(block.timestamp + rngLookahead);
+        sortitionModule.passPhase(); // Drawing phase
+
+        core.draw(disputeID, DEFAULT_NB_OF_JURORS);
+
+        vm.warp(block.timestamp + timesPerPeriod[0]);
+        core.passPeriod(disputeID); // Vote
+
+        uint256[] memory voteIDs = new uint256[](3);
+        voteIDs[0] = 0;
+        voteIDs[1] = 1;
+        voteIDs[2] = 2;
+        vm.prank(staker1);
+        disputeKit.castVote(disputeID, voteIDs, 1, 0, "XYZ");
+
+        core.passPeriod(disputeID); // Appeal
+
+        vm.warp(block.timestamp + timesPerPeriod[3]);
+        core.passPeriod(disputeID); // Execution
+
+        vm.prank(address(core));
+        feeToken.transfer(disputer, 0.18 ether); // Manually send all balance to make rewards fail
+        assertEq(feeToken.balanceOf(staker1), 0, "Wrong fee token balance of staker1");
+        assertEq(feeToken.balanceOf(disputer), 1 ether, "Wrong fee token balance of disputer");
+
+        vm.expectEmit(true, true, true, true);
+        emit SafeERC20.SafeTransferFailed(feeToken, staker1, 0.06 ether); // One failed iteration has 0.06 eth
+        core.execute(disputeID, 0, 6);
+
+        KlerosCore.Round memory round = core.getRoundInfo(disputeID, 0);
+        assertEq(round.repartitions, 6, "Wrong repartitions");
+        assertEq(feeToken.balanceOf(staker1), 0, "Staker1 still has no balance");
+    }
+
     function test_execute_NoCoherence_feeToken() public {
         uint256 disputeID = 0;