fix(billing): sell a VPN plan inactive, and purge abandoned orders - #413
Merged
Conversation
A VPN plan's subscription was created is_active = 1, so an unpaid plan was reported as an active subscription while billing_state on the same object said unpaid. Every other product creates its subscription inactive and lets payment set both flags. No service was configured early -- the peer set joins through is_setup -- but two fields describing one fact were free to disagree, and one of them was wrong. Nothing ever revisited a subscription whose first payment never arrived. The lifecycle sweep reads list_lifecycle_subscriptions, which is active-with-an-expiry, and an abandoned checkout is neither, so it lived in the database for good. For a VPN plan that is worse than clutter: create_vpn_plan returns an existing plan whenever its state is not Expired, and Unpaid is not Expired, so the customer stayed pinned to it at the price it was created at. check_never_paid_subscriptions purges them 24h after creation, keeping any whose invoice is unexpired or whose on-chain deposit has been detected but not confirmed (#194). Never paid means never provisioned for every product -- the operator's gate gives an Unpaid deployment no cluster objects at all -- so the product rows go with the order: an app deployment, and a VPN plan with its devices and keys. A marketplace node keeps its registration and loses only the billing link, the unpaid order being the listing fee rather than the node. VMs are left alone; check_vms already deletes never-paid ones on its own shorter clock, and it has a hypervisor to talk to.
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.
Two bugs, both in how an order that has not been paid for is represented.
A VPN plan was reported active before it was paid for
create_vpn_planbuilt its subscription withis_active: true. Every other creation path in the codebase sets it false with the comment "Inactive until first payment" (api/subscriptions.rs,api/apps.rs,api/marketplace.rs), and payment setsis_activeandis_setuptogether insubscription_payment_paid.Nothing leaked: the peer set query joins through
is_setupas well, so no device reached a route server. What was wrong was the reporting. An unpaid plan appeared inGET /api/v1/subscriptionsand serialisedis_active: truewhilebilling_stateon the same object saidunpaid.Abandoned orders were never cleaned up
check_subscriptionsreadslist_lifecycle_subscriptions, which isis_active = 1 AND expires IS NOT NULL. A never-paid subscription has a NULL expiry and (now)is_active = 0, so both filters exclude it and nothing ever looked at it again. Only VMs escaped this, becausecheck_vmsscans thevmtable and deletes never-paid ones after an hour.For VPN it also pinned the customer:
create_vpn_planreturns an existing plan whenever its state is notExpired, andUnpaidis notExpired, so an abandoned plan was the only one they could ever have, at the price it was created at.Worker::check_never_paid_subscriptionsruns at the tail ofcheck_subscriptionsover a newlist_never_paid_subscriptions(older_than_seconds)— the complement of the lifecycle query. Each candidate is re-read (a payment may have settled since the snapshot) and skipped while an invoice is unexpired or an on-chain deposit has been detected but not confirmed, reusing the #194 helper. It is renamedpayment_blocks_unpaid_deletionsince it is no longer VM-only.Never paid means never provisioned for every product —
provisions_cluster_objectsgives aBillingState::Unpaiddeployment no Kubernetes objects at all — so there is nothing running to tear down, only rows:Vpscheck_vmsowns it, on a shorter clock and with a hypervisor to talk toVpndelete_vpn_subscription)Apphard_delete_app_deployment, which takes the subscription with itMarketplaceNodeFeeIpRange/AsnSponsoring/DnsHostingTTL is 24h. A lightning invoice has expired many times over by then, but a bank transfer or a late on-chain send has not, and being early means deleting an order the customer is still paying for.
Tests
Nine added: the
is_activeregression, and eight covering the sweep (purge past the TTL, kept within it, kept with a pending payment, VM left alone, VPN plan purged with its devices and tunnels, marketplace node kept and unlinked, app deployment purged, and the re-read that lets a payment settling mid-sweep win).Full workspace suite passes; clippy warning count unchanged. Note the two new MySQL queries are exercised only through
MockDbin unit tests, as is the rest ofmysql.rs.