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

x/pse/keeper/distribute.go line 70 at r1 (raw file):

		kv.Value.LastChangedUnixSec = currentBlockTime
		// TODO review all the logic for score reset
		key := collections.Join3(distributionID+1, kv.Key.K2(), kv.Key.K3())

distribution ids are validated to be sequential integers, right ?

Code quote:

distributionID+1

x/pse/keeper/genesis.go line 32 at r1 (raw file):

	// TODO revise this logic for distribution id and genesis state
	var currentDistributionID uint64

I didn't get this part

x/pse/keeper/hooks.go line 39 at r1 (raw file):

		return err
	}
	distributionID := distribution.ID // TODO update to use distribution ID

nit: this todo is not very clear for me.
I guess we can't just use distribution.ID instead but not sure why ?

Code quote:

// TODO update to use distribution ID

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

	AccountScoreSnapshot collections.Map[collections.Pair[uint64, sdk.AccAddress], sdkmath.Int]
	AllocationSchedule   collections.Map[uint64, types.ScheduledDistribution] // Map: id -> ScheduledDistribution
	// OngoingDistribution   collections.Item[uint64]

question: could you remind me what we decided here. Do we store OngoingDistribution inside store or query it from Schedule each time ?

(could be discussed on call)

x/pse/keeper/params.go line 125 at r1 (raw file):

// Removing DelegationTimeEntries ensures they start completely fresh when re-included.
// Entries will be recreated naturally when hooks fire after re-inclusion.
func (k Keeper) removeExcludedAccountData(ctx context.Context, addrStr string, distributionID uint64) error {

nit: distributionId should be 2nd arg I guess
To be consistent with other methods

x/pse/types/params.go line 133 at r1 (raw file):

	for i, period := range schedule {
		// TODO: this was uncommented to allow tests to pass.

lets discuss

@miladz68 made 5 comments and resolved 1 discussion.
Reviewable status: 14 of 18 files reviewed, 5 unresolved discussions (waiting on masihyeganeh, metalarm10, TxCorpi0x, and ysv).

x/pse/keeper/distribute.go line 70 at r1 (raw file):

correct.

x/pse/keeper/genesis.go line 32 at r1 (raw file):

this was a place holder to be fixed later once ID and genesis changes are finalized. But we can make those changes now.

x/pse/keeper/hooks.go line 39 at r1 (raw file):

added more context on what should be done.

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

we decided to leave it for later. John will decide at implementation, I will remove the commented code from this PR.

x/pse/keeper/params.go line 125 at r1 (raw file):

Done.

@metalarm10 made 3 comments.
Reviewable status: 12 of 21 files reviewed, 6 unresolved discussions (waiting on masihyeganeh, miladz68, TxCorpi0x, and ysv).

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

for context: I've added "OngoingDistribution" store in my branch as collections.Item[types.ScheduledDistribution] , because during multi-block processing, hooks need to know whether a distribution is actively being processed.

x/pse/types/params.go line 133 at r1 (raw file):

I think no need to comment out this validation since it was added and tested in pse task-1. tests should use ID: 1 instead of ID: 0.
also starting from 1 is intentional, proto3 defaults to 0 for unset fields, so 1-based IDs let us distinguish "not set" from "first entry.

x/pse/keeper/genesis.go line 34 at r2 (raw file):

	var currentDistributionID uint64
	if len(genState.ScheduledDistributions) > 0 {
		currentDistributionID = genState.ScheduledDistributions[0].Timestamp

this should be genState.ScheduledDistributions[0].IDi think.

@miladz68 made 3 comments.
Reviewable status: 12 of 21 files reviewed, 6 unresolved discussions (waiting on masihyeganeh, metalarm10, TxCorpi0x, and ysv).

x/pse/keeper/genesis.go line 34 at r2 (raw file):

this no longer applies. removed the code.

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

great, in that case I will comment it out to be fixed by your PR.

x/pse/types/params.go line 133 at r1 (raw file):

starting from 0 will break the tests, we need to decide what to do with the scenario where delegations are present but no scheduled distribution exists, and then we add the distribution.

@ysv resolved 5 discussions.
Reviewable status: 12 of 21 files reviewed, 1 unresolved discussion (waiting on masihyeganeh, metalarm10, and TxCorpi0x).

@metalarm10 resolved 1 discussion.
Reviewable status: 11 of 21 files reviewed, all discussions resolved (waiting on masihyeganeh, TxCorpi0x, and ysv).

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