refactor: replaced Core._setStakeBySystem by ._setStake

Neo NFT checking moved from CoreNeo to VaultNeo

Author: jaybuidl

contracts/src/arbitration/KlerosCoreXBase.sol

@@ -474,12 +474,13 @@ abstract contract KlerosCoreXBase is IArbitratorV2, Initializable, UUPSProxiable
     /// @param _courtID The ID of the court.
     /// @param _newStake The new stake amount for the juror in the court.
     /// @return success Whether the stake was successfully set or not.
-    function setStakeBySystem(
+    function setStakeByController(
         address _account,
         uint96 _courtID,
         uint256 _newStake
     ) external onlyStakeController returns (bool success) {
-        return _setStakeBySystem(_account, _courtID, _newStake);
+        // TODO: use TRY/CATCH ?? then delete _stakingFailed()
+        return _setStake(_account, _courtID, _newStake, OnError.Return);
     }
 
     /// @inheritdoc IArbitratorV2
@@ -1121,47 +1122,6 @@ abstract contract KlerosCoreXBase is IArbitratorV2, Initializable, UUPSProxiable
         return true;
     }
 
-    /// @dev Internal implementation of setStakeBySystem
-    /// @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.
-    /// @return success Whether the stake was successfully set or not.
-    function _setStakeBySystem(
-        address _account,
-        uint96 _courtID,
-        uint256 _newStake
-    ) internal virtual returns (bool success) {
-        (uint256 pnkDeposit, uint256 pnkWithdrawal, StakingResult stakingResult) = stakeController.setStake(
-            _account,
-            _courtID,
-            _newStake
-        );
-        OnError onError = OnError.Return;
-        if (stakingResult != StakingResult.Successful) {
-            _stakingFailed(onError, stakingResult);
-            return false;
-        }
-        if (pnkDeposit > 0) {
-            try vault.deposit(_account, pnkDeposit) {
-                // Successfully deposited PNK
-            } catch {
-                // Revert with a specific error or reuse existing one
-                _stakingFailed(onError, StakingResult.StakingTransferFailed); // Indicating failure in the deposit part of staking
-                return false;
-            }
-        }
-        if (pnkWithdrawal > 0) {
-            try vault.withdraw(_account, pnkWithdrawal) {
-                // Successfully withdrew PNK via Vault
-            } catch {
-                // Revert with a specific error or reuse existing one
-                _stakingFailed(onError, StakingResult.UnstakingTransferFailed); // Indicating failure in the withdrawal part of unstaking
-                return 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;
@@ -1221,6 +1181,7 @@ abstract contract KlerosCoreXBase is IArbitratorV2, Initializable, UUPSProxiable
     error InvalidForkingCourtAsParent();
     error WrongDisputeKitIndex();
     error CannotDisableClassicDK();
+    error StakingZeroWhenNoStake();
     error StakingInTooManyCourts();
     error StakingNotPossibleInThisCourt();
     error StakingLessThanCourtMinStake();
@@ -1244,5 +1205,4 @@ abstract contract KlerosCoreXBase is IArbitratorV2, Initializable, UUPSProxiable
     error TransferFailed();
     error WhenNotPausedOnly();
     error WhenPausedOnly();
-    error StakingZeroWhenNoStake();
 }

contracts/src/arbitration/KlerosCoreXNeo.sol

@@ -6,17 +6,15 @@ import "./KlerosCoreXBase.sol";
 import "@openzeppelin/contracts/token/ERC721/IERC721.sol";
 
 /// @title KlerosCoreXNeo
-/// @notice KlerosCore implementation with new StakeController architecture for production environments
+/// @notice KlerosCore with whitelisted arbitrables
 contract KlerosCoreXNeo is KlerosCoreXBase {
-    /// @notice Version of the implementation contract
     string public constant override version = "0.0.1";
 
     // ************************************* //
     // *             Storage               * //
     // ************************************* //
 
     mapping(address => bool) public arbitrableWhitelist; // Arbitrable whitelist.
-    IERC721 public jurorNft; // Eligible jurors NFT.
 
     // ************************************* //
     // *            Constructor            * //
@@ -37,7 +35,6 @@ contract KlerosCoreXNeo is KlerosCoreXBase {
     /// @param _timesPerPeriod The timesPerPeriod array for courts
     /// @param _sortitionExtraData Extra data for sortition module setup
     /// @param _stakeController The stake controller for coordination
-    /// @param _jurorNft NFT contract to vet the jurors
     function initialize(
         address _governor,
         address _guardian,
@@ -48,8 +45,7 @@ contract KlerosCoreXNeo is KlerosCoreXBase {
         uint256[4] memory _timesPerPeriod,
         bytes memory _sortitionExtraData,
         IStakeController _stakeController,
-        IVault _vault,
-        IERC721 _jurorNft
+        IVault _vault
     ) external reinitializer(2) {
         __KlerosCoreXBase_initialize(
             _governor,
@@ -63,7 +59,6 @@ contract KlerosCoreXNeo is KlerosCoreXBase {
             _stakeController,
             _vault
         );
-        jurorNft = _jurorNft;
     }
 
     function initialize5() external reinitializer(5) {
@@ -77,13 +72,7 @@ contract KlerosCoreXNeo is KlerosCoreXBase {
     /// @notice Access Control to perform implementation upgrades (UUPS Proxiable)
     ///         Only the governor can perform upgrades (`onlyByGovernor`)
     function _authorizeUpgrade(address) internal view override onlyByGovernor {
-        // Empty block: access control implemented by `onlyByGovernor` modifier
-    }
-
-    /// @dev Changes the `jurorNft` storage variable.
-    /// @param _jurorNft The new value for the `jurorNft` storage variable.
-    function changeJurorNft(IERC721 _jurorNft) external onlyByGovernor {
-        jurorNft = _jurorNft;
+        // NOP
     }
 
     /// @dev Adds or removes an arbitrable from whitelist.
@@ -93,20 +82,6 @@ contract KlerosCoreXNeo is KlerosCoreXBase {
         arbitrableWhitelist[_arbitrable] = _allowed;
     }
 
-    // ************************************* //
-    // *         State Modifiers           * //
-    // ************************************* //
-
-    /// @dev Sets the caller's stake in a court.
-    /// Note: Staking and unstaking is forbidden during pause.
-    /// @param _courtID The ID of the court.
-    /// @param _newStake The new stake.
-    /// Note that the existing delayed stake will be nullified as non-relevant.
-    function setStake(uint96 _courtID, uint256 _newStake) external override whenNotPaused {
-        if (jurorNft.balanceOf(msg.sender) == 0) revert NotEligibleForStaking();
-        _setStake(msg.sender, _courtID, _newStake, OnError.Revert);
-    }
-
     // ************************************* //
     // *            Internal               * //
     // ************************************* //
@@ -131,7 +106,6 @@ contract KlerosCoreXNeo is KlerosCoreXBase {
     // *              Errors               * //
     // ************************************* //
 
-    error NotEligibleForStaking();
     error StakingMoreThanMaxStakePerJuror();
     error StakingMoreThanMaxTotalStaked();
     error ArbitrableNotWhitelisted();

contracts/src/arbitration/StakeControllerBase.sol

@@ -188,7 +188,7 @@ abstract contract StakeControllerBase is IStakeController, Initializable, UUPSPr
             if (delayedStake.account == address(0)) continue;
 
             // Let KlerosCore coordinate stake update and vault deposit/withdrawal.
-            core.setStakeBySystem(delayedStake.account, delayedStake.courtID, delayedStake.stake);
+            core.setStakeByController(delayedStake.account, delayedStake.courtID, delayedStake.stake);
             delete delayedStakes[i];
         }
         delayedStakeReadIndex = newDelayedStakeReadIndex;

contracts/src/arbitration/VaultNeo.sol

@@ -103,7 +103,7 @@ contract VaultNeo is VaultBase {
     function _deposit(address _from, uint256 _amount) internal override {
         // Check NFT requirement if set
         if (address(depositNft) != address(0) && depositNft.balanceOf(_from) == 0) {
-            revert DepositNftRequired();
+            revert NotEligible();
         }
 
         // Check per-user deposit limit
@@ -135,7 +135,7 @@ contract VaultNeo is VaultBase {
     // *              Errors               * //
     // ************************************* //
 
-    error DepositNftRequired();
+    error NotEligible();
     error ExceedsMaxDepositPerUser();
     error ExceedsTotalDepositCap();
 }