[multiblock-PSE] Fix possible failures in delegation phase#119
Conversation
TxCorpi0x
left a comment
There was a problem hiding this comment.
@TxCorpi0x made 1 comment.
Reviewable status: 0 of 2 files reviewed, 1 unresolved discussion (waiting on masihyeganeh, metalarm10, miladz68, and ysv).
x/pse/keeper/distribute.go line 381 at r1 (raw file):
for _, delegation := range delegations { // Skip fully-slashed validators (Balance=0) and auto-delegate only to healthy ones. // Otherwise SDK Delegate returns ErrDelegatorShareExRateInvalid and disables PSE.
So the only error that may lead to delegation failure and PSE disablement is ErrDelegatorShareExRateInvalid
Can we have the list of all possible errors in a comment, or having TODO for each version to make sure there is no new logic has been defined in the upgrade version of SDK
metalarm10
left a comment
There was a problem hiding this comment.
@metalarm10 made 1 comment.
Reviewable status: 0 of 2 files reviewed, 1 unresolved discussion (waiting on masihyeganeh, miladz68, TxCorpi0x, and ysv).
x/pse/keeper/distribute.go line 381 at r1 (raw file):
Previously, TxCorpi0x wrote…
So the only error that may lead to delegation failure and PSE disablement is
ErrDelegatorShareExRateInvalid
Can we have the list of all possible errors in a comment, or having TODO for each version to make sure there is no new logic has been defined in the upgrade version of SDK
PR description includes all possible errors based on used Staking module functions:
ErrDelegatorShareExRateInvalid: fully-slashed validator; fixed by this PR.ErrInsufficientFunds: unreachable; PSE sends funds to the wallet right beforeDelegatein the same atomic call.ErrInvalidCoins: unreachable;sdk.NewCoinsfilters zero coins,Delegate(val, 0)is a silent no-op.ErrNoValidatorFound: unreachable; validator removal also removes the delegation row.- KV / hook / panic errors: genuinely critical; existing disable behavior is preserved.
If you have some other possible error concerns that we need to handle, please feel free to specify.
TxCorpi0x
left a comment
There was a problem hiding this comment.
@TxCorpi0x reviewed 2 files and all commit messages, and resolved 1 discussion.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on masihyeganeh, miladz68, and ysv).
ysv
left a comment
There was a problem hiding this comment.
@ysv reviewed 2 files and all commit messages.
Reviewable status:
Closes: https://app.clickup.com/t/868j5ydyz
Description
Problem
In PSE reward distribution phase,
distributeToDelegatorfunction callsstakingKeeper.Delegate(...)once per validator the delegator has stake on. If one validator is 100%-slashed (Tokens=0,Shares>0), SDK returnsErrDelegatorShareExRateInvalidfrom thex/stakingmodule, which propagates toEndBlockand disables PSE chain-wide.The existing
totalDelegationAmount.IsZero()check only fires when ALL of the delegator's validators are fully slashed (so the sum of balances is zero). It does NOT fire in the mixed case, where the delegator has at least one healthy validator alongside slashed ones -totalDelegationAmountstays positive, the early-return is skipped, and the per-validator loop reaches the slashed one and triggersErrDelegatorShareExRateInvalid.The exact scenario described above is produced with unit tests, which resulted in PSE being disabled due to returned error from the Staking module.
Fix
Skip zero-balance delegations in the per-validator loop. The proportional split formula already excludes them from the denominator, so the slashed slice is naturally redirected to healthy validators with no leftover. The delegator receives their full reward; but it just lands entirely on healthy delegations.
Why only this error is handled
Staking module keeper method usages in multi-block PSE reviewed:
stakingKeeper.Delegate,GetValidator,DelegatorDelegations. Possible returned error cases are:ErrDelegatorShareExRateInvalid: fully-slashed validator; fixed by this PR.ErrInsufficientFunds: unreachable; PSE sends funds to the wallet right beforeDelegatein the same atomic call.ErrInvalidCoins: unreachable;sdk.NewCoinsfilters zero coins,Delegate(val, 0)is a silent no-op.ErrNoValidatorFound: unreachable; validator removal also removes the delegation row.Based on this assessment, only the fully-slashed validator case requires handling at the PSE layer. All other reachable failures are either unreachable in practice or genuinely critical and should keep the existing disable behavior.
Tests
TestDistribution_SlashedValidator_RedirectsRewardToHealthy: full EndBlocker flow, asserts PSE not disabled, distribution finalized, slashed slice fully redirected to healthy validator (wallet leftover ≤ 1 subunit). Fails without the fix.Reviewers checklist:
Authors checklist
This change is