diff --git a/sei-cosmos/x/distribution/keeper/delegation.go b/sei-cosmos/x/distribution/keeper/delegation.go index 825654e914..12020de5d8 100644 --- a/sei-cosmos/x/distribution/keeper/delegation.go +++ b/sei-cosmos/x/distribution/keeper/delegation.go @@ -3,12 +3,22 @@ package keeper import ( "fmt" + "golang.org/x/mod/semver" + sdk "github.com/sei-protocol/sei-chain/sei-cosmos/types" "github.com/sei-protocol/sei-chain/sei-cosmos/x/distribution/types" stakingtypes "github.com/sei-protocol/sei-chain/sei-cosmos/x/staking/types" "github.com/sei-protocol/seilog" ) +// ReadOnlyRewardsUpgrade is the upgrade at which delegation-reward queries stopped +// persisting a validator-period increment (see DelegationRewardsForQuery). It is +// only consulted when re-tracing a historical block (ctx.IsTracing()), and only via +// semver comparison against ctx.ClosestUpgradeName(), so it just needs to be +// semver-equal to that release. It follows the app/tags convention (…, v6.5, v6.6), +// hence "v6.7". +const ReadOnlyRewardsUpgrade = "v6.7" + var logger = seilog.NewLogger("cosmos", "x", "distribution", "keeper") // initialize starting info for a new delegation @@ -29,6 +39,18 @@ func (k Keeper) initializeDelegation(ctx sdk.Context, val sdk.ValAddress, del sd k.SetDelegatorStartingInfo(ctx, val, del, types.NewDelegatorStartingInfo(previousPeriod, stake, uint64(ctx.BlockHeight()))) //nolint:gosec // block heights are always non-negative } +// rewardsFromRatios returns stake * (endingRatio - startingRatio), truncated. It +// centralizes the ratio math shared by the store-backed reward calculation and the +// read-only calculation used by queries (which supply an in-memory ending ratio). +func rewardsFromRatios(startingRatio, endingRatio sdk.DecCoins, stake sdk.Dec) sdk.DecCoins { + difference := endingRatio.Sub(startingRatio) + if difference.IsAnyNegative() { + panic("negative rewards should not be possible") + } + // note: necessary to truncate so we don't allow withdrawing more rewards than owed + return difference.MulDecTruncate(stake) +} + // calculate the rewards accrued by a delegation between two periods func (k Keeper) calculateDelegationRewardsBetween(ctx sdk.Context, val stakingtypes.ValidatorI, startingPeriod, endingPeriod uint64, stake sdk.Dec) (rewards sdk.DecCoins) { @@ -45,17 +67,96 @@ func (k Keeper) calculateDelegationRewardsBetween(ctx sdk.Context, val stakingty // return staking * (ending - starting) starting := k.GetValidatorHistoricalRewards(ctx, val.GetOperator(), startingPeriod) ending := k.GetValidatorHistoricalRewards(ctx, val.GetOperator(), endingPeriod) - difference := ending.CumulativeRewardRatio.Sub(starting.CumulativeRewardRatio) - if difference.IsAnyNegative() { - panic("negative rewards should not be possible") - } - // note: necessary to truncate so we don't allow withdrawing more rewards than owed - rewards = difference.MulDecTruncate(stake) - return + return rewardsFromRatios(starting.CumulativeRewardRatio, ending.CumulativeRewardRatio, stake) } -// calculate the total rewards accrued by a delegation +// CalculateDelegationRewards calculates the total rewards accrued by a delegation +// up to endingPeriod, reading that period's cumulative reward ratio from the +// store. State-changing callers (e.g. withdrawals) persist endingPeriod via +// IncrementValidatorPeriod before calling this. Passing a nil ending ratio makes +// the core read endingPeriod from the store lazily (only if the final period is +// reached), preserving this path's exact store-access pattern and gas cost. func (k Keeper) CalculateDelegationRewards(ctx sdk.Context, val stakingtypes.ValidatorI, del stakingtypes.DelegationI, endingPeriod uint64) (rewards sdk.DecCoins) { + return k.calculateDelegationRewards(ctx, val, del, endingPeriod, nil) +} + +// currentRewardsEndingPeriodAndRatio returns the ending period and its cumulative +// reward ratio that IncrementValidatorPeriod WOULD produce for the validator's +// current (open) period — computed in memory, WITHOUT writing to the store. This +// lets read-only reward queries avoid the state mutation (historical/current +// rewards, reference counts, and the zero-token community-pool transfer) that +// IncrementValidatorPeriod performs. The returned ratio is identical to the one +// IncrementValidatorPeriod would persist for that period, so the resulting reward +// figure matches the state-changing path exactly. +func (k Keeper) currentRewardsEndingPeriodAndRatio(ctx sdk.Context, val stakingtypes.ValidatorI) (uint64, sdk.DecCoins) { + rewards := k.GetValidatorCurrentRewards(ctx, val.GetOperator()) + + var current sdk.DecCoins + if val.GetTokens().IsZero() { + // The state-changing path routes a zero-token validator's rewards to the + // community pool and uses a zero ratio; a read-only query only needs the + // (zero) ratio, not the transfer. + current = sdk.DecCoins{} + } else { + // note: necessary to truncate so we don't allow withdrawing more rewards than owed + current = rewards.Rewards.QuoDecTruncate(val.GetTokens().ToDec()) + } + + historical := k.GetValidatorHistoricalRewards(ctx, val.GetOperator(), rewards.Period-1).CumulativeRewardRatio + return rewards.Period, historical.Add(current...) +} + +// CalculateDelegationRewardsReadOnly computes a delegation's outstanding rewards +// without mutating any state. It mirrors the "IncrementValidatorPeriod then +// CalculateDelegationRewards" sequence used by the withdraw path, but derives the +// ending period's cumulative reward ratio in memory instead of persisting it, so +// it is safe to call from read-only queries. +func (k Keeper) CalculateDelegationRewardsReadOnly(ctx sdk.Context, val stakingtypes.ValidatorI, del stakingtypes.DelegationI) sdk.DecCoins { + endingPeriod, endingRatio := k.currentRewardsEndingPeriodAndRatio(ctx, val) + return k.calculateDelegationRewards(ctx, val, del, endingPeriod, &endingRatio) +} + +// DelegationRewardsForQuery computes a delegation's rewards for a read-only reward +// query. It uses the non-mutating read-only path (CalculateDelegationRewardsReadOnly). +// Before v6.7 these queries persisted a validator-period increment +// (IncrementValidatorPeriod), which committed when the EVM distribution `rewards()` +// precompile ran the query on the live context. That old behavior only needs to be +// reproduced when re-tracing a pre-v6.7 block via debug_trace* — live execution runs +// only on the current (post-upgrade) binary, which never non-tracingly re-executes a +// pre-upgrade block (see rewardQueryIsReadOnly). +func (k Keeper) DelegationRewardsForQuery(ctx sdk.Context, val stakingtypes.ValidatorI, del stakingtypes.DelegationI) sdk.DecCoins { + if k.rewardQueryIsReadOnly(ctx) { + return k.CalculateDelegationRewardsReadOnly(ctx, val, del) + } + // Pre-v6.7 behavior: the query incremented the validator period and read the + // freshly-persisted ending ratio. Reproduce it exactly for deterministic replay. + endingPeriod := k.IncrementValidatorPeriod(ctx, val) + return k.CalculateDelegationRewards(ctx, val, del, endingPeriod) +} + +// rewardQueryIsReadOnly reports whether a reward query should use the non-mutating +// read-only path. Live execution always does: the current binary only ever executes +// at/after the upgrade that shipped this behavior, so a non-tracing block is never a +// pre-upgrade block. The only place the old, state-mutating behavior must be +// reproduced is when re-tracing a historical pre-v6.7 block via debug_trace*, where +// the era is signaled by the trace harness through ClosestUpgradeName (see +// app.RPCContextProvider) and compared by semver (empty name sorts before v6.7). +func (k Keeper) rewardQueryIsReadOnly(ctx sdk.Context) bool { + if !ctx.IsTracing() { + return true + } + return semver.Compare(ctx.ClosestUpgradeName(), ReadOnlyRewardsUpgrade) >= 0 +} + +// calculateDelegationRewards is the shared core of the reward calculation. When +// endingRatio is non-nil it is used as the cumulative reward ratio at endingPeriod +// for the final period, so read-only callers can pass an in-memory ratio and avoid +// persisting the period. When endingRatio is nil the ratio is read from the store +// (the state-changing withdraw path, which persists it via IncrementValidatorPeriod +// beforehand); reading it lazily here — only if the final period is reached — +// preserves that path's exact store-access pattern and gas cost. Intermediate +// slash periods are always read from the store (they are always persisted). +func (k Keeper) calculateDelegationRewards(ctx sdk.Context, val stakingtypes.ValidatorI, del stakingtypes.DelegationI, endingPeriod uint64, endingRatio *sdk.DecCoins) (rewards sdk.DecCoins) { // fetch starting info for delegation startingInfo := k.GetDelegatorStartingInfo(ctx, del.GetValidatorAddr(), del.GetDelegatorAddr()) @@ -133,8 +234,23 @@ func (k Keeper) CalculateDelegationRewards(ctx sdk.Context, val stakingtypes.Val } } - // calculate rewards for final period - rewards = rewards.Add(k.calculateDelegationRewardsBetween(ctx, val, startingPeriod, endingPeriod, stake)...) + // calculate rewards for the final period (mirrors calculateDelegationRewardsBetween: + // same sanity checks and same two historical-rewards reads for the nil path). + if startingPeriod > endingPeriod { + panic("startingPeriod cannot be greater than endingPeriod") + } + if stake.IsNegative() { + panic("stake should not be negative") + } + startingRatio := k.GetValidatorHistoricalRewards(ctx, val.GetOperator(), startingPeriod).CumulativeRewardRatio + finalRatio := endingRatio + if finalRatio == nil { + // state-changing path: read the persisted ending ratio here (lazily, only + // once the final period is reached) to preserve the original gas cost. + ratio := k.GetValidatorHistoricalRewards(ctx, val.GetOperator(), endingPeriod).CumulativeRewardRatio + finalRatio = &ratio + } + rewards = rewards.Add(rewardsFromRatios(startingRatio, *finalRatio, stake)...) return rewards } diff --git a/sei-cosmos/x/distribution/keeper/delegation_readonly_test.go b/sei-cosmos/x/distribution/keeper/delegation_readonly_test.go new file mode 100644 index 0000000000..2a1746ef55 --- /dev/null +++ b/sei-cosmos/x/distribution/keeper/delegation_readonly_test.go @@ -0,0 +1,151 @@ +package keeper_test + +import ( + "testing" + + tmproto "github.com/sei-protocol/sei-chain/sei-tendermint/proto/tendermint/types" + "github.com/stretchr/testify/require" + + seiapp "github.com/sei-protocol/sei-chain/app" + sdk "github.com/sei-protocol/sei-chain/sei-cosmos/types" + distrkeeper "github.com/sei-protocol/sei-chain/sei-cosmos/x/distribution/keeper" + "github.com/sei-protocol/sei-chain/sei-cosmos/x/staking" + "github.com/sei-protocol/sei-chain/sei-cosmos/x/staking/teststaking" + stakingtypes "github.com/sei-protocol/sei-chain/sei-cosmos/x/staking/types" + + types "github.com/sei-protocol/sei-chain/sei-cosmos/x/distribution/types" +) + +// setupRewardsWithSlash creates a bonded validator with a self-delegation, slashes +// it, and allocates rewards, leaving accrued (un-incremented) rewards so both the +// simple and slash-event reward paths are exercised. It returns the context, the +// validator, the delegation, and the initial allocation. +func setupRewardsWithSlash(t *testing.T) (sdk.Context, *seiapp.App, stakingtypes.ValidatorI, stakingtypes.DelegationI, sdk.Int, sdk.ValAddress) { + t.Helper() + app := seiapp.Setup(t, false, false, false) + ctx := app.BaseApp.NewContext(false, tmproto.Header{}) + + addr := seiapp.AddTestAddrs(app, ctx, 2, sdk.NewInt(100000000)) + valAddrs := seiapp.ConvertAddrsToValAddrs(addr) + tstaking := teststaking.NewHelper(t, ctx, app.StakingKeeper) + + tstaking.Commission = stakingtypes.NewCommissionRates(sdk.NewDecWithPrec(5, 1), sdk.NewDecWithPrec(5, 1), sdk.NewDec(0)) + valPower := int64(100) + tstaking.CreateValidatorWithValPower(valAddrs[0], valConsPk1, valPower, true) + + staking.EndBlocker(ctx, app.StakingKeeper) + ctx = ctx.WithBlockHeight(ctx.BlockHeight() + 1) + + // slash so the reward computation walks a slash event + ctx = ctx.WithBlockHeight(ctx.BlockHeight() + 3) + app.StakingKeeper.Slash(ctx, valConsAddr1, ctx.BlockHeight(), valPower, sdk.NewDecWithPrec(5, 1)) + val := app.StakingKeeper.Validator(ctx, valAddrs[0]) + + ctx = ctx.WithBlockHeight(ctx.BlockHeight() + 3) + initial := app.StakingKeeper.TokensFromConsensusPower(ctx, 10) + tokens := sdk.DecCoins{{Denom: sdk.DefaultBondDenom, Amount: initial.ToDec()}} + app.DistrKeeper.AllocateTokensToValidator(ctx, val, tokens) + + del := app.StakingKeeper.Delegation(ctx, sdk.AccAddress(valAddrs[0]), valAddrs[0]) + return ctx, app, val, del, initial, valAddrs[0] +} + +// TestCalculateDelegationRewardsReadOnlyMatchesWritePath asserts the read-only +// reward computation (1) returns exactly what the IncrementValidatorPeriod + +// CalculateDelegationRewards sequence returns, and (2) mutates no distribution +// state. +func TestCalculateDelegationRewardsReadOnlyMatchesWritePath(t *testing.T) { + ctx, app, val, del, initial, valAddr := setupRewardsWithSlash(t) + + // snapshot the state IncrementValidatorPeriod would mutate + periodBefore := app.DistrKeeper.GetValidatorCurrentRewards(ctx, valAddr).Period + refCountBefore := app.DistrKeeper.GetValidatorHistoricalReferenceCount(ctx) + + readOnly := app.DistrKeeper.CalculateDelegationRewardsReadOnly(ctx, val, del) + + // (2) no state mutated by the read-only path + require.Equal(t, periodBefore, app.DistrKeeper.GetValidatorCurrentRewards(ctx, valAddr).Period, + "read-only reward calc must not increment the validator period") + require.Equal(t, refCountBefore, app.DistrKeeper.GetValidatorHistoricalReferenceCount(ctx), + "read-only reward calc must not change the historical reference count") + + // (1) equals the write path, computed on an isolated (cached) context + cctx, _ := ctx.CacheContext() + endingPeriod := app.DistrKeeper.IncrementValidatorPeriod(cctx, val) + writePath := app.DistrKeeper.CalculateDelegationRewards(cctx, val, del, endingPeriod) + require.Equal(t, writePath, readOnly, "read-only rewards must equal the write-path rewards") + + // sanity: the slashing scenario yields half the allocation (other half is commission) + require.Equal(t, sdk.DecCoins{{Denom: sdk.DefaultBondDenom, Amount: initial.QuoRaw(2).ToDec()}}, readOnly) +} + +// TestDelegationTotalRewardsQueryIsReadOnly asserts the gRPC query (live, i.e. not +// tracing) does not mutate distribution state and is idempotent across repeated +// calls (before the fix, each call incremented the validator period). +func TestDelegationTotalRewardsQueryIsReadOnly(t *testing.T) { + ctx, app, _, _, _, valAddr := setupRewardsWithSlash(t) + + periodBefore := app.DistrKeeper.GetValidatorCurrentRewards(ctx, valAddr).Period + refCountBefore := app.DistrKeeper.GetValidatorHistoricalReferenceCount(ctx) + + req := &types.QueryDelegationTotalRewardsRequest{ + DelegatorAddress: sdk.AccAddress(valAddr).String(), + } + + resp1, err := app.DistrKeeper.DelegationTotalRewards(sdk.WrapSDKContext(ctx), req) + require.NoError(t, err) + require.False(t, resp1.Total.IsZero(), "test should have non-zero rewards") + + require.Equal(t, periodBefore, app.DistrKeeper.GetValidatorCurrentRewards(ctx, valAddr).Period, + "query must not increment the validator period") + require.Equal(t, refCountBefore, app.DistrKeeper.GetValidatorHistoricalReferenceCount(ctx), + "query must not change the historical reference count") + + // idempotent: a second query returns identical totals and still no mutation + resp2, err := app.DistrKeeper.DelegationTotalRewards(sdk.WrapSDKContext(ctx), req) + require.NoError(t, err) + require.Equal(t, resp1.Total, resp2.Total) + require.Equal(t, periodBefore, app.DistrKeeper.GetValidatorCurrentRewards(ctx, valAddr).Period) +} + +// TestDelegationRewardsForQueryUpgradeGate asserts the gating: live (non-tracing) +// execution always uses the read-only path, while re-tracing a historical block +// reproduces the old mutating behavior for pre-v6.7 blocks and the read-only path +// for v6.7+ blocks. The reward figure is identical across every path. +func TestDelegationRewardsForQueryUpgradeGate(t *testing.T) { + period := func(app *seiapp.App, ctx sdk.Context, valAddr sdk.ValAddress) uint64 { + return app.DistrKeeper.GetValidatorCurrentRewards(ctx, valAddr).Period + } + expected := func(initial sdk.Int) sdk.DecCoins { + return sdk.DecCoins{{Denom: sdk.DefaultBondDenom, Amount: initial.QuoRaw(2).ToDec()}} + } + + t.Run("live is read-only", func(t *testing.T) { + ctx, app, val, del, initial, valAddr := setupRewardsWithSlash(t) + before := period(app, ctx, valAddr) + reward := app.DistrKeeper.DelegationRewardsForQuery(ctx, val, del) + require.Equal(t, before, period(app, ctx, valAddr), + "live (non-tracing) execution must not mutate") + require.Equal(t, expected(initial), reward) + }) + + t.Run("tracing pre-v6.7 mutates", func(t *testing.T) { + ctx, app, val, del, initial, valAddr := setupRewardsWithSlash(t) + traceOld := ctx.WithIsTracing(true).WithClosestUpgradeName("v6.6") + before := period(app, traceOld, valAddr) + reward := app.DistrKeeper.DelegationRewardsForQuery(traceOld, val, del) + require.Equal(t, before+1, period(app, traceOld, valAddr), + "tracing a pre-v6.7 block must reproduce the period increment") + require.Equal(t, expected(initial), reward) + }) + + t.Run("tracing v6.7+ is read-only", func(t *testing.T) { + ctx, app, val, del, initial, valAddr := setupRewardsWithSlash(t) + traceNew := ctx.WithIsTracing(true).WithClosestUpgradeName(distrkeeper.ReadOnlyRewardsUpgrade) + before := period(app, traceNew, valAddr) + reward := app.DistrKeeper.DelegationRewardsForQuery(traceNew, val, del) + require.Equal(t, before, period(app, traceNew, valAddr), + "tracing a v6.7+ block must not mutate") + require.Equal(t, expected(initial), reward) + }) +} diff --git a/sei-cosmos/x/distribution/keeper/grpc_query.go b/sei-cosmos/x/distribution/keeper/grpc_query.go index 057fb76456..0c0a098610 100644 --- a/sei-cosmos/x/distribution/keeper/grpc_query.go +++ b/sei-cosmos/x/distribution/keeper/grpc_query.go @@ -150,8 +150,9 @@ func (k Keeper) DelegationRewards(c context.Context, req *types.QueryDelegationR return nil, types.ErrNoDelegationExists } - endingPeriod := k.IncrementValidatorPeriod(ctx, val) - rewards := k.CalculateDelegationRewards(ctx, val, del, endingPeriod) + // Read-only for v6.7.0+; reproduces the pre-v6.7.0 period-increment write only + // when re-tracing an older block (see DelegationRewardsForQuery). + rewards := k.DelegationRewardsForQuery(ctx, val, del) return &types.QueryDelegationRewardsResponse{Rewards: rewards}, nil } @@ -181,8 +182,9 @@ func (k Keeper) DelegationTotalRewards(c context.Context, req *types.QueryDelega func(_ int64, del stakingtypes.DelegationI) (stop bool) { valAddr := del.GetValidatorAddr() val := k.stakingKeeper.Validator(ctx, valAddr) - endingPeriod := k.IncrementValidatorPeriod(ctx, val) - delReward := k.CalculateDelegationRewards(ctx, val, del, endingPeriod) + // Read-only for v6.7.0+; reproduces the pre-v6.7.0 period-increment write + // only when re-tracing an older block (see DelegationRewardsForQuery). + delReward := k.DelegationRewardsForQuery(ctx, val, del) delRewards = append(delRewards, types.NewDelegationDelegatorReward(valAddr, delReward)) total = total.Add(delReward...) diff --git a/sei-cosmos/x/distribution/keeper/querier.go b/sei-cosmos/x/distribution/keeper/querier.go index 2503448e08..a88c0af9f5 100644 --- a/sei-cosmos/x/distribution/keeper/querier.go +++ b/sei-cosmos/x/distribution/keeper/querier.go @@ -142,8 +142,10 @@ func queryDelegationRewards(ctx sdk.Context, _ []string, req abci.RequestQuery, return nil, types.ErrNoDelegationExists } - endingPeriod := k.IncrementValidatorPeriod(ctx, val) - rewards := k.CalculateDelegationRewards(ctx, val, del, endingPeriod) + // Read-only for v6.7.0+; reproduces the pre-v6.7.0 period-increment write only + // when re-tracing an older block (see DelegationRewardsForQuery). The + // CacheContext above already isolates any such write from committed state. + rewards := k.DelegationRewardsForQuery(ctx, val, del) if rewards == nil { rewards = sdk.DecCoins{} } @@ -175,8 +177,9 @@ func queryDelegatorTotalRewards(ctx sdk.Context, _ []string, req abci.RequestQue func(_ int64, del stakingtypes.DelegationI) (stop bool) { valAddr := del.GetValidatorAddr() val := k.stakingKeeper.Validator(ctx, valAddr) - endingPeriod := k.IncrementValidatorPeriod(ctx, val) - delReward := k.CalculateDelegationRewards(ctx, val, del, endingPeriod) + // Read-only for v6.7.0+; reproduces the pre-v6.7.0 period-increment write + // only when re-tracing an older block (see DelegationRewardsForQuery). + delReward := k.DelegationRewardsForQuery(ctx, val, del) delRewards = append(delRewards, types.NewDelegationDelegatorReward(valAddr, delReward)) total = total.Add(delReward...)