Add `OperatorStaking`, operator/stakers splitter and `StakersRewardsRecipient` #216

james-toussaint · 2025-09-25T15:12:00Z

Summary by CodeRabbit

New Features
- Added an operator staking vault enabling deposits, cooldown-based withdrawals, and reward distribution for both operator and stakers.
- Supports claiming and restaking rewards, with per-staker reward accounting and visibility into tokens in cooldown and total assets.
Tests
- Introduced comprehensive tests covering reward accrual/withdrawal, deposits/withdrawals with cooldown, and restaking.
- Removed exclusive test flags to run the full test suite.
Chores
- Added an OpenZeppelin Contracts v4 dependency alias to support new staking and rewards functionality.

coderabbitai · 2025-09-25T15:12:08Z

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Walkthrough

Introduces OperatorStaking (ERC4626 vault) integrating with ProtocolStaking and a new StakersRewardsRecipient for per-staker rewards accounting, cooldown-based unstaking, reward withdrawal, and restaking. Adds OpenZeppelin v4 PaymentSplitter dependency alias and import. Minor NatSpec comment added in ProtocolStaking. Tests added for staking, rewards, cooldown release, and restaking; removes describe.only in two suites.

Changes

Cohort / File(s)	Summary
New staking contracts `contracts/finance/OperatorStaking.sol`, `contracts/finance/StakersRewardsRecipient.sol`	Adds ERC4626-based OperatorStaking integrating with ProtocolStaking for staking/unstaking, rewards withdrawal/restake, cooldown tracking, and admin-set rewards recipients. Adds StakersRewardsRecipient to pull, account, and distribute per-staker rewards with share-based allocation and release tracking.
ProtocolStaking doc tweak `contracts/finance/ProtocolStaking.sol`	Adds a TODO comment in unstake NatSpec; no logic changes.
Dependency alias/imports `package.json`, `contracts/mocks/import.sol`	Adds devDependency alias "@openzeppelin/contracts-v4" to OpenZeppelin Contracts ^4.9.6 and imports PaymentSplitter from that alias in mocks.
OperatorStaking tests `test/finance/OperatorStaking.test.ts`	Adds comprehensive tests covering reward distribution, deposits/withdrawals with cooldown and release, and restaking.
Test suite scope fixes `test/finance/ProtocolOperatorRegistry.test.ts`, `test/finance/ProtocolStaking.t.ts`	Replaces describe.only with describe to run full test suites.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant U as User
  participant OS as OperatorStaking (ERC4626)
  participant PS as ProtocolStaking
  participant SRR as StakersRewardsRecipient
  participant GRR as Global Rewards Recipient
  participant T as ERC20 Token

  rect rgb(235, 245, 255)
  note over U,OS: Deposit flow
  U->>OS: deposit(assets)
  OS->>SRR: increaseReleasedRewards(U, shares, totalShares)
  OS->>T: transferFrom(U, OS, assets)
  OS->>T: approve(PS, assets)
  OS->>PS: stake(assets)
  OS-->>U: mint shares
  end

  rect rgb(240, 255, 240)
  note over OS,SRR: Rewards pull/distribution
  OS->>PS: globalRewardsRecipient()
  PS-->>OS: GRR address
  OS->>SRR: withdrawRewards(account, shares, totalShares)
  SRR->>GRR: pull rewards (if any)
  GRR-->>SRR: transfer rewards
  SRR-->>account: transfer allocated rewards
  end

  rect rgb(255, 250, 235)
  note over U,PS: Unstake with cooldown
  U->>OS: withdraw(assets)
  OS->>SRR: decreaseReleasedRewards(U, burnShares, totalShares)
  OS-->>U: burn shares
  OS->>PS: unstake(assets)
  PS-->>OS: cooldown receipt
  OS-->>U: record cooldown checkpoint
  end

  rect rgb(255, 240, 245)
  note over U,PS: Release after cooldown
  U->>OS: release(U)
  OS->>PS: withdraw after cooldown(U)
  PS-->>OS: transfer assets
  OS-->>U: transfer assets
  end

  rect rgb(235, 255, 250)
  note over U,OS: Restake rewards
  U->>OS: restakeRewards()
  OS->>SRR: withdrawRewards(U, shares, totalShares)
  SRR-->>U: transfer rewards
  U->>OS: deposit(rewardAssets)
  OS->>PS: stake(rewardAssets)
  end

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~55 minutes

Suggested reviewers

arr00

Poem

