refactor: failed delayed stake handling now relies on a try/catch block rather a returned error enum

Author: jaybuidl

contracts/src/arbitration/KlerosCoreXBase.sol

@@ -465,7 +465,7 @@ abstract contract KlerosCoreXBase is IArbitratorV2, Initializable, UUPSProxiable
     /// @param _newStake The new stake.
     /// Note that the existing delayed stake will be nullified as non-relevant.
     function setStake(uint96 _courtID, uint256 _newStake) external virtual whenNotPaused {
-        _setStake(msg.sender, _courtID, _newStake, OnError.Revert);
+        _setStake(msg.sender, _courtID, _newStake);
     }
 
     /// @notice Executes a stake change initiated by the system (e.g., processing a delayed stake).
@@ -479,8 +479,7 @@ abstract contract KlerosCoreXBase is IArbitratorV2, Initializable, UUPSProxiable
         uint96 _courtID,
         uint256 _newStake
     ) external onlyStakeController returns (bool success) {
-        // TODO: use TRY/CATCH ?? then delete _stakingFailed()
-        return _setStake(_account, _courtID, _newStake, OnError.Return);
+        return _setStake(_account, _courtID, _newStake);
     }
 
     /// @inheritdoc IArbitratorV2
@@ -1078,16 +1077,13 @@ abstract contract KlerosCoreXBase is IArbitratorV2, Initializable, UUPSProxiable
     /// @param _account The account to set the stake for.
     /// @param _courtID The ID of the court to set the stake for.
     /// @param _newStake The new stake.
-    /// @param _onError Whether to revert or return false on error.
-    /// @return Whether the stake was successfully set or not.
-    function _setStake(address _account, uint96 _courtID, uint256 _newStake, OnError _onError) internal returns (bool) {
+    /// @return success Whether the stake was successfully set or not.
+    function _setStake(address _account, uint96 _courtID, uint256 _newStake) internal returns (bool success) {
         if (_courtID == FORKING_COURT || _courtID >= courts.length) {
-            _stakingFailed(_onError, StakingResult.CannotStakeInThisCourt); // Staking directly into the forking court is not allowed.
-            return false;
+            revert StakingNotPossibleInThisCourt();
         }
         if (_newStake != 0 && _newStake < courts[_courtID].minStake) {
-            _stakingFailed(_onError, StakingResult.CannotStakeLessThanMinStake); // Staking less than the minimum stake is not allowed.
-            return false;
+            revert StakingLessThanCourtMinStake();
         }
         (uint256 pnkDeposit, uint256 pnkWithdrawal, StakingResult stakingResult) = stakeController.setStake(
             _account,
@@ -1097,40 +1093,21 @@ abstract contract KlerosCoreXBase is IArbitratorV2, Initializable, UUPSProxiable
         if (stakingResult == StakingResult.Delayed) {
             return true;
         }
-        if (stakingResult != StakingResult.Successful) {
-            _stakingFailed(_onError, stakingResult);
-            return false;
-        }
         if (pnkDeposit > 0) {
             try vault.deposit(_account, pnkDeposit) {
-                // Successfully deposited PNK
+                success = true;
             } catch {
-                // Revert with a specific error or reuse existing one
-                _stakingFailed(_onError, StakingResult.StakingTransferFailed); // Indicating failure in the deposit part of staking
-                return false;
+                success = false;
             }
         }
         if (pnkWithdrawal > 0) {
             try vault.withdraw(_account, pnkWithdrawal) {
-                // Successfully withdrew PNK via Vault
+                success = true;
             } catch {
-                // Revert with a specific error or reuse existing one
-                _stakingFailed(_onError, StakingResult.UnstakingTransferFailed); // Indicating failure in the withdrawal part of unstaking
-                return false;
+                success = false;
             }
         }
-        return true;
-    }
-
-    /// @dev It may revert depending on the _onError parameter.
-    function _stakingFailed(OnError _onError, StakingResult _result) internal pure virtual {
-        if (_onError == OnError.Return) return;
-        if (_result == StakingResult.StakingTransferFailed) revert StakingTransferFailed();
-        if (_result == StakingResult.UnstakingTransferFailed) revert UnstakingTransferFailed();
-        if (_result == StakingResult.CannotStakeInMoreCourts) revert StakingInTooManyCourts();
-        if (_result == StakingResult.CannotStakeInThisCourt) revert StakingNotPossibleInThisCourt();
-        if (_result == StakingResult.CannotStakeLessThanMinStake) revert StakingLessThanCourtMinStake();
-        if (_result == StakingResult.CannotStakeZeroWhenNoStake) revert StakingZeroWhenNoStake();
+        return success;
     }
 
     /// @dev Gets a court ID, the minimum number of jurors and an ID of a dispute kit from a specified extra data bytes array.
@@ -1181,8 +1158,6 @@ abstract contract KlerosCoreXBase is IArbitratorV2, Initializable, UUPSProxiable
     error InvalidForkingCourtAsParent();
     error WrongDisputeKitIndex();
     error CannotDisableClassicDK();
-    error StakingZeroWhenNoStake();
-    error StakingInTooManyCourts();
     error StakingNotPossibleInThisCourt();
     error StakingLessThanCourtMinStake();
     error StakingTransferFailed();

contracts/src/arbitration/KlerosCoreXNeo.sol

@@ -96,17 +96,9 @@ contract KlerosCoreXNeo is KlerosCoreXBase {
         return super._createDispute(_numberOfChoices, _extraData, _feeToken, _feeAmount);
     }
 
-    function _stakingFailed(OnError _onError, StakingResult _result) internal pure override {
-        super._stakingFailed(_onError, _result);
-        if (_result == StakingResult.CannotStakeMoreThanMaxStakePerJuror) revert StakingMoreThanMaxStakePerJuror();
-        if (_result == StakingResult.CannotStakeMoreThanMaxTotalStaked) revert StakingMoreThanMaxTotalStaked();
-    }
-
     // ************************************* //
     // *              Errors               * //
     // ************************************* //
 
-    error StakingMoreThanMaxStakePerJuror();
-    error StakingMoreThanMaxTotalStaked();
     error ArbitrableNotWhitelisted();
 }

contracts/src/arbitration/StakeControllerBase.sol

@@ -188,7 +188,11 @@ abstract contract StakeControllerBase is IStakeController, Initializable, UUPSPr
             if (delayedStake.account == address(0)) continue;
 
             // Let KlerosCore coordinate stake update and vault deposit/withdrawal.
-            core.setStakeByController(delayedStake.account, delayedStake.courtID, delayedStake.stake);
+            try core.setStakeByController(delayedStake.account, delayedStake.courtID, delayedStake.stake) {
+                // NOP
+            } catch (bytes memory data) {
+                emit DelayedStakeSetFailed(data);
+            }
             delete delayedStakes[i];
         }
         delayedStakeReadIndex = newDelayedStakeReadIndex;
