Skip to content

Conversation

vedhavyas
Copy link
Contributor

How de-registration of operator works?
Operator sends a de-registration extrinsics and status is moved to DeRegistered with ability to unlock funds after specified withdrawal period. After this no more deposits, withdrawals, and unlocking of already initiated withdrawals is accepted. Once the withdrawal period is complete, each nominator can submit unlock_nominator to get their funds back. Operator is also removed from the next_operator set.

How Operator rewards are distributed?
Once a domain ER is confirmed, we note the rewards for operators who submitted that ER and distribute any rewards from that ER to these operators. If the operator is slashed or queued to be slashed, then that operator will not receive these rewards and instead deposited to treasury else they will be give these rewards.

How Epoch is transitioned?
When epoch is complete, protocol transitions given domain to next epoch. Before doing that, it accounts for all the deposits, withdrawals submitted to this operator during the epoch and finalizes them.

If the operator has set any nomination tax percentage then portion of rewards given to operator during the epoch are given to the operator and rest is distributed pro-rate to each nominator of the operator. Now when operator has non-zero tax-amount, protocol automatically deposit that tax-amount to operator as a new deposit.

During the epoch transition, if any operators in the next_operator set has any non-zero deposits or withdrawals, protocol calculates the share price for convert deposits into shares and withdrawals into Ai3.

What happened on taurus?
Seems like operator-8 have de-registered at epoch 23285 but they got a reward at epoch 23299.
So during the epoch transition, operator 8 had a non-zero tax amount and protocol deposited that tax-amount to operator as a new deposit.
Unfortunately, share price for operator 8 is not calculated since they are not in next_operator set as they have de-registered previously even though they had a new deposit done by the protocol.

Later coming to epoch 23347, operator 8 received another reward and had a non-zero tax amount. When depositing this new tax-amount reward for operator 8, protocol tried to convert previous deposit from 23299 but failed to convert to shares since there was no share price.

Due to this, epoch was never transitioned and all the bundles being submitted are failing.

Fix
This PR handles two things

  • when there is a reward for operator which gave rise to new deposit, this change ensures protocol calculates the share price for the operator if the operator is not part of the next_operator set be it InvalidBundelAuthor, which is already handled, and De-registered operators, which this PR handles.
  • When an operator de-registers, during the epoch transition protocol calculates the share price if there exists any non-zero deposits or withdrawals that were initiated in this epoch before operator de-registered.

Code contributor checklist:

Copy link

immunefi-magnus bot commented Aug 2, 2025

🛡️ Immunefi PR Reviews

Thanks for submitting this PR for review!

We’ve assigned 6 code reviewer(s) to take a look. They’ll provide feedback here as soon as possible.

This review is based on the current state of your pull request. If you make changes after the review starts, they won’t be reflected here. To ensure the review includes your latest updates, you’ll need to open a new pull request.

@vedhavyas vedhavyas force-pushed the missing_share_price branch from 4f85096 to 9248e56 Compare August 2, 2025 12:07
@vedhavyas vedhavyas added bug Something isn't working consensus Something that impacts consensus labels Aug 2, 2025
@vedhavyas vedhavyas force-pushed the missing_share_price branch from 9248e56 to 76c1547 Compare August 3, 2025 13:38
Copy link
Member

@teor2345 teor2345 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, and thank you for the tests.

My comment isn't blocking, feel free to fix it in a separate PR.

@@ -60,11 +64,22 @@ pub(crate) fn do_finalize_domain_current_epoch<T: Config>(
})
}

/// Result holding after `operator_take_reward_tax_and_stake`
pub(crate) struct OperatorTakeRewardTaxResult {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Usually in Rust, structs named Result have an Ok and Error case. A better name here would be:

Suggested change
pub(crate) struct OperatorTakeRewardTaxResult {
pub(crate) struct OperatorTakeRewardTaxOutcome {

@vedhavyas vedhavyas added this pull request to the merge queue Aug 4, 2025
Merged via the queue into main with commit f55a2a3 Aug 4, 2025
13 checks passed
@vedhavyas vedhavyas deleted the missing_share_price branch August 4, 2025 04:38
Copy link

@immunefi-magnus immunefi-magnus bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR looks straightforward with the introduction to new storage mechanism for validators. We verified that this doesn't break any functionality and with detailed description of the changes.

  • Oxrudrapratap

@vanhauser-thc vanhauser-thc moved this from Backlog to Scheduled in Security Audit (PRs) Aug 12, 2025
@vanhauser-thc vanhauser-thc moved this from Scheduled to In progress in Security Audit (PRs) Aug 13, 2025
@R9295 R9295 moved this from In progress to Audited in Security Audit (PRs) Aug 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working consensus Something that impacts consensus
Projects
Status: Audited
Development

Successfully merging this pull request may close these issues.

2 participants