From 60a156d77149008f61ea519d44510125ea572b9a Mon Sep 17 00:00:00 2001 From: unknownunknown1 Date: Tue, 29 Aug 2023 17:56:39 +1000 Subject: [PATCH 01/10] feat(KC): instant staking --- contracts/src/arbitration/KlerosCore.sol | 94 +++++++++++++++---- contracts/src/arbitration/SortitionModule.sol | 62 ++++++++++-- .../interfaces/ISortitionModule.sol | 9 +- 3 files changed, 136 insertions(+), 29 deletions(-) diff --git a/contracts/src/arbitration/KlerosCore.sol b/contracts/src/arbitration/KlerosCore.sol index 871df03c6..6fca182a2 100644 --- a/contracts/src/arbitration/KlerosCore.sol +++ b/contracts/src/arbitration/KlerosCore.sol @@ -116,6 +116,7 @@ contract KlerosCore is IArbitratorV2, UUPSProxiable, Initializable { event StakeSet(address indexed _address, uint256 _courtID, uint256 _amount); event StakeDelayed(address indexed _address, uint256 _courtID, uint256 _amount); + event StakePartiallyDelayed(address indexed _address, uint256 _courtID, uint256 _amount); event NewPeriod(uint256 indexed _disputeID, Period _period); event AppealPossible(uint256 indexed _disputeID, IArbitrableV2 indexed _arbitrable); event AppealDecision(uint256 indexed _disputeID, IArbitrableV2 indexed _arbitrable); @@ -170,6 +171,7 @@ contract KlerosCore is IArbitratorV2, UUPSProxiable, Initializable { uint256 _feeAmount, IERC20 _feeToken ); + event PartiallyDelayedStakeWithdrawn(uint96 indexed _courtID, address indexed _account, uint256 _withdrawnAmount); // ************************************* // // * Function Modifiers * // @@ -456,13 +458,54 @@ contract KlerosCore is IArbitratorV2, UUPSProxiable, Initializable { /// @dev Sets the caller's stake in a court. /// @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 { - if (!_setStakeForAccount(msg.sender, _courtID, _newStake)) revert StakingFailed(); + removeDelayedStake(_courtID); + if (!_setStakeForAccount(msg.sender, _courtID, _newStake, false)) revert StakingFailed(); } - function setStakeBySortitionModule(address _account, uint96 _courtID, uint256 _newStake) external { + /// @dev Removes the latest delayed stake if there is any. + /// @param _courtID The ID of the court. + function removeDelayedStake(uint96 _courtID) public { + sortitionModule.checkExistingDelayedStake(_courtID, msg.sender); + } + + function withdrawPartiallyDelayedStake(uint96 _courtID, address _juror, uint256 _amountToWithdraw) external { if (msg.sender != address(sortitionModule)) revert WrongCaller(); - _setStakeForAccount(_account, _courtID, _newStake); + uint256 actualAmount = _amountToWithdraw; + Juror storage juror = jurors[_juror]; + if (juror.stakedPnk <= actualAmount) { + actualAmount = juror.stakedPnk; + } + require(pinakion.safeTransfer(_juror, actualAmount)); + // StakePnk can become lower because of penalty, thus we adjust the amount for it. stakedPnkByCourt can't be penalized so subtract the default amount. + juror.stakedPnk -= actualAmount; + juror.stakedPnkByCourt[_courtID] -= _amountToWithdraw; + emit PartiallyDelayedStakeWithdrawn(_courtID, _juror, _amountToWithdraw); + // Note that if we don't delete court here it'll be duplicated after staking. + if (juror.stakedPnkByCourt[_courtID] == 0) { + for (uint256 i = juror.courtIDs.length; i > 0; i--) { + if (juror.courtIDs[i - 1] == _courtID) { + juror.courtIDs[i - 1] = juror.courtIDs[juror.courtIDs.length - 1]; + juror.courtIDs.pop(); + break; + } + } + } + } + + function setStakeBySortitionModule( + address _account, + uint96 _courtID, + uint256 _stake, + bool _alreadyTransferred + ) external { + if (msg.sender != address(sortitionModule)) revert WrongCaller(); + // Always nullify the latest delayed stake before setting a new value. + // Note that we check the delayed stake here too because the check in `setStake` can be bypassed + // if the stake was updated automatically during `execute` (e.g. when unstaking inactive juror). + removeDelayedStake(_courtID); + _setStakeForAccount(_account, _courtID, _newStake, _alreadyTransferred); } /// @inheritdoc IArbitratorV2 @@ -1029,11 +1072,17 @@ contract KlerosCore is IArbitratorV2, UUPSProxiable, Initializable { /// @param _account The address of the juror. /// @param _courtID The ID of the court. /// @param _newStake The new stake. + /// @param _alreadyTransferred True if the tokens were already transferred from juror. Only relevant for delayed stakes. /// @return succeeded True if the call succeeded, false otherwise. function _setStakeForAccount( + address _account, + uint96 _courtID, + uint256 _newStake + , + bool _alreadyTransferred ) internal returns (bool succeeded) { if (_courtID == Constants.FORKING_COURT || _courtID > courts.length) return false; @@ -1056,22 +1105,24 @@ contract KlerosCore is IArbitratorV2, UUPSProxiable, Initializable { uint256 transferredAmount; if (_newStake >= currentStake) { - // Stake increase + if (!_alreadyTransferred) { + // Stake increase // When stakedPnk becomes lower than lockedPnk count the locked tokens in when transferring tokens from juror. - // (E.g. stakedPnk = 0, lockedPnk = 150) which can happen if the juror unstaked fully while having some tokens locked. - uint256 previouslyLocked = (juror.lockedPnk >= juror.stakedPnk) ? juror.lockedPnk - juror.stakedPnk : 0; // underflow guard - transferredAmount = (_newStake >= currentStake + previouslyLocked) // underflow guard - ? _newStake - currentStake - previouslyLocked - : 0; - if (transferredAmount > 0) { - if (!pinakion.safeTransferFrom(_account, address(this), transferredAmount)) { - return false; + // (E.g. stakedPnk = 0, lockedPnk = 150) which can happen if the juror unstaked fully while having some tokens locked. + uint256 previouslyLocked = (juror.lockedPnk >= juror.stakedPnk) ? juror.lockedPnk - juror.stakedPnk : 0; // underflow guard + transferredAmount = (_newStake >= currentStake + previouslyLocked) // underflow guard + ? _newStake - currentStake - previouslyLocked + : 0; + if (transferredAmount > 0) { + // Note we don't return false after incorrect transfer because when stake is increased the transfer is done immediately, thus it can't disrupt delayed stakes' queue. + pinakion.safeTransferFrom(_account, address(this), transferredAmount); + } + if (currentStake == 0) { + juror.courtIDs.push(_courtID); } - } - if (currentStake == 0) { - juror.courtIDs.push(_courtID); } } else { + // Note that stakes can be partially delayed only when stake is increased. // Stake decrease: make sure locked tokens always stay in the contract. They can only be released during Execution. if (juror.stakedPnk >= currentStake - _newStake + juror.lockedPnk) { // We have enough pnk staked to afford withdrawal while keeping locked tokens. @@ -1097,8 +1148,17 @@ contract KlerosCore is IArbitratorV2, UUPSProxiable, Initializable { } // Note that stakedPnk can become async with currentStake (e.g. after penalty). - juror.stakedPnk = (juror.stakedPnk >= currentStake) ? juror.stakedPnk - currentStake + _newStake : _newStake; - juror.stakedPnkByCourt[_courtID] = _newStake; + // Also note that these values were already updated if the stake was only partially delayed. + if (!_alreadyTransferred) { + juror.stakedPnk = (juror.stakedPnk >= currentStake) ? juror.stakedPnk - currentStake + _newStake : _newStake; + juror.stakedPnkByCourt[_courtID] = _newStake; + } + + // Transfer the tokens but don't update sortition module. + if (result == ISortitionModule.preStakeHookResult.partiallyDelayed) { + emit StakePartiallyDelayed(_account, _courtID, _stake); + return true; + } sortitionModule.setStake(_account, _courtID, _newStake); emit StakeSet(_account, _courtID, _newStake); diff --git a/contracts/src/arbitration/SortitionModule.sol b/contracts/src/arbitration/SortitionModule.sol index 67c787aff..55bcdf594 100644 --- a/contracts/src/arbitration/SortitionModule.sol +++ b/contracts/src/arbitration/SortitionModule.sol @@ -39,6 +39,7 @@ contract SortitionModule is ISortitionModule, UUPSProxiable, Initializable { address account; // The address of the juror. uint96 courtID; // The ID of the court. uint256 stake; // The new stake. + bool alreadyTransferred; // True if tokens were already transferred before delayed stake's execution. } // ************************************* // @@ -63,6 +64,7 @@ contract SortitionModule is ISortitionModule, UUPSProxiable, Initializable { uint256 public delayedStakeReadIndex; // The index of the next `delayedStake` item that should be processed. Starts at 1 because 0 index is skipped. mapping(bytes32 => SortitionSumTree) sortitionSumTrees; // The mapping trees by keys. mapping(uint256 => DelayedStake) public delayedStakes; // Stores the stakes that were changed during Drawing phase, to update them when the phase is switched to Staking. + mapping(address => mapping(uint96 => uint256)) public latestDelayedStakeIndex; // Maps the juror to its latest delayed stake. If there is already a delayed stake for this juror then it'll be replaced. latestDelayedStakeIndex[juror][courtID]. // ************************************* // // * Function Modifiers * // @@ -201,12 +203,40 @@ contract SortitionModule is ISortitionModule, UUPSProxiable, Initializable { for (uint256 i = delayedStakeReadIndex; i < newDelayedStakeReadIndex; i++) { DelayedStake storage delayedStake = delayedStakes[i]; - core.setStakeBySortitionModule(delayedStake.account, delayedStake.courtID, delayedStake.stake); - delete delayedStakes[i]; + // Delayed stake could've been manually removed already. In this case simply move on to the next item. + if (delayedStake.account != address(0)) { + core.setStakeBySortitionModule( + delayedStake.account, + delayedStake.courtID, + delayedStake.stake, + delayedStake.alreadyTransferred + ); + delete latestDelayedStakeIndex[delayedStake.account][delayedStake.courtID]; + delete delayedStakes[i]; + } } delayedStakeReadIndex = newDelayedStakeReadIndex; } + /// @dev Checks if there is already a delayed stake. In this case consider it irrelevant and remove it. + /// @param _courtID ID of the court. + /// @param _juror Juror whose stake to check. + function checkExistingDelayedStake(uint96 _courtID, address _juror) external override onlyByCore { + uint256 latestIndex = latestDelayedStakeIndex[_juror][_courtID]; + if (latestIndex != 0) { + DelayedStake storage delayedStake = delayedStakes[latestIndex]; + if (delayedStake.alreadyTransferred) { + bytes32 stakePathID = _accountAndCourtIDToStakePathID(_juror, _courtID); + // Sortition stake represents the stake value that was last updated during Staking phase. + uint256 sortitionStake = stakeOf(bytes32(uint256(_courtID)), stakePathID); + // Withdraw the tokens that were added with the latest delayed stake. + core.withdrawPartiallyDelayedStake(_courtID, _juror, delayedStake.stake - sortitionStake); + } + delete delayedStakes[latestIndex]; + delete latestDelayedStakeIndex[_juror][_courtID]; + } + } + function preStakeHook( address _account, uint96 _courtID, @@ -218,12 +248,18 @@ contract SortitionModule is ISortitionModule, UUPSProxiable, Initializable { return preStakeHookResult.failed; } else { if (phase != Phase.staking) { - delayedStakes[++delayedStakeWriteIndex] = DelayedStake({ - account: _account, - courtID: _courtID, - stake: _stake - }); - return preStakeHookResult.delayed; + DelayedStake storage delayedStake = delayedStakes[++delayedStakeWriteIndex]; + delayedStake.account = _account; + delayedStake.courtID = _courtID; + delayedStake.stake = _stake; + latestDelayedStakeIndex[_account][_courtID] = delayedStakeWriteIndex; + if (_stake > currentStake) { + // Actual token transfer is done right after this hook. + delayedStake.alreadyTransferred = true; + return preStakeHookResult.partiallyDelayed; + } else { + return preStakeHookResult.delayed; + } } } return preStakeHookResult.ok; @@ -273,7 +309,7 @@ contract SortitionModule is ISortitionModule, UUPSProxiable, Initializable { function setJurorInactive(address _account) external override onlyByCore { uint96[] memory courtIDs = core.getJurorCourtIDs(_account); for (uint256 j = courtIDs.length; j > 0; j--) { - core.setStakeBySortitionModule(_account, courtIDs[j - 1], 0); + core.setStakeBySortitionModule(_account, courtIDs[j - 1], 0, false); } } @@ -486,4 +522,12 @@ contract SortitionModule is ISortitionModule, UUPSProxiable, Initializable { stakePathID := mload(ptr) } } + + function stakeOf(bytes32 _key, bytes32 _ID) public view returns (uint256 value) { + SortitionSumTree storage tree = sortitionSumTrees[_key]; + uint treeIndex = tree.IDsToNodeIndexes[_ID]; + + if (treeIndex == 0) value = 0; + else value = tree.nodes[treeIndex]; + } } diff --git a/contracts/src/arbitration/interfaces/ISortitionModule.sol b/contracts/src/arbitration/interfaces/ISortitionModule.sol index 3bee0e270..c9553fb61 100644 --- a/contracts/src/arbitration/interfaces/ISortitionModule.sol +++ b/contracts/src/arbitration/interfaces/ISortitionModule.sol @@ -9,9 +9,10 @@ interface ISortitionModule { } enum preStakeHookResult { - ok, - delayed, - failed + ok, // Correct phase. All checks are passed. + partiallyDelayed, // Wrong phase but stake is increased, so transfer the tokens without updating the drawing chance. + delayed, // Wrong phase and stake is decreased. Delay the token transfer and drawing chance update. + failed // Checks didn't pass. Do no changes. } event NewPhase(Phase _phase); @@ -31,4 +32,6 @@ interface ISortitionModule { function createDisputeHook(uint256 _disputeID, uint256 _roundID) external; function postDrawHook(uint256 _disputeID, uint256 _roundID) external; + + function checkExistingDelayedStake(uint96 _courtID, address _juror) external; } From d0dc7dd83bc08c3d38ee51085dea058e41c79379 Mon Sep 17 00:00:00 2001 From: unknownunknown1 Date: Wed, 13 Sep 2023 05:34:40 +1000 Subject: [PATCH 02/10] fix(KC): compiler error --- contracts/src/arbitration/KlerosCore.sol | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/contracts/src/arbitration/KlerosCore.sol b/contracts/src/arbitration/KlerosCore.sol index 6fca182a2..7a4518e6a 100644 --- a/contracts/src/arbitration/KlerosCore.sol +++ b/contracts/src/arbitration/KlerosCore.sol @@ -497,7 +497,7 @@ contract KlerosCore is IArbitratorV2, UUPSProxiable, Initializable { function setStakeBySortitionModule( address _account, uint96 _courtID, - uint256 _stake, + uint256 _newStake, bool _alreadyTransferred ) external { if (msg.sender != address(sortitionModule)) revert WrongCaller(); @@ -1075,13 +1075,9 @@ contract KlerosCore is IArbitratorV2, UUPSProxiable, Initializable { /// @param _alreadyTransferred True if the tokens were already transferred from juror. Only relevant for delayed stakes. /// @return succeeded True if the call succeeded, false otherwise. function _setStakeForAccount( - address _account, - uint96 _courtID, - - uint256 _newStake - , + uint256 _newStake, bool _alreadyTransferred ) internal returns (bool succeeded) { if (_courtID == Constants.FORKING_COURT || _courtID > courts.length) return false; @@ -1107,7 +1103,7 @@ contract KlerosCore is IArbitratorV2, UUPSProxiable, Initializable { if (_newStake >= currentStake) { if (!_alreadyTransferred) { // Stake increase - // When stakedPnk becomes lower than lockedPnk count the locked tokens in when transferring tokens from juror. + // When stakedPnk becomes lower than lockedPnk count the locked tokens in when transferring tokens from juror. // (E.g. stakedPnk = 0, lockedPnk = 150) which can happen if the juror unstaked fully while having some tokens locked. uint256 previouslyLocked = (juror.lockedPnk >= juror.stakedPnk) ? juror.lockedPnk - juror.stakedPnk : 0; // underflow guard transferredAmount = (_newStake >= currentStake + previouslyLocked) // underflow guard @@ -1150,13 +1146,15 @@ contract KlerosCore is IArbitratorV2, UUPSProxiable, Initializable { // Note that stakedPnk can become async with currentStake (e.g. after penalty). // Also note that these values were already updated if the stake was only partially delayed. if (!_alreadyTransferred) { - juror.stakedPnk = (juror.stakedPnk >= currentStake) ? juror.stakedPnk - currentStake + _newStake : _newStake; + juror.stakedPnk = (juror.stakedPnk >= currentStake) + ? juror.stakedPnk - currentStake + _newStake + : _newStake; juror.stakedPnkByCourt[_courtID] = _newStake; } // Transfer the tokens but don't update sortition module. if (result == ISortitionModule.preStakeHookResult.partiallyDelayed) { - emit StakePartiallyDelayed(_account, _courtID, _stake); + emit StakePartiallyDelayed(_account, _courtID, _newStake); return true; } From 3adabf57144898523732abd015356af197f954b8 Mon Sep 17 00:00:00 2001 From: jaybuidl Date: Fri, 8 Dec 2023 23:41:16 +0000 Subject: [PATCH 03/10] test: delayed stakes --- contracts/.mocharc.json | 4 - contracts/hardhat.config.ts | 3 + contracts/test/arbitration/staking.ts | 236 ++++++++++++++++++++++++++ contracts/test/arbitration/unstake.ts | 84 --------- 4 files changed, 239 insertions(+), 88 deletions(-) delete mode 100644 contracts/.mocharc.json create mode 100644 contracts/test/arbitration/staking.ts delete mode 100644 contracts/test/arbitration/unstake.ts diff --git a/contracts/.mocharc.json b/contracts/.mocharc.json deleted file mode 100644 index c119ef460..000000000 --- a/contracts/.mocharc.json +++ /dev/null @@ -1,4 +0,0 @@ -{ - "require": "ts-node/register/files", - "timeout": 20000 -} diff --git a/contracts/hardhat.config.ts b/contracts/hardhat.config.ts index fb3dd48ce..9ab5e2f03 100644 --- a/contracts/hardhat.config.ts +++ b/contracts/hardhat.config.ts @@ -302,6 +302,9 @@ const config: HardhatUserConfig = { clear: true, runOnCompile: false, }, + mocha: { + timeout: 20000, + }, tenderly: { project: process.env.TENDERLY_PROJECT !== undefined ? process.env.TENDERLY_PROJECT : "kleros-v2", username: process.env.TENDERLY_USERNAME !== undefined ? process.env.TENDERLY_USERNAME : "", diff --git a/contracts/test/arbitration/staking.ts b/contracts/test/arbitration/staking.ts new file mode 100644 index 000000000..bde6b99bb --- /dev/null +++ b/contracts/test/arbitration/staking.ts @@ -0,0 +1,236 @@ +import { ethers, getNamedAccounts, network, deployments } from "hardhat"; +import { BigNumber } from "ethers"; +import { + PNK, + KlerosCore, + DisputeKitClassic, + SortitionModule, + RandomizerRNG, + RandomizerMock, +} from "../../typechain-types"; +import { expect } from "chai"; +import exp from "constants"; + +/* eslint-disable no-unused-vars */ +/* eslint-disable no-unused-expressions */ + +describe("Staking", async () => { + const ONE_TENTH_ETH = BigNumber.from(10).pow(17); + const ONE_THOUSAND_PNK = BigNumber.from(10).pow(21); + + // 2nd court, 3 jurors, 1 dispute kit + const extraData = + "0x000000000000000000000000000000000000000000000000000000000000000200000000000000000000000000000000000000000000000000000000000000030000000000000000000000000000000000000000000000000000000000000001"; + + let deployer; + let disputeKit; + let pnk; + let core; + let sortition; + let rng; + let randomizer; + + const deploy = async () => { + ({ deployer } = await getNamedAccounts()); + await deployments.fixture(["Arbitration"], { + fallbackToGlobal: true, + keepExistingDeployments: false, + }); + disputeKit = (await ethers.getContract("DisputeKitClassic")) as DisputeKitClassic; + pnk = (await ethers.getContract("PNK")) as PNK; + core = (await ethers.getContract("KlerosCore")) as KlerosCore; + sortition = (await ethers.getContract("SortitionModule")) as SortitionModule; + rng = (await ethers.getContract("RandomizerRNG")) as RandomizerRNG; + randomizer = (await ethers.getContract("RandomizerMock")) as RandomizerMock; + }; + + describe("When outside the Staking phase", async () => { + let balanceBefore; + + const reachDrawingPhase = async () => { + expect(await sortition.phase()).to.be.equal(0); // Staking + const arbitrationCost = ONE_TENTH_ETH.mul(3); + + await core.createCourt(1, false, ONE_THOUSAND_PNK, 1000, ONE_TENTH_ETH, 3, [0, 0, 0, 0], 3, [1]); // Parent - general court, Classic dispute kit + + await pnk.approve(core.address, ONE_THOUSAND_PNK.mul(4)); + await core.setStake(1, ONE_THOUSAND_PNK.mul(2)); + await core.setStake(2, ONE_THOUSAND_PNK.mul(2)); + + expect(await core.getJurorCourtIDs(deployer)).to.be.deep.equal([BigNumber.from("1"), BigNumber.from("2")]); + + await core.functions["createDispute(uint256,bytes)"](2, extraData, { value: arbitrationCost }); + + await network.provider.send("evm_increaseTime", [2000]); // Wait for minStakingTime + await network.provider.send("evm_mine"); + + const lookahead = await sortition.rngLookahead(); + await sortition.passPhase(); // Staking -> Generating + for (let index = 0; index < lookahead; index++) { + await network.provider.send("evm_mine"); + } + + balanceBefore = await pnk.balanceOf(deployer); + }; + + describe("When decreasing then increasing back stake", async () => { + before("Setup", async () => { + await deploy(); + await reachDrawingPhase(); + }); + + it("Should be outside the Staking phase", async () => { + expect(await sortition.phase()).to.be.equal(1); // Drawing + expect(await core.getJurorBalance(deployer, 2)).to.be.deep.equal([ + ONE_THOUSAND_PNK.mul(4), + BigNumber.from(0), + ONE_THOUSAND_PNK.mul(2), + BigNumber.from(2), + ]); + }); + + it("Should delay the stake decrease", async () => { + expect(await sortition.delayedStakeWriteIndex()).to.be.equal(0); + expect(await sortition.delayedStakeReadIndex()).to.be.equal(1); + expect(await sortition.latestDelayedStakeIndex(deployer, 2)).to.be.equal(0); + await expect(core.setStake(2, ONE_THOUSAND_PNK.mul(1))) + .to.emit(core, "StakeDelayed") + .withArgs(deployer, 2, ONE_THOUSAND_PNK.mul(1)); + expect(await sortition.latestDelayedStakeIndex(deployer, 2)).to.be.equal(1); + expect(await core.getJurorBalance(deployer, 2)).to.be.deep.equal([ + ONE_THOUSAND_PNK.mul(4), + BigNumber.from(0), + ONE_THOUSAND_PNK.mul(2), + BigNumber.from(2), + ]); // stake unchanged, delayed + expect(await pnk.balanceOf(deployer)).to.be.equal(balanceBefore); // No PNK transfer yet + expect(await sortition.delayedStakeWriteIndex()).to.be.equal(1); + expect(await sortition.delayedStakeReadIndex()).to.be.equal(1); + expect(await sortition.delayedStakes(1)).to.be.deep.equal([deployer, 2, ONE_THOUSAND_PNK.mul(1), false]); + }); + + it("Should delay the stake increase back to the previous amount", async () => { + balanceBefore = await pnk.balanceOf(deployer); + expect(await sortition.latestDelayedStakeIndex(deployer, 2)).to.be.equal(1); + await expect(core.setStake(2, ONE_THOUSAND_PNK.mul(2))) + .to.emit(core, "StakeDelayed") + .withArgs(deployer, 2, ONE_THOUSAND_PNK.mul(2)); + expect(await sortition.latestDelayedStakeIndex(deployer, 2)).to.be.equal(2); + expect(await core.getJurorBalance(deployer, 2)).to.be.deep.equal([ + ONE_THOUSAND_PNK.mul(4), + BigNumber.from(0), + ONE_THOUSAND_PNK.mul(2), + BigNumber.from(2), + ]); // stake unchanged, delayed + expect(await pnk.balanceOf(deployer)).to.be.equal(balanceBefore); // No PNK transfer yet + expect(await sortition.delayedStakeWriteIndex()).to.be.equal(2); + expect(await sortition.delayedStakeReadIndex()).to.be.equal(1); + expect(await sortition.delayedStakes(1)).to.be.deep.equal([ethers.constants.AddressZero, 0, 0, false]); // the 1st delayed stake got deleted + expect(await sortition.delayedStakes(2)).to.be.deep.equal([deployer, 2, ONE_THOUSAND_PNK.mul(2), false]); + }); + }); + + describe("When increasing then decreasing back stake", async () => { + before("Setup", async () => { + await deploy(); + await reachDrawingPhase(); + }); + + it("Should be outside the Staking phase", async () => { + expect(await sortition.phase()).to.be.equal(1); // Drawing + expect(await core.getJurorBalance(deployer, 2)).to.be.deep.equal([ + ONE_THOUSAND_PNK.mul(4), + BigNumber.from(0), + ONE_THOUSAND_PNK.mul(2), + BigNumber.from(2), + ]); + }); + + it("Should transfer PNK but delay the stake increase", async () => { + expect(await sortition.delayedStakeWriteIndex()).to.be.equal(0); + expect(await sortition.delayedStakeReadIndex()).to.be.equal(1); + await pnk.approve(core.address, ONE_THOUSAND_PNK.mul(1)); + expect(await sortition.latestDelayedStakeIndex(deployer, 2)).to.be.equal(0); + await expect(core.setStake(2, ONE_THOUSAND_PNK.mul(3))) + .to.emit(core, "StakePartiallyDelayed") + .withArgs(deployer, 2, ONE_THOUSAND_PNK.mul(3)); + expect(await sortition.latestDelayedStakeIndex(deployer, 2)).to.be.equal(1); + expect(await core.getJurorBalance(deployer, 2)).to.be.deep.equal([ + ONE_THOUSAND_PNK.mul(5), + BigNumber.from(0), + ONE_THOUSAND_PNK.mul(3), + BigNumber.from(2), + ]); // stake has changed immediately, WARNING: this is misleading because it's not actually added to the SortitionSumTree + expect(await pnk.balanceOf(deployer)).to.be.equal(balanceBefore.sub(ONE_THOUSAND_PNK)); // PNK is transferred out of the juror's account + expect(await sortition.delayedStakeWriteIndex()).to.be.equal(1); + expect(await sortition.delayedStakeReadIndex()).to.be.equal(1); + expect(await sortition.delayedStakes(1)).to.be.deep.equal([deployer, 2, ONE_THOUSAND_PNK.mul(3), true]); + }); + + it("Should cancel out the stake decrease back", async () => { + balanceBefore = await pnk.balanceOf(deployer); + expect(await sortition.latestDelayedStakeIndex(deployer, 2)).to.be.equal(1); + await expect(core.setStake(2, ONE_THOUSAND_PNK.mul(2))) + .to.emit(core, "StakeDelayed") + .withArgs(deployer, 2, ONE_THOUSAND_PNK.mul(2)); + expect(await sortition.latestDelayedStakeIndex(deployer, 2)).to.be.equal(2); + expect(await core.getJurorBalance(deployer, 2)).to.be.deep.equal([ + ONE_THOUSAND_PNK.mul(4), + BigNumber.from(0), + ONE_THOUSAND_PNK.mul(2), + BigNumber.from(2), + ]); // stake has changed immediately + expect(await pnk.balanceOf(deployer)).to.be.equal(balanceBefore.add(ONE_THOUSAND_PNK)); // PNK is sent back to the juror + expect(await sortition.delayedStakeWriteIndex()).to.be.equal(2); + expect(await sortition.delayedStakeReadIndex()).to.be.equal(1); + expect(await sortition.delayedStakes(1)).to.be.deep.equal([ethers.constants.AddressZero, 0, 0, false]); // the 1st delayed stake got deleted + expect(await sortition.delayedStakes(2)).to.be.deep.equal([deployer, 2, ONE_THOUSAND_PNK.mul(2), false]); + }); + }); + }); + + describe("When a juror is inactive", async () => { + before("Setup", async () => { + await deploy(); + }); + + it("Should unstake from all courts", async () => { + const arbitrationCost = ONE_TENTH_ETH.mul(3); + + await core.createCourt(1, false, ONE_THOUSAND_PNK, 1000, ONE_TENTH_ETH, 3, [0, 0, 0, 0], 3, [1]); // Parent - general court, Classic dispute kit + + await pnk.approve(core.address, ONE_THOUSAND_PNK.mul(4)); + await core.setStake(1, ONE_THOUSAND_PNK.mul(2)); + await core.setStake(2, ONE_THOUSAND_PNK.mul(2)); + + expect(await core.getJurorCourtIDs(deployer)).to.be.deep.equal([BigNumber.from("1"), BigNumber.from("2")]); + + await core.functions["createDispute(uint256,bytes)"](2, extraData, { value: arbitrationCost }); + + await network.provider.send("evm_increaseTime", [2000]); // Wait for minStakingTime + await network.provider.send("evm_mine"); + + const lookahead = await sortition.rngLookahead(); + await sortition.passPhase(); // Staking -> Generating + for (let index = 0; index < lookahead; index++) { + await network.provider.send("evm_mine"); + } + await randomizer.relay(rng.address, 0, ethers.utils.randomBytes(32)); + await sortition.passPhase(); // Generating -> Drawing + + await core.draw(0, 5000); + + await core.passPeriod(0); // Evidence -> Voting + await core.passPeriod(0); // Voting -> Appeal + await core.passPeriod(0); // Appeal -> Execution + + await sortition.passPhase(); // Freezing -> Staking. Change so we don't deal with delayed stakes + + expect(await core.getJurorCourtIDs(deployer)).to.be.deep.equal([BigNumber.from("1"), BigNumber.from("2")]); + + await core.execute(0, 0, 1); // 1 iteration should unstake from both courts + + expect(await core.getJurorCourtIDs(deployer)).to.be.deep.equal([]); + }); + }); +}); diff --git a/contracts/test/arbitration/unstake.ts b/contracts/test/arbitration/unstake.ts deleted file mode 100644 index 671fe3c80..000000000 --- a/contracts/test/arbitration/unstake.ts +++ /dev/null @@ -1,84 +0,0 @@ -import { ethers, getNamedAccounts, network, deployments } from "hardhat"; -import { BigNumber } from "ethers"; -import { - PNK, - KlerosCore, - DisputeKitClassic, - SortitionModule, - RandomizerRNG, - RandomizerMock, -} from "../../typechain-types"; -import { expect } from "chai"; - -/* eslint-disable no-unused-vars */ -/* eslint-disable no-unused-expressions */ - -describe("Unstake juror", async () => { - const ONE_TENTH_ETH = BigNumber.from(10).pow(17); - const ONE_THOUSAND_PNK = BigNumber.from(10).pow(21); - - // 2nd court, 3 jurors, 1 dispute kit - const extraData = - "0x000000000000000000000000000000000000000000000000000000000000000200000000000000000000000000000000000000000000000000000000000000030000000000000000000000000000000000000000000000000000000000000001"; - - let deployer; - let disputeKit; - let pnk; - let core; - let sortitionModule; - let rng; - let randomizer; - - beforeEach("Setup", async () => { - ({ deployer } = await getNamedAccounts()); - await deployments.fixture(["Arbitration"], { - fallbackToGlobal: true, - keepExistingDeployments: false, - }); - disputeKit = (await ethers.getContract("DisputeKitClassic")) as DisputeKitClassic; - pnk = (await ethers.getContract("PNK")) as PNK; - core = (await ethers.getContract("KlerosCore")) as KlerosCore; - sortitionModule = (await ethers.getContract("SortitionModule")) as SortitionModule; - rng = (await ethers.getContract("RandomizerRNG")) as RandomizerRNG; - randomizer = (await ethers.getContract("RandomizerMock")) as RandomizerMock; - }); - - it("Unstake inactive juror", async () => { - const arbitrationCost = ONE_TENTH_ETH.mul(3); - - await core.createCourt(1, false, ONE_THOUSAND_PNK, 1000, ONE_TENTH_ETH, 3, [0, 0, 0, 0], 3, [1]); // Parent - general court, Classic dispute kit - - await pnk.approve(core.address, ONE_THOUSAND_PNK.mul(4)); - await core.setStake(1, ONE_THOUSAND_PNK.mul(2)); - await core.setStake(2, ONE_THOUSAND_PNK.mul(2)); - - expect(await core.getJurorCourtIDs(deployer)).to.be.deep.equal([BigNumber.from("1"), BigNumber.from("2")]); - - await core.functions["createDispute(uint256,bytes)"](2, extraData, { value: arbitrationCost }); - - await network.provider.send("evm_increaseTime", [2000]); // Wait for minStakingTime - await network.provider.send("evm_mine"); - - const lookahead = await sortitionModule.rngLookahead(); - await sortitionModule.passPhase(); // Staking -> Generating - for (let index = 0; index < lookahead; index++) { - await network.provider.send("evm_mine"); - } - await randomizer.relay(rng.address, 0, ethers.utils.randomBytes(32)); - await sortitionModule.passPhase(); // Generating -> Drawing - - await core.draw(0, 5000); - - await core.passPeriod(0); // Evidence -> Voting - await core.passPeriod(0); // Voting -> Appeal - await core.passPeriod(0); // Appeal -> Execution - - await sortitionModule.passPhase(); // Freezing -> Staking. Change so we don't deal with delayed stakes - - expect(await core.getJurorCourtIDs(deployer)).to.be.deep.equal([BigNumber.from("1"), BigNumber.from("2")]); - - await core.execute(0, 0, 1); // 1 iteration should unstake from both courts - - expect(await core.getJurorCourtIDs(deployer)).to.be.deep.equal([]); - }); -}); From 03ad590cce924ac0c07012366b8597f4d7a41785 Mon Sep 17 00:00:00 2001 From: jaybuidl Date: Sat, 9 Dec 2023 00:09:12 +0000 Subject: [PATCH 04/10] refactor: naming things --- contracts/src/arbitration/KlerosCore.sol | 68 ++++++++++--------- contracts/src/arbitration/SortitionModule.sol | 37 ++++++---- .../interfaces/ISortitionModule.sol | 10 +-- contracts/test/arbitration/staking.ts | 8 +-- subgraph/src/KlerosCore.ts | 4 +- subgraph/subgraph.yaml | 4 +- 6 files changed, 71 insertions(+), 60 deletions(-) diff --git a/contracts/src/arbitration/KlerosCore.sol b/contracts/src/arbitration/KlerosCore.sol index 7a4518e6a..13a1e9104 100644 --- a/contracts/src/arbitration/KlerosCore.sol +++ b/contracts/src/arbitration/KlerosCore.sol @@ -115,8 +115,13 @@ contract KlerosCore is IArbitratorV2, UUPSProxiable, Initializable { // ************************************* // event StakeSet(address indexed _address, uint256 _courtID, uint256 _amount); - event StakeDelayed(address indexed _address, uint256 _courtID, uint256 _amount); - event StakePartiallyDelayed(address indexed _address, uint256 _courtID, uint256 _amount); + event StakeDelayedNotTransferred(address indexed _address, uint256 _courtID, uint256 _amount); + event StakeDelayedAlreadyTransferred(address indexed _address, uint256 _courtID, uint256 _amount); + event StakeDelayedAlreadyTransferredWithdrawn( + uint96 indexed _courtID, + address indexed _account, + uint256 _withdrawnAmount + ); event NewPeriod(uint256 indexed _disputeID, Period _period); event AppealPossible(uint256 indexed _disputeID, IArbitrableV2 indexed _arbitrable); event AppealDecision(uint256 indexed _disputeID, IArbitrableV2 indexed _arbitrable); @@ -171,7 +176,6 @@ contract KlerosCore is IArbitratorV2, UUPSProxiable, Initializable { uint256 _feeAmount, IERC20 _feeToken ); - event PartiallyDelayedStakeWithdrawn(uint96 indexed _courtID, address indexed _account, uint256 _withdrawnAmount); // ************************************* // // * Function Modifiers * // @@ -460,18 +464,26 @@ contract KlerosCore is IArbitratorV2, UUPSProxiable, Initializable { /// @param _newStake The new stake. /// Note that the existing delayed stake will be nullified as non-relevant. function setStake(uint96 _courtID, uint256 _newStake) external { - removeDelayedStake(_courtID); + _deleteDelayedStake(_courtID); if (!_setStakeForAccount(msg.sender, _courtID, _newStake, false)) revert StakingFailed(); } - /// @dev Removes the latest delayed stake if there is any. - /// @param _courtID The ID of the court. - function removeDelayedStake(uint96 _courtID) public { - sortitionModule.checkExistingDelayedStake(_courtID, msg.sender); + function setStakeBySortitionModule( + address _account, + uint96 _courtID, + uint256 _newStake, + bool _alreadyTransferred + ) external { + if (msg.sender != address(sortitionModule)) revert SortitionModuleOnly(); + // Always nullify the latest delayed stake before setting a new value. + // Note that we check the delayed stake here too because the check in `setStake` can be bypassed + // if the stake was updated automatically during `execute` (e.g. when unstaking inactive juror). + _deleteDelayedStake(_courtID); + _setStakeForAccount(_account, _courtID, _newStake, _alreadyTransferred); } function withdrawPartiallyDelayedStake(uint96 _courtID, address _juror, uint256 _amountToWithdraw) external { - if (msg.sender != address(sortitionModule)) revert WrongCaller(); + if (msg.sender != address(sortitionModule)) revert SortitionModuleOnly(); uint256 actualAmount = _amountToWithdraw; Juror storage juror = jurors[_juror]; if (juror.stakedPnk <= actualAmount) { @@ -481,7 +493,7 @@ contract KlerosCore is IArbitratorV2, UUPSProxiable, Initializable { // StakePnk can become lower because of penalty, thus we adjust the amount for it. stakedPnkByCourt can't be penalized so subtract the default amount. juror.stakedPnk -= actualAmount; juror.stakedPnkByCourt[_courtID] -= _amountToWithdraw; - emit PartiallyDelayedStakeWithdrawn(_courtID, _juror, _amountToWithdraw); + emit StakeDelayedAlreadyTransferredWithdrawn(_courtID, _juror, _amountToWithdraw); // Note that if we don't delete court here it'll be duplicated after staking. if (juror.stakedPnkByCourt[_courtID] == 0) { for (uint256 i = juror.courtIDs.length; i > 0; i--) { @@ -494,20 +506,6 @@ contract KlerosCore is IArbitratorV2, UUPSProxiable, Initializable { } } - function setStakeBySortitionModule( - address _account, - uint96 _courtID, - uint256 _newStake, - bool _alreadyTransferred - ) external { - if (msg.sender != address(sortitionModule)) revert WrongCaller(); - // Always nullify the latest delayed stake before setting a new value. - // Note that we check the delayed stake here too because the check in `setStake` can be bypassed - // if the stake was updated automatically during `execute` (e.g. when unstaking inactive juror). - removeDelayedStake(_courtID); - _setStakeForAccount(_account, _courtID, _newStake, _alreadyTransferred); - } - /// @inheritdoc IArbitratorV2 function createDispute( uint256 _numberOfChoices, @@ -1063,6 +1061,12 @@ contract KlerosCore is IArbitratorV2, UUPSProxiable, Initializable { emit DisputeKitEnabled(_courtID, _disputeKitID, _enable); } + /// @dev Removes the latest delayed stake if there is any. + /// @param _courtID The ID of the court. + function _deleteDelayedStake(uint96 _courtID) private { + sortitionModule.deleteDelayedStake(_courtID, msg.sender); + } + /// @dev Sets the specified juror's stake in a court. /// `O(n + p * log_k(j))` where /// `n` is the number of courts the juror has staked in, @@ -1091,11 +1095,11 @@ contract KlerosCore is IArbitratorV2, UUPSProxiable, Initializable { return false; } - ISortitionModule.preStakeHookResult result = sortitionModule.preStakeHook(_account, _courtID, _newStake); - if (result == ISortitionModule.preStakeHookResult.failed) { + ISortitionModule.PreStakeHookResult result = sortitionModule.preStakeHook(_account, _courtID, _newStake); + if (result == ISortitionModule.PreStakeHookResult.failed) { return false; - } else if (result == ISortitionModule.preStakeHookResult.delayed) { - emit StakeDelayed(_account, _courtID, _newStake); + } else if (result == ISortitionModule.PreStakeHookResult.stakeDelayedNotTransferred) { + emit StakeDelayedNotTransferred(_account, _courtID, _newStake); return true; } @@ -1153,8 +1157,8 @@ contract KlerosCore is IArbitratorV2, UUPSProxiable, Initializable { } // Transfer the tokens but don't update sortition module. - if (result == ISortitionModule.preStakeHookResult.partiallyDelayed) { - emit StakePartiallyDelayed(_account, _courtID, _newStake); + if (result == ISortitionModule.PreStakeHookResult.stakeDelayedAlreadyTransferred) { + emit StakeDelayedAlreadyTransferred(_account, _courtID, _newStake); return true; } @@ -1201,6 +1205,8 @@ contract KlerosCore is IArbitratorV2, UUPSProxiable, Initializable { // ************************************* // error GovernorOnly(); + error DisputeKitOnly(); + error SortitionModuleOnly(); error UnsuccessfulCall(); error InvalidDisputKitParent(); error DepthLevelMax(); @@ -1211,7 +1217,6 @@ contract KlerosCore is IArbitratorV2, UUPSProxiable, Initializable { error CannotDisableClassicDK(); error ArraysLengthMismatch(); error StakingFailed(); - error WrongCaller(); error ArbitrationFeesNotEnough(); error DisputeKitNotSupportedByCourt(); error MustSupportDisputeKitClassic(); @@ -1224,7 +1229,6 @@ contract KlerosCore is IArbitratorV2, UUPSProxiable, Initializable { error NotEvidencePeriod(); error AppealFeesNotEnough(); error DisputeNotAppealable(); - error DisputeKitOnly(); error NotExecutionPeriod(); error RulingAlreadyExecuted(); error DisputePeriodIsFinal(); diff --git a/contracts/src/arbitration/SortitionModule.sol b/contracts/src/arbitration/SortitionModule.sol index 55bcdf594..ea50851aa 100644 --- a/contracts/src/arbitration/SortitionModule.sol +++ b/contracts/src/arbitration/SortitionModule.sol @@ -221,7 +221,7 @@ contract SortitionModule is ISortitionModule, UUPSProxiable, Initializable { /// @dev Checks if there is already a delayed stake. In this case consider it irrelevant and remove it. /// @param _courtID ID of the court. /// @param _juror Juror whose stake to check. - function checkExistingDelayedStake(uint96 _courtID, address _juror) external override onlyByCore { + function deleteDelayedStake(uint96 _courtID, address _juror) external override onlyByCore { uint256 latestIndex = latestDelayedStakeIndex[_juror][_courtID]; if (latestIndex != 0) { DelayedStake storage delayedStake = delayedStakes[latestIndex]; @@ -241,28 +241,31 @@ contract SortitionModule is ISortitionModule, UUPSProxiable, Initializable { address _account, uint96 _courtID, uint256 _stake - ) external override onlyByCore returns (preStakeHookResult) { + ) external override onlyByCore returns (PreStakeHookResult) { (, , uint256 currentStake, uint256 nbCourts) = core.getJurorBalance(_account, _courtID); if (currentStake == 0 && nbCourts >= MAX_STAKE_PATHS) { // Prevent staking beyond MAX_STAKE_PATHS but unstaking is always allowed. - return preStakeHookResult.failed; + return PreStakeHookResult.failed; } else { if (phase != Phase.staking) { + // Store the stake change as delayed, to be applied when the phase switches back to Staking. DelayedStake storage delayedStake = delayedStakes[++delayedStakeWriteIndex]; delayedStake.account = _account; delayedStake.courtID = _courtID; delayedStake.stake = _stake; latestDelayedStakeIndex[_account][_courtID] = delayedStakeWriteIndex; if (_stake > currentStake) { - // Actual token transfer is done right after this hook. + // PNK deposit: tokens to be transferred now (right after this hook), + // but the stake will be added to the sortition sum tree later. delayedStake.alreadyTransferred = true; - return preStakeHookResult.partiallyDelayed; + return PreStakeHookResult.stakeDelayedAlreadyTransferred; } else { - return preStakeHookResult.delayed; + // PNK withdrawal: tokens are not transferred yet. + return PreStakeHookResult.stakeDelayedNotTransferred; } } } - return preStakeHookResult.ok; + return PreStakeHookResult.ok; } function createDisputeHook(uint256 /*_disputeID*/, uint256 /*_roundID*/) external override onlyByCore { @@ -362,6 +365,18 @@ contract SortitionModule is ISortitionModule, UUPSProxiable, Initializable { drawnAddress = _stakePathIDToAccount(tree.nodeIndexesToIDs[treeIndex]); } + /// @dev Get the stake of a juror in a court. + /// @param _key The key of the tree, corresponding to a court. + /// @param _ID The stake path ID, corresponding to a juror. + /// @return value The stake of the juror in the court. + function stakeOf(bytes32 _key, bytes32 _ID) public view returns (uint256 value) { + SortitionSumTree storage tree = sortitionSumTrees[_key]; + uint treeIndex = tree.IDsToNodeIndexes[_ID]; + + if (treeIndex == 0) value = 0; + else value = tree.nodes[treeIndex]; + } + // ************************************* // // * Internal * // // ************************************* // @@ -522,12 +537,4 @@ contract SortitionModule is ISortitionModule, UUPSProxiable, Initializable { stakePathID := mload(ptr) } } - - function stakeOf(bytes32 _key, bytes32 _ID) public view returns (uint256 value) { - SortitionSumTree storage tree = sortitionSumTrees[_key]; - uint treeIndex = tree.IDsToNodeIndexes[_ID]; - - if (treeIndex == 0) value = 0; - else value = tree.nodes[treeIndex]; - } } diff --git a/contracts/src/arbitration/interfaces/ISortitionModule.sol b/contracts/src/arbitration/interfaces/ISortitionModule.sol index c9553fb61..42eb2a4c0 100644 --- a/contracts/src/arbitration/interfaces/ISortitionModule.sol +++ b/contracts/src/arbitration/interfaces/ISortitionModule.sol @@ -8,10 +8,10 @@ interface ISortitionModule { drawing // Jurors can be drawn. Pass after all disputes have jurors or `maxDrawingTime` passes. } - enum preStakeHookResult { + enum PreStakeHookResult { ok, // Correct phase. All checks are passed. - partiallyDelayed, // Wrong phase but stake is increased, so transfer the tokens without updating the drawing chance. - delayed, // Wrong phase and stake is decreased. Delay the token transfer and drawing chance update. + stakeDelayedAlreadyTransferred, // Wrong phase but stake is increased, so transfer the tokens without updating the drawing chance. + stakeDelayedNotTransferred, // Wrong phase and stake is decreased. Delay the token transfer and drawing chance update. failed // Checks didn't pass. Do no changes. } @@ -27,11 +27,11 @@ interface ISortitionModule { function draw(bytes32 _court, uint256 _coreDisputeID, uint256 _nonce) external view returns (address); - function preStakeHook(address _account, uint96 _courtID, uint256 _stake) external returns (preStakeHookResult); + function preStakeHook(address _account, uint96 _courtID, uint256 _stake) external returns (PreStakeHookResult); function createDisputeHook(uint256 _disputeID, uint256 _roundID) external; function postDrawHook(uint256 _disputeID, uint256 _roundID) external; - function checkExistingDelayedStake(uint96 _courtID, address _juror) external; + function deleteDelayedStake(uint96 _courtID, address _juror) external; } diff --git a/contracts/test/arbitration/staking.ts b/contracts/test/arbitration/staking.ts index bde6b99bb..e761779a6 100644 --- a/contracts/test/arbitration/staking.ts +++ b/contracts/test/arbitration/staking.ts @@ -94,7 +94,7 @@ describe("Staking", async () => { expect(await sortition.delayedStakeReadIndex()).to.be.equal(1); expect(await sortition.latestDelayedStakeIndex(deployer, 2)).to.be.equal(0); await expect(core.setStake(2, ONE_THOUSAND_PNK.mul(1))) - .to.emit(core, "StakeDelayed") + .to.emit(core, "StakeDelayedNotTransferred") .withArgs(deployer, 2, ONE_THOUSAND_PNK.mul(1)); expect(await sortition.latestDelayedStakeIndex(deployer, 2)).to.be.equal(1); expect(await core.getJurorBalance(deployer, 2)).to.be.deep.equal([ @@ -113,7 +113,7 @@ describe("Staking", async () => { balanceBefore = await pnk.balanceOf(deployer); expect(await sortition.latestDelayedStakeIndex(deployer, 2)).to.be.equal(1); await expect(core.setStake(2, ONE_THOUSAND_PNK.mul(2))) - .to.emit(core, "StakeDelayed") + .to.emit(core, "StakeDelayedNotTransferred") .withArgs(deployer, 2, ONE_THOUSAND_PNK.mul(2)); expect(await sortition.latestDelayedStakeIndex(deployer, 2)).to.be.equal(2); expect(await core.getJurorBalance(deployer, 2)).to.be.deep.equal([ @@ -152,7 +152,7 @@ describe("Staking", async () => { await pnk.approve(core.address, ONE_THOUSAND_PNK.mul(1)); expect(await sortition.latestDelayedStakeIndex(deployer, 2)).to.be.equal(0); await expect(core.setStake(2, ONE_THOUSAND_PNK.mul(3))) - .to.emit(core, "StakePartiallyDelayed") + .to.emit(core, "StakeDelayedAlreadyTransferred") .withArgs(deployer, 2, ONE_THOUSAND_PNK.mul(3)); expect(await sortition.latestDelayedStakeIndex(deployer, 2)).to.be.equal(1); expect(await core.getJurorBalance(deployer, 2)).to.be.deep.equal([ @@ -171,7 +171,7 @@ describe("Staking", async () => { balanceBefore = await pnk.balanceOf(deployer); expect(await sortition.latestDelayedStakeIndex(deployer, 2)).to.be.equal(1); await expect(core.setStake(2, ONE_THOUSAND_PNK.mul(2))) - .to.emit(core, "StakeDelayed") + .to.emit(core, "StakeDelayedNotTransferred") .withArgs(deployer, 2, ONE_THOUSAND_PNK.mul(2)); expect(await sortition.latestDelayedStakeIndex(deployer, 2)).to.be.equal(2); expect(await core.getJurorBalance(deployer, 2)).to.be.deep.equal([ diff --git a/subgraph/src/KlerosCore.ts b/subgraph/src/KlerosCore.ts index deb9fbba8..2c7983733 100644 --- a/subgraph/src/KlerosCore.ts +++ b/subgraph/src/KlerosCore.ts @@ -12,7 +12,7 @@ import { TokenAndETHShift as TokenAndETHShiftEvent, CourtJump, Ruling, - StakeDelayed, + StakeDelayedNotTransferred, AcceptedFeeToken, } from "../generated/KlerosCore/KlerosCore"; import { ZERO, ONE } from "./utils"; @@ -198,7 +198,7 @@ export function handleStakeSet(event: StakeSet): void { } } -export function handleStakeDelayed(event: StakeDelayed): void { +export function handleStakeDelayedNotTransferred(event: StakeDelayedNotTransferred): void { updateJurorDelayedStake(event.params._address.toHexString(), event.params._courtID.toString(), event.params._amount); } diff --git a/subgraph/subgraph.yaml b/subgraph/subgraph.yaml index 0fd7441f9..e4d86d63d 100644 --- a/subgraph/subgraph.yaml +++ b/subgraph/subgraph.yaml @@ -48,8 +48,8 @@ dataSources: handler: handleDisputeKitEnabled - event: StakeSet(indexed address,uint256,uint256) handler: handleStakeSet - - event: StakeDelayed(indexed address,uint256,uint256) - handler: handleStakeDelayed + - event: StakeDelayedNotTransferred(indexed address,uint256,uint256) + handler: handleStakeDelayedNotTransferred - event: TokenAndETHShift(indexed address,indexed uint256,indexed uint256,uint256,int256,int256,address) handler: handleTokenAndETHShift - event: Ruling(indexed address,indexed uint256,uint256) From 3d7a80e968442c0f3cda17b4b27df38029b8f163 Mon Sep 17 00:00:00 2001 From: jaybuidl Date: Sat, 9 Dec 2023 00:56:02 +0000 Subject: [PATCH 05/10] test: coverage of delayed stakes --- contracts/test/arbitration/staking.ts | 132 +++++++++++++++++--------- 1 file changed, 88 insertions(+), 44 deletions(-) diff --git a/contracts/test/arbitration/staking.ts b/contracts/test/arbitration/staking.ts index e761779a6..d9afe1672 100644 --- a/contracts/test/arbitration/staking.ts +++ b/contracts/test/arbitration/staking.ts @@ -1,4 +1,5 @@ import { ethers, getNamedAccounts, network, deployments } from "hardhat"; +const { anyValue } = require("@nomicfoundation/hardhat-chai-matchers/withArgs"); import { BigNumber } from "ethers"; import { PNK, @@ -15,8 +16,8 @@ import exp from "constants"; /* eslint-disable no-unused-expressions */ describe("Staking", async () => { - const ONE_TENTH_ETH = BigNumber.from(10).pow(17); - const ONE_THOUSAND_PNK = BigNumber.from(10).pow(21); + const ETH = (amount: number) => ethers.utils.parseUnits(amount.toString()); + const PNK = ETH; // 2nd court, 3 jurors, 1 dispute kit const extraData = @@ -49,13 +50,13 @@ describe("Staking", async () => { const reachDrawingPhase = async () => { expect(await sortition.phase()).to.be.equal(0); // Staking - const arbitrationCost = ONE_TENTH_ETH.mul(3); + const arbitrationCost = ETH(0.1).mul(3); - await core.createCourt(1, false, ONE_THOUSAND_PNK, 1000, ONE_TENTH_ETH, 3, [0, 0, 0, 0], 3, [1]); // Parent - general court, Classic dispute kit + await core.createCourt(1, false, PNK(1000), 1000, ETH(0.1), 3, [0, 0, 0, 0], 3, [1]); // Parent - general court, Classic dispute kit - await pnk.approve(core.address, ONE_THOUSAND_PNK.mul(4)); - await core.setStake(1, ONE_THOUSAND_PNK.mul(2)); - await core.setStake(2, ONE_THOUSAND_PNK.mul(2)); + await pnk.approve(core.address, PNK(4000)); + await core.setStake(1, PNK(2000)); + await core.setStake(2, PNK(2000)); expect(await core.getJurorCourtIDs(deployer)).to.be.deep.equal([BigNumber.from("1"), BigNumber.from("2")]); @@ -73,6 +74,13 @@ describe("Staking", async () => { balanceBefore = await pnk.balanceOf(deployer); }; + const reachStakingPhaseAfterDrawing = async () => { + await randomizer.relay(rng.address, 0, ethers.utils.randomBytes(32)); + await sortition.passPhase(); // Generating -> Drawing + await core.draw(0, 5000); + await sortition.passPhase(); // Drawing -> Staking + }; + describe("When decreasing then increasing back stake", async () => { before("Setup", async () => { await deploy(); @@ -82,9 +90,9 @@ describe("Staking", async () => { it("Should be outside the Staking phase", async () => { expect(await sortition.phase()).to.be.equal(1); // Drawing expect(await core.getJurorBalance(deployer, 2)).to.be.deep.equal([ - ONE_THOUSAND_PNK.mul(4), + PNK(4000), BigNumber.from(0), - ONE_THOUSAND_PNK.mul(2), + PNK(2000), BigNumber.from(2), ]); }); @@ -93,40 +101,58 @@ describe("Staking", async () => { expect(await sortition.delayedStakeWriteIndex()).to.be.equal(0); expect(await sortition.delayedStakeReadIndex()).to.be.equal(1); expect(await sortition.latestDelayedStakeIndex(deployer, 2)).to.be.equal(0); - await expect(core.setStake(2, ONE_THOUSAND_PNK.mul(1))) + await expect(core.setStake(2, PNK(1000))) .to.emit(core, "StakeDelayedNotTransferred") - .withArgs(deployer, 2, ONE_THOUSAND_PNK.mul(1)); - expect(await sortition.latestDelayedStakeIndex(deployer, 2)).to.be.equal(1); + .withArgs(deployer, 2, PNK(1000)); expect(await core.getJurorBalance(deployer, 2)).to.be.deep.equal([ - ONE_THOUSAND_PNK.mul(4), + PNK(4000), BigNumber.from(0), - ONE_THOUSAND_PNK.mul(2), + PNK(2000), BigNumber.from(2), ]); // stake unchanged, delayed expect(await pnk.balanceOf(deployer)).to.be.equal(balanceBefore); // No PNK transfer yet + expect(await sortition.latestDelayedStakeIndex(deployer, 2)).to.be.equal(1); expect(await sortition.delayedStakeWriteIndex()).to.be.equal(1); expect(await sortition.delayedStakeReadIndex()).to.be.equal(1); - expect(await sortition.delayedStakes(1)).to.be.deep.equal([deployer, 2, ONE_THOUSAND_PNK.mul(1), false]); + expect(await sortition.delayedStakes(1)).to.be.deep.equal([deployer, 2, PNK(1000), false]); }); it("Should delay the stake increase back to the previous amount", async () => { balanceBefore = await pnk.balanceOf(deployer); expect(await sortition.latestDelayedStakeIndex(deployer, 2)).to.be.equal(1); - await expect(core.setStake(2, ONE_THOUSAND_PNK.mul(2))) + await expect(core.setStake(2, PNK(2000))) .to.emit(core, "StakeDelayedNotTransferred") - .withArgs(deployer, 2, ONE_THOUSAND_PNK.mul(2)); - expect(await sortition.latestDelayedStakeIndex(deployer, 2)).to.be.equal(2); + .withArgs(deployer, 2, PNK(2000)); expect(await core.getJurorBalance(deployer, 2)).to.be.deep.equal([ - ONE_THOUSAND_PNK.mul(4), + PNK(4000), BigNumber.from(0), - ONE_THOUSAND_PNK.mul(2), + PNK(2000), BigNumber.from(2), ]); // stake unchanged, delayed expect(await pnk.balanceOf(deployer)).to.be.equal(balanceBefore); // No PNK transfer yet + expect(await sortition.latestDelayedStakeIndex(deployer, 2)).to.be.equal(2); expect(await sortition.delayedStakeWriteIndex()).to.be.equal(2); expect(await sortition.delayedStakeReadIndex()).to.be.equal(1); expect(await sortition.delayedStakes(1)).to.be.deep.equal([ethers.constants.AddressZero, 0, 0, false]); // the 1st delayed stake got deleted - expect(await sortition.delayedStakes(2)).to.be.deep.equal([deployer, 2, ONE_THOUSAND_PNK.mul(2), false]); + expect(await sortition.delayedStakes(2)).to.be.deep.equal([deployer, 2, PNK(2000), false]); + }); + + it("Should execute the delayed stakes but the stakes should remain the same", async () => { + await reachStakingPhaseAfterDrawing(); + balanceBefore = await pnk.balanceOf(deployer); + await expect(sortition.executeDelayedStakes(10)).to.emit(core, "StakeSet").withArgs(deployer, 2, PNK(2000)); + expect(await core.getJurorBalance(deployer, 2)).to.be.deep.equal([ + PNK(4000), + PNK(300), // we're the only juror so we are drawn 3 times + PNK(2000), + BigNumber.from(2), + ]); // stake unchanged, delayed + expect(await pnk.balanceOf(deployer)).to.be.equal(balanceBefore); // No PNK transfer yet + expect(await sortition.delayedStakeWriteIndex()).to.be.equal(2); + expect(await sortition.delayedStakeReadIndex()).to.be.equal(3); + expect(await sortition.delayedStakes(1)).to.be.deep.equal([ethers.constants.AddressZero, 0, 0, false]); // the 1st delayed stake got deleted + expect(await sortition.delayedStakes(2)).to.be.deep.equal([ethers.constants.AddressZero, 0, 0, false]); // the 2nd delayed stake got deleted + expect(await sortition.latestDelayedStakeIndex(deployer, 2)).to.be.equal(0); // no delayed stakes left }); }); @@ -139,9 +165,9 @@ describe("Staking", async () => { it("Should be outside the Staking phase", async () => { expect(await sortition.phase()).to.be.equal(1); // Drawing expect(await core.getJurorBalance(deployer, 2)).to.be.deep.equal([ - ONE_THOUSAND_PNK.mul(4), + PNK(4000), BigNumber.from(0), - ONE_THOUSAND_PNK.mul(2), + PNK(2000), BigNumber.from(2), ]); }); @@ -149,42 +175,60 @@ describe("Staking", async () => { it("Should transfer PNK but delay the stake increase", async () => { expect(await sortition.delayedStakeWriteIndex()).to.be.equal(0); expect(await sortition.delayedStakeReadIndex()).to.be.equal(1); - await pnk.approve(core.address, ONE_THOUSAND_PNK.mul(1)); + await pnk.approve(core.address, PNK(1000)); expect(await sortition.latestDelayedStakeIndex(deployer, 2)).to.be.equal(0); - await expect(core.setStake(2, ONE_THOUSAND_PNK.mul(3))) + await expect(core.setStake(2, PNK(3000))) .to.emit(core, "StakeDelayedAlreadyTransferred") - .withArgs(deployer, 2, ONE_THOUSAND_PNK.mul(3)); - expect(await sortition.latestDelayedStakeIndex(deployer, 2)).to.be.equal(1); + .withArgs(deployer, 2, PNK(3000)); expect(await core.getJurorBalance(deployer, 2)).to.be.deep.equal([ - ONE_THOUSAND_PNK.mul(5), + PNK(5000), BigNumber.from(0), - ONE_THOUSAND_PNK.mul(3), + PNK(3000), BigNumber.from(2), ]); // stake has changed immediately, WARNING: this is misleading because it's not actually added to the SortitionSumTree - expect(await pnk.balanceOf(deployer)).to.be.equal(balanceBefore.sub(ONE_THOUSAND_PNK)); // PNK is transferred out of the juror's account + expect(await pnk.balanceOf(deployer)).to.be.equal(balanceBefore.sub(PNK(1000))); // PNK is transferred out of the juror's account + expect(await sortition.latestDelayedStakeIndex(deployer, 2)).to.be.equal(1); expect(await sortition.delayedStakeWriteIndex()).to.be.equal(1); expect(await sortition.delayedStakeReadIndex()).to.be.equal(1); - expect(await sortition.delayedStakes(1)).to.be.deep.equal([deployer, 2, ONE_THOUSAND_PNK.mul(3), true]); + expect(await sortition.delayedStakes(1)).to.be.deep.equal([deployer, 2, PNK(3000), true]); }); it("Should cancel out the stake decrease back", async () => { balanceBefore = await pnk.balanceOf(deployer); expect(await sortition.latestDelayedStakeIndex(deployer, 2)).to.be.equal(1); - await expect(core.setStake(2, ONE_THOUSAND_PNK.mul(2))) + await expect(core.setStake(2, PNK(2000))) .to.emit(core, "StakeDelayedNotTransferred") - .withArgs(deployer, 2, ONE_THOUSAND_PNK.mul(2)); - expect(await sortition.latestDelayedStakeIndex(deployer, 2)).to.be.equal(2); + .withArgs(deployer, 2, PNK(2000)); expect(await core.getJurorBalance(deployer, 2)).to.be.deep.equal([ - ONE_THOUSAND_PNK.mul(4), + PNK(4000), BigNumber.from(0), - ONE_THOUSAND_PNK.mul(2), + PNK(2000), BigNumber.from(2), ]); // stake has changed immediately - expect(await pnk.balanceOf(deployer)).to.be.equal(balanceBefore.add(ONE_THOUSAND_PNK)); // PNK is sent back to the juror + expect(await pnk.balanceOf(deployer)).to.be.equal(balanceBefore.add(PNK(1000))); // PNK is sent back to the juror + expect(await sortition.latestDelayedStakeIndex(deployer, 2)).to.be.equal(2); expect(await sortition.delayedStakeWriteIndex()).to.be.equal(2); expect(await sortition.delayedStakeReadIndex()).to.be.equal(1); expect(await sortition.delayedStakes(1)).to.be.deep.equal([ethers.constants.AddressZero, 0, 0, false]); // the 1st delayed stake got deleted - expect(await sortition.delayedStakes(2)).to.be.deep.equal([deployer, 2, ONE_THOUSAND_PNK.mul(2), false]); + expect(await sortition.delayedStakes(2)).to.be.deep.equal([deployer, 2, PNK(2000), false]); + }); + + it("Should execute the delayed stakes but the stakes should remain the same", async () => { + await reachStakingPhaseAfterDrawing(); + balanceBefore = await pnk.balanceOf(deployer); + await expect(sortition.executeDelayedStakes(10)).to.emit(core, "StakeSet").withArgs(deployer, 2, PNK(2000)); + expect(await core.getJurorBalance(deployer, 2)).to.be.deep.equal([ + PNK(4000), + PNK(300), // we're the only juror so we are drawn 3 times + PNK(2000), + BigNumber.from(2), + ]); // stake unchanged, delayed + expect(await pnk.balanceOf(deployer)).to.be.equal(balanceBefore); // No PNK transfer yet + expect(await sortition.delayedStakeWriteIndex()).to.be.equal(2); + expect(await sortition.delayedStakeReadIndex()).to.be.equal(3); + expect(await sortition.delayedStakes(1)).to.be.deep.equal([ethers.constants.AddressZero, 0, 0, false]); // the 1st delayed stake got deleted + expect(await sortition.delayedStakes(2)).to.be.deep.equal([ethers.constants.AddressZero, 0, 0, false]); // the 2nd delayed stake got deleted + expect(await sortition.latestDelayedStakeIndex(deployer, 2)).to.be.equal(0); // no delayed stakes left }); }); }); @@ -195,13 +239,13 @@ describe("Staking", async () => { }); it("Should unstake from all courts", async () => { - const arbitrationCost = ONE_TENTH_ETH.mul(3); + const arbitrationCost = ETH(0.1).mul(3); - await core.createCourt(1, false, ONE_THOUSAND_PNK, 1000, ONE_TENTH_ETH, 3, [0, 0, 0, 0], 3, [1]); // Parent - general court, Classic dispute kit + await core.createCourt(1, false, PNK(1000), 1000, ETH(0.1), 3, [0, 0, 0, 0], 3, [1]); // Parent - general court, Classic dispute kit - await pnk.approve(core.address, ONE_THOUSAND_PNK.mul(4)); - await core.setStake(1, ONE_THOUSAND_PNK.mul(2)); - await core.setStake(2, ONE_THOUSAND_PNK.mul(2)); + await pnk.approve(core.address, PNK(4000)); + await core.setStake(1, PNK(2000)); + await core.setStake(2, PNK(2000)); expect(await core.getJurorCourtIDs(deployer)).to.be.deep.equal([BigNumber.from("1"), BigNumber.from("2")]); @@ -224,7 +268,7 @@ describe("Staking", async () => { await core.passPeriod(0); // Voting -> Appeal await core.passPeriod(0); // Appeal -> Execution - await sortition.passPhase(); // Freezing -> Staking. Change so we don't deal with delayed stakes + await sortition.passPhase(); // Drawing -> Staking. Change so we don't deal with delayed stakes expect(await core.getJurorCourtIDs(deployer)).to.be.deep.equal([BigNumber.from("1"), BigNumber.from("2")]); From 6dcc035985c409bb4e4db76f2607f28cb1ae8e85 Mon Sep 17 00:00:00 2001 From: jaybuidl Date: Sat, 9 Dec 2023 01:42:59 +0000 Subject: [PATCH 06/10] test: refactored and improved --- contracts/test/arbitration/staking.ts | 227 ++++++++++++++------------ 1 file changed, 120 insertions(+), 107 deletions(-) diff --git a/contracts/test/arbitration/staking.ts b/contracts/test/arbitration/staking.ts index d9afe1672..171b7e67c 100644 --- a/contracts/test/arbitration/staking.ts +++ b/contracts/test/arbitration/staking.ts @@ -11,6 +11,7 @@ import { } from "../../typechain-types"; import { expect } from "chai"; import exp from "constants"; +import { it } from "mocha"; /* eslint-disable no-unused-vars */ /* eslint-disable no-unused-expressions */ @@ -81,7 +82,7 @@ describe("Staking", async () => { await sortition.passPhase(); // Drawing -> Staking }; - describe("When decreasing then increasing back stake", async () => { + describe("When stake is decreased then increased back", async () => { before("Setup", async () => { await deploy(); await reachDrawingPhase(); @@ -89,74 +90,80 @@ describe("Staking", async () => { it("Should be outside the Staking phase", async () => { expect(await sortition.phase()).to.be.equal(1); // Drawing - expect(await core.getJurorBalance(deployer, 2)).to.be.deep.equal([ - PNK(4000), - BigNumber.from(0), - PNK(2000), - BigNumber.from(2), - ]); + expect(await core.getJurorBalance(deployer, 2)).to.be.deep.equal([PNK(4000), 0, PNK(2000), 2]); }); - it("Should delay the stake decrease", async () => { - expect(await sortition.delayedStakeWriteIndex()).to.be.equal(0); - expect(await sortition.delayedStakeReadIndex()).to.be.equal(1); - expect(await sortition.latestDelayedStakeIndex(deployer, 2)).to.be.equal(0); - await expect(core.setStake(2, PNK(1000))) - .to.emit(core, "StakeDelayedNotTransferred") - .withArgs(deployer, 2, PNK(1000)); - expect(await core.getJurorBalance(deployer, 2)).to.be.deep.equal([ - PNK(4000), - BigNumber.from(0), - PNK(2000), - BigNumber.from(2), - ]); // stake unchanged, delayed - expect(await pnk.balanceOf(deployer)).to.be.equal(balanceBefore); // No PNK transfer yet - expect(await sortition.latestDelayedStakeIndex(deployer, 2)).to.be.equal(1); - expect(await sortition.delayedStakeWriteIndex()).to.be.equal(1); - expect(await sortition.delayedStakeReadIndex()).to.be.equal(1); - expect(await sortition.delayedStakes(1)).to.be.deep.equal([deployer, 2, PNK(1000), false]); + describe("When stake is decreased", async () => { + it("Should delay the stake decrease", async () => { + expect(await sortition.delayedStakeWriteIndex()).to.be.equal(0); + expect(await sortition.delayedStakeReadIndex()).to.be.equal(1); + expect(await sortition.latestDelayedStakeIndex(deployer, 2)).to.be.equal(0); + await expect(core.setStake(2, PNK(1000))) + .to.emit(core, "StakeDelayedNotTransferred") + .withArgs(deployer, 2, PNK(1000)); + expect(await core.getJurorBalance(deployer, 2)).to.be.deep.equal([PNK(4000), 0, PNK(2000), 2]); // stake unchanged, delayed + }); + + it("Should not transfer any PNK", async () => { + expect(await pnk.balanceOf(deployer)).to.be.equal(balanceBefore); // No PNK transfer yet + }); + + it("Should store the delayed stake for later", async () => { + expect(await sortition.latestDelayedStakeIndex(deployer, 2)).to.be.equal(1); + expect(await sortition.delayedStakeWriteIndex()).to.be.equal(1); + expect(await sortition.delayedStakeReadIndex()).to.be.equal(1); + expect(await sortition.delayedStakes(1)).to.be.deep.equal([deployer, 2, PNK(1000), false]); + }); }); - it("Should delay the stake increase back to the previous amount", async () => { - balanceBefore = await pnk.balanceOf(deployer); - expect(await sortition.latestDelayedStakeIndex(deployer, 2)).to.be.equal(1); - await expect(core.setStake(2, PNK(2000))) - .to.emit(core, "StakeDelayedNotTransferred") - .withArgs(deployer, 2, PNK(2000)); - expect(await core.getJurorBalance(deployer, 2)).to.be.deep.equal([ - PNK(4000), - BigNumber.from(0), - PNK(2000), - BigNumber.from(2), - ]); // stake unchanged, delayed - expect(await pnk.balanceOf(deployer)).to.be.equal(balanceBefore); // No PNK transfer yet - expect(await sortition.latestDelayedStakeIndex(deployer, 2)).to.be.equal(2); - expect(await sortition.delayedStakeWriteIndex()).to.be.equal(2); - expect(await sortition.delayedStakeReadIndex()).to.be.equal(1); - expect(await sortition.delayedStakes(1)).to.be.deep.equal([ethers.constants.AddressZero, 0, 0, false]); // the 1st delayed stake got deleted - expect(await sortition.delayedStakes(2)).to.be.deep.equal([deployer, 2, PNK(2000), false]); + describe("When stake is increased back to the previous amount", () => { + it("Should delay the stake increase", async () => { + balanceBefore = await pnk.balanceOf(deployer); + expect(await sortition.latestDelayedStakeIndex(deployer, 2)).to.be.equal(1); + await expect(core.setStake(2, PNK(2000))) + .to.emit(core, "StakeDelayedNotTransferred") + .withArgs(deployer, 2, PNK(2000)); + expect(await core.getJurorBalance(deployer, 2)).to.be.deep.equal([PNK(4000), 0, PNK(2000), 2]); // stake unchanged, delayed + }); + + it("Should not transfer any PNK", async () => { + expect(await pnk.balanceOf(deployer)).to.be.equal(balanceBefore); // No PNK transfer yet + }); + + it("Should store the delayed stake for later", async () => { + expect(await sortition.latestDelayedStakeIndex(deployer, 2)).to.be.equal(2); + expect(await sortition.delayedStakeWriteIndex()).to.be.equal(2); + expect(await sortition.delayedStakeReadIndex()).to.be.equal(1); + expect(await sortition.delayedStakes(1)).to.be.deep.equal([ethers.constants.AddressZero, 0, 0, false]); // the 1st delayed stake got deleted + expect(await sortition.delayedStakes(2)).to.be.deep.equal([deployer, 2, PNK(2000), false]); + }); }); - it("Should execute the delayed stakes but the stakes should remain the same", async () => { - await reachStakingPhaseAfterDrawing(); - balanceBefore = await pnk.balanceOf(deployer); - await expect(sortition.executeDelayedStakes(10)).to.emit(core, "StakeSet").withArgs(deployer, 2, PNK(2000)); - expect(await core.getJurorBalance(deployer, 2)).to.be.deep.equal([ - PNK(4000), - PNK(300), // we're the only juror so we are drawn 3 times - PNK(2000), - BigNumber.from(2), - ]); // stake unchanged, delayed - expect(await pnk.balanceOf(deployer)).to.be.equal(balanceBefore); // No PNK transfer yet - expect(await sortition.delayedStakeWriteIndex()).to.be.equal(2); - expect(await sortition.delayedStakeReadIndex()).to.be.equal(3); - expect(await sortition.delayedStakes(1)).to.be.deep.equal([ethers.constants.AddressZero, 0, 0, false]); // the 1st delayed stake got deleted - expect(await sortition.delayedStakes(2)).to.be.deep.equal([ethers.constants.AddressZero, 0, 0, false]); // the 2nd delayed stake got deleted - expect(await sortition.latestDelayedStakeIndex(deployer, 2)).to.be.equal(0); // no delayed stakes left + describe("When the Phase passes back to Staking", () => { + it("Should execute the delayed stakes but the stakes should remain the same", async () => { + await reachStakingPhaseAfterDrawing(); + balanceBefore = await pnk.balanceOf(deployer); + await expect(sortition.executeDelayedStakes(10)).to.emit(core, "StakeSet").withArgs(deployer, 2, PNK(2000)); + expect(await core.getJurorBalance(deployer, 2)).to.be.deep.equal([ + PNK(4000), + PNK(300), // we're the only juror so we are drawn 3 times + PNK(2000), + 2, + ]); // stake unchanged, delayed + expect(await sortition.delayedStakeWriteIndex()).to.be.equal(2); + expect(await sortition.delayedStakeReadIndex()).to.be.equal(3); + expect(await sortition.delayedStakes(1)).to.be.deep.equal([ethers.constants.AddressZero, 0, 0, false]); // the 1st delayed stake got deleted + expect(await sortition.delayedStakes(2)).to.be.deep.equal([ethers.constants.AddressZero, 0, 0, false]); // the 2nd delayed stake got deleted + expect(await sortition.latestDelayedStakeIndex(deployer, 2)).to.be.equal(0); // no delayed stakes left + }); + + it("Should not transfer any PNK", async () => { + expect(await pnk.balanceOf(deployer)).to.be.equal(balanceBefore); // No PNK transfer yet + }); }); }); - describe("When increasing then decreasing back stake", async () => { + describe("When stake is increased then decreased back", async () => { before("Setup", async () => { await deploy(); await reachDrawingPhase(); @@ -164,55 +171,58 @@ describe("Staking", async () => { it("Should be outside the Staking phase", async () => { expect(await sortition.phase()).to.be.equal(1); // Drawing - expect(await core.getJurorBalance(deployer, 2)).to.be.deep.equal([ - PNK(4000), - BigNumber.from(0), - PNK(2000), - BigNumber.from(2), - ]); + expect(await core.getJurorBalance(deployer, 2)).to.be.deep.equal([PNK(4000), 0, PNK(2000), 2]); }); - it("Should transfer PNK but delay the stake increase", async () => { - expect(await sortition.delayedStakeWriteIndex()).to.be.equal(0); - expect(await sortition.delayedStakeReadIndex()).to.be.equal(1); - await pnk.approve(core.address, PNK(1000)); - expect(await sortition.latestDelayedStakeIndex(deployer, 2)).to.be.equal(0); - await expect(core.setStake(2, PNK(3000))) - .to.emit(core, "StakeDelayedAlreadyTransferred") - .withArgs(deployer, 2, PNK(3000)); - expect(await core.getJurorBalance(deployer, 2)).to.be.deep.equal([ - PNK(5000), - BigNumber.from(0), - PNK(3000), - BigNumber.from(2), - ]); // stake has changed immediately, WARNING: this is misleading because it's not actually added to the SortitionSumTree - expect(await pnk.balanceOf(deployer)).to.be.equal(balanceBefore.sub(PNK(1000))); // PNK is transferred out of the juror's account - expect(await sortition.latestDelayedStakeIndex(deployer, 2)).to.be.equal(1); - expect(await sortition.delayedStakeWriteIndex()).to.be.equal(1); - expect(await sortition.delayedStakeReadIndex()).to.be.equal(1); - expect(await sortition.delayedStakes(1)).to.be.deep.equal([deployer, 2, PNK(3000), true]); + describe("When stake is increased", () => { + it("Should transfer PNK but delay the stake increase", async () => { + expect(await sortition.delayedStakeWriteIndex()).to.be.equal(0); + expect(await sortition.delayedStakeReadIndex()).to.be.equal(1); + await pnk.approve(core.address, PNK(1000)); + expect(await sortition.latestDelayedStakeIndex(deployer, 2)).to.be.equal(0); + await expect(core.setStake(2, PNK(3000))) + .to.emit(core, "StakeDelayedAlreadyTransferred") + .withArgs(deployer, 2, PNK(3000)); + expect(await core.getJurorBalance(deployer, 2)).to.be.deep.equal([PNK(5000), 0, PNK(3000), 2]); // stake has changed immediately, WARNING: this is misleading because it's not actually added to the SortitionSumTree + }); + + it("Should transfer some PNK out of the juror's account", async () => { + expect(await pnk.balanceOf(deployer)).to.be.equal(balanceBefore.sub(PNK(1000))); // PNK is transferred out of the juror's account + }); + + it("Should store the delayed stake for later", async () => { + expect(await sortition.latestDelayedStakeIndex(deployer, 2)).to.be.equal(1); + expect(await sortition.delayedStakeWriteIndex()).to.be.equal(1); + expect(await sortition.delayedStakeReadIndex()).to.be.equal(1); + expect(await sortition.delayedStakes(1)).to.be.deep.equal([deployer, 2, PNK(3000), true]); + }); }); - it("Should cancel out the stake decrease back", async () => { - balanceBefore = await pnk.balanceOf(deployer); - expect(await sortition.latestDelayedStakeIndex(deployer, 2)).to.be.equal(1); - await expect(core.setStake(2, PNK(2000))) - .to.emit(core, "StakeDelayedNotTransferred") - .withArgs(deployer, 2, PNK(2000)); - expect(await core.getJurorBalance(deployer, 2)).to.be.deep.equal([ - PNK(4000), - BigNumber.from(0), - PNK(2000), - BigNumber.from(2), - ]); // stake has changed immediately - expect(await pnk.balanceOf(deployer)).to.be.equal(balanceBefore.add(PNK(1000))); // PNK is sent back to the juror - expect(await sortition.latestDelayedStakeIndex(deployer, 2)).to.be.equal(2); - expect(await sortition.delayedStakeWriteIndex()).to.be.equal(2); - expect(await sortition.delayedStakeReadIndex()).to.be.equal(1); - expect(await sortition.delayedStakes(1)).to.be.deep.equal([ethers.constants.AddressZero, 0, 0, false]); // the 1st delayed stake got deleted - expect(await sortition.delayedStakes(2)).to.be.deep.equal([deployer, 2, PNK(2000), false]); + describe("When stake is decreased back to the previous amount", () => { + it("Should cancel out the stake decrease back", async () => { + balanceBefore = await pnk.balanceOf(deployer); + expect(await sortition.latestDelayedStakeIndex(deployer, 2)).to.be.equal(1); + await expect(core.setStake(2, PNK(2000))) + .to.emit(core, "StakeDelayedNotTransferred") + .withArgs(deployer, 2, PNK(2000)); + expect(await core.getJurorBalance(deployer, 2)).to.be.deep.equal([PNK(4000), 0, PNK(2000), 2]); // stake has changed immediately + }); + + it("Should transfer back some PNK to the juror", async () => { + expect(await pnk.balanceOf(deployer)).to.be.equal(balanceBefore.add(PNK(1000))); // PNK is sent back to the juror + }); + + it("Should store the delayed stake for later", async () => { + expect(await sortition.latestDelayedStakeIndex(deployer, 2)).to.be.equal(2); + expect(await sortition.delayedStakeWriteIndex()).to.be.equal(2); + expect(await sortition.delayedStakeReadIndex()).to.be.equal(1); + expect(await sortition.delayedStakes(1)).to.be.deep.equal([ethers.constants.AddressZero, 0, 0, false]); // the 1st delayed stake got deleted + expect(await sortition.delayedStakes(2)).to.be.deep.equal([deployer, 2, PNK(2000), false]); + }); }); + }); + describe("When the Phase passes back to Staking", () => { it("Should execute the delayed stakes but the stakes should remain the same", async () => { await reachStakingPhaseAfterDrawing(); balanceBefore = await pnk.balanceOf(deployer); @@ -221,15 +231,18 @@ describe("Staking", async () => { PNK(4000), PNK(300), // we're the only juror so we are drawn 3 times PNK(2000), - BigNumber.from(2), + 2, ]); // stake unchanged, delayed - expect(await pnk.balanceOf(deployer)).to.be.equal(balanceBefore); // No PNK transfer yet expect(await sortition.delayedStakeWriteIndex()).to.be.equal(2); expect(await sortition.delayedStakeReadIndex()).to.be.equal(3); expect(await sortition.delayedStakes(1)).to.be.deep.equal([ethers.constants.AddressZero, 0, 0, false]); // the 1st delayed stake got deleted expect(await sortition.delayedStakes(2)).to.be.deep.equal([ethers.constants.AddressZero, 0, 0, false]); // the 2nd delayed stake got deleted expect(await sortition.latestDelayedStakeIndex(deployer, 2)).to.be.equal(0); // no delayed stakes left }); + + it("Should not transfer any PNK", async () => { + expect(await pnk.balanceOf(deployer)).to.be.equal(balanceBefore); // No PNK transfer yet + }); }); }); @@ -247,7 +260,7 @@ describe("Staking", async () => { await core.setStake(1, PNK(2000)); await core.setStake(2, PNK(2000)); - expect(await core.getJurorCourtIDs(deployer)).to.be.deep.equal([BigNumber.from("1"), BigNumber.from("2")]); + expect(await core.getJurorCourtIDs(deployer)).to.be.deep.equal([1, 2]); await core.functions["createDispute(uint256,bytes)"](2, extraData, { value: arbitrationCost }); @@ -270,7 +283,7 @@ describe("Staking", async () => { await sortition.passPhase(); // Drawing -> Staking. Change so we don't deal with delayed stakes - expect(await core.getJurorCourtIDs(deployer)).to.be.deep.equal([BigNumber.from("1"), BigNumber.from("2")]); + expect(await core.getJurorCourtIDs(deployer)).to.be.deep.equal([1, 2]); await core.execute(0, 0, 1); // 1 iteration should unstake from both courts From 4328a5f9aab75ded6ec35bf736e64ae60bbfcf05 Mon Sep 17 00:00:00 2001 From: jaybuidl Date: Tue, 12 Dec 2023 13:00:05 +0000 Subject: [PATCH 07/10] docs: comment --- contracts/src/arbitration/KlerosCore.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/src/arbitration/KlerosCore.sol b/contracts/src/arbitration/KlerosCore.sol index 13a1e9104..0b1cf6f8b 100644 --- a/contracts/src/arbitration/KlerosCore.sol +++ b/contracts/src/arbitration/KlerosCore.sol @@ -476,7 +476,7 @@ contract KlerosCore is IArbitratorV2, UUPSProxiable, Initializable { ) external { if (msg.sender != address(sortitionModule)) revert SortitionModuleOnly(); // Always nullify the latest delayed stake before setting a new value. - // Note that we check the delayed stake here too because the check in `setStake` can be bypassed + // Note that we call _deleteDelayedStake() here too because the one in `setStake` can be bypassed // if the stake was updated automatically during `execute` (e.g. when unstaking inactive juror). _deleteDelayedStake(_courtID); _setStakeForAccount(_account, _courtID, _newStake, _alreadyTransferred); From 7e0d1bd0f66c13146158479ac4863343601ad896 Mon Sep 17 00:00:00 2001 From: jaybuidl Date: Tue, 12 Dec 2023 13:14:47 +0000 Subject: [PATCH 08/10] refactor: simplification by moving KC._deleteDelayedStake() inside SM.preStakeHook() --- contracts/src/arbitration/KlerosCore.sol | 11 ----- contracts/src/arbitration/SortitionModule.sol | 40 ++++++++++--------- .../interfaces/ISortitionModule.sol | 2 - 3 files changed, 21 insertions(+), 32 deletions(-) diff --git a/contracts/src/arbitration/KlerosCore.sol b/contracts/src/arbitration/KlerosCore.sol index 0b1cf6f8b..52ab49669 100644 --- a/contracts/src/arbitration/KlerosCore.sol +++ b/contracts/src/arbitration/KlerosCore.sol @@ -464,7 +464,6 @@ contract KlerosCore is IArbitratorV2, UUPSProxiable, Initializable { /// @param _newStake The new stake. /// Note that the existing delayed stake will be nullified as non-relevant. function setStake(uint96 _courtID, uint256 _newStake) external { - _deleteDelayedStake(_courtID); if (!_setStakeForAccount(msg.sender, _courtID, _newStake, false)) revert StakingFailed(); } @@ -475,10 +474,6 @@ contract KlerosCore is IArbitratorV2, UUPSProxiable, Initializable { bool _alreadyTransferred ) external { if (msg.sender != address(sortitionModule)) revert SortitionModuleOnly(); - // Always nullify the latest delayed stake before setting a new value. - // Note that we call _deleteDelayedStake() here too because the one in `setStake` can be bypassed - // if the stake was updated automatically during `execute` (e.g. when unstaking inactive juror). - _deleteDelayedStake(_courtID); _setStakeForAccount(_account, _courtID, _newStake, _alreadyTransferred); } @@ -1061,12 +1056,6 @@ contract KlerosCore is IArbitratorV2, UUPSProxiable, Initializable { emit DisputeKitEnabled(_courtID, _disputeKitID, _enable); } - /// @dev Removes the latest delayed stake if there is any. - /// @param _courtID The ID of the court. - function _deleteDelayedStake(uint96 _courtID) private { - sortitionModule.deleteDelayedStake(_courtID, msg.sender); - } - /// @dev Sets the specified juror's stake in a court. /// `O(n + p * log_k(j))` where /// `n` is the number of courts the juror has staked in, diff --git a/contracts/src/arbitration/SortitionModule.sol b/contracts/src/arbitration/SortitionModule.sol index ea50851aa..355121224 100644 --- a/contracts/src/arbitration/SortitionModule.sol +++ b/contracts/src/arbitration/SortitionModule.sol @@ -218,30 +218,13 @@ contract SortitionModule is ISortitionModule, UUPSProxiable, Initializable { delayedStakeReadIndex = newDelayedStakeReadIndex; } - /// @dev Checks if there is already a delayed stake. In this case consider it irrelevant and remove it. - /// @param _courtID ID of the court. - /// @param _juror Juror whose stake to check. - function deleteDelayedStake(uint96 _courtID, address _juror) external override onlyByCore { - uint256 latestIndex = latestDelayedStakeIndex[_juror][_courtID]; - if (latestIndex != 0) { - DelayedStake storage delayedStake = delayedStakes[latestIndex]; - if (delayedStake.alreadyTransferred) { - bytes32 stakePathID = _accountAndCourtIDToStakePathID(_juror, _courtID); - // Sortition stake represents the stake value that was last updated during Staking phase. - uint256 sortitionStake = stakeOf(bytes32(uint256(_courtID)), stakePathID); - // Withdraw the tokens that were added with the latest delayed stake. - core.withdrawPartiallyDelayedStake(_courtID, _juror, delayedStake.stake - sortitionStake); - } - delete delayedStakes[latestIndex]; - delete latestDelayedStakeIndex[_juror][_courtID]; - } - } - function preStakeHook( address _account, uint96 _courtID, uint256 _stake ) external override onlyByCore returns (PreStakeHookResult) { + _deleteDelayedStake(_courtID, _account); + (, , uint256 currentStake, uint256 nbCourts) = core.getJurorBalance(_account, _courtID); if (currentStake == 0 && nbCourts >= MAX_STAKE_PATHS) { // Prevent staking beyond MAX_STAKE_PATHS but unstaking is always allowed. @@ -268,6 +251,25 @@ contract SortitionModule is ISortitionModule, UUPSProxiable, Initializable { return PreStakeHookResult.ok; } + /// @dev Checks if there is already a delayed stake. In this case consider it irrelevant and remove it. + /// @param _courtID ID of the court. + /// @param _juror Juror whose stake to check. + function _deleteDelayedStake(uint96 _courtID, address _juror) internal { + uint256 latestIndex = latestDelayedStakeIndex[_juror][_courtID]; + if (latestIndex != 0) { + DelayedStake storage delayedStake = delayedStakes[latestIndex]; + if (delayedStake.alreadyTransferred) { + bytes32 stakePathID = _accountAndCourtIDToStakePathID(_juror, _courtID); + // Sortition stake represents the stake value that was last updated during Staking phase. + uint256 sortitionStake = stakeOf(bytes32(uint256(_courtID)), stakePathID); + // Withdraw the tokens that were added with the latest delayed stake. + core.withdrawPartiallyDelayedStake(_courtID, _juror, delayedStake.stake - sortitionStake); + } + delete delayedStakes[latestIndex]; + delete latestDelayedStakeIndex[_juror][_courtID]; + } + } + function createDisputeHook(uint256 /*_disputeID*/, uint256 /*_roundID*/) external override onlyByCore { disputesWithoutJurors++; } diff --git a/contracts/src/arbitration/interfaces/ISortitionModule.sol b/contracts/src/arbitration/interfaces/ISortitionModule.sol index 42eb2a4c0..611118be3 100644 --- a/contracts/src/arbitration/interfaces/ISortitionModule.sol +++ b/contracts/src/arbitration/interfaces/ISortitionModule.sol @@ -32,6 +32,4 @@ interface ISortitionModule { function createDisputeHook(uint256 _disputeID, uint256 _roundID) external; function postDrawHook(uint256 _disputeID, uint256 _roundID) external; - - function deleteDelayedStake(uint96 _courtID, address _juror) external; } From f0f2d1e57f483310702dafb70e0fae7750b84680 Mon Sep 17 00:00:00 2001 From: jaybuidl Date: Tue, 12 Dec 2023 23:20:25 +0000 Subject: [PATCH 09/10] refactor: moved the juror stakes accounting from KlerosCore to SortitionModule --- contracts/src/arbitration/KlerosCore.sol | 186 ++---------- contracts/src/arbitration/SortitionModule.sol | 267 ++++++++++++++---- .../dispute-kits/DisputeKitClassic.sol | 2 +- .../dispute-kits/DisputeKitSybilResistant.sol | 2 +- .../interfaces/ISortitionModule.sol | 29 +- contracts/test/arbitration/draw.ts | 30 +- contracts/test/arbitration/staking.ts | 45 +-- contracts/test/integration/index.ts | 8 +- 8 files changed, 306 insertions(+), 263 deletions(-) diff --git a/contracts/src/arbitration/KlerosCore.sol b/contracts/src/arbitration/KlerosCore.sol index 52ab49669..941990e6a 100644 --- a/contracts/src/arbitration/KlerosCore.sol +++ b/contracts/src/arbitration/KlerosCore.sol @@ -70,13 +70,6 @@ contract KlerosCore is IArbitratorV2, UUPSProxiable, Initializable { uint256 drawIterations; // The number of iterations passed drawing the jurors for this round. } - struct Juror { - uint96[] courtIDs; // The IDs of courts where the juror's stake path ends. A stake path is a path from the general court to a court the juror directly staked in using `_setStake`. - uint256 stakedPnk; // The juror's total amount of tokens staked in subcourts. Reflects actual pnk balance. - uint256 lockedPnk; // The juror's total amount of tokens locked in disputes. Can reflect actual pnk balance when stakedPnk are fully withdrawn. - mapping(uint96 => uint256) stakedPnkByCourt; // The amount of PNKs the juror has staked in the court in the form `stakedPnkByCourt[courtID]`. - } - // Workaround "stack too deep" errors struct ExecuteParams { uint256 disputeID; // The ID of the dispute to execute. @@ -107,21 +100,12 @@ contract KlerosCore is IArbitratorV2, UUPSProxiable, Initializable { Court[] public courts; // The courts. IDisputeKit[] public disputeKits; // Array of dispute kits. Dispute[] public disputes; // The disputes. - mapping(address => Juror) internal jurors; // The jurors. mapping(IERC20 => CurrencyRate) public currencyRates; // The price of each token in ETH. // ************************************* // // * Events * // // ************************************* // - event StakeSet(address indexed _address, uint256 _courtID, uint256 _amount); - event StakeDelayedNotTransferred(address indexed _address, uint256 _courtID, uint256 _amount); - event StakeDelayedAlreadyTransferred(address indexed _address, uint256 _courtID, uint256 _amount); - event StakeDelayedAlreadyTransferredWithdrawn( - uint96 indexed _courtID, - address indexed _account, - uint256 _withdrawnAmount - ); event NewPeriod(uint256 indexed _disputeID, Period _period); event AppealPossible(uint256 indexed _disputeID, IArbitrableV2 indexed _arbitrable); event AppealDecision(uint256 indexed _disputeID, IArbitrableV2 indexed _arbitrable); @@ -464,7 +448,7 @@ contract KlerosCore is IArbitratorV2, UUPSProxiable, Initializable { /// @param _newStake The new stake. /// Note that the existing delayed stake will be nullified as non-relevant. function setStake(uint96 _courtID, uint256 _newStake) external { - if (!_setStakeForAccount(msg.sender, _courtID, _newStake, false)) revert StakingFailed(); + _setStake(msg.sender, _courtID, _newStake, false); } function setStakeBySortitionModule( @@ -474,31 +458,7 @@ contract KlerosCore is IArbitratorV2, UUPSProxiable, Initializable { bool _alreadyTransferred ) external { if (msg.sender != address(sortitionModule)) revert SortitionModuleOnly(); - _setStakeForAccount(_account, _courtID, _newStake, _alreadyTransferred); - } - - function withdrawPartiallyDelayedStake(uint96 _courtID, address _juror, uint256 _amountToWithdraw) external { - if (msg.sender != address(sortitionModule)) revert SortitionModuleOnly(); - uint256 actualAmount = _amountToWithdraw; - Juror storage juror = jurors[_juror]; - if (juror.stakedPnk <= actualAmount) { - actualAmount = juror.stakedPnk; - } - require(pinakion.safeTransfer(_juror, actualAmount)); - // StakePnk can become lower because of penalty, thus we adjust the amount for it. stakedPnkByCourt can't be penalized so subtract the default amount. - juror.stakedPnk -= actualAmount; - juror.stakedPnkByCourt[_courtID] -= _amountToWithdraw; - emit StakeDelayedAlreadyTransferredWithdrawn(_courtID, _juror, _amountToWithdraw); - // Note that if we don't delete court here it'll be duplicated after staking. - if (juror.stakedPnkByCourt[_courtID] == 0) { - for (uint256 i = juror.courtIDs.length; i > 0; i--) { - if (juror.courtIDs[i - 1] == _courtID) { - juror.courtIDs[i - 1] = juror.courtIDs[juror.courtIDs.length - 1]; - juror.courtIDs.pop(); - break; - } - } - } + _setStake(_account, _courtID, _newStake, _alreadyTransferred); } /// @inheritdoc IArbitratorV2 @@ -625,7 +585,7 @@ contract KlerosCore is IArbitratorV2, UUPSProxiable, Initializable { if (drawnAddress == address(0)) { continue; } - jurors[drawnAddress].lockedPnk += round.pnkAtStakePerJuror; + sortitionModule.lockStake(drawnAddress, round.pnkAtStakePerJuror); emit Draw(drawnAddress, _disputeID, currentRound, round.drawnJurors.length); round.drawnJurors.push(drawnAddress); if (round.drawnJurors.length == round.nbVotes) { @@ -764,15 +724,10 @@ contract KlerosCore is IArbitratorV2, UUPSProxiable, Initializable { // Unlock the PNKs affected by the penalty address account = round.drawnJurors[_params.repartition]; - jurors[account].lockedPnk -= penalty; + sortitionModule.unlockStake(account, penalty); // Apply the penalty to the staked PNKs. - // Note that lockedPnk will always cover penalty while stakedPnk can become lower after manual unstaking. - if (jurors[account].stakedPnk >= penalty) { - jurors[account].stakedPnk -= penalty; - } else { - jurors[account].stakedPnk = 0; - } + sortitionModule.penalizeStake(account, penalty); emit TokenAndETHShift( account, _params.disputeID, @@ -831,10 +786,10 @@ contract KlerosCore is IArbitratorV2, UUPSProxiable, Initializable { uint256 pnkLocked = (round.pnkAtStakePerJuror * degreeOfCoherence) / ALPHA_DIVISOR; // Release the rest of the PNKs of the juror for this round. - jurors[account].lockedPnk -= pnkLocked; + sortitionModule.unlockStake(account, pnkLocked); // Give back the locked PNKs in case the juror fully unstaked earlier. - if (jurors[account].stakedPnk == 0) { + if (!sortitionModule.isJurorStaked(account)) { pinakion.safeTransfer(account, pnkLocked); } @@ -980,17 +935,6 @@ contract KlerosCore is IArbitratorV2, UUPSProxiable, Initializable { return disputes[_disputeID].rounds.length; } - function getJurorBalance( - address _juror, - uint96 _courtID - ) external view returns (uint256 totalStaked, uint256 totalLocked, uint256 stakedInCourt, uint256 nbCourts) { - Juror storage juror = jurors[_juror]; - totalStaked = juror.stakedPnk; - totalLocked = juror.lockedPnk; - stakedInCourt = juror.stakedPnkByCourt[_courtID]; - nbCourts = juror.courtIDs.length; - } - function isSupported(uint96 _courtID, uint256 _disputeKitID) external view returns (bool) { return courts[_courtID].supportedDisputeKits[_disputeKitID]; } @@ -1033,12 +977,6 @@ contract KlerosCore is IArbitratorV2, UUPSProxiable, Initializable { return disputeKits.length; } - /// @dev Gets the court identifiers where a specific `_juror` has staked. - /// @param _juror The address of the juror. - function getJurorCourtIDs(address _juror) public view returns (uint96[] memory) { - return jurors[_juror].courtIDs; - } - function convertEthToTokenAmount(IERC20 _toToken, uint256 _amountInEth) public view returns (uint256) { return (_amountInEth * 10 ** currencyRates[_toToken].rateDecimals) / currencyRates[_toToken].rateInEth; } @@ -1056,104 +994,34 @@ contract KlerosCore is IArbitratorV2, UUPSProxiable, Initializable { emit DisputeKitEnabled(_courtID, _disputeKitID, _enable); } - /// @dev Sets the specified juror's stake in a court. - /// `O(n + p * log_k(j))` where - /// `n` is the number of courts the juror has staked in, - /// `p` is the depth of the court tree, - /// `k` is the minimum number of children per node of one of these courts' sortition sum tree, - /// and `j` is the maximum number of jurors that ever staked in one of these courts simultaneously. - /// @param _account The address of the juror. - /// @param _courtID The ID of the court. - /// @param _newStake The new stake. - /// @param _alreadyTransferred True if the tokens were already transferred from juror. Only relevant for delayed stakes. - /// @return succeeded True if the call succeeded, false otherwise. - function _setStakeForAccount( + function _setStake( address _account, uint96 _courtID, uint256 _newStake, bool _alreadyTransferred - ) internal returns (bool succeeded) { - if (_courtID == Constants.FORKING_COURT || _courtID > courts.length) return false; - - Juror storage juror = jurors[_account]; - uint256 currentStake = juror.stakedPnkByCourt[_courtID]; - - if (_newStake != 0) { - if (_newStake < courts[_courtID].minStake) return false; - } else if (currentStake == 0) { - return false; + ) internal returns (bool success) { + if (_courtID == Constants.FORKING_COURT || _courtID > courts.length) { + return false; // Staking directly into the forking court is not allowed. } - - ISortitionModule.PreStakeHookResult result = sortitionModule.preStakeHook(_account, _courtID, _newStake); - if (result == ISortitionModule.PreStakeHookResult.failed) { - return false; - } else if (result == ISortitionModule.PreStakeHookResult.stakeDelayedNotTransferred) { - emit StakeDelayedNotTransferred(_account, _courtID, _newStake); - return true; + if (_newStake != 0 && _newStake < courts[_courtID].minStake) { + return false; // Staking less than the minimum stake is not allowed. } - - uint256 transferredAmount; - if (_newStake >= currentStake) { - if (!_alreadyTransferred) { - // Stake increase - // When stakedPnk becomes lower than lockedPnk count the locked tokens in when transferring tokens from juror. - // (E.g. stakedPnk = 0, lockedPnk = 150) which can happen if the juror unstaked fully while having some tokens locked. - uint256 previouslyLocked = (juror.lockedPnk >= juror.stakedPnk) ? juror.lockedPnk - juror.stakedPnk : 0; // underflow guard - transferredAmount = (_newStake >= currentStake + previouslyLocked) // underflow guard - ? _newStake - currentStake - previouslyLocked - : 0; - if (transferredAmount > 0) { - // Note we don't return false after incorrect transfer because when stake is increased the transfer is done immediately, thus it can't disrupt delayed stakes' queue. - pinakion.safeTransferFrom(_account, address(this), transferredAmount); - } - if (currentStake == 0) { - juror.courtIDs.push(_courtID); - } - } - } else { - // Note that stakes can be partially delayed only when stake is increased. - // Stake decrease: make sure locked tokens always stay in the contract. They can only be released during Execution. - if (juror.stakedPnk >= currentStake - _newStake + juror.lockedPnk) { - // We have enough pnk staked to afford withdrawal while keeping locked tokens. - transferredAmount = currentStake - _newStake; - } else if (juror.stakedPnk >= juror.lockedPnk) { - // Can't afford withdrawing the current stake fully. Take whatever is available while keeping locked tokens. - transferredAmount = juror.stakedPnk - juror.lockedPnk; - } - if (transferredAmount > 0) { - if (!pinakion.safeTransfer(_account, transferredAmount)) { - return false; - } - } - if (_newStake == 0) { - for (uint256 i = juror.courtIDs.length; i > 0; i--) { - if (juror.courtIDs[i - 1] == _courtID) { - juror.courtIDs[i - 1] = juror.courtIDs[juror.courtIDs.length - 1]; - juror.courtIDs.pop(); - break; - } - } + (uint256 pnkDeposit, uint256 pnkWithdrawal, bool sortitionSuccess) = sortitionModule.setStake( + _account, + _courtID, + _newStake, + _alreadyTransferred + ); + if (pnkDeposit > 0 && pnkWithdrawal > 0) revert StakingFailed(); + if (pnkDeposit > 0) { + // Note we don't return false after incorrect transfer because when stake is increased the transfer is done immediately, thus it can't disrupt delayed stakes' queue. + pinakion.safeTransferFrom(_account, address(this), pnkDeposit); + } else if (pnkWithdrawal > 0) { + if (!pinakion.safeTransfer(_account, pnkWithdrawal)) { + return false; } } - - // Note that stakedPnk can become async with currentStake (e.g. after penalty). - // Also note that these values were already updated if the stake was only partially delayed. - if (!_alreadyTransferred) { - juror.stakedPnk = (juror.stakedPnk >= currentStake) - ? juror.stakedPnk - currentStake + _newStake - : _newStake; - juror.stakedPnkByCourt[_courtID] = _newStake; - } - - // Transfer the tokens but don't update sortition module. - if (result == ISortitionModule.PreStakeHookResult.stakeDelayedAlreadyTransferred) { - emit StakeDelayedAlreadyTransferred(_account, _courtID, _newStake); - return true; - } - - sortitionModule.setStake(_account, _courtID, _newStake); - emit StakeSet(_account, _courtID, _newStake); - return true; + return sortitionSuccess; } /// @dev Gets a court ID, the minimum number of jurors and an ID of a dispute kit from a specified extra data bytes array. diff --git a/contracts/src/arbitration/SortitionModule.sol b/contracts/src/arbitration/SortitionModule.sol index 355121224..1ef0f4306 100644 --- a/contracts/src/arbitration/SortitionModule.sol +++ b/contracts/src/arbitration/SortitionModule.sol @@ -25,6 +25,13 @@ contract SortitionModule is ISortitionModule, UUPSProxiable, Initializable { // * Enums / Structs * // // ************************************* // + enum PreStakeHookResult { + ok, // Correct phase. All checks are passed. + stakeDelayedAlreadyTransferred, // Wrong phase but stake is increased, so transfer the tokens without updating the drawing chance. + stakeDelayedNotTransferred, // Wrong phase and stake is decreased. Delay the token transfer and drawing chance update. + failed // Checks didn't pass. Do no changes. + } + struct SortitionSumTree { uint256 K; // The maximum number of children per node. // We use this to keep track of vacant positions in the tree after removing a leaf. This is for keeping the tree as balanced as possible without spending gas on moving nodes around. @@ -42,6 +49,13 @@ contract SortitionModule is ISortitionModule, UUPSProxiable, Initializable { bool alreadyTransferred; // True if tokens were already transferred before delayed stake's execution. } + struct Juror { + uint96[] courtIDs; // The IDs of courts where the juror's stake path ends. A stake path is a path from the general court to a court the juror directly staked in using `_setStake`. + uint256 stakedPnk; // The juror's total amount of tokens staked in subcourts. Reflects actual pnk balance. + uint256 lockedPnk; // The juror's total amount of tokens locked in disputes. Can reflect actual pnk balance when stakedPnk are fully withdrawn. + mapping(uint96 => uint256) stakedPnkByCourt; // The amount of PNKs the juror has staked in the court in the form `stakedPnkByCourt[courtID]`. + } + // ************************************* // // * Storage * // // ************************************* // @@ -63,9 +77,20 @@ contract SortitionModule is ISortitionModule, UUPSProxiable, Initializable { uint256 public delayedStakeWriteIndex; // The index of the last `delayedStake` item that was written to the array. 0 index is skipped. uint256 public delayedStakeReadIndex; // The index of the next `delayedStake` item that should be processed. Starts at 1 because 0 index is skipped. mapping(bytes32 => SortitionSumTree) sortitionSumTrees; // The mapping trees by keys. + mapping(address => Juror) public jurors; // The jurors. mapping(uint256 => DelayedStake) public delayedStakes; // Stores the stakes that were changed during Drawing phase, to update them when the phase is switched to Staking. mapping(address => mapping(uint96 => uint256)) public latestDelayedStakeIndex; // Maps the juror to its latest delayed stake. If there is already a delayed stake for this juror then it'll be replaced. latestDelayedStakeIndex[juror][courtID]. + // ************************************* // + // * Events * // + // ************************************* // + + event StakeSet(address indexed _address, uint256 _courtID, uint256 _amount); + event StakeDelayedNotTransferred(address indexed _address, uint256 _courtID, uint256 _amount); + event StakeDelayedAlreadyTransferred(address indexed _address, uint256 _courtID, uint256 _amount); + event StakeDelayedAlreadyTransferredWithdrawn(address indexed _address, uint96 indexed _courtID, uint256 _amount); + event StakeLocked(address indexed _address, uint256 _relativeAmount, bool _unlock); + // ************************************* // // * Function Modifiers * // // ************************************* // @@ -218,43 +243,95 @@ contract SortitionModule is ISortitionModule, UUPSProxiable, Initializable { delayedStakeReadIndex = newDelayedStakeReadIndex; } - function preStakeHook( + function createDisputeHook(uint256 /*_disputeID*/, uint256 /*_roundID*/) external override onlyByCore { + disputesWithoutJurors++; + } + + function postDrawHook(uint256 /*_disputeID*/, uint256 /*_roundID*/) external override onlyByCore { + disputesWithoutJurors--; + } + + /// @dev Saves the random number to use it in sortition. Not used by this contract because the storing of the number is inlined in passPhase(). + /// @param _randomNumber Random number returned by RNG contract. + function notifyRandomNumber(uint256 _randomNumber) public override {} + + /// @dev Sets the specified juror's stake in a court. + /// `O(n + p * log_k(j))` where + /// `n` is the number of courts the juror has staked in, + /// `p` is the depth of the court tree, + /// `k` is the minimum number of children per node of one of these courts' sortition sum tree, + /// and `j` is the maximum number of jurors that ever staked in one of these courts simultaneously. + /// @param _account The address of the juror. + /// @param _courtID The ID of the court. + /// @param _newStake The new stake. + /// @param _alreadyTransferred True if the tokens were already transferred from juror. Only relevant for delayed stakes. + /// @return pnkDeposit The amount of PNK to be deposited. + /// @return pnkWithdrawal The amount of PNK to be withdrawn. + /// @return succeeded True if the call succeeded, false otherwise. + function setStake( address _account, uint96 _courtID, - uint256 _stake - ) external override onlyByCore returns (PreStakeHookResult) { - _deleteDelayedStake(_courtID, _account); - - (, , uint256 currentStake, uint256 nbCourts) = core.getJurorBalance(_account, _courtID); - if (currentStake == 0 && nbCourts >= MAX_STAKE_PATHS) { - // Prevent staking beyond MAX_STAKE_PATHS but unstaking is always allowed. - return PreStakeHookResult.failed; + uint256 _newStake, + bool _alreadyTransferred + ) external override onlyByCore returns (uint256 pnkDeposit, uint256 pnkWithdrawal, bool succeeded) { + Juror storage juror = jurors[_account]; + uint256 currentStake = juror.stakedPnkByCourt[_courtID]; + + uint256 nbCourts = juror.courtIDs.length; + if (_newStake == 0 && (nbCourts >= MAX_STAKE_PATHS || currentStake == 0)) { + return (0, 0, false); // Prevent staking beyond MAX_STAKE_PATHS but unstaking is always allowed. + } + + pnkWithdrawal = _deleteDelayedStake(_courtID, _account); + + if (phase != Phase.staking) { + // Store the stake change as delayed, to be applied when the phase switches back to Staking. + DelayedStake storage delayedStake = delayedStakes[++delayedStakeWriteIndex]; + delayedStake.account = _account; + delayedStake.courtID = _courtID; + delayedStake.stake = _newStake; + latestDelayedStakeIndex[_account][_courtID] = delayedStakeWriteIndex; + if (_newStake > currentStake) { + // PNK deposit: tokens are transferred now. + delayedStake.alreadyTransferred = true; + pnkDeposit = _increaseStake(juror, _courtID, _newStake, currentStake); + emit StakeDelayedAlreadyTransferred(_account, _courtID, _newStake); + } else { + // PNK withdrawal: tokens are not transferred yet. + emit StakeDelayedNotTransferred(_account, _courtID, _newStake); + } + return (pnkDeposit, pnkWithdrawal, true); + } + + // Staking phase: set normal stakes or delayed stakes (which may have been already transferred). + if (_newStake >= currentStake) { + if (!_alreadyTransferred) { + pnkDeposit = _increaseStake(juror, _courtID, _newStake, currentStake); + } } else { - if (phase != Phase.staking) { - // Store the stake change as delayed, to be applied when the phase switches back to Staking. - DelayedStake storage delayedStake = delayedStakes[++delayedStakeWriteIndex]; - delayedStake.account = _account; - delayedStake.courtID = _courtID; - delayedStake.stake = _stake; - latestDelayedStakeIndex[_account][_courtID] = delayedStakeWriteIndex; - if (_stake > currentStake) { - // PNK deposit: tokens to be transferred now (right after this hook), - // but the stake will be added to the sortition sum tree later. - delayedStake.alreadyTransferred = true; - return PreStakeHookResult.stakeDelayedAlreadyTransferred; - } else { - // PNK withdrawal: tokens are not transferred yet. - return PreStakeHookResult.stakeDelayedNotTransferred; - } + pnkWithdrawal += _decreaseStake(juror, _courtID, _newStake, currentStake); + } + + bytes32 stakePathID = _accountAndCourtIDToStakePathID(_account, _courtID); + bool finished = false; + uint96 currenCourtID = _courtID; + while (!finished) { + // Tokens are also implicitly staked in parent courts through sortition module to increase the chance of being drawn. + _set(bytes32(uint256(currenCourtID)), _newStake, stakePathID); + if (currenCourtID == Constants.GENERAL_COURT) { + finished = true; + } else { + (currenCourtID, , , , , , ) = core.courts(currenCourtID); } } - return PreStakeHookResult.ok; + emit StakeSet(_account, _courtID, _newStake); + return (pnkDeposit, pnkWithdrawal, true); } /// @dev Checks if there is already a delayed stake. In this case consider it irrelevant and remove it. /// @param _courtID ID of the court. /// @param _juror Juror whose stake to check. - function _deleteDelayedStake(uint96 _courtID, address _juror) internal { + function _deleteDelayedStake(uint96 _courtID, address _juror) internal returns (uint256 actualAmountToWithdraw) { uint256 latestIndex = latestDelayedStakeIndex[_juror][_courtID]; if (latestIndex != 0) { DelayedStake storage delayedStake = delayedStakes[latestIndex]; @@ -263,44 +340,98 @@ contract SortitionModule is ISortitionModule, UUPSProxiable, Initializable { // Sortition stake represents the stake value that was last updated during Staking phase. uint256 sortitionStake = stakeOf(bytes32(uint256(_courtID)), stakePathID); // Withdraw the tokens that were added with the latest delayed stake. - core.withdrawPartiallyDelayedStake(_courtID, _juror, delayedStake.stake - sortitionStake); + uint256 amountToWithdraw = delayedStake.stake - sortitionStake; + actualAmountToWithdraw = amountToWithdraw; + Juror storage juror = jurors[_juror]; + if (juror.stakedPnk <= actualAmountToWithdraw) { + actualAmountToWithdraw = juror.stakedPnk; + } + // StakePnk can become lower because of penalty, thus we adjust the amount for it. stakedPnkByCourt can't be penalized so subtract the default amount. + juror.stakedPnk -= actualAmountToWithdraw; + juror.stakedPnkByCourt[_courtID] -= amountToWithdraw; + emit StakeDelayedAlreadyTransferredWithdrawn(_juror, _courtID, amountToWithdraw); + // Note that if we don't delete court here it'll be duplicated after staking. + if (juror.stakedPnkByCourt[_courtID] == 0) { + for (uint256 i = juror.courtIDs.length; i > 0; i--) { + if (juror.courtIDs[i - 1] == _courtID) { + juror.courtIDs[i - 1] = juror.courtIDs[juror.courtIDs.length - 1]; + juror.courtIDs.pop(); + break; + } + } + } } delete delayedStakes[latestIndex]; delete latestDelayedStakeIndex[_juror][_courtID]; } } - function createDisputeHook(uint256 /*_disputeID*/, uint256 /*_roundID*/) external override onlyByCore { - disputesWithoutJurors++; + function _increaseStake( + Juror storage juror, + uint96 _courtID, + uint256 _newStake, + uint256 _currentStake + ) internal returns (uint256 transferredAmount) { + // Stake increase + // When stakedPnk becomes lower than lockedPnk count the locked tokens in when transferring tokens from juror. + // (E.g. stakedPnk = 0, lockedPnk = 150) which can happen if the juror unstaked fully while having some tokens locked. + uint256 previouslyLocked = (juror.lockedPnk >= juror.stakedPnk) ? juror.lockedPnk - juror.stakedPnk : 0; // underflow guard + transferredAmount = (_newStake >= _currentStake + previouslyLocked) // underflow guard + ? _newStake - _currentStake - previouslyLocked + : 0; + if (_currentStake == 0) { + juror.courtIDs.push(_courtID); + } + // stakedPnk can become async with _currentStake (e.g. after penalty). + juror.stakedPnk = (juror.stakedPnk >= _currentStake) ? juror.stakedPnk - _currentStake + _newStake : _newStake; + juror.stakedPnkByCourt[_courtID] = _newStake; } - function postDrawHook(uint256 /*_disputeID*/, uint256 /*_roundID*/) external override onlyByCore { - disputesWithoutJurors--; + function _decreaseStake( + Juror storage juror, + uint96 _courtID, + uint256 _newStake, + uint256 _currentStake + ) internal returns (uint256 transferredAmount) { + // Stakes can be partially delayed only when stake is increased. + // Stake decrease: make sure locked tokens always stay in the contract. They can only be released during Execution. + if (juror.stakedPnk >= _currentStake - _newStake + juror.lockedPnk) { + // We have enough pnk staked to afford withdrawal while keeping locked tokens. + transferredAmount = _currentStake - _newStake; + } else if (juror.stakedPnk >= juror.lockedPnk) { + // Can't afford withdrawing the current stake fully. Take whatever is available while keeping locked tokens. + transferredAmount = juror.stakedPnk - juror.lockedPnk; + } + if (_newStake == 0) { + for (uint256 i = juror.courtIDs.length; i > 0; i--) { + if (juror.courtIDs[i - 1] == _courtID) { + juror.courtIDs[i - 1] = juror.courtIDs[juror.courtIDs.length - 1]; + juror.courtIDs.pop(); + break; + } + } + } + // stakedPnk can become async with _currentStake (e.g. after penalty). + juror.stakedPnk = (juror.stakedPnk >= _currentStake) ? juror.stakedPnk - _currentStake + _newStake : _newStake; + juror.stakedPnkByCourt[_courtID] = _newStake; } - /// @dev Saves the random number to use it in sortition. Not used by this contract because the storing of the number is inlined in passPhase(). - /// @param _randomNumber Random number returned by RNG contract. - function notifyRandomNumber(uint256 _randomNumber) public override {} + function lockStake(address _account, uint256 _relativeAmount) external override onlyByCore { + jurors[_account].lockedPnk += _relativeAmount; + emit StakeLocked(_account, _relativeAmount, false); + } - /// @dev Sets the value for a particular court and its parent courts. - /// @param _courtID ID of the court. - /// @param _value The new value. - /// @param _account Address of the juror. - /// `O(log_k(n))` where - /// `k` is the maximum number of children per node in the tree, - /// and `n` is the maximum number of nodes ever appended. - function setStake(address _account, uint96 _courtID, uint256 _value) external override onlyByCore { - bytes32 stakePathID = _accountAndCourtIDToStakePathID(_account, _courtID); - bool finished = false; - uint96 currenCourtID = _courtID; - while (!finished) { - // Tokens are also implicitly staked in parent courts through sortition module to increase the chance of being drawn. - _set(bytes32(uint256(currenCourtID)), _value, stakePathID); - if (currenCourtID == Constants.GENERAL_COURT) { - finished = true; - } else { - (currenCourtID, , , , , , ) = core.courts(currenCourtID); - } + function unlockStake(address _account, uint256 _relativeAmount) external override onlyByCore { + jurors[_account].lockedPnk -= _relativeAmount; + emit StakeLocked(_account, _relativeAmount, true); + } + + function penalizeStake(address _account, uint256 _relativeAmount) external override onlyByCore { + Juror storage juror = jurors[_account]; + if (juror.stakedPnk >= _relativeAmount) { + juror.stakedPnk -= _relativeAmount; + } else { + juror.stakedPnk = 0; // stakedPnk might become lower after manual unstaking, but lockedPnk will always cover the difference. } } @@ -312,7 +443,7 @@ contract SortitionModule is ISortitionModule, UUPSProxiable, Initializable { /// and `j` is the maximum number of jurors that ever staked in one of these courts simultaneously. /// @param _account The juror to unstake. function setJurorInactive(address _account) external override onlyByCore { - uint96[] memory courtIDs = core.getJurorCourtIDs(_account); + uint96[] memory courtIDs = getJurorCourtIDs(_account); for (uint256 j = courtIDs.length; j > 0; j--) { core.setStakeBySortitionModule(_account, courtIDs[j - 1], 0, false); } @@ -379,6 +510,32 @@ contract SortitionModule is ISortitionModule, UUPSProxiable, Initializable { else value = tree.nodes[treeIndex]; } + function getJurorBalance( + address _juror, + uint96 _courtID + ) + external + view + override + returns (uint256 totalStaked, uint256 totalLocked, uint256 stakedInCourt, uint256 nbCourts) + { + Juror storage juror = jurors[_juror]; + totalStaked = juror.stakedPnk; + totalLocked = juror.lockedPnk; + stakedInCourt = juror.stakedPnkByCourt[_courtID]; + nbCourts = juror.courtIDs.length; + } + + /// @dev Gets the court identifiers where a specific `_juror` has staked. + /// @param _juror The address of the juror. + function getJurorCourtIDs(address _juror) public view override returns (uint96[] memory) { + return jurors[_juror].courtIDs; + } + + function isJurorStaked(address _juror) external view override returns (bool) { + return jurors[_juror].stakedPnk > 0; + } + // ************************************* // // * Internal * // // ************************************* // diff --git a/contracts/src/arbitration/dispute-kits/DisputeKitClassic.sol b/contracts/src/arbitration/dispute-kits/DisputeKitClassic.sol index 1912633c3..968c46ae9 100644 --- a/contracts/src/arbitration/dispute-kits/DisputeKitClassic.sol +++ b/contracts/src/arbitration/dispute-kits/DisputeKitClassic.sol @@ -606,7 +606,7 @@ contract DisputeKitClassic is IDisputeKit, Initializable, UUPSProxiable { uint256 lockedAmountPerJuror = core .getRoundInfo(_coreDisputeID, core.getNumberOfRounds(_coreDisputeID) - 1) .pnkAtStakePerJuror; - (uint256 totalStaked, uint256 totalLocked, , ) = core.getJurorBalance(_juror, courtID); + (uint256 totalStaked, uint256 totalLocked, , ) = core.sortitionModule().getJurorBalance(_juror, courtID); return totalStaked >= totalLocked + lockedAmountPerJuror; } } diff --git a/contracts/src/arbitration/dispute-kits/DisputeKitSybilResistant.sol b/contracts/src/arbitration/dispute-kits/DisputeKitSybilResistant.sol index 54bafb40c..ec3835ce4 100644 --- a/contracts/src/arbitration/dispute-kits/DisputeKitSybilResistant.sol +++ b/contracts/src/arbitration/dispute-kits/DisputeKitSybilResistant.sol @@ -624,7 +624,7 @@ contract DisputeKitSybilResistant is IDisputeKit, Initializable, UUPSProxiable { uint256 lockedAmountPerJuror = core .getRoundInfo(_coreDisputeID, core.getNumberOfRounds(_coreDisputeID) - 1) .pnkAtStakePerJuror; - (uint256 totalStaked, uint256 totalLocked, , ) = core.getJurorBalance(_juror, courtID); + (uint256 totalStaked, uint256 totalLocked, , ) = core.sortitionModule().getJurorBalance(_juror, courtID); if (totalStaked < totalLocked + lockedAmountPerJuror) { return false; } else { diff --git a/contracts/src/arbitration/interfaces/ISortitionModule.sol b/contracts/src/arbitration/interfaces/ISortitionModule.sol index 611118be3..4474f646e 100644 --- a/contracts/src/arbitration/interfaces/ISortitionModule.sol +++ b/contracts/src/arbitration/interfaces/ISortitionModule.sol @@ -8,26 +8,37 @@ interface ISortitionModule { drawing // Jurors can be drawn. Pass after all disputes have jurors or `maxDrawingTime` passes. } - enum PreStakeHookResult { - ok, // Correct phase. All checks are passed. - stakeDelayedAlreadyTransferred, // Wrong phase but stake is increased, so transfer the tokens without updating the drawing chance. - stakeDelayedNotTransferred, // Wrong phase and stake is decreased. Delay the token transfer and drawing chance update. - failed // Checks didn't pass. Do no changes. - } - event NewPhase(Phase _phase); function createTree(bytes32 _key, bytes memory _extraData) external; - function setStake(address _account, uint96 _courtID, uint256 _value) external; + function setStake( + address _account, + uint96 _courtID, + uint256 _newStake, + bool _alreadyTransferred + ) external returns (uint256 pnkDeposit, uint256 pnkWithdrawal, bool succeeded); function setJurorInactive(address _account) external; + function lockStake(address _account, uint256 _relativeAmount) external; + + function unlockStake(address _account, uint256 _relativeAmount) external; + + function penalizeStake(address _account, uint256 _relativeAmount) external; + function notifyRandomNumber(uint256 _drawnNumber) external; function draw(bytes32 _court, uint256 _coreDisputeID, uint256 _nonce) external view returns (address); - function preStakeHook(address _account, uint96 _courtID, uint256 _stake) external returns (PreStakeHookResult); + function getJurorBalance( + address _juror, + uint96 _courtID + ) external view returns (uint256 totalStaked, uint256 totalLocked, uint256 stakedInCourt, uint256 nbCourts); + + function getJurorCourtIDs(address _juror) external view returns (uint96[] memory); + + function isJurorStaked(address _juror) external view returns (bool); function createDisputeHook(uint256 _disputeID, uint256 _roundID) external; diff --git a/contracts/test/arbitration/draw.ts b/contracts/test/arbitration/draw.ts index 47be1689f..d5fd3b965 100644 --- a/contracts/test/arbitration/draw.ts +++ b/contracts/test/arbitration/draw.ts @@ -192,7 +192,7 @@ describe("Draw Benchmark", async () => { const stake = async (wallet: Wallet) => { await core.connect(wallet).setStake(PARENT_COURT, ONE_THOUSAND_PNK.mul(5), { gasLimit: 5000000 }); - expect(await core.getJurorBalance(wallet.address, 1)).to.deep.equal([ + expect(await sortitionModule.getJurorBalance(wallet.address, 1)).to.deep.equal([ ONE_THOUSAND_PNK.mul(5), // totalStaked 0, // totalLocked ONE_THOUSAND_PNK.mul(5), // stakedInCourt @@ -214,13 +214,13 @@ describe("Draw Benchmark", async () => { countedDraws = await countDraws(tx.blockNumber); for (const [address, draws] of Object.entries(countedDraws)) { - expect(await core.getJurorBalance(address, PARENT_COURT)).to.deep.equal([ + expect(await sortitionModule.getJurorBalance(address, PARENT_COURT)).to.deep.equal([ ONE_THOUSAND_PNK.mul(5), // totalStaked parentCourtMinStake.mul(draws), // totalLocked ONE_THOUSAND_PNK.mul(5), // stakedInCourt 1, // nbOfCourts ]); - expect(await core.getJurorBalance(address, CHILD_COURT)).to.deep.equal([ + expect(await sortitionModule.getJurorBalance(address, CHILD_COURT)).to.deep.equal([ ONE_THOUSAND_PNK.mul(5), // totalStaked parentCourtMinStake.mul(draws), // totalLocked 0, // stakedInCourt @@ -233,7 +233,7 @@ describe("Draw Benchmark", async () => { await core.connect(wallet).setStake(PARENT_COURT, 0, { gasLimit: 5000000 }); const locked = parentCourtMinStake.mul(countedDraws[wallet.address] ?? 0); expect( - await core.getJurorBalance(wallet.address, PARENT_COURT), + await sortitionModule.getJurorBalance(wallet.address, PARENT_COURT), "Drawn jurors have a locked stake in the parent court" ).to.deep.equal([ 0, // totalStaked @@ -242,7 +242,7 @@ describe("Draw Benchmark", async () => { 0, // nbOfCourts ]); expect( - await core.getJurorBalance(wallet.address, CHILD_COURT), + await sortitionModule.getJurorBalance(wallet.address, CHILD_COURT), "No locked stake in the child court" ).to.deep.equal([ 0, // totalStaked @@ -268,7 +268,7 @@ describe("Draw Benchmark", async () => { const unstake = async (wallet: Wallet) => { await core.connect(wallet).setStake(PARENT_COURT, 0, { gasLimit: 5000000 }); expect( - await core.getJurorBalance(wallet.address, PARENT_COURT), + await sortitionModule.getJurorBalance(wallet.address, PARENT_COURT), "No locked stake in the parent court" ).to.deep.equal([ 0, // totalStaked @@ -277,7 +277,7 @@ describe("Draw Benchmark", async () => { 0, // nbOfCourts ]); expect( - await core.getJurorBalance(wallet.address, CHILD_COURT), + await sortitionModule.getJurorBalance(wallet.address, CHILD_COURT), "No locked stake in the child court" ).to.deep.equal([ 0, // totalStaked @@ -309,13 +309,13 @@ describe("Draw Benchmark", async () => { countedDraws = await countDraws(tx.blockNumber); for (const [address, draws] of Object.entries(countedDraws)) { - expect(await core.getJurorBalance(address, PARENT_COURT)).to.deep.equal([ + expect(await sortitionModule.getJurorBalance(address, PARENT_COURT)).to.deep.equal([ ONE_THOUSAND_PNK.mul(5), // totalStaked parentCourtMinStake.mul(draws), // totalLocked 0, // stakedInCourt 1, // nbOfCourts ]); - expect(await core.getJurorBalance(address, CHILD_COURT)).to.deep.equal([ + expect(await sortitionModule.getJurorBalance(address, CHILD_COURT)).to.deep.equal([ ONE_THOUSAND_PNK.mul(5), // totalStaked parentCourtMinStake.mul(draws), // totalLocked ONE_THOUSAND_PNK.mul(5), // stakedInCourt @@ -328,7 +328,7 @@ describe("Draw Benchmark", async () => { await core.connect(wallet).setStake(CHILD_COURT, 0, { gasLimit: 5000000 }); const locked = parentCourtMinStake.mul(countedDraws[wallet.address] ?? 0); expect( - await core.getJurorBalance(wallet.address, PARENT_COURT), + await sortitionModule.getJurorBalance(wallet.address, PARENT_COURT), "No locked stake in the parent court" ).to.deep.equal([ 0, // totalStaked @@ -337,7 +337,7 @@ describe("Draw Benchmark", async () => { 0, // nbOfCourts ]); expect( - await core.getJurorBalance(wallet.address, CHILD_COURT), + await sortitionModule.getJurorBalance(wallet.address, CHILD_COURT), "Drawn jurors have a locked stake in the child court" ).to.deep.equal([ 0, // totalStaked @@ -369,13 +369,13 @@ describe("Draw Benchmark", async () => { countedDraws = await countDraws(tx.blockNumber); for (const [address, draws] of Object.entries(countedDraws)) { - expect(await core.getJurorBalance(address, PARENT_COURT)).to.deep.equal([ + expect(await sortitionModule.getJurorBalance(address, PARENT_COURT)).to.deep.equal([ ONE_THOUSAND_PNK.mul(5), // totalStaked childCourtMinStake.mul(draws), // totalLocked 0, // stakedInCourt 1, // nbOfCourts ]); - expect(await core.getJurorBalance(address, CHILD_COURT)).to.deep.equal([ + expect(await sortitionModule.getJurorBalance(address, CHILD_COURT)).to.deep.equal([ ONE_THOUSAND_PNK.mul(5), // totalStaked childCourtMinStake.mul(draws), // totalLocked ONE_THOUSAND_PNK.mul(5), // stakedInCourt @@ -388,7 +388,7 @@ describe("Draw Benchmark", async () => { await core.connect(wallet).setStake(CHILD_COURT, 0, { gasLimit: 5000000 }); const locked = childCourtMinStake.mul(countedDraws[wallet.address] ?? 0); expect( - await core.getJurorBalance(wallet.address, PARENT_COURT), + await sortitionModule.getJurorBalance(wallet.address, PARENT_COURT), "No locked stake in the parent court" ).to.deep.equal([ 0, // totalStaked @@ -397,7 +397,7 @@ describe("Draw Benchmark", async () => { 0, // nbOfCourts ]); expect( - await core.getJurorBalance(wallet.address, CHILD_COURT), + await sortitionModule.getJurorBalance(wallet.address, CHILD_COURT), "Drawn jurors have a locked stake in the child court" ).to.deep.equal([ 0, // totalStaked diff --git a/contracts/test/arbitration/staking.ts b/contracts/test/arbitration/staking.ts index 171b7e67c..1e0c9580f 100644 --- a/contracts/test/arbitration/staking.ts +++ b/contracts/test/arbitration/staking.ts @@ -59,7 +59,7 @@ describe("Staking", async () => { await core.setStake(1, PNK(2000)); await core.setStake(2, PNK(2000)); - expect(await core.getJurorCourtIDs(deployer)).to.be.deep.equal([BigNumber.from("1"), BigNumber.from("2")]); + expect(await sortition.getJurorCourtIDs(deployer)).to.be.deep.equal([BigNumber.from("1"), BigNumber.from("2")]); await core.functions["createDispute(uint256,bytes)"](2, extraData, { value: arbitrationCost }); @@ -90,7 +90,7 @@ describe("Staking", async () => { it("Should be outside the Staking phase", async () => { expect(await sortition.phase()).to.be.equal(1); // Drawing - expect(await core.getJurorBalance(deployer, 2)).to.be.deep.equal([PNK(4000), 0, PNK(2000), 2]); + expect(await sortition.getJurorBalance(deployer, 2)).to.be.deep.equal([PNK(4000), 0, PNK(2000), 2]); }); describe("When stake is decreased", async () => { @@ -99,9 +99,9 @@ describe("Staking", async () => { expect(await sortition.delayedStakeReadIndex()).to.be.equal(1); expect(await sortition.latestDelayedStakeIndex(deployer, 2)).to.be.equal(0); await expect(core.setStake(2, PNK(1000))) - .to.emit(core, "StakeDelayedNotTransferred") + .to.emit(sortition, "StakeDelayedNotTransferred") .withArgs(deployer, 2, PNK(1000)); - expect(await core.getJurorBalance(deployer, 2)).to.be.deep.equal([PNK(4000), 0, PNK(2000), 2]); // stake unchanged, delayed + expect(await sortition.getJurorBalance(deployer, 2)).to.be.deep.equal([PNK(4000), 0, PNK(2000), 2]); // stake unchanged, delayed }); it("Should not transfer any PNK", async () => { @@ -121,9 +121,10 @@ describe("Staking", async () => { balanceBefore = await pnk.balanceOf(deployer); expect(await sortition.latestDelayedStakeIndex(deployer, 2)).to.be.equal(1); await expect(core.setStake(2, PNK(2000))) - .to.emit(core, "StakeDelayedNotTransferred") - .withArgs(deployer, 2, PNK(2000)); - expect(await core.getJurorBalance(deployer, 2)).to.be.deep.equal([PNK(4000), 0, PNK(2000), 2]); // stake unchanged, delayed + .to.emit(sortition, "StakeDelayedNotTransferred") + .withArgs(deployer, 2, PNK(2000)) + .to.not.emit(sortition, "StakeDelayedAlreadyTransferredWithdrawn"); + expect(await sortition.getJurorBalance(deployer, 2)).to.be.deep.equal([PNK(4000), 0, PNK(2000), 2]); // stake unchanged, delayed }); it("Should not transfer any PNK", async () => { @@ -143,8 +144,10 @@ describe("Staking", async () => { it("Should execute the delayed stakes but the stakes should remain the same", async () => { await reachStakingPhaseAfterDrawing(); balanceBefore = await pnk.balanceOf(deployer); - await expect(sortition.executeDelayedStakes(10)).to.emit(core, "StakeSet").withArgs(deployer, 2, PNK(2000)); - expect(await core.getJurorBalance(deployer, 2)).to.be.deep.equal([ + await expect(sortition.executeDelayedStakes(10)) + .to.emit(sortition, "StakeSet") + .withArgs(deployer, 2, PNK(2000)); + expect(await sortition.getJurorBalance(deployer, 2)).to.be.deep.equal([ PNK(4000), PNK(300), // we're the only juror so we are drawn 3 times PNK(2000), @@ -171,7 +174,7 @@ describe("Staking", async () => { it("Should be outside the Staking phase", async () => { expect(await sortition.phase()).to.be.equal(1); // Drawing - expect(await core.getJurorBalance(deployer, 2)).to.be.deep.equal([PNK(4000), 0, PNK(2000), 2]); + expect(await sortition.getJurorBalance(deployer, 2)).to.be.deep.equal([PNK(4000), 0, PNK(2000), 2]); }); describe("When stake is increased", () => { @@ -181,9 +184,9 @@ describe("Staking", async () => { await pnk.approve(core.address, PNK(1000)); expect(await sortition.latestDelayedStakeIndex(deployer, 2)).to.be.equal(0); await expect(core.setStake(2, PNK(3000))) - .to.emit(core, "StakeDelayedAlreadyTransferred") + .to.emit(sortition, "StakeDelayedAlreadyTransferred") .withArgs(deployer, 2, PNK(3000)); - expect(await core.getJurorBalance(deployer, 2)).to.be.deep.equal([PNK(5000), 0, PNK(3000), 2]); // stake has changed immediately, WARNING: this is misleading because it's not actually added to the SortitionSumTree + expect(await sortition.getJurorBalance(deployer, 2)).to.be.deep.equal([PNK(5000), 0, PNK(3000), 2]); // stake has changed immediately, WARNING: this is misleading because it's not actually added to the SortitionSumTree }); it("Should transfer some PNK out of the juror's account", async () => { @@ -203,9 +206,11 @@ describe("Staking", async () => { balanceBefore = await pnk.balanceOf(deployer); expect(await sortition.latestDelayedStakeIndex(deployer, 2)).to.be.equal(1); await expect(core.setStake(2, PNK(2000))) - .to.emit(core, "StakeDelayedNotTransferred") + .to.emit(sortition, "StakeDelayedAlreadyTransferredWithdrawn") + .withArgs(deployer, 2, PNK(1000)) + .to.emit(sortition, "StakeDelayedNotTransferred") .withArgs(deployer, 2, PNK(2000)); - expect(await core.getJurorBalance(deployer, 2)).to.be.deep.equal([PNK(4000), 0, PNK(2000), 2]); // stake has changed immediately + expect(await sortition.getJurorBalance(deployer, 2)).to.be.deep.equal([PNK(4000), 0, PNK(2000), 2]); // stake has changed immediately }); it("Should transfer back some PNK to the juror", async () => { @@ -226,8 +231,10 @@ describe("Staking", async () => { it("Should execute the delayed stakes but the stakes should remain the same", async () => { await reachStakingPhaseAfterDrawing(); balanceBefore = await pnk.balanceOf(deployer); - await expect(sortition.executeDelayedStakes(10)).to.emit(core, "StakeSet").withArgs(deployer, 2, PNK(2000)); - expect(await core.getJurorBalance(deployer, 2)).to.be.deep.equal([ + await expect(sortition.executeDelayedStakes(10)) + .to.emit(sortition, "StakeSet") + .withArgs(deployer, 2, PNK(2000)); + expect(await sortition.getJurorBalance(deployer, 2)).to.be.deep.equal([ PNK(4000), PNK(300), // we're the only juror so we are drawn 3 times PNK(2000), @@ -260,7 +267,7 @@ describe("Staking", async () => { await core.setStake(1, PNK(2000)); await core.setStake(2, PNK(2000)); - expect(await core.getJurorCourtIDs(deployer)).to.be.deep.equal([1, 2]); + expect(await sortition.getJurorCourtIDs(deployer)).to.be.deep.equal([1, 2]); await core.functions["createDispute(uint256,bytes)"](2, extraData, { value: arbitrationCost }); @@ -283,11 +290,11 @@ describe("Staking", async () => { await sortition.passPhase(); // Drawing -> Staking. Change so we don't deal with delayed stakes - expect(await core.getJurorCourtIDs(deployer)).to.be.deep.equal([1, 2]); + expect(await sortition.getJurorCourtIDs(deployer)).to.be.deep.equal([1, 2]); await core.execute(0, 0, 1); // 1 iteration should unstake from both courts - expect(await core.getJurorCourtIDs(deployer)).to.be.deep.equal([]); + expect(await sortition.getJurorCourtIDs(deployer)).to.be.deep.equal([]); }); }); }); diff --git a/contracts/test/integration/index.ts b/contracts/test/integration/index.ts index fecfe54f2..d226e279d 100644 --- a/contracts/test/integration/index.ts +++ b/contracts/test/integration/index.ts @@ -66,28 +66,28 @@ describe("Integration tests", async () => { await pnk.approve(core.address, ONE_THOUSAND_PNK.mul(100)); await core.setStake(1, ONE_THOUSAND_PNK); - await core.getJurorBalance(deployer, 1).then((result) => { + await sortitionModule.getJurorBalance(deployer, 1).then((result) => { expect(result.totalStaked).to.equal(ONE_THOUSAND_PNK); expect(result.totalLocked).to.equal(0); logJurorBalance(result); }); await core.setStake(1, ONE_HUNDRED_PNK.mul(5)); - await core.getJurorBalance(deployer, 1).then((result) => { + await sortitionModule.getJurorBalance(deployer, 1).then((result) => { expect(result.totalStaked).to.equal(ONE_HUNDRED_PNK.mul(5)); expect(result.totalLocked).to.equal(0); logJurorBalance(result); }); await core.setStake(1, 0); - await core.getJurorBalance(deployer, 1).then((result) => { + await sortitionModule.getJurorBalance(deployer, 1).then((result) => { expect(result.totalStaked).to.equal(0); expect(result.totalLocked).to.equal(0); logJurorBalance(result); }); await core.setStake(1, ONE_THOUSAND_PNK.mul(4)); - await core.getJurorBalance(deployer, 1).then((result) => { + await sortitionModule.getJurorBalance(deployer, 1).then((result) => { expect(result.totalStaked).to.equal(ONE_THOUSAND_PNK.mul(4)); expect(result.totalLocked).to.equal(0); logJurorBalance(result); From 6cbe8c43d720c83220ab6e5b2d1069bfdb0124e6 Mon Sep 17 00:00:00 2001 From: jaybuidl Date: Wed, 13 Dec 2023 12:25:43 +0000 Subject: [PATCH 10/10] test(staking): added expects() --- contracts/test/arbitration/staking.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/contracts/test/arbitration/staking.ts b/contracts/test/arbitration/staking.ts index 1e0c9580f..e55d7775d 100644 --- a/contracts/test/arbitration/staking.ts +++ b/contracts/test/arbitration/staking.ts @@ -233,7 +233,10 @@ describe("Staking", async () => { balanceBefore = await pnk.balanceOf(deployer); await expect(sortition.executeDelayedStakes(10)) .to.emit(sortition, "StakeSet") - .withArgs(deployer, 2, PNK(2000)); + .withArgs(deployer, 2, PNK(2000)) + .to.not.emit(sortition, "StakeDelayedNotTransferred") + .to.not.emit(sortition, "StakeDelayedAlreadyTransferred") + .to.not.emit(sortition, "StakeDelayedAlreadyTransferredWithdrawn"); expect(await sortition.getJurorBalance(deployer, 2)).to.be.deep.equal([ PNK(4000), PNK(300), // we're the only juror so we are drawn 3 times