Pull Request Overview

This pull request introduces support for multiple recipients per clearing account in the PSE module's fund distribution system. Previously limited to a single recipient per clearing account, the system now supports one or more recipients with precise equal distribution of allocations and remainder handling to ensure no tokens are lost to rounding.

Key Changes:

• Updated protocol buffers to use repeated recipient_addresses instead of single recipient_address, with a new RecipientDistribution message for detailed event tracking
• Modified distribution logic to split allocations equally among recipients, with any remainder going to the first recipient to guarantee 100% distribution
• Enhanced validation to prevent Community clearing account mappings, check for empty recipient lists, and detect duplicate recipients within the same clearing account

Reviewed Changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 7 comments.

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

@ysv reviewed 1 of 6 files at r2, all commit messages.
Reviewable status: 1 of 13 files reviewed, 10 unresolved discussions (waiting on @masihyeganeh, @miladz68, and @TxCorpi0x)

proto/tx/pse/v1/event.proto line 10 at r2 (raw file):

// RecipientDistribution represents a single recipient and the amount they received.
message RecipientDistribution {

suggested name: AllocatedAmount

proto/tx/pse/v1/event.proto line 29 at r2 (raw file):

  // recipients contains the list of recipients and the exact amount each received.
  // The sum of all recipient amounts equals total_amount.
  repeated RecipientDistribution recipients = 2 [

I think field name should be different. Maybe allocated_amounts

x/pse/keeper/distribution.go line 156 at r2 (raw file):

			if i == 0 && !remainder.IsZero() {
				amountToSend = amountToSend.Add(remainder)
			}

I prefer to keep remainder undistributed. All account should be equally treated independently on order they have

Code quote:

			// Calculate amount for this recipient
			// First recipient gets the base amount plus any remainder to avoid dust
			amountToSend := amountPerRecipient
			if i == 0 && !remainder.IsZero() {
				amountToSend = amountToSend.Add(remainder)
			}

@miladz68 reviewed 7 of 13 files at r1, 6 of 6 files at r2, all commit messages.
Reviewable status: all files reviewed, 10 unresolved discussions (waiting on @masihyeganeh and @TxCorpi0x)

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

proto/tx/pse/v1/event.proto line 10 at r2 (raw file):

These are actually distributed (this is being used for the events after distribution is done), may be DistributedAmount

proto/tx/pse/v1/event.proto line 29 at r2 (raw file):

Or according to the previous comment, DistributedAmounts

x/pse/keeper/distribution.go line 156 at r2 (raw file):

Imaging 1000 and 3 participants, each get 333.33, we need to send the truncated values to the addresses (bank module send accepts Int), in that case the sum of the amounts would not get equal to the allocated values in the allocations.

@ysv reviewed 7 of 13 files at r1, 4 of 6 files at r2, 1 of 1 files at r3, all commit messages.
Reviewable status: all files reviewed, 10 unresolved discussions (waiting on @masihyeganeh and @TxCorpi0x)

proto/tx/pse/v1/event.proto line 10 at r2 (raw file):

Both AllocatedAmount & DistributedAmount are good

proto/tx/pse/v1/event.proto line 29 at r2 (raw file):

Both AllocatedAmount & DistributedAmount are good

x/pse/keeper/distribution.go line 156 at r2 (raw file):

yes , I think it is better to do it this way.
We should probably also suggest numbers like 4,5, 8 accounts so it is divisible. But IMO it is better to leave this remainder, @miladz68 WDYT ?

@miladz68 reviewed 1 of 1 files at r3, all commit messages.
Reviewable status: all files reviewed, 10 unresolved discussions (waiting on @masihyeganeh and @TxCorpi0x)

@ysv reviewed 6 of 6 files at r4, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @masihyeganeh and @TxCorpi0x)