Harden subscription batching and query pagination

.github/workflows/subscription-contract-ci.yml

@@ -0,0 +1,44 @@
+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: 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

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 `25` subscriptions per call. Passing a larger `limit` processes only the first `25` 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<SubscriptionPage, ContractError>
+```
+
+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`

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 = 25;
+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<Subscription>,
+    pub total: u32,
+}
+
 // ---------------------------------------------------------------------------
 // Contract
 // ---------------------------------------------------------------------------
@@ -217,6 +226,26 @@ fn do_approve(
     Ok(())
 }
 
+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);
+
+    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<u64> = 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<SubscriptionPage, ContractError> {
+        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<u64> = 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<Service, ContractError> {
         let svc_key = DataKey::Service(service_id);
         env.storage()

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();