@ysv reviewed 23 of 23 files at r1, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @miladz68)

proto/tx/pse/v1/params.proto line 12 at r1 (raw file):

message Params {
  // excluded_addresses is a list of addresses excluded from PSE distribution.
  // This list includes module accounts and other addresses that should not receive PSE rewards.

good point about module accounts.
Should we exclude them by default (independently on this list) or add them to this list ?

Code quote:

  // This list includes module accounts and other addresses that should not receive PSE rewards.

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

	ctx context.Context,
	authority string,
	addressesToAdd, addressesToRemove []string,

what if

1. addressesToAdd contains duplicate ?
2. what if address is included in both ToAdd & ToRemove

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @miladz68 and @ysv)

proto/tx/pse/v1/params.proto line 12 at r1 (raw file):

We have two ways:

1. Append a list of addresses in the blockchain startup
2. Include the addresses in the list in the upgrade handlers

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

Two layers of validation in ValidateBasic and Params validation prevents all of wrong data scenarios, there are also tests in types and keeper packages.

@miladz68 reviewed 23 of 23 files at r1, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @ysv)

proto/tx/pse/v1/params.proto line 12 at r1 (raw file):

I don't think module accounts should be part of this param field. they should be dynamically appended on demand.

x/pse/module.go line 111 at r1 (raw file):

// InitGenesis performs genesis initialization for the module. It returns
// no validator updates.
func (am AppModule) InitGenesis(ctx sdk.Context, cdc codec.JSONCodec, data json.RawMessage) {

you should add genesis test.

Pull Request Overview

This PR implements a governance-controlled parameter system for the PSE module, specifically to manage a list of excluded addresses. The main changes replace a generic MsgUpdateParams with a more specific MsgUpdateExcludedAddresses message that allows governance to add and remove addresses from an exclusion list.

• Introduces a new Params protobuf message containing an excluded_addresses field
• Replaces MsgUpdateParams with MsgUpdateExcludedAddresses to handle incremental updates
• Implements keeper methods for parameter management and genesis state handling

Reviewed Changes

Copilot reviewed 25 out of 25 changed files in this pull request and generated 3 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Reviewable status: 18 of 25 files reviewed, 6 unresolved discussions (waiting on @copilot, @miladz68, and @ysv)

proto/tx/pse/v1/params.proto line 12 at r1 (raw file):

Removed from the comment.

x/pse/module.go line 111 at r1 (raw file):

Done

@ysv reviewed 3 of 7 files at r2, 2 of 2 files at r3, 2 of 2 files at r4, all commit messages.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @miladz68 and @TxCorpi0x)

proto/tx/pse/v1/params.proto line 12 at r1 (raw file):

so does it mean that we exclude module accounts inside code by default independently on this list ?
@miladz68 do you want to add this details to a task ?

Reviewable status: 24 of 25 files reviewed, 3 unresolved discussions (waiting on @miladz68 and @ysv)

@miladz68 reviewed 3 of 7 files at r2, 2 of 2 files at r3, 2 of 2 files at r4, 1 of 1 files at r5, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @TxCorpi0x and @ysv)

proto/tx/pse/v1/params.proto line 12 at r1 (raw file):

I will add a task to double check it and make sure we do not forget it.

@ysv reviewed 1 of 1 files at r5, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @TxCorpi0x)