From 9064f8a1811e88763f3bc6f1ae2d151c984fdeba Mon Sep 17 00:00:00 2001 From: GitHub Copilot Date: Wed, 22 Apr 2026 13:12:32 -0400 Subject: [PATCH 1/4] Harden subscription batching and queries --- contracts/subscription/README.md | 36 +++++++++++- contracts/subscription/src/lib.rs | 88 +++++++++++++++++++++++++++++- contracts/subscription/src/test.rs | 74 +++++++++++++++++++++++++ 3 files changed, 196 insertions(+), 2 deletions(-) diff --git a/contracts/subscription/README.md b/contracts/subscription/README.md index 1677ccc..031d7f3 100644 --- a/contracts/subscription/README.md +++ b/contracts/subscription/README.md @@ -27,7 +27,7 @@ The contract implements a merchant-initiated pull-payment model: Key design properties: - **No charge drift** -- `next_charge_ts` is always computed as `old_next_charge_ts + period_secs`, not from wall-clock time. -- **Paginated batch processing** -- `process()` accepts `offset` and `limit` parameters to handle large subscriber sets across multiple transactions. +- **Paginated batch processing** -- `process()` accepts `offset` and `limit` parameters and enforces a hard maximum page size per call to handle large subscriber sets across multiple transactions. - **Graceful failure isolation** -- failed charges and timestamp overflows disable auto-renewal (`auto_renew = false`) for the individual subscription instead of reverting the entire batch. - **Deduplication** -- a subscriber cannot hold two active subscriptions to the same service simultaneously. - **Trial abuse prevention** -- a subscriber who already used a free trial cannot re-subscribe without `auto_renew=true`, preventing infinite free trials. @@ -138,6 +138,7 @@ Creates a new subscription service. - `period_secs > 0` -> `InvalidPeriod` - `approve_periods > 0` -> `InvalidPeriod` - `name` non-empty -> `InvalidServiceName` +- approval window arithmetic must fit in the contract's timestamp and token amount calculations -> `TimestampOverflow` **Effects:** - Stores `Service` under `DataKey::Service(service_id)` @@ -269,6 +270,8 @@ Batch-charges due subscriptions for a service. This is the core billing function **Pagination:** Soroban transactions have finite resource limits (read/write ledger entries and bytes per transaction). Large subscriber sets must be processed in batches using `offset` and `limit`. The returned `ProcessResult.total` indicates the total number of subscriptions, allowing the caller to determine how many batches are needed. +`process()` also enforces a hard page-size cap of `100` subscriptions per call. Passing a larger `limit` processes only the first `100` entries in that window. + **Example batch loop:** ``` // Process 20 subscriptions per transaction @@ -351,6 +354,21 @@ Returns all subscriptions for a subscriber. Gracefully skips subscriptions that **Auth:** `subscriber` +#### `get_subscriber_subs_paginated` + +```rust +fn get_subscriber_subs_paginated( + env: Env, + subscriber: Address, + offset: u32, + limit: u32, +) -> SubscriptionPage +``` + +Returns one page of subscriptions for a subscriber plus the total result count. `limit` is capped at `100` per call. + +**Auth:** `subscriber` + --- #### `get_merchant_subs` @@ -367,6 +385,22 @@ Returns all subscriptions for a service. Gracefully skips subscriptions that hav **Auth:** `merchant` (must own the service) +#### `get_merchant_subs_paginated` + +```rust +fn get_merchant_subs_paginated( + env: Env, + merchant: Address, + service_id: u64, + offset: u32, + limit: u32, +) -> Result +``` + +Returns one page of subscriptions for a service plus the total result count. `limit` is capped at `100` per call. + +**Auth:** `merchant` (must own the service) + --- #### `is_subscription_active` diff --git a/contracts/subscription/src/lib.rs b/contracts/subscription/src/lib.rs index e0656ca..929c73e 100644 --- a/contracts/subscription/src/lib.rs +++ b/contracts/subscription/src/lib.rs @@ -13,6 +13,8 @@ const INSTANCE_TTL_EXTEND: u32 = 518_400; // ~30 days const PERSISTENT_TTL_THRESHOLD: u32 = 17_280; const PERSISTENT_TTL_EXTEND_MIN: u32 = 518_400; // ~30 days floor const SECS_PER_LEDGER: u64 = 5; +const MAX_PROCESS_BATCH_SIZE: u32 = 100; +const MAX_QUERY_PAGE_SIZE: u32 = 100; // --------------------------------------------------------------------------- // Errors @@ -97,6 +99,13 @@ pub struct ProcessResult { pub total: u32, } +#[derive(Clone, PartialEq, Debug)] +#[contracttype] +pub struct SubscriptionPage { + pub subscriptions: Vec, + pub total: u32, +} + // --------------------------------------------------------------------------- // Contract // --------------------------------------------------------------------------- @@ -217,6 +226,26 @@ fn do_approve( Ok(()) } +fn paginate_subscriptions(env: &Env, sub_ids: &Vec, 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); + + let mut subscriptions = Vec::new(env); + for i in start..end { + let sid = sub_ids.get(i).unwrap(); + if let Some(sub) = env.storage().persistent().get::<_, Subscription>(&DataKey::Sub(sid)) { + subscriptions.push_back(sub); + } + } + + SubscriptionPage { + subscriptions, + total, + } +} + // --------------------------------------------------------------------------- // Implementation // --------------------------------------------------------------------------- @@ -256,6 +285,14 @@ impl SubscriptionContract { return Err(ContractError::InvalidPeriod); } + period_secs + .checked_mul(approve_periods) + .and_then(|secs| secs.checked_add(trial_period_secs)) + .ok_or(ContractError::TimestampOverflow)?; + price + .checked_mul(approve_periods as i128) + .ok_or(ContractError::TimestampOverflow)?; + merchant.require_auth(); let service_id = next_service_id(&env); @@ -655,7 +692,8 @@ impl SubscriptionContract { let total = sub_ids.len(); let start = offset.min(total); - let end = start.saturating_add(limit).min(total); + let capped_limit = core::cmp::min(limit, MAX_PROCESS_BATCH_SIZE); + let end = start.saturating_add(capped_limit).min(total); let mut charged: u32 = 0; let mut failed: u32 = 0; @@ -822,6 +860,24 @@ impl SubscriptionContract { result } + pub fn get_subscriber_subs_paginated( + env: Env, + subscriber: Address, + offset: u32, + limit: u32, + ) -> SubscriptionPage { + subscriber.require_auth(); + + let ss_key = DataKey::SubscriberSubs(subscriber); + let sub_ids: Vec = env + .storage() + .persistent() + .get(&ss_key) + .unwrap_or_else(|| Vec::new(&env)); + + paginate_subscriptions(&env, &sub_ids, offset, limit) + } + pub fn get_merchant_subs( env: Env, merchant: Address, @@ -859,6 +915,36 @@ impl SubscriptionContract { Ok(result) } + pub fn get_merchant_subs_paginated( + env: Env, + merchant: Address, + service_id: u64, + offset: u32, + limit: u32, + ) -> Result { + merchant.require_auth(); + + let svc_key = DataKey::Service(service_id); + let service: Service = env + .storage() + .persistent() + .get(&svc_key) + .ok_or(ContractError::ServiceNotFound)?; + + if service.merchant != merchant { + return Err(ContractError::NotServiceOwner); + } + + let svc_subs_key = DataKey::ServiceSubs(service_id); + let sub_ids: Vec = env + .storage() + .persistent() + .get(&svc_subs_key) + .unwrap_or_else(|| Vec::new(&env)); + + Ok(paginate_subscriptions(&env, &sub_ids, offset, limit)) + } + pub fn get_service(env: Env, service_id: u64) -> Result { let svc_key = DataKey::Service(service_id); env.storage() diff --git a/contracts/subscription/src/test.rs b/contracts/subscription/src/test.rs index ee269b2..95fc27a 100644 --- a/contracts/subscription/src/test.rs +++ b/contracts/subscription/src/test.rs @@ -155,6 +155,20 @@ fn test_register_service_invalid_name() { assert_eq!(result, Err(Ok(ContractError::InvalidServiceName))); } +#[test] +fn test_register_service_timestamp_overflow() { + let s = setup(); + let result = s.client.try_register_service( + &s.merchant, + &String::from_str(&s.env, "Overflow"), + &PRICE, + &u64::MAX, + &1, + &2, + ); + assert_eq!(result, Err(Ok(ContractError::TimestampOverflow))); +} + #[test] fn test_register_multiple_services() { let s = setup(); @@ -812,6 +826,26 @@ fn test_process_skips_no_auto_renew() { assert_eq!(s.token.balance(&s.subscriber), INITIAL_BALANCE - PRICE); } +#[test] +fn test_process_caps_batch_size() { + let s = setup(); + let svc = register_default_service(&s); + + for _ in 0..(MAX_PROCESS_BATCH_SIZE + 1) { + let subscriber = Address::generate(&s.env); + s.token_admin.mint(&subscriber, &INITIAL_BALANCE); + s.client.subscribe(&subscriber, &svc.service_id, &true); + s.token + .approve(&subscriber, &s.contract_addr, &INITIAL_BALANCE, &10000); + } + + advance_time(&s.env, MONTH + 1); + + let result = s.client.process(&s.merchant, &svc.service_id, &0, &u32::MAX); + assert_eq!(result.charged, MAX_PROCESS_BATCH_SIZE); + assert_eq!(result.total, MAX_PROCESS_BATCH_SIZE + 1); +} + // =========================================================================== // Access Control // =========================================================================== @@ -868,6 +902,30 @@ fn test_get_subscriber_subs() { assert_eq!(subs.len(), 2); } +#[test] +fn test_get_subscriber_subs_paginated() { + let s = setup(); + let svc1 = register_default_service(&s); + let svc2 = s.client.register_service( + &s.merchant2, + &String::from_str(&s.env, "Other"), + &500, + &WEEK, + &0, + &12, + ); + + s.client.subscribe(&s.subscriber, &svc1.service_id, &true); + s.client.subscribe(&s.subscriber, &svc2.service_id, &true); + + let page = s + .client + .get_subscriber_subs_paginated(&s.subscriber, &1, &1); + assert_eq!(page.total, 2); + assert_eq!(page.subscriptions.len(), 1); + assert_eq!(page.subscriptions.get(0).unwrap().service_id, svc2.service_id); +} + #[test] fn test_get_merchant_subs() { let s = setup(); @@ -880,6 +938,22 @@ fn test_get_merchant_subs() { assert_eq!(subs.len(), 2); } +#[test] +fn test_get_merchant_subs_paginated() { + let s = setup(); + let svc = register_default_service(&s); + + s.client.subscribe(&s.subscriber, &svc.service_id, &true); + s.client.subscribe(&s.subscriber2, &svc.service_id, &true); + + let page = s + .client + .get_merchant_subs_paginated(&s.merchant, &svc.service_id, &1, &1); + assert_eq!(page.total, 2); + assert_eq!(page.subscriptions.len(), 1); + assert_eq!(page.subscriptions.get(0).unwrap().subscriber, s.subscriber2); +} + #[test] fn test_get_merchant_subs_wrong_merchant() { let s = setup(); From 90daabeeb202d6542d8c5463c9d4a4ba87b90c42 Mon Sep 17 00:00:00 2001 From: GitHub Copilot Date: Wed, 22 Apr 2026 13:32:22 -0400 Subject: [PATCH 2/4] Add CI workflow for subscription contract tests --- .../workflows/subscription-contract-ci.yml | 38 +++++++++++++++++++ 1 file changed, 38 insertions(+) create mode 100644 .github/workflows/subscription-contract-ci.yml diff --git a/.github/workflows/subscription-contract-ci.yml b/.github/workflows/subscription-contract-ci.yml new file mode 100644 index 0000000..0fbf706 --- /dev/null +++ b/.github/workflows/subscription-contract-ci.yml @@ -0,0 +1,38 @@ +name: Subscription Contract CI + +on: + pull_request: + paths: + - "contracts/subscription/**" + - "Cargo.toml" + - "Cargo.lock" + - ".github/workflows/subscription-contract-ci.yml" + push: + branches: + - main + - fix/subscription-batch-and-pagination + paths: + - "contracts/subscription/**" + - "Cargo.toml" + - "Cargo.lock" + - ".github/workflows/subscription-contract-ci.yml" + +permissions: + contents: read + +jobs: + test-subscription: + name: Test subscription contract + runs-on: ubuntu-latest + steps: + - name: Checkout + uses: actions/checkout@v4 + + - name: Install Rust toolchain + uses: dtolnay/rust-toolchain@stable + + - name: Cache cargo artifacts + uses: swatinem/rust-cache@v2 + + - name: Run tests + run: cargo test -p subscription From aca9ffdd5e98d93f0a180f4ef86a0a466220e4cb Mon Sep 17 00:00:00 2001 From: GitHub Copilot Date: Wed, 22 Apr 2026 14:07:07 -0400 Subject: [PATCH 3/4] Build wasm artifact before subscription tests --- .github/workflows/subscription-contract-ci.yml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/.github/workflows/subscription-contract-ci.yml b/.github/workflows/subscription-contract-ci.yml index 0fbf706..d3a746a 100644 --- a/.github/workflows/subscription-contract-ci.yml +++ b/.github/workflows/subscription-contract-ci.yml @@ -34,5 +34,11 @@ jobs: - name: Cache cargo artifacts uses: swatinem/rust-cache@v2 + - name: Add wasm target + run: rustup target add wasm32v1-none + + - name: Build subscription wasm + run: cargo build -p subscription --target wasm32v1-none --release + - name: Run tests run: cargo test -p subscription From 13209735f1ef0f073cab162065dd1008ca2ec89d Mon Sep 17 00:00:00 2001 From: GitHub Copilot Date: Wed, 22 Apr 2026 14:29:02 -0400 Subject: [PATCH 4/4] Tune process batch cap for Soroban budget limits --- contracts/subscription/README.md | 2 +- contracts/subscription/src/lib.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/contracts/subscription/README.md b/contracts/subscription/README.md index 031d7f3..b136abb 100644 --- a/contracts/subscription/README.md +++ b/contracts/subscription/README.md @@ -270,7 +270,7 @@ Batch-charges due subscriptions for a service. This is the core billing function **Pagination:** Soroban transactions have finite resource limits (read/write ledger entries and bytes per transaction). Large subscriber sets must be processed in batches using `offset` and `limit`. The returned `ProcessResult.total` indicates the total number of subscriptions, allowing the caller to determine how many batches are needed. -`process()` also enforces a hard page-size cap of `100` subscriptions per call. Passing a larger `limit` processes only the first `100` entries in that window. +`process()` also enforces a hard page-size cap of `25` subscriptions per call. Passing a larger `limit` processes only the first `25` entries in that window. **Example batch loop:** ``` diff --git a/contracts/subscription/src/lib.rs b/contracts/subscription/src/lib.rs index 929c73e..1b3db4f 100644 --- a/contracts/subscription/src/lib.rs +++ b/contracts/subscription/src/lib.rs @@ -13,7 +13,7 @@ const INSTANCE_TTL_EXTEND: u32 = 518_400; // ~30 days const PERSISTENT_TTL_THRESHOLD: u32 = 17_280; const PERSISTENT_TTL_EXTEND_MIN: u32 = 518_400; // ~30 days floor const SECS_PER_LEDGER: u64 = 5; -const MAX_PROCESS_BATCH_SIZE: u32 = 100; +const MAX_PROCESS_BATCH_SIZE: u32 = 25; const MAX_QUERY_PAGE_SIZE: u32 = 100; // ---------------------------------------------------------------------------