@TxCorpi0x made 1 comment.
Reviewable status: 0 of 35 files reviewed, 1 unresolved discussion (waiting on akhlopiachyi, masihyeganeh, metalarm10, miladz68, and ysv).

x/pse/keeper/distribute_test.go line 354 at r1 (raw file):

		}},
	}
	err = pseKeeper.BeginCommunityDistribution(ctx, scheduledDistribution, bondDenom)

This issue is not clear to me:
UpdateDistributionScheduleprevents updates during ongoing distribution, but UpdateExcludedAddresses does not have this check. So the exclusion may happen during phase 2, which modifies TotalScore in the middle of the round.

• Adding exclusion: reduces total denom → increases payouts for remaining delegators in later batches
• Removing exclusion: increases total denom → decreases payouts for remaining delegators in later batches

It is worth having test coverage for that

@metalarm10 made 1 comment.
Reviewable status: 0 of 35 files reviewed, 1 unresolved discussion (waiting on akhlopiachyi, masihyeganeh, miladz68, TxCorpi0x, and ysv).

x/pse/keeper/distribute_test.go line 354 at r1 (raw file):

Done.

@TxCorpi0x reviewed 35 files and all commit messages, and resolved 1 discussion.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on akhlopiachyi, masihyeganeh, miladz68, and ysv).

@ysv reviewed 29 files and all commit messages, and made 5 comments.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on akhlopiachyi, masihyeganeh, metalarm10, and miladz68).

a discussion (no related file):

ExcludedAddressScore store: New Map[AccAddr -> Int] collection. Added addScoreForAddress helper that routes score writes based on exclusion status: excluded addresses accumulate to ExcludedAddressScore instead of AccountScoreSnapshot and TotalScore. Score is restored to AccountScoreSnapshot on re-inclusion. Preserved across genesis export/import.

But we still keep DelegationTimeEntries for excluded addresses, right ?

a discussion (no related file):
TODO myself: check deeper

• Fairness bonus: Normalizes batch-ordering bias. After each delegator receives tokens in distribution phase, bonus score credited to next distribution: bonusScore = distributedAmount × elapsed_seconds.

x/pse/keeper/delegation.go line 75 at r3 (raw file):

// addToExcludedScore atomically adds a score value to an excluded address's accumulated score.
// Unlike addToScore, this does NOT update TotalScore - excluded addresses don't participate in distribution.
func (k Keeper) addToExcludedScore(ctx context.Context, addr sdk.AccAddress, score sdkmath.Int) error {

do we resent this score together with 'main score' in the end of PSE distribution ?

x/pse/keeper/delegation.go line 129 at r3 (raw file):

// moveExcludedScoreToSnapshot moves accumulated ExcludedAddressScore back into AccountScoreSnapshot+TotalScore.
// Called when an address is re-included (removed from the exclusion list).
func (k Keeper) moveExcludedScoreToSnapshot(ctx context.Context, distributionID uint64, addr sdk.AccAddress) error {

nit: as idea to differentiate Excluded score and 'other one' maybe you can name it something like 'main'

so it will be:

• moveExcludedScoreToMain & moveMainScoreToExcluded
• addToExcludedScore & addToMainScore
  etc.

Renaming of store isn't critical

x/pse/keeper/distribution.go line 92 at r3 (raw file):

	communityCoins := sdk.NewCoins(sdk.NewCoin(bondDenom, communityAmount))
	return k.bankKeeper.SendCoinsFromModuleToModule(
		ctx, types.ClearingAccountCommunity, types.ClearingAccountCommunityBuffer, communityCoins,

nit; name suggestion Buffer -> intermediary (similar to name in banking)

If not painful I would rename

Code quote:

ClearingAccountCommunityBuffer

@metalarm10 made 2 comments.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on akhlopiachyi, masihyeganeh, miladz68, and ysv).

a discussion (no related file):

Correct, hooks still fire and record delegation time entries for excluded addresses, with proper shares + timestamp.

x/pse/keeper/delegation.go line 75 at r3 (raw file):

Done. Added pruning of ExcludedAddressScore in cleanup at the end of each distribution round. Now score accumulation will start fresh for each period.

@ysv reviewed 8 files and all commit messages, made 1 comment, and resolved 3 discussions.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on akhlopiachyi, masihyeganeh, metalarm10, and miladz68).

x/pse/types/params.go line 13 at r4 (raw file):

	ClearingAccountCommunity = "pse_community"
	// ClearingAccountCommunityBuffer is a short-lived buffer account used during community distribution.
	ClearingAccountCommunityBuffer = "pse_community_buffer"

nit: does it makes sense to put this account separately ?
Either in the beginning or end of these contstants

Code quote:

ClearingAccountCommunityBuffer = "pse_community_buffer"

@metalarm10 made 3 comments and resolved 3 discussions.
Reviewable status: 21 of 35 files reviewed, all discussions resolved (waiting on akhlopiachyi, masihyeganeh, miladz68, TxCorpi0x, and ysv).

x/pse/keeper/delegation.go line 129 at r3 (raw file):

Done

x/pse/keeper/distribution.go line 92 at r3 (raw file):

Done

x/pse/types/params.go line 13 at r4 (raw file):

Done

@TxCorpi0x reviewed 14 files and all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on akhlopiachyi, masihyeganeh, miladz68, and ysv).

@ysv reviewed 14 files and all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on akhlopiachyi, masihyeganeh, and miladz68).