Harden subscription batching and query pagination#11
Open
jdrains110-beep wants to merge 4 commits intoPiNetwork:mainfrom
Open
Harden subscription batching and query pagination#11jdrains110-beep wants to merge 4 commits intoPiNetwork:mainfrom
jdrains110-beep wants to merge 4 commits intoPiNetwork:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Hardens the Soroban subscription contract against unbounded batch/query sizes and shifts overflow validation earlier to make scaling behavior safer and more predictable for clients.
Changes:
- Enforce a hard
process()batch cap (100) regardless of caller-providedlimit. - Add paginated query APIs for subscriber/service subscription listings, with a hard page-size cap (100).
- Reject approval-window arithmetic overflow during
register_service()and expand tests/docs accordingly.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
contracts/subscription/src/lib.rs |
Adds batch/page caps, early overflow validation, and new paginated query endpoints. |
contracts/subscription/src/test.rs |
Adds tests for early overflow rejection, batch cap enforcement, and basic pagination behavior. |
contracts/subscription/README.md |
Documents the new caps and paginated query APIs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+229
to
+233
| fn paginate_subscriptions(env: &Env, sub_ids: &Vec<u64>, offset: u32, limit: u32) -> SubscriptionPage { | ||
| let total = sub_ids.len(); | ||
| let capped_limit = core::cmp::min(limit, MAX_QUERY_PAGE_SIZE); | ||
| let start = offset.min(total); | ||
| let end = start.saturating_add(capped_limit).min(total); |
Comment on lines
+292
to
+294
| price | ||
| .checked_mul(approve_periods as i128) | ||
| .ok_or(ContractError::TimestampOverflow)?; |
| Ok(()) | ||
| } | ||
|
|
||
| fn paginate_subscriptions(env: &Env, sub_ids: &Vec<u64>, offset: u32, limit: u32) -> SubscriptionPage { |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR hardens the subscription contract's scaling behavior and adds a safer read path for large subscription sets.
What changed
process()to a contract-enforced maximum batch size of 100 entries per callregister_service()instead of deferring it to later lifecycle callsget_subscriber_subs_paginated()andget_merchant_subs_paginated()so clients can page through large result sets without relying on unbounded vectorsWhy
The previous API surface depended on caller discipline for batch and query sizing. That makes
process()and the listing APIs harder to use safely as subscription counts grow.Closes #10
Validation
contracts/subscription/src/lib.rscontracts/subscription/src/test.rscargo testcould not be run here becausecargois not installed in this environment