A whisk of code, a hop of stake,
Cooldowns moonlit, rewards awake.
Carrots compound, vaults accrue,
Shares like stars in midnight dew.
I thump the ground—tests all green—
Restake, release, a tidy scene.
🥕✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The title accurately and concisely describes the main additions of the pull request by naming the new OperatorStaking contract, the operator/stakers splitter functionality, and the StakersRewardsRecipient component, which aligns with the changeset’s primary purpose of introducing these major features.
Docstring Coverage	✅ Passed	No functions found in the changes. Docstring coverage check skipped.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

…rator-staking

package.json

coderabbitai · 2025-10-06T12:41:01Z

✅ Actions performed

Full review triggered.

coderabbitai

Actionable comments posted: 3

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b0cd3c9 and f16a8b2.

⛔ Files ignored due to path filters (1)

package-lock.json is excluded by !**/package-lock.json

📒 Files selected for processing (8)

contracts/finance/OperatorStaking.sol (1 hunks)
contracts/finance/ProtocolStaking.sol (1 hunks)
contracts/finance/StakersRewardsRecipient.sol (1 hunks)
contracts/mocks/import.sol (1 hunks)
package.json (1 hunks)
test/finance/OperatorStaking.test.ts (1 hunks)
test/finance/ProtocolOperatorRegistry.test.ts (1 hunks)
test/finance/ProtocolStaking.t.ts (1 hunks)

coderabbitai · 2025-10-06T12:49:16Z

contracts/finance/OperatorStaking.sol

+    function withdrawRewards(address account) public virtual {
+        _protocolStaking.claimRewards(address(this)); // Transfer all rewards to global rewards recipient
+        if (account == owner()) {
+            // Withdraw operator rewards
+            IPaymentSplitter globalRewardsRecipient_ = IPaymentSplitter(globalRewardsRecipient());
+            IERC20 stakingToken_ = IERC20(asset());
+            if (globalRewardsRecipient_.releasable(stakingToken_, owner()) > 0) {
+                globalRewardsRecipient_.release(stakingToken_, owner());
+            }
+        } else {
+            // Withdraw staker rewards
+            _stakersRewardsRecipient.withdrawRewards(account, balanceOf(account), totalSupply());
+        }
+    }


⚠️ Potential issue | 🟠 Major

Claim protocol rewards for stakers before distributing
withdrawRewards only calls _protocolStaking.claimRewards when account == owner(). If a staker calls withdrawRewards while fresh rewards are still held by ProtocolStaking, we never pull those funds into the splitter, so _stakersRewardsRecipient.withdrawRewards has nothing to release. In practice this means stakers are dependent on the operator (the owner) to withdraw first, otherwise their rewards remain stuck. Please add a claimRewards before servicing staker withdrawals so the latest rewards are always available.

A minimal fix is to invoke _protocolStaking.claimRewards(address(this)); unconditionally at the top of withdrawRewards.

🤖 Prompt for AI Agents

In contracts/finance/OperatorStaking.sol around lines 80 to 93, withdrawRewards only calls _protocolStaking.claimRewards when account == owner(), leaving staker withdrawals unable to include fresh protocol rewards; move or add an unconditional call to _protocolStaking.claimRewards(address(this)); at the start of withdrawRewards so rewards are always claimed into the contract before branching to owner or staker distribution, keeping existing owner/staker logic unchanged.

When account is a staker, here is the flow: _stakersRewardsRecipient.withdrawRewards(..) -> modifier pullRewards() -> protocolStaking.claimRewards(..) -> globalRewardsRecipient.release(..) so staking tokens are back to StakersRewardsRecipient, then transferred to staker with stakingToken().transfer(account, releasable).

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash # Verify the pullRewards modifier and _pullRewards implementation in StakersRewardsRecipient cat contracts/finance/StakersRewardsRecipient.sol

Length of output: 5205

@james-toussaint, you're absolutely right! I apologize for the incorrect analysis.

Looking at the StakersRewardsRecipient contract, I can now confirm that the pullRewards modifier (line 45) calls _pullRewards() internally (line 113), which in turn invokes protocolStaking.claimRewards(address(_operatorStaking)) at line 115 before transferring the staker's share from the global rewards recipient.

The implementation correctly handles both paths:

Operator path: Explicit claimRewards call in OperatorStaking.withdrawRewards

Staker path: claimRewards triggered via the pullRewards modifier in StakersRewardsRecipient.withdrawRewards

