From 33ddfb648e5ac3c29e028aed12e60a82dc51e106 Mon Sep 17 00:00:00 2001 From: dev-mondoshawan Date: Wed, 22 Jul 2026 08:20:48 -0500 Subject: [PATCH] fix(staking): close slash exit-dodge via the unbonding queue initiateSlash gated only on the active stake (s.amount), so an operator could move their entire stake into the unbonding queue and make the slash impossible to even initiate (NoStake), despite the queued funds being nominally slashable. The claimWithdrawal/executeSlash defenses never fired because no proposal could be created. This nullified slashing, the protocol's core accountability. - initiateSlash now gates on the total slashable balance (amount + unbondingAmount), so a slash can be initiated against fully-queued stake; claimWithdrawal then blocks (pending slash) and executeSlash sweeps the queue as before. - initiateWithdrawal no longer lets an Active agent drain to zero in one step (removed the remaining != 0 carve-out); full exit requires suspending first, matching the documented intent and keeping an active agent always backed. - Regression tests: slash stays initiable/executable against a fully-queued stake; Active withdraw-to-zero reverts. Full suite 171 passing. Found during an internal adversarial review of the stake-exit/slash lifecycle. Co-Authored-By: Claude Opus 4.8 --- src/CountersigStaking.sol | 15 +++++++-- test/CountersigStaking.t.sol | 60 ++++++++++++++++++++++++++++++++++++ 2 files changed, 72 insertions(+), 3 deletions(-) diff --git a/src/CountersigStaking.sol b/src/CountersigStaking.sol index cf47f8c..05d353d 100644 --- a/src/CountersigStaking.sol +++ b/src/CountersigStaking.sol @@ -229,9 +229,14 @@ contract CountersigStaking is } uint256 remaining = s.amount - amount; - // If agent is still active, enforce minimum stake post-withdrawal. + // While Active, the remaining active stake must stay at or above minimumStake — + // withdrawing to zero (fully exiting) requires the operator to Suspend first (see + // CountersigIdentity.updateStatus). Previously a `remaining != 0` carve-out let an + // active agent drain its entire backing stake in one step, which the total-balance + // check in initiateSlash also now guards, but an active agent should never be + // unbacked to begin with. bool active = id.registeredAt != 0 && id.status == CountersigIdentity.AgentStatus.Active; - if (active && remaining < minimumStake && remaining != 0) { + if (active && remaining < minimumStake) { revert InsufficientStake(didHash, remaining, minimumStake); } @@ -291,7 +296,11 @@ contract CountersigStaking is bytes calldata evidenceHash ) external nonReentrant onlyRole(SLASHING_COMMITTEE_ROLE) { if (victim == address(0)) revert ZeroAddress(); - if (stakes[didHash].amount == 0) revert NoStake(didHash); + // Slashable balance is the active stake PLUS anything queued for withdrawal. + // Checking only the active stake let an operator drain everything into the + // unbonding queue (which executeSlash still sweeps) and thereby block the slash + // from ever being initiated — nullifying accountability. Gate on the total. + if (stakes[didHash].amount + stakes[didHash].unbondingAmount == 0) revert NoStake(didHash); if (slashProposals[didHash].state == SlashState.Pending) revert SlashAlreadyPending(didHash); // Write state before the external call (CEI pattern). diff --git a/test/CountersigStaking.t.sol b/test/CountersigStaking.t.sol index 22bf6ac..0581c78 100644 --- a/test/CountersigStaking.t.sol +++ b/test/CountersigStaking.t.sol @@ -255,6 +255,66 @@ contract CountersigStakingTest is Test { assertEq(csig.balanceOf(operator), before + MIN_STAKE * 2); } + // Regression: an operator must not be able to dodge a slash by draining the + // entire stake into the unbonding queue. Before the fix, initiateSlash checked + // only the active stake (s.amount) and reverted NoStake once everything was + // queued, so the committee could never even start a slash. + function test_exitDodge_slashStillInitiableWhenFullyQueued() public { + _deposit(); // 2 * MIN_STAKE + + // Legitimate full-exit path: Suspend, then withdraw everything to the queue. + vm.prank(operator); + identity.updateStatus(didHash, CountersigIdentity.AgentStatus.Suspended); + vm.prank(operator); + staking.initiateWithdrawal(didHash, MIN_STAKE * 2); + + assertEq(staking.getStake(didHash), 0); + (uint256 queued,) = staking.getPendingWithdrawal(didHash); + assertEq(queued, MIN_STAKE * 2); + + // The committee can still initiate a slash against the queued funds. + vm.prank(committee); + staking.initiateSlash(didHash, victim, "evidence"); + + // ... the operator cannot claim while the slash is pending ... + vm.expectRevert( + abi.encodeWithSelector(CountersigStaking.SlashAlreadyPending.selector, didHash) + ); + vm.prank(operator); + staking.claimWithdrawal(didHash); + + // ... and after the challenge window the queued funds are swept and burned/distributed. + vm.warp(block.timestamp + CHALLENGE + 1); + staking.executeSlash(didHash); + + assertEq(csig.balanceOf(address(0xdead)), (MIN_STAKE * 2) / 2); + (uint256 pendingAmount,) = staking.getPendingWithdrawal(didHash); + assertEq(pendingAmount, 0); + + vm.expectRevert( + abi.encodeWithSelector(CountersigStaking.NoWithdrawalPending.selector, didHash) + ); + vm.prank(operator); + staking.claimWithdrawal(didHash); + } + + // An Active agent must not be able to withdraw its entire backing stake in one + // step; full exit requires suspending first. + function test_initiateWithdrawal_reverts_toZeroWhileActive() public { + _deposit(); // 2 * MIN_STAKE + + vm.expectRevert( + abi.encodeWithSelector( + CountersigStaking.InsufficientStake.selector, + didHash, + 0, // remaining after draining everything + MIN_STAKE + ) + ); + vm.prank(operator); + staking.initiateWithdrawal(didHash, MIN_STAKE * 2); + } + function test_initiateWithdrawal_reverts_pendingSlash() public { _deposit();