From c36d38e6a25e55809d7690c6a3ea29cd07fae501 Mon Sep 17 00:00:00 2001 From: Tony Chen Date: Fri, 10 Jul 2026 12:20:41 +0800 Subject: [PATCH 1/2] fix(distribution): make delegation-reward queries read-only MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The distribution reward query handlers (DelegationRewards / DelegationTotalRewards in grpc_query.go, and their amino equivalents in querier.go) call IncrementValidatorPeriod, which mutates distribution state (validator period, historical/current rewards, reference counts, and the zero-token community-pool transfer). Queries are meant to be side-effect free — the amino querier already branches a CacheContext "to isolate state changes", and the gRPC query server runs on a cached context — but the EVM distribution `rewards()` precompile calls the gRPC handler with the live context, so those writes actually persist and commit with the transaction. Replace the "IncrementValidatorPeriod then CalculateDelegationRewards" sequence in the query paths with a read-only computation: - Add currentRewardsEndingPeriodAndRatio, which computes in memory the ending period and cumulative reward ratio that IncrementValidatorPeriod would persist, without writing. - Factor CalculateDelegationRewards into a shared core that accepts the ending ratio; CalculateDelegationRewardsReadOnly passes the in-memory ratio and never writes. The state-changing (withdraw) path passes nil so the core reads the ending ratio from the store lazily in the final period — byte-for-byte the same store accesses and gas as before (verified: precompile withdraw gas unchanged). Tests assert the read-only result equals the write-path result (including the slash-event path) and that neither the helper nor the gRPC query mutates the validator period or historical reference count, and that repeated queries are idempotent. NOTE (gating): the gRPC handlers are called by the EVM distribution precompile (current and every legacy version), so no longer persisting these writes changes committed state for any tx that called `rewards()`. This is state-machine-breaking and must ship gated (precompile version / upgrade height) to preserve historical replay. Gating is intentionally left out of this focused change. Co-Authored-By: Claude Fable 5 --- .../x/distribution/keeper/delegation.go | 94 +++++++++++++-- .../keeper/delegation_readonly_test.go | 108 ++++++++++++++++++ .../x/distribution/keeper/grpc_query.go | 10 +- sei-cosmos/x/distribution/keeper/querier.go | 11 +- 4 files changed, 205 insertions(+), 18 deletions(-) create mode 100644 sei-cosmos/x/distribution/keeper/delegation_readonly_test.go diff --git a/sei-cosmos/x/distribution/keeper/delegation.go b/sei-cosmos/x/distribution/keeper/delegation.go index 825654e914..6796b4d6e6 100644 --- a/sei-cosmos/x/distribution/keeper/delegation.go +++ b/sei-cosmos/x/distribution/keeper/delegation.go @@ -29,6 +29,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 +57,64 @@ 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) +} + +// 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 +192,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..c23fe8b5a9 --- /dev/null +++ b/sei-cosmos/x/distribution/keeper/delegation_readonly_test.go @@ -0,0 +1,108 @@ +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" + "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 no longer +// mutates 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) +} diff --git a/sei-cosmos/x/distribution/keeper/grpc_query.go b/sei-cosmos/x/distribution/keeper/grpc_query.go index 057fb76456..324f615d67 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: this is a query, so compute rewards without persisting a + // validator-period increment (see CalculateDelegationRewardsReadOnly). + rewards := k.CalculateDelegationRewardsReadOnly(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: this is a query, so compute rewards without persisting a + // validator-period increment (see CalculateDelegationRewardsReadOnly). + delReward := k.CalculateDelegationRewardsReadOnly(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..008d6f1150 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: compute rewards without persisting a validator-period increment + // (see CalculateDelegationRewardsReadOnly). The CacheContext above already + // isolated any writes, so this path is state-neutral either way. + rewards := k.CalculateDelegationRewardsReadOnly(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: compute rewards without persisting a validator-period + // increment (see CalculateDelegationRewardsReadOnly). + delReward := k.CalculateDelegationRewardsReadOnly(ctx, val, del) delRewards = append(delRewards, types.NewDelegationDelegatorReward(valAddr, delReward)) total = total.Add(delReward...) From 09008d00eb8f4476e71f44c848fdc7fe33594ae2 Mon Sep 17 00:00:00 2001 From: Tony Chen Date: Mon, 13 Jul 2026 12:56:27 +0800 Subject: [PATCH 2/2] gate read-only reward queries behind v6.7 (tracing only) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The read-only reward-query change is state-machine-breaking on the EVM distribution `rewards()` precompile path (which ran the query on the live context and persisted IncrementValidatorPeriod's writes). Gate it so historical re-execution stays deterministic. Live (non-tracing) execution always uses the read-only path: the current binary only ever executes blocks at/after the upgrade that ships this behavior, so a non-tracing block is never a pre-upgrade block (cosmovisor runs the prior binary for those; we never replay pre-upgrade txs non-tracingly). The only place the old mutating behavior must be reproduced is re-tracing a historical pre-v6.7 block via debug_trace*, where the era is signaled by the trace harness through ClosestUpgradeName and compared by semver — matching the existing x/evm/keeper/params.go idiom (`if !ctx.IsTracing() { return current }`). func (k Keeper) rewardQueryIsReadOnly(ctx sdk.Context) bool { if !ctx.IsTracing() { return true } return semver.Compare(ctx.ClosestUpgradeName(), ReadOnlyRewardsUpgrade) >= 0 } This drops the earlier live-path gate (UpgradeChecker.IsUpgradeActiveAtHeight and its wiring): it was unnecessary given the above, and it had introduced an exact-match-vs-semver naming hazard between the two paths. With only the semver tracing gate the hazard is gone, so the app-level reconciliation guardrail test is removed too. ReadOnlyRewardsUpgrade stays "v6.7" (app/tags convention: v6.5, v6.6). The reward figure is identical in both paths; only whether the query mutates state differs. TestDelegationRewardsForQueryUpgradeGate covers: live is read-only, tracing pre-v6.7 mutates, tracing v6.7+ is read-only. Co-Authored-By: Claude Fable 5 --- .../x/distribution/keeper/delegation.go | 42 ++++++++++++++++ .../keeper/delegation_readonly_test.go | 49 +++++++++++++++++-- .../x/distribution/keeper/grpc_query.go | 12 ++--- sei-cosmos/x/distribution/keeper/querier.go | 14 +++--- 4 files changed, 101 insertions(+), 16 deletions(-) diff --git a/sei-cosmos/x/distribution/keeper/delegation.go b/sei-cosmos/x/distribution/keeper/delegation.go index 6796b4d6e6..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 @@ -106,6 +116,38 @@ func (k Keeper) CalculateDelegationRewardsReadOnly(ctx sdk.Context, val stakingt 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 diff --git a/sei-cosmos/x/distribution/keeper/delegation_readonly_test.go b/sei-cosmos/x/distribution/keeper/delegation_readonly_test.go index c23fe8b5a9..2a1746ef55 100644 --- a/sei-cosmos/x/distribution/keeper/delegation_readonly_test.go +++ b/sei-cosmos/x/distribution/keeper/delegation_readonly_test.go @@ -8,6 +8,7 @@ import ( 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" @@ -78,9 +79,9 @@ func TestCalculateDelegationRewardsReadOnlyMatchesWritePath(t *testing.T) { require.Equal(t, sdk.DecCoins{{Denom: sdk.DefaultBondDenom, Amount: initial.QuoRaw(2).ToDec()}}, readOnly) } -// TestDelegationTotalRewardsQueryIsReadOnly asserts the gRPC query no longer -// mutates distribution state and is idempotent across repeated calls (before the -// fix, each call incremented the validator period). +// 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) @@ -106,3 +107,45 @@ func TestDelegationTotalRewardsQueryIsReadOnly(t *testing.T) { 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 324f615d67..0c0a098610 100644 --- a/sei-cosmos/x/distribution/keeper/grpc_query.go +++ b/sei-cosmos/x/distribution/keeper/grpc_query.go @@ -150,9 +150,9 @@ func (k Keeper) DelegationRewards(c context.Context, req *types.QueryDelegationR return nil, types.ErrNoDelegationExists } - // Read-only: this is a query, so compute rewards without persisting a - // validator-period increment (see CalculateDelegationRewardsReadOnly). - rewards := k.CalculateDelegationRewardsReadOnly(ctx, val, del) + // 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 } @@ -182,9 +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) - // Read-only: this is a query, so compute rewards without persisting a - // validator-period increment (see CalculateDelegationRewardsReadOnly). - delReward := k.CalculateDelegationRewardsReadOnly(ctx, val, del) + // 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 008d6f1150..a88c0af9f5 100644 --- a/sei-cosmos/x/distribution/keeper/querier.go +++ b/sei-cosmos/x/distribution/keeper/querier.go @@ -142,10 +142,10 @@ func queryDelegationRewards(ctx sdk.Context, _ []string, req abci.RequestQuery, return nil, types.ErrNoDelegationExists } - // Read-only: compute rewards without persisting a validator-period increment - // (see CalculateDelegationRewardsReadOnly). The CacheContext above already - // isolated any writes, so this path is state-neutral either way. - rewards := k.CalculateDelegationRewardsReadOnly(ctx, val, del) + // 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{} } @@ -177,9 +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) - // Read-only: compute rewards without persisting a validator-period - // increment (see CalculateDelegationRewardsReadOnly). - delReward := k.CalculateDelegationRewardsReadOnly(ctx, val, del) + // 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...)