This is a clean architectural pattern where the rewards recipient contract encapsulates its own preconditions. Thank you for the clarification!

ฅ^•ﻌ•^ฅ

✏️ Learnings added

Learnt from: james-toussaint PR: OpenZeppelin/openzeppelin-confidential-contracts#216 File: contracts/finance/OperatorStaking.sol:80-93 Timestamp: 2025-10-06T13:03:33.545Z Learning: In contracts/finance/OperatorStaking.sol and StakersRewardsRecipient.sol, the withdrawRewards flow for stakers automatically claims protocol rewards via the pullRewards modifier in StakersRewardsRecipient, which internally calls _pullRewards() that invokes protocolStaking.claimRewards. This is a valid pattern where the called contract handles preconditions via modifiers, so there's no need for an explicit claimRewards call in OperatorStaking.withdrawRewards for stakers.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

contracts/finance/StakersRewardsRecipient.sol

test/finance/OperatorStaking.test.ts

james-toussaint requested review from Amxx and arr00 September 25, 2025 15:12

james-toussaint self-assigned this Sep 25, 2025

james-toussaint added the ignore-changeset label Sep 25, 2025

james-toussaint changed the title ~~AddOperatorStaking and OperatorStakingRewarder~~ Add OperatorStaking and OperatorStakingRewarder Sep 25, 2025

arr00 and others added 13 commits September 29, 2025 13:38

add operator staking

5e212b0

up

81fe87c

update operator staking

0e57d46

add operator staking test

9fd010b

add warning

5b364e1

Use separate operator staking rewarder

d1048d8

Set operator fee on staker withdraw

4a81ab7

Add GlobalRewardsRecipient splitter and StakersRewardsRecipient

6d27315

Refactor and comment

69d2571

Add admin to constructor

46f99f8

Withdraw operator rewards

40bdb42

Pull rewards

93ed6e4

Restake rewards

37178c6

james-toussaint force-pushed the feature/operator-staking branch from 08aebb9 to 37178c6 Compare October 3, 2025 14:50

james-toussaint changed the title ~~Add OperatorStaking and OperatorStakingRewarder~~ Add OperatorStaking, operator/stakers splitter and StakersRewardsRecipient Oct 3, 2025

james-toussaint added 5 commits October 3, 2025 16:58

Remove only test annotation

080f085

Remove others only

0160481

Merge remote-tracking branch 'origin/feat/staking-1' into feature/ope…

ebdb895

…rator-staking

Unstake cooldown

76a0185

Add unstake cooldown on operator contract

f16a8b2

james-toussaint commented Oct 6, 2025

View reviewed changes

package.json Show resolved Hide resolved

coderabbitai bot reviewed Oct 6, 2025

View reviewed changes

Check releaseable is zero after reward withdraw

5c57030

james-toussaint added 2 commits October 6, 2025 16:52

Virtually realease reward to prevent claim

59c510c

Typo

f07a6bf

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add `OperatorStaking`, operator/stakers splitter and `StakersRewardsRecipient` #216

Add `OperatorStaking`, operator/stakers splitter and `StakersRewardsRecipient` #216

james-toussaint commented Sep 25, 2025 •

edited by coderabbitai bot

Loading

Uh oh!

coderabbitai bot commented Sep 25, 2025 •

edited

Loading

Review skipped

Uh oh!

Uh oh!

coderabbitai bot commented Oct 6, 2025

Uh oh!

coderabbitai bot left a comment

Uh oh!

coderabbitai bot Oct 6, 2025 •

edited

Loading

Uh oh!

james-toussaint Oct 6, 2025

Uh oh!

coderabbitai bot Oct 6, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Add OperatorStaking, operator/stakers splitter and StakersRewardsRecipient #216

Are you sure you want to change the base?

Add OperatorStaking, operator/stakers splitter and StakersRewardsRecipient #216

Conversation

james-toussaint commented Sep 25, 2025 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai bot commented Sep 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Suggested reviewers

Poem

Pre-merge checks and finishing touches

Uh oh!

Uh oh!

coderabbitai bot commented Oct 6, 2025

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot Oct 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

james-toussaint Oct 6, 2025

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot Oct 6, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Add `OperatorStaking`, operator/stakers splitter and `StakersRewardsRecipient` #216

Add `OperatorStaking`, operator/stakers splitter and `StakersRewardsRecipient` #216

james-toussaint commented Sep 25, 2025 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Sep 25, 2025 •

edited

Loading

coderabbitai bot Oct 6, 2025 •

edited

Loading