fix(contracts): update jail expiration timestamp correctly (#376)

kroma-network · Sep 2, 2024 · b83718c · b83718c
1 parent 87fc8ab
commit b83718c
Show file tree

Hide file tree

Showing 3 changed files with 73 additions and 8 deletions.
diff --git a/packages/contracts/contracts/L1/ValidatorManager.sol b/packages/contracts/contracts/L1/ValidatorManager.sol
@@ -355,13 +355,22 @@ contract ValidatorManager is ISemver, IValidatorManager {
 
  emit SlashReverted(outputIndex, loser, challengeReward);
 
- // Unjail the original loser.
- delete _jail[loser];
+ if (inJail(loser)) {
+ // Revert jail expiration timestamp of the original loser.
+ uint128 expiresAt = _jail[loser] - HARD_JAIL_PERIOD_SECONDS;
+ if (block.timestamp < expiresAt) {
+ _jail[loser] = expiresAt;
 
- emit ValidatorUnjailed(loser);
+ emit ValidatorJailed(loser, expiresAt);
+ } else {
+ delete _jail[loser];
+
+ emit ValidatorUnjailed(loser);
 
- if (getStatus(loser) == ValidatorStatus.READY) {
- _activateValidator(loser);
+ if (getStatus(loser) == ValidatorStatus.READY) {
+ _activateValidator(loser);
+ }
+ }
  }
  }
 
@@ -709,9 +718,7 @@ contract ValidatorManager is ISemver, IValidatorManager {
  */
  function _sendToJail(address validator, bool isSoft) private {
  uint128 jailSeconds = isSoft ? SOFT_JAIL_PERIOD_SECONDS : HARD_JAIL_PERIOD_SECONDS;
- uint128 expiresAt = inJail(validator)
- ? _jail[validator] + jailSeconds
-  : uint128(block.timestamp) + jailSeconds;
+ uint128 expiresAt = _jail[validator].max(uint128(block.timestamp)) + jailSeconds;
  _jail[validator] = expiresAt;
 
  emit ValidatorJailed(validator, expiresAt);

diff --git a/packages/contracts/contracts/libraries/Uint128Math.sol b/packages/contracts/contracts/libraries/Uint128Math.sol
@@ -7,6 +7,13 @@ pragma solidity 0.8.15;
  * This library is motivated from the open-source Openzeppelin's Math library.
  */
 library Uint128Math {
+ /**
+ * @dev Returns the largest of two numbers.
+ */
+ function max(uint128 a, uint128 b) internal pure returns (uint128) {
+ return a > b ? a : b;
+ }
+
  /**
  * @notice Calculates floor(x * y / denominator) with full precision. Throws if result overflows a uint128 or denominator == 0
  * @dev Original credit to Remco Bloemen under MIT license (https://xn--2-umb.com/21/muldiv)

diff --git a/packages/contracts/contracts/test/ValidatorManager.t.sol b/packages/contracts/contracts/test/ValidatorManager.t.sol
@@ -62,6 +62,10 @@ contract MockValidatorManager is ValidatorManager {
  _nextPriorityValidator = validator;
  }
 
+ function setJailExpiresAt(address validator, uint128 expiresAt) external {
+ _jail[validator] = expiresAt;
+ }
+
  function nextPriorityValidator() external view returns (address) {
  return _nextPriorityValidator;
  }
@@ -819,6 +823,27 @@ contract ValidatorManagerTest is ValidatorSystemUpgrade_Initializer {
  );
  }
 
+ function test_slash_alreadyInJail_succeeds() external {
+ _registerValidator(asserter, minActivateAmount);
+
+ // Submit the first output which interacts with ValidatorManager
+ mockValMgr.updatePriorityValidator(asserter);
+ warpToSubmitTime();
+ _submitL2OutputV2(false);
+ uint256 challengedOutputIndex = oracle.latestOutputIndex();
+
+ // Before slashed, send to jail
+ uint128 firstJailExpiresAt = uint128(block.timestamp) + softJailPeriodSeconds;
+ mockValMgr.setJailExpiresAt(asserter, firstJailExpiresAt);
+
+ // After jail expired, slash
+ vm.warp(firstJailExpiresAt + 1);
+ vm.prank(address(colosseum));
+ valMgr.slash(challengedOutputIndex, challenger, asserter);
+
+ assertEq(valMgr.jailExpiresAt(asserter), firstJailExpiresAt + 1 + hardJailPeriodSeconds);
+ }
+
  function test_slash_notColosseum_reverts() external {
  vm.prank(address(1));
  vm.expectRevert(IValidatorManager.NotAllowedCaller.selector);
@@ -876,6 +901,32 @@ contract ValidatorManagerTest is ValidatorSystemUpgrade_Initializer {
  assertTrue(valMgr.getStatus(asserter) == IValidatorManager.ValidatorStatus.ACTIVE);
  }
 
+ function test_revertSlash_stillInJail_succeeds() external {
+ _registerValidator(asserter, minActivateAmount);
+
+ // Submit the first output which interacts with ValidatorManager
+ mockValMgr.updatePriorityValidator(asserter);
+ warpToSubmitTime();
+ _submitL2OutputV2(false);
+ uint256 challengedOutputIndex = oracle.latestOutputIndex();
+
+ // Before slashed, send to jail
+ uint128 firstJailExpiresAt = uint128(block.timestamp) + softJailPeriodSeconds;
+ mockValMgr.setJailExpiresAt(asserter, firstJailExpiresAt);
+
+ // Before jail expired, slash
+ vm.prank(address(colosseum));
+ valMgr.slash(challengedOutputIndex, challenger, asserter);
+
+ assertEq(valMgr.jailExpiresAt(asserter), firstJailExpiresAt + hardJailPeriodSeconds);
+
+ // Revert slash
+ vm.prank(address(colosseum));
+ valMgr.revertSlash(challengedOutputIndex, asserter);
+
+ assertEq(valMgr.jailExpiresAt(asserter), firstJailExpiresAt);
+ }
+
  function test_revertSlash_notColosseum_reverts() external {
  vm.prank(trusted);
  vm.expectRevert(IValidatorManager.NotAllowedCaller.selector);