Skip to content

chore(protocol-contracts): remove unstake recipient from ProtocolStaking #1370

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Nov 25, 2025
Merged

chore(protocol-contracts): remove unstake recipient from ProtocolStaking #1370

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
125cfe9
chore(protocol-contracts): remove unstake recipient from `ProtocolSta…
arr00 Nov 19, 2025
fec0e2f
Update protocol-contracts/staking/contracts/ProtocolStaking.sol
arr00 Nov 20, 2025
7dff121
docs
arr00 Nov 20, 2025
4b5bea5
Merge branch 'main' into chore/remove-unstake-recipient-protocol-staking
arr00 Nov 24, 2025
4831126
terminate early for 0 shares redemption
arr00 Nov 24, 2025
67293ba
Merge branch 'main' into chore/remove-unstake-recipient-protocol-staking
arr00 Nov 25, 2025
4378383
Remove recipient from `TokensUnstaked` event
arr00 Nov 25, 2025
d1c7058
Merge branch 'main' into chore/remove-unstake-recipient-protocol-staking
arr00 Nov 25, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 2 additions & 4 deletions protocol-contracts/staking/contracts/OperatorStaking.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ contract OperatorStaking is ERC20, Ownable, ReentrancyGuardTransient {
* @param owner The owner of the shares.
*/
function requestRedeem(uint208 shares, address controller, address owner) public virtual {
if (shares == 0) return;
require(controller != address(0), InvalidController());
if (msg.sender != owner) {
_spendAllowance(owner, msg.sender, shares);
Expand All @@ -126,10 +127,7 @@ contract OperatorStaking is ERC20, Ownable, ReentrancyGuardTransient {
);

(, uint48 lastReleaseTime, uint208 controllerSharesRedeemed) = _unstakeRequests[controller].latestCheckpoint();
uint48 releaseTime = protocolStaking_.unstake(
address(this),
SafeCast.toUint256(SignedMath.max(assetsToWithdraw, 0))
);
uint48 releaseTime = protocolStaking_.unstake(SafeCast.toUint256(SignedMath.max(assetsToWithdraw, 0)));
assert(releaseTime >= lastReleaseTime); // should never happen
_unstakeRequests[controller].push(releaseTime, controllerSharesRedeemed + shares);

Expand Down
33 changes: 15 additions & 18 deletions protocol-contracts/staking/contracts/ProtocolStaking.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,8 @@ contract ProtocolStaking is AccessControlDefaultAdminRulesUpgradeable, ERC20Vote
uint256 _totalEligibleStakedWeight;
// Stake - release
uint48 _unstakeCooldownPeriod;
mapping(address recipient => Checkpoints.Trace208) _unstakeRequests;
mapping(address recipient => uint256) _released;
mapping(address account => Checkpoints.Trace208) _unstakeRequests;
mapping(address account => uint256) _released;
// Reward - issuance curve
uint256 _lastUpdateTimestamp;
uint256 _lastUpdateReward;
Expand All @@ -58,7 +58,7 @@ contract ProtocolStaking is AccessControlDefaultAdminRulesUpgradeable, ERC20Vote
/// @dev Emitted when tokens are staked by an account.
event TokensStaked(address indexed account, uint256 amount);
/// @dev Emitted when tokens are unstaked by an account.
event TokensUnstaked(address indexed account, address indexed recipient, uint256 amount, uint48 releaseTime);
event TokensUnstaked(address indexed account, uint256 amount, uint48 releaseTime);
/// @dev Emitted when tokens are released to a recipient after the unstaking cooldown period.
event TokensReleased(address indexed recipient, uint256 amount);
/// @dev Emitted when rewards of an account are claimed.
Expand All @@ -70,8 +70,6 @@ contract ProtocolStaking is AccessControlDefaultAdminRulesUpgradeable, ERC20Vote
/// @dev Emitted when the reward recipient of an account is updated.
event RewardsRecipientSet(address indexed account, address indexed recipient);

/// @dev Emitted when an account unstakes to the zero address.
error InvalidUnstakeRecipient();
/// @dev The account cannot be made eligible.
error InvalidEligibleAccount(address account);
/// @dev The tokens cannot be transferred.
Expand Down Expand Up @@ -117,34 +115,33 @@ contract ProtocolStaking is AccessControlDefaultAdminRulesUpgradeable, ERC20Vote
}

/**
* @dev Unstake `amount` tokens from `msg.sender`'s staked balance to `recipient`.
* @param recipient The recipient where unstaked tokens should be sent.
* @param amount The amount of tokens to unstake.
* @return releaseTime The timestamp when the unstaked tokens can be released.
* @dev Unstake `amount` tokens from `msg.sender`'s staked balance to `msg.sender`.
*
* NOTE: Unstaked tokens are released by calling {release} after {unstakeCooldownPeriod}.
* WARNING: Unstake release times are strictly increasing per account even if the cooldown period
* is reduced. For a given account to fully realize the reduction in cooldown period, they may need
* to wait up to `OLD_COOLDOWN_PERIOD - NEW_COOLDOWN_PERIOD` seconds after the cooldown period is updated.
*
* @param amount The amount of tokens to unstake.
* @return releaseTime The timestamp when the unstaked tokens can be released.
*/
function unstake(address recipient, uint256 amount) public returns (uint48) {
require(recipient != address(0), InvalidUnstakeRecipient());
function unstake(uint256 amount) public returns (uint48) {
_burn(msg.sender, amount);

ProtocolStakingStorage storage $ = _getProtocolStakingStorage();
(, uint256 lastReleaseTime, uint256 totalRequestedToWithdraw) = $
._unstakeRequests[recipient]
._unstakeRequests[msg.sender]
.latestCheckpoint();
uint48 releaseTime = SafeCast.toUint48(Math.max(Time.timestamp() + $._unstakeCooldownPeriod, lastReleaseTime));
$._unstakeRequests[recipient].push(releaseTime, uint208(totalRequestedToWithdraw + amount));
$._unstakeRequests[msg.sender].push(releaseTime, uint208(totalRequestedToWithdraw + amount));

emit TokensUnstaked(msg.sender, recipient, amount, releaseTime);
emit TokensUnstaked(msg.sender, amount, releaseTime);
return releaseTime;
}

/**
* @dev Releases tokens requested for unstaking after the cooldown period to `account`.
* @param account The account to release tokens to.
*
* WARNING: If this contract is upgraded to add slashing, the ability to withdraw to a
* different address should be reconsidered.
*/
function release(address account) public virtual {
ProtocolStakingStorage storage $ = _getProtocolStakingStorage();
Expand Down Expand Up @@ -206,7 +203,7 @@ contract ProtocolStaking is AccessControlDefaultAdminRulesUpgradeable, ERC20Vote

/**
* @dev Sets the {unstake} cooldown period in seconds to `unstakeCooldownPeriod`. Only callable
* by `MANAGER_ROLE` role.
* by `MANAGER_ROLE` role. See {unstake} for important notes regarding the cooldown period.
* @param unstakeCooldownPeriod_ The new unstake cooldown period.
*/
function setUnstakeCooldownPeriod(uint48 unstakeCooldownPeriod_) public onlyRole(MANAGER_ROLE) {
Expand Down
11 changes: 9 additions & 2 deletions protocol-contracts/staking/test/OperatorStaking.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,13 @@ describe('OperatorStaking', function () {
await expect(this.token.balanceOf(this.mock)).to.eventually.be.eq(0);
});

it('zero redemption should terminate early', async function () {
await expect(this.mock.connect(this.staker1).requestRedeem(0, this.staker1, this.staker1)).to.not.emit(
this.mock,
'RedeemRequest',
);
});

it('should not redeem twice', async function () {
await this.mock.connect(this.staker2).deposit(ethers.parseEther('5'), this.staker2);
await this.mock.connect(this.staker1).deposit(ethers.parseEther('10'), this.staker1);
Expand Down Expand Up @@ -306,7 +313,7 @@ describe('OperatorStaking', function () {

await expect(this.mock.connect(this.staker2).requestRedeem(ethers.parseEther('2'), this.staker2, this.staker2))
.to.emit(this.protocolStaking, 'TokensUnstaked')
.withArgs(this.mock, this.mock, ethers.parseEther('0.5'), anyValue);
.withArgs(this.mock, ethers.parseEther('0.5'), anyValue);
});

it('take excess into account on requestRedeem after slashing fully covered', async function () {
Expand All @@ -320,7 +327,7 @@ describe('OperatorStaking', function () {

await expect(this.mock.connect(this.staker2).requestRedeem(ethers.parseEther('1'), this.staker2, this.staker2))
.to.emit(this.protocolStaking, 'TokensUnstaked')
.withArgs(this.mock, this.mock, 0, anyValue);
.withArgs(this.mock, 0, anyValue);

await time.increase(30);
await expect(this.mock.maxRedeem(this.staker2)).to.eventually.eq(0);
Expand Down
77 changes: 35 additions & 42 deletions protocol-contracts/staking/test/ProtocolStaking.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,34 +208,18 @@ describe('Protocol Staking', function () {

it('should not transfer instantly', async function () {
await this.mock.connect(this.manager).setUnstakeCooldownPeriod(60); // 1 minute
await expect(this.mock.connect(this.staker1).unstake(this.staker1, ethers.parseEther('50')))
await expect(this.mock.connect(this.staker1).unstake(ethers.parseEther('50')))
.to.emit(this.mock, 'Transfer')
.withArgs(this.staker1, ethers.ZeroAddress, ethers.parseEther('50'))
.to.not.emit(this.token, 'Transfer');
});

it('should be able to unstake to someone else', async function () {
const currentTime = BigInt(await time.latest());
const cooldownPeriod = BigInt(await this.mock.unstakeCooldownPeriod());
await expect(this.mock.connect(this.staker1).unstake(this.staker2, ethers.parseEther('50')))
.to.emit(this.mock, 'TokensUnstaked')
.withArgs(this.staker1, this.staker2, ethers.parseEther('50'), currentTime + cooldownPeriod + 1n);
await mine();
await expect(this.mock.release(this.staker2))
.to.emit(this.token, 'Transfer')
.withArgs(this.mock, this.staker2, ethers.parseEther('50'));
});

it('should not unstake to zero address', async function () {
await expect(
this.mock.connect(this.staker1).unstake(ethers.ZeroAddress, ethers.parseEther('50')),
).to.be.revertedWithCustomError(this.mock, 'InvalidUnstakeRecipient');
.withArgs(this.staker1, ethers.parseEther('50'), anyValue)
.to.not.emit(this.token, 'Transfer');
});

describe('Release', function () {
it('should transfer after cooldown complete', async function () {
await this.mock.connect(this.manager).setUnstakeCooldownPeriod(60); // 1 minute
await this.mock.connect(this.staker1).unstake(this.staker1, ethers.parseEther('50'));
await this.mock.connect(this.staker1).unstake(ethers.parseEther('50'));
await expect(this.mock.awaitingRelease(this.staker1)).to.eventually.eq(ethers.parseEther('50'));

await timeIncreaseNoMine(60);
Expand All @@ -248,7 +232,7 @@ describe('Protocol Staking', function () {

it('should only release once', async function () {
await this.mock.connect(this.manager).setUnstakeCooldownPeriod(60); // 1 minute
await this.mock.connect(this.staker1).unstake(this.staker1, ethers.parseEther('50'));
await this.mock.connect(this.staker1).unstake(ethers.parseEther('50'));

await timeIncreaseNoMine(60);

Expand All @@ -262,18 +246,18 @@ describe('Protocol Staking', function () {

it("should not release if cooldown isn't complete", async function () {
await this.mock.connect(this.manager).setUnstakeCooldownPeriod(60);
await this.mock.connect(this.staker1).unstake(this.staker1, ethers.parseEther('50'));
await this.mock.connect(this.staker1).unstake(ethers.parseEther('50'));

await timeIncreaseNoMine(30);
await expect(this.mock.release(this.staker1)).to.not.emit(this.token, 'Transfer');
});

it('should combine multiple complete withdrawals', async function () {
await this.mock.connect(this.manager).setUnstakeCooldownPeriod(60); // 1 minute
await this.mock.connect(this.staker1).unstake(this.staker1, ethers.parseEther('50'));
await this.mock.connect(this.staker1).unstake(ethers.parseEther('50'));

await timeIncreaseNoMine(30);
await this.mock.connect(this.staker1).unstake(this.staker1, ethers.parseEther('50'));
await this.mock.connect(this.staker1).unstake(ethers.parseEther('50'));
await expect(this.mock.awaitingRelease(this.staker1)).to.eventually.eq(ethers.parseEther('100'));

await timeIncreaseNoMine(60);
Expand All @@ -285,13 +269,13 @@ describe('Protocol Staking', function () {

it('should only release completed cooldowns in batch', async function () {
await this.mock.connect(this.manager).setUnstakeCooldownPeriod(60); // 1 minute
await this.mock.connect(this.staker1).unstake(this.staker1, ethers.parseEther('25'));
await this.mock.connect(this.staker1).unstake(ethers.parseEther('25'));

await timeIncreaseNoMine(20);
await this.mock.connect(this.staker1).unstake(this.staker1, ethers.parseEther('25'));
await this.mock.connect(this.staker1).unstake(ethers.parseEther('25'));

await timeIncreaseNoMine(20);
await this.mock.connect(this.staker1).unstake(this.staker1, ethers.parseEther('25'));
await this.mock.connect(this.staker1).unstake(ethers.parseEther('25'));

await timeIncreaseNoMine(40);
await expect(this.mock.release(this.staker1))
Expand All @@ -301,11 +285,11 @@ describe('Protocol Staking', function () {

it('should handle decrease in cooldown period gracefully', async function () {
await this.mock.connect(this.manager).setUnstakeCooldownPeriod(120);
await this.mock.connect(this.staker1).unstake(this.staker1, ethers.parseEther('25'));
await this.mock.connect(this.staker1).unstake(ethers.parseEther('25'));

await timeIncreaseNoMine(30);
await this.mock.connect(this.manager).setUnstakeCooldownPeriod(30);
await this.mock.connect(this.staker1).unstake(this.staker1, ethers.parseEther('25'));
await this.mock.connect(this.staker1).unstake(ethers.parseEther('25'));

// advance 30 seconds. Still need to wait another 60 seconds for the original unstake request to complete.
await timeIncreaseNoMine(30);
Expand All @@ -323,7 +307,7 @@ describe('Protocol Staking', function () {

const beforetotalStakedWeight = await this.mock.totalStakedWeight();
const beforeStaker1Log = await this.mock.weight(await this.mock.balanceOf(this.staker1));
await this.mock.connect(this.staker1).unstake(this.staker1, ethers.parseEther('75'));
await this.mock.connect(this.staker1).unstake(ethers.parseEther('75'));
const afterStaker1Log = await this.mock.weight(await this.mock.balanceOf(this.staker1));
const aftertotalStakedWeight = await this.mock.totalStakedWeight();
expect(beforetotalStakedWeight - aftertotalStakedWeight).to.equal(beforeStaker1Log - afterStaker1Log);
Expand Down Expand Up @@ -375,7 +359,7 @@ describe('Protocol Staking', function () {

await timeIncreaseNoMine(10);

await this.mock.connect(this.staker1).unstake(this.staker1, ethers.parseEther('100'));
await this.mock.connect(this.staker1).unstake(ethers.parseEther('100'));

// time passes (10 seconds) while no one is staked
await timeIncreaseNoMine(10);
Expand All @@ -400,9 +384,9 @@ describe('Protocol Staking', function () {
await timeIncreaseNoMine(10);

// 3 in rewards for 1 (since 1 block at the beginning alone)
await this.mock.connect(this.staker1).unstake(this.staker1, ethers.parseEther('100'));
await this.mock.connect(this.staker1).unstake(ethers.parseEther('100'));
// 3 in rewards for 2 (since 1 block at the end alone)
await this.mock.connect(this.staker2).unstake(this.staker2, ethers.parseEther('100'));
await this.mock.connect(this.staker2).unstake(ethers.parseEther('100'));

await timeIncreaseNoMine(10);

Expand Down Expand Up @@ -431,7 +415,7 @@ describe('Protocol Staking', function () {

await timeIncreaseNoMine(10);

await this.mock.connect(this.staker1).unstake(this.staker1, ethers.parseEther('100'));
await this.mock.connect(this.staker1).unstake(ethers.parseEther('100'));

// time passes (10 seconds) while no one is staked
await timeIncreaseNoMine(10);
Expand All @@ -456,9 +440,9 @@ describe('Protocol Staking', function () {
await timeIncreaseNoMine(10);

// 3 in rewards for 1 (since 1 block at the beginning alone)
await this.mock.connect(this.staker1).unstake(this.staker1, ethers.parseEther('100'));
await this.mock.connect(this.staker1).unstake(ethers.parseEther('100'));
// 3 in rewards for 2 (since 1 block at the end alone)
await this.mock.connect(this.staker2).unstake(this.staker2, ethers.parseEther('100'));
await this.mock.connect(this.staker2).unstake(ethers.parseEther('100'));

await timeIncreaseNoMine(10);

Expand Down Expand Up @@ -487,7 +471,7 @@ describe('Protocol Staking', function () {

await timeIncreaseNoMine(10);

await this.mock.connect(this.staker1).unstake(this.staker1, ethers.parseEther('100'));
await this.mock.connect(this.staker1).unstake(ethers.parseEther('100'));

// time passes (10 seconds) while no one is staked
await timeIncreaseNoMine(10);
Expand All @@ -512,9 +496,9 @@ describe('Protocol Staking', function () {
await timeIncreaseNoMine(10);

// 3 in rewards for 1 (since 1 block at the beginning alone)
await this.mock.connect(this.staker1).unstake(this.staker1, ethers.parseEther('100'));
await this.mock.connect(this.staker1).unstake(ethers.parseEther('100'));
// 3 in rewards for 2 (since 1 block at the end alone)
await this.mock.connect(this.staker2).unstake(this.staker2, ethers.parseEther('100'));
await this.mock.connect(this.staker2).unstake(ethers.parseEther('100'));

await timeIncreaseNoMine(10);

Expand Down Expand Up @@ -543,7 +527,7 @@ describe('Protocol Staking', function () {

await timeIncreaseNoMine(10);

await this.mock.connect(this.staker1).unstake(this.staker1, ethers.parseEther('100'));
await this.mock.connect(this.staker1).unstake(ethers.parseEther('100'));

// time passes (10 seconds) while no one is staked
await timeIncreaseNoMine(10);
Expand All @@ -568,9 +552,9 @@ describe('Protocol Staking', function () {
await timeIncreaseNoMine(10);

// 3 in rewards for 1 (since 1 block at the beginning alone)
await this.mock.connect(this.staker1).unstake(this.staker1, ethers.parseEther('100'));
await this.mock.connect(this.staker1).unstake(ethers.parseEther('100'));
// 3 in rewards for 2 (since 1 block at the end alone)
await this.mock.connect(this.staker2).unstake(this.staker2, ethers.parseEther('100'));
await this.mock.connect(this.staker2).unstake(ethers.parseEther('100'));

await timeIncreaseNoMine(10);

Expand All @@ -595,6 +579,15 @@ describe('Protocol Staking', function () {
.withArgs(eligibleAccountRole, this.staker1, this.manager);
});

it('should not update total staked amount if no balance', async function () {
await this.mock.connect(this.manager).addEligibleAccount(this.staker1);
await this.mock.connect(this.staker1).stake(ethers.parseEther('1'));

const weightBefore = await this.mock.totalStakedWeight();
await this.mock.connect(this.manager).addEligibleAccount(this.staker2);
await expect(this.mock.totalStakedWeight()).to.eventually.eq(weightBefore);
});

it('should reflect in eligible account storage', async function () {
await this.mock.connect(this.manager).addEligibleAccount(this.staker1);
await this.mock.connect(this.manager).addEligibleAccount(this.staker2);
Expand Down