Skip to content

Comments

[PSE] Use incremental ID for ScheduledDistributions#91

Open
metalarm10 wants to merge 14 commits intomasterfrom
john/pse-incremental-distribution-id
Open

[PSE] Use incremental ID for ScheduledDistributions#91
metalarm10 wants to merge 14 commits intomasterfrom
john/pse-incremental-distribution-id

Conversation

@metalarm10
Copy link
Contributor

@metalarm10 metalarm10 commented Feb 23, 2026

Description

This PR closes: https://app.clickup.com/t/868hj36t9

  • Add sequential id field to ScheduledDistribution
  • Change AllocationSchedule storage key from timestamp to id
  • Add min_distribution_gap_seconds governance param (default: 1 day) — enforces a minimum time gap between consecutive distribution timestamps
  • Add MsgUpdateMinDistributionGap governance message to update the gap param; validates the proposed gap against the existing on-chain schedule
  • Add validation in ValidateDistributionSchedule: IDs must be non-zero and strictly sequential (each = previous + 1)
  • Add ValidateDistributionGap to enforce minimum time gap between every consecutive pair of distribution timestamps (called during both genesis validation and governance schedule updates)

Reviewers checklist:

  • Try to write more meaningful comments with clear actions to be taken.
  • Nit-picking should be unblocking. Focus on core issues.

Authors checklist

  • Provide a concise and meaningful description
  • Review the code yourself first, before making the PR.
  • Annotate your PR in places that require explanation.
  • Think and try to split the PR to smaller PR if it is big.

This change is Reviewable

@metalarm10 metalarm10 requested a review from a team as a code owner February 23, 2026 11:20
@metalarm10 metalarm10 requested review from TxCorpi0x, masihyeganeh, miladz68 and ysv and removed request for a team February 23, 2026 11:20
Copy link
Contributor

@miladz68 miladz68 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@miladz68 reviewed 23 files and all commit messages, and made 1 comment.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on masihyeganeh, metalarm10, TxCorpi0x, and ysv).


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

				"period %d: id must be sequential, expected %d but got %d",
				i, schedule[i-1].Id+1, period.Id)
		}

I guess we should also add the check to that the current timestamp is greater than the previous timestamp (i.e monotonically increasing).
I think there was already such a check somewhere, can you check where it was and if it makes sense to merge them ? let's discuss on call.

Copy link
Contributor

@miladz68 miladz68 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@miladz68 made 1 comment.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on masihyeganeh, metalarm10, TxCorpi0x, and ysv).


proto/tx/pse/v1/distribution.proto line 61 at r1 (raw file):

  // id is the unique, sequential identifier for this distribution.
  // Used as the storage key in the AllocationSchedule map.
  uint64 id = 3 [

you should add proto tags such that the golang structs are formed as ID not Id. There are examples in the code base.

Copy link
Contributor

@miladz68 miladz68 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@miladz68 reviewed 1 file and all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on masihyeganeh, metalarm10, TxCorpi0x, and ysv).

Copy link
Contributor Author

@metalarm10 metalarm10 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@metalarm10 made 2 comments.
Reviewable status: 13 of 24 files reviewed, 2 unresolved discussions (waiting on masihyeganeh, miladz68, TxCorpi0x, and ysv).


proto/tx/pse/v1/distribution.proto line 61 at r1 (raw file):

Previously, miladz68 (milad) wrote…

you should add proto tags such that the golang structs are formed as ID not Id. There are examples in the code base.

Done.


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

Previously, miladz68 (milad) wrote…

I guess we should also add the check to that the current timestamp is greater than the previous timestamp (i.e monotonically increasing).
I think there was already such a check somewhere, can you check where it was and if it makes sense to merge them ? let's discuss on call.

Done.

@metalarm10 metalarm10 requested a review from miladz68 February 23, 2026 13:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants