diff --git a/.github/workflows/subscription-contract-ci.yml b/.github/workflows/subscription-contract-ci.yml new file mode 100644 index 0000000..d3a746a --- /dev/null +++ b/.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 diff --git a/contracts/subscription/README.md b/contracts/subscription/README.md index 1677ccc..b136abb 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 `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 +``` + +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..1b3db4f 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 = 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, + 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();