Exit Window Period for add/remove sanctions (#325)

* Implement sanction expiry and exit window periods in KintoID contract, update related tests, and add new error handling for exit window period violations.

* Add tests for sanction management in KintoID, including adding and removing sanctions with time warp simulation.

* Add sanction expiry and exit window periods; update contract to KintoIDV10 and fix typo in IKintoID interface.

* Upgrade KintoID to version V10, update contract addresses, and modify governance role assertions in migration script.

script/migrations/135-upgrade_faucet_id.s.sol

@@ -11,32 +11,18 @@ contract UpgradeKintoIDScript is MigrationHelper {
 
         bytes memory bytecode = abi.encodePacked(
             type(KintoID).creationCode,
-            abi.encode(
-                _getChainDeployment("KintoWalletFactory"),
-                _getChainDeployment("Faucet")
-            )
+            abi.encode(_getChainDeployment("KintoWalletFactory"), _getChainDeployment("Faucet"))
         );
 
-        address impl = _deployImplementationAndUpgrade("KintoID", "V9", bytecode);
-        saveContractAddress("KintoIDV9-impl", impl);
+        address impl = _deployImplementationAndUpgrade("KintoID", "V10", bytecode);
+        saveContractAddress("KintoIDV10-impl", impl);
 
         KintoID kintoID = KintoID(_getChainDeployment("KintoID"));
         address nioGovernor = _getChainDeployment("NioGovernor");
-        bytes32 governanceRole = kintoID.GOVERNANCE_ROLE(); 
+        bytes32 governanceRole = kintoID.GOVERNANCE_ROLE();
 
-        assertFalse(kintoID.hasRole(governanceRole, kintoAdminWallet));
-        assertFalse(kintoID.hasRole(governanceRole, nioGovernor));
-
-        _handleOps(
-            abi.encodeWithSelector(IAccessControl.grantRole.selector, governanceRole, kintoAdminWallet), address(kintoID)
-        );
-
-        _handleOps(
-            abi.encodeWithSelector(IAccessControl.grantRole.selector, governanceRole, nioGovernor), address(kintoID)
-        );
-
-        assertTrue(kintoID.hasRole(kintoID.GOVERNANCE_ROLE(), kintoAdminWallet));
-        assertTrue(kintoID.hasRole(kintoID.GOVERNANCE_ROLE(), nioGovernor));
+        assertTrue(kintoID.hasRole(governanceRole, kintoAdminWallet));
+        assertTrue(kintoID.hasRole(governanceRole, nioGovernor));
 
         assertTrue(kintoID.isKYC(deployer));
     }

src/KintoID.sol

@@ -92,6 +92,12 @@ contract KintoID is
     /// @notice Role identifier for governance actions
     bytes32 public constant override GOVERNANCE_ROLE = keccak256("GOVERNANCE_ROLE");
 
+    /// @notice The period of time after which sanction is expired unless approved by governance
+    uint256 public constant SANCTION_EXPIRY_PERIOD = 3 days;
+
+    /// @notice The period of time during which additional sanctions can't be applied and user can exit, unless sanctions approved by governance
+    uint256 public constant EXIT_WINDOW_PERIOD = 10 days;
+
     /// @notice Address of the wallet factory contract
     address public immutable override walletFactory;
 
@@ -384,6 +390,13 @@ contract KintoID is
      */
     function addSanction(address _account, uint16 _countryId) public override onlyRole(KYC_PROVIDER_ROLE) {
         if (balanceOf(_account) == 0) revert KYCRequired();
+
+        // Check if account is in protection period (10 days from last sanction)
+        uint256 lastSanctionTime = sanctionedAt[_account];
+        if (lastSanctionTime != 0 && block.timestamp - lastSanctionTime < EXIT_WINDOW_PERIOD) {
+            revert ExitWindowPeriod(_account, lastSanctionTime);
+        }
+
         Metadata storage meta = _kycmetas[_account];
         if (!meta.sanctions.get(_countryId)) {
             meta.sanctions.set(_countryId);
@@ -405,13 +418,23 @@ contract KintoID is
      */
     function removeSanction(address _account, uint16 _countryId) public override onlyRole(KYC_PROVIDER_ROLE) {
         if (balanceOf(_account) == 0) revert KYCRequired();
+
+        // Check if account is in protection period (10 days from last sanction)
+        uint256 lastSanctionTime = sanctionedAt[_account];
+        if (lastSanctionTime != 0 && block.timestamp - lastSanctionTime < EXIT_WINDOW_PERIOD) {
+            revert ExitWindowPeriod(_account, lastSanctionTime);
+        }
+
         Metadata storage meta = _kycmetas[_account];
         if (meta.sanctions.get(_countryId)) {
             meta.sanctions.unset(_countryId);
             meta.sanctionsCount -= 1;
             meta.updatedAt = block.timestamp;
             lastMonitoredAt = block.timestamp;
             emit SanctionRemoved(_account, _countryId, block.timestamp);
+
+            // Reset sanction timestamp
+            sanctionedAt[_account] = 0;
         }
     }
 
@@ -438,32 +461,32 @@ contract KintoID is
 
     /**
      * @notice Checks if an account has active sanctions
-     * @dev Account is considered safe if sanctions are not confirmed within 3 days
+     * @dev Account is considered safe if sanctions are not confirmed within SANCTION_EXPIRY_PERIOD
      * @param _account Address to check
      * @return bool True if the account has no active sanctions
      */
     function isSanctionsSafe(address _account) public view virtual override returns (bool) {
-        // If the sanction is not confirmed within 3 days, consider the account sanctions safe
+        // If the sanction is not confirmed within SANCTION_EXPIRY_PERIOD, consider the account sanctions safe
         return isSanctionsMonitored(7)
             && (
                 _kycmetas[_account].sanctionsCount == 0
-                    || (sanctionedAt[_account] != 0 && (block.timestamp - sanctionedAt[_account]) > 3 days)
+                    || (sanctionedAt[_account] != 0 && (block.timestamp - sanctionedAt[_account]) > SANCTION_EXPIRY_PERIOD)
             );
     }
 
     /**
      * @notice Checks if an account is sanctioned in a specific country
-     * @dev Account is considered safe if sanction is not confirmed within 3 days
+     * @dev Account is considered safe if sanction is not confirmed within SANCTION_EXPIRY_PERIOD
      * @param _account Address to check
      * @param _countryId ID of the country to check sanctions for
      * @return bool True if the account is not sanctioned in the specified country
      */
     function isSanctionsSafeIn(address _account, uint16 _countryId) external view virtual override returns (bool) {
-        // If the sanction is not confirmed within 3 days, consider the account sanctions safe
+        // If the sanction is not confirmed within SANCTION_EXPIRY_PERIOD, consider the account sanctions safe
         return isSanctionsMonitored(7)
             && (
                 !_kycmetas[_account].sanctions.get(_countryId)
-                    || (sanctionedAt[_account] != 0 && (block.timestamp - sanctionedAt[_account]) > 3 days)
+                    || (sanctionedAt[_account] != 0 && (block.timestamp - sanctionedAt[_account]) > SANCTION_EXPIRY_PERIOD)
             );
     }
 
@@ -629,6 +652,6 @@ contract KintoID is
     }
 }
 
-contract KintoIDV9 is KintoID {
+contract KintoIDV10 is KintoID {
     constructor(address _walletFactory, address _faucet) KintoID(_walletFactory, _faucet) {}
 }

src/interfaces/IKintoID.sol

@@ -45,6 +45,9 @@ interface IKintoID {
     /// @notice Thrown when attempting unauthorized token transfers
     error OnlyMintBurnOrTransfer();
 
+    /// @notice Thrown when attempting to add or removing sanctions during exit widnow period
+    error ExitWindowPeriod(address user, uint256 sanctionedAt);
+
     /* ============ Structs ============ */
 
     struct Metadata {

test/artifacts/7887/addresses.json

@@ -244,5 +244,6 @@
   "AAVE": "0xaa0e00F095Eb986CB65FD3FA328782c7Fe4ceFD9",
   "KintoAppRegistryV22": "0xb9cE6BC89b79c713f34fd15D82a70900fEFD0de1",
   "KintoIDV9-impl": "0x7CFe474936fA50181ae7c2C43EeB8806e25bc983",
-  "SponsorPaymasterV15-impl": "0x2A10b80bE8Ee546C52Fde9b58d65D089C6B929BB"
-}
+  "SponsorPaymasterV15-impl": "0x2A10b80bE8Ee546C52Fde9b58d65D089C6B929BB",
+  "KintoIDV10-impl": "0xaa0726829d41E3C70B84Bc5390cce82afC56871A"
+}

test/unit/KintoID.t.sol

@@ -278,18 +278,36 @@ contract KintoIDTest is SharedSetup {
         updates[0] = new IKintoID.MonitorUpdateData[](4);
         updates[0][0] = IKintoID.MonitorUpdateData(true, true, 5); // add trait 5
         updates[0][1] = IKintoID.MonitorUpdateData(true, false, 1); // remove trait 1
-        updates[0][2] = IKintoID.MonitorUpdateData(false, true, 6); // add sanction 6
-        updates[0][3] = IKintoID.MonitorUpdateData(false, false, 2); // remove sanction 2
+        updates[0][2] = IKintoID.MonitorUpdateData(true, true, 6); // add trait 6
+        updates[0][3] = IKintoID.MonitorUpdateData(true, false, 2); // remove trait 2
 
         vm.prank(_kycProvider);
         _kintoID.monitor(accounts, updates);
 
         assertEq(_kintoID.hasTrait(_user, 5), true);
         assertEq(_kintoID.hasTrait(_user, 1), false);
-        assertEq(_kintoID.isSanctionsSafeIn(_user, 5), true);
-        assertEq(_kintoID.isSanctionsSafeIn(_user, 1), true);
-        assertEq(_kintoID.isSanctionsSafeIn(_user, 6), false);
-        assertEq(_kintoID.isSanctionsSafeIn(_user, 2), true);
+        assertEq(_kintoID.hasTrait(_user, 6), true);
+        assertEq(_kintoID.hasTrait(_user, 2), false);
+
+        updates = new IKintoID.MonitorUpdateData[][](1);
+        updates[0] = new IKintoID.MonitorUpdateData[](1);
+        updates[0][0] = IKintoID.MonitorUpdateData(false, true, 3); // add sanction 3
+
+        vm.prank(_kycProvider);
+        _kintoID.monitor(accounts, updates);
+
+        assertEq(_kintoID.isSanctionsSafeIn(_user, 3), false);
+
+        vm.warp(block.timestamp + 10 days);
+
+        updates = new IKintoID.MonitorUpdateData[][](1);
+        updates[0] = new IKintoID.MonitorUpdateData[](1);
+        updates[0][0] = IKintoID.MonitorUpdateData(false, false, 3); // remove sanction 3
+
+        vm.prank(_kycProvider);
+        _kintoID.monitor(accounts, updates);
+
+        assertEq(_kintoID.isSanctionsSafeIn(_user, 3), true);
     }
 
     /* ============ Trait tests ============ */
@@ -375,11 +393,10 @@ contract KintoIDTest is SharedSetup {
     /* ============ Sanction tests ============ */
 
     function testAddSanction() public {
+        addKYC();
+
         vm.startPrank(_kycProvider);
-        IKintoID.SignatureData memory sigdata = _auxCreateSignature(_kintoID, _user, _userPk, block.timestamp + 1000);
-        uint16[] memory traits = new uint16[](1);
-        traits[0] = 1;
-        _kintoID.mintIndividualKyc(sigdata, traits);
+
         _kintoID.addSanction(_user, 1);
 
         assertEq(_kintoID.isSanctionsSafeIn(_user, 1), false);
@@ -388,11 +405,10 @@ contract KintoIDTest is SharedSetup {
     }
 
     function testAddSanction_WhenNotConfirmed() public {
+        addKYC();
+
         vm.startPrank(_kycProvider);
-        IKintoID.SignatureData memory sigdata = _auxCreateSignature(_kintoID, _user, _userPk, block.timestamp + 1000);
-        uint16[] memory traits = new uint16[](1);
-        traits[0] = 1;
-        _kintoID.mintIndividualKyc(sigdata, traits);
+
         _kintoID.addSanction(_user, 1);
 
         assertEq(_kintoID.isSanctionsSafeIn(_user, 1), false);
@@ -410,20 +426,109 @@ contract KintoIDTest is SharedSetup {
         assertEq(_kintoID.sanctionedAt(_user), sanctionTime);
     }
 
+    function testRemoveSancion_RevertWhenInExitWindowPeriod() public {
+        addKYC();
+
+        vm.startPrank(_kycProvider);
+        _kintoID.addSanction(_user, 1);
+        assertEq(_kintoID.isSanctionsSafeIn(_user, 1), false);
+
+        vm.expectRevert(abi.encodeWithSelector(IKintoID.ExitWindowPeriod.selector, _user, _kintoID.sanctionedAt(_user)));
+        _kintoID.removeSanction(_user, 1);
+        vm.stopPrank();
+    }
+
     function testRemoveSancion() public {
+        addKYC();
+
         vm.startPrank(_kycProvider);
-        IKintoID.SignatureData memory sigdata = _auxCreateSignature(_kintoID, _user, _userPk, block.timestamp + 1000);
-        uint16[] memory traits = new uint16[](1);
-        traits[0] = 1;
-        _kintoID.mintIndividualKyc(sigdata, traits);
         _kintoID.addSanction(_user, 1);
         assertEq(_kintoID.isSanctionsSafeIn(_user, 1), false);
+
+        // has to wait for the exit window to be over
+        vm.warp(block.timestamp + 10 days);
+
         _kintoID.removeSanction(_user, 1);
+        vm.stopPrank();
+
         assertEq(_kintoID.isSanctionsSafeIn(_user, 1), true);
         assertEq(_kintoID.isSanctionsSafe(_user), true);
         assertEq(_kintoID.lastMonitoredAt(), block.timestamp);
     }
 
+    function testAddSanction_BlockedDuringExitWindow() public {
+        addKYC();
+
+        vm.startPrank(_kycProvider);
+
+        // Add initial sanction
+        _kintoID.addSanction(_user, 1);
+        uint256 sanctionTime = block.timestamp;
+
+        // Try adding another sanction during exit window
+        vm.expectRevert(abi.encodeWithSelector(IKintoID.ExitWindowPeriod.selector, _user, sanctionTime));
+        _kintoID.addSanction(_user, 2);
+
+        // Try at different times during window
+        vm.warp(block.timestamp + 5 days);
+        vm.expectRevert(abi.encodeWithSelector(IKintoID.ExitWindowPeriod.selector, _user, sanctionTime));
+        _kintoID.addSanction(_user, 2);
+
+        // Should succeed after window
+        vm.warp(sanctionTime + 10 days + 1);
+        _kintoID.addSanction(_user, 2);
+
+        vm.stopPrank();
+    }
+
+    function testRemoveSanction_BlockedDuringExitWindow() public {
+        addKYC();
+
+        vm.startPrank(_kycProvider);
+
+        // Add sanction
+        _kintoID.addSanction(_user, 1);
+        uint256 sanctionTime = block.timestamp;
+
+        // Try removing during exit window
+        vm.expectRevert(abi.encodeWithSelector(IKintoID.ExitWindowPeriod.selector, _user, sanctionTime));
+        _kintoID.removeSanction(_user, 1);
+
+        // Try halfway through window
+        vm.warp(block.timestamp + 5 days);
+        vm.expectRevert(abi.encodeWithSelector(IKintoID.ExitWindowPeriod.selector, _user, sanctionTime));
+        _kintoID.removeSanction(_user, 1);
+
+        // Should succeed after window
+        vm.warp(sanctionTime + 10 days + 1);
+        _kintoID.removeSanction(_user, 1);
+
+        vm.stopPrank();
+    }
+
+    function testExitWindow_MultipleSanctions() public {
+        addKYC();
+
+        vm.startPrank(_kycProvider);
+
+        // Add first sanction
+        _kintoID.addSanction(_user, 1);
+        uint256 firstSanctionTime = block.timestamp;
+
+        // Advance past window
+        vm.warp(firstSanctionTime + 10 days + 1);
+
+        // Add second sanction
+        _kintoID.addSanction(_user, 2);
+        uint256 secondSanctionTime = block.timestamp;
+
+        // Try removing first sanction during second's window
+        vm.expectRevert(abi.encodeWithSelector(IKintoID.ExitWindowPeriod.selector, _user, secondSanctionTime));
+        _kintoID.removeSanction(_user, 1);
+
+        vm.stopPrank();
+    }
+
     function testAddSanction_RevertWhen_CallerIsNotKYCProvider() public {
         approveKYC(_kycProvider, _user, _userPk, new uint16[](1));
 
@@ -559,4 +664,13 @@ contract KintoIDTest is SharedSetup {
 
         assertTrue(_kintoID.supportsInterface(InterfaceERC721Upgradeable));
     }
+
+    function addKYC() public {
+        vm.startPrank(_kycProvider);
+        IKintoID.SignatureData memory sigdata = _auxCreateSignature(_kintoID, _user, _userPk, block.timestamp + 1000);
+        uint16[] memory traits = new uint16[](1);
+        traits[0] = 1;
+        _kintoID.mintIndividualKyc(sigdata, traits);
+        vm.stopPrank();
+    }
 }