fix: rewards v2.2 audit fixes#494
Conversation
…lia rewardsCoordinator and StrategyManager
- Table 17/20: evaluate refunds per operator-set/operator, not per-snapshot - operatorShareSnapshots: apply allocations to all backfill days, include cutoff snapshot
2a4cc2b to
28444d2
Compare
- stakerShareSnapshots: Add '+ INTERVAL 1 day' to end_time calculation to include cutoff date snapshot (matches operatorShareSnapshots fix) - 15_goldOperatorOperatorSetUniqueStakeRewards: Fix deregistration queue check to only flag the 14-day window AFTER deregistration, not the entire period from registration to slashable_until
33f9055 to
a36a9cc
Compare
| @@ -173,8 +185,9 @@ func (sp *SlashingProcessor) createSlashingAdjustments(slashEvent *SlashingEvent | |||
| @logIndex as log_index | |||
| FROM queued_slashing_withdrawals qsw | |||
There was a problem hiding this comment.
So we're selecting from queued_slashing_withdrawals, but that hasn't been committed yet. Thus, we could have a lag for slashes that occur in the same block as a queued withdrawal. The reason the tests pass is that they insert the slash into the queued withdrawal table (fine for tests, but doesn't match how this works E2E).
Comment from other PR:
Since this query is selecting from queued_slashing_withdrawal table, which hasn't been committed yet (ie. latest block not updated), could we have a staleness issue? Edge case: a withdrawal has been queued in this block and then we are slashed a couple times in the same block.
We'll need to get the accumulator for withdrawals that occurred in this given block
There was a problem hiding this comment.
For a regression, what we can do is something like:
return sharesModel.HandleStateChange(&beaconChainSlashingFactorDecreasedLog)Since the handleStateChange is called prior to committing the final state to the DB.
| } | ||
|
|
||
| func insertQueuedWithdrawalWithLogIndex(t *testing.T, grm *gorm.DB, staker, operator, strategy string, blockNumber uint64, logIndex uint64, shares string) { | ||
| res := grm.Exec(` |
There was a problem hiding this comment.
I realized inserting this directly is causing us to miss the edge case of a withdrawal and a slash in the same block. In this case, queued_slashing_withdrawals won't have its state committed yet and db won't be up to date. For this edge case of a withdrawal and slash in the same block, we should do something like:
return sharesModel.HandleStateChange(&beaconChainSlashingFactorDecreasedLog)And then use the accumulator in slashing.go
| t.Run("CSA-3: Same-block event ordering", func(t *testing.T) { | ||
| cleanupCSATest(t, grm) | ||
|
|
||
| // Setup: Staker queues withdrawal on block 1000, log_index 2 |
There was a problem hiding this comment.
Ordering reversed here, we want to queue withdrawal (early log index) then slash (later log index) per test plan
There was a problem hiding this comment.
This is the edge case for the accumulator
There was a problem hiding this comment.
Added as a new test case.
…ares - Remove FULL OUTER JOIN with operator_allocation_snapshots from query - operator_share_snapshots now contains only raw shares from operator_shares - Allocation magnitudes are stored separately in operator_allocation_snapshots - Table 15/18 rewards calculations join both tables as needed: allocated_stake = shares * magnitude / max_magnitude - This removes the dependency on operator_allocation_snapshots being generated before operator_share_snapshots
c419fde to
9e7094c
Compare
pkg/eigenState/totalStakeRewardSubmissions/totalStakeRewardSubmissions.go
Outdated
Show resolved
Hide resolved
pkg/eigenState/uniqueStakeRewardSubmissions/uniqueStakeRewardSubmissions.go
Outdated
Show resolved
Hide resolved
a53235b to
0f4188d
Compare
…distribution Implement support for UniqueStakeRewardsSubmissionCreated and TotalStakeRewardsSubmissionCreated events with pro-rata distribution across operators. Changes: - Add migration for unique_stake_reward_submissions and total_stake_reward_submissions tables - Create eigenState models for stake-based reward submission events - Add stake_operator_set_rewards denormalized table - Rename Tables 15-21 to 16-22 to accommodate new table - Add Table 15 (ActiveUniqueAndTotalStakeRewards) as source for unique/total stake calculations - Update Tables 16 and 19 to support both OD rewards (per-operator amounts) and stake rewards (pro-rata by operator weight)
0f4188d to
ffcbafb
Compare
| const _16_goldOperatorOperatorSetUniqueStakeRewardsQuery = ` | ||
| CREATE TABLE {{.destTableName}} AS | ||
|
|
||
| -- ============================================================================ |
There was a problem hiding this comment.
Part 1 here doesn't make sense - we are taking active rewards from table 11, but it should be taken from table 15. This query should start from step 2 (it seems like we're doing the same thing in step 1 and step 2)
| pow.avs, pow.operator_set_id, pow.operator, pow.strategy, pow.multiplier, | ||
| pow.reward_submission_date, pow.registration_snapshot, pow.slashable_until, | ||
| pow.operator_allocated_weight, | ||
| 'stake' as reward_source |
There was a problem hiding this comment.
What is the purpose of reward_source?
There was a problem hiding this comment.
Irrelevant after removing Part 1.
| FROM combined_operators | ||
| ), | ||
|
|
||
| -- Common Step 2: Calculate cumulative slash multiplier during deregistration queue |
There was a problem hiding this comment.
When an operator is slashed, we already decrement their shares. The deregistration queue doesn't need to be factored here - this should already be taken care of in operator registration snapshots (via the slashable_until parameter).
The key issue to solve for is not accounting for queued withdrawals for an operator:
- Operator has a staker, Alice. Alice has 500 shares
- Alice queues a withdrawal for 200 shares. The operator now has 300 shares. But, we want the operator to have 500 shares!
How do we ensure that the queued withdrawal for Alice is counted in the operator shares? Here is the discrepancy: Operator shares are decremented immediately when a withdrawal is queued. Therefore, the number of shares an operator has does not handle the queued withdrawal adjustments. To fix, one option there is to add these shares back into operator_share_snapshots, like we do for staker shares.
86a36ea to
2ab53ed
Compare
OD rewards are operator-directed with staker distribution handled by staker share snapshots. Removes duplicate processing and reward_source column.
| Withdrawer: strings.ToLower(outputData.Withdrawal.Withdrawer), | ||
| Nonce: outputData.Withdrawal.Nonce.String(), | ||
| StartBlock: outputData.Withdrawal.StartBlock, | ||
| Strategy: strategy, |
There was a problem hiding this comment.
Strategy is stored without strings.ToLower() here, the slashing adjustment query joins on qsw.strategy = @strategy ,if the original event had mixed-case, the join silently fails and no adjustments are created.
should lowercase the strategy here
Description
Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.
Fixes # (issue)
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
Checklist: