Summary
Three related bugs in contracts/subscription affecting subscriber lifecycle correctness, fund safety, and scalability.
Bug 1: Trial-only subscribers permanently blocked from re-subscribing after trial expires
Affected function: subscribe()
Description:
A subscriber who uses a free trial with pay_upfront = false (trial-only path — no payment, no approval) has a SubServicePair written to storage. After the trial expires, calling subscribe() again on the same service returns AlreadySubscribed because the dedup check does not distinguish between an active subscription and an expired trial-only one.
Steps to reproduce:
- Register a service with
trial_period_secs > 0
- Call
subscribe(subscriber, service_id, pay_upfront = false) — trial starts, no payment made
- Wait for
trial_period_secs to elapse (subscription expires)
- Call
subscribe(subscriber, service_id, pay_upfront = true)
Expected: new subscription is created
Actual: AlreadySubscribed error — stale SubServicePair blocks re-subscription
Suggested fix: The dedup check should also verify that the existing subscription has not expired (i.e., service_end_ts > now). If the prior subscription is expired, allow a new one and clean up the stale pair.
Bug 2: cancel() does not revoke the token allowance
Affected function: cancel()
Description:
When a subscriber cancels, the contract sets pay_upfront = false but never calls approve(subscriber, contract, 0) to zero out the token allowance. The pre-approved allowance (up to approve_periods × price) remains live on the token contract. Any party with knowledge of the allowance can call token.transfer_from(subscriber, recipient, amount) directly — bypassing process() entirely — and drain funds up to the approved amount even after cancellation.
Steps to reproduce:
subscribe(subscriber, service_id, pay_upfront = true, approve_periods = 12)- Immediately call
cancel(subscriber, sub_id)
- Externally call
token.transfer_from(subscriber, merchant, price * 12) on the token contract directly
Expected: transfer fails or allowance is zero after cancel
Actual: transfer succeeds — full original allowance is still live
Suggested fix: cancel() should set the subscriber's allowance to 0 via token.approve(subscriber, contract_address, 0, ...) before returning.
Bug 3: process() has no pagination — unusable for large subscriber sets
Affected function: process()
Description:
The data flow diagram in the spec shows process(offset, limit) with paginated iteration, but the actual function signature is process(merchant, service_id) with no pagination. For services with a large number of subscribers, the function iterates all ServiceSubs in a single transaction, which will exhaust Soroban's per-transaction instruction budget and cause the call to abort — leaving no subscribers charged.
Steps to reproduce:
- Register a service
- Subscribe 500+ accounts with
pay_upfront = true
- Advance time past
next_charge_ts
- Call
process(merchant, service_id)
Expected: subscriptions charged in paginated batches
Actual: transaction likely aborts due to instruction budget exhaustion; zero subscriptions charged
Suggested fix: Add offset: u32 and limit: u32 parameters to process(), consistent with the spec flowchart. Merchants call in a loop until ProcessResult.total == 0 or charged + failed + skipped < limit.
Environment
- Repo:
PiNetwork/SmartContracts
- Contract:
contracts/subscription
- SDK:
soroban-sdk = "22.0.0"
- Spec reference:
PiNetwork/PiRC — PiRC2
Summary
Three related bugs in
contracts/subscriptionaffecting subscriber lifecycle correctness, fund safety, and scalability.Bug 1: Trial-only subscribers permanently blocked from re-subscribing after trial expires
Affected function:
subscribe()Description:
A subscriber who uses a free trial with
pay_upfront = false(trial-only path — no payment, no approval) has aSubServicePairwritten to storage. After the trial expires, callingsubscribe()again on the same service returnsAlreadySubscribedbecause the dedup check does not distinguish between an active subscription and an expired trial-only one.Steps to reproduce:
trial_period_secs > 0subscribe(subscriber, service_id, pay_upfront = false)— trial starts, no payment madetrial_period_secsto elapse (subscription expires)subscribe(subscriber, service_id, pay_upfront = true)Expected: new subscription is created
Actual:
AlreadySubscribederror — staleSubServicePairblocks re-subscriptionSuggested fix: The dedup check should also verify that the existing subscription has not expired (i.e.,
service_end_ts > now). If the prior subscription is expired, allow a new one and clean up the stale pair.Bug 2:
cancel()does not revoke the token allowanceAffected function:
cancel()Description:
When a subscriber cancels, the contract sets
pay_upfront = falsebut never callsapprove(subscriber, contract, 0)to zero out the token allowance. The pre-approved allowance (up toapprove_periods × price) remains live on the token contract. Any party with knowledge of the allowance can calltoken.transfer_from(subscriber, recipient, amount)directly — bypassingprocess()entirely — and drain funds up to the approved amount even after cancellation.Steps to reproduce:
subscribe(subscriber, service_id, pay_upfront = true, approve_periods = 12)cancel(subscriber, sub_id)token.transfer_from(subscriber, merchant, price * 12)on the token contract directlyExpected: transfer fails or allowance is zero after cancel
Actual: transfer succeeds — full original allowance is still live
Suggested fix:
cancel()should set the subscriber's allowance to 0 viatoken.approve(subscriber, contract_address, 0, ...)before returning.Bug 3:
process()has no pagination — unusable for large subscriber setsAffected function:
process()Description:
The data flow diagram in the spec shows
process(offset, limit)with paginated iteration, but the actual function signature isprocess(merchant, service_id)with no pagination. For services with a large number of subscribers, the function iterates allServiceSubsin a single transaction, which will exhaust Soroban's per-transaction instruction budget and cause the call to abort — leaving no subscribers charged.Steps to reproduce:
pay_upfront = truenext_charge_tsprocess(merchant, service_id)Expected: subscriptions charged in paginated batches
Actual: transaction likely aborts due to instruction budget exhaustion; zero subscriptions charged
Suggested fix: Add
offset: u32andlimit: u32parameters toprocess(), consistent with the spec flowchart. Merchants call in a loop untilProcessResult.total == 0orcharged + failed + skipped < limit.Environment
PiNetwork/SmartContractscontracts/subscriptionsoroban-sdk = "22.0.0"PiNetwork/PiRC— PiRC2