@@ -337,12 +341,9 @@ abstract contract StakeControllerBase is IStakeController, Initializable, UUPSPr
 
         if (phase == Phase.staking) {
             if (currentStakeInCourt == 0) {
-                if (_newStake == 0)
-                    // No change needed
-                    return (0, 0, StakingResult.CannotStakeZeroWhenNoStake);
+                if (_newStake == 0) revert StakingZeroWhenNoStake();
                 else if (_newStake > 0 && currentJurorStake.stakedCourtIDs.length >= MAX_STAKE_PATHS)
-                    // Cannot stake in more courts
-                    return (0, 0, StakingResult.CannotStakeInMoreCourts);
+                    revert StakingInTooManyCourts();
             }
 
             currentJurorStake.stakes[_courtID] = _newStake;
@@ -374,7 +375,6 @@ abstract contract StakeControllerBase is IStakeController, Initializable, UUPSPr
             pnkWithdrawal = currentStakeInCourt - _newStake;
         }
 
-        // MAX_STAKE_PATHS is checked by _setStakeBySystem() called during executeDelayedStakes().
         DelayedStake storage delayedStake = delayedStakes[++delayedStakeWriteIndex];
         delayedStake.account = _account;
         delayedStake.courtID = _courtID;
@@ -443,8 +443,7 @@ abstract contract StakeControllerBase is IStakeController, Initializable, UUPSPr
                     continue;
                 }
 
-                // _setStakeBySystem will update local juror stake mappings (jurorStakes)
-                // AND call sortition.setStake.
+                // _setStake will update local juror stake mappings (jurorStakes) AND call sortition.setStake.
                 // The pnkDeposit/pnkWithdrawal are calculated but not used by this import function.
                 (, , StakingResult stakingResult) = _setStake(account, courtID, stakeToImport);
 
@@ -517,6 +516,8 @@ abstract contract StakeControllerBase is IStakeController, Initializable, UUPSPr
     error RandomNumberNotReady();
     error RandomNumberNotReadyYet();
     error StillDrawingDisputes();
+    error StakingZeroWhenNoStake();
+    error StakingInTooManyCourts();
     error NotInStakingPhase();
     error NotDrawingPhase();
     error NoDelayedStakes();

contracts/src/arbitration/interfaces/IStakeController.sol

@@ -36,6 +36,10 @@ interface IStakeController {
     /// @param _totalStake The amount of tokens staked in all courts
     event StakeSet(address indexed _account, uint96 indexed _courtID, uint256 _stakeInCourt, uint256 _totalStake);
 
+    /// @notice Emitted when a delayed stake execution fails
+    /// @param _data The data of the error defined as `abi.encodeWithSelector(CustomError.selector, /*args…*/ )`
+    event DelayedStakeSetFailed(bytes _data);
+
     // Migration events
     event StakeImported(address indexed _juror, uint96 indexed _courtID, uint256 _stake);
     event DelayedStakeImported(address indexed _juror, uint96 indexed _courtID, uint256 _stake, uint256 _index);

contracts/src/libraries/Constants.sol

@@ -20,6 +20,7 @@ uint256 constant DEFAULT_K = 6; // Default number of children per node.
 uint256 constant DEFAULT_NB_OF_JURORS = 3; // The default number of jurors in a dispute.
 IERC20 constant NATIVE_CURRENCY = IERC20(address(0)); // The native currency, such as ETH on Arbitrum, Optimism and Ethereum L1.
 
+// DEPRECATED
 enum OnError {
     Revert,
     Return
@@ -28,12 +29,12 @@ enum OnError {
 enum StakingResult {
     Successful,
     Delayed,
-    StakingTransferFailed,
-    UnstakingTransferFailed,
-    CannotStakeInMoreCourts,
-    CannotStakeInThisCourt,
-    CannotStakeLessThanMinStake,
-    CannotStakeMoreThanMaxStakePerJuror,
-    CannotStakeMoreThanMaxTotalStaked,
-    CannotStakeZeroWhenNoStake
+    StakingTransferFailed, // DEPRECATED
+    UnstakingTransferFailed, // DEPRECATED
+    CannotStakeInMoreCourts, // DEPRECATED
+    CannotStakeInThisCourt, // DEPRECATED
+    CannotStakeLessThanMinStake, // DEPRECATED
+    CannotStakeMoreThanMaxStakePerJuror, // DEPRECATED
+    CannotStakeMoreThanMaxTotalStaked, // DEPRECATED
+    CannotStakeZeroWhenNoStake // DEPRECATED
 }