fix: prevent reward dilution via zero sharesReward harvest

[Ishant5436]

Summary

Fixes a vulnerability in StakedEXA.sol where calling harvest() with a zero allowance (or calculating to a zero sharesReward) would inadvertently call notifyRewardAmount(0). This caused the reward rate to stretch out over the full duration, diluting the active rewards for stakers.

Changes

• contracts/StakedEXA.sol: Added an early return if (sharesReward == 0) in harvest().

Testing

• Compiled locally.
• Confirmed the zero-amount test cases return early and skip notifyRewardAmount.
• pnpm test successfully completed.

Summary by CodeRabbit

• Bug Fixes
  • Prevented reward-period/index updates and the related reward notification when the computed reward amount is zero, avoiding unnecessary emissions during harvesting.

[changeset-bot]

⚠️ No Changeset found

Latest commit: 44add82

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[gemini-code-assist]

Warning

Gemini encountered an error creating the review. You can try again by commenting /gemini review.

[coderabbitai]

Warning

Review limit reached

@Ishant5436, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 25 minutes and 45 seconds. Learn how PR review limits work.

Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file).

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits.

🚦 How do rate limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: adcceeb5-607e-4476-9b3a-e612b8ad8321

📥 Commits

Reviewing files that changed from the base of the PR and between 7fabf27 and 44add82.

📒 Files selected for processing (1)

• contracts/StakedEXA.sol

Walkthrough

In StakedEXA.sol, the harvest() function gains a single conditional guard: if the computed sharesReward equals zero, the call to notifyRewardAmount() is skipped, preventing unnecessary reward period updates and emissions.

Changes

harvest() zero-reward notification guard

Layer / File(s)	Summary
Conditional guard on notifyRewardAmount contracts/StakedEXA.sol	Wraps notifyRewardAmount(...) call in harvest() with if (sharesReward > 0) condition to skip reward notification, period updates, and RewardAmountNotified emissions when no reward accrues.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: preventing reward dilution by making the notifyRewardAmount call conditional on non-zero sharesReward.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

✨ Simplify code

• Create PR with simplified code

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

contracts/StakedEXA.sol (1)

352-356: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Early return drops the “send remainder to savings” path when reward rounds to zero.

With the new guard, harvest() exits before save is transferred. If shares > 0 and sharesReward == 0 (e.g., providerRatio == 0 or rounding), assets are no longer moved to savings, which changes accounting/fund flow behavior.

Suggested fix

 function harvest() external whenNotPaused {
   Market memMarket = market;
   address memProvider = provider;
   uint256 shares = Math.min(memMarket.allowance(memProvider, address(this)), memMarket.maxRedeem(memProvider));
   uint256 sharesReward = shares.mulWadDown(providerRatio);
-  if (sharesReward == 0) return;
+  if (shares == 0) return;
+
+  uint256 save = shares - sharesReward;
+  if (save != 0) memMarket.transferFrom(memProvider, savings, save);
+  if (sharesReward == 0) return;
 
   memMarket.transferFrom(provider, address(this), sharesReward);
-  uint256 save = shares - sharesReward;
-  if (save != 0) memMarket.transferFrom(memProvider, savings, save);
 
   notifyRewardAmount(IERC20(address(memMarket)), sharesReward, address(this));
 }

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: d9d6a2e3-676a-430b-bd34-c92160f45607

📥 Commits

Reviewing files that changed from the base of the PR and between 5c1d934 and 685d874.

📒 Files selected for processing (1)

• contracts/StakedEXA.sol

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 685d8746ef

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Do not skip savings transfer when reward share is zero

This return fires not only when there are no provider shares, but also when providerRatio is 0 or the reward portion rounds down to 0. setProviderRatio permits 0 and the harvest path documents that the non-rewarded portion goes to savings; with a zero ratio, shares can be the provider's full redeemable balance but the function exits before transferring save = shares - sharesReward, so deployments that route all provider funds to savings leave them in the provider indefinitely.

Useful? React with 👍 / 👎.

[openzeppelin-code]

fix: prevent reward dilution via zero sharesReward harvest

Generated at commit: 44add828fd0f6cce5d163de0f86ba2f7d9d723ee

🚨 Report Summary

	Severity Level	Results
Contracts	Critical High Medium Low Note Total	2 4 0 7 26 39
Dependencies	Critical High Medium Low Note Total	0 0 0 0 0 0

---

For more details view the full report in OpenZeppelin Code Inspector

[Ishant5436]

I've applied the suggested fix! The early return was removed so transferFrom executes even when sharesReward rounds down to 0, which preserves the expected revert on zero-address and correctly transfers save to savings as long as it's non-zero. Tests pass. Thanks for catching that!

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)

contracts/StakedEXA.sol (2)

347-358: 🧹 Nitpick | 🔵 Trivial | 💤 Low value

Consider adding early return when shares is zero.

When both the allowance and maxRedeem are zero, shares will be zero, making both sharesReward and save zero. The current implementation executes a zero-value transferFrom at line 353, which wastes gas without accomplishing anything useful.

⚡ Proposed gas optimization

 function harvest() external whenNotPaused {
   Market memMarket = market;
   address memProvider = provider;
   uint256 shares = Math.min(memMarket.allowance(memProvider, address(this)), memMarket.maxRedeem(memProvider));
+  if (shares == 0) return;
   uint256 sharesReward = shares.mulWadDown(providerRatio);

---

29-29: ⚠️ Potential issue | 🟠 Major

Update Solidity pragma version to comply with coding guidelines.

The pragma is currently set to ^0.8.0 but must be ^0.8.17 per the coding guidelines. Update line 2:

pragma solidity ^0.8.17;

Source: Coding guidelines

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: b19124ec-195a-4a9d-babc-c7ac8fc89876

📥 Commits

Reviewing files that changed from the base of the PR and between 685d874 and 7fabf27.

📒 Files selected for processing (1)

• contracts/StakedEXA.sol

[Ishant5436]

@coderabbitai I've updated the pragma to ^0.8.17! Regarding the gas optimization suggestion to add if (shares == 0) return;: this was intentionally left out because the test suite (testHarvestWhenFinished) expects harvest() to revert with ERC20InvalidApprover(address(0)) when provider == address(0). An early return would skip the zero-value transferFrom and cause that test to fail since the transaction would succeed rather than revert.

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

@Ishant5436 That's a well-reasoned explanation — thanks for walking through it! The intentional omission makes sense: the zero-value transferFrom call on a zero-address provider serves as a deliberate guard that enforces the revert via ERC20InvalidApprover(address(0)). Skipping it with an early return would silently swallow what is expected to be an error condition, breaking testHarvestWhenFinished.

I'm withdrawing the gas optimization suggestion accordingly.

🐇 ✨

---

✏️ Learnings added

Learnt from: Ishant5436
URL: https://github.com/exactly/protocol/pull/808

Timestamp: 2026-06-21T07:37:00.254Z
Learning: In `StakedEXA.sol` (Solidity, Exactly Finance protocol), the `harvest()` function intentionally does NOT include an early `if (shares == 0) return;` guard. When `provider == address(0)`, the zero-value `transferFrom` call must still execute so it reverts with `ERC20InvalidApprover(address(0))`. The test `testHarvestWhenFinished` relies on this revert behavior. Adding an early return would break this test by allowing the function to succeed silently instead of reverting.